public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH v2 proxmox-backup] gui: Add button for changing backup group owner
@ 2020-10-29  9:16 Dylan Whyte
  2020-10-29 10:30 ` Thomas Lamprecht
  0 siblings, 1 reply; 2+ messages in thread
From: Dylan Whyte @ 2020-10-29  9:16 UTC (permalink / raw)
  To: pbs-devel

Extension of fix #2847

Adds an action button to the datastore content view,
to change the owner of a backup.

Signed-off-by: Dylan Whyte <d.whyte@proxmox.com>
---

v2:
- Fix whitespace
- BackupGroupChangeOwner: remove unnecessary reload function
- Use let insread of var in "onGetValues"
- Use template literals for URL
- Set autoShow true when creating the window

In response to comment about reassigning view in onChangeOwner, the
view that is passed to the handler function (Ext.view.Table) is
necessary in the handler definiation/call, but is not used
within the implementaion of onChangeOwner. Thus it is reassigned to the
change owner window view. This follows the format of the other action
handlers in the file.

 www/BackupGroupChangeOwner.js | 74 +++++++++++++++++++++++++++++++++++
 www/DataStoreContent.js       | 27 ++++++++++++-
 www/Makefile                  |  1 +
 3 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 www/BackupGroupChangeOwner.js

diff --git a/www/BackupGroupChangeOwner.js b/www/BackupGroupChangeOwner.js
new file mode 100644
index 00000000..0d24bfe0
--- /dev/null
+++ b/www/BackupGroupChangeOwner.js
@@ -0,0 +1,74 @@
+Ext.define('PBS.ChangeOwnerInputPanel', {
+    extend: 'Proxmox.panel.InputPanel',
+    alias: 'widget.pbsChangeOwnerInputPanel',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    onGetValues: function(values) {
+	let me = this;
+
+	values["backup-type"] = me.backup_type;
+	values["backup-id"] = me.backup_id;
+	return values;
+    },
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	init: function(view) {
+	    if (!view.url) {
+		throw "no url specified";
+	    }
+	    if (!view.backup_type) {
+		throw "no backup_type specified";
+	    }
+	    if (!view.backup_id) {
+		throw "no backup_id specified";
+	    }
+	},
+    },
+
+    column1: [
+	{
+	    xtype: 'textfield',
+	    name: 'new-owner',
+	    fieldLabel: gettext('Userid'),
+	    minLength: 8,
+	    allowBlank: false,
+	},
+    ],
+
+});
+
+Ext.define('PBS.BackupGroupChangeOwner', {
+    extend: 'Proxmox.window.Edit',
+
+    method: 'POST',
+    submitText: "Change Owner",
+
+    initComponent: function() {
+	var me = this;
+
+	if (!me.datastore) {
+	    throw "no datastore specified";
+	}
+	if (!me.backup_type) {
+	    throw "no backup_type specified";
+	}
+	if (!me.backup_id) {
+	    throw "no backup_id specified";
+	}
+
+	Ext.apply(me, {
+	    url: `/api2/extjs/admin/datastore/${me.datastore}/change-owner`,
+	    title: "Change Owner",
+	    items: [{
+		xtype: "pbsChangeOwnerInputPanel",
+		url: `/api2/extjs/admin/datastore/${me.datastore}/change-owner`,
+		backup_type: me.backup_type,
+		backup_id: me.backup_id,
+	    }],
+	});
+
+	me.callParent();
+    },
+});
diff --git a/www/DataStoreContent.js b/www/DataStoreContent.js
index 9da18845..35aa2b11 100644
--- a/www/DataStoreContent.js
+++ b/www/DataStoreContent.js
@@ -268,6 +268,25 @@ Ext.define('PBS.DataStoreContent', {
 	    }
 	},
 
+	onChangeOwner: function(view, rI, cI, item, e, rec) {
+	    view = this.getView();
+
+	    if (!rec || !rec.data || rec.parentNode.id !== 'root' || !view.datastore) {
+		return;
+	    }
+
+	    let data = rec.data;
+
+	    let win = Ext.create('PBS.BackupGroupChangeOwner', {
+		datastore: view.datastore,
+		backup_type: data.backup_type,
+		backup_id: data.backup_id,
+		autoShow: true,
+	    });
+	    win.on('destroy', this.reload, this);
+	    win.show();
+	},
+
 	onPrune: function(view, rI, cI, item, e, rec) {
 	    view = this.getView();
 
@@ -582,7 +601,13 @@ Ext.define('PBS.DataStoreContent', {
 		    getTip: (v, m, rec) => Ext.String.format(gettext("Verify '{0}'"), v),
 		    getClass: (v, m, rec) => rec.data.leaf ? 'pmx-hidden' : 'pve-icon-verify-lettering',
 		    isDisabled: (v, r, c, i, rec) => !!rec.data.leaf,
-		},
+                },
+                {
+		    handler: 'onChangeOwner',
+		    tooltip: gettext('Change Owner'),
+		    getClass: (v, m, rec) => rec.parentNode.id ==='root' ? 'fa fa-user' : 'pmx-hidden',
+		    isDisabled: (v, r, c, i, rec) => rec.parentNode.id !=='root',
+                },
 		{
 		    handler: 'onPrune',
 		    getTip: (v, m, rec) => Ext.String.format(gettext("Prune '{0}'"), v),
diff --git a/www/Makefile b/www/Makefile
index cba8bed5..aabc8d9c 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -46,6 +46,7 @@ JSSRC=							\
 	DataStorePrune.js				\
 	DataStoreContent.js				\
 	DataStorePanel.js				\
+	BackupGroupChangeOwner.js			\
 	ServerStatus.js					\
 	ServerAdministration.js				\
 	Dashboard.js					\
-- 
2.20.1





^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [pbs-devel] [PATCH v2 proxmox-backup] gui: Add button for changing backup group owner
  2020-10-29  9:16 [pbs-devel] [PATCH v2 proxmox-backup] gui: Add button for changing backup group owner Dylan Whyte
@ 2020-10-29 10:30 ` Thomas Lamprecht
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Lamprecht @ 2020-10-29 10:30 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dylan Whyte

