Re: [pbs-devel] [PATCH proxmox-backup] ui: sync job: fix source group filters based on sync direction

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

On 11/25/24 10:06, Dominik Csapak wrote:
> two issues here:
> 
> if i use the add button on the top level 'Datastore' sync pane, there is 
> no initial datastore selected, but the groups get loaded with '/api2/ 
> json/admin/datastore//groups' which returns
> the index for a datastore. This populates the group dropdown with 
> records that don't contain
> the relevant infos, and will displayed as empty lines.
> 
> imho we should only trigger the initial load only when the datastore is set

Ah yes, thanks for noticing! I did overlook that way of setting the sync 
job on friday.

> 
> also even if i select a datastore here, the groups won't get reloaded,
> only if i change the local namespace or the remote (!) namespace.

Yes, the namespace has to be set when changing the store as well, not 
just the datastore. Will send a new version covering also this case.

> 
> the second issue is inline:
> 
> On 11/22/24 17:54, Christian Ebner wrote:
>> Fix switching the source for group filters based on the sync jobs
>> sync direction.
>>
>> The helper to set the local namespace for the group filers was
>> introduced in commit 43a92c8c ("ui: group filter: allow to set
>> namespace for local datastore"), but never used because lost during
>> subsequent iterations of reworking the patch series.
>>
>> The switching is corrected by:
>> - correctly initializing the local store and namespace for the group
>>    filer of sync jobs in push direction in the controller init.
>> - fixing an incorrect check for the sync direction in the remote
>>    datastore selector change listener.
>> - conditionally switching namespace to be set for the group filter in
>>    the remote and local namespace selector change listeners.
>>
>> Reported-by: Lukas Wagner <l.wagner@proxmox.com>
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>>   www/window/SyncJobEdit.js | 23 +++++++++++++++++++++--
>>   1 file changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
>> index 0e648e7b3..4f46eacc4 100644
>> --- a/www/window/SyncJobEdit.js
>> +++ b/www/window/SyncJobEdit.js
>> @@ -69,6 +69,16 @@ Ext.define('PBS.window.SyncJobEdit', {
>>           let nsSelector = view.down('pbsNamespaceSelector[name=ns]');
>>           nsSelector.setDatastore(value);
>>       },
>> +
>> +    init: function() {
>> +        let view = this.getView();
>> +        if (view.syncDirectionPush) {
>> +        let localNs = 
>> view.down('pbsNamespaceSelector[name=ns]').getValue();
>> +        let localStore = view.down('field[name=store]').getValue();
>> +        view.down('pbsGroupFilter').setLocalDatastore(localStore);
>> +        view.down('pbsGroupFilter').setLocalNamespace(localStore, 
>> localNs);
>> +        }
>> +    }
> 
> eslint complains about a missing trailing comma here ;)

Acked, will be fixed in the upcoming version as well.

> 
>>       },
>>       setValues: function(values) {
>> @@ -134,6 +144,11 @@ Ext.define('PBS.window.SyncJobEdit', {
>>                   let me = this;
>>                   let view = me.up('pbsSyncJobEdit');
>> +                if (view.syncDirectionPush) {
>> +                    let localStore = 
>> view.down('field[name=store]').getValue();
>> +                    
>> view.down('pbsGroupFilter').setLocalNamespace(localStore, localNs);
>> +                }
>> +
>>                   let remoteNs = 
>> view.down('pbsRemoteNamespaceSelector[name=remote-ns]').getValue();
>>                   let maxDepthField = view.down('field[name=max-depth]');
>>                   maxDepthField.setLimit(localNs, remoteNs);
>> @@ -268,7 +283,8 @@ Ext.define('PBS.window.SyncJobEdit', {
>>                   remoteNamespaceField.setRemote(remote);
>>                   remoteNamespaceField.setRemoteStore(value);
>> -                if (!me.syncDirectionPush) {
>> +                let view = me.up('pbsSyncJobEdit');
>> +                if (!view.syncDirectionPush) {
>>                       
>> me.up('tabpanel').down('pbsGroupFilter').setRemoteDatastore(remote, 
>> value);
>>                   } else {
>>                       let localStore = 
>> me.up('pbsSyncJobEdit').down('field[name=store]').getValue();
>> @@ -293,7 +309,10 @@ Ext.define('PBS.window.SyncJobEdit', {
>>                   let remote = 
>> view.down('field[name=remote]').getValue();
>>                   let remoteStore = view.down('field[name=remote- 
>> store]').getValue();
>> -                
>> me.up('tabpanel').down('pbsGroupFilter').setRemoteNamespace(remote, 
>> remoteStore, remoteNs);
>> +
>> +                if (!view.syncDirectionPush) {
>> +                    
>> me.up('tabpanel').down('pbsGroupFilter').setRemoteNamespace(remote, 
>> remoteStore, remoteNs);
>> +                }
>>                   let localNs = 
>> view.down('pbsNamespaceSelector[name=ns]').getValue();
>>                   let maxDepthField = view.down('field[name=max-depth]');
> 
> 



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