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 E04A51FF0A5 for ; Fri, 04 Sep 2026 14:16:34 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 9922E21591; Fri, 04 Sep 2026 14:16:34 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 04 Sep 2026 14:16:29 +0200 Message-Id: To: "Arthur Bied-Charreton" , "Lukas Wagner" Subject: Re: [PATCH proxmox-widget-toolkit 18/29] notification: matcher: add better calendar editor From: "Lukas Wagner" X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260709115716.299836-1-l.wagner@proxmox.com> <20260709115716.299836-19-l.wagner@proxmox.com> In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1788524185799 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.470 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: TMSE7PHXMGWU4VNZGRT55EIIWEIHUYPS X-Message-ID-Hash: TMSE7PHXMGWU4VNZGRT55EIIWEIHUYPS X-MailFrom: l.wagner@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 Fri Jul 24, 2026 at 8:46 AM CEST, Arthur Bied-Charreton wrote: > 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') >>=20 >> 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. >>=20 > one comment inline >> Signed-off-by: Lukas Wagner >> --- >> .../NotificationMatchExpressionEditPanel.js | 2 +- >> src/window/NotificationMatcherEdit.js | 300 +++++++++++++++++- >> 2 files changed, 290 insertions(+), 12 deletions(-) >>=20 >> diff --git a/src/panel/NotificationMatchExpressionEditPanel.js b/src/pan= el/NotificationMatchExpressionEditPanel.js >> index 6d1f93e..e117ea5 100644 >> --- a/src/panel/NotificationMatchExpressionEditPanel.js >> +++ b/src/panel/NotificationMatchExpressionEditPanel.js >> @@ -87,7 +87,7 @@ Ext.define('Proxmox.panel.NotificationMatchExpressionE= ditPanel', { >> break; >> case 'match-calendar': >> data =3D { >> - value: '', >> + value: '00:00-23:59', >> }; >> leaf =3D true; >> break; >> diff --git a/src/window/NotificationMatcherEdit.js b/src/window/Notifica= tionMatcherEdit.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 =3D { >> - value: '', >> + value: '00:00-23:59', >> }; >> break; >> } >> @@ -977,6 +977,11 @@ Ext.define('Proxmox.panel.MatchCalendarSettings', { >> }, >> set: function (value) { >> let me =3D this; >> + >> + if (!me.get('typeIsMatchCalendar')) { >> + return; >> + } >> + >> let record =3D me.get('selectedRecord'); >> let currentData =3D 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=3Dtimeframe]': { >> + change: 'setGridData', >> + }, >> + }, >> + >> + weekdays: ['mon', 'tue', 'wed', 'thu', 'fri', 'sat', 'sun'], >> + >> + setGridData: function (field, value) { >> + let me =3D this; >> + >> + let record =3D 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 =3D this; >> + let [, days, start, end] =3D /^(?:(\S*)\s+)?([0-9:]+)-([0-9= :]+)$/.exec(timeframe) || []; >> + >> + if (start =3D=3D=3D '0') { >> + start =3D '00:00'; >> + } >> + >> + let record =3D { >> + start, >> + end, >> + }; >> + >> + if (!days) { >> + days =3D 'mon..sun'; >> + } >> + >> + days =3D days.split(','); >> + days.forEach((day) =3D> { >> + if (record[day]) { >> + return; >> + } >> + >> + if (me.weekdays.indexOf(day) !=3D=3D -1) { >> + record[day] =3D true; >> + } else { >> + // we have a range 'xxx..yyy' >> + let [startDay, endDay] =3D day.split('..'); >> + let startIdx =3D me.weekdays.indexOf(startDay); >> + let endIdx =3D me.weekdays.indexOf(endDay); >> + >> + if (endIdx < startIdx) { >> + endIdx +=3D me.weekdays.length; >> + } >> + >> + for (let dayIdx =3D startIdx; dayIdx <=3D endIdx; d= ayIdx++) { >> + let curDay =3D me.weekdays[dayIdx % me.weekdays= .length]; >> + if (!record[curDay]) { >> + record[curDay] =3D true; >> + } >> + } >> + } >> + }); >> + >> + return record; >> + }, >> + >> + dowChanged: function (field, value) { >> + let me =3D this; >> + let record =3D field.getWidgetRecord(); >> + if (record =3D=3D=3D undefined) { >> + // this is sometimes called before a record/column is i= nitialized >> + return; >> + } >> + let col =3D field.getWidgetColumn(); >> + record.set(col.dataIndex, value); >> + record.commit(); >> + >> + let startField =3D me.lookup('timeStart'); >> + let endField =3D me.lookup('timeEnd'); >> + >> + me.updateTimeframeField(startField, endField); >> + }, >> + >> + timeChanged: function (field, value) { >> + let me =3D this; >> + >> + let startField =3D me.lookup('timeStart'); >> + let endField =3D me.lookup('timeEnd'); >> + >> + let start =3D startField.getValue(); >> + let end =3D endField.getValue(); >> + >> + let valid =3D !(start && end && start >=3D end); >> + >> + if (!valid) { >> + startField.markInvalid(gettext('Start time must be befo= re end time')); >> + endField.markInvalid(gettext('End time must be after st= art time')); >> + } else { >> + startField.clearInvalid(); >> + endField.clearInvalid(); >> + } >> + >> + me.updateTimeframeField(startField, endField); >> + }, >> + >> + updateTimeframeField: function (startField, endField) { >> + let me =3D this; >> + >> + let data =3D me.lookup('weekdayGrid').getStore().getData().= getAt(0); >> + >> + let timeframe =3D me.formatSelectedDays(data.data); >> + >> + let start =3D me.formatTime(startField); >> + let end =3D me.formatTime(endField); >> + >> + timeframe +=3D ` ${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 schedu= le: unable to parse daily duration at ' 08:00-12:00'=20 > > i think a good approach could be blocking creation of calendar matchers= =20 > without days selected in the UI. the semantics of the day-less calendar= =20 > expression '8-12' are "every day from 8-12", which the UI already=20 > represents as "8-12 + all days selected", so not sure what not=20 > selecting any day would/should mean except "never match"?=20 Yeah, its a bit odd right now. For the record, the same odd semantics are used for traffic control schedules in PBS; there all days selected means the same as no days selected, so whatever we decide here should be applied there as well. I think i could indeed make sense to require at least one day to be selected in the UI. I will try that in v2. > > 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@p= roxmox.com/T/#mf1dabe82fe5ecce4fadd6860689167c6734ee0d4 >> + >> + let field =3D me.lookup('timeframe'); >> + field.suspendEvent('change'); >> + field.setValue(timeframe); >> + >> + me.getViewModel().set('matchCalendarValue', timeframe); >> + >> + field.resumeEvent('change'); >> + }, >> + > [...]