[pve-devel] [PATCH series 0/5] removal of directories in PBS WebUI

[pve-devel] [PATCH series 0/5] removal of directories in PBS WebUI

[Hannes Laimer]

Added functionallity to remove directories in the PBS WebUI. In order to do that SafeDestroy had to be refactored from pve-manager into proxmox-widget-toolkit and the possibility to show a small note in the dialog was added. Due to the refactorization the usages of SafeDestroy in pve-manager had to be corrected. In order to avoid the extraction of the directory name from the path in the frontend, the api2 now also returns the name of a directory.   

proxmox-widget-toolkit needs a version bump, pve-manager and proxmox-backup require that new version.


proxmox-widget-toolkit: Hannes Laimer (2):
  ui refactoring: refactored SafeDestroy from pve-manager into
    proxmox-widget-toolkit
  ui: added possibility to show a small note in SafeRemove dialog

 src/Makefile             |   1 +
 src/window/SafeRemove.js | 221 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 222 insertions(+)
 create mode 100644 src/window/SafeRemove.js


pve-manager: Hannes Laimer (1):
  ui refactoring: SafeDestroy moved into widgettoolkit + adjusted usages

 www/manager6/Makefile               |   1 -
 www/manager6/ceph/Pool.js           |   2 +-
 www/manager6/lxc/Config.js          |   2 +-
 www/manager6/qemu/Config.js         |   2 +-
 www/manager6/storage/ContentView.js |   2 +-
 www/manager6/window/SafeDestroy.js  | 194 ----------------------------
 6 files changed, 4 insertions(+), 199 deletions(-)
 delete mode 100644 www/manager6/window/SafeDestroy.js


proxmox-backup: Hannes Laimer (2):
  api2: DatastoreMountInfo now also contains the name of the mount
  ui: added possiblity to remove directories/mount-units in the WebUI

 src/api2/node/disks/directory.rs |  5 +++++
 www/DirectoryList.js             | 22 ++++++++++++++++++++++
 2 files changed, 27 insertions(+)

-- 
2.20.1

[pve-devel] [PATCH proxmox-widget-toolkit 1/5] ui refactoring: refactored SafeDestroy from pve-manager into proxmox-widget-toolkit

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/Makefile             |   1 +
 src/window/SafeRemove.js | 193 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 194 insertions(+)
 create mode 100644 src/window/SafeRemove.js

diff --git a/src/Makefile b/src/Makefile
index 12dda30..e7da4eb 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -43,6 +43,7 @@ JSSRC=					\
 	panel/GaugeWidget.js		\
 	window/Edit.js			\
 	window/PasswordEdit.js		\
+	window/SafeRemove.js		\
 	window/TaskViewer.js		\
 	window/LanguageEdit.js		\
 	window/DiskSmart.js		\
