[pve-devel] [PATCH manager] Revert "ui: dc: remove notify key from datacenter option view"

[pve-devel] [PATCH manager] Revert "ui: dc: remove notify key from datacenter option view"

[Lukas Wagner]

This reverts commit c81bca2d28744616098448b81fa58e133d3ac5ed.

In the first iteration of the notification system, notification
routing and enabling/disabling notifications was configured via
the (extended) `notify` parameter in `datacenter.cfg`.
Because of that, the configuration UI for this parameter was moved to
a new panel as a part of the notification UI.
When changing to the newer approach for notification routing (matcher
based), the "new" panel this setting was moved to was dropped from the
UI.

Notification sending for package updates is still influenced by this
parameter (see bin/pveupdate, line 55), so there should be a way to
configure this from the GUI. At some point, the `notify` parameter
should be dropped, but that'd be a thing for a major release.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
https://forum.proxmox.com/threads/package-update-notifs-not-working.141182/

Notes:
    Alternatively, we could just *always* send package update
    notifications and just ignore that parameter from now on but this
    might leave users wondering who have previously set
    `package-updates=never`...

 www/manager6/dc/OptionView.js | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/www/manager6/dc/OptionView.js b/www/manager6/dc/OptionView.js
index b200fd12..19884bbe 100644
--- a/www/manager6/dc/OptionView.js
+++ b/www/manager6/dc/OptionView.js
@@ -91,6 +91,26 @@ Ext.define('PVE.dc.OptionView', {
 	    vtype: 'proxmoxMail',
 	    defaultValue: 'root@$hostname',
 	});
+	me.add_inputpanel_row('notify', gettext('Notify'), {
+	    renderer: v => !v ? 'package-updates=auto' : PVE.Parser.printPropertyString(v),
+	    labelWidth: 120,
+	    url: "/api2/extjs/cluster/options",
+	    //onlineHelp: 'ha_manager_shutdown_policy',
+	    items: [{
+		xtype: 'proxmoxKVComboBox',
+		name: 'package-updates',
+		fieldLabel: gettext('Package Updates'),
+		deleteEmpty: false,
+		value: '__default__',
+		comboItems: [
+		    ['__default__', Proxmox.Utils.defaultText + ' (auto)'],
+		    ['auto', gettext('Automatically')],
+		    ['always', gettext('Always')],
+		    ['never', gettext('Never')],
+		],
+		defaultValue: '__default__',
+	    }],
+	});
 	me.add_text_row('mac_prefix', gettext('MAC address prefix'), {
 	    deleteEmpty: true,
 	    vtype: 'MacPrefix',
-- 
2.39.2

Re: [pve-devel] [PATCH manager] Revert "ui: dc: remove notify key from datacenter option view"

[Thomas Lamprecht]

Am 09/02/2024 um 11:16 schrieb Lukas Wagner:
> This reverts commit c81bca2d28744616098448b81fa58e133d3ac5ed.
> 
> In the first iteration of the notification system, notification
> routing and enabling/disabling notifications was configured via
> the (extended) `notify` parameter in `datacenter.cfg`.
> Because of that, the configuration UI for this parameter was moved to
> a new panel as a part of the notification UI.
> When changing to the newer approach for notification routing (matcher
> based), the "new" panel this setting was moved to was dropped from the
> UI.
> 
> Notification sending for package updates is still influenced by this
> parameter (see bin/pveupdate, line 55), so there should be a way to
> configure this from the GUI. At some point, the `notify` parameter
> should be dropped, but that'd be a thing for a major release.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> https://forum.proxmox.com/threads/package-update-notifs-not-working.141182/
> 
> Notes:
>     Alternatively, we could just *always* send package update
>     notifications and just ignore that parameter from now on but this
>     might leave users wondering who have previously set
>     `package-updates=never`...

I'd propose two changes:

- add a hint to redirect users to the new mechanisms so that a future
  deprecation would be more expected (if we already plan that now)

- only show it if defined? While that's a bit magic, it'd avoid that
  users set it, but rather use the new mechanism.
  If, I'd never delete the setting via the UI, so that it doesn't
  suddenly disappears if one switches it from some value to default.

What do you think?

Re: [pve-devel] [PATCH manager] Revert "ui: dc: remove notify key from datacenter option view"

[Lukas Wagner]

On  2024-04-09 15:07, Thomas Lamprecht wrote:
> I'd propose two changes:
> 
> - add a hint to redirect users to the new mechanisms so that a future
>   deprecation would be more expected (if we already plan that now)
> 
> - only show it if defined? While that's a bit magic, it'd avoid that
>   users set it, but rather use the new mechanism.
>   If, I'd never delete the setting via the UI, so that it doesn't
>   suddenly disappears if one switches it from some value to default.
> 
> What do you think?

I think that would make a lot of sense. I'll send a v2 with the suggested
changes.

Thanks a lot for the feedback!

-- 
- Lukas