Re: [pve-devel] [PATCH manager 1/3] fix #4678: ui: don't sort storage backup content by vmid by default

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

On 20/04/2023 10:06, Dominik Csapak wrote:
> instead, add the vmid as extra column, so that the user can still sort
> by vmid if they wish to
> 

missing Reported-by tag, would potentially also split this 

> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  www/manager6/storage/BackupView.js | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/www/manager6/storage/BackupView.js b/www/manager6/storage/BackupView.js
> index fbdf573d3..9ad1a4915 100644
> --- a/www/manager6/storage/BackupView.js
> +++ b/www/manager6/storage/BackupView.js
> @@ -220,16 +220,20 @@ Ext.define('PVE.storage.BackupView', {
>  		    },
>  		},
>  	    };
> +	} else {
> +	    me.extraColumns = {};

this is a bit odd, would rather set it upfront and change the the isPBS
branch to set each column explicitly, but has nothing to do with your
patch directly and if, the cleanup should happen in a separate one in
any way.

>  	}
> +	me.extraColumns.vmid = {
> +	    header: gettext('VMID'),
> +	    dataIndex: 'vmid',
> +	    hidden: true,
> +	    sorter: (a, b) => a.data.vmid - b.data.vmid,

doesn't this breaks, or at least behaves possibly confusingly, on
custom file names? At least we don't handle the 'vmid' field explicitly
in the 'pve-storage-content' model.

Note that our regex here is very liberal, it basically returns any file
that ends with tar or vma and an optional gz, lzo or zst compressor.

> +	};
>  
>  	me.callParent();
>  
>  	me.store.getSorters().clear();
>  	me.store.setSorters([
> -	    {
> -		property: 'vmid',
> -		direction: 'ASC',
> -	    },
>  	    {
>  		property: 'vdate',
>  		direction: 'DESC',