[pve-devel] [PATCH many v8 00/13] notifications: notification metadata matching improvements

[pve-devel] [PATCH many v8 00/13] notifications: notification metadata matching improvements

[Lukas Wagner]

This patch series attempts to improve the user experience when creating
notification matchers.

Some of the noteworthy changes:
  - Allow setting a custom backup job ID, similar how we handle it for
  sync/prune jobs in PBS (to allow recognizable names used in matchers)
  - New metadata fields:
    - job-id: Job ID for backup-jobs or replication-jobs
  - Add an API that enumerates known notification metadata fields/values
  - Suggest known fields/values in match rule window
  - Some code clean up for match rule edit window
  - Extended the 'exact' match-field mode - it now allows setting multiple
    allowed values, separated via ',':
      e.g. `match-field exact:type=replication,fencing
    Originally, I created a separate 'list' match type for this, but
    since the semantics for a list with one value and 'exact' mode
    are identical, I decided to just extend 'exact'.
    This is a safe change since there are are no values where a ','
    makes any sense (config IDs, hostnames)

NOTE: Might need a versionened break, since the widget-toolkit-patches
depend on new APIs provided by pve-manager. If the API is not present,
creating matchers with 'match-field' does not work (cannot load lists
of known values/fields)

Inter-Dependencies:
  - the widget-toolkit dep in pve-manager needs to be bumped
    to at least 4.1.4
    (we need "utils: add mechanism to add and override translatable notification event
    descriptions in the product specific UIs", otherwise the UI breaks
    with the pve-manager patches applied) --> already included a patch for
    this
  - widget-toolkit relies on a new API endpoint provided by pve-manager:
    --> we require a versioned break in widget-toolkit on pve-manager
  - pve-manager needs bumped pve-guest-common (thx @Fabian)

Changelog:
  - v8: incorporate feedback from @Fabian, thx a lot!
    - Made 'job-id' API param usable by root@pam only - this should prevent
      abuse by spoofing job-id, potentially bothering other users with bogus
      notifications.
    - Don't set 'job-id' when starting a backup job via 'Run now' in the UI
    - Add a note to the docs explaining when job-id is set and when not.
    - Drop already applied patches
  - v7: incorporated some more feedback from @Fiona, thx!
    - Fixed error when switching from 'exact' to 'regex' if the text field
      was empty
    - rebased to latest master
    - 'backport' doc improvements from PBS
    - bumped widget-toolkit dep
  - v6: incorporate feedback from @Fiona, thx!
    - rename 'id' -> 'job-id' in VZDump API handler
    - consolidate 'replication-job'/'backup-job' to 'job-id'
    - Move 'job-id' setting to advanced tab in backup job edit.
    - Don't use 'internal' flag to mark translatable fields, since
      the only field where that's necessary is 'type' for now - so
      just add a hardcoded check
  - v5:
    - Rebased onto latest master, resolving some small conflict
  - v4:
    - widget-toolkit: break out changes for the utils module so that they
      can be applied ahead of time to ease dep bumping
    - don't show Job IDs in the backup/replication job columns
  - v3:
    - Drop already applied patches for `proxmox`
    - Rebase onto latest master - minor conflict resolution was needed
  - v2:
    - include 'type' metadata field for forwarded mails
      --> otherwise it's not possible to match them
    - include Maximilliano's T-b trailer in UI patches

pve-guest-common:

Lukas Wagner (1):
  vzdump: common: allow 'job-id' as a parameter without being in schema

 src/PVE/VZDump/Common.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


pve-manager:

Lukas Wagner (5):
  api: jobs: vzdump: pass job 'job-id' parameter
  ui: dc: backup: allow to set custom job id in  advanced settings
  api: notification: add API for getting known metadata fields/values
  ui: utils: add overrides for translatable notification fields/values
  d/control: bump proxmox-widget-toolkit dependency to 4.1.4

 PVE/API2/Backup.pm                          |   2 +-
 PVE/API2/Cluster/Notifications.pm           | 139 ++++++++++++++++++++
 PVE/API2/VZDump.pm                          |  13 +-
 PVE/Jobs/VZDump.pm                          |   4 +-
 PVE/VZDump.pm                               |   6 +-
 debian/control                              |   2 +-
 www/manager6/Utils.js                       |  11 ++
 www/manager6/dc/Backup.js                   |   4 -
 www/manager6/panel/BackupAdvancedOptions.js |  23 ++++
 9 files changed, 192 insertions(+), 12 deletions(-)


proxmox-widget-toolkit:

Lukas Wagner (4):
  notification: matcher: match-field: show known fields/values
  notification: matcher: move match-field formulas to local viewModel
  notification: matcher: move match-calendar fields to panel
  notification: matcher: move match-severity fields to panel

 src/data/model/NotificationConfig.js  |  12 +
 src/window/NotificationMatcherEdit.js | 613 ++++++++++++++++++--------
 2 files changed, 441 insertions(+), 184 deletions(-)


pve-docs:

Lukas Wagner (3):
  notifications: describe new notification metadata fields
  notifications: match-field 'exact'-mode can now match multiple values
  notifications: add note regarding when 'job-id' is set for backups

 notifications.adoc | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)


Summary over all repositories:
  13 files changed, 655 insertions(+), 218 deletions(-)

-- 
Generated by git-murpp 0.7.1


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

[pve-devel] [PATCH pve-guest-common v8 01/13] vzdump: common: allow 'job-id' as a parameter without being in schema

[Lukas Wagner]

'job-id' is passed when a backup as started as a job and will be
passed to the notification system as matchable metadata. It it
can be considered 'internal'.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 src/PVE/VZDump/Common.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index 1996c5b..2532b42 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -503,7 +503,7 @@ sub command_line {
 
     foreach my $p (keys %$param) {
 	next if $p eq 'id' || $p eq 'vmid' || $p eq 'starttime' ||
-	        $p eq 'dow' || $p eq 'stdout' || $p eq 'enabled';
+	        $p eq 'dow' || $p eq 'stdout' || $p eq 'enabled' || $p eq 'job-id';
 	my $v = $param->{$p};
 	my $pd = $confdesc->{$p} || die "no such vzdump option '$p'\n";
 	if ($p eq 'exclude-path') {
-- 
2.39.2



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

[pve-devel] [PATCH manager v8 02/13] api: jobs: vzdump: pass job 'job-id' parameter

[Lukas Wagner]

This allows us to access us the backup job id in the send_notification
function, where we can set it as metadata for the notification.
The 'job-id' parameter can only be used by 'root@pam' to prevent
abuse. This has the side effect that manually triggered backup jobs
cannot have the 'job-id' parameter at the moment. To mitigate that,
manually triggered backup jobs could be changed so that they
are not performed by a direct API call by the UI, but by requesting
pvescheduler to execute the job in the near future (similar to how
manually triggered replication jobs work).

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/API2/Backup.pm |  2 +-
 PVE/API2/VZDump.pm | 13 +++++++++++--
 PVE/Jobs/VZDump.pm |  4 +++-
 PVE/VZDump.pm      |  6 +++---
 4 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
index 88140323..48598b8f 100644
--- a/PVE/API2/Backup.pm
+++ b/PVE/API2/Backup.pm
@@ -45,7 +45,7 @@ sub assert_param_permission_common {
     my ($rpcenv, $user, $param, $is_delete) = @_;
     return if $user eq 'root@pam'; # always OK
 
-    for my $key (qw(tmpdir dumpdir script)) {
+    for my $key (qw(tmpdir dumpdir script job-id)) {
 	raise_param_exc({ $key => "Only root may set this option."}) if exists $param->{$key};
     }
 
diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 7f92e7ec..15c9b0dc 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -42,8 +42,8 @@ __PACKAGE__->register_method ({
     permissions => {
 	description => "The user needs 'VM.Backup' permissions on any VM, and "
 	    ."'Datastore.AllocateSpace' on the backup storage (and fleecing storage when fleecing "
-	    ."is used). The 'tmpdir', 'dumpdir' and 'script' parameters are restricted to the "
-	    ."'root\@pam' user. The 'maxfiles' and 'prune-backups' settings require "
+	    ."is used). The 'tmpdir', 'dumpdir', 'script' and 'job-id' parameters are restricted "
+	    ."to the 'root\@pam' user. The 'maxfiles' and 'prune-backups' settings require "
 	    ."'Datastore.Allocate' on the backup storage. The 'bwlimit', 'performance' and "
 	    ."'ionice' parameters require 'Sys.Modify' on '/'.",
 	user => 'all',
@@ -53,6 +53,15 @@ __PACKAGE__->register_method ({
     parameters => {
 	additionalProperties => 0,
 	properties => PVE::VZDump::Common::json_config_properties({
+	    'job-id' => {
+		description => "The ID of the backup job. If set, the 'backup-job' metadata field"
+		    . " of the backup notification will be set to this value. Only root\@pam"
+		    . " can set this parameter.",
+		type => 'string',
+		format => 'pve-configid',
+		maxLength => 256,
+		optional => 1,
+	    },
 	    stdout => {
 		type => 'boolean',
 		description => "Write tar to stdout, not to a file.",
diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
index b8e57945..2dad3f55 100644
--- a/PVE/Jobs/VZDump.pm
+++ b/PVE/Jobs/VZDump.pm
@@ -12,7 +12,7 @@ use PVE::API2::VZDump;
 use base qw(PVE::VZDump::JobBase);
 
 sub run {
-    my ($class, $conf) = @_;
+    my ($class, $conf, $job_id) = @_;
 
     my $props = $class->properties();
     # remove all non vzdump related options
@@ -20,6 +20,8 @@ sub run {
 	delete $conf->{$opt} if !defined($props->{$opt});
     }
 
+    $conf->{'job-id'} = $job_id;
+
     # Required as string parameters # FIXME why?! we could just check ref()
     for my $key (keys $PVE::VZDump::Common::PROPERTY_STRINGS->%*) {
 	if ($conf->{$key} && ref($conf->{$key}) eq 'HASH') {
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 8dbcc4a9..f1a6b220 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -483,6 +483,7 @@ sub send_notification {
     my ($self, $tasklist, $total_time, $err, $detail_pre, $detail_post) = @_;
 
     my $opts = $self->{opts};
+    my $job_id = $opts->{'job-id'};
     my $mailto = $opts->{mailto};
     my $cmdline = $self->{cmdline};
     my $policy = $opts->{mailnotification} // 'always';
@@ -528,13 +529,12 @@ sub send_notification {
     };
 
     my $fields = {
-	# TODO: There is no straight-forward way yet to get the
-	# backup job id here... (I think pvescheduler would need
-	# to pass that to the vzdump call?)
 	type => "vzdump",
 	# Hostname (without domain part)
 	hostname => PVE::INotify::nodename(),
     };
+    # Add backup-job metadata field in case this is a backup job.
+    $fields->{'job-id'} = $job_id if $job_id;
 
     my $severity = $failed ? "error" : "info";
     my $email_configured = $mailto && scalar(@$mailto);
-- 
2.39.2



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

[pve-devel] [PATCH manager v8 03/13] ui: dc: backup: allow to set custom job id in advanced settings

[Lukas Wagner]

This might be useful if somebody wants to match on the new
'backup-job' field in a notification match rule.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/manager6/dc/Backup.js                   |  4 ----
 www/manager6/panel/BackupAdvancedOptions.js | 23 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 4ba80b31..381402ca 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -45,10 +45,6 @@ Ext.define('PVE.dc.BackupEdit', {
 		Proxmox.Utils.assemble_field_data(values, { 'delete': 'notification-target' });
 	    }
 
-	    if (!values.id && isCreate) {
-		values.id = 'backup-' + Ext.data.identifier.Uuid.Global.generate().slice(0, 13);
-	    }
-
 	    let selMode = values.selMode;
 	    delete values.selMode;
 
diff --git a/www/manager6/panel/BackupAdvancedOptions.js b/www/manager6/panel/BackupAdvancedOptions.js
index 7dd19f96..acb2fbd0 100644
--- a/www/manager6/panel/BackupAdvancedOptions.js
+++ b/www/manager6/panel/BackupAdvancedOptions.js
@@ -37,6 +37,10 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
 	    return {};
 	}
 
+	if (!formValues.id && me.isCreate) {
+	    formValues.id = 'backup-' + Ext.data.identifier.Uuid.Global.generate().slice(0, 13);
+	}
+
 	let options = {};
 
 	if (!me.isCreate) {
@@ -108,6 +112,25 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
     },
 
     items: [
+	{
+	    xtype: 'pveTwoColumnContainer',
+	    startColumn: {
+		xtype: 'pmxDisplayEditField',
+		vtype: 'ConfigId',
+		fieldLabel: gettext('Job ID'),
+		emptyText: gettext('Autogenerate'),
+		name: 'id',
+		allowBlank: true,
+		cbind: {
+		    editable: '{isCreate}',
+		},
+	    },
+	    endFlex: 2,
+	    endColumn: {
+		xtype: 'displayfield',
+		value: gettext('Can be used in notification matchers to match this job.'),
+	    },
+	},
 	{
 	    xtype: 'pveTwoColumnContainer',
 	    startColumn: {
-- 
2.39.2



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

[pve-devel] [PATCH manager v8 04/13] api: notification: add API for getting known metadata fields/values

[Lukas Wagner]

This new API route returns known notification metadata fields and
a list of known possible values. This will be used by the UI to
provide suggestions when adding/modifying match rules.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/API2/Cluster/Notifications.pm | 139 ++++++++++++++++++++++++++++++
 1 file changed, 139 insertions(+)

diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index 68fdda2a..2b202c28 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -79,12 +79,151 @@ __PACKAGE__->register_method ({
 	    { name => 'endpoints' },
 	    { name => 'matchers' },
 	    { name => 'targets' },
+	    { name => 'matcher-fields' },
+	    { name => 'matcher-field-values' },
 	];
 
 	return $result;
     }
 });
 
+__PACKAGE__->register_method ({
+    name => 'get_matcher_fields',
+    path => 'matcher-fields',
+    method => 'GET',
+    description => 'Returns known notification metadata fields',
+    permissions => {
+	check => ['or',
+	    ['perm', '/mapping/notifications', ['Mapping.Modify']],
+	    ['perm', '/mapping/notifications', ['Mapping.Audit']],
+	],
+    },
+    protected => 0,
+    parameters => {
+	additionalProperties => 0,
+	properties => {},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => {
+		name => {
+		    description => 'Name of the field.',
+		    type => 'string',
+		},
+	    },
+	},
+	links => [ { rel => 'child', href => '{name}' } ],
+    },
+    code => sub {
+	# TODO: Adapt this API handler once we have a 'notification registry'
+
+	my $result = [
+	    { name => 'type' },
+	    { name => 'hostname' },
+	    { name => 'job-id' },
+	];
+
+	return $result;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'get_matcher_field_values',
+    path => 'matcher-field-values',
+    method => 'GET',
+    description => 'Returns known notification metadata fields and their known values',
+    permissions => {
+	check => ['or',
+	    ['perm', '/mapping/notifications', ['Mapping.Modify']],
+	    ['perm', '/mapping/notifications', ['Mapping.Audit']],
+	],
+    },
+    protected => 1,
+    parameters => {
+	additionalProperties => 0,
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => {
+		'value' => {
+		    description => 'Notification metadata value known by the system.',
+		    type => 'string'
+		},
+		'comment' => {
+		    description => 'Additional comment for this value.',
+		    type => 'string',
+		    optional => 1,
+		},
+		'field' => {
+		    description => 'Field this value belongs to.',
+		    type => 'string',
+		},
+	    },
+	},
+    },
+    code => sub {
+	# TODO: Adapt this API handler once we have a 'notification registry'
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $user = $rpcenv->get_user();
+
+	my $values = [
+	    {
+		value => 'package-updates',
+		field => 'type',
+	    },
+	    {
+		value => 'fencing',
+		field => 'type',
+	    },
+	    {
+		value => 'replication',
+		field => 'type',
+	    },
+	    {
+		value => 'vzdump',
+		field => 'type',
+	    },
+	    {
+		value => 'system-mail',
+		field => 'type',
+	    },
+	];
+
+	# Here we need a manual permission check.
+	if ($rpcenv->check($user, "/", ["Sys.Audit"], 1)) {
+	    for my $backup_job (@{PVE::API2::Backup->index({})}) {
+		push @$values, {
+		    value => $backup_job->{id},
+		    comment => $backup_job->{comment},
+		    field => 'job-id'
+		};
+	    }
+	}
+	# The API call returns only returns jobs for which the user
+	# has adequate permissions.
+	for my $sync_job (@{PVE::API2::ReplicationConfig->index({})}) {
+	    push @$values, {
+		value => $sync_job->{id},
+		comment => $sync_job->{comment},
+		field => 'job-id'
+	    };
+	}
+
+	for my $node (@{PVE::Cluster::get_nodelist()}) {
+	    push @$values, {
+		value => $node,
+		field => 'hostname',
+	    }
+	}
+
+	return $values;
+    }
+});
+
 __PACKAGE__->register_method ({
     name => 'endpoints_index',
     path => 'endpoints',
-- 
2.39.2



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

[pve-devel] [PATCH manager v8 05/13] ui: utils: add overrides for translatable notification fields/values

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/manager6/Utils.js | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index f5608944..5b9d86ca 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -2059,6 +2059,17 @@ Ext.define('PVE.Utils', {
 	    zfscreate: [gettext('ZFS Storage'), gettext('Create')],
 	    zfsremove: ['ZFS Pool', gettext('Remove')],
 	});
+
+	Proxmox.Utils.overrideNotificationFieldName({
+	    'job-id': gettext('Job ID'),
+	});
+
+	Proxmox.Utils.overrideNotificationFieldValue({
+	    'package-updates': gettext('Package updates are available'),
+	    'vzdump': gettext('Backup notifications'),
+	    'replication': gettext('Replication job notifications'),
+	    'fencing': gettext('Node fencing notifications'),
+	});
     },
 
 });
-- 
2.39.2



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

[pve-devel] [PATCH manager v8 06/13] d/control: bump proxmox-widget-toolkit dependency to 4.1.4

[Lukas Wagner]

We need
  "utils: add mechanism to add and override translatable notification
  event descriptions in the product specific UIs"
otherwise there is an error in the browser console.

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

diff --git a/debian/control b/debian/control
index d4c254d4..bc9c7218 100644
--- a/debian/control
+++ b/debian/control
@@ -20,7 +20,7 @@ Build-Depends: debhelper-compat (= 13),
                libtemplate-perl,
                libtest-mockmodule-perl,
                lintian,
-               proxmox-widget-toolkit (>= 4.0.7),
+               proxmox-widget-toolkit (>= 4.1.4),
                pve-cluster,
                pve-container,
                pve-doc-generator (>= 8.0.5),
-- 
2.39.2



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

[pve-devel] [PATCH widget-toolkit v8 07/13] notification: matcher: match-field: show known fields/values

[Lukas Wagner]

These changes introduce combogrid pickers for the 'field' and 'value'
form elements for 'match-field' match rules. The 'field' picker shows
a list of all known metadata fields, while the 'value' picker shows a
list of all known values, filtered depending on the current value of
'field'.

The list of known fields/values is retrieved from new API endpoints.
Some values are marked 'internal' by the backend. This means that the
'value' field was not user-created (counter example: backup job
IDs) and can therefore be used as a base for translations.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/data/model/NotificationConfig.js  |  12 ++
 src/window/NotificationMatcherEdit.js | 297 +++++++++++++++++++++-----
 2 files changed, 253 insertions(+), 56 deletions(-)

diff --git a/src/data/model/NotificationConfig.js b/src/data/model/NotificationConfig.js
index e8ebf28..03cf317 100644
--- a/src/data/model/NotificationConfig.js
+++ b/src/data/model/NotificationConfig.js
@@ -15,3 +15,15 @@ Ext.define('proxmox-notification-matchers', {
     },
     idProperty: 'name',
 });
+
+Ext.define('proxmox-notification-fields', {
+    extend: 'Ext.data.Model',
+    fields: ['name', 'description'],
+    idProperty: 'name',
+});
+
+Ext.define('proxmox-notification-field-values', {
+    extend: 'Ext.data.Model',
+    fields: ['value', 'comment', 'field'],
+    idProperty: 'value',
+});
diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js
index e717ad7..be33efe 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -79,7 +79,7 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', {
 	labelWidth: 120,
     },
 
-    width: 700,
+    width: 800,
 
     initComponent: function() {
 	let me = this;
@@ -416,10 +416,22 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 		    let me = this;
 		    let record = me.get('selectedRecord');
 		    let currentData = record.get('data');
+
+		    let newValue = [];
+
+		    // Build equivalent regular expression if switching
+		    // to 'regex' mode
+		    if (value === 'regex') {
+			let regexVal = "^(";
+			regexVal += currentData.value.join('|') + ")$";
+			newValue.push(regexVal);
+		    }
+
 		    record.set({
 			data: {
 			    ...currentData,
 			    type: value,
+			    value: newValue,
 			},
 		    });
 		},
@@ -441,6 +453,8 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 			data: {
 			    ...currentData,
 			    field: value,
+			    // Reset value if field changes
+			    value: [],
 			},
 		    });
 		},
@@ -549,6 +563,9 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
     column2: [
 	{
 	    xtype: 'pmxNotificationMatchRuleSettings',
+	    cbind: {
+		baseUrl: '{baseUrl}',
+	    },
 	},
 
     ],
@@ -601,7 +618,7 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
 		let value = data.value;
 		text = Ext.String.format(gettext("Match field: {0}={1}"), field, value);
 		iconCls = 'fa fa-square-o';
-		if (!field || !value) {
+		if (!field || !value || (Ext.isArray(value) && !value.length)) {
 		    iconCls += ' internal-error';
 		}
 	    } break;
@@ -821,6 +838,11 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
 		if (type === undefined) {
 		    type = "exact";
 		}
+
+		if (type === 'exact') {
+		    matchedValue = matchedValue.split(',');
+		}
+
 		return {
 		    type: 'match-field',
 		    data: {
@@ -982,7 +1004,9 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
 Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
     extend: 'Ext.panel.Panel',
     xtype: 'pmxNotificationMatchRuleSettings',
+    mixins: ['Proxmox.Mixin.CBind'],
     border: false,
+    layout: 'anchor',
 
     items: [
 	{
@@ -1000,6 +1024,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 		['notall', gettext('At least one rule does not match')],
 		['notany', gettext('No rule matches')],
 	    ],
+	    // Hide initially to avoid glitches when opening the window
+	    hidden: true,
 	    bind: {
 		hidden: '{!showMatchingMode}',
 		disabled: '{!showMatchingMode}',
@@ -1011,7 +1037,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    fieldLabel: gettext('Node type'),
 	    isFormField: false,
 	    allowBlank: false,
-
+	    // Hide initially to avoid glitches when opening the window
+	    hidden: true,
 	    bind: {
 		value: '{nodeType}',
 		hidden: '{!showMatcherType}',
@@ -1025,57 +1052,9 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    ],
 	},
 	{
-	    fieldLabel: gettext('Match Type'),
-	    xtype: 'proxmoxKVComboBox',
-	    reference: 'type',
-	    isFormField: false,
-	    allowBlank: false,
-	    submitValue: false,
-	    field: 'type',
-
-	    bind: {
-		hidden: '{!typeIsMatchField}',
-		disabled: '{!typeIsMatchField}',
-		value: '{matchFieldType}',
-	    },
-
-	    comboItems: [
-		['exact', gettext('Exact')],
-		['regex', gettext('Regex')],
-	    ],
-	},
-	{
-	    fieldLabel: gettext('Field'),
-	    xtype: 'proxmoxKVComboBox',
-	    isFormField: false,
-	    submitValue: false,
-	    allowBlank: false,
-	    editable: true,
-	    displayField: 'key',
-	    field: 'field',
-	    bind: {
-		hidden: '{!typeIsMatchField}',
-		disabled: '{!typeIsMatchField}',
-		value: '{matchFieldField}',
-	    },
-	    // TODO: Once we have a 'notification registry', we should
-	    // retrive those via an API call.
-	    comboItems: [
-		['type', ''],
-		['hostname', ''],
-	    ],
-	},
-	{
-	    fieldLabel: gettext('Value'),
-	    xtype: 'textfield',
-	    isFormField: false,
-	    submitValue: false,
-	    allowBlank: false,
-	    field: 'value',
-	    bind: {
-		hidden: '{!typeIsMatchField}',
-		disabled: '{!typeIsMatchField}',
-		value: '{matchFieldValue}',
+	    xtype: 'pmxNotificationMatchFieldSettings',
+	    cbind: {
+		baseUrl: '{baseUrl}',
 	    },
 	},
 	{
@@ -1085,7 +1064,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    allowBlank: true,
 	    multiSelect: true,
 	    field: 'value',
-
+	    // Hide initially to avoid glitches when opening the window
+	    hidden: true,
 	    bind: {
 		value: '{matchSeverityValue}',
 		hidden: '{!typeIsMatchSeverity}',
@@ -1108,7 +1088,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    editable: true,
 	    displayField: 'key',
 	    field: 'value',
-
+	    // Hide initially to avoid glitches when opening the window
+	    hidden: true,
 	    bind: {
 		value: '{matchCalendarValue}',
 		hidden: '{!typeIsMatchCalendar}',
@@ -1122,3 +1103,207 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	},
     ],
 });
+
+Ext.define('Proxmox.panel.MatchFieldSettings', {
+    extend: 'Ext.panel.Panel',
+    xtype: 'pmxNotificationMatchFieldSettings',
+    border: false,
+    layout: 'anchor',
+    // Hide initially to avoid glitches when opening the window
+    hidden: true,
+    bind: {
+	hidden: '{!typeIsMatchField}',
+    },
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	control: {
+	    'field[reference=fieldSelector]': {
+		change: function(field) {
+		    let view = this.getView();
+		    let valueField = view.down('field[reference=valueSelector]');
+		    let store = valueField.getStore();
+		    let val = field.getValue();
+
+		    if (val) {
+			store.setFilters([
+			    {
+				property: 'field',
+				value: val,
+			    },
+			]);
+		    }
+		},
+	    },
+	},
+    },
+
+
+    initComponent: function() {
+	let me = this;
+
+	let store = Ext.create('Ext.data.Store', {
+	    model: 'proxmox-notification-fields',
+	    autoLoad: true,
+	    proxy: {
+		type: 'proxmox',
+		url: `/api2/json/${me.baseUrl}/matcher-fields`,
+	    },
+	    listeners: {
+		'load': function() {
+		    this.each(function(record) {
+			record.set({
+			    description:
+				Proxmox.Utils.formatNotificationFieldName(
+				    record.get('name'),
+				),
+			});
+		    });
+
+		    // Commit changes so that the description field is not marked
+		    // as dirty
+		    this.commitChanges();
+		},
+	    },
+	});
+
+	let valueStore = Ext.create('Ext.data.Store', {
+	    model: 'proxmox-notification-field-values',
+	    autoLoad: true,
+	    proxy: {
+		type: 'proxmox',
+
+		url: `/api2/json/${me.baseUrl}/matcher-field-values`,
+	    },
+	    listeners: {
+		'load': function() {
+		    this.each(function(record) {
+			if (record.get('field') === 'type') {
+			    record.set({
+				comment:
+				    Proxmox.Utils.formatNotificationFieldValue(
+					record.get('value'),
+				    ),
+			    });
+			}
+		    }, this, true);
+
+		    // Commit changes so that the description field is not marked
+		    // as dirty
+		    this.commitChanges();
+		},
+	    },
+	});
+
+	Ext.apply(me, {
+	    viewModel: Ext.create('Ext.app.ViewModel', {
+		parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(),
+		formulas: {
+		    isRegex: function(get) {
+			return get('matchFieldType') === 'regex';
+		    },
+		},
+	    }),
+	    items: [
+		{
+		    fieldLabel: gettext('Match Type'),
+		    xtype: 'proxmoxKVComboBox',
+		    reference: 'type',
+		    isFormField: false,
+		    allowBlank: false,
+		    submitValue: false,
+		    field: 'type',
+
+		    bind: {
+			value: '{matchFieldType}',
+		    },
+
+		    comboItems: [
+			['exact', gettext('Exact')],
+			['regex', gettext('Regex')],
+		    ],
+		},
+		{
+		    fieldLabel: gettext('Field'),
+		    reference: 'fieldSelector',
+		    xtype: 'proxmoxComboGrid',
+		    isFormField: false,
+		    submitValue: false,
+		    allowBlank: false,
+		    editable: false,
+		    store: store,
+		    queryMode: 'local',
+		    valueField: 'name',
+		    displayField: 'description',
+		    field: 'field',
+		    bind: {
+			value: '{matchFieldField}',
+		    },
+		    listConfig: {
+			columns: [
+			    {
+				header: gettext('Description'),
+				dataIndex: 'description',
+				flex: 2,
+			    },
+			    {
+				header: gettext('Field Name'),
+				dataIndex: 'name',
+				flex: 1,
+			    },
+			],
+		    },
+		},
+		{
+		    fieldLabel: gettext('Value'),
+		    reference: 'valueSelector',
+		    xtype: 'proxmoxComboGrid',
+		    autoSelect: false,
+		    editable: false,
+		    isFormField: false,
+		    submitValue: false,
+		    allowBlank: false,
+		    showClearTrigger: true,
+		    field: 'value',
+		    store: valueStore,
+		    valueField: 'value',
+		    displayField: 'value',
+		    notFoundIsValid: false,
+		    multiSelect: true,
+		    bind: {
+			value: '{matchFieldValue}',
+			hidden: '{isRegex}',
+		    },
+		    listConfig: {
+			columns: [
+			    {
+				header: gettext('Value'),
+				dataIndex: 'value',
+				flex: 1,
+			    },
+			    {
+				header: gettext('Comment'),
+				dataIndex: 'comment',
+				flex: 2,
+			    },
+			],
+		    },
+		},
+		{
+		    fieldLabel: gettext('Regex'),
+		    xtype: 'proxmoxtextfield',
+		    editable: true,
+		    isFormField: false,
+		    submitValue: false,
+		    allowBlank: false,
+		    field: 'value',
+		    bind: {
+			value: '{matchFieldValue}',
+			hidden: '{!isRegex}',
+		    },
+		},
+	    ],
+	});
+	me.callParent();
+    },
+});
-- 
2.39.2



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

[pve-devel] [PATCH widget-toolkit v8 08/13] notification: matcher: move match-field formulas to local viewModel

[Lukas Wagner]

This should make the code more cohesive and easier to follow.

No functional changes.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/window/NotificationMatcherEdit.js | 189 +++++++++++++-------------
 1 file changed, 95 insertions(+), 94 deletions(-)

diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js
index be33efe..559b405 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -380,15 +380,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 		}
 		return !record.isRoot();
 	    },
-	    typeIsMatchField: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		get: function(record) {
-		    return record?.get('type') === 'match-field';
-		},
-	    },
 	    typeIsMatchSeverity: {
 		bind: {
 		    bindTo: '{selectedRecord}',
@@ -407,89 +398,13 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 		    return record?.get('type') === 'match-calendar';
 		},
 	    },
-	    matchFieldType: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		set: function(value) {
-		    let me = this;
-		    let record = me.get('selectedRecord');
-		    let currentData = record.get('data');
-
-		    let newValue = [];
-
-		    // Build equivalent regular expression if switching
-		    // to 'regex' mode
-		    if (value === 'regex') {
-			let regexVal = "^(";
-			regexVal += currentData.value.join('|') + ")$";
-			newValue.push(regexVal);
-		    }
-
-		    record.set({
-			data: {
-			    ...currentData,
-			    type: value,
-			    value: newValue,
-			},
-		    });
-		},
-		get: function(record) {
-		    return record?.get('data')?.type;
-		},
-	    },
-	    matchFieldField: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		set: function(value) {
-		    let me = this;
-		    let record = me.get('selectedRecord');
-		    let currentData = record.get('data');
-
-		    record.set({
-			data: {
-			    ...currentData,
-			    field: value,
-			    // Reset value if field changes
-			    value: [],
-			},
-		    });
-		},
-		get: function(record) {
-		    return record?.get('data')?.field;
-		},
-	    },
-	    matchFieldValue: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		set: function(value) {
-		    let me = this;
-		    let record = me.get('selectedRecord');
-		    let currentData = record.get('data');
-		    record.set({
-			data: {
-			    ...currentData,
-			    value: value,
-			},
-		    });
-		},
-		get: function(record) {
-		    return record?.get('data')?.value;
-		},
-	    },
 	    matchSeverityValue: {
 		bind: {
 		    bindTo: '{selectedRecord}',
 		    deep: true,
 		},
 		set: function(value) {
-		    let me = this;
-		    let record = me.get('selectedRecord');
+		    let record = this.get('selectedRecord');
 		    let currentData = record.get('data');
 		    record.set({
 			data: {
@@ -1137,7 +1052,98 @@ Ext.define('Proxmox.panel.MatchFieldSettings', {
 	    },
 	},
     },
+    viewModel: {
+	// parent is set in `initComponents`
+	formulas: {
+	    typeIsMatchField: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		get: function(record) {
+		    return record?.get('type') === 'match-field';
+		},
+	    },
+	    isRegex: function(get) {
+		return get('matchFieldType') === 'regex';
+	    },
+	    matchFieldType: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		set: function(value) {
+		    let record = this.get('selectedRecord');
+		    let currentData = record.get('data');
+
+		    let newValue = [];
+
+		    // Build equivalent regular expression if switching
+		    // to 'regex' mode
+		    if (value === 'regex') {
+			let regexVal = "^";
+			if (currentData.value && currentData.value.length) {
+			    regexVal += `(${currentData.value.join('|')})`;
+			}
+			regexVal += "$";
+			newValue.push(regexVal);
+		    }
+
+		    record.set({
+			data: {
+			    ...currentData,
+			    type: value,
+			    value: newValue,
+			},
+		    });
+		},
+		get: function(record) {
+		    return record?.get('data')?.type;
+		},
+	    },
+	    matchFieldField: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		set: function(value) {
+		    let record = this.get('selectedRecord');
+		    let currentData = record.get('data');
 
+		    record.set({
+			data: {
+			    ...currentData,
+			    field: value,
+			    // Reset value if field changes
+			    value: [],
+			},
+		    });
+		},
+		get: function(record) {
+		    return record?.get('data')?.field;
+		},
+	    },
+	    matchFieldValue: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		set: function(value) {
+		    let record = this.get('selectedRecord');
+		    let currentData = record.get('data');
+		    record.set({
+			data: {
+			    ...currentData,
+			    value: value,
+			},
+		    });
+		},
+		get: function(record) {
+		    return record?.get('data')?.value;
+		},
+	    },
+	},
+    },
 
     initComponent: function() {
 	let me = this;
@@ -1195,15 +1201,10 @@ Ext.define('Proxmox.panel.MatchFieldSettings', {
 	    },
 	});
 
+	Ext.apply(me.viewModel, {
+	    parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(),
+	});
 	Ext.apply(me, {
-	    viewModel: Ext.create('Ext.app.ViewModel', {
-		parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(),
-		formulas: {
-		    isRegex: function(get) {
-			return get('matchFieldType') === 'regex';
-		    },
-		},
-	    }),
 	    items: [
 		{
 		    fieldLabel: gettext('Match Type'),
-- 
2.39.2



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

[pve-devel] [PATCH widget-toolkit v8 09/13] notification: matcher: move match-calendar fields to panel

[Lukas Wagner]

Also introduce a local viewModel that is linked to a parent viewModel,
allowing us to move the formulas to the panel.
This should make the code more cohesive and easier to follow.

No functional changes.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/window/NotificationMatcherEdit.js | 92 +++++++++++++++++----------
 1 file changed, 60 insertions(+), 32 deletions(-)

diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js
index 559b405..50145e3 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -389,15 +389,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 		    return record?.get('type') === 'match-severity';
 		},
 	    },
-	    typeIsMatchCalendar: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		get: function(record) {
-		    return record?.get('type') === 'match-calendar';
-		},
-	    },
 	    matchSeverityValue: {
 		bind: {
 		    bindTo: '{selectedRecord}',
@@ -417,26 +408,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 		    return record?.get('data')?.value;
 		},
 	    },
-	    matchCalendarValue: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		set: function(value) {
-		    let me = this;
-		    let record = me.get('selectedRecord');
-		    let currentData = record.get('data');
-		    record.set({
-			data: {
-			    ...currentData,
-			    value: value,
-			},
-		    });
-		},
-		get: function(record) {
-		    return record?.get('data')?.value;
-		},
-	    },
 	    rootMode: {
 		bind: {
 		    bindTo: '{selectedRecord}',
@@ -995,6 +966,58 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 		['unknown', gettext('Unknown')],
 	    ],
 	},
+	{
+	    xtype: 'pmxNotificationMatchCalendarSettings',
+	},
+    ],
+});
+
+Ext.define('Proxmox.panel.MatchCalendarSettings', {
+    extend: 'Ext.panel.Panel',
+    xtype: 'pmxNotificationMatchCalendarSettings',
+    border: false,
+    layout: 'anchor',
+    // Hide initially to avoid glitches when opening the window
+    hidden: true,
+    bind: {
+	hidden: '{!typeIsMatchCalendar}',
+    },
+    viewModel: {
+	// parent is set in `initComponents`
+	formulas: {
+	    typeIsMatchCalendar: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		get: function(record) {
+		    return record?.get('type') === 'match-calendar';
+		},
+	    },
+
+	    matchCalendarValue: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		set: function(value) {
+		    let me = this;
+		    let record = me.get('selectedRecord');
+		    let currentData = record.get('data');
+		    record.set({
+			data: {
+			    ...currentData,
+			    value: value,
+			},
+		    });
+		},
+		get: function(record) {
+		    return record?.get('data')?.value;
+		},
+	    },
+	},
+    },
+    items: [
 	{
 	    xtype: 'proxmoxKVComboBox',
 	    fieldLabel: gettext('Timespan to match'),
@@ -1003,11 +1026,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    editable: true,
 	    displayField: 'key',
 	    field: 'value',
-	    // Hide initially to avoid glitches when opening the window
-	    hidden: true,
 	    bind: {
 		value: '{matchCalendarValue}',
-		hidden: '{!typeIsMatchCalendar}',
 		disabled: '{!typeIsMatchCalender}',
 	    },
 
@@ -1017,6 +1037,14 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    ],
 	},
     ],
+
+    initComponent: function() {
+	let me = this;
+	Ext.apply(me.viewModel, {
+	    parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(),
+	});
+	me.callParent();
+    },
 });
 
 Ext.define('Proxmox.panel.MatchFieldSettings', {
-- 
2.39.2



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

[pve-devel] [PATCH widget-toolkit v8 10/13] notification: matcher: move match-severity fields to panel

[Lukas Wagner]

Also introduce a local viewModel that is linked to a parent viewModel,
allowing us to move the formulas to the panel.
This should make the code more cohesive and easier to follow.

No functional changes.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/window/NotificationMatcherEdit.js | 129 ++++++++++++++++----------
 1 file changed, 80 insertions(+), 49 deletions(-)

diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js
index 50145e3..9ab443f 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -380,34 +380,7 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 		}
 		return !record.isRoot();
 	    },
-	    typeIsMatchSeverity: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		get: function(record) {
-		    return record?.get('type') === 'match-severity';
-		},
-	    },
-	    matchSeverityValue: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		set: function(value) {
-		    let record = this.get('selectedRecord');
-		    let currentData = record.get('data');
-		    record.set({
-			data: {
-			    ...currentData,
-			    value: value,
-			},
-		    });
-		},
-		get: function(record) {
-		    return record?.get('data')?.value;
-		},
-	    },
+
 	    rootMode: {
 		bind: {
 		    bindTo: '{selectedRecord}',
@@ -944,27 +917,7 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    },
 	},
 	{
-	    xtype: 'proxmoxKVComboBox',
-	    fieldLabel: gettext('Severities to match'),
-	    isFormField: false,
-	    allowBlank: true,
-	    multiSelect: true,
-	    field: 'value',
-	    // Hide initially to avoid glitches when opening the window
-	    hidden: true,
-	    bind: {
-		value: '{matchSeverityValue}',
-		hidden: '{!typeIsMatchSeverity}',
-		disabled: '{!typeIsMatchSeverity}',
-	    },
-
-	    comboItems: [
-		['info', gettext('Info')],
-		['notice', gettext('Notice')],
-		['warning', gettext('Warning')],
-		['error', gettext('Error')],
-		['unknown', gettext('Unknown')],
-	    ],
+	    xtype: 'pmxNotificationMatchSeveritySettings',
 	},
 	{
 	    xtype: 'pmxNotificationMatchCalendarSettings',
@@ -1047,6 +1000,84 @@ Ext.define('Proxmox.panel.MatchCalendarSettings', {
     },
 });
 
+Ext.define('Proxmox.panel.MatchSeveritySettings', {
+    extend: 'Ext.panel.Panel',
+    xtype: 'pmxNotificationMatchSeveritySettings',
+    border: false,
+    layout: 'anchor',
+    // Hide initially to avoid glitches when opening the window
+    hidden: true,
+    bind: {
+	hidden: '{!typeIsMatchSeverity}',
+    },
+    viewModel: {
+	// parent is set in `initComponents`
+	formulas: {
+	    typeIsMatchSeverity: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		get: function(record) {
+		    return record?.get('type') === 'match-severity';
+		},
+	    },
+	    matchSeverityValue: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		set: function(value) {
+		    let record = this.get('selectedRecord');
+		    let currentData = record.get('data');
+		    record.set({
+			data: {
+			    ...currentData,
+			    value: value,
+			},
+		    });
+		},
+		get: function(record) {
+		    return record?.get('data')?.value;
+		},
+	    },
+	},
+    },
+    items: [
+	{
+	    xtype: 'proxmoxKVComboBox',
+	    fieldLabel: gettext('Severities to match'),
+	    isFormField: false,
+	    allowBlank: true,
+	    multiSelect: true,
+	    field: 'value',
+	    // Hide initially to avoid glitches when opening the window
+	    hidden: true,
+	    bind: {
+		value: '{matchSeverityValue}',
+		hidden: '{!typeIsMatchSeverity}',
+		disabled: '{!typeIsMatchSeverity}',
+	    },
+
+	    comboItems: [
+		['info', gettext('Info')],
+		['notice', gettext('Notice')],
+		['warning', gettext('Warning')],
+		['error', gettext('Error')],
+		['unknown', gettext('Unknown')],
+	    ],
+	},
+    ],
+
+    initComponent: function() {
+	let me = this;
+	Ext.apply(me.viewModel, {
+	    parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(),
+	});
+	me.callParent();
+    },
+});
+
 Ext.define('Proxmox.panel.MatchFieldSettings', {
     extend: 'Ext.panel.Panel',
     xtype: 'pmxNotificationMatchFieldSettings',
-- 
2.39.2



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

[pve-devel] [PATCH docs v8 11/13] notifications: describe new notification metadata fields

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 notifications.adoc | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/notifications.adoc b/notifications.adoc
index 25a9391..acca19b 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -301,19 +301,21 @@ Notification Events
 
 [width="100%",options="header"]
 |===========================================================================
-| Event                        | `type`            | Severity | Metadata fields (in addition to `type`)
-| System updates available     |`package-updates`  | `info`   | `hostname`
-| Cluster node fenced          |`fencing`          | `error`  | `hostname`
-| Storage replication failed   |`replication`      | `error`  | -
-| Backup finished              |`vzdump`           | `info` (`error` on failure) | `hostname`
-| Mail for root                |`system-mail`      | `unknown`| -
+| Event                            | `type`            | Severity | Metadata fields (in addition to `type`)
+| System updates available         |`package-updates`  | `info`   | `hostname`
+| Cluster node fenced              |`fencing`          | `error`  | `hostname`
+| Storage replication job failed   |`replication`      | `error`  | `hostname`, `job-id`
+| Backup succeeded                 |`vzdump`           | `info`   | `hostname`, `job-id` (only for backup jobs)
+| Backup failed                    |`vzdump`           | `error`  | `hostname`, `job-id` (only for backup jobs)
+| Mail for root                    |`system-mail`      | `unknown`| `hostname`
 |===========================================================================
 
 [width="100%",options="header"]
 |=======================================================================
-| Field name | Description
-| `type`     | Type of the notification
-| `hostname` | Hostname, without domain (e.g. `pve1`)
+| Field name        | Description
+| `type`            | Type of the notification
+| `hostname`        | Hostname, without domain (e.g. `pve1`)
+| `job-id`          | Job ID
 |=======================================================================
 
 System Mail Forwarding
-- 
2.39.2



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

[pve-devel] [PATCH docs v8 12/13] notifications: match-field 'exact'-mode can now match multiple values

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 notifications.adoc | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/notifications.adoc b/notifications.adoc
index acca19b..bdfebd0 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -233,11 +233,16 @@ configurable schedule.
 Field Matching Rules
 ~~~~~~~~~~~~~~~~~~~~
 Notifications have a selection of metadata fields that can be matched.
+When using `exact` as a matching mode, a `,` can be used as a separator.
+The matching rule then matches if the metadata field has *any* of the specified
+values.
 
 * `match-field exact:type=vzdump` Only match notifications about backups.
+* `match-field exact:type=replication,fencing` Match `replication` and `fencing` notifications.
 * `match-field regex:hostname=^.+\.example\.com$` Match the hostname of
 the node.
 
+
 If a matched metadata field does not exist, the notification will not be
 matched.
 For instance, a `match-field regex:hostname=.*` directive will only match
@@ -279,18 +284,7 @@ matcher: backup-failures
         comment Send notifications about backup failures to one group of admins
 
 matcher: cluster-failures
-        match-field exact:type=replication
-        match-field exact:type=fencing
-        mode any
-        target cluster-admins
-        comment Send cluster-related notifications to other group of admins
-----
-
-The last matcher could also be rewritten using a field matcher with a regular
-expression:
-----
-matcher: cluster-failures
-        match-field regex:type=^(replication|fencing)$
+        match-field exact:type=replication,fencing
         target cluster-admins
         comment Send cluster-related notifications to other group of admins
 ----
-- 
2.39.2



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

[pve-devel] [PATCH docs v8 13/13] notifications: add note regarding when 'job-id' is set for backups

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 notifications.adoc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/notifications.adoc b/notifications.adoc
index bdfebd0..6425e6c 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -312,6 +312,10 @@ Notification Events
 | `job-id`          | Job ID
 |=======================================================================
 
+NOTE: Backup job notifications only have `job-id` set if the backup job
+  was executed automatically based on its schedule, but not if it was triggered
+  manually by the 'Run now' button in the UI.
+
 System Mail Forwarding
 ---------------------
 
-- 
2.39.2



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

Re: [pve-devel] [PATCH many v8 00/13] notifications: notification metadata matching improvements

[Max Carrara]

On Fri Jul 5, 2024 at 3:46 PM CEST, Lukas Wagner wrote:
> This patch series attempts to improve the user experience when creating
> notification matchers.

The below can pretty much just be considered "proofreading" as I haven't
built and tested your changes, but since you already got a lot of
feedback on the last couple versions, I think that's fine. ;) Just
wanted to comment anyway.

The patches are rather easy to follow, and even though I'm no expert
when it comes to Ext JS, the UI changes look fine to me too. The new UI
logic feels (and is) much cleaner than before. There's nothing I can
otherwise comment on; everything's pretty straight-forward.

The *only* things I have noticed are rather minor - there are two tiny
typos in the commit messages of patch 01 and 02, but these can probably
be fixed when applying the series:
  01: Last sentence of message - "It it can be considered internal." 
  02: First sentence of message - "This allows us to access us the [...]"

That's it otherwise from me - LGTM.

Reviewed-by: Max Carrara <m.carrara@proxmox.com>

>
> Some of the noteworthy changes:
>   - Allow setting a custom backup job ID, similar how we handle it for
>   sync/prune jobs in PBS (to allow recognizable names used in matchers)
>   - New metadata fields:
>     - job-id: Job ID for backup-jobs or replication-jobs
>   - Add an API that enumerates known notification metadata fields/values
>   - Suggest known fields/values in match rule window
>   - Some code clean up for match rule edit window
>   - Extended the 'exact' match-field mode - it now allows setting multiple
>     allowed values, separated via ',':
>       e.g. `match-field exact:type=replication,fencing
>     Originally, I created a separate 'list' match type for this, but
>     since the semantics for a list with one value and 'exact' mode
>     are identical, I decided to just extend 'exact'.
>     This is a safe change since there are are no values where a ','
>     makes any sense (config IDs, hostnames)
>
> NOTE: Might need a versionened break, since the widget-toolkit-patches
> depend on new APIs provided by pve-manager. If the API is not present,
> creating matchers with 'match-field' does not work (cannot load lists
> of known values/fields)
>
> Inter-Dependencies:
>   - the widget-toolkit dep in pve-manager needs to be bumped
>     to at least 4.1.4
>     (we need "utils: add mechanism to add and override translatable notification event
>     descriptions in the product specific UIs", otherwise the UI breaks
>     with the pve-manager patches applied) --> already included a patch for
>     this
>   - widget-toolkit relies on a new API endpoint provided by pve-manager:
>     --> we require a versioned break in widget-toolkit on pve-manager
>   - pve-manager needs bumped pve-guest-common (thx @Fabian)
>
> Changelog:
>   - v8: incorporate feedback from @Fabian, thx a lot!
>     - Made 'job-id' API param usable by root@pam only - this should prevent
>       abuse by spoofing job-id, potentially bothering other users with bogus
>       notifications.
>     - Don't set 'job-id' when starting a backup job via 'Run now' in the UI
>     - Add a note to the docs explaining when job-id is set and when not.
>     - Drop already applied patches
>   - v7: incorporated some more feedback from @Fiona, thx!
>     - Fixed error when switching from 'exact' to 'regex' if the text field
>       was empty
>     - rebased to latest master
>     - 'backport' doc improvements from PBS
>     - bumped widget-toolkit dep
>   - v6: incorporate feedback from @Fiona, thx!
>     - rename 'id' -> 'job-id' in VZDump API handler
>     - consolidate 'replication-job'/'backup-job' to 'job-id'
>     - Move 'job-id' setting to advanced tab in backup job edit.
>     - Don't use 'internal' flag to mark translatable fields, since
>       the only field where that's necessary is 'type' for now - so
>       just add a hardcoded check
>   - v5:
>     - Rebased onto latest master, resolving some small conflict
>   - v4:
>     - widget-toolkit: break out changes for the utils module so that they
>       can be applied ahead of time to ease dep bumping
>     - don't show Job IDs in the backup/replication job columns
>   - v3:
>     - Drop already applied patches for `proxmox`
>     - Rebase onto latest master - minor conflict resolution was needed
>   - v2:
>     - include 'type' metadata field for forwarded mails
>       --> otherwise it's not possible to match them
>     - include Maximilliano's T-b trailer in UI patches
>
> pve-guest-common:
>
> Lukas Wagner (1):
>   vzdump: common: allow 'job-id' as a parameter without being in schema
>
>  src/PVE/VZDump/Common.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
>
> pve-manager:
>
> Lukas Wagner (5):
>   api: jobs: vzdump: pass job 'job-id' parameter
>   ui: dc: backup: allow to set custom job id in  advanced settings
>   api: notification: add API for getting known metadata fields/values
>   ui: utils: add overrides for translatable notification fields/values
>   d/control: bump proxmox-widget-toolkit dependency to 4.1.4
>
>  PVE/API2/Backup.pm                          |   2 +-
>  PVE/API2/Cluster/Notifications.pm           | 139 ++++++++++++++++++++
>  PVE/API2/VZDump.pm                          |  13 +-
>  PVE/Jobs/VZDump.pm                          |   4 +-
>  PVE/VZDump.pm                               |   6 +-
>  debian/control                              |   2 +-
>  www/manager6/Utils.js                       |  11 ++
>  www/manager6/dc/Backup.js                   |   4 -
>  www/manager6/panel/BackupAdvancedOptions.js |  23 ++++
>  9 files changed, 192 insertions(+), 12 deletions(-)
>
>
> proxmox-widget-toolkit:
>
> Lukas Wagner (4):
>   notification: matcher: match-field: show known fields/values
>   notification: matcher: move match-field formulas to local viewModel
>   notification: matcher: move match-calendar fields to panel
>   notification: matcher: move match-severity fields to panel
>
>  src/data/model/NotificationConfig.js  |  12 +
>  src/window/NotificationMatcherEdit.js | 613 ++++++++++++++++++--------
>  2 files changed, 441 insertions(+), 184 deletions(-)
>
>
> pve-docs:
>
> Lukas Wagner (3):
>   notifications: describe new notification metadata fields
>   notifications: match-field 'exact'-mode can now match multiple values
>   notifications: add note regarding when 'job-id' is set for backups
>
>  notifications.adoc | 42 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
>
>
> Summary over all repositories:
>   13 files changed, 655 insertions(+), 218 deletions(-)



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

Re: [pve-devel] [PATCH many v8 00/13] notifications: notification metadata matching improvements

[Lukas Wagner]

On  2024-07-08 10:12, Max Carrara wrote:
> On Fri Jul 5, 2024 at 3:46 PM CEST, Lukas Wagner wrote:
>> This patch series attempts to improve the user experience when creating
>> notification matchers.
> 
> The below can pretty much just be considered "proofreading" as I haven't
> built and tested your changes, but since you already got a lot of
> feedback on the last couple versions, I think that's fine. ;) Just
> wanted to comment anyway.
> 
> The patches are rather easy to follow, and even though I'm no expert
> when it comes to Ext JS, the UI changes look fine to me too. The new UI
> logic feels (and is) much cleaner than before. There's nothing I can
> otherwise comment on; everything's pretty straight-forward.
> 
> The *only* things I have noticed are rather minor - there are two tiny
> typos in the commit messages of patch 01 and 02, but these can probably
> be fixed when applying the series:
>   01: Last sentence of message - "It it can be considered internal." 
>   02: First sentence of message - "This allows us to access us the [...]"
> 
> That's it otherwise from me - LGTM.
> 
> Reviewed-by: Max Carrara <m.carrara@proxmox.com>
> 

Thanks a lot for your review, Max!

I'll send a v9 with the typos fixed and your R-b's added.

-- 
- Lukas


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