[pve-devel] [manager 6/6] Add imageRemoveButton

[Thomas Lamprecht]

On 11/21/18 5:05 PM, Wolfgang Link wrote:
> If an image has a <vmid>  encoded in the image name
> and the guest does not exist in the cluster
> we can delete it on the GUI.
> 
> Also, if a config exists on another node and the storage is local
> we can delete.
> 
> In all other cases, it is not allowed to delete it.
> 
> For safety reason the safe remove windows are used.

works OK, I guess, some style and grammar nits below.

> ---
>  www/manager6/storage/ContentView.js | 55 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
> index 8716fba3..0b2bce4d 100644
> --- a/www/manager6/storage/ContentView.js
> +++ b/www/manager6/storage/ContentView.js
> @@ -405,6 +405,8 @@ Ext.define('PVE.storage.ContentView', {
>  	    selModel: sm,
>  	    enableFn: function(rec) {
>  		if (rec && rec.data.content !== 'images') {
> +		    imageRemoveButton.setVisible(false);
> +		    removeButton.setVisible(true);
>  		    return true;
>  		}
>  		return false;
> @@ -415,6 +417,58 @@ Ext.define('PVE.storage.ContentView', {
>  	    baseurl: baseurl + '/'
>  	});
>  

You could do this with one remove button, keep the remove button as is, remove it's
enableFN, and do something like the following in the handler:

handler: function(btn, event, rec) {
                var me = this;

                // let StdRemoveButton handle this if we're not a disk image
                if (rec && rec.data.content !== 'images') {
                    me.callParent(arguments);
                    return;
                }

                var url = baseurl + '/' + rec.data.volid;
                var vmid = rec.data.vmid;
                var storage_path = 'storage/' + nodename + '/' + storage;
		  // continue with the image remove special handling, maybe move that to it's
                // own function to keep the handler a bit shorter.


allows to ommit patch 5/6 and the exclusive or visibility toggling of the buttons.


> +	var imageRemoveButton = Ext.create('Proxmox.button.Button',{
> +	    selModel: sm,
> +	    hidden: true,
> +	    text: gettext('Remove'),
> +	    enableFn: function(rec) {
> +		if (rec && rec.data.content === 'images') {
> +		    removeButton.setVisible(false);
> +		    imageRemoveButton.setVisible(true);
> +		    return true;
> +		}
> +		return false;
> +	    },
> +	    handler: function(btn, event, rec) {
> +		me = this;
> +
> +		var url = baseurl + '/' + rec.data.volid;
> +		var vmid = rec.data.vmid;
> +		var storage_path = 'storage/' + nodename + '/' + storage;
> +
> +		var store = PVE.data.ResourceStore;
> +		var vmid_exists = vmid && store.findVMID(vmid);
> +		if (vmid_exists) {
> +		    var guest_node = store.guestNode(vmid);
> +		    var storage_is_shared = store.storageIsShared(storage_path);
> +		    if (storage_is_shared || guest_node == nodename) {

maybe a comment would be nice above, something like

// allow to delete local backed images if a VMID exists on another node.

> +			var msg = '<p><b>' +
> +			    gettext("Can't remove image Guest exists.")



Cannot remove image, a guest with its VMID ({}) exists!
(suggestion only)

> +			    + '</p></b>' +
> +			    gettext('Remove Image from guest hardware configuration.');

You can delete this from the guest's hardware panel.

also maybe just to a single <br /> between the lines instead of a paragraph?

> +			Ext.Msg.show({
> +			    title: gettext('Not Allowed to remove the image'),

Cannot remove disk image.

> +			    icon: Ext.Msg.ERROR,
> +			    msg: msg,
> +			});
> +			return;
> +		    }
> +		}
> +		var win = Ext.create('PVE.window.SafeDestroy', {
> +		    showProgress: true,
> +		    url: url,
> +		    item: { type: 'Image', id: vmid },
> +		}).show();
> +		win.on('destroy', function() {
> +		    me.statusStore = Ext.create('Proxmox.data.ObjectStore', {
> +			url: '/api2/json/nodes/' + nodename + '/storage/' + storage + '/status'
> +		    });
> +		    reload();
> +
> +		});
> +	    },
> +	});
> +
>  	me.statusStore = Ext.create('Proxmox.data.ObjectStore', {
>  	    url: '/api2/json/nodes/' + nodename + '/storage/' + storage + '/status'
>  	});
> @@ -452,6 +506,7 @@ Ext.define('PVE.storage.ContentView', {
>  		    }
>  		},
>  		removeButton,
> +		imageRemoveButton,
>  		templateButton,
>  		uploadButton,
>  		{
>