From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id B72B21FF144 for ; Tue, 24 Mar 2026 11:27:57 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 46919B54C; Tue, 24 Mar 2026 11:28:17 +0100 (CET) Message-ID: <2ef0f52f-af26-41f2-ab77-36beec3014ba@proxmox.com> Date: Tue, 24 Mar 2026 11:28:05 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH proxmox-backup v5 1/9] ui: show empty groups To: Hannes Laimer , pbs-devel@lists.proxmox.com References: <20260319161325.206846-1-h.laimer@proxmox.com> <20260319161325.206846-2-h.laimer@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: <20260319161325.206846-2-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1774348046887 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.041 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 5G34CVI5I7SWAXIZNCBW447QNE3DF3I2 X-Message-ID-Hash: 5G34CVI5I7SWAXIZNCBW447QNE3DF3I2 X-MailFrom: d.csapak@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Hi, high level comments: the verify button is disabled for empty groups (good!) but the prune button is not, better to disable that too also the 'verify state' column shows 'all ok', while technically not wrong, I think it would be better to don't render anything at all there if the groups does not have any snapshots, but this could be done as a follow up / extra patch. one comment inline On 3/19/26 5:12 PM, Hannes Laimer wrote: [snip] > @@ -232,6 +241,29 @@ Ext.define('PBS.DataStoreContent', { > > let groups = this.getRecordGroups(records); > > + for (const item of groupList) { > + let btype = item['backup-type']; > + let group = btype + '/' + item['backup-id']; > + if (groups[group] !== undefined) { > + continue; > + } > + let cls = PBS.Utils.get_type_icon_cls(btype); > + if (cls === '') { > + continue; > + } > + groups[group] = { > + text: group, > + leaf: true, > + iconCls: 'fa ' + cls, > + expanded: false, > + backup_type: btype, > + backup_id: item['backup-id'], > + comment: item.comment, > + owner: item.owner, > + children: [], > + }; > + } > + this hunk is more or less a 1:1 copy of 'getRecordGroups' (minus the comment/owner column) I think we should reuse that (maybe with a flag to set the comment/owner when we know it's from the group, or factor out the inner part of the loop and add the comment/owner manually after calling that) > let selected; > let expanded = {}; > > @@ -399,7 +431,7 @@ Ext.define('PBS.DataStoreContent', { > ); > } > > - this.updateGroupNotes(view); > + this.updateGroupNotes(view, groupList); > > if (selected !== undefined) { > let selection = view.getRootNode().findChildBy( > @@ -985,7 +1017,7 @@ Ext.define('PBS.DataStoreContent', { > flex: 1, > renderer: (v, meta, record) => { > let data = record.data; > - if (!data || data.leaf || data.root) { > + if (!data || (data.leaf && data.ty !== 'group') || data.root) { > return ''; > } > > @@ -1029,7 +1061,7 @@ Ext.define('PBS.DataStoreContent', { > }, > dblclick: function (tree, el, row, col, ev, rec) { > let data = rec.data || {}; > - if (data.leaf || data.root) { > + if ((data.leaf && data.ty !== 'group') || data.root) { > return; > } > let view = tree.up();