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:   | f...@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.wag...@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,
+           },
+       ],
+    },
+});



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to