Re: [PATCH proxmox-widget-toolkit 17/29] notifications: matcher: add support for match expressions

[Arthur Bied-Charreton <a.bied-charreton@proxmox.com>]

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 <l.wagner@proxmox.com>
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
> 
> 
> 
> 
>