Re: [pbs-devel] [PATCH proxmox-backup] ui: switch datastore destroy label text on backend type

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

On 7/30/25 9:36 AM, Lukas Wagner wrote:
> On Tue Jul 29, 2025 at 6:52 PM CEST, Christian Ebner wrote:
>> Load also the backend configuration via the rstore, but do not show
>> it in the grid. By passing the backend type along to the datastore
>> destroy window, the destroy all data checkbox label is set based on
>> the given type.
>>
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>> Note: This is the ui followup for the datastore destroy dialog as requested in:
>> https://lore.proxmox.com/pbs-devel/DBOL7XELUICP.1CAPP8NTAR2KR@proxmox.com/
>>
>>   www/datastore/OptionView.js | 23 ++++++++++++++++++++++-
>>   1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/www/datastore/OptionView.js b/www/datastore/OptionView.js
>> index 913bdfc74..9aafd2e9c 100644
>> --- a/www/datastore/OptionView.js
>> +++ b/www/datastore/OptionView.js
>> @@ -18,6 +18,10 @@ Ext.define('PBS.window.SafeDatastoreDestroy', {
>>                       ? gettext('All backup snapshots and their data will be permanently destroyed!')
>>                       : gettext('Configuration change only, no data will be deleted.'),
>>               destroyNoteCls: (get) => (get('destroyData') ? 'pmx-hint' : ''),
>> +            destroyCheckboxLabel: (get) =>
>> +                get('backendType') === 's3'
>> +                    ? gettext('Remove data from S3 bucket and local cache (dangerous!)')
>> +                    : gettext('Destroy all data (dangerous!)'),
>>           },
>>       },
>>   
>> @@ -46,10 +50,10 @@ Ext.define('PBS.window.SafeDatastoreDestroy', {
>>           {
>>               xtype: 'proxmoxcheckbox',
>>               name: 'destroy-data',
>> -            boxLabel: gettext('Destroy all data (dangerous!)'),
>>               defaultValue: false,
>>               bind: {
>>                   value: '{destroyData}',
>> +                boxLabel: '{destroyCheckboxLabel}',
>>               },
>>           },
>>           {
>> @@ -70,6 +74,12 @@ Ext.define('PBS.window.SafeDatastoreDestroy', {
>>               },
>>           },
>>       ],
>> +
>> +    initComponent: function () {
>> +        let me = this;
>> +        me.getViewModel().set('backendType', me.backendType);
>> +        me.callParent();
>> +    },
>>   });
>>   
>>   Ext.define('PBS.Datastore.Options', {
>> @@ -118,7 +128,14 @@ Ext.define('PBS.Datastore.Options', {
>>   
>>           removeDatastore: function () {
>>               let me = this;
>> +            let backendType = 'filesystem';
>> +            let backend = me.getView().rstore.data.items.find((item) => item.id === 'backend');
>> +            if (backend) {
>> +                let backendConfig = PBS.Utils.parsePropertyString(backend.data.value, 'type');
>> +                backendType = backendConfig.type;
>> +            }
>>               Ext.create('PBS.window.SafeDatastoreDestroy', {
>> +                backendType: backendType,
>>                   datastore: me.getView().datastore,
>>               });
>>           },
>> @@ -164,6 +181,10 @@ Ext.define('PBS.Datastore.Options', {
>>       },
>>   
>>       rows: {
>> +        backend: {
>> +            required: false,
>> +            visible: false,
>> +        },
>>           'notification-mode': {
>>               required: true,
>>               defaultValue: 'notification-system',
> 
> Applied this on top of your other patch series, works fine on my end.
> 
> Tested-by: Lukas Wagner <l.wagner@proxmox.com>
> 
> For the `removeDatastore` function I would use ExtJS's native accessor
> methods for the data instead of accessing the data directly, for
> example:
> 
> 
> diff --git i/www/datastore/OptionView.js w/www/datastore/OptionView.js
> index 9aafd2e9c..2f9aa9ebe 100644
> --- i/www/datastore/OptionView.js
> +++ w/www/datastore/OptionView.js
> @@ -129,9 +129,10 @@ Ext.define('PBS.Datastore.Options', {
>           removeDatastore: function () {
>               let me = this;
>               let backendType = 'filesystem';
> -            let backend = me.getView().rstore.data.items.find((item) => item.id === 'backend');
> +            let backend = me.getView().getStore().getById('backend');
> +
>               if (backend) {
> -                let backendConfig = PBS.Utils.parsePropertyString(backend.data.value, 'type');
> +                let backendConfig = PBS.Utils.parsePropertyString(backend.get('value'), 'type');
>                   backendType = backendConfig.type;
>               }
>               Ext.create('PBS.window.SafeDatastoreDestroy', {
> 
> 
> 
> If you choose to use this hunk directly, consider this
> 
> Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>

Nice! Makes it definitely more readable. Will fold the suggested changes 
in an resend the patch series including the followup as v2 then, to give 
the complete picture.

Thanks!


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel