Re: [PATCH proxmox-widget-toolkit 18/29] notification: matcher: add better calendar editor

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

On Thu, Jul 09, 2026 at 01:57:05PM +0200, Lukas Wagner wrote:
> This new editor allows one to enter the start time, end time and tick
> the matched week-days, instead of having to enter the appropriate string
> representation of the time range (e.g. 'mon..tue 08:00-12:00')
> 
> The general approach was copied from PBS's traffic rule edit panel, but
> it is too different to generalize this into a new, reusable component.
> 
one comment inline
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  .../NotificationMatchExpressionEditPanel.js   |   2 +-
>  src/window/NotificationMatcherEdit.js         | 300 +++++++++++++++++-
>  2 files changed, 290 insertions(+), 12 deletions(-)
> 
> diff --git a/src/panel/NotificationMatchExpressionEditPanel.js b/src/panel/NotificationMatchExpressionEditPanel.js
> index 6d1f93e..e117ea5 100644
> --- a/src/panel/NotificationMatchExpressionEditPanel.js
> +++ b/src/panel/NotificationMatchExpressionEditPanel.js
> @@ -87,7 +87,7 @@ Ext.define('Proxmox.panel.NotificationMatchExpressionEditPanel', {
>                              break;
>                          case 'match-calendar':
>                              data = {
> -                                value: '',
> +                                value: '00:00-23:59',
>                              };
>                              leaf = true;
>                              break;
> diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js
> index 893c3e3..433f86c 100644
> --- a/src/window/NotificationMatcherEdit.js
> +++ b/src/window/NotificationMatcherEdit.js
> @@ -361,7 +361,7 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
>                              break;
>                          case 'match-calendar':
>                              data = {
> -                                value: '',
> +                                value: '00:00-23:59',
>                              };
>                              break;
>                      }
> @@ -977,6 +977,11 @@ Ext.define('Proxmox.panel.MatchCalendarSettings', {
>                  },
>                  set: function (value) {
>                      let me = this;
> +
> +                    if (!me.get('typeIsMatchCalendar')) {
> +                        return;
> +                    }
> +
>                      let record = me.get('selectedRecord');
>                      let currentData = record.get('data');
>                      record.set({
> @@ -992,23 +997,296 @@ Ext.define('Proxmox.panel.MatchCalendarSettings', {
>              },
>          },
>      },
> +    controller: {
> +        xclass: 'Ext.app.ViewController',
> +        control: {
> +            'grid checkbox': {
> +                change: 'dowChanged',
> +            },
> +            timefield: {
> +                change: 'timeChanged',
> +            },
> +            'field[reference=timeframe]': {
> +                change: 'setGridData',
> +            },
> +        },
> +
> +        weekdays: ['mon', 'tue', 'wed', 'thu', 'fri', 'sat', 'sun'],
> +
> +        setGridData: function (field, value) {
> +            let me = this;
> +
> +            let record = me.parseTimeframe(value);
> +
> +            me.lookup('weekdayGrid').getStore().setData([record]);
> +            me.lookup('timeStart').setValue(record.start);
> +            me.lookup('timeEnd').setValue(record.end);
> +        },
> +
> +        parseTimeframe: function (timeframe) {
> +            let me = this;
> +            let [, days, start, end] = /^(?:(\S*)\s+)?([0-9:]+)-([0-9:]+)$/.exec(timeframe) || [];
> +
> +            if (start === '0') {
> +                start = '00:00';
> +            }
> +
> +            let record = {
> +                start,
> +                end,
> +            };
> +
> +            if (!days) {
> +                days = 'mon..sun';
> +            }
> +
> +            days = days.split(',');
> +            days.forEach((day) => {
> +                if (record[day]) {
> +                    return;
> +                }
> +
> +                if (me.weekdays.indexOf(day) !== -1) {
> +                    record[day] = true;
> +                } else {
> +                    // we have a range 'xxx..yyy'
> +                    let [startDay, endDay] = day.split('..');
> +                    let startIdx = me.weekdays.indexOf(startDay);
> +                    let endIdx = me.weekdays.indexOf(endDay);
> +
> +                    if (endIdx < startIdx) {
> +                        endIdx += me.weekdays.length;
> +                    }
> +
> +                    for (let dayIdx = startIdx; dayIdx <= endIdx; dayIdx++) {
> +                        let curDay = me.weekdays[dayIdx % me.weekdays.length];
> +                        if (!record[curDay]) {
> +                            record[curDay] = true;
> +                        }
> +                    }
> +                }
> +            });
> +
> +            return record;
> +        },
> +
> +        dowChanged: function (field, value) {
> +            let me = this;
> +            let record = field.getWidgetRecord();
> +            if (record === undefined) {
> +                // this is sometimes called before a record/column is initialized
> +                return;
> +            }
> +            let col = field.getWidgetColumn();
> +            record.set(col.dataIndex, value);
> +            record.commit();
> +
> +            let startField = me.lookup('timeStart');
> +            let endField = me.lookup('timeEnd');
> +
> +            me.updateTimeframeField(startField, endField);
> +        },
> +
> +        timeChanged: function (field, value) {
> +            let me = this;
> +
> +            let startField = me.lookup('timeStart');
> +            let endField = me.lookup('timeEnd');
> +
> +            let start = startField.getValue();
> +            let end = endField.getValue();
> +
> +            let valid = !(start && end && start >= end);
> +
> +            if (!valid) {
> +                startField.markInvalid(gettext('Start time must be before end time'));
> +                endField.markInvalid(gettext('End time must be after start time'));
> +            } else {
> +                startField.clearInvalid();
> +                endField.clearInvalid();
> +            }
> +
> +            me.updateTimeframeField(startField, endField);
> +        },
> +
> +        updateTimeframeField: function (startField, endField) {
> +            let me = this;
> +
> +            let data = me.lookup('weekdayGrid').getStore().getData().getAt(0);
> +
> +            let timeframe = me.formatSelectedDays(data.data);
> +
> +            let start = me.formatTime(startField);
> +            let end = me.formatTime(endField);
> +
> +            timeframe += ` ${start}-${end}`;
when no days are selected, this creates a timeframe with a leading
whitespace (` 8:00-12:00`), which the backend fails to parse with the
following error:

invalid matcher config: 'expression' is not valid: could not parse schedule: unable to parse daily duration at ' 08:00-12:00' 

i think a good approach could be blocking creation of calendar matchers 
without days selected in the UI. the semantics of the day-less calendar 
expression '8-12' are "every day from 8-12", which the UI already 
represents as "8-12 + all days selected", so not sure what not 
selecting any day would/should mean except "never match"? 

in the backend, we might wanna trim the input to be more robust against
whitespace errors as well [0] (no very strong opinion on that though,
mostly got confused by the error message).

[0] https://lore.proxmox.com/pve-devel/20260709115716.299836-1-l.wagner@proxmox.com/T/#mf1dabe82fe5ecce4fadd6860689167c6734ee0d4
> +
> +            let field = me.lookup('timeframe');
> +            field.suspendEvent('change');
> +            field.setValue(timeframe);
> +
> +            me.getViewModel().set('matchCalendarValue', timeframe);
> +
> +            field.resumeEvent('change');
> +        },
> +
[...]