[pbs-devel] [PATCH proxmox{,-backup} 00/10] change of default notification-mode; UX improvements

[pbs-devel] [PATCH proxmox{,-backup} 00/10] change of default notification-mode; UX improvements

[Lukas Wagner]

This is intended for the PBS 4.0 release (trixie based).

This patch series changes the default notification mode for PBS to
'notification-system' from the former default of 'legacy-sendmail'.

The rationale for the new default is:
  - 'notification-system' was already used as a default for any datastore 
    created by the GUI
  - consistency, since on PVE, the default is 'auto' (which does not exist
    on PBS, but is equivalent to 'notification-system' unless an email 
    address has been added as well)
  - users are gently nudged to use the new notification stack

The new default affects any datastore/tape backup job that has no explicit
value set. The proxmox-backup part of this series contains migration code which
is invoked from d/postinst to migrate users who relied on the old default to
the appropriate explicit setting.

As mentioned above, the GUI already explicitly set the value to the new default
for a long time. As a result, the changed default mostly affects newly created
datastores/tape jobs which were created via third-party API users or the CLI.

This patch series also contains various UX improvements to make the two
notification modes much easier to understand. The approach is generally the
same as in my series for PVE [1].

[1]: https://lore.proxmox.com/all/20250617084448.87536-1-l.wagner@proxmox.com/T/#t

proxmox:

Lukas Wagner (1):
  pbs-api-types: change default notification mode to
    'notification-system'

 pbs-api-types/src/datastore.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


proxmox-backup:

Lukas Wagner (9):
  cli: manager: move update-to-prune-jobs command to new migrate-config
    sub-command
  cli: manager: add 'migrate-config default-notification-mode' command
  d/postinst: migrate notification mode default on update
  ui: datastore options view: switch to new notification-mode default
  ui: tape backup job: move notification settings to a separate tab
  ui: one-shot tape backup: use same wording as tape-backup jobs
  ui: datastore options: notifications: use same jargon as tape-jobs and
    PVE
  ui: datastore options: drop notify and notify-user rows
  ui: datastore options: notification: use radio controls to select mode

 debian/postinst                               |   9 +-
 src/bin/proxmox-backup-manager.rs             |   6 +-
 .../proxmox_backup_manager/migrate_config.rs  | 171 ++++++++++++++++++
 src/bin/proxmox_backup_manager/mod.rs         |   1 +
 src/bin/proxmox_backup_manager/prune.rs       |  87 +--------
 www/datastore/OptionView.js                   |  33 +---
 www/tape/window/TapeBackup.js                 |  10 +-
 www/tape/window/TapeBackupJob.js              | 109 ++++++-----
 www/window/NotifyOptions.js                   |  43 +++--
 9 files changed, 292 insertions(+), 177 deletions(-)
 create mode 100644 src/bin/proxmox_backup_manager/migrate_config.rs


Summary over all repositories:
  10 files changed, 293 insertions(+), 178 deletions(-)

-- 
Generated by git-murpp 0.8.1


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

[pbs-devel] [PATCH proxmox 1/1] pbs-api-types: change default notification mode to 'notification-system'

[Lukas Wagner]

The former default was chose avoid breaking changes during a minor
release, but with the upcoming major release we can change the default.

The rationale for the new default is:
  - 'notification-system' was already used as a default for any datastore
    created by the GUI
  - consistency, since on PVE, the default is 'auto' (which does not exist
    on PBS, but is equivalent to 'notification-system' unless an email
    address has been added as well)
  - users are gently nudged to use the new notification stack

The new default affects any datastore/tape backup job that has no
explicit value set. On the PBS side some migration code will be added to
d/postinst to migrate users who relied on the old default to the
appropriate explicit setting.

As mentioned above, the GUI already explicitly set the value to the new
default for a long time. As a result, this mostly affects newly created
datastores/tape jobs from the CLI/API.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 5bd953ac..ecbeed8e 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -402,9 +402,9 @@ pub struct DataStoreConfig {
 pub enum NotificationMode {
     /// Send notifications via the system's sendmail command to the user
     /// configured in `notify-user`
-    #[default]
     LegacySendmail,
     /// Emit notification events to the notification system
+    #[default]
     NotificationSystem,
 }
 
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 1/9] cli: manager: move update-to-prune-jobs command to new migrate-config sub-command

[Lukas Wagner]

The new subcommand is introduced so that we have a common name space for
any config migration tasks which are triggered by d/postinst (or potentially
by hand).

No functional changes.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    I guess at this point we could also drop this code entirely?

 debian/postinst                               |  2 +-
 src/bin/proxmox-backup-manager.rs             |  6 +-
 .../proxmox_backup_manager/migrate_config.rs  | 98 +++++++++++++++++++
 src/bin/proxmox_backup_manager/mod.rs         |  1 +
 src/bin/proxmox_backup_manager/prune.rs       | 87 +---------------
 5 files changed, 104 insertions(+), 90 deletions(-)
 create mode 100644 src/bin/proxmox_backup_manager/migrate_config.rs