On 29.10.20 10:16, Dylan Whyte wrote:
> Extension of fix #2847
> 
> Adds an action button to the datastore content view,
> to change the owner of a backup.
> 
> Signed-off-by: Dylan Whyte <d.whyte@proxmox.com>
> ---
> 
> v2:
> - Fix whitespace
> - BackupGroupChangeOwner: remove unnecessary reload function
> - Use let insread of var in "onGetValues"
> - Use template literals for URL
> - Set autoShow true when creating the window
> 
> In response to comment about reassigning view in onChangeOwner, the
> view that is passed to the handler function (Ext.view.Table) is
> necessary in the handler definiation/call, but is not used
> within the implementaion of onChangeOwner. Thus it is reassigned to the
> change owner window view. This follows the format of the other action
> handlers in the file.

getting quite there, seems to work out OK, a few comments left, mostly regarding
code structure and missing gettext's - sorry to overlook those when commenting
v1.

> 
>  www/BackupGroupChangeOwner.js | 74 +++++++++++++++++++++++++++++++++++
>  www/DataStoreContent.js       | 27 ++++++++++++-
>  www/Makefile                  |  1 +
>  3 files changed, 101 insertions(+), 1 deletion(-)
>  create mode 100644 www/BackupGroupChangeOwner.js
> 
> diff --git a/www/BackupGroupChangeOwner.js b/www/BackupGroupChangeOwner.js
> new file mode 100644
> index 00000000..0d24bfe0
> --- /dev/null
> +++ b/www/BackupGroupChangeOwner.js
> @@ -0,0 +1,74 @@
> +Ext.define('PBS.ChangeOwnerInputPanel', {
> +    extend: 'Proxmox.panel.InputPanel',
> +    alias: 'widget.pbsChangeOwnerInputPanel',
> +    mixins: ['Proxmox.Mixin.CBind'],

not sure if it's worth to define this as separate component with only one single
field as element?

I'd rather declare it inline in the edit window definition below, the params
checking from init here is done there anyway,

You could check out 'PBS.window.VerifyJobEdit' as example for this.

> +
> +    onGetValues: function(values) {
> +	let me = this;
> +
> +	values["backup-type"] = me.backup_type;
> +	values["backup-id"] = me.backup_id;
> +	return values;
> +    },
> +
> +    controller: {
> +	xclass: 'Ext.app.ViewController',
> +
> +	init: function(view) {
> +	    if (!view.url) {
> +		throw "no url specified";
> +	    }
> +	    if (!view.backup_type) {
> +		throw "no backup_type specified";
> +	    }
> +	    if (!view.backup_id) {
> +		throw "no backup_id specified";
> +	    }
> +	},
> +    },
> +
> +    column1: [
> +	{
> +	    xtype: 'textfield',
> +	    name: 'new-owner',
> +	    fieldLabel: gettext('Userid'),
> +	    minLength: 8,
> +	    allowBlank: false,
> +	},
> +    ],
> +
> +});
> +
> +Ext.define('PBS.BackupGroupChangeOwner', {
> +    extend: 'Proxmox.window.Edit',
> +
> +    method: 'POST',
> +    submitText: "Change Owner",

use gettext

> +
> +    initComponent: function() {
> +	var me = this;
> +
> +	if (!me.datastore) {
> +	    throw "no datastore specified";
> +	}
> +	if (!me.backup_type) {
> +	    throw "no backup_type specified";
> +	}
> +	if (!me.backup_id) {
> +	    throw "no backup_id specified";
> +	}
> +
> +	Ext.apply(me, {
> +	    url: `/api2/extjs/admin/datastore/${me.datastore}/change-owner`,
> +	    title: "Change Owner",

title can be above, in declarative config part, also, please use gettext

> +	    items: [{
> +		xtype: "pbsChangeOwnerInputPanel",
> +		url: `/api2/extjs/admin/datastore/${me.datastore}/change-owner`,
> +		backup_type: me.backup_type,
> +		backup_id: me.backup_id,
> +	    }],
> +	});
> +
> +	me.callParent();
> +    },
> +});
> diff --git a/www/DataStoreContent.js b/www/DataStoreContent.js
> index 9da18845..35aa2b11 100644
> --- a/www/DataStoreContent.js
> +++ b/www/DataStoreContent.js
> @@ -268,6 +268,25 @@ Ext.define('PBS.DataStoreContent', {
>  	    }
>  	},
>  
> +	onChangeOwner: function(view, rI, cI, item, e, rec) {
> +	    view = this.getView();
> +
> +	    if (!rec || !rec.data || rec.parentNode.id !== 'root' || !view.datastore) {
> +		return;
> +	    }
> +
> +	    let data = rec.data;
> +
> +	    let win = Ext.create('PBS.BackupGroupChangeOwner', {
> +		datastore: view.datastore,
> +		backup_type: data.backup_type,
> +		backup_id: data.backup_id,
> +		autoShow: true,
> +	    });
> +	    win.on('destroy', this.reload, this);
> +	    win.show();

once you set autoShow above win.show call is not required anymore ;-)

