From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 8F7D41FF0AA for ; Tue, 03 Nov 2026 14:56:18 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id E097F2187C; Tue, 03 Nov 2026 14:56:17 +0100 (CET) Message-ID: <8f8b7a19-02f9-4505-bad2-271631301381@proxmox.com> Date: Tue, 3 Nov 2026 14:56:11 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH proxmox-widget-toolkit v7 22/41] notifications: matcher: add better calendar editor To: Lukas Wagner , pbs-devel@lists.proxmox.com, pve-devel@lists.proxmox.com References: <20261103104859.172466-1-l.wagner@proxmox.com> <20261103104859.172466-23-l.wagner@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: <20261103104859.172466-23-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: 1793714172138 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.328 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: ZDMJGRQOUG4GFJBYKSBSOTUMANY3UCH2 X-Message-ID-Hash: ZDMJGRQOUG4GFJBYKSBSOTUMANY3UCH2 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: a few high level ux things i noticed (some are pre-existing) same start/end time is valid (mon 12-12), not sure if that makes sense ranges are only ever from mon to sun e.g. checking sun,mon,tue could be sun..tue but instead it's mon,tue,sun (no biggie) one issue though: the old input accepted times like: '0:1' e.g. `mon 0:1-12:2` this will not be correctly parsed and hold up the window though it is quetsionable if that should be accepted in the first place, as it's a big ambiguous what is meant e.g. 12:1 -> 12:01 or 12:10? code looks good to me On 11/3/26 11:56 AM, 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. > > Signed-off-by: Lukas Wagner > Reviewed-by: Michael Köppl > Tested-by: Michael Köppl > --- > > Notes: > Changes since v1: > - don't allow users to type anything but digits and : into the > timefields > - Show error message/icon in new match expression edit panel if > - start time is after end time > - no day is selected > > Changes since v2: > - In the time range selectors, set the altFormats property. > This allows users to enter time in varying formats and the > field will automatically normalize the format: > 0:00 -> 00:00 > 730 -> 07:30 > 5 -> 05:00 > > - Accept and normalize existing valid non-standard time durations, > e.g. > > - Mon..Fri 12:00-14:00 > - mon..fri 12:00 - 14:00 > > Change since v3: > - Even broader compatibility for other time range formats accepted > by proxmox-time, support both 'mon' as well as 'monday' > > - Code formatting with proxmox-biome > > Changes since v4: > - Properly parse 'mon8-12' (missing space between day and time) > > Changes since v5: > - Improve regex once again, fixing a regressing that > was introduced in v5 (e.g. 00:00-23:00 did not parse > properly anymore) > - Aligned time range validation with the backend. > The backend accepts start time and end time being the same, > while the UI did not. Changed the UI to allow the same > as the backend > > .../NotificationMatchExpressionEditPanel.js | 27 +- > src/window/NotificationMatcherEdit.js | 350 +++++++++++++++++- > 2 files changed, 363 insertions(+), 14 deletions(-) > > diff --git a/src/panel/NotificationMatchExpressionEditPanel.js b/src/panel/NotificationMatchExpressionEditPanel.js > index ec9fd3c..cff5854 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; > @@ -169,10 +169,19 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTreeExpression', { > mixins: ['Proxmox.Mixin.CBind'], > border: false, > > + // Converts a 'HH:MM' time into minutes since midnight, or NaN if it cannot > + // be parsed. > + parseTime: function (time) { > + let [hours, minutes] = time.split(':'); > + return Number(hours) * 60 + Number(minutes ?? '0'); > + }, > + > // 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) { > + let me = this; > + > switch (type) { > case 'match-severity': { > let v = Ext.isArray(data.value) ? data.value.join(', ') : data.value; > @@ -192,7 +201,16 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTreeExpression', { > case 'match-calendar': { > let v = data.value; > if (!v || !v.length) { > - return gettext('Calendar rules must have a schedule.'); > + return gettext('Calendar rules must match at least one day.'); > + } > + > + // Only schedules which the editor itself can produce are > + // checked, any other one is left to the backend. Unparsable > + // times compare as NaN and are thus not reported either. > + let [, start, end] = /^(?:\S*\s+)?([0-9:]+)-([0-9:]+)$/.exec(v) ?? []; > + > + if (start !== undefined && (me.parseTime(start) > me.parseTime(end))) { > + return gettext("A calendar rule's start time cannot be after the end time."); > } > break; > } > @@ -330,7 +348,10 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTreeExpression', { > } > } > case 'match-calendar': { > - let value = data.value ? data.value : '?'; > + // A schedule which is not fully specified, for instance because > + // no weekday is selected, renders as blank, so show it as unset > + // instead. > + let value = Ext.isString(data.value) && data.value.trim() ? data.value : '?'; > value = `${Ext.String.htmlEncode(value)}`; > > if (invert) { > diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js > index 893c3e3..4e1aa59 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,346 @@ 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 matches = /^(?:([A-Za-z.,]+)\s*)?([0-9:]+)\s*-\s*([0-9:]+)$/.exec(timeframe); > + > + // Schedules which the editor cannot represent, an empty one in > + // particular, start out without any selected day and are thus > + // shown as invalid. > + if (!matches) { > + return { > + start: '00:00', > + end: '23:59', > + }; > + } > + > + let [, days, start, end] = matches; > + > + if (start === '0') { > + start = '00:00'; > + } > + > + let record = { > + start, > + end, > + }; > + > + if (!days) { > + days = 'mon..sun'; > + } > + > + days = days.split(','); > + days.forEach((day) => { > + // We don't care about casing > + day = day.toLowerCase(); > + > + if (record[day]) { > + return; > + } > + > + if (day.includes('..')) { > + // we have a range 'xxx..yyy' > + let [startDay, endDay] = day.split('..'); > + > + // We accept both 'monday' and 'mon' > + startDay = startDay.substring(0, 3); > + endDay = endDay.substring(0, 3); > + > + 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; > + } > + } > + } else { > + // We accept both 'monday' and 'mon' > + let dayTrimmed = day.substring(0, 3); > + if (me.weekdays.indexOf(dayTrimmed) !== -1) { > + record[dayTrimmed] = 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 days = me.formatSelectedDays(data.data); > + > + let start = me.formatTime(startField); > + let end = me.formatTime(endField); > + > + // An empty selection cannot be expressed, as a schedule without > + // weekdays already matches every day. Store an empty schedule > + // instead, which is reported as invalid by the rule tree. > + let timeframe = ''; > + > + if (days !== undefined) { > + timeframe = days ? `${days} ${start}-${end}` : `${start}-${end}`; > + } > + > + let field = me.lookup('timeframe'); > + field.suspendEvent('change'); > + field.setValue(timeframe); > + > + me.getViewModel().set('matchCalendarValue', timeframe); > + > + field.resumeEvent('change'); > + }, > + > + // Returns the weekday part of a schedule, which is empty if every day is > + // selected, or `undefined` if no day is selected at all. > + formatSelectedDays: function (days) { > + let me = this; > + let selected = me.weekdays.filter((day) => days[day]); > + > + if (selected.length === 0) { > + return undefined; > + } > + if (selected.length === me.weekdays.length) { > + return ''; > + } > + if (selected.length === 1) { > + return selected[0]; > + } > + > + // Check if selected days are a contiguous block in weekday order > + let indices = selected.map((day) => me.weekdays.indexOf(day)); > + let isContiguous = indices.every((idx, i) => i === 0 || idx === indices[i - 1] + 1); > + > + if (isContiguous) { > + return `${selected[0]}..${selected[selected.length - 1]}`; > + } > + > + return selected.join(','); > + }, > + > + formatTime: function (timefield) { > + let value = timefield.getValue(); > + > + if (!value) { > + return ''; > + } > + > + let hours = value.getHours().toString().padStart(2, '0'); > + let minutes = value.getMinutes().toString().padStart(2, '0'); > + return `${hours}:${minutes}`; > + }, > + }, > items: [ > { > - xtype: 'proxmoxKVComboBox', > - fieldLabel: gettext('Timespan to match'), > + xtype: 'hidden', > + reference: 'timeframe', > isFormField: false, > allowBlank: false, > - editable: true, > - displayField: 'key', > - field: 'value', > bind: { > value: '{matchCalendarValue}', > - disabled: '{!typeIsMatchCalender}', > }, > - > - comboItems: [ > - ['mon 8-12', ''], > - ['tue..fri,sun 0:00-23:59', ''], > + }, > + { > + xtype: 'timefield', > + reference: 'timeStart', > + fieldLabel: gettext('Time Start'), > + isFormField: false, > + format: 'H:i', > + altFormats: 'G:i|Gi|G', > + formatText: 'HH:MM', > + maskRe: /[\d:]/, > + stripCharsRe: /[^\d:]+/g, > + allowBlank: false, > + }, > + { > + xtype: 'timefield', > + reference: 'timeEnd', > + fieldLabel: gettext('Time End'), > + isFormField: false, > + format: 'H:i', > + altFormats: 'G:i|Gi|G', > + formatText: 'HH:MM', > + maxValue: '23:59', > + maskRe: /[\d:]/, > + stripCharsRe: /[^\d:]+/g, > + allowBlank: false, > + }, > + { > + xtype: 'fieldcontainer', > + items: [ > + { > + xtype: 'grid', > + margin: '10 0 0 0', > + reference: 'weekdayGrid', > + store: { > + fields: ['mon', 'tue', 'wed', 'thu', 'fri', 'sat', 'sun'], > + data: [ > + { > + mon: true, > + tue: true, > + wed: true, > + thu: true, > + fri: true, > + sat: true, > + sun: true, > + }, > + ], > + }, > + enableColumnMove: false, > + columns: { > + defaults: { > + menuDisabled: true, > + resizable: false, > + }, > + items: [ > + { > + text: gettext('Mon'), > + xtype: 'widgetcolumn', > + dataIndex: 'mon', > + flex: 1, > + widget: { > + xtype: 'checkbox', > + isFormField: false, > + }, > + }, > + { > + text: gettext('Tue'), > + xtype: 'widgetcolumn', > + dataIndex: 'tue', > + flex: 1, > + widget: { > + xtype: 'checkbox', > + isFormField: false, > + }, > + }, > + { > + text: gettext('Wed'), > + xtype: 'widgetcolumn', > + dataIndex: 'wed', > + flex: 1, > + widget: { > + xtype: 'checkbox', > + isFormField: false, > + }, > + }, > + { > + text: gettext('Thu'), > + xtype: 'widgetcolumn', > + dataIndex: 'thu', > + flex: 1, > + widget: { > + xtype: 'checkbox', > + isFormField: false, > + }, > + }, > + { > + text: gettext('Fri'), > + xtype: 'widgetcolumn', > + dataIndex: 'fri', > + flex: 1, > + widget: { > + xtype: 'checkbox', > + isFormField: false, > + }, > + }, > + { > + text: gettext('Sat'), > + xtype: 'widgetcolumn', > + dataIndex: 'sat', > + flex: 1, > + widget: { > + xtype: 'checkbox', > + isFormField: false, > + }, > + }, > + { > + text: gettext('Sun'), > + xtype: 'widgetcolumn', > + dataIndex: 'sun', > + flex: 1, > + widget: { > + xtype: 'checkbox', > + isFormField: false, > + }, > + }, > + ], > + }, > + }, > ], > }, > ],