[pbs-devel] [PATCH proxmox-backup 1/3] ui: form/GroupFilter: correctly resolve the reference cycle

[pbs-devel] [PATCH proxmox-backup 1/3] ui: form/GroupFilter: correctly resolve the reference cycle

[Dominik Csapak]

'record[widget]' does not contain anything since the widgets are
in the 'widgets' property so delete that

we also have to remove the 'record' entry of the widget so that
the widget does not have a link to the record anymore

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/form/GroupFilter.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/www/form/GroupFilter.js b/www/form/GroupFilter.js
index 453152e2..5c75f1d0 100644
--- a/www/form/GroupFilter.js
+++ b/www/form/GroupFilter.js
@@ -10,7 +10,8 @@ Ext.define('PBS.form.GroupFilter', {
 
 	removeReferences: function(record) {
 	    for (const widget of Object.keys(record.widgets || {})) {
-		delete record[widget];
+		delete record.widgets[widget].record;
+		delete record.widgets[widget];
 	    }
 
 	    delete record.widgets;
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 2/3] ui: form/GroupFilter: improve group load callback handling

[Dominik Csapak]

if 'me' is already destroyed here, return
if records is 'null' (which can happen on a not successful load),
load an empty list instead

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/form/GroupFilter.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/www/form/GroupFilter.js b/www/form/GroupFilter.js
index 5c75f1d0..11aa24c4 100644
--- a/www/form/GroupFilter.js
+++ b/www/form/GroupFilter.js
@@ -214,8 +214,11 @@ Ext.define('PBS.form.GroupFilter', {
 	me.setDsStoreUrl(url);
 	me.dsStore.load({
 	    callback: (records) => {
+		if (me.isDestroyed) {
+		    return;
+		}
 		me.query('pbsGroupSelector').forEach((selector) => {
-		    selector.getStore().setData(records);
+		    selector.getStore().setData(records || []);
 		});
 	    },
 	});
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 3/3] ui: form/GroupFilter: copy records for the pbsGroupSelectors

[Dominik Csapak]

store.getData() returns an 'Ext.util.Collection' which is a special
class that does more than being an array of records. Namely, it can
have 'observers' which can react on the change of the collection

Here, the 'onWidgetAttach' callback will be called twice on the first
row add and the widgets (and thus stores) are cached by extjs. When
doing a 'setData' of a Collection, it tries to add the store as an
observer, but due to the above caching and multiple calling this fails
since the store is already an observer.

For this reason, we want to actually copy the records (which neither
the store, nor the Collection has a method for...)

This gives us an additional benefit: The different pbsGroupSelectors can
sort independently now, before it was all linked to the original store's
collection.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/form/GroupFilter.js | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/www/form/GroupFilter.js b/www/form/GroupFilter.js
index 11aa24c4..7398d331 100644
--- a/www/form/GroupFilter.js
+++ b/www/form/GroupFilter.js
@@ -134,7 +134,11 @@ Ext.define('PBS.form.GroupFilter', {
 	    let regex = widget.down('textfield[type=regex]');
 	    let group = widget.down('pbsGroupSelector');
 
-	    group.getStore().setData(view.dsStore.getData());
+	    let recs = [];
+	    view.dsStore.each((record) => {
+		recs.push(record.data);
+	    });
+	    group.getStore().setData(recs);
 
 	    // add a widget reference to the record so we can acces them
 	    // from the other column
-- 
2.30.2

[pbs-devel] applied-series: [PATCH proxmox-backup 1/3] ui: form/GroupFilter: correctly resolve the reference cycle

[Thomas Lamprecht]

On 01.12.21 11:57, Dominik Csapak wrote:
> 'record[widget]' does not contain anything since the widgets are
> in the 'widgets' property so delete that
> 
> we also have to remove the 'record' entry of the widget so that
> the widget does not have a link to the record anymore
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  www/form/GroupFilter.js | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
>

applied series, thanks! Added a patch that merges duplicate filter-entries
on top.

As mentioned off-list, ideally we'd let our combo grid cope with shared stores,
then we could use a single backing store for all group fields, which would be
more efficient too, anyhow, for now this is just fine as is.