From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 425A91FF0E6 for ; Fri, 24 Jul 2026 08:46:07 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 01DAB21492; Fri, 24 Jul 2026 08:46:07 +0200 (CEST) Date: Fri, 24 Jul 2026 08:45:26 +0200 From: Arthur Bied-Charreton To: Lukas Wagner Subject: Re: [PATCH proxmox-widget-toolkit 17/29] notifications: matcher: add support for match expressions Message-ID: <6vju5ooikx342avsjxyulvqjew3qbjciwu3ig7c4qfous3qenh@vlr3ld4vombn> References: <20260709115716.299836-1-l.wagner@proxmox.com> <20260709115716.299836-18-l.wagner@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260709115716.299836-18-l.wagner@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784875496902 X-SPAM-LEVEL: Spam detection results: 0 AWL 1.350 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust 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: HVXOGYXOK5RZW7VQQTIELEWZUYPHG7IH X-Message-ID-Hash: HVXOGYXOK5RZW7VQQTIELEWZUYPHG7IH X-MailFrom: a.bied-charreton@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 CC: pbs-devel@lists.proxmox.com, pve-devel@lists.proxmox.com 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 Thu, Jul 09, 2026 at 01:57:04PM +0200, Lukas Wagner wrote: > This patches changes to 'matcher edit window' to support the new > 'expression' paramter in the matcher config. Instead of of flat > 'match-*' configuration keys, 'expression' contains the match rules as a > serialized JSON blob. This allows us to create arbitrarily nested match > formulas. > > Support for match expressions has to be enabled via a feature flag. This > avoids awkward versioned breaks between the widget toolkit package and > the main product. > > The entire match rule panel has changed so much that copying the old > component and using that as a base was deemed more useful than modifying > the existing panel in-place. The old component can be removed once we > have migrated all products to this new system. > > Signed-off-by: Lukas Wagner amazing job on the drag and drop! some comments inline > --- > src/Makefile | 1 + > src/Schema.js | 10 + > src/css/ext6-pmx.css | 27 + > .../NotificationMatchExpressionEditPanel.js | 786 ++++++++++++++++++ > src/proxmox-dark/scss/extjs/_treepanel.scss | 5 + > src/proxmox-dark/scss/proxmox/_general.scss | 4 + > src/window/NotificationMatcherEdit.js | 13 +- > 7 files changed, 841 insertions(+), 5 deletions(-) > create mode 100644 src/panel/NotificationMatchExpressionEditPanel.js > [...] > + > +Ext.define('Proxmox.panel.NotificationMatchRuleTreeExpression', { > + extend: 'Ext.panel.Panel', > + xtype: 'pmxNotificationMatchRuleTreeExpression', > + mixins: ['Proxmox.Mixin.CBind'], > + border: false, > + > + getNodeIcon: function (type, data, invert) { > + let iconCls = 'fa'; > + > + switch (type) { > + case 'match-severity': > + { > + let v = data.value; > + if (Ext.isArray(data.value)) { > + v = data.value.join(', '); > + } > + iconCls += ' fa-exclamation'; > + if (!v) { > + iconCls += ' internal-error'; i really like this way of displaying errors :) i did notice a small regression here, the old UI used to not allow creating empty exact/severity matches, this new one does. the backend also does not validate this, meaning nothing is stopping me from creating a field matcher matching nothing. i think it would make sense to block submitting if there are any errors. > + } > + } > + break; > + case 'match-field': > + { > + let field = data.field; > + let value = data.value; > + iconCls += ' fa-square-o'; > + if (!field || !value || (Ext.isArray(value) && !value.length)) { > + iconCls += ' internal-error'; > + } > + } > + break; > + case 'match-calendar': > + { > + let v = data.value; > + iconCls += ' fa-calendar-o'; > + if (!v || !v.length) { > + iconCls += ' internal-error'; > + } > + } > + break; > + case 'any-of': > + iconCls += ' fa-filter'; > + > + break; > + case 'all-of': > + iconCls += ' fa-filter'; > + > + break; > + case 'one-of': > + iconCls = 'fa fa-filter'; > + break; > + > + case 'match-always': > + if (invert) { > + iconCls += ' fa-ban'; > + } else { > + iconCls += ' fa-check-circle'; > + } > + } > + > + return iconCls; > + }, > + [...] > + { > + xtype: 'actioncolumn', > + width: 60, > + items: [ > + { > + handler: (view, rI, cI, item, e, record) => { > + let node = record.appendChild({ > + type: 'match-field', > + data: { > + type: 'exact', > + field: '', > + value: '', > + }, > + leaf: true, > + }); > + > + view.setSelection(node); > + }, > + getTip: (v, m, rec) => > + Ext.String.format(gettext('Add new child node'), v), 'rule' and 'node' seem to be used interchangeably (or please correct me if i am not understanding the difference), we might want to stick with one and use that everywhere? i would personally go with 'rule'. also noticed some inconsistency in the UI: when clicking the + ('Add new child node'), the default rule type is whichever was last added, but the field matcher config fields (match type, field, value/regex) are displayed. this means you can get into a UI state where the rule type is "Always match" and still set a regex to match on. interestingly, the request sent when submitting has 'match-field' set as rule type, so this seems to be only a rendering issue. > + getClass: (v, m, { data }) => (data.leaf ? '' : 'fa fa-plus-circle'), > + isActionDisabled: (v, r, c, i, rec) => rec.data.leaf, > + }, > + { > + handler: function (view, rI, cI, item, e, record) { > + if (record.hasChildNodes()) { > + Ext.Msg.confirm( > + gettext('Remove rule?'), > + gettext('Remove rule and all of its sub-rules?'), > + (decision) => { > + if (decision === 'yes') { > + record.remove(true); > + } > + }, > + ); > + } else { > + record.remove(true); > + } > + }, > + getTip: (v, m, rec) => > + Ext.String.format(gettext('Remove this node'), v), > + getClass: (v, m, { data }) => 'fa fa-trash-o', > + isActionDisabled: (v, r, c, i, rec) => rec.isRoot(), > + }, > + ], > + }, > + ], > + }); > + > + Ext.apply(me, { > + items: [realExpression, treePanel], > + }); > + me.callParent(); > + }, > +}); > + > +Ext.define('Proxmox.panel.NotificationMatchExpressionNodePanel', { > + extend: 'Ext.panel.Panel', > + xtype: 'pmxNotificationMatchExpressionNodePanel', > + mixins: ['Proxmox.Mixin.CBind'], > + border: false, > + layout: 'anchor', > + > + items: [ > + { > + xtype: 'proxmoxKVComboBox', > + fieldLabel: gettext('Rule Type'), > + isFormField: false, > + allowBlank: false, > + // Hide initially to avoid glitches when opening the window > + hidden: true, > + bind: { > + value: '{nodeType}', > + hidden: '{!showMatcherType}', > + }, > + > + comboItems: [ > + ['match-always', gettext('Always match')], > + ['all-of', gettext('All rules match')], > + ['any-of', gettext('At least one rule matches')], > + ['one-of', gettext('Exactly one node matches')], here as well, got confused when reading 'node' instead of 'rule' ^^ > + ['match-field', gettext('Match Field')], > + ['match-severity', gettext('Match Severity')], > + ['match-calendar', gettext('Match Calendar')], > + ], > + }, > + { > + xtype: 'proxmoxcheckbox', > + isFormField: false, > + fieldLabel: gettext('Invert Rule'), > + allowBlank: false, > + bind: { > + value: '{invertMatch}', > + hidden: '{!showMatcherType}', > + }, > + }, > + { > + xtype: 'component', > + height: 1, > + margin: '20 0 20 0', > + cls: 'pmx-horizontal-separator', > + bind: { > + hidden: '{!showMatcherType}', > + }, > + }, > + { > + xtype: 'pmxNotificationMatchFieldSettings', > + cbind: { > + baseUrl: '{baseUrl}', > + }, > + }, > + { > + xtype: 'pmxNotificationMatchSeveritySettings', > + }, > + { > + xtype: 'pmxNotificationMatchCalendarSettings', > + }, > + ], > +}); > diff --git a/src/proxmox-dark/scss/extjs/_treepanel.scss b/src/proxmox-dark/scss/extjs/_treepanel.scss > index 0480371..bf92f0b 100644 > --- a/src/proxmox-dark/scss/extjs/_treepanel.scss > +++ b/src/proxmox-dark/scss/extjs/_treepanel.scss > @@ -22,3 +22,8 @@ > background-color: $background-darker; > border-color: $border-color; > } > + > +.x-tree-node-text > code { > + background-color: $background-darkest; > +} > + > diff --git a/src/proxmox-dark/scss/proxmox/_general.scss b/src/proxmox-dark/scss/proxmox/_general.scss > index dcbf049..36ff65c 100644 > --- a/src/proxmox-dark/scss/proxmox/_general.scss > +++ b/src/proxmox-dark/scss/proxmox/_general.scss > @@ -51,3 +51,7 @@ div.eol-notice + div[id^="panel-"] > div[id^="panel-"][id$="-bodyWrap"] > div { > .pmx-unclickable { > pointer-events: none; > } > + > +.pmx-horizontal-separator { > + border-top: 1px solid $border-color-alt; > +} > diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js > index 1999e20..893c3e3 100644 > --- a/src/window/NotificationMatcherEdit.js > +++ b/src/window/NotificationMatcherEdit.js > @@ -95,6 +95,9 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', { > me.method = 'POST'; > } else { > me.url += `/${me.name}`; > + if (Proxmox.Schema.notificationMatcherExpressions) { > + me.loadUrl = me.url + '?migrate-to-expression=1'; > + } > me.method = 'PUT'; > } > > @@ -119,7 +122,7 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', { > { > name: me.name, > title: gettext('Match Rules'), > - xtype: 'pmxNotificationMatchRulesEditPanel', > + xtype: Proxmox.Schema.notificationMatcherPanelType(), > isCreate: me.isCreate, > baseUrl: me.baseUrl, > }, > @@ -1013,7 +1016,7 @@ Ext.define('Proxmox.panel.MatchCalendarSettings', { > initComponent: function () { > let me = this; > Ext.apply(me.viewModel, { > - parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(), > + parent: me.up(Proxmox.Schema.notificationMatcherPanelType()).getViewModel(), > }); > me.callParent(); > }, > @@ -1091,7 +1094,7 @@ Ext.define('Proxmox.panel.MatchSeveritySettings', { > initComponent: function () { > let me = this; > Ext.apply(me.viewModel, { > - parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(), > + parent: me.up(Proxmox.Schema.notificationMatcherPanelType()).getViewModel(), > }); > me.callParent(); > }, > @@ -1164,7 +1167,7 @@ Ext.define('Proxmox.panel.MatchFieldSettings', { > regexVal += `(${currentData.value.join('|')})`; > } > regexVal += '$'; > - newValue.push(regexVal); > + newValue = regexVal; > } > > record.set({ > @@ -1282,7 +1285,7 @@ Ext.define('Proxmox.panel.MatchFieldSettings', { > }); > > Ext.apply(me.viewModel, { > - parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(), > + parent: me.up(Proxmox.Schema.notificationMatcherPanelType()).getViewModel(), > }); > Ext.apply(me, { > items: [ > -- > 2.47.3 > > > > >