[pve-devel] [PATCH v3 pve-manager 54/66] ui: backup: allow to select notification target for jobs
Dominik Csapak
d.csapak at proxmox.com
Wed Jul 19 14:20:59 CEST 2023
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 at 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 <l.wagner at proxmox.com>
> ---
> 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,
> + },
> + ],
> + },
> +});
More information about the pve-devel
mailing list