[pve-devel] [PATCH manager/widget-toolkit v2 0/2] close #3181: ui: display guest name in confirm dialogs

[pve-devel] [PATCH manager/widget-toolkit v2 0/2] close #3181: ui: display guest name in confirm dialogs

[Michael Köppl]

Closes #3181 [0].

The goal of this series is to extend the information displayed in the
confirmation dialogs when cloning, migrating, removing VMs or CTs by
adding the guest name in addition to the VM ID. This also unifies the
displayed information since there are multiple actions that already
include the guest name, such as reboot and shutdown [1].
This makes it easier for users to tell which VM or CT they are currently
targeting, reducing the likelihood of erroneous actions.

To make the information in the dialogs consistent with sorting behavior
selected by the user, "VMID (VM name)" is displayed if no sorting field
has been explicitly set or the sorting field "VMID" was set through the
tree settings. If "Name" was selected, the information is displayed as
"VM name (VMID)".

This affects the following actions performed on VMs/CTs:
* Remove
* Clone
* Migrate
* Snapshot
* Snapshot rollback
* Backup VM/CT from config view
* Restore VM/CT from config view

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=3181
[1] https://lore.proxmox.com/pve-devel/96d831f7-8435-412a-8205-b33f71357d0f@proxmox.com

Changes since v1:
* Add utils helper function for formatting guest information
* Make order of VM/CT ID and name dependent on sorting field chosen by
  user

proxmox-widget-toolkit:

Michael Köppl (1):
  close #3181: ui: add formatted guest identifier to safe destroy dialog
    window

 src/window/SafeDestroy.js | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


pve-manager:

Michael Köppl (1):
  close #3181: ui: display guest name in confirm dialogs

 www/manager6/Utils.js             | 10 +++++++++-
 www/manager6/grid/BackupView.js   |  4 ++++
 www/manager6/lxc/CmdMenu.js       |  9 ++++++++-
 www/manager6/lxc/Config.js        | 15 +++++++++++++--
 www/manager6/qemu/CmdMenu.js      |  9 ++++++++-
 www/manager6/qemu/Config.js       | 15 +++++++++++++--
 www/manager6/tree/SnapshotTree.js | 12 ++++++++++--
 www/manager6/window/Backup.js     |  6 +++---
 www/manager6/window/Clone.js      |  7 +++++--
 www/manager6/window/Migrate.js    | 10 ++++++++--
 www/manager6/window/Restore.js    |  3 ++-
 www/manager6/window/Snapshot.js   |  7 ++++++-
 12 files changed, 89 insertions(+), 18 deletions(-)


Summary over all repositories:
  13 files changed, 95 insertions(+), 19 deletions(-)

-- 
Generated by git-murpp 0.8.0


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH widget-toolkit v2 1/1] close #3181: ui: add formatted guest identifier to safe destroy dialog window

[Michael Köppl]

