[pve-devel] [PATCH manager v3] ui: storage: combine RBD external and hyperconverged add dialog

[Dominik Csapak]

looks good, some comments:

i would prefer that the 'not editable' boxes do not get disabled,
but shown in a display field (we could have 2 components for those, and 
hide/show the correct one) because the contrast is very low for those 
disabled labels

also like we talked off-list, it would probably be good to try to 
autodetect if a pve-managed ceph is available (with the pool selector; 
if none is available, we cannot configure a pve-managed rbd storage anyway)

otherwise: great :)

On 07/03/2018 03:55 PM, Thomas Lamprecht wrote:
> Combine both dialogues. This not only helps to reuse code but also
> reduces storage choices from the Storage -> Add menu, and thus
> improves usability.
> 
> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> ---
> 
> changes v2 -> v3:
> * don't show pveceph checkbox when editing
> * address the issue that displayfields have no submitValue config getter/setter
>    and thus cannot bind on those - as the bind mixin handles binding keys with
>    null/undefined value poorly we need to actively diferrentiate between those
>    cases, thus add a small closure helper
> 
>   www/manager6/Utils.js           |  11 +--
>   www/manager6/dc/StorageView.js  |   4 -
>   www/manager6/storage/RBDEdit.js | 148 ++++++++++++++++++++++----------
>   3 files changed, 104 insertions(+), 59 deletions(-)
> 
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index ad5a0a61..db240eda 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -430,17 +430,12 @@ Ext.define('PVE.Utils', { utilities: {
>   	rbd: {
>   	    name: 'RBD',
>   	    ipanel: 'RBDInputPanel',
> -	    hideAdd: true,
> -	    faIcon: 'building'
> -	},
> -	rbd_ext: {
> -	    name: 'RBD (external)',
> -	    ipanel: 'RBDInputPanel',
>   	    faIcon: 'building'
>   	},
>   	pveceph: {
>   	    name: 'RBD (PVE)',
> -	    ipanel: 'PVERBDInputPanel',
> +	    ipanel: 'RBDInputPanel',
> +	    hideAdd: true,
>   	    faIcon: 'building'
>   	},
>   	zfs: {
> @@ -461,7 +456,7 @@ Ext.define('PVE.Utils', { utilities: {
>   
>       format_storage_type: function(value, md, record) {
>   	if (value === 'rbd' && record) {
> -	    value = (record.get('monhost')?'rbd_ext':'pveceph');
> +	    value = (record.get('monhost') ? 'rbd' : 'pveceph');
>   	}
>   
>   	var schema = PVE.Utils.storageSchema[value];
> diff --git a/www/manager6/dc/StorageView.js b/www/manager6/dc/StorageView.js
> index 57214b7f..cc1e3ed6 100644
> --- a/www/manager6/dc/StorageView.js
> +++ b/www/manager6/dc/StorageView.js
> @@ -54,10 +54,6 @@ Ext.define('PVE.dc.StorageView', {
>   	    var type = rec.data.type,
>   	        sid = rec.data.storage;
>   
> -	    if (type === 'rbd' && !rec.data.monhost) {
> -		type = 'pveceph';
> -	    }
> -
>   	    me.createStorageEditWindow(type, sid);
>   	};
>   
> diff --git a/www/manager6/storage/RBDEdit.js b/www/manager6/storage/RBDEdit.js
> index d26a6ac3..e862f036 100644
> --- a/www/manager6/storage/RBDEdit.js
> +++ b/www/manager6/storage/RBDEdit.js
> @@ -1,6 +1,44 @@
> +/*jslint confusion: true*/
>   Ext.define('PVE.storage.RBDInputPanel', {
>       extend: 'PVE.panel.StorageBase',
>   
> +    viewModel: {
> +	parent: null,
> +	data: {
> +	    pveceph: true
> +	}
> +    },
> +
> +    controller: {
> +	xclass: 'Ext.app.ViewController',
> +	control: {
> +	    'textfield[name=username]': {
> +		disable: 'resetField'
> +	    },
> +	    'textfield[name=monhost]': {
> +		disable: 'disableMonField',
> +		enable: 'enableMonField'
> +	    }
> +	},
> +	resetField: function(field) {
> +	    field.reset();
> +	},
> +	enableMonField: function(field) {
> +	    field.setEmptyText('');
> +	},
> +	disableMonField: function(field) {
> +	    field.reset();
> +	    field.setEmptyText(gettext('Autodetected'));
> +	}
> +    },
> +
> +    setValues: function(values) {
> +	if (values.monhost) {
> +	    this.viewModel.set('pveceph', false);
> +	}
> +	this.callParent([values]);
> +    },
> +
>       initComponent : function() {
>   	var me = this;
>   
> @@ -9,48 +47,58 @@ Ext.define('PVE.storage.RBDInputPanel', {
>   	}
>   	me.type = 'rbd';
>   
> -	me.column1 = [];
> +	var getBinds = function (activeIfPVECeph, hide) {
> +	    var bind = {
> +		disabled: activeIfPVECeph ? '{!pveceph}' : '{pveceph}'
> +	    };
>   
> -	if (me.pveceph) {
> -	    me.column1.push(
> -		{
> -		    xtype: me.isCreate ? 'pveCephPoolSelector' : 'displayfield',
> -		    nodename: me.nodename,
> -		    name: 'pool',
> -		    fieldLabel: gettext('Pool'),
> -		    allowBlank: false
> -		}
> -	    );
> -	} else {
> -	    me.column1.push(
> -		{
> -		    xtype: me.isCreate ? 'textfield' : 'displayfield',
> -		    name: 'pool',
> -		    value: 'rbd',
> -		    fieldLabel: gettext('Pool'),
> -		    allowBlank: false
> -		},
> -		{
> -		    xtype: me.isCreate ? 'textfield' : 'displayfield',
> -		    name: 'monhost',
> -		    vtype: 'HostList',
> -		    value: '',
> -		    fieldLabel: 'Monitor(s)',
> -		    allowBlank: false
> -		},
> -		{
> -		    xtype: me.isCreate ? 'textfield' : 'displayfield',
> -		    name: 'username',
> -		    value: me.isCreate ? 'admin': '',
> -		    fieldLabel: gettext('User name'),
> -		    allowBlank: true
> -		}
> -	    );
> -	}
> +	    // displayfield has no submitValue and bind mixin cannot handle that
> +	    if (me.isCreate) {
> +		bind.submitValue = activeIfPVECeph ? '{pveceph}' : '{!pveceph}';
> +	    }
> +	    if (hide) {
> +		bind.hidden = activeIfPVECeph ? '{!pveceph}' : '{pveceph}';
> +	    }
> +
> +	    return bind;
> +	};
> +
> +	me.column1 = [
> +	    {
> +		xtype: me.isCreate ? 'pveCephPoolSelector' : 'displayfield',
> +		nodename: me.nodename,
> +		name: 'pool',
> +		bind: getBinds(true, true),
> +		fieldLabel: gettext('Pool'),
> +		allowBlank: false
> +	    },
> +	    {
> +		xtype: me.isCreate ? 'textfield' : 'displayfield',
> +		name: 'pool',
> +		value: 'rbd',
> +		bind: getBinds(false, true),
> +		fieldLabel: gettext('Pool'),
> +		allowBlank: false
> +	    },
> +	    {
> +		xtype: 'textfield',
> +		name: 'monhost',
> +		vtype: 'HostList',
> +		bind: getBinds(false),
> +		value: '',
> +		fieldLabel: 'Monitor(s)',
> +		allowBlank: false
> +	    },
> +	    {
> +		xtype: me.isCreate ? 'textfield' : 'displayfield',
> +		name: 'username',
> +		bind: getBinds(false),
> +		value: 'admin',
> +		fieldLabel: gettext('User name'),
> +		allowBlank: true
> +	    }
> +	];
>   
> -	// here value is an array,
> -	// while before it was a string
> -	/*jslint confusion: true*/
>   	me.column2 = [
>   	    {
>   		xtype: 'pveContentTypeSelector',
> @@ -68,14 +116,20 @@ Ext.define('PVE.storage.RBDInputPanel', {
>   		fieldLabel: 'KRBD'
>   	    }
>   	];
> -	/*jslint confusion: false*/
> +
> +	me.columnB = [{
> +	    xtype: 'proxmoxcheckbox',
> +	    name: 'pveceph',
> +	    bind : {
> +		value: '{pveceph}'
> +	    },
> +	    checked: true,
> +	    uncheckedValue: 0,
> +	    submitValue: false,
> +	    hidden: !me.isCreate,
> +	    boxLabel: gettext('Use Proxmox VE Hyperconverged managed ceph pool')
> +	}];
>   
>   	me.callParent();
>       }
>   });
> -
> -Ext.define('PVE.storage.PVERBDInputPanel', {
> -    extend: 'PVE.storage.RBDInputPanel',
> -
> -    pveceph: 1
> -});
>