From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 486F7E8EF for ; Wed, 19 Jul 2023 14:21:32 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 233B57239 for ; Wed, 19 Jul 2023 14:21:02 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 19 Jul 2023 14:21:01 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id E846D40FF0; Wed, 19 Jul 2023 14:21:00 +0200 (CEST) Message-ID: <077e07e8-5a21-b454-367d-718d78bcc639@proxmox.com> Date: Wed, 19 Jul 2023 14:20:59 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox VE development discussion , Lukas Wagner References: <20230717150051.710464-1-l.wagner@proxmox.com> <20230717150051.710464-55-l.wagner@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: <20230717150051.710464-55-l.wagner@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.016 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [values.id] Subject: Re: [pve-devel] [PATCH v3 pve-manager 54/66] ui: backup: allow to select notification target for jobs X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Jul 2023 12:21:32 -0000 looks fine, one comment inline On 7/17/23 17:00, Lukas Wagner wrote: > This commit adds a possibility to choose between different options > for notifications for backup jobs: > - Notify via email, in the same manner as before > - Notify via an endpoint/group > > If 'notify via mail' is selected, a text field where an email address > can be entered is displayed: > > Notify: | Always notify v | > Notify via: | E-Mail v | > Send Mail to: | foo@example.com | > Compression: | ..... v | > > If the other option is selected selected, a combo picker for selecting > a channel is displayed: > > Notify: | Always notify v | > Notify via: | Endpoint/Group v | > Target: | endpoint-foo v | > Compression: | ..... v | > > The code has also been adapted to use the newly introduced > 'notification-policy' parameter, which replaces the 'mailnotification' > paramter for backup jobs. Some logic which automatically migrates from > 'mailnotification' has been added. > > Signed-off-by: Lukas Wagner > --- > www/manager6/Makefile | 4 +- > www/manager6/dc/Backup.js | 84 +++++++++++++++++-- > www/manager6/form/NotificationModeSelector.js | 8 ++ > ...ector.js => NotificationPolicySelector.js} | 1 + > .../form/NotificationTargetSelector.js | 54 ++++++++++++ > 5 files changed, 143 insertions(+), 8 deletions(-) > create mode 100644 www/manager6/form/NotificationModeSelector.js > rename www/manager6/form/{EmailNotificationSelector.js => NotificationPolicySelector.js} (87%) > create mode 100644 www/manager6/form/NotificationTargetSelector.js > > diff --git a/www/manager6/Makefile b/www/manager6/Makefile > index 5b455c80..140b20f0 100644 > --- a/www/manager6/Makefile > +++ b/www/manager6/Makefile > @@ -36,7 +36,6 @@ JSSRC= \ > form/DayOfWeekSelector.js \ > form/DiskFormatSelector.js \ > form/DiskStorageSelector.js \ > - form/EmailNotificationSelector.js \ > form/FileSelector.js \ > form/FirewallPolicySelector.js \ > form/GlobalSearchField.js \ > @@ -51,6 +50,9 @@ JSSRC= \ > form/MultiPCISelector.js \ > form/NetworkCardSelector.js \ > form/NodeSelector.js \ > + form/NotificationModeSelector.js \ > + form/NotificationTargetSelector.js \ > + form/NotificationPolicySelector.js \ > form/PCISelector.js \ > form/PCIMapSelector.js \ > form/PermPathSelector.js \ > diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js > index 03a02651..625b5430 100644 > --- a/www/manager6/dc/Backup.js > +++ b/www/manager6/dc/Backup.js > @@ -36,6 +36,30 @@ Ext.define('PVE.dc.BackupEdit', { > delete values.node; > } > > + if (!isCreate) { > + // 'mailnotification' is deprecated in favor of 'notification-policy' > + // -> Migration to the new parameter happens in init, so we are > + // safe to remove the old parameter here. > + Proxmox.Utils.assemble_field_data(values, { 'delete': 'mailnotification' }); > + > + // If sending notifications via mail, remove the current value of > + // 'notification-target' > + if (values['notification-mode'] === "mailto") { > + Proxmox.Utils.assemble_field_data( > + values, > + { 'delete': 'notification-target' }, > + ); > + } else { > + // and vice versa... > + Proxmox.Utils.assemble_field_data( > + values, > + { 'delete': 'mailto' }, > + ); > + } > + } > + > + delete values['notification-mode']; > + > if (!values.id && isCreate) { > values.id = 'backup-' + Ext.data.identifier.Uuid.Global.generate().slice(0, 13); > } > @@ -146,6 +170,22 @@ Ext.define('PVE.dc.BackupEdit', { > success: function(response, _options) { > let data = response.result.data; > > + // 'mailnotification' is deprecated. Let's automatically > + // migrate to the compatible 'notification-policy' parameter > + if (data.mailnotification) { > + if (!data["notification-policy"]) { > + data["notification-policy"] = data.mailnotification; > + } > + > + delete data.mailnotification; > + } > + > + if (data['notification-target']) { > + data['notification-mode'] = 'notification-target'; > + } else if (data.mailto) { > + data['notification-mode'] = 'mailto'; > + } > + > if (data.exclude) { > data.vmid = data.exclude; > data.selMode = 'exclude'; > @@ -188,11 +228,13 @@ Ext.define('PVE.dc.BackupEdit', { > viewModel: { > data: { > selMode: 'include', > + notificationMode: 'notification-target', > }, > > formulas: { > poolMode: (get) => get('selMode') === 'pool', > disableVMSelection: (get) => get('selMode') !== 'include' && get('selMode') !== 'exclude', > + mailNotificationSelected: (get) => get('notificationMode') === 'mailto', > }, > }, > > @@ -282,20 +324,48 @@ Ext.define('PVE.dc.BackupEdit', { > }, > ], > column2: [ > - { > - xtype: 'textfield', > - fieldLabel: gettext('Send email to'), > - name: 'mailto', > - }, > { > xtype: 'pveEmailNotificationSelector', > - fieldLabel: gettext('Email'), > - name: 'mailnotification', > + fieldLabel: gettext('Notify'), > + name: 'notification-policy', > cbind: { > value: (get) => get('isCreate') ? 'always' : '', > deleteEmpty: '{!isCreate}', > }, > }, > + { > + xtype: 'pveNotificationModeSelector', > + fieldLabel: gettext('Notify via'), > + name: 'notification-mode', > + bind: { > + value: '{notificationMode}', > + }, > + }, > + { > + xtype: 'pveNotificationTargetSelector', > + fieldLabel: gettext('Notification Target'), > + name: 'notification-target', > + allowBlank: true, > + editable: true, > + autoSelect: false, > + bind: { > + hidden: '{mailNotificationSelected}', > + disabled: '{mailNotificationSelected}', > + }, > + cbind: { > + deleteEmpty: '{!isCreate}', > + }, > + }, > + { > + xtype: 'textfield', > + fieldLabel: gettext('Send email to'), > + name: 'mailto', > + hidden: true, > + bind: { > + hidden: '{!mailNotificationSelected}', > + disabled: '{!mailNotificationSelected}', > + }, > + }, > { > xtype: 'pveCompressionSelector', > reference: 'compressionSelector', > diff --git a/www/manager6/form/NotificationModeSelector.js b/www/manager6/form/NotificationModeSelector.js > new file mode 100644 > index 00000000..58fddd56 > --- /dev/null > +++ b/www/manager6/form/NotificationModeSelector.js > @@ -0,0 +1,8 @@ > +Ext.define('PVE.form.NotificationModeSelector', { > + extend: 'Proxmox.form.KVComboBox', > + alias: ['widget.pveNotificationModeSelector'], > + comboItems: [ > + ['notification-target', gettext('Target')], > + ['mailto', gettext('E-Mail')], > + ], > +}); > diff --git a/www/manager6/form/EmailNotificationSelector.js b/www/manager6/form/NotificationPolicySelector.js > similarity index 87% > rename from www/manager6/form/EmailNotificationSelector.js > rename to www/manager6/form/NotificationPolicySelector.js > index f318ea18..68087275 100644 > --- a/www/manager6/form/EmailNotificationSelector.js > +++ b/www/manager6/form/NotificationPolicySelector.js > @@ -4,5 +4,6 @@ Ext.define('PVE.form.EmailNotificationSelector', { > comboItems: [ > ['always', gettext('Notify always')], > ['failure', gettext('On failure only')], > + ['never', gettext('Notify never')], > ], > }); > diff --git a/www/manager6/form/NotificationTargetSelector.js b/www/manager6/form/NotificationTargetSelector.js > new file mode 100644 > index 00000000..9ead28e7 > --- /dev/null > +++ b/www/manager6/form/NotificationTargetSelector.js > @@ -0,0 +1,54 @@ > +Ext.define('PVE.form.NotificationTargetSelector', { > + extend: 'Proxmox.form.ComboGrid', > + alias: ['widget.pveNotificationTargetSelector'], > + > + // set default value to empty array, else it inits it with > + // null and after the store load it is an empty array, > + // triggering dirtychange > + value: [], seeing this sent me down a small rabbit hole, since i was convinced this would only be necessary for multiselect combogrids (like the nodeselector, which i guess is the component you copied this from?) anyway, i sent a series for wt/manager that should make that unnecessary, so we might coordinate that (if my patches are not applied when you send the next version, just leave it as is, we can remove it later) > + valueField: 'name', > + displayField: 'name', > + deleteEmpty: true, > + skipEmptyText: true, > + > + store: { > + fields: ['name', 'type', 'comment'], > + proxy: { > + type: 'proxmox', > + url: '/api2/json/cluster/notifications/targets', > + }, > + sorters: [ > + { > + property: 'name', > + direction: 'ASC', > + }, > + ], > + autoLoad: true, > + }, > + > + listConfig: { > + columns: [ > + { > + header: gettext('Target'), > + dataIndex: 'name', > + sortable: true, > + hideable: false, > + flex: 1, > + }, > + { > + header: gettext('Type'), > + dataIndex: 'type', > + sortable: true, > + hideable: false, > + flex: 1, > + }, > + { > + header: gettext('Comment'), > + dataIndex: 'comment', > + sortable: true, > + hideable: false, > + flex: 2, > + }, > + ], > + }, > +});