diff --git a/src/window/SafeRemove.js b/src/window/SafeRemove.js
new file mode 100644
index 0000000..c541272
--- /dev/null
+++ b/src/window/SafeRemove.js
@@ -0,0 +1,193 @@
+/* Popup a message window
+ * where the user has to manually enter the resource ID
+ * to enable the destroy button
+ */
+Ext.define('Proxmox.window.SafeRemove', {
+    extend: 'Ext.window.Window',
+    alias: 'proxmoxSafeRemove',
+
+    title: gettext('Confirm'),
+    modal: true,
+    buttonAlign: 'center',
+    bodyPadding: 10,
+    width: 450,
+    layout: {type: 'hbox'},
+    defaultFocus: 'confirmField',
+    showProgress: false,
+
+    config: {
+        item: {
+            id: undefined,
+            type: undefined
+        },
+        url: undefined,
+        params: {}
+    },
+
+    getParams: function () {
+        const me = this;
+        const purgeCheckbox = me.lookupReference('purgeCheckbox');
+        if (purgeCheckbox.checked) {
+            me.params.purge = 1;
+        }
+        if (Ext.Object.isEmpty(me.params)) {
+            return '';
+        }
+        return '?' + Ext.Object.toQueryString(me.params);
+    },
+
+    controller: {
+        xclass: 'Ext.app.ViewController',
+        control: {
+            'field[name=confirm]': {
+                change: function (f, value) {
+                    var view = this.getView();
+                    var removeButton = this.lookupReference('removeButton');
+                    if (value === view.getItem().id.toString()) {
+                        removeButton.enable();
+                    } else {
+                        removeButton.disable();
+                    }
+                },
+                specialkey: function (field, event) {
+                    var removeButton = this.lookupReference('removeButton');
+                    if (!removeButton.isDisabled()
+                        && event.getKey() === event.ENTER) {
+                        removeButton.fireEvent('click', removeButton, event);
+                    }
+                }
+            },
+            'button[reference=removeButton]': {
+                click: function () {
+                    const view = this.getView();
+                    Proxmox.Utils.API2Request({
+                        url: view.getUrl() + view.getParams(),
+                        method: 'DELETE',
+                        waitMsgTarget: view,
+                        failure: function (response, opts) {
+                            view.close();
+                            Ext.Msg.alert('Error', response.htmlStatus);
+                        },
+                        success: function (response, options) {
+                            const hasProgressBar = !!(view.showProgress &&
+                                response.result.data);
+
+                            if (hasProgressBar) {
+                                // stay around so we can trigger our close
+                                // events when background action is completed
+                                view.hide();
+
+                                var upid = response.result.data;
+                                var win =
+                                    Ext.create('Proxmox.window.TaskProgress', {
+                                        upid: upid,
+                                        listeners: {
+                                            destroy: function () {
+                                                view.close();
+                                            }
+                                        }
+                                    });
+                                win.show();
+                            } else {
+                                view.close();
+                            }
+                        }
+                    });
+                }
+            }
+        }
+    },
+
+    items: [
+        {
+            xtype: 'component',
+            cls: [Ext.baseCSSPrefix + 'message-box-icon',
+                Ext.baseCSSPrefix + 'message-box-warning',
+                Ext.baseCSSPrefix + 'dlg-icon']
+        },
+        {
+            xtype: 'container',
+            flex: 1,
+            layout: {
+                type: 'vbox',
+                align: 'stretch'
+            },
+            items: [
+                {
+                    xtype: 'component',
+                    reference: 'messageCmp'
+                },
+                {
+                    itemId: 'confirmField',
+                    reference: 'confirmField',
+                    xtype: 'textfield',
+                    name: 'confirm',
+                    labelWidth: 300,
+                    hideTrigger: true,
+                    allowBlank: false
+                },
+                {
+                    xtype: 'proxmoxcheckbox',
+                    name: 'purge',
+                    reference: 'purgeCheckbox',
+                    boxLabel: gettext('Wipe'),
+                    checked: false,
+                    autoEl: {
+                        tag: 'div',
+                        'data-qtip': gettext('Wipe disk after the removal of mountpoint')
+                    }
+                },
+            ]
+        }
+    ],
+    buttons: [
+        {
+            reference: 'removeButton',
+            text: gettext('Remove'),
+            disabled: true
+        }
+    ],
+
+    initComponent: function () {
+        const me = this;
+        me.callParent();
+
+        const item = me.getItem();
+
+        if (!Ext.isDefined(item.id)) {
+            throw "no ID specified";
+        }
+
+        if (!Ext.isDefined(item.type)) {
+            throw "no Disk type specified";
+        }
+
+        const messageCmp = me.lookupReference('messageCmp');
+        let msg;
+
+        if (item.type === 'VM') {
+            msg = Proxmox.Utils.format_task_description('qmdestroy', item.id);
+        } else if (item.type === 'CT') {
+            msg = Proxmox.Utils.format_task_description('vzdestroy', item.id);
+        } else if (item.type === 'CephPool') {
+            msg = Proxmox.Utils.format_task_description('cephdestroypool', item.id);
+        } else if (item.type === 'Image') {
+            msg = Proxmox.Utils.format_task_description('unknownimgdel', item.id);
+        } else {
+            throw "unknown item type specified";
+        }
+
+        if (!(item.type === 'VM' || item.type === 'CT')) {
+            let purgeCheckbox = me.lookupReference('purgeCheckbox');
+            purgeCheckbox.setDisabled(true);
+            purgeCheckbox.setHidden(true);
+        }
+
+        messageCmp.setHtml(msg);
+
+        const confirmField = me.lookupReference('confirmField');
+        msg = gettext('Please enter the ID to confirm') +
+            ' (' + item.id + ')';
+        confirmField.setFieldLabel(msg);
+    }
+});
-- 
2.20.1

[pve-devel] [PATCH pve-manager 2/5] ui refactoring: SafeDestroy moved into widgettoolkit + adjusted usages

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/manager6/Makefile               |   1 -
 www/manager6/ceph/Pool.js           |   2 +-
 www/manager6/lxc/Config.js          |   2 +-
 www/manager6/qemu/Config.js         |   2 +-
 www/manager6/storage/ContentView.js |   2 +-
 www/manager6/window/SafeDestroy.js  | 194 ----------------------------
 6 files changed, 4 insertions(+), 199 deletions(-)
 delete mode 100644 www/manager6/window/SafeDestroy.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 4288acdd..89a45d85 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -247,7 +247,6 @@ JSSRC= 							\
 	window/Migrate.js				\
 	window/NotesEdit.js				\
 	window/Restore.js				\
-	window/SafeDestroy.js				\
 	window/Settings.js				\
 	window/Snapshot.js				\
 	window/StartupEdit.js				\
diff --git a/www/manager6/ceph/Pool.js b/www/manager6/ceph/Pool.js
index 19eb01e9..4c3a29c8 100644
--- a/www/manager6/ceph/Pool.js
+++ b/www/manager6/ceph/Pool.js
@@ -229,7 +229,7 @@ Ext.define('PVE.node.CephPoolList', {
 		var base_url = '/nodes/' + nodename + '/ceph/pools/' +
 		    rec.data.pool_name;
 
-		var win = Ext.create('PVE.window.SafeDestroy', {
+		var win = Ext.create('Proxmox.window.SafeRemove', {
 		    showProgress: true,
 		    url: base_url,
 		    params: {
diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js
index c2222d3a..94c4bc11 100644
--- a/www/manager6/lxc/Config.js
+++ b/www/manager6/lxc/Config.js
@@ -149,7 +149,7 @@ Ext.define('PVE.lxc.Config', {
 		    disabled: !caps.vms['VM.Allocate'],
 		    itemId: 'removeBtn',
 		    handler: function() {
-			Ext.create('PVE.window.SafeDestroy', {
+			Ext.create('Proxmox.window.SafeRemove', {
 			    url: base_url,
 			    item: { type: 'CT', id: vmid }
 			}).show();
diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
index a13bf0c5..cf397f59 100644
--- a/www/manager6/qemu/Config.js
+++ b/www/manager6/qemu/Config.js
@@ -124,7 +124,7 @@ Ext.define('PVE.qemu.Config', {
 		    itemId: 'removeBtn',
 		    disabled: !caps.vms['VM.Allocate'],
 		    handler: function() {
-			Ext.create('PVE.window.SafeDestroy', {
+			Ext.create('Proxmox.window.SafeRemove', {
 			    url: base_url,
 			    item: { type: 'VM', id: vmid }
 			}).show();
diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
index c70c732c..83c9ab1d 100644
--- a/www/manager6/storage/ContentView.js
+++ b/www/manager6/storage/ContentView.js
@@ -483,7 +483,7 @@ Ext.define('PVE.storage.ContentView', {
 			return;
 		    }
 		}
-		var win = Ext.create('PVE.window.SafeDestroy', {
+		var win = Ext.create('Proxmox.window.SafeRemove', {
 		    title: Ext.String.format(gettext("Destroy '{0}'"), rec.data.volid),
 		    showProgress: true,
 		    url: url,
diff --git a/www/manager6/window/SafeDestroy.js b/www/manager6/window/SafeDestroy.js
deleted file mode 100644
index cc32f6e0..00000000
--- a/www/manager6/window/SafeDestroy.js
+++ /dev/null
@@ -1,194 +0,0 @@
-/* Popup a message window
- * where the user has to manually enter the resource ID
- * to enable the destroy button
- */
-Ext.define('PVE.window.SafeDestroy', {
-    extend: 'Ext.window.Window',
-    alias: 'widget.pveSafeDestroy',
-
-    title: gettext('Confirm'),
-    modal: true,
-    buttonAlign: 'center',
-    bodyPadding: 10,
-    width: 450,
-    layout: { type:'hbox' },
-    defaultFocus: 'confirmField',
-    showProgress: false,
-
-    config: {
-	item: {
-	    id: undefined,
-	    type: undefined
-	},
-	url: undefined,
-	params: {}
-    },
-
-    getParams: function() {
-	var me = this;
-	var purgeCheckbox = me.lookupReference('purgeCheckbox');
-	if (purgeCheckbox.checked) {
-	    me.params.purge = 1;
-	}
-	if (Ext.Object.isEmpty(me.params)) {
-	    return '';
-	}
-	return '?' + Ext.Object.toQueryString(me.params);
-    },
-
-    controller: {
-
-	xclass: 'Ext.app.ViewController',
-
-	control: {
-	    'field[name=confirm]': {
-		change: function(f, value) {
-		    var view = this.getView();
-		    var removeButton = this.lookupReference('removeButton');
-		    if (value === view.getItem().id.toString()) {
-			removeButton.enable();
-		    } else {
-			removeButton.disable();
-		    }
-		},
-		specialkey: function (field, event) {
-		    var removeButton = this.lookupReference('removeButton');
-		    if (!removeButton.isDisabled() && event.getKey() == event.ENTER) {
-			removeButton.fireEvent('click', removeButton, event);
-		    }
-		}
-	    },
-           'button[reference=removeButton]': {
-		click: function() {
-		    var view = this.getView();
-		    Proxmox.Utils.API2Request({
-			url: view.getUrl() + view.getParams(),
-			method: 'DELETE',
-			waitMsgTarget: view,
-			failure: function(response, opts) {
-			    view.close();
-			    Ext.Msg.alert('Error', response.htmlStatus);
-			},
-			success: function(response, options) {
-			    var hasProgressBar = view.showProgress &&
-				response.result.data ? true : false;
-
-			    if (hasProgressBar) {
-				// stay around so we can trigger our close events
-				// when background action is completed
-				view.hide();
-
-				var upid = response.result.data;
-				var win = Ext.create('Proxmox.window.TaskProgress', {
-				    upid: upid,
-				    listeners: {
-					destroy: function () {
-					    view.close();
-					}
-				    }
-				});
-				win.show();
-			    } else {
-				view.close();
-			    }
-			}
-		    });
-		}
-            }
-	}
-    },
-
-    items: [
-	{
-	    xtype: 'component',
-	    cls: [ Ext.baseCSSPrefix + 'message-box-icon',
-		   Ext.baseCSSPrefix + 'message-box-warning',
-		   Ext.baseCSSPrefix + 'dlg-icon']
-	},
-	{
-	    xtype: 'container',
-	    flex: 1,
-	    layout: {
-		type: 'vbox',
-		align: 'stretch'
-	    },
-	    items: [
-		{
-		    xtype: 'component',
-		    reference: 'messageCmp'
-		},
-		{
-		    itemId: 'confirmField',
-		    reference: 'confirmField',
-		    xtype: 'textfield',
-		    name: 'confirm',
-		    labelWidth: 300,
-		    hideTrigger: true,
-		    allowBlank: false
-		},
-		{
-		    xtype: 'proxmoxcheckbox',
-		    name: 'purge',
-		    reference: 'purgeCheckbox',
-		    boxLabel: gettext('Purge'),
-		    checked: false,
-		    autoEl: {
-			tag: 'div',
-			'data-qtip': gettext('Remove from replication and backup jobs')
-		    }
-		}
-	    ]
-	}
-    ],
-    buttons: [
-	{
-	    reference: 'removeButton',
-	    text: gettext('Remove'),
-	    disabled: true
-	}
-    ],
-
-    initComponent : function() {
-	var me = this;
-
-	me.callParent();
-
-	var item = me.getItem();
-
-	if (!Ext.isDefined(item.id)) {
-	    throw "no ID specified";
-	}
-
-	if (!Ext.isDefined(item.type)) {
-	    throw "no VM type specified";
-	}
-
-	var messageCmp = me.lookupReference('messageCmp');
-	var msg;
-
-	if (item.type === 'VM') {
-	    msg = Proxmox.Utils.format_task_description('qmdestroy', item.id);
-	} else if (item.type === 'CT') {
-	    msg = Proxmox.Utils.format_task_description('vzdestroy', item.id);
-	} else if (item.type === 'CephPool') {
-	    msg = Proxmox.Utils.format_task_description('cephdestroypool', item.id);
-	} else if (item.type === 'Image') {
-	    msg = Proxmox.Utils.format_task_description('unknownimgdel', item.id);
-	} else {
-	    throw "unknown item type specified";
-	}
-
-	messageCmp.setHtml(msg);
-
-	if (!(item.type === 'VM' || item.type === 'CT')) {
-	    let purgeCheckbox = me.lookupReference('purgeCheckbox');
-	    purgeCheckbox.setDisabled(true);
-	    purgeCheckbox.setHidden(true);
-	}
-
-	var confirmField = me.lookupReference('confirmField');
-	msg = gettext('Please enter the ID to confirm') +
-	    ' (' + item.id + ')';
-	confirmField.setFieldLabel(msg);
-    }
-});
-- 
2.20.1

[pve-devel] [PATCH proxmox-widget-toolkit 3/5] ui: added possibility to show a small note in SafeRemove dialog

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/window/SafeRemove.js | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/src/window/SafeRemove.js b/src/window/SafeRemove.js
index c541272..c3fc7dd 100644
--- a/src/window/SafeRemove.js
+++ b/src/window/SafeRemove.js
@@ -21,6 +21,7 @@ Ext.define('Proxmox.window.SafeRemove', {
             type: undefined
         },
         url: undefined,
+        note: undefined,
         params: {}
     },
 
@@ -137,6 +138,22 @@ Ext.define('Proxmox.window.SafeRemove', {
                         'data-qtip': gettext('Wipe disk after the removal of mountpoint')
                     }
                 },
+                {
+                    xtype: 'container',
+                    reference: 'noteContainer',
+                    flex: 1,
+                    layout: {
+                        type: 'vbox',
+                        align: 'middle'
+                    },
+                    height: 25,
+                    items: [
+                        {
+                            xtype: 'component',
+                            reference: 'noteCmp'
+                        },
+                    ]
+                },
             ]
         }
     ],
@@ -163,8 +180,17 @@ Ext.define('Proxmox.window.SafeRemove', {
         }
 
         const messageCmp = me.lookupReference('messageCmp');
+        const noteCmp = me.lookupReference('noteCmp');
         let msg;
 
+        if (me.getNote() !== undefined) {
+            noteCmp.setHtml(`<small>${me.getNote()}</small>`);
+        } else {
+            const noteContainer = me.lookupReference('noteContainer');
+            noteContainer.setDisabled(true);
+            noteContainer.setHidden(true);
+        }
+
         if (item.type === 'VM') {
             msg = Proxmox.Utils.format_task_description('qmdestroy', item.id);
         } else if (item.type === 'CT') {
@@ -173,6 +199,8 @@ Ext.define('Proxmox.window.SafeRemove', {
             msg = Proxmox.Utils.format_task_description('cephdestroypool', item.id);
         } else if (item.type === 'Image') {
             msg = Proxmox.Utils.format_task_description('unknownimgdel', item.id);
+        } else if (item.type === 'Dir') {
+            msg = `${gettext('Directory')} ${item.id} - ${gettext('Remove')}`
         } else {
             throw "unknown item type specified";
         }
-- 
2.20.1

[pve-devel] [PATCH proxmox-backup 4/5] api2: DatastoreMountInfo now also contains the name of the mount

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/node/disks/directory.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index 0d9ddeef..7dbf9a29 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -34,6 +34,8 @@ pub struct DatastoreMountInfo {
     pub unitfile: String,
     /// The mount path.
     pub path: String,
+    /// Id of the mount.
+    pub id: String,
     /// The mounted device.
     pub device: String,
     /// File system type
@@ -76,6 +78,8 @@ pub fn  list_datastore_mounts() -> Result<Vec<DatastoreMountInfo>, Error> {
         let item = item?;
         let name = item.file_name().to_string_lossy().to_string();
 
+        let id = String::from(MOUNT_NAME_REGEX.captures(&name).unwrap().get(1).map_or("", |m| m.as_str()));
+
         let unitfile = format!("{}/{}", basedir, name);
         let config = systemd::config::parse_systemd_mount(&unitfile)?;
         let data: SystemdMountSection = config.lookup("Mount", "Mount")?;
@@ -84,6 +88,7 @@ pub fn  list_datastore_mounts() -> Result<Vec<DatastoreMountInfo>, Error> {
             unitfile,
             device: data.What,
             path: data.Where,
+            id,
             filesystem: data.Type,
             options: data.Options,
         });
-- 
2.20.1

[pve-devel] [PATCH proxmox-backup 5/5] ui: added possiblity to remove directories/mount-units in the WebUI

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/DirectoryList.js | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/www/DirectoryList.js b/www/DirectoryList.js
index 00531fd0..b4313e49 100644
--- a/www/DirectoryList.js
+++ b/www/DirectoryList.js
@@ -20,6 +20,21 @@ Ext.define('PBS.admin.Directorylist', {
 		},
 	    }).show();
 	},
+    removeDir: function () {
+        let me = this;
+        let view = me.getView();
+        let rec = view.getSelection();
+        Ext.create('Proxmox.window.SafeRemove', {
+            url: `/nodes/localhost/disks/directory/${rec[0].data.id}`,
+            item: {type: 'Dir', id: rec[0].data.id},
+            note: gettext('Data and partitions on the disk will be left untouched.'),
+            listeners: {
+                destroy: function () {
+                    me.reload();
+                },
+            },
+        }).show();
+    },
 
 	reload: function() {
 	    let me = this;
@@ -49,6 +64,13 @@ Ext.define('PBS.admin.Directorylist', {
 	    text: gettext('Create') + ': Directory',
 	    handler: 'createDirectory',
 	},
+	{
+		xtype: 'proxmoxButton',
+		text: gettext('Remove'),
+		handler: 'removeDir',
+		disabled: true,
+		iconCls: 'fa fa-trash-o'
+	}
     ],
 
     columns: [
-- 
2.20.1

Re: [pve-devel] [PATCH proxmox-widget-toolkit 1/5] ui refactoring: refactored SafeDestroy from pve-manager into proxmox-widget-toolkit

[Thomas Lamprecht]

You say refactored: What, besides the name, did you change and why?

On 18.08.20 10:40, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/Makefile             |   1 +
>  src/window/SafeRemove.js | 193 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 194 insertions(+)
>  create mode 100644 src/window/SafeRemove.js
> 

> +
> +    initComponent: function () {
> +        const me = this;
> +        me.callParent();
> +
> +        const item = me.getItem();
> +
> +        if (!Ext.isDefined(item.id)) {
> +            throw "no ID specified";
> +        }
> +
> +        if (!Ext.isDefined(item.type)) {
> +            throw "no Disk type specified";

does this always have to by a disk type? It can be a VM, or something completely different.

> +        }
> +
> +        const messageCmp = me.lookupReference('messageCmp');
> +        let msg;
> +
> +        if (item.type === 'VM') {
> +            msg = Proxmox.Utils.format_task_description('qmdestroy', item.id);
> +        } else if (item.type === 'CT') {
> +            msg = Proxmox.Utils.format_task_description('vzdestroy', item.id);
> +        } else if (item.type === 'CephPool') {
> +            msg = Proxmox.Utils.format_task_description('cephdestroypool', item.id);
> +        } else if (item.type === 'Image') {
> +            msg = Proxmox.Utils.format_task_description('unknownimgdel', item.id);
> +        } else {
> +            throw "unknown item type specified";
> +        }

I'd like to avoid having downstream logic here, that adds cyclic coupling.
Why not either do:
* a minimal overwrite in PVE, would have the additional benefit of not needing
  to touch all use sites there.
* replace the "type" property with a "task" property, avoiding the need to map.

> +
> +        if (!(item.type === 'VM' || item.type === 'CT')) {
> +            let purgeCheckbox = me.lookupReference('purgeCheckbox');
> +            purgeCheckbox.setDisabled(true);
> +            purgeCheckbox.setHidden(true);

If we touch this we could also make this more explicit with a default off "purgeable"
(or similar name) config property.

> +        }
> +
> +        messageCmp.setHtml(msg);
> +
> +        const confirmField = me.lookupReference('confirmField');
> +        msg = gettext('Please enter the ID to confirm') +
> +            ' (' + item.id + ')';
> +        confirmField.setFieldLabel(msg);
> +    }
> +});
> 

Re: [pve-devel] [PATCH proxmox-backup 5/5] ui: added possiblity to remove directories/mount-units in the WebUI

[Thomas Lamprecht]

Can I re-add the same one after wards?

On 18.08.20 10:40, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  www/DirectoryList.js | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/www/DirectoryList.js b/www/DirectoryList.js
> index 00531fd0..b4313e49 100644
> --- a/www/DirectoryList.js
> +++ b/www/DirectoryList.js
> @@ -20,6 +20,21 @@ Ext.define('PBS.admin.Directorylist', {
>  		},
>  	    }).show();
>  	},
> +    removeDir: function () {
> +        let me = this;
> +        let view = me.getView();
> +        let rec = view.getSelection();

could do
let rec = me.getView().getSelection();
let id = rec[0].data.id;

to avoid a only once used variable (they often can be OK, but here it adds
no value when reading it) and instead avoid doing two 'rec[0].data.id' uses
below.

> +        Ext.create('Proxmox.window.SafeRemove', {
> +            url: `/nodes/localhost/disks/directory/${rec[0].data.id}`,
> +            item: {type: 'Dir', id: rec[0].data.id},

please try to have new additions eslint compatible, i.e., 

item: {
    type: 'Dir',
    id: id,
},

> +            note: gettext('Data and partitions on the disk will be left untouched.'),

did you checked available gettext translations for something which could be used
here, to avoid a new translation? I'm fine with new ones, but only if there's really
nothing which could be used.

> +            listeners: {
> +                destroy: function () {
> +                    me.reload();
> +                },

nit: could use arrow function:

destroy: () => me.reload(),

> +            },
> +        }).show();
> +    },
>  
>  	reload: function() {
>  	    let me = this;
> @@ -49,6 +64,13 @@ Ext.define('PBS.admin.Directorylist', {
>  	    text: gettext('Create') + ': Directory',
>  	    handler: 'createDirectory',
>  	},
> +	{
> +		xtype: 'proxmoxButton',
> +		text: gettext('Remove'),
> +		handler: 'removeDir',
> +		disabled: true,
> +		iconCls: 'fa fa-trash-o'
> +	}
>      ],
>  
>      columns: [
> 

Re: [pve-devel] [PATCH proxmox-widget-toolkit 3/5] ui: added possibility to show a small note in SafeRemove dialog

[Thomas Lamprecht]

general note, while I find tags in the comment subject nice, this doesn't really
adds value as everything in widget toolkit is "ui" :) We only use that in things
like pve-manager or proxmox-backup-server as there API, UI and possibly other stuff
are shared in the same repo. You could use:

"safe destroy: add possibility to show a small extra note"

On 18.08.20 10:40, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/window/SafeRemove.js | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/src/window/SafeRemove.js b/src/window/SafeRemove.js
> index c541272..c3fc7dd 100644
> --- a/src/window/SafeRemove.js
> +++ b/src/window/SafeRemove.js
...
> @@ -173,6 +199,8 @@ Ext.define('Proxmox.window.SafeRemove', {
>              msg = Proxmox.Utils.format_task_description('cephdestroypool', item.id);
>          } else if (item.type === 'Image') {
>              msg = Proxmox.Utils.format_task_description('unknownimgdel', item.id);
> +        } else if (item.type === 'Dir') {
> +            msg = `${gettext('Directory')} ${item.id} - ${gettext('Remove')}`

few issues:
1. misses a semicolon - did you even built it once? I'd guess that eslint complains about
   it then.
2. has nothing to do with this patch, should be separate
3. why not a format task description? In combination with my comments on patch 1/5 that
   could be much nicer, avoiding this clunky if/else block completely.

>          } else {
>              throw "unknown item type specified";
>          }
>