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 BA0A21FF0AA for ; Tue, 03 Nov 2026 13:58:02 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 4272A218DE; Tue, 03 Nov 2026 13:57:58 +0100 (CET) Message-ID: Date: Tue, 3 Nov 2026 13:57:49 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH proxmox-widget-toolkit v7 21/41] notifications: matcher: add support for match expressions To: Lukas Wagner , pbs-devel@lists.proxmox.com, pve-devel@lists.proxmox.com References: <20261103104859.172466-1-l.wagner@proxmox.com> <20261103104859.172466-22-l.wagner@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: <20261103104859.172466-22-l.wagner@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793710669920 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.331 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_MED -2.3 Sender listed at https://www.dnswl.org/, medium 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: APXSN5EGE6UEFUBM6OEU4Z3R3UJBXJVM X-Message-ID-Hash: APXSN5EGE6UEFUBM6OEU4Z3R3UJBXJVM 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 VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: hi, looks mostly good to me, a few nits and comments inline no blockers for me though (maybe only consider them as follow up improvements or if you need to send another version) In general i'm not a fan of the viewmodel to be declared and modified in one component and the effects only applied in another (e.g. which match field is shown) Nothing tragic (and i think it's pre-existing), but the connection between components is often lost when one does not know what to look for.... On 11/3/26 11:57 AM, 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 > Reviewed-by: Michael Köppl > Tested-by: Michael Köppl > --- > > Notes: > Changes since v1: > - Don't allow empty all-of/any-of/one-of rules, show an > error below the tree then > > Changes since v2: > - properly handle nested 'not' expressions. > While the UI never produces them, they could be added > via the API or CLI, so the UI must understand them correctly. > The UI will automatically flatten/simply these on update. > > Changes since v3: > - remove left-over console.log statement > > src/Makefile | 1 + > src/Schema.js | 10 + > src/css/ext6-pmx.css | 51 + > .../NotificationMatchExpressionEditPanel.js | 872 ++++++++++++++++++ > src/proxmox-dark/scss/extjs/_treepanel.scss | 4 + > src/proxmox-dark/scss/proxmox/_general.scss | 4 + > src/window/NotificationMatcherEdit.js | 13 +- > 7 files changed, 950 insertions(+), 5 deletions(-) > create mode 100644 src/panel/NotificationMatchExpressionEditPanel.js > > diff --git a/src/Makefile b/src/Makefile > index 9c52339..f11716a 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -64,6 +64,7 @@ JSSRC= \ > panel/LogView.js \ > panel/NodeInfoRepoStatus.js \ > panel/NotificationConfigView.js \ > + panel/NotificationMatchExpressionEditPanel.js \ > panel/JournalView.js \ > panel/PermissionView.js \ > panel/PruneKeepPanel.js \ > diff --git a/src/Schema.js b/src/Schema.js > index 5de3f53..e72d301 100644 > --- a/src/Schema.js > +++ b/src/Schema.js > @@ -86,6 +86,16 @@ Ext.define('Proxmox.Schema', { > } > }, > > + notificationMatcherExpressions: false, > + > + notificationMatcherPanelType: function () { > + if (Proxmox.Schema.notificationMatcherExpressions) { > + return 'pmxNotificationMatchExpressionEditPanel'; > + } else { > + return 'pmxNotificationMatchRulesEditPanel'; > + } > + }, > + > pxarFileTypes: { > b: { icon: 'cube', label: gettext('Block Device') }, > c: { icon: 'tty', label: gettext('Character Device') }, > diff --git a/src/css/ext6-pmx.css b/src/css/ext6-pmx.css > index 878dfa0..93010e6 100644 > --- a/src/css/ext6-pmx.css > +++ b/src/css/ext6-pmx.css > @@ -248,6 +248,53 @@ div.right-aligned { > background-color: #f5f5f5; > } > > +/* the small icons */ > +.x-tree-icon-custom:after { > + position: relative; > + left: -5px; > + top: 1px; > + font-size: 0.75em; > + text-shadow: -1px 0px 2px #fff; > + content: "\ "; > +} > + > +/* yellow ! triangle */ > +.x-tree-icon-custom.internal-error:after { > + content: "\f071"; > + color: #ffcc00; > +} > + > +.x-tree-node-text > code { > + display: inline-block; > + background-color: #f5f5f5; > + border-radius: 4px; > + padding: 1px 4px; > + margin: 1px 0; > + font-size: 11px; > + /* a single long value, e.g. a regex, has to break instead of overflowing */ > + overflow-wrap: anywhere; > +} we could probably have a follow up removing them in pve-manager and proxmox-backup ? > + > +/* The notification matcher rule tree wraps its labels, see 'cellWrap'. Laying > + * the cell out as a flex row keeps the wrapped lines aligned with the first > + * one, instead of restarting at the left edge under the indentation. */ > +.pmx-match-rule-tree .x-grid-cell-inner-treecolumn { > + display: flex; > + align-items: flex-start; > +} > + > +/* indentation, elbows and the node icon have a fixed size and must not shrink */ > +.pmx-match-rule-tree .x-tree-elbow-img, > +.pmx-match-rule-tree .x-tree-icon { > + flex: none; > +} > + > +/* 'min-width' lets the label get narrower than its longest value */ > +.pmx-match-rule-tree .x-tree-node-text { > + flex: 1; > + min-width: 0; > +} > + > /* fix padding for legend in header */ > .x-legend-inner { > padding: 0; > @@ -431,3 +478,7 @@ div.right-aligned { > font-style: italic; > } > /* journal log level coloring end */ > + > +.pmx-horizontal-separator { > + border-top: 1px solid #cfcfcf; > +} > diff --git a/src/panel/NotificationMatchExpressionEditPanel.js b/src/panel/NotificationMatchExpressionEditPanel.js > new file mode 100644 > index 0000000..ec9fd3c > --- /dev/null > +++ b/src/panel/NotificationMatchExpressionEditPanel.js > @@ -0,0 +1,872 @@ > +Ext.define('Proxmox.panel.NotificationMatchExpressionEditPanel', { > + extend: 'Proxmox.panel.InputPanel', > + xtype: 'pmxNotificationMatchExpressionEditPanel', > + mixins: ['Proxmox.Mixin.CBind'], > + > + controller: { > + xclass: 'Ext.app.ViewController', > + > + // we want to also set the empty value, but 'bind' does not do that so > + // we have to set it then (and only then) to get the correct value in > + // the tree > + control: { > + field: { > + change: function (cmp) { > + let me = this; > + let vm = me.getViewModel(); > + if (cmp.field) { > + let record = vm.get('selectedRecord'); > + if (!record) { > + return; > + } > + let data = Ext.apply({}, record.get('data')); > + let value = cmp.getValue(); > + // only update if the value is empty (or empty array) > + if (!value || !value.length) { > + data[cmp.field] = value; > + record.set({ data }); > + } > + } > + }, > + }, > + }, > + }, > + > + viewModel: { > + data: { > + invertMatch: false, > + selectedRecord: null, > + matchFieldType: 'exact', > + matchFieldField: '', > + matchFieldValue: '', > + }, > + > + formulas: { > + showMatcherType: (get) => get('selectedRecord') !== null, > + > + invertMatch: { > + bind: { > + bindTo: '{selectedRecord}', > + deep: true, > + }, > + get: function (record) { > + return record?.get('invert'); > + }, > + set: function (value) { > + let me = this; > + let record = me.get('selectedRecord'); > + record.set('invert', value); > + }, > + }, > + nodeType: { > + get: function (get) { > + let record = get('selectedRecord'); > + return record?.get('type'); > + }, > + set: function (value) { > + let me = this; > + let record = me.get('selectedRecord'); > + > + let data; > + let leaf; > + > + switch (value) { > + case 'match-severity': > + data = { > + value: ['info', 'notice', 'warning', 'error', 'unknown'], > + }; > + leaf = true; > + break; > + case 'match-field': > + data = { > + type: 'exact', > + field: '', > + value: '', > + }; > + leaf = true; > + break; > + case 'match-calendar': > + data = { > + value: '', > + }; > + leaf = true; > + break; > + case 'any-of': > + data = {}; > + leaf = false; > + break; > + case 'all-of': > + data = {}; > + leaf = false; > + break; > + case 'one-of': > + data = {}; > + leaf = false; > + break; nit: these could be combined: case 'any-of': case 'all-of': case 'one-of': ... break; > + case 'match-always': > + value = 'match-always'; nit: we match on value for 'match-always' and set it to 'match-always' ? some leftover probably > + data = {}; > + leaf = true; > + break; > + } > + > + let node = { > + type: value, > + invert: false, > + data, > + leaf, > + }; > + > + if (!leaf) { > + node.expanded = true; > + node.expandable = false; > + } else { > + record.removeAll(); > + } > + > + record.set(node); > + }, > + }, > + }, > + }, > + > + column1: [ > + { > + xtype: 'pmxNotificationMatchRuleTreeExpression', > + cbind: { > + baseUrl: '{baseUrl}', > + }, > + }, > + ], > + > + column2: [ > + { > + xtype: 'pmxNotificationMatchExpressionNodePanel', > + cbind: { > + baseUrl: '{baseUrl}', > + }, > + }, > + ], > + > + onGetValues: function (values) { > + let me = this; > + > + if (!me.isCreate) { > + Proxmox.Utils.assemble_field_data(values, { delete: 'match-field' }); > + Proxmox.Utils.assemble_field_data(values, { delete: 'match-severity' }); > + Proxmox.Utils.assemble_field_data(values, { delete: 'match-calendar' }); > + Proxmox.Utils.assemble_field_data(values, { delete: 'mode' }); > + Proxmox.Utils.assemble_field_data(values, { delete: 'invert-match' }); > + } > + > + return values; > + }, > +}); > + > +Ext.define('Proxmox.panel.NotificationMatchRuleTreeExpression', { > + extend: 'Ext.panel.Panel', > + xtype: 'pmxNotificationMatchRuleTreeExpression', > + mixins: ['Proxmox.Mixin.CBind'], > + border: false, > + > + // Returns a message describing why a node is incomplete, or undefined if > + // the node is fully configured. Incomplete nodes are marked in the tree and > + // block submitting the form. > + getNodeError: function (type, data, hasChildren) { > + switch (type) { > + case 'match-severity': { > + let v = Ext.isArray(data.value) ? data.value.join(', ') : data.value; > + if (!v) { > + return gettext('Severity rules must match at least one severity.'); > + } > + break; nit: we don't need the result of 'v' anywhere just if it exists, so we don't need to assemble with `.join(', ')` ? just: let has_value = Ext.isArray(data.value) ? data.value.length > 0 : !!data.value; if (!has_value) { ... } (or have a similar check as below) > + } > + case 'match-field': { > + let field = data.field; > + let value = data.value; > + if (!field || !value || (Ext.isArray(value) && !value.length)) { basically what i suggested above (sans boolean reversal) > + return gettext('Field rules must have a field and a value.'); > + } > + break; > + } > + case 'match-calendar': { > + let v = data.value; > + if (!v || !v.length) { here is the third variant of that check, these should probably be unified/refactored let hasValue = (data) => Ext.isArray(data?.value) ? data.value.length > 0: !!data?.value; for instance (untested) > + return gettext('Calendar rules must have a schedule.'); > + } > + break; > + } > + case 'any-of': > + case 'all-of': > + case 'one-of': > + if (!hasChildren) { > + return gettext('Rule groups must contain at least one sub-rule.'); > + } > + break; > + } > + > + return undefined; > + }, > + > + getNodeIcon: function (type, data, invert, hasChildren) { > + let me = this; > + let iconCls = 'fa'; > + > + switch (type) { > + case 'match-severity': > + iconCls += ' fa-exclamation'; > + break; > + case 'match-field': > + iconCls += ' fa-square-o'; > + break; > + case 'match-calendar': > + iconCls += ' fa-calendar-o'; > + break; > + case 'any-of': > + case 'all-of': > + case 'one-of': > + iconCls += ' fa-filter'; > + break; > + case 'match-always': > + if (invert) { > + iconCls += ' fa-ban'; > + } else { > + iconCls += ' fa-check-circle'; > + } > + break; > + } > + > + if (me.getNodeError(type, data, hasChildren)) { > + iconCls += ' internal-error'; > + } > + > + return iconCls; > + }, > + > + renderTreeLabel: function (_value, meta, record) { > + let data = record.get('data'); > + let invert = record.get('invert'); > + let type = record.get('type'); > + > + switch (type) { > + case 'match-severity': { > + let v; > + > + if (data.value) { > + v = data.value.map((x) => `${Ext.String.htmlEncode(x)}`).join(' '); > + } else { > + v = '?'; > + } > + > + if (invert) { > + return Ext.String.format(gettext('Severity: all except {0}'), v); > + } else { > + return Ext.String.format(gettext('Severity: {0}'), v); > + } > + } > + case 'match-field': { > + let valueIsArray = Ext.isArray(data.value); > + > + let field = data.field ? data.field : '?'; > + let value = > + (!valueIsArray && data.value) || (valueIsArray && data.value.length) here is even a fourth variant of that check from above ;) maybe it's not a good idea to mix nonarray and array values on the same field? we could have a 'value' field and 'arrayValue' field and use them separately? also in error case this is always the '?' string, which reads odd.. > + ? data.value > + : '?'; > + let matchType = data.type; > + > + let fieldHtml = `${Ext.String.htmlEncode(field)}`; > + let valueHtml = `${Ext.String.htmlEncode(value)}`; here 'value' can be an array, this will be joined with ','. is that intended? wouldn't it be better to move the check length > 1 here and produce a different valueHtml upfront? then the branches below disappear? > + > + if (matchType === 'exact') { > + if (Ext.isArray(value) && value.length > 1) { the array check already exists in 'valueIsArray', it was a different 'value' technically but couldn't we reuse that result here? (the length == 0 case behaves differently then, but we could move that length check into valueIsArray) > + valueHtml = value > + .map((x) => `${Ext.String.htmlEncode(x)}`) > + .join(' '); > + > + if (invert) { > + // inverted field matcher, matching multiple values > + return Ext.String.format( > + gettext('Field: {0} none of {1}'), > + fieldHtml, > + valueHtml, > + ); > + } else { > + // non-inverted field matcher, matching multiple values > + return Ext.String.format( > + gettext('Field: {0} any of {1}'), > + fieldHtml, > + valueHtml, > + ); > + } > + } else if (invert) { > + // inverted field matcher, matching a single value > + return Ext.String.format( > + gettext('Field: {0} is not {1}'), > + fieldHtml, > + valueHtml, > + ); > + } else { > + // non-inverted field matcher, matching a single value > + return Ext.String.format( > + gettext('Field: {0} is {1}'), > + fieldHtml, > + valueHtml, > + ); > + } > + } else if (invert) { > + // inverted regex-based field matcher > + return Ext.String.format( > + gettext('Field: {0} does not match {1}'), > + fieldHtml, > + valueHtml, > + ); > + } else { > + // non-inverted regex-based field matcher > + return Ext.String.format( > + gettext('Field: {0} matches {1}'), > + fieldHtml, > + valueHtml, > + ); > + } > + } > + case 'match-calendar': { > + let value = data.value ? data.value : '?'; this could just be let value = data.value || '?'; or ? (reads a bit cleaner) > + value = `${Ext.String.htmlEncode(value)}`; we do have basically the same snippet in 3 branches here, unifying them would be nice > + > + if (invert) { > + return Ext.String.format(gettext('Calendar: outside {0}'), value); > + } else { > + return Ext.String.format(gettext('Calendar: within {0}'), value); > + } > + } > + case 'any-of': > + if (invert) { > + return gettext('No sub-rule matches'); > + } else { > + return gettext('At least one sub-rule matches'); > + } > + > + case 'all-of': > + if (invert) { > + return gettext('At least one sub-rule does not match'); > + } else { > + return gettext('All sub-rules match'); > + } > + > + case 'one-of': > + if (invert) { > + return gettext('Not: Exactly one sub-rule matches'); > + } else { > + return gettext('Exactly one sub-rule matches'); > + } > + > + case 'match-always': > + if (invert) { > + return gettext('Never match'); > + } else { > + return gettext('Always match'); > + } > + } > + }, imho the big switch with nested ifs is really hard to follow. i would find it better to have the gettext declared semantically e.g. in a map or as a return value of a function (together with the number of parameters), like: labelGetText: function(type, value, invert) { switch (type) { case 'match-severity': return [0, invert ? gettext('A') : gettext('B')] case '....': ... } } then the whole method here would collapse to : extract type, value; generate valueHtml, fieldHtml let [params, label] = labelGetText(...); if (params === 0) { return label; } elseif (params === 1) { return Ext.String.format(label, valueHtml); } ... > + > + initComponent: function () { > + let me = this; > + > + let treeStore = Ext.create('Ext.data.TreeStore', { > + root: { > + expanded: true, > + expandable: false, > + text: '', > + type: 'match-always', > + data: {}, > + invert: false, > + leaf: true, > + children: [], > + iconCls: 'fa fa-filter', > + }, > + }); > + > + // The field holding the expression is hidden, so render its validation > + // errors below the tree instead. > + let errorLabel = Ext.create({ > + xtype: 'component', > + hidden: true, > + cls: 'x-form-invalid-under-default', > + }); > + > + let getExpressionErrors = function () { > + let errors = []; > + > + treeStore.getRoot().cascade(function (node) { > + let error = me.getNodeError( > + node.get('type'), > + node.get('data'), > + node.hasChildNodes(), > + ); > + > + if (error && !errors.includes(error)) { > + errors.push(error); > + } > + }); > + > + return errors; > + }; > + > + let realExpression = Ext.create({ > + xtype: 'hiddenfield', > + name: 'expression', > + getErrors: function () { > + let errors = getExpressionErrors(); > + > + errorLabel.setHtml( > + errors.map((error) => Ext.String.htmlEncode(error)).join('
'), > + ); > + errorLabel.setHidden(!errors.length); > + > + return errors; > + }, > + setValue: function (value) { > + this.value = value; > + this.checkChange(); > + }, > + getValue: function () { > + return this.value; > + }, > + getSubmitValue: function () { > + let value = this.value; > + return value; nit: this could also just be `return this.value;` or not? > + }, > + }); > + > + let storeChanged = function (store) { > + store.suspendEvent('datachanged'); > + > + store.each(function (model) { > + let type = model.get('type'); > + let data = model.get('data'); > + let invert = model.get('invert'); > + > + let iconCls = me.getNodeIcon(type, data, invert, model.hasChildNodes()); > + > + model.set({ > + iconCls, > + }); > + }); > + > + let root = store.getRoot(); > + let stack = [[]]; > + > + root.cascade({ > + before: function (node) { > + let type = node.get('type'); > + let data = node.get('data'); > + let invert = node.get('invert'); > + > + switch (type) { > + case 'all-of': > + case 'any-of': > + case 'one-of': > + stack.push([]); > + break; > + case 'match-calendar': > + { > + let obj = { > + match: { > + type: 'calendar', > + schedule: data.value, > + }, > + }; > + > + if (invert) { > + obj = { > + not: obj, > + }; > + } > + > + stack.at(-1).push(obj); > + } > + > + break; > + case 'match-field': > + { > + let obj = { > + match: { > + type: 'field', > + field: data.field, > + }, > + }; > + > + if (data.type === 'exact') { > + obj.match.values = data.value; > + } else { > + obj.match.regex = data.value; > + } > + > + if (invert) { > + obj = { > + not: obj, > + }; > + } > + > + stack.at(-1).push(obj); > + } > + break; > + case 'match-severity': > + { > + let obj = { > + match: { > + type: 'severity', > + severities: data.value, > + }, > + }; > + > + if (invert) { > + obj = { > + not: obj, > + }; > + } > + > + stack.at(-1).push(obj); > + } > + break; > + case 'match-always': > + stack.at(-1).push({ > + constant: !invert, > + }); > + break; > + } > + > + // Visit this node > + return true; > + }, > + after: function (node) { > + let type = node.get('type'); > + let invert = node.get('invert'); > + > + switch (type) { > + case 'all-of': > + case 'any-of': > + case 'one-of': > + { > + let children = stack.pop(); > + > + let a = {}; > + a[type] = children; > + > + if (invert) { > + a = { > + not: a, > + }; > + } > + > + stack.at(-1).push(a); > + } > + break; > + } > + }, > + }); > + > + let topLevelNodes = stack.pop(); > + let obj = topLevelNodes[0]; > + > + realExpression.suspendEvent('change'); > + realExpression.setValue(JSON.stringify(obj)); > + realExpression.resumeEvent('change'); > + > + // the expression can stay the same while the tree became (in)valid, > + // e.g. when the last child of a group is removed > + realExpression.validate(); > + > + store.resumeEvent('datachanged'); > + }; > + > + realExpression.addListener('change', function (field, value) { > + let obj; > + > + try { > + obj = JSON.parse(value); > + } catch (e) { > + Ext.Msg.alert(gettext('Invalid matcher expression'), e); > + return; > + } > + > + function visit(obj) { > + for (const [key, value] of Object.entries(obj)) { > + let ret; > + > + switch (key) { > + case 'match': > + { this block... > + switch (value.type) { > + case 'calendar': > + { and these blocks are unnecessary. leaving them out reduces the indentation level here... > + ret = { > + type: 'match-calendar', > + data: { > + value: value.schedule, > + }, > + leaf: true, > + invert: false, > + }; > + } > + break; > + case 'field': > + { > + ret = { > + type: 'match-field', > + data: { > + field: value.field, > + }, > + leaf: true, > + invert: false, > + }; > + > + if (value.regex) { > + ret.data.type = 'regex'; > + ret.data.value = value.regex; > + } else { > + ret.data.type = 'exact'; > + ret.data.value = value.values; > + } > + } > + break; > + case 'severity': > + { > + ret = { > + type: 'match-severity', > + data: { > + value: value.severities, > + }, > + leaf: true, > + invert: false, > + }; > + } > + break; > + } > + } > + break; > + case 'all-of': > + case 'any-of': > + case 'one-of': > + { > + let children = []; > + for (let child of value) { > + children.push(visit(child)); > + } > + ret = { > + type: key, > + data: {}, > + children, > + leaf: false, > + invert: false, > + }; > + } > + break; > + case 'not': > + { > + let child = visit(value); > + ret = { > + ...child, > + invert: !child.invert, > + }; > + } > + break; > + case 'constant': { > + ret = { > + type: 'match-always', > + data: {}, > + invert: !value, > + leaf: true, > + }; > + } > + } > + > + let iconCls = me.getNodeIcon( > + ret.type, > + ret.data, > + ret.invert, > + !!ret.children?.length, > + ); > + > + ret.iconCls = iconCls; > + if (!ret.leaf) { > + ret.expanded = true; > + ret.expandable = false; > + } > + > + return ret; > + } > + } > + > + let rootNode = visit(obj); > + > + treeStore.setRootNode(rootNode); > + }); > + > + treeStore.addListener('datachanged', storeChanged); > + > + let treePanel = Ext.create({ > + xtype: 'treepanel', > + store: treeStore, > + // scope for the label wrapping rules in ext6-pmx.css > + cls: 'pmx-match-rule-tree', > + minHeight: 400, > + maxHeight: 400, > + scrollable: true, > + // wrapped labels have no predictable height, so render all rows > + // instead of estimating the scroll range from a sample row > + bufferedRenderer: false, > + > + // Allow drag&drop for tree nodes > + viewConfig: { > + plugins: { > + ptype: 'treeviewdragdrop', > + containerScroll: true, > + }, > + }, > + > + bind: { > + selection: '{selectedRecord}', > + }, > + > + columns: [ > + { > + xtype: 'treecolumn', > + dataIndex: 'text', > + renderer: me.renderTreeLabel, > + flex: 1, > + // rules can list many values, wrap them so that all of > + // them stay visible; this implies variableRowHeight > + cellWrap: true, > + }, > + { > + 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 sub-rule'), v), > + 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 rule'), v), > + getClass: (v, m, { data }) => 'fa fa-trash-o', > + isActionDisabled: (v, r, c, i, rec) => rec.isRoot(), > + }, > + ], > + }, > + ], > + }); > + > + Ext.apply(me, { > + items: [ > + realExpression, > + treePanel, > + { > + // nothing in the tree hints at the nodes being movable > + xtype: 'container', > + margin: '5 0 0 0', > + html: gettext( > + 'Rules can be reordered and moved between groups with drag and drop.', > + ), > + }, > + errorLabel, > + ], > + }); > + 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 sub-rules match')], > + ['any-of', gettext('At least one sub-rule matches')], > + ['one-of', gettext('Exactly one sub-rule matches')], > + ['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', > + }, I meant this here with the viewmodel. instead of binding there, we could also do the hidden, etc. bind here and have the immediate connection... (but that would be part of a cleanup of the whole thing...) > + ], > +}); > diff --git a/src/proxmox-dark/scss/extjs/_treepanel.scss b/src/proxmox-dark/scss/extjs/_treepanel.scss > index 0480371..eb7d1cc 100644 > --- a/src/proxmox-dark/scss/extjs/_treepanel.scss > +++ b/src/proxmox-dark/scss/extjs/_treepanel.scss > @@ -22,3 +22,7 @@ > 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; doesn't that change the type from array to string now? does that have any obvious effects on the older code? > } > > 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: [