From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 050F21FF13C for ; Thu, 30 Apr 2026 14:44:21 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 78AB261FC; Thu, 30 Apr 2026 14:44:20 +0200 (CEST) Message-ID: <9cc836b3-121c-4514-bb8d-9c7205d2f186@proxmox.com> Date: Thu, 30 Apr 2026 14:44:15 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH proxmox-backup 1/1] fix #6691: allow search by comment in datastore. To: Erik Fastermann , pbs-devel@lists.proxmox.com References: <20260430103103.32588-1-e.fastermann@proxmox.com> <20260430103103.32588-2-e.fastermann@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: <20260430103103.32588-2-e.fastermann@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: 1777552956786 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.051 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: CFWED74VICN733KPJO6BA5245MJTFWZT X-Message-ID-Hash: CFWED74VICN733KPJO6BA5245MJTFWZT 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: On 4/30/26 12:30 PM, Erik Fastermann wrote: > Enable search by the comment field in the datastore content. > > Also includes multiple small bug fixes: > > - Display items again when searching by a non existing value and > clearing the search bar. > - Correctly disable the remove box and tooltip for namespaces. > - Correctly disable the browse box and tooltip for the root namespace. > please send separate patches for these things to keep the git history clean and making bisecting and changelog writing easier a patch/commit for each issue (or very similar issues) would be good otherwise comments inline > Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6691 > Signed-off-by: Erik Fastermann > --- > www/datastore/Content.js | 28 +++++++++++++++++++++------- > 1 file changed, 21 insertions(+), 7 deletions(-) > > diff --git a/www/datastore/Content.js b/www/datastore/Content.js > index 1ec07505d..29b27eced 100644 > --- a/www/datastore/Content.js > +++ b/www/datastore/Content.js > @@ -941,6 +941,10 @@ Ext.define('PBS.DataStoreContent', { > return true; > } > > + if (item.data.comment && item.data.comment.indexOf(value) !== -1) { > + return true; > + } > + a bit preexisting, but we might want to do the search case insensitive? we could use 'localeCompare' with case sensitivity disabled here > return false; > }, > > @@ -963,7 +967,6 @@ Ext.define('PBS.DataStoreContent', { > // we do it a little bit later for the error mask to work > setTimeout(function () { > store.clearFilter(); > - store.getRoot().collapseChildren(true); to which fix does this change belong? couldn't tell from the commit message > > store.beginUpdate(); > store.getRoot().cascadeBy({ > @@ -1150,14 +1153,18 @@ Ext.define('PBS.DataStoreContent', { > return Ext.String.format(gettext("Move namespace '{0}'"), v); > }, > getClass: (v, m, { data }) => { > - if (data.ty === 'group') { return 'fa fa-arrows'; } > + if (data.ty === 'group') { > + return 'fa fa-arrows'; > + } > if (data.ty === 'ns' && !data.isRootNS && data.ns === undefined) { > return 'fa fa-arrows'; > } > return 'pmx-hidden'; > }, > isActionDisabled: (v, r, c, i, { data }) => { > - if (data.ty === 'group') { return false; } > + if (data.ty === 'group') { > + return false; > + } these two hunks are only reformatting, please split that also out into a separate patch > if (data.ty === 'ns' && !data.isRootNS && data.ns === undefined) { > return false; > } > @@ -1210,7 +1217,12 @@ Ext.define('PBS.DataStoreContent', { > data.ty === 'dir' > ? 'fa critical fa-trash-o' > : 'pmx-hidden', > - isActionDisabled: (v, r, c, i, { data }) => false, > + isActionDisabled: (v, r, c, i, { data }) => > + !( > + (data.ty === 'ns' && !data.isRootNS && data.ns === undefined) || > + data.ty === 'group' || > + data.ty === 'dir' > + ), > }, > { > handler: 'downloadFile', > @@ -1235,12 +1247,14 @@ Ext.define('PBS.DataStoreContent', { > return 'pmx-hidden'; > }, > isActionDisabled: (v, r, c, i, { data }) => > - !( > + (!( > data.ty === 'file' && > (data.filename.endsWith('.pxar.didx') || > data.filename.endsWith('.mpxar.didx')) && > data['crypt-mode'] < 3 > - ) && data.ty !== 'ns', > + ) && > + data.ty !== 'ns') || > + data.root, meh this boolean logic is rather unreadable (before and after), can you split that out into multiple statements so one can see better what is happening? > }, > ], > }, > @@ -1486,7 +1500,7 @@ Ext.define('PBS.DataStoreContent', { > { > xtype: 'textfield', > reference: 'searchbox', > - emptyText: gettext('group, date or owner'), > + emptyText: gettext('group, date, owner or comment'), > triggers: { > clear: { > cls: 'pmx-clear-trigger',