[pve-devel] [PATCH v2 manager] close #584: ui qemu: changed remove unused disk to asynchron call

[Thomas Lamprecht]

On 10/17/18 1:54 PM, Thomas Lamprecht wrote:
> On 10/17/18 1:24 PM, Tim Marx wrote:
>> changed commit message to be more specific
>> added condition to only spawn tasks if its a unused disk as originally intended
>>
>> Signed-off-by: Tim Marx <t.marx at proxmox.com>
>> ---
>>  www/manager6/qemu/HardwareView.js | 18 ++++++++++++++++--
>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
>> index a1bccc3c..99aae6ed 100644
>> --- a/www/manager6/qemu/HardwareView.js
>> +++ b/www/manager6/qemu/HardwareView.js
>> @@ -376,7 +376,7 @@ Ext.define('PVE.qemu.HardwareView', {
>>  		Proxmox.Utils.API2Request({
>>  		    url: '/api2/extjs/' + baseurl,
>>  		    waitMsgTarget: me,
>> -		    method: 'PUT',
>> +		    method: rec.data.key.match(/^unused\d+$/) ? 'POST': 'PUT',
>>  		    params: {
>>  			'delete': rec.data.key
>>  		    },
>> @@ -385,7 +385,21 @@ Ext.define('PVE.qemu.HardwareView', {
>>  		    },
>>  		    failure: function (response, opts) {
>>  			Ext.Msg.alert('Error', response.htmlStatus);
>> -		    }
>> +		    },
>> +		    success: function(response, options) {
>> +			var upid = response.result.data;
>> +			if (upid !== null) {
>> +			    var win = Ext.create('Proxmox.window.TaskProgress', {
>> +				upid: upid,
>> +				listeners: {
>> +				    destroy: function () {
>> +					me.reload();
>> +				    }
>> +				}
>> +			    });
>> +			    win.show();
>> +			}
>> +		    },
> 
> trailing ',' here (yeah I know, I'd like them too, but some IE version can't handle them)
> please run `make lint` in the www/manager directory, it should run through without errors.
> 
>>  		});
>>  	    },
>>  	    listeners: {> 
> 
> 
> Looks much better now, thanks!
> But we have a isUsedDisk check already in a helper method (set_button_status), which
> sets the remove button's text, so maybe there is the better place to handle this?
> 
> 
> A short, not to much tested, version could be:
> 
> (one could just keep callback and always reload immediately, even if we do a async POST,
> could make it simpler)
> 
> what do you think?
> 
> ----8<----
> diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
> index a1bccc3c..3cebdb0e 100644
> --- a/www/manager6/qemu/HardwareView.js
> +++ b/www/manager6/qemu/HardwareView.js
> @@ -355,6 +355,7 @@ Ext.define('PVE.qemu.HardwareView', {
>             altText: gettext('Detach'),
>             selModel: sm,
>             disabled: true,
> +           RESTMethod: 'PUT',
>             dangerous: true,
>             confirmMsg: function(rec) {
>                 var warn = gettext('Are you sure you want to remove entry {0}');
> @@ -376,15 +377,29 @@ Ext.define('PVE.qemu.HardwareView', {
>                 Proxmox.Utils.API2Request({
>                     url: '/api2/extjs/' + baseurl,
>                     waitMsgTarget: me,
> -                   method: 'PUT',
> +                   method: b.RESTMethod,
>                     params: {
>                         'delete': rec.data.key
>                     },
> -                   callback: function() {
> -                       reload();
> -                   },
>                     failure: function (response, opts) {
>                         Ext.Msg.alert('Error', response.htmlStatus);
> +                       reload();
> +                   },
> +                   success: function(response, options) {
> +                       if (b.RESTMethod === 'POST') {
> +                           var upid = response.result.data;
> +                           var win = Ext.create('Proxmox.window.TaskProgress', {
> +                               upid: upid,
> +                               listeners: {
> +                                   destroy: function () {
> +                                       me.reload();
> +                                   }
> +                               }
> +                           });
> +                           win.show();
> +                       } else {
> +                           reload();
> +                       }
>                     }
>                 });
>             },
> @@ -498,6 +513,7 @@ Ext.define('PVE.qemu.HardwareView', {
> 
>             remove_btn.setDisabled(rec.data['delete'] || (rowdef.never_delete === true));
>             remove_btn.setText((isUsedDisk && !isCloudInit) ? remove_btn.altText : remove_btn.defaultText);
> +           remove_btn.RESTMethod = isUsedDisk ? 'PUT' : 'POST';

This is wrong, I initially read isUnused and was confused - sorry, should be:

var isUnused = key.match(/^unused\d+/);

remove_btn.RESTMethod = isUnused ? 'POST' : 'PUT';

(I moved the check in it's own variable because we check it for isUsedDisk too and can then reuse it)