[pve-devel] [PATCH pve-manager] fixes #1503 Add role CRUD to the GUI.

Dominik Csapak d.csapak at proxmox.com
Tue May 8 09:22:40 CEST 2018


looks good in general, a few comments inline (most are nitpicks)

On 05/07/2018 05:41 PM, René Jochum wrote:
> As given in the subject this implements role create/update/delete over
> the manager.
> 
> There's currently no coler highlightning for "special" roles.
> 
> I decided to call "special" roles "Builtin" any feedback on that is much
> appreciated.
> 
> Signed-off-by: René Jochum <r.jochum at proxmox.com>
> ---
>   www/manager6/Makefile                   |  2 +
>   www/manager6/dc/RoleEdit.js             | 60 ++++++++++++++++++++++++++++++
>   www/manager6/dc/RoleView.js             | 66 +++++++++++++++++++++++++++++++--
>   www/manager6/form/PrivilegesSelector.js | 40 ++++++++++++++++++++
>   4 files changed, 164 insertions(+), 4 deletions(-)
>   create mode 100644 www/manager6/dc/RoleEdit.js
>   create mode 100644 www/manager6/form/PrivilegesSelector.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index 81ddcc8d..7e9877b2 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -21,6 +21,7 @@ JSSRC= 				                 	\
>   	form/Boolean.js					\
>   	form/CompressionSelector.js			\
>   	form/PoolSelector.js				\
> +	form/PrivilegesSelector.js			\
>   	form/GroupSelector.js				\
>   	form/UserSelector.js				\
>   	form/RoleSelector.js				\
> @@ -185,6 +186,7 @@ JSSRC= 				                 	\
>   	dc/GroupView.js					\
>   	dc/GroupEdit.js					\
>   	dc/RoleView.js					\
> +	dc/RoleEdit.js					\
>   	dc/ACLView.js					\
>   	dc/AuthView.js					\
>   	dc/AuthEdit.js					\
> diff --git a/www/manager6/dc/RoleEdit.js b/www/manager6/dc/RoleEdit.js
> new file mode 100644
> index 00000000..692a88e0
> --- /dev/null
> +++ b/www/manager6/dc/RoleEdit.js
> @@ -0,0 +1,60 @@
> +Ext.define('PVE.dc.RoleEdit', {
> +    extend: 'Proxmox.window.Edit',
> +    xtype: ['pveDcRoleEdit'],

you can omit the [], just write

xtype:  'yourXtype',


> +
> +    initComponent : function() {
> +	var me = this;
> +
> +	console.log("RoleId: " + me.privs);

i guess this is a leftover?

> +	me.isCreate = !me.roleid;
> +
> +	var url;
> +	var method;
> +
> +	if (me.isCreate) {
> +	    url = '/api2/extjs/access/roles';
> +	    method = 'POST';
> +	} else {
> +	    url = '/api2/extjs/access/roles/' + me.roleid;
> +	    method = 'PUT';
> +	}
> +
> +	Ext.applyIf(me, {
> +	    subject: gettext('Role'),
> +	    url: url,
> +	    method: method,
> +	    items: [
> +		{
> +		    xtype: me.isCreate ? 'proxmoxtextfield' : 'displayfield',
> +		    name: 'roleid',
> +		    value: me.roleid,
> +		    allowBlank: false,
> +		    fieldLabel: gettext('Name')
> +		},
> +		{
> +		    xtype: 'PrivilegesSelector',
> +		    name: 'privs',
> +		    value: me.privs,
> +		    allowBlank: false,
> +		    fieldLabel: gettext('Privileges')
> +		}
> +	    ]
> +	});
> +
> +	me.callParent();
> +
> +	if (!me.isCreate) {
> +	    me.load({
> +		success: function(response) {
> +		    var data = response.result.data;
> +		    var keys = Ext.Object.getKeys(data);
> +
> +		    me.setValues({
> +			privs: keys,
> +			roleid: me.roleid
> +		    });
> +		}
> +	    });
> +	}
> +    }
> +});
> diff --git a/www/manager6/dc/RoleView.js b/www/manager6/dc/RoleView.js
> index 611dfbb6..c1c91151 100644
> --- a/www/manager6/dc/RoleView.js
> +++ b/www/manager6/dc/RoleView.js
> @@ -13,9 +13,9 @@ Ext.define('PVE.dc.RoleView', {
>   
>   	var store = new Ext.data.Store({
>   	    model: 'pve-roles',
> -	    sorters: {
> -		property: 'roleid',
> -		order: 'DESC'
> +	    sorters: {
> +		property: 'roleid',
> +		order: 'DESC'
>   	    }
>   	});
>   
> @@ -33,14 +33,46 @@ Ext.define('PVE.dc.RoleView', {
>   
>   	Proxmox.Utils.monStoreErrors(me, store);
>   
> +	var sm = Ext.create('Ext.selection.RowModel', {});
> +
> +	var reload = function() {
> +		store.load();
> +	};
> +
> +	var run_editor = function() {
> +	    var rec = sm.getSelection()[0];
> +	    if (!rec) {
> +		return;
> +	    }
> +
> +	    if (rec.data.special === "1") {
> +		return;
> +	    }
> +
> +	    var win = Ext.create('PVE.dc.RoleEdit',{
> +		roleid: rec.data.roleid,
> +		privs: rec.data.privs,
> +	    });
> +	    win.on('destroy', reload);
> +	    win.show();
> +	};
> +
>   	Ext.apply(me, {
>   	    store: store,
> +	    selModel: sm,
>   
>   	    viewConfig: {
>   		trackOver: false
>   	    },
>   	    columns: [
>   		{
> +		    header: gettext('Builtin'),
> +		    width: 60,
> +		    sortable: true,
> +		    dataIndex: 'special',
> +		    renderer: Proxmox.Utils.format_boolean
> +		},
> +		{
>   		    header: gettext('Name'),
>   		    width: 150,
>   		    sortable: true,
> @@ -58,8 +90,34 @@ Ext.define('PVE.dc.RoleView', {
>   	    listeners: {
>   		activate: function() {
>   		    store.load();
> +		},
> +		itemdblclick: run_editor,
> +	    },
> +	    tbar: [
> +		{
> +		    text: gettext('Create'),
> +		    handler: function() {
> +			var win = Ext.create('PVE.dc.GroupEdit', {});

this should be 'PVE.dc.RoleEdit'

> +			win.on('destroy', reload);
> +			win.show();
> +		    }
> +		},
> +		{
> +		    xtype: 'proxmoxButton',
> +		    text: gettext('Edit'),
> +		    disabled: true,
> +		    selModel: sm,
> +		    handler: run_editor

for this we probably want an 'enableFn' so that we only enable the edit 
button on non builtin entries, like this:

enableFn: function(record) {
     return record.data.special !== '1';
}

> +		},
> +		{
> +		    xtype: 'proxmoxStdRemoveButton',
> +		    selModel: sm,
> +		    callback: function() {
> +			reload();
> +		    },
> +		    baseurl: '/access/roles/'
>   		}
> -	    }
> +	    ]
>   	});
>   
>   	me.callParent();
> diff --git a/www/manager6/form/PrivilegesSelector.js b/www/manager6/form/PrivilegesSelector.js
> new file mode 100644
> index 00000000..1514c85f
> --- /dev/null
> +++ b/www/manager6/form/PrivilegesSelector.js
> @@ -0,0 +1,40 @@
> +Ext.define('PVE.form.PrivilegesSelector', {
> +    extend: 'Proxmox.form.KVComboBox',
> +    xtype: ['PrivilegesSelector'],

again the []

> +
> +    listConfig:{
> +	minHeight: 50
> +    },

this is not necessary

> +
> +    initComponent: function() {
> +	var me = this;
> +
> +	me.multiSelect = true;

this can be statically defined

> +	

here is trailing whitespace

> +	// So me.store is available.
> +	me.callParent();
> +
> +	Proxmox.Utils.API2Request({
> +	    url: '/access/roles/Administrator',
> +	    method: 'GET',
> +	    success: function(response, options) {
> +		var data = [];
> +		for (var key in response.result.data) {
> +		    data.push([key, key]);
> +		}
> +
> +		data.sort(function(a, b) {
> +		if (a[0] < b[0]) return -1;
> +		if (a[0] > b[0]) return 1;
> +		return 0;
> +		});

this is not necessary, extjs stores can be sorted already like this 
(after setting the data)

me.store.sort({
     property: 'key',
     direction: 'ASC'
});

> +
> +		me.store.setData(data);
> +	    },
> +

also trailing whitespace (even if my mail client removes it when quoting)

> +            failure: function (response, opts) {
> +		Ext.Msg.alert(gettext('Error'), response.htmlStatus);
> +	    }
> +	});
> +    }
> +});
> 




More information about the pve-devel mailing list