Re: [RFC PATCH proxmox-backup] ui: always use arrays for 'delete' property

[Christian Ebner <c.ebner@proxmox.com>]

Thanks for the patch, only a tiny nit inline.

On 6/23/26 1:21 PM, Dominik Csapak wrote:
> In PBS we always expect an array for deletable properties, never a
> comma-separated string. The code that assembles this in the gui this was
> carried over from PVE where we often use those.
> 
> Instead of working around in every place where we use `delete_if_default`,
> simply change the implementation to always use arrays.
> 
> Our input panel should always generate an array when having multiple
> 'delete' properties anyway (see 'assemble_field_data' in Utils.js) so
> this could only happen if 'delete_if_default' is called when exactly one
> field itself sent a 'delete: field-name' value.
> 
> Fixes: https://forum.proxmox.com/threads/183130/post-858403
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>

> ---
> sending as RFC because i might have overlooked something here.
> This is an alternative to Christians patch here:
> https://lore.proxmox.com/pbs-devel/20260623100525.69448-1-c.ebner@proxmox.com/T/#u
> 
> i omitted the 'fixes' tag for the specific issue the forum fixes since
> it does quite a bit more than that...
> 
> feel free to add it on apply or tell me to send a new version with it,
> if it's preferred to have that ;)
> 
>   www/Utils.js                     | 3 ++-
>   www/window/NotifyOptions.js      | 4 ----
>   www/window/PruneJobEdit.js       | 5 -----
>   www/window/S3ClientEdit.js       | 3 ---
>   www/window/SyncJobEdit.js        | 3 ---
>   www/window/TrafficControlEdit.js | 3 ---
>   6 files changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/www/Utils.js b/www/Utils.js
> index ed3806bfc..641ccc773 100644
> --- a/www/Utils.js
> +++ b/www/Utils.js
> @@ -117,7 +117,8 @@ Ext.define('PBS.Utils', {
>                       if (Ext.isArray(values.delete)) {
>                           values.delete.push(fieldname);
>                       } else {
> -                        values.delete += ',' + fieldname;
> +                        values.delete = [values.delete];
> +                        values.delete.push(fieldname);

nit: the push happens for both branches, so it would make sense to do it 
unconditionally and invert the Ext.isArray() check.

>                       }
>                   } else {
>                       values.delete = [fieldname];
> diff --git a/www/window/NotifyOptions.js b/www/window/NotifyOptions.js
> index fc43de656..13e2bc40d 100644
> --- a/www/window/NotifyOptions.js
> +++ b/www/window/NotifyOptions.js
> @@ -64,10 +64,6 @@ Ext.define('PBS.window.NotifyOptions', {
>               }
>               values.notify = PBS.Utils.printPropertyString(notify);
>   
> -            if (values.delete && !Ext.isArray(values.delete)) {
> -                values.delete = values.delete.split(',');
> -            }
> -
>               PBS.Utils.delete_if_default(values, 'notify', '');
>               PBS.Utils.delete_if_default(values, 'notify-user', '');
>   
> diff --git a/www/window/PruneJobEdit.js b/www/window/PruneJobEdit.js
> index 9fdfb9884..a3dafde21 100644
> --- a/www/window/PruneJobEdit.js
> +++ b/www/window/PruneJobEdit.js
> @@ -51,11 +51,6 @@ Ext.define('PBS.window.PruneJobEdit', {
>               if (!values.id && me.up('pbsPruneJobEdit').isCreate) {
>                   values.id = 's-' + Ext.data.identifier.Uuid.Global.generate().slice(0, 13);
>               }
> -            if (!me.isCreate) {
> -                if (typeof values.delete === 'string') {
> -                    values.delete = values.delete.split(',');
> -                }
> -            }
>               values.disable = !values.enable;
>               delete values.enable;
>   
> diff --git a/www/window/S3ClientEdit.js b/www/window/S3ClientEdit.js
> index c7e8ade1f..5a114037b 100644
> --- a/www/window/S3ClientEdit.js
> +++ b/www/window/S3ClientEdit.js
> @@ -179,9 +179,6 @@ Ext.define('PBS.window.S3ClientEdit', {
>           let me = this;
>           let values = me.callParent(arguments);
>   
> -        if (values.delete && !Ext.isArray(values.delete)) {
> -            values.delete = values.delete.split(',');
> -        }
>           PBS.Utils.delete_if_default(values, 'path-style', false, me.isCreate);
>           PBS.Utils.delete_if_default(values, 'rate-in', undefined, me.isCreate);
>           PBS.Utils.delete_if_default(values, 'burst-in', undefined, me.isCreate);
> diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
> index 216653d17..961ce2951 100644
> --- a/www/window/SyncJobEdit.js
> +++ b/www/window/SyncJobEdit.js
> @@ -129,9 +129,6 @@ Ext.define('PBS.window.SyncJobEdit', {
>                           PBS.Utils.delete_if_default(values, 'rate-in');
>                           PBS.Utils.delete_if_default(values, 'rate-out');
>                           PBS.Utils.delete_if_default(values, 'remote');
> -                        if (typeof values.delete === 'string') {
> -                            values.delete = values.delete.split(',');
> -                        }
>                       }
>                       return values;
>                   },
> diff --git a/www/window/TrafficControlEdit.js b/www/window/TrafficControlEdit.js
> index 60b3f42ad..f41b4fe89 100644
> --- a/www/window/TrafficControlEdit.js
> +++ b/www/window/TrafficControlEdit.js
> @@ -216,9 +216,6 @@ Ext.define('PBS.window.TrafficControlEdit', {
>                   PBS.Utils.delete_if_default(values, 'burst-in');
>                   PBS.Utils.delete_if_default(values, 'burst-out');
>                   PBS.Utils.delete_if_default(values, 'users');
> -                if (typeof values.delete === 'string') {
> -                    values.delete = values.delete.split(',');
> -                }
>               }
>   
>               return values;