diff --git a/debian/postinst b/debian/postinst
index f38a8c66..7f4c57cc 100644
--- a/debian/postinst
+++ b/debian/postinst
@@ -35,7 +35,7 @@ case "$1" in
 
 		if dpkg --compare-versions "$2" 'lt' '2.2.2~'; then
 			echo "moving prune schedule from datacenter config to new prune job config"
-			proxmox-backup-manager update-to-prune-jobs-config \
+			proxmox-backup-manager migrate-config update-to-prune-jobs-config \
 			    || echo "Failed to move prune jobs, please check manually"
 			true
 		fi
diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
index d4363e71..378c5c7f 100644
--- a/src/bin/proxmox-backup-manager.rs
+++ b/src/bin/proxmox-backup-manager.rs
@@ -712,9 +712,9 @@ async fn run() -> Result<(), Error> {
         .insert("report", CliCommand::new(&API_METHOD_REPORT))
         .insert("versions", CliCommand::new(&API_METHOD_GET_VERSIONS));
 
-    let args: Vec<String> = std::env::args().take(2).collect();
-    if args.len() >= 2 && args[1] == "update-to-prune-jobs-config" {
-        return update_to_prune_jobs_config();
+    let args: Vec<String> = std::env::args().take(3).collect();
+    if args.len() >= 3 && args[1] == "migrate-config" {
+        return migrate_config::handle_command(&args[2]);
     }
     let avoid_init = args.len() >= 2 && (args[1] == "bashcomplete" || args[1] == "printdoc");
 
diff --git a/src/bin/proxmox_backup_manager/migrate_config.rs b/src/bin/proxmox_backup_manager/migrate_config.rs
new file mode 100644
index 00000000..214c8d71
--- /dev/null
+++ b/src/bin/proxmox_backup_manager/migrate_config.rs
@@ -0,0 +1,98 @@
+use anyhow::{bail, Error};
+use serde::Deserialize;
+
+use pbs_api_types::{DataStoreConfig, PruneJobConfig, PruneJobOptions};
+use pbs_config::prune;
+
+/// Handle a 'migrate-config' command.
+pub fn handle_command(command: &str) -> Result<(), Error> {
+    match command {
+        "update-to-prune-jobs-config" => return update_to_prune_jobs_config(),
+        _ => bail!("invalid fixup command: {command}"),
+    }
+}
+
+/// Migrate a datastore's prune setting to a prune job.
+pub(crate) fn update_to_prune_jobs_config() -> Result<(), Error> {
+    use pbs_config::datastore;
+
+    let _prune_lock = prune::lock_config()?;
+    let _datastore_lock = datastore::lock_config()?;
+
+    let (mut data, _digest) = prune::config()?;
+    let (mut storeconfig, _digest) = datastore::config()?;
+
+    for (store, entry) in storeconfig.sections.iter_mut() {
+        let ty = &entry.0;
+
+        if ty != "datastore" {
+            continue;
+        }
+
+        let mut config = match DataStoreConfig::deserialize(&entry.1) {
+            Ok(c) => c,
+            Err(err) => {
+                eprintln!("failed to parse config of store {store}: {err}");
+                continue;
+            }
+        };
+
+        let options = PruneJobOptions {
+            keep: std::mem::take(&mut config.keep),
+            ..Default::default()
+        };
+
+        let schedule = config.prune_schedule.take();
+
+        entry.1 = serde_json::to_value(config)?;
+
+        let schedule = match schedule {
+            Some(s) => s,
+            None => {
+                if options.keeps_something() {
+                    eprintln!(
+                        "dropping prune job without schedule from datastore '{store}' in datastore.cfg"
+                    );
+                } else {
+                    eprintln!("ignoring empty prune job of datastore '{store}' in datastore.cfg");
+                }
+                continue;
+            }
+        };
+
+        let mut id = format!("storeconfig-{store}");
+        id.truncate(32);
+        if data.sections.contains_key(&id) {
+            eprintln!("skipping existing converted prune job for datastore '{store}': {id}");
+            continue;
+        }
+
+        if !options.keeps_something() {
+            eprintln!("dropping empty prune job of datastore '{store}' in datastore.cfg");
+            continue;
+        }
+
+        let prune_config = PruneJobConfig {
+            id: id.clone(),
+            store: store.clone(),
+            disable: false,
+            comment: None,
+            schedule,
+            options,
+        };
+
+        let prune_config = serde_json::to_value(prune_config)?;
+
+        data.sections
+            .insert(id, ("prune".to_string(), prune_config));
+
+        eprintln!(
+            "migrating prune job of datastore '{store}' from datastore.cfg to prune.cfg jobs"
+        );
+    }
+
+    prune::save_config(&data)?;
+    datastore::save_config(&storeconfig)?;
+
+    Ok(())
+}
diff --git a/src/bin/proxmox_backup_manager/mod.rs b/src/bin/proxmox_backup_manager/mod.rs
index 11fb6dd3..14bd729e 100644
--- a/src/bin/proxmox_backup_manager/mod.rs
+++ b/src/bin/proxmox_backup_manager/mod.rs
@@ -36,3 +36,4 @@ mod openid;
 pub use openid::*;
 mod traffic_control;
 pub use traffic_control::*;
+pub mod migrate_config;
diff --git a/src/bin/proxmox_backup_manager/prune.rs b/src/bin/proxmox_backup_manager/prune.rs
index 923eb6f5..eb06608b 100644
--- a/src/bin/proxmox_backup_manager/prune.rs
+++ b/src/bin/proxmox_backup_manager/prune.rs
@@ -1,13 +1,12 @@
 use std::collections::HashMap;
 
 use anyhow::Error;
-use serde::Deserialize;
 use serde_json::Value;
 
 use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
 use proxmox_schema::api;
 
-use pbs_api_types::{DataStoreConfig, PruneJobConfig, PruneJobOptions, JOB_ID_SCHEMA};
+use pbs_api_types::{PruneJobConfig, JOB_ID_SCHEMA};
 use pbs_config::prune;
 
 use proxmox_backup::api2;
@@ -180,87 +179,3 @@ fn get_prune_job(id: &str) -> Result<PruneJobConfig, Error> {
 
     config.lookup("prune", id)
 }
-
-pub(crate) fn update_to_prune_jobs_config() -> Result<(), Error> {
-    use pbs_config::datastore;
-
-    let _prune_lock = prune::lock_config()?;
-    let _datastore_lock = datastore::lock_config()?;
-
-    let (mut data, _digest) = prune::config()?;
-    let (mut storeconfig, _digest) = datastore::config()?;
-
-    for (store, entry) in storeconfig.sections.iter_mut() {
-        let ty = &entry.0;
-
-        if ty != "datastore" {
-            continue;
-        }
-
-        let mut config = match DataStoreConfig::deserialize(&entry.1) {
-            Ok(c) => c,
-            Err(err) => {
-                eprintln!("failed to parse config of store {store}: {err}");
-                continue;
-            }
-        };
-
-        let options = PruneJobOptions {
-            keep: std::mem::take(&mut config.keep),
-            ..Default::default()
-        };
-
-        let schedule = config.prune_schedule.take();
-
-        entry.1 = serde_json::to_value(config)?;
-
-        let schedule = match schedule {
-            Some(s) => s,
-            None => {
-                if options.keeps_something() {
-                    eprintln!(
-                        "dropping prune job without schedule from datastore '{store}' in datastore.cfg"
-                    );
-                } else {
-                    eprintln!("ignoring empty prune job of datastore '{store}' in datastore.cfg");
-                }
-                continue;
-            }
-        };
-
-        let mut id = format!("storeconfig-{store}");
-        id.truncate(32);
-        if data.sections.contains_key(&id) {
-            eprintln!("skipping existing converted prune job for datastore '{store}': {id}");
-            continue;
-        }
-
-        if !options.keeps_something() {
-            eprintln!("dropping empty prune job of datastore '{store}' in datastore.cfg");
-            continue;
-        }
-
-        let prune_config = PruneJobConfig {
-            id: id.clone(),
-            store: store.clone(),
-            disable: false,
-            comment: None,
-            schedule,
-            options,
-        };
-
-        let prune_config = serde_json::to_value(prune_config)?;
-
-        data.sections
-            .insert(id, ("prune".to_string(), prune_config));
-
-        eprintln!(
-            "migrating prune job of datastore '{store}' from datastore.cfg to prune.cfg jobs"
-        );
-    }
-
-    prune::save_config(&data)?;
-    datastore::save_config(&storeconfig)?;
-
-    Ok(())
-}
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 2/9] cli: manager: add 'migrate-config default-notification-mode' command

[Lukas Wagner]

This one migrates any datastore or tape backup job that relied on the
old default (legacy-sendmail) to an explicit setting of
legacy-sendmail. This allows us the change the default without changing
behavior for anybody.

This new command is intended to be called by d/postinst on upgrade to
the package version which introduces the new default value for
'notification-mode'.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 .../proxmox_backup_manager/migrate_config.rs  | 77 ++++++++++++++++++-
 1 file changed, 75 insertions(+), 2 deletions(-)

diff --git a/src/bin/proxmox_backup_manager/migrate_config.rs b/src/bin/proxmox_backup_manager/migrate_config.rs
index 214c8d71..9897218a 100644
--- a/src/bin/proxmox_backup_manager/migrate_config.rs
+++ b/src/bin/proxmox_backup_manager/migrate_config.rs
@@ -1,13 +1,20 @@
 use anyhow::{bail, Error};
 use serde::Deserialize;
 
-use pbs_api_types::{DataStoreConfig, PruneJobConfig, PruneJobOptions};
+use pbs_api_types::{
+    DataStoreConfig, NotificationMode, PruneJobConfig, PruneJobOptions, TapeBackupJobConfig,
+};
 use pbs_config::prune;
 
 /// Handle a 'migrate-config' command.
 pub fn handle_command(command: &str) -> Result<(), Error> {
     match command {
-        "update-to-prune-jobs-config" => return update_to_prune_jobs_config(),
+        "update-to-prune-jobs-config" => update_to_prune_jobs_config(),
+        "default-notification-mode" => {
+            migrate_tape_job_notification_mode()?;
+            migrate_datastore_notification_mode_default()?;
+            Ok(())
+        }
         _ => bail!("invalid fixup command: {command}"),
     }
 }
@@ -96,3 +103,69 @@ pub(crate) fn update_to_prune_jobs_config() -> Result<(), Error> {
 
     Ok(())
 }
+
+/// Explicitly set 'notification-mode' to 'legacy-sendmail' for any datastore which
+/// relied on the previous default of 'legacy-sendmail'.
+///
+/// This allows us to change the default to 'notification-system' without any noticeable
+/// change in behavior.
+fn migrate_datastore_notification_mode_default() -> Result<(), Error> {
+    let _lock = pbs_config::datastore::lock_config()?;
+
+    let (mut datastore_config, _digest) = pbs_config::datastore::config()?;
+
+    for (store, entry) in datastore_config.sections.iter_mut() {
+        let mut config = match DataStoreConfig::deserialize(&entry.1) {
+            Ok(c) => c,
+            Err(err) => {
+                eprintln!("failed to parse config of store {store}: {err}");
+                continue;
+            }
+        };
+
+        if config.notification_mode.is_none() {
+            config.notification_mode = Some(NotificationMode::LegacySendmail);
+            eprintln!("setting notification-mode of datastore '{store}' to 'legacy-sendmail' to presere previous default behavior.");
+        }
+
+        entry.1 = serde_json::to_value(config)?;
+    }
+
+    pbs_config::datastore::save_config(&datastore_config)?;
+
+    Ok(())
+}
+
+/// Explicitly set 'notification-mode' to 'legacy-sendmail' for any tape backup job which
+/// relied on the previous default of 'legacy-sendmail'.
+///
+/// This allows us to change the default to 'notification-system' without any noticeable
+/// change in behavior.
+fn migrate_tape_job_notification_mode() -> Result<(), Error> {
+    let _lock = pbs_config::tape_job::lock()?;
+
+    let (mut tapejob_config, _digest) = pbs_config::tape_job::config()?;
+
+    for (job_id, entry) in tapejob_config.sections.iter_mut() {
+        let mut config = match TapeBackupJobConfig::deserialize(&entry.1) {
+            Ok(c) => c,
+            Err(err) => {
+                eprintln!("failed to parse config of tape-backup job {job_id}: {err}");
+                continue;
+            }
+        };
+
+        if config.setup.notification_mode.is_none() {
+            config.setup.notification_mode = Some(NotificationMode::LegacySendmail);
+            eprintln!(
+                "setting notification-mode of tape backup job '{job_id}' to 'legacy-sendmail' to preserve previous default behavior."
+            );
+        }
+
+        entry.1 = serde_json::to_value(config)?;
+    }
+
+    pbs_config::tape_job::save_config(&tapejob_config)?;
+
+    Ok(())
+}
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 3/9] d/postinst: migrate notification mode default on update

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Please set the version guard in d/postinst to the correct version when
    applying this patch

 debian/postinst | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/debian/postinst b/debian/postinst