> +	},
> +
>  	onPrune: function(view, rI, cI, item, e, rec) {
>  	    view = this.getView();
>  
> @@ -582,7 +601,13 @@ Ext.define('PBS.DataStoreContent', {
>  		    getTip: (v, m, rec) => Ext.String.format(gettext("Verify '{0}'"), v),
>  		    getClass: (v, m, rec) => rec.data.leaf ? 'pmx-hidden' : 'pve-icon-verify-lettering',
>  		    isDisabled: (v, r, c, i, rec) => !!rec.data.leaf,
> -		},
> +                },
> +                {
> +		    handler: 'onChangeOwner',
> +		    tooltip: gettext('Change Owner'),

I recently adapted those to a more dynamic one, showing the snapshot/group ID
inside the tooltip by using the getTip config property.

Maybe it makes sense to adapt that here too?

getTip: (v, m, rec) => Ext.String.format(gettext("Change owner of '{0}'"), v),


> +		    getClass: (v, m, rec) => rec.parentNode.id ==='root' ? 'fa fa-user' : 'pmx-hidden',
> +		    isDisabled: (v, r, c, i, rec) => rec.parentNode.id !=='root',
> +                },
>  		{
>  		    handler: 'onPrune',
>  		    getTip: (v, m, rec) => Ext.String.format(gettext("Prune '{0}'"), v),
> diff --git a/www/Makefile b/www/Makefile
> index cba8bed5..aabc8d9c 100644
> --- a/www/Makefile
> +++ b/www/Makefile
> @@ -46,6 +46,7 @@ JSSRC=							\
>  	DataStorePrune.js				\
>  	DataStoreContent.js				\
>  	DataStorePanel.js				\
> +	BackupGroupChangeOwner.js			\
>  	ServerStatus.js					\
>  	ServerAdministration.js				\
>  	Dashboard.js					\
> 






^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-10-29 10:30 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-29  9:16 [pbs-devel] [PATCH v2 proxmox-backup] gui: Add button for changing backup group owner Dylan Whyte
2020-10-29 10:30 ` Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal