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:17, Christian Ebner wrote:
> 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.

Correction, I meant the group filter datastore has to be set.

> 
>>
>> 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



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