[pve-devel] [PATCH manager v3] ui: improve user experience for vm/container transfer between pools

[pve-devel] [PATCH manager v3] ui: improve user experience for vm/container transfer between pools

[Philipp Hufnagl]

After the implementation of fix #474, it has been suggested that
instead of requiring the user to click a checkbox allowing migration,
it should be allowed automatically and and a warning should be displayed

Further it has been discussed to rename the feature from "transfer" to
"migrate". However and API change would break already implemented usage
and so it has been decided to call it (for now) transfer everywhere to

Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
---
 www/manager6/grid/PoolMembers.js | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/www/manager6/grid/PoolMembers.js b/www/manager6/grid/PoolMembers.js
index 224daca3..fe0de181 100644
--- a/www/manager6/grid/PoolMembers.js
+++ b/www/manager6/grid/PoolMembers.js
@@ -4,6 +4,10 @@ Ext.define('PVE.pool.AddVM', {
     height: 420,
     isAdd: true,
     isCreate: true,
+    extraRequestParams: {
+	transfer: 1,
+    },
+
     initComponent: function() {
 	var me = this;
 
@@ -35,6 +39,13 @@ Ext.define('PVE.pool.AddVM', {
 	    ],
 	});
 
+	let transferWarning = Ext.create('Ext.form.field.Display', {
+	    userCls: 'pmx-hint',
+	    value: gettext('Virtual guests are removed from current pools'),
+	    hidden: true,
+	});
+
+
 	var vmGrid = Ext.create('widget.grid', {
 	    store: vmStore,
 	    border: true,
@@ -46,9 +57,15 @@ Ext.define('PVE.pool.AddVM', {
 		listeners: {
 		    selectionchange: function(model, selected, opts) {
 			var selectedVms = [];
+			var isTransfer = false;
 			selected.forEach(function(vm) {
 			    selectedVms.push(vm.data.vmid);
+			    if (vm.data.pool?.length) {
+				isTransfer = true;
+			    }
 			});
+			this.transfer.setValue(isTransfer);
+			transferWarning.setHidden(!isTransfer);
 			vmsField.setValue(selectedVms);
 		    },
 		},
@@ -90,15 +107,10 @@ Ext.define('PVE.pool.AddVM', {
 	    ],
 	});
 
-	let transfer = Ext.create('Ext.form.field.Checkbox', {
-	    name: 'transfer',
-	    boxLabel: gettext('Allow Transfer'),
-	    inputValue: 1,
-	    value: 0,
-	});
+
 	Ext.apply(me, {
 	    subject: gettext('Virtual Machine'),
-	    items: [vmsField, vmGrid, transfer],
+	    items: [vmsField, vmGrid, transferWarning, this.transfer],
 	});
 
 	me.callParent();
-- 
2.39.2

Re: [pve-devel] [PATCH manager v3] ui: improve user experience for vm/container transfer between pools

[Thomas Lamprecht]

Am 08/09/2023 um 15:18 schrieb Philipp Hufnagl:
> After the implementation of fix #474, it has been suggested that
> instead of requiring the user to click a checkbox allowing migration,
> it should be allowed automatically and and a warning should be displayed
> 
> Further it has been discussed to rename the feature from "transfer" to
> "migrate". However and API change would break already implemented usage
> and so it has been decided to call it (for now) transfer everywhere to
> 
> Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
> ---

Missing description of changes since previous patch version here.

Also missing defining the API defaults like asked multiple times, but I
pushed out a changeset [0] to address most of this on Friday before you
sent the v2, so that I was able to bump pve-manager for some other, a
bit pressing, bug fixes w.r.t. notifications stuff that Lukas sent in a
timely manner.

After rethinking it, I also renamed the APÌ parameter. As the package
that added the new parameter was only on pvetest I saw no real concerns
for practical ABI breakage, especially as this is a bit of a niche feature
(so @all please do not use this as general precedence, such things should
be considered case-by-case).

[0]: https://git.proxmox.com/?p=pve-manager.git;a=shortlog;h=57490ff2c6a38448d511d32ec1b4c9bc825545d9