[pve-devel] [v2 manager 25/27] add a 'create storage' button for ceph pools

[Thomas Lamprecht]

On 08/29/2017 01:04 PM, Fabian Grünbichler wrote:
> From: Dominik Csapak <d.csapak at proxmox.com>
> 
> with this, you can create a pveceph managed storage for a ceph pool
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> ---
> changes since Dominik's v1:
> - adapted API path for storageS
> 
>   www/manager6/ceph/Pool.js | 67 ++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/www/manager6/ceph/Pool.js b/www/manager6/ceph/Pool.js
> index 1f304df2..5cb70814 100644
> --- a/www/manager6/ceph/Pool.js
> +++ b/www/manager6/ceph/Pool.js
> @@ -70,6 +70,50 @@ Ext.define('PVE.CephCreatePool', {
>       }
>   });
>   
> +Ext.define('PVE.CephCreatePoolStorage', {
> +    extend: 'PVE.window.Edit',
> +    alias: 'widget.pveCephCreatePoolStorage',

suggesting class name: CephAddPoolAsStorage
fits more the semantics and used API entry, IMO

> +
> +    subject: 'Ceph Pool Storage',
> +    isCreate: true,
> +    method: 'POST',
> +    items: [
> +	{
> +	    xtype: 'textfield',
> +	    fieldLabel: 'ID',
> +	    name: 'storage',
> +	    vtype: 'StorageId',
> +	    allowBlank: false
> +	},
> +	{
> +	    xtype: 'pvecheckbox',
> +	    fieldLabel: 'KRBD',
> +	    name: 'krbd'
> +	}
> +    ],
> +    initComponent : function() {
> +	 /*jslint confusion: true */

unnecessary jslint confusion - this is highly probably a copy paste result as
the "PVE.CephCreatePool" class above (outside of this patch context) has this
too, maybe a remainder from the past.

> +        var me = this;
> +
> +	if (!me.nodename) {
> +	    throw "no node name specified";
> +	}
> +
> +	if (!me.pool) {
> +	    throw "no pool defined";
> +	}
> +
> +        Ext.apply(me, {
> +	    url: "/nodes/" + me.nodename + "/ceph/pools/" + me.pool + '/storages',
> +	    defaults: {
> +		nodename: me.nodename
> +	    }
> +        });
> +
> +        me.callParent();
> +    }
> +});
> +
>   Ext.define('PVE.node.CephPoolList', {
>       extend: 'Ext.grid.GridPanel',
>       alias: 'widget.pveNodeCephPoolList',
> @@ -202,10 +246,31 @@ Ext.define('PVE.node.CephPoolList', {
>   	    }
>   	});
>   
> +	var storages_btn = Ext.create('PVE.button.Button', {
> +	    text: gettext('Create Storage'),

Same as in patch 23 (add create storages checkbox to ceph pool creation) here:

With this patch applied we have:

[ Create ] [ Remove ] [ Create Storage ]

In the Pools panel header, which is again confusing.

Suggesting something in the like of: 'Add as/to Storage' or 'Configure as/to Storage'
I prefer 'Add..' because we use that wording normally in this context.

I notice that the lack of context specific help buttons for the Ceph stuff in general.
This is not a critique to this patch series, just something I noticed in general.
We should try to improve that.



> +	    selModel: sm,
> +	    disabled: true,
> +	    handler: function() {
> +		var rec = sm.getSelection()[0];
> +
> +		if (!rec.data.pool_name) {
> +		    return;
> +		}
> +		var win = Ext.create('PVE.CephCreatePoolStorage', {
> +                    nodename: nodename,
> +		    pool: rec.data.pool_name
> +		});
> +		win.show();
> +		win.on('destroy', function() {
> +		    rstore.load();
> +		});
> +	    }
> +	});
> +
>   	Ext.apply(me, {
>   	    store: store,
>   	    selModel: sm,
> -	    tbar: [ create_btn, remove_btn ],
> +	    tbar: [ create_btn, remove_btn, storages_btn ],
>   	    listeners: {
>   		activate: rstore.startUpdate,
>   		destroy: rstore.stopUpdate
>