Adds a formatted guest identifier (e.g. "VMID (VM name)") to the
SafeDestroy window. The formatted identifier is only used if it is
explicitly set by the caller. Otherwise, the VMID is used, avoiding
changes in behavior for other components.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/window/SafeDestroy.js | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/window/SafeDestroy.js b/src/window/SafeDestroy.js
index c058465..a19747d 100644
--- a/src/window/SafeDestroy.js
+++ b/src/window/SafeDestroy.js
@@ -26,6 +26,7 @@ Ext.define('Proxmox.window.SafeDestroy', {
     config: {
 	item: {
 	    id: undefined,
+	    formattedIdentifier: undefined,
 	},
 	url: undefined,
 	note: undefined,
@@ -187,9 +188,13 @@ Ext.define('Proxmox.window.SafeDestroy', {
 	}
 
 	let taskName = me.getTaskName();
+	let formattedIdentifier = me.getItem().formattedIdentifier;
 	if (Ext.isDefined(taskName)) {
 	    me.lookupReference('messageCmp').setHtml(
-		Proxmox.Utils.format_task_description(taskName, itemId),
+		Proxmox.Utils.format_task_description(
+		    taskName,
+		    formattedIdentifier ? formattedIdentifier : itemId,
+		),
 	    );
 	} else {
 	    throw "no task name specified";
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v2 1/1] close #3181: ui: display guest name in confirm dialogs

[Michael Köppl]

The confirmation dialogs of the following actions are affected by this
change:
* Remove
* Clone
* Migrate
* Snapshot
* Snapshot restore
* Backup VM/CT from config view
* Restore VM/CT from config view

A combination of VM/CT id and name is added to each confirmation dialog.
The order of id and name depends on the sort field selected in the tree
settings. If "Name" is selected, the confirmation dialogs will show "VM
name (VMID)". In any other case, "VMID (VM name)" will be used.

The VM/CT name is considered optional in all handled cases. If it is
undefined, only the VMID will be displayed in the dialog window. No
exceptions are thrown in case of an undefined guest name because it
only extends the information displayed to the user and is not essential
for performing any of the actions above.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 www/manager6/Utils.js             | 10 +++++++++-
 www/manager6/grid/BackupView.js   |  4 ++++
 www/manager6/lxc/CmdMenu.js       |  9 ++++++++-
 www/manager6/lxc/Config.js        | 15 +++++++++++++--
 www/manager6/qemu/CmdMenu.js      |  9 ++++++++-
 www/manager6/qemu/Config.js       | 15 +++++++++++++--
 www/manager6/tree/SnapshotTree.js | 12 ++++++++++--
 www/manager6/window/Backup.js     |  6 +++---
 www/manager6/window/Clone.js      |  7 +++++--
 www/manager6/window/Migrate.js    | 10 ++++++++--
 www/manager6/window/Restore.js    |  3 ++-
 www/manager6/window/Snapshot.js   |  7 ++++++-
 12 files changed, 89 insertions(+), 18 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index aa415759..1160d8d2 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1968,8 +1968,16 @@ Ext.define('PVE.Utils', {
 	return languageCookie || Proxmox.defaultLang || 'en';
     },
 
+    getFormattedGuestIdentifier: function(vmid, guestName) {
+	if (PVE.UIOptions.getTreeSortingValue('sort-field') === 'vmid') {
+	    return guestName ? `${vmid} (${guestName})` : vmid;
+	} else {
+	    return guestName ? `${guestName} (${vmid})` : vmid;
+	}
+    },
+
     formatGuestTaskConfirmation: function(taskType, vmid, guestName) {
-	return Proxmox.Utils.format_task_description(taskType, `${vmid} (${guestName})`);
+	return Proxmox.Utils.format_task_description(taskType, this.getFormattedGuestIdentifier(vmid, guestName));
     },
 },
 
diff --git a/www/manager6/grid/BackupView.js b/www/manager6/grid/BackupView.js
index e71d1c88..99f2a49b 100644
--- a/www/manager6/grid/BackupView.js
+++ b/www/manager6/grid/BackupView.js
@@ -39,6 +39,8 @@ Ext.define('PVE.grid.BackupView', {
 	    throw "unsupported VM type '" + vmtype + "'";
 	}
 
+	let vmname = me.pveSelNode.data.name;
+
 	var searchFilter = {
 	    property: 'volid',
 	    value: '',
@@ -167,6 +169,7 @@ Ext.define('PVE.grid.BackupView', {
 		    nodename: nodename,
 		    vmid: vmid,
 		    vmtype: vmtype,
+		    vmname: vmname,
 		    storage: storagesel.getValue(),
 		    listeners: {
 			close: function() {
@@ -189,6 +192,7 @@ Ext.define('PVE.grid.BackupView', {
 		let win = Ext.create('PVE.window.Restore', {
 		    nodename: nodename,
 		    vmid: vmid,
+		    vmname: vmname,
 		    volid: rec.data.volid,
 		    volidText: PVE.Utils.render_storage_content(rec.data.volid, {}, rec),
 		    vmtype: vmtype,
diff --git a/www/manager6/lxc/CmdMenu.js b/www/manager6/lxc/CmdMenu.js
index e30989a6..e4dadd2f 100644
--- a/www/manager6/lxc/CmdMenu.js
+++ b/www/manager6/lxc/CmdMenu.js
@@ -89,7 +89,13 @@ Ext.define('PVE.lxc.CmdMenu', {
 		text: gettext('Clone'),
 		iconCls: 'fa fa-fw fa-clone',
 		hidden: !caps.vms['VM.Clone'],
-		handler: () => PVE.window.Clone.wrap(info.node, info.vmid, me.isTemplate, 'lxc'),
+		handler: () => PVE.window.Clone.wrap(
+		    info.node,
+		    info.vmid,
+		    info.name,
+		    me.isTemplate,
+		    'lxc',
+		),
 	    },
 	    {
 		text: gettext('Migrate'),
@@ -100,6 +106,7 @@ Ext.define('PVE.lxc.CmdMenu', {
 			vmtype: 'lxc',
 			nodename: info.node,
 			vmid: info.vmid,
+			vmname: info.name,
 			autoShow: true,
 		    });
 		},
diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js
index a7191fa2..cda6a396 100644
--- a/www/manager6/lxc/Config.js
+++ b/www/manager6/lxc/Config.js
@@ -100,6 +100,7 @@ Ext.define('PVE.lxc.Config', {
 		    vmtype: 'lxc',
 		    nodename: nodename,
 		    vmid: vmid,
+		    vmname: vm.name,
 		});
 		win.show();
 	    },
@@ -115,7 +116,13 @@ Ext.define('PVE.lxc.Config', {
 		    iconCls: 'fa fa-fw fa-clone',
 		    hidden: !caps.vms['VM.Clone'],
 		    handler: function() {
-			PVE.window.Clone.wrap(nodename, vmid, template, 'lxc');
+			PVE.window.Clone.wrap(
+			    nodename,
+			    vmid,
+			    vm.name,
+			    template,
+			    'lxc',
+			);
 		    },
 		},
 		{
@@ -156,7 +163,11 @@ Ext.define('PVE.lxc.Config', {
 		    handler: function() {
 			Ext.create('PVE.window.SafeDestroyGuest', {
 			    url: base_url,
-			    item: { type: 'CT', id: vmid },
+			    item: {
+				type: 'CT',
+				id: vmid,
+				formattedIdentifier: PVE.Utils.getFormattedGuestIdentifier(vmid, vm.name),
+			    },
 			    taskName: 'vzdestroy',
 			}).show();
 		    },
diff --git a/www/manager6/qemu/CmdMenu.js b/www/manager6/qemu/CmdMenu.js
index 7a4e0a0e..e7dd6782 100644
--- a/www/manager6/qemu/CmdMenu.js
+++ b/www/manager6/qemu/CmdMenu.js
@@ -121,6 +121,7 @@ Ext.define('PVE.qemu.CmdMenu', {
 			vmtype: 'qemu',
 			nodename: info.node,
 			vmid: info.vmid,
+			vmname: info.name,
 			autoShow: true,
 		    });
 		},
@@ -129,7 +130,13 @@ Ext.define('PVE.qemu.CmdMenu', {
 		text: gettext('Clone'),
 		iconCls: 'fa fa-fw fa-clone',
 		hidden: !caps.vms['VM.Clone'],
-		handler: () => PVE.window.Clone.wrap(info.node, info.vmid, me.isTemplate, 'qemu'),
+		handler: () => PVE.window.Clone.wrap(
+		    info.node,
+		    info.vmid,
+		    info.name,
+		    me.isTemplate,
+		    'qemu',
+		),
 	    },
 	    {
 		text: gettext('Convert to template'),
diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
index 48eb753e..0699175e 100644
--- a/www/manager6/qemu/Config.js
+++ b/www/manager6/qemu/Config.js
@@ -73,6 +73,7 @@ Ext.define('PVE.qemu.Config', {
 		    vmtype: 'qemu',
 		    nodename: nodename,
 		    vmid: vmid,
+		    vmname: vm.name,
 		});
 		win.show();
 	    },
@@ -88,7 +89,13 @@ Ext.define('PVE.qemu.Config', {
 		    iconCls: 'fa fa-fw fa-clone',
 		    hidden: !caps.vms['VM.Clone'],
 		    handler: function() {
-			PVE.window.Clone.wrap(nodename, vmid, template, 'qemu');
+			PVE.window.Clone.wrap(
+			    nodename,
+			    vmid,
+			    vm.name,
+			    template,
+			    'qemu',
+			);
 		    },
 		},
 		{
@@ -128,7 +135,11 @@ Ext.define('PVE.qemu.Config', {
 		    handler: function() {
 			Ext.create('PVE.window.SafeDestroyGuest', {
 			    url: base_url,
-			    item: { type: 'VM', id: vmid },
+			    item: {
+				type: 'VM',
+				id: vmid,
+				formattedIdentifier: PVE.Utils.getFormattedGuestIdentifier(vmid, vm.name),
+			    },
 			    taskName: 'qmdestroy',
 			}).show();
 		    },
diff --git a/www/manager6/tree/SnapshotTree.js b/www/manager6/tree/SnapshotTree.js
index 97268072..e909584e 100644
--- a/www/manager6/tree/SnapshotTree.js
+++ b/www/manager6/tree/SnapshotTree.js
@@ -11,6 +11,7 @@ Ext.define('PVE.guest.SnapshotTree', {
 	    type: undefined,
 	    nodename: undefined,
 	    vmid: undefined,
+	    vmname: undefined,
 	    snapshotAllowed: false,
 	    rollbackAllowed: false,
 	    snapshotFeature: false,
@@ -50,6 +51,7 @@ Ext.define('PVE.guest.SnapshotTree', {
 	    let win = Ext.create('PVE.window.Snapshot', {
 		nodename: vm.get('nodename'),
 		vmid: vm.get('vmid'),
+		vmname: vm.get('vmname'),
 		viewonly: !vm.get('snapshotAllowed'),
 		type: vm.get('type'),
 		isCreate: !edit,
@@ -213,6 +215,8 @@ Ext.define('PVE.guest.SnapshotTree', {
 	    }
 	    vm.set('vmid', view.pveSelNode.data.vmid);
 
+	    vm.set('vmname', view.pveSelNode.data.name);
+
 	    let caps = Ext.state.Manager.get('GuiCap');
 	    vm.set('snapshotAllowed', !!caps.vms['VM.Snapshot']);
 	    vm.set('rollbackAllowed', !!caps.vms['VM.Snapshot.Rollback']);
@@ -259,8 +263,12 @@ Ext.define('PVE.guest.SnapshotTree', {
 		let view = this.up('treepanel');
 		let rec = view.getSelection()[0];
 		let vmid = view.getViewModel().get('vmid');
-		return Proxmox.Utils.format_task_description('qmrollback', vmid) +
-		    ` '${rec.data.name}'? ${gettext("Current state will be lost.")}`;
+		let vmname = view.getViewModel().get('vmname');
+		return PVE.Utils.formatGuestTaskConfirmation(
+		    'qmrollback',
+		    vmid,
+		    vmname,
+		) + ` '${rec.data.name}'? ${gettext("Current state will be lost.")}`;
 	    },
 	    handler: 'rollback',
 	},
diff --git a/www/manager6/window/Backup.js b/www/manager6/window/Backup.js
index 4418a9c7..9f0f3974 100644
--- a/www/manager6/window/Backup.js
+++ b/www/manager6/window/Backup.js
@@ -328,9 +328,9 @@ Ext.define('PVE.window.Backup', {
 	    hidden: false,
 	});
 
-	var title = gettext('Backup') + " " +
-	    (me.vmtype === 'lxc' ? "CT" : "VM") +
-	    " " + me.vmid;
+	let guestTypeStr = me.vmtype === 'lxc' ? "CT" : "VM";
+	let formattedGuestIdentifier = PVE.Utils.getFormattedGuestIdentifier(me.vmid, me.vmname);
+	let title = `${gettext('Backup')} ${guestTypeStr} ${formattedGuestIdentifier}`;
 
 	Ext.apply(me, {
 	    title: title,
diff --git a/www/manager6/window/Clone.js b/www/manager6/window/Clone.js
index 2d3f2678..6efa2391 100644
--- a/www/manager6/window/Clone.js
+++ b/www/manager6/window/Clone.js
@@ -21,7 +21,7 @@ Ext.define('PVE.window.Clone', {
 
     statics: {
 	// display a snapshot selector only if needed
-	wrap: function(nodename, vmid, isTemplate, guestType) {
+	wrap: function(nodename, vmid, vmname, isTemplate, guestType) {
 	    Proxmox.Utils.API2Request({
 		url: '/nodes/' + nodename + '/' + guestType + '/' + vmid +'/snapshot',
 		failure: function(response, opts) {
@@ -36,6 +36,7 @@ Ext.define('PVE.window.Clone', {
 			nodename: nodename,
 			guestType: guestType,
 			vmid: vmid,
+			vmname: vmname,
 			isTemplate: isTemplate,
 			hasSnapshots: hasSnapshots,
 		    }).show();
@@ -155,7 +156,9 @@ Ext.define('PVE.window.Clone', {
 	if (me.isTemplate) {
 	    titletext += ' Template';
 	}
-	me.title = "Clone " + titletext + " " + me.vmid;
+
+	let formattedGuestIdentifier = PVE.Utils.getFormattedGuestIdentifier(me.vmid, me.vmname);
+	me.title = `Clone ${titletext} ${formattedGuestIdentifier}`;
 
 	var col1 = [];
 	var col2 = [];
diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
index 78d03921..8ec14485 100644
--- a/www/manager6/window/Migrate.js
+++ b/www/manager6/window/Migrate.js
@@ -4,6 +4,7 @@ Ext.define('PVE.window.Migrate', {
     vmtype: undefined,
     nodename: undefined,
     vmid: undefined,
+    vmname: undefined,
     maxHeight: 450,
 
     viewModel: {
@@ -92,9 +93,14 @@ Ext.define('PVE.window.Migrate', {
 	    }
 	    vm.set('vmtype', view.vmtype);
 
-	    view.setTitle(
-		Ext.String.format('{0} {1} {2}', gettext('Migrate'), vm.get(view.vmtype).commonName, view.vmid),
+	    let title = Ext.String.format(
+		'{0} {1} {2}',
+		gettext('Migrate'),
+		vm.get(view.vmtype).commonName,
+		PVE.Utils.getFormattedGuestIdentifier(view.vmid, view.vmname),
 	    );
+	    view.setTitle(title);
+
 	    me.lookup('proxmoxHelpButton').setHelpConfig({
 		onlineHelp: vm.get(view.vmtype).onlineHelp,
 	    });
diff --git a/www/manager6/window/Restore.js b/www/manager6/window/Restore.js
index 6efe1313..42fc5622 100644
--- a/www/manager6/window/Restore.js
+++ b/www/manager6/window/Restore.js
@@ -356,7 +356,8 @@ Ext.define('PVE.window.Restore', {
 
 	let title = gettext('Restore') + ": " + (me.vmtype === 'lxc' ? 'CT' : 'VM');
 	if (me.vmid) {
-	    title = `${gettext('Overwrite')} ${title} ${me.vmid}`;
+	    let formattedGuestIdentifier = PVE.Utils.getFormattedGuestIdentifier(me.vmid, me.vmname);
+	    title = `${gettext('Overwrite')} ${title} ${formattedGuestIdentifier}`;
 	}
 
 	Ext.apply(me, {
diff --git a/www/manager6/window/Snapshot.js b/www/manager6/window/Snapshot.js
index 426be7cc..0cbe7cc7 100644
--- a/www/manager6/window/Snapshot.js
+++ b/www/manager6/window/Snapshot.js
@@ -137,7 +137,12 @@ Ext.define('PVE.window.Snapshot', {
 
 	let subject;
 	if (me.isCreate) {
-	    subject = (me.type === 'qemu' ? 'VM' : 'CT') + me.vmid + ' ' + gettext('Snapshot');
+	    let guestTypeStr = me.type === 'qemu' ? 'VM' : 'CT';
+	    let formattedGuestIdentifier = PVE.Utils.getFormattedGuestIdentifier(
+		me.vmid,
+		me.vmname,
+	    );
+	    subject = `${guestTypeStr} ${formattedGuestIdentifier} ${gettext('Snapshot')}}`
 	    me.method = 'POST';
 	    me.showTaskViewer = true;
 	} else {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel