Re: [pve-devel] [PATCH v2 manager 1/3] ui: restore: disallow empty storage selection if it wouldn't work

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 21.04.22 13:26, Fabian Ebner wrote:
> Namely, if there is a storage in the backup configuration that's not
> available on the current node.

Better than the status quo, but in the long run all the "force all volumes to a single storage"
on restore and also migrate isn't ideal for the case where one or more storages do not exist on
the target node. An per-volume override would be nicer, but may require some gui adaptions to
present that in a sensible way with good UX.

> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> New in v2.
> 
>  www/manager6/window/Restore.js | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/www/manager6/window/Restore.js b/www/manager6/window/Restore.js
> index e7b3e945..25babf89 100644
> --- a/www/manager6/window/Restore.js
> +++ b/www/manager6/window/Restore.js
> @@ -15,6 +15,40 @@ Ext.define('PVE.window.Restore', {
>  		},
>  	    },
>  	},
> +
> +	afterRender: function() {
> +	    let view = this.getView();
> +
> +	    Proxmox.Utils.API2Request({
> +		url: `/nodes/${view.nodename}/vzdump/extractconfig`,
> +		method: 'GET',
> +		params: {
> +		    volume: view.volid,
> +		},
> +		failure: (response) => Ext.Msg.alert('Error', response.htmlStatus),

nit: no parenthesis required for single parameter arrow fns

> +		success: function(response, options) {
> +		    let storagesAvailable = true;
> +
> +		    response.result.data.split('\n').forEach(function(line) {

style nit: makes no big difference but for new code we prefer arrow fns for inline closures.

> +			let match = line.match(
> +			    /^#qmdump#map:(\S+):(\S+):(\S*):(\S*):$/,
> +			);

fits in 100 cc so would rather keep in in a single line

> +			if (match) {
> +			    let currentAvailable = !!PVE.data.ResourceStore.getById(
> +				`storage/${view.nodename}/${match[3]}`,
> +			    );
> +			    storagesAvailable = storagesAvailable && currentAvailable;



JS has a &&= operator, so could be:

storagesAvailable &&= !!PVE.data.ResourceStore.getById(
    `storage/${view.nodename}/${match[3]}`);

but as we only want to do one thing if any storage is not available we could do:

```
let allStoragesAvailable = response.result.data.split('\n').every(line => {
    let match = line.match(/^#qmdump#map:(\S+):(\S+):(\S*):(\S*):$/);
    if (match) {
        return !!PVE.data.ResourceStore.getById(`storage/${view.nodename}/${match[3]}`);
    }
    return true;
}

if (!allStoragesAvailable) {
    // ...
}
```

IMO a bit easier to follow.

> +			}
> +		    });
> +
> +		    if (!storagesAvailable) {
> +			let storagesel = view.down('pveStorageSelector[name=storage]');
> +			storagesel.allowBlank = false;
> +			storagesel.setEmptyText('');
> +		    }
> +		},
> +	    });
> +	},
>      },
>  
>      initComponent: function() {