Re: [pbs-devel] [PATCH v5 proxmox-backup 19/31] ui: group filter: allow to set namespace for local datastore

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

On 10/25/24 12:32, Dominik Csapak wrote:
> one nit inline (but no blocker for me IMHO)
> 
> On 10/18/24 10:42, Christian Ebner wrote:
>> The namespace has to be set in order to get the correct groups to be
>> used as group filter options with a local datastore as source,
>> required for sync jobs in push direction.
>>
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>> changes since version 4:
>> - no changes
>>
>> changes since version 3:
>> - no changes
>>
>>   www/form/GroupFilter.js | 18 +++++++++++++++++-
>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/www/form/GroupFilter.js b/www/form/GroupFilter.js
>> index c9c2d913e..7275b00ed 100644
>> --- a/www/form/GroupFilter.js
>> +++ b/www/form/GroupFilter.js
>> @@ -258,7 +258,11 @@ Ext.define('PBS.form.GroupFilter', {
>>           return;
>>       }
>>       if (me.namespace) {
>> -        url += `?namespace=${me.namespace}`;
>> +        if (me.remote) {
>> +        url += `?namespace=${me.namespace}`;
>> +        } else {
>> +        url += `?ns=${me.namespace}`;
>> +        }
> 
> this not really possible to see from the context,
> but imho this would be better suited in the if branches above,
> 
> e.g.
> 
> if (me.remote) {
>   url = ...;
>   if (me.namespace) {
>    url += ...;
>   }
> } else {
>    ...
> }
> 
> that way the connection between the api call and the parameter is much 
> more obvious

Yes, I did move the corresponding if statements accordingly for the next 
version of the patches.



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