On 2025-07-08 20:07, Michael Köppl wrote:
> 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.
>
The parameter to the gettext function needs to be a static string, otherwise
xgettext[1,2] cannot
extract it to generate the .pot file. While the xgettext tool has some basic
understanding of the
syntax of supported languages to parse these strings, it does not evaluate or
execute any code.
That's also the reason why we have to do a
Ext.String.format(gettext("... {0}"), var)
instead of a
gettext(`.... ${var}`)
Also, I'm not the biggest fan of the ternary operator, I'm not sure if your
suggestion is any easier
to read and comprehend. But that might just be me :)
[1] https://man7.org/linux/man-pages/man1/xgettext.1.html
[2]
https://git.proxmox.com/?p=proxmox-i18n.git;a=blob;f=Makefile;h=34859f5cfa776111b991da23e922cc5eb83cf306;hb=HEAD#l140
--
- Lukas
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