index 7f4c57cc..6f060810 100644
--- a/debian/postinst
+++ b/debian/postinst
@@ -40,6 +40,13 @@ case "$1" in
 			true
 		fi
 
+		# TODO: Make sure the version is correct when applying
+		if dpkg --compare-versions "$2" 'lt' '4.0.0~'; then
+			proxmox-backup-manager migrate-config default-notification-mode \
+			    || echo "Failed migrate tape-job/datastore notification mode, please check manually"
+			true
+		fi
+
 		if dpkg --compare-versions "$2" 'lt' '2.3.1~' && test -e /etc/proxmox-backup/.datastore.lck; then
 			lock_user="$(stat --format '%U' /etc/proxmox-backup/.datastore.lck)"
 			if [ "${lock_user}" != "backup" ]; then
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 4/9] ui: datastore options view: switch to new notification-mode default

[Lukas Wagner]

This default is displayed in the grid panel if the datastore config
retrieved from the API does not contain any value for notification-mode.
Since the default changed from 'legacy-sendmail' to 'notification-mode'
in the schema datatype, the defaultValue field needs to be adapted as
well.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/datastore/OptionView.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/datastore/OptionView.js b/www/datastore/OptionView.js
index adbb3b65..184353e0 100644
--- a/www/datastore/OptionView.js
+++ b/www/datastore/OptionView.js
@@ -166,7 +166,7 @@ Ext.define('PBS.Datastore.Options', {
     rows: {
         'notification-mode': {
             required: true,
-            defaultValue: 'legacy-sendmail',
+            defaultValue: 'notification-system',
             header: gettext('Notification mode'),
             renderer: function (value) {
                 if (value === 'notification-system') {
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 5/9] ui: tape backup job: move notification settings to a separate tab

[Lukas Wagner]

For consistency, use the same UI approach as for PVE's backup jobs. Tape
backup jobs now gain a new tab for all notification related settings:

  ( ) Use global notification settings
  (x) Use sendmail to send an email (legacy)
      Recipient: [              ]

'Recipient' is disabled when the first radio control is selected.

The term 'Notification System' is altogether from the UI. It is not
necessarily clear to a user that this refers to the settings in
Configuration > Notifications.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/tape/window/TapeBackupJob.js | 109 +++++++++++++++++++------------
 1 file changed, 67 insertions(+), 42 deletions(-)

diff --git a/www/tape/window/TapeBackupJob.js b/www/tape/window/TapeBackupJob.js
index c06311bf..ad03b607 100644
--- a/www/tape/window/TapeBackupJob.js
+++ b/www/tape/window/TapeBackupJob.js
@@ -46,23 +46,6 @@ Ext.define('PBS.TapeManagement.BackupJobEdit', {
         },
     },
 
-    viewModel: {
-        data: {
-            notificationMode: '__default__',
-        },
-        formulas: {
-            notificationSystemSelected: (get) => get('notificationMode') === 'notification-system',
-        },
-    },
-
-    initComponent: function () {
-        let me = this;
-        // Automatically select the new system for new jobs
-        let mode = me.isCreate ? 'notification-system' : '__default__';
-        me.getViewModel().set('notificationMode', mode);
-        me.callParent();
-    },
-
     items: {
         xtype: 'tabpanel',
         bodyPadding: 10,
@@ -126,31 +109,6 @@ Ext.define('PBS.TapeManagement.BackupJobEdit', {
                         fieldLabel: gettext('Drive'),
                         name: 'drive',
                     },
-                    {
-                        xtype: 'proxmoxKVComboBox',
-                        comboItems: [
-                            ['__default__', `${Proxmox.Utils.defaultText}  (Email)`],
-                            ['legacy-sendmail', gettext('Email (legacy)')],
-                            ['notification-system', gettext('Notification system')],
-                        ],
-                        fieldLabel: gettext('Notification mode'),
-                        name: 'notification-mode',
-                        bind: {
-                            value: '{notificationMode}',
-                        },
-                    },
-                    {
-                        xtype: 'pmxUserSelector',
-                        name: 'notify-user',
-                        fieldLabel: gettext('Notify User'),
-                        emptyText: 'root@pam',
-                        allowBlank: true,
-                        value: null,
-                        renderer: Ext.String.htmlEncode,
-                        bind: {
-                            disabled: '{notificationSystemSelected}',
-                        },
-                    },
                 ],
 
                 column2: [
@@ -260,6 +218,73 @@ Ext.define('PBS.TapeManagement.BackupJobEdit', {
                     },
                 ],
             },
+            {
+                xtype: 'inputpanel',
+                title: gettext('Notifications'),
+
+                viewModel: {
+                    data: {
+                        notificationMode: undefined,
+                    },
+                    formulas: {
+                        notificationSystemSelected: (get) =>
+                            get('notificationMode')['notification-mode'] === 'notification-system',
+                    },
+                },
+
+                onSetValues: function (values) {
+                    let me = this;
+
+                    let mode = values['notification-mode'];
+                    me.getViewModel().set('notificationMode', { 'notification-mode': mode });
+
+                    return values;
+                },
+
+                items: [
+                    {
+                        xtype: 'radiogroup',
+                        height: '15px',
+                        layout: {
+                            type: 'vbox',
+                        },
+                        bind: {
+                            value: '{notificationMode}',
+                        },
+                        items: [
+                            {
+                                xtype: 'radiofield',
+                                name: 'notification-mode',
+                                inputValue: 'notification-system',
+                                boxLabel: gettext('Use global notification settings'),
+                                cbind: {
+                                    checked: '{isCreate}',
+                                },
+                            },
+                            {
+                                xtype: 'radiofield',
+                                name: 'notification-mode',
+                                inputValue: 'legacy-sendmail',
+                                boxLabel: gettext('Use sendmail to send an email (legacy)'),
+                            },
+                        ],
+                    },
+                    {
+                        xtype: 'pmxUserSelector',
+                        name: 'notify-user',
+                        fieldLabel: gettext('Recipient'),
+                        emptyText: 'root@pam',
+                        allowBlank: false,
+                        value: null,
+                        renderer: Ext.String.htmlEncode,
+                        padding: '0 0 0 50',
+                        disabled: true,
+                        bind: {
+                            disabled: '{notificationSystemSelected}',
+                        },
+                    },
+                ],
+            },
         ],
     },
 });
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 6/9] ui: one-shot tape backup: use same wording as tape-backup jobs

[Lukas Wagner]

Change the dialog of one-shot tape-backups in such a way that they use
the same jargon as scheduled tape backup jobs.

The width of the dialog is increased by 150px to 750px so that the
slightly larger amount of text fits nicely.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/tape/window/TapeBackup.js | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/www/tape/window/TapeBackup.js b/www/tape/window/TapeBackup.js
index b075c8c4..7e7fa8f3 100644
--- a/www/tape/window/TapeBackup.js
+++ b/www/tape/window/TapeBackup.js
@@ -8,6 +8,8 @@ Ext.define('PBS.TapeManagement.TapeBackupWindow', {
     showTaskViewer: true,
     isCreate: true,
 
+    width: 750,
+
     viewModel: {
         data: {
             notificationMode: 'notification-system',
@@ -95,10 +97,10 @@ Ext.define('PBS.TapeManagement.TapeBackupWindow', {
                     xtype: 'proxmoxKVComboBox',
                     labelWidth: 150,
                     comboItems: [
-                        ['legacy-sendmail', gettext('Email (legacy)')],
-                        ['notification-system', gettext('Notification system')],
+                        ['notification-system', gettext('Use global settings')],
+                        ['legacy-sendmail', gettext('Use sendmail (legacy)')],
                     ],
-                    fieldLabel: gettext('Notification mode'),
+                    fieldLabel: gettext('Notification'),
                     name: 'notification-mode',
                     bind: {
                         value: '{notificationMode}',
@@ -114,7 +116,7 @@ Ext.define('PBS.TapeManagement.TapeBackupWindow', {
                     allowBlank: true,
                     skipEmptyText: true,
                     bind: {
-                        disabled: '{notificationSystemSelected}',
+                        hidden: '{notificationSystemSelected}',
                     },
                     renderer: Ext.String.htmlEncode,
                 },
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 7/9] ui: datastore options: notifications: use same jargon as tape-jobs and PVE

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/datastore/OptionView.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/www/datastore/OptionView.js b/www/datastore/OptionView.js
index 184353e0..5a10a65d 100644
--- a/www/datastore/OptionView.js
+++ b/www/datastore/OptionView.js
@@ -167,12 +167,12 @@ Ext.define('PBS.Datastore.Options', {
         'notification-mode': {
             required: true,
             defaultValue: 'notification-system',
-            header: gettext('Notification mode'),
+            header: gettext('Notifications'),
             renderer: function (value) {
                 if (value === 'notification-system') {
-                    return gettext('Notification system');
+                    return gettext('Use global settings');
                 } else {
-                    return gettext('Email (legacy)');
+                    return gettext('Use sendmail to send an email (legacy)');
                 }
             },
             editor: {
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 8/9] ui: datastore options: drop notify and notify-user rows

[Lukas Wagner]

Even if the notification mode is set to 'notification-system', the
datastore options grid still shows the keys for 'Notify' and 'Notify
User', which have no effect in this mode:

        Notification:      [Use global notification settings]
        Notify:            [Prune: Default(always), etc...]
        Notify User:       [root@pam]

This is quite confusing.

Unfortunately, it seems be quite hard to dynamically disable/hide rows
in the grid panel used in this view.

For that reason these rows are removed completely for now. The options
are still visible when opening the edit window for the 'Notification'
row.

While this slightly worsens UX in some cases (information is hidden), it
improves clarity by reducing ambiguity, which is also a vital part of
good UX.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/datastore/OptionView.js | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/www/datastore/OptionView.js b/www/datastore/OptionView.js
index 5a10a65d..913bdfc7 100644
--- a/www/datastore/OptionView.js
+++ b/www/datastore/OptionView.js
@@ -179,31 +179,6 @@ Ext.define('PBS.Datastore.Options', {
                 xtype: 'pbsNotifyOptionEdit',
             },
         },
-        notify: {
-            required: true,
-            header: gettext('Notify'),
-            renderer: (value) => {
-                let notify = PBS.Utils.parsePropertyString(value);
-                let res = [];
-                for (const k of ['Verify', 'Sync', 'GC', 'Prune']) {
-                    let fallback = k === 'Prune' ? 'Error' : 'Always';
-                    let v = Ext.String.capitalize(notify[k.toLowerCase()]) || fallback;
-                    res.push(`${k}=${v}`);
-                }
-                return res.join(', ');
-            },
-            editor: {
-                xtype: 'pbsNotifyOptionEdit',
-            },
-        },
-        'notify-user': {
-            required: true,
-            defaultValue: 'root@pam',
-            header: gettext('Notify User'),
-            editor: {
-                xtype: 'pbsNotifyOptionEdit',
-            },
-        },
         'verify-new': {
             required: true,
             header: gettext('Verify New Snapshots'),
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 9/9] ui: datastore options: notification: use radio controls to select mode

[Lukas Wagner]

This makes it consistent with tape backup job options and PVE's backup
jobs. It also visualizes the dependency of 'notify' and 'notify-user'
onto 'notification-mode'.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/window/NotifyOptions.js | 43 ++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/www/window/NotifyOptions.js b/www/window/NotifyOptions.js
index 12a2e561..fc43de65 100644
--- a/www/window/NotifyOptions.js
+++ b/www/window/NotifyOptions.js
@@ -49,7 +49,8 @@ Ext.define('PBS.window.NotifyOptions', {
             notificationMode: '__default__',
         },
         formulas: {
-            notificationSystemSelected: (get) => get('notificationMode') === 'notification-system',
+            notificationSystemSelected: (get) =>
+                get('notificationMode')['notification-mode'] === 'notification-system',
         },
     },
 
@@ -74,23 +75,36 @@ Ext.define('PBS.window.NotifyOptions', {
         },
         items: [
             {
-                xtype: 'proxmoxKVComboBox',
-                comboItems: [
-                    ['__default__', `${Proxmox.Utils.defaultText}  (Email)`],
-                    ['legacy-sendmail', gettext('Email (legacy)')],
-                    ['notification-system', gettext('Notification system')],
-                ],
-                deleteEmpty: true,
-                fieldLabel: gettext('Notification mode'),
-                name: 'notification-mode',
+                xtype: 'radiogroup',
+                height: '15px',
+                layout: {
+                    type: 'vbox',
+                },
                 bind: {
                     value: '{notificationMode}',
                 },
+                items: [
+                    {
+                        xtype: 'radiofield',
+                        name: 'notification-mode',
+                        inputValue: 'notification-system',
+                        boxLabel: gettext('Use global notification settings'),
+                        cbind: {
+                            checked: '{isCreate}',
+                        },
+                    },
+                    {
+                        xtype: 'radiofield',
+                        name: 'notification-mode',
+                        inputValue: 'legacy-sendmail',
+                        boxLabel: gettext('Use sendmail to send an email (legacy)'),
+                    },
+                ],
             },
             {
                 xtype: 'pmxUserSelector',
                 name: 'notify-user',
-                fieldLabel: gettext('Notify User'),
+                fieldLabel: gettext('Recipient'),
                 emptyText: 'root@pam',
                 value: null,
                 allowBlank: true,
@@ -99,6 +113,7 @@ Ext.define('PBS.window.NotifyOptions', {
                 bind: {
                     disabled: '{notificationSystemSelected}',
                 },
+                padding: '0 0 0 50',
             },
             {
                 xtype: 'pbsNotifyType',
@@ -109,6 +124,7 @@ Ext.define('PBS.window.NotifyOptions', {
                 bind: {
                     disabled: '{notificationSystemSelected}',
                 },
+                padding: '0 0 0 50',
             },
             {
                 xtype: 'pbsNotifyType',
@@ -119,6 +135,7 @@ Ext.define('PBS.window.NotifyOptions', {
                 bind: {
                     disabled: '{notificationSystemSelected}',
                 },
+                padding: '0 0 0 50',
             },
             {
                 xtype: 'pbsNotifyErrorDefaultType',
@@ -129,6 +146,7 @@ Ext.define('PBS.window.NotifyOptions', {
                 bind: {
                     disabled: '{notificationSystemSelected}',
                 },
+                padding: '0 0 0 50',
             },
             {
                 xtype: 'pbsNotifyType',
@@ -139,6 +157,7 @@ Ext.define('PBS.window.NotifyOptions', {
                 bind: {
                     disabled: '{notificationSystemSelected}',
                 },
+                padding: '0 0 0 50',
             },
         ],
     },
@@ -151,7 +170,7 @@ Ext.define('PBS.window.NotifyOptions', {
             'verify-new': values['verify-new'],
             'notification-mode': values['notification-mode']
                 ? values['notification-mode']
-                : '__default__',
+                : 'notification-system',
         };
 
         let notify = {};
-- 
2.39.5



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

[pbs-devel] applied: [PATCH proxmox 1/1] pbs-api-types: change default notification mode to 'notification-system'

[Thomas Lamprecht]

On Mon, 23 Jun 2025 16:13:06 +0200, Lukas Wagner wrote:
> The former default was chose avoid breaking changes during a minor
> release, but with the upcoming major release we can change the default.
> 
> The rationale for the new default is:
>   - 'notification-system' was already used as a default for any datastore
>     created by the GUI
>   - consistency, since on PVE, the default is 'auto' (which does not exist
>     on PBS, but is equivalent to 'notification-system' unless an email
>     address has been added as well)
>   - users are gently nudged to use the new notification stack
> 
> [...]

Applied this one for now, thanks!

[1/1] pbs-api-types: change default notification mode to 'notification-system'
      commit: 982eca008a1a85abdd9bc6f531cc1a4ac4e625ca


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

Re: [pbs-devel] [PATCH proxmox-backup 1/9] cli: manager: move update-to-prune-jobs command to new migrate-config sub-command

[Thomas Lamprecht]

Am 23.06.25 um 16:13 schrieb Lukas Wagner:
> The new subcommand is introduced so that we have a common name space for
> any config migration tasks which are triggered by d/postinst (or potentially
> by hand).
> 
> No functional changes.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> 
> Notes:
>     I guess at this point we could also drop this code entirely?
> 
>  debian/postinst                               |  2 +-
>  src/bin/proxmox-backup-manager.rs             |  6 +-
>  .../proxmox_backup_manager/migrate_config.rs  | 98 +++++++++++++++++++
>  src/bin/proxmox_backup_manager/mod.rs         |  1 +
>  src/bin/proxmox_backup_manager/prune.rs       | 87 +---------------
>  5 files changed, 104 insertions(+), 90 deletions(-)
>  create mode 100644 src/bin/proxmox_backup_manager/migrate_config.rs
> 
> diff --git a/debian/postinst b/debian/postinst
> index f38a8c66..7f4c57cc 100644
> --- a/debian/postinst
> +++ b/debian/postinst
> @@ -35,7 +35,7 @@ case "$1" in
>  
>  		if dpkg --compare-versions "$2" 'lt' '2.2.2~'; then

FWIW, this can actually be dropped for PBS 4, as we do not support upgrading from 2,
so this cannot be ever evaluate to true anymore there.


>  			echo "moving prune schedule from datacenter config to new prune job config"
> -			proxmox-backup-manager update-to-prune-jobs-config \
> +			proxmox-backup-manager migrate-config update-to-prune-jobs-config \
>  			    || echo "Failed to move prune jobs, please check manually"
>  			true
>  		fi


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

Re: [pbs-devel] [PATCH proxmox-backup 1/9] cli: manager: move update-to-prune-jobs command to new migrate-config sub-command

[Thomas Lamprecht]

Am 16.07.25 um 01:09 schrieb Thomas Lamprecht:
> Am 23.06.25 um 16:13 schrieb Lukas Wagner:
>> The new subcommand is introduced so that we have a common name space for
>> any config migration tasks which are triggered by d/postinst (or potentially
>> by hand).
>>
>> No functional changes.
>>
>> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>> ---
>>
>> Notes:
>>     I guess at this point we could also drop this code entirely?

I read this only now, sorry, but the answer is indeed yes.


>> @@ -35,7 +35,7 @@ case "$1" in
>>  
>>  		if dpkg --compare-versions "$2" 'lt' '2.2.2~'; then
> 
> FWIW, this can actually be dropped for PBS 4, as we do not support upgrading from 2,
> so this cannot be ever evaluate to true anymore there.


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

Re: [pbs-devel] [PATCH proxmox{,-backup} 00/10] change of default notification-mode; UX improvements

[Thomas Lamprecht]

On Mon, 23 Jun 2025 16:13:05 +0200, Lukas Wagner wrote:
> This is intended for the PBS 4.0 release (trixie based).
> 
> This patch series changes the default notification mode for PBS to
> 'notification-system' from the former default of 'legacy-sendmail'.
> 
> The rationale for the new default is:
>   - 'notification-system' was already used as a default for any datastore
>     created by the GUI
>   - consistency, since on PVE, the default is 'auto' (which does not exist
>     on PBS, but is equivalent to 'notification-system' unless an email
>     address has been added as well)
>   - users are gently nudged to use the new notification stack
> 
> [...]

With a trivial merge conflict in the context of the first patch resolved:

Applied, thanks!

As replied, and you correctly noted already, the old migrate-config command for
prune jobs can now go, but IMO still does not hurt to have a dedicated command
group, and it also doesn't hurt if we keep it a bit longer, we can now remove
it at any time after all.

[01/9] cli: manager: move update-to-prune-jobs command to new migrate-config sub-command
       commit: 9526aee10a0c1b86de70a5d775a7f7cac048ced0
[02/9] cli: manager: add 'migrate-config default-notification-mode' command
       commit: 243e6a57841839e4f00c15e3997bb7a1cf7d86d3
[PATCH 3/9] d/postinst: migrate notification mode default on update
       commit: 93390b67df34db664e174f706ac71e02a3d477d5
[04/9] ui: datastore options view: switch to new notification-mode default
       commit: 20053ec2160f861cade003cb72cd66396c84985a
[05/9] ui: tape backup job: move notification settings to a separate tab
       commit: 025afdb9fee735fd23e93b6889298c44dd8e3909
[06/9] ui: one-shot tape backup: use same wording as tape-backup jobs
       commit: 82963b7d4b93992dce3c92ae8ae937301a773515
[07/9] ui: datastore options: notifications: use same jargon as tape-jobs and PVE
       commit: 0471464f4e9a7857b3d5042014ef3b47ad3d979f
[08/9] ui: datastore options: drop notify and notify-user rows
       commit: 00290872a3787f31578247902960d82ea72b69ad
[09/9] ui: datastore options: notification: use radio controls to select mode
       commit: b289d294c879376853586221b984290928f0f0f1


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