Just added a suggestion inline
On 6/24/25 13:28, Lukas Wagner wrote:
> The backup job details view was never updated after the overhaul of the
> notification system. In this commit we remove the left-over
> notification-policy/target handling and change the view so that we
> display the current configuration based on notification-mode, mailto and
> mailnotification.
>
> Signed-off-by: Lukas Wagner <l.wag...@proxmox.com>
> ---
>
> Notes:
> Changes since v1:
> - Rebased onto latest master (PVE 9)
>
> www/manager6/dc/BackupJobDetail.js | 37 +++++++++++++++++-------------
> 1 file changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/www/manager6/dc/BackupJobDetail.js
> b/www/manager6/dc/BackupJobDetail.js
> index 58cb7bef..8d10a0da 100644
> --- a/www/manager6/dc/BackupJobDetail.js
> +++ b/www/manager6/dc/BackupJobDetail.js
> @@ -206,28 +206,33 @@ Ext.define('PVE.dc.BackupInfo', {
> column2: [
> {
> xtype: 'displayfield',
> - name: 'notification-policy',
> + name: 'notification-mode',
> fieldLabel: gettext('Notification'),
> renderer: function (value) {
> + value = value ?? 'auto';
> let record = this.up('pveBackupInfo')?.record;
> + let mailto = record?.mailto;
> + let mailnotification = record?.mailnotification ?? 'always';
>
> - // Fall back to old value, in case this option is not
> migrated yet.
> - let policy = value || record?.mailnotification || 'always';
> + if ((value === 'auto' && mailto === undefined) || value ===
> 'notification-system') {
> + return gettext('Use global notification settings');
> + } else {
> + if (mailto === undefined) {
> + mailto = '-';
> + }
>
> - let when = gettext('Always');
> - if (policy === 'failure') {
> - when = gettext('On failure only');
> - } else if (policy === 'never') {
> - when = gettext('Never');
> + if (mailnotification === 'always') {
> + return Ext.String.format(
> + gettext('Always use sendmail to send an email
> to: {0}'),
> + mailto,
> + );
> + } else {
> + return Ext.String.format(
> + gettext('On failure, use sendmail to send an
> email to: {0}'),
> + mailto,
> + );
> + }
Could maybe shorten this to
return Ext.String.format(
gettext((mailnotification === 'always' ? 'Always' : 'On
failure,') + '
use sendmail to send an email to: {0}'),
mailto,
);
Admittedly, opinions might differ regarding readability.
> }
> -
> - // Notification-target takes precedence
> - let target =
> - record?.['notification-target'] ||
> - record?.mailto ||
> - gettext('No target configured');
> -
> - return `${when} (${target})`;
> },
> },
> {
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
