public inbox for pmg-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Dominik Csapak <d.csapak@proxmox.com>, pmg-devel@lists.proxmox.com
Subject: Re: [pmg-devel] [PATCH pmg-gui v2 2/2] rules/objects: add mode selector dropdown
Date: Wed, 21 Feb 2024 19:31:08 +0100	[thread overview]
Message-ID: <8d4b2f87-1075-4e99-9809-b0c3499ab5e5@proxmox.com> (raw)
In-Reply-To: <20240221122439.1281024-14-d.csapak@proxmox.com>

Am 21/02/2024 um 13:24 schrieb Dominik Csapak:
> for objects and object types in rules. We add a simple dropdown for the
> 'and' and 'invert' flags, to be somewhat consistent with the
> notification matchers from pve and to make the wording more clear than
> simple and/invert.
> 
> For What matches add a special warning hint, since that behaves a bit
> special because of the mail parts.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  js/Makefile                    |  1 +
>  js/ObjectGroup.js              | 64 ++++++++++++++++++++++++++++++++--
>  js/ObjectGroupConfiguration.js |  4 +++
>  js/RuleInfo.js                 | 44 +++++++++++++++++++++++
>  js/form/ModeSelector.js        | 11 ++++++
>  5 files changed, 122 insertions(+), 2 deletions(-)
>  create mode 100644 js/form/ModeSelector.js
> 
> diff --git a/js/Makefile b/js/Makefile
> index 78f2b57..4267092 100644
> --- a/js/Makefile
> +++ b/js/Makefile
> @@ -2,6 +2,7 @@ include ../defines.mk
>  
>  JSSRC=							\
>  	Utils.js					\
> +	form/ModeSelector.js				\

Please codify in the file and class name which mode this is for, as of now
the name is rather overly generic.

Something like "MatchModeSelector" would  be already better IMO.

>  	FilterProxy.js					\
>  	LoginView.js					\
>  	RoleSelector.js					\
> diff --git a/js/ObjectGroup.js b/js/ObjectGroup.js
> index 2223ffa..b91bf97 100644
> --- a/js/ObjectGroup.js
> +++ b/js/ObjectGroup.js
> @@ -10,6 +10,7 @@ Ext.define('PMG.ObjectGroup', {
>      showDirection: false, // only important for SMTP Whitelist
>  
>      ogdata: undefined,
> +    oclass: undefined,

not a fan of those needlessly abbreviated config keys, this, and the existing
one, just makes the code harder to read.

>  
>      emptyText: gettext('Please select an object.'),
>  
> @@ -32,10 +33,14 @@ Ext.define('PMG.ObjectGroup', {
>      setObjectInfo: function(ogdata) {
>  	let me = this;
>  
> +	let mode = ogdata.invert ? 'not' : '';
> +	mode += ogdata.and ? 'all' : 'any';
> +
>  	me.ogdata = ogdata;
>  
>  	if (me.ogdata === undefined) {
>  	    me.down('#oginfo').update(me.emptyText);
> +	    me.down('#modeBox').setHidden(true);
>  	} else {
>  	    let html = '<b>' + Ext.String.htmlEncode(me.ogdata.name) + '</b>';
>  	    html += "<br><br>";
> @@ -43,6 +48,12 @@ Ext.define('PMG.ObjectGroup', {
>  
>  	    me.down('#oginfo').update(html);
>  	    me.down('#ogdata').setHidden(false);
> +	    let modeSelector = me.down('#modeSelector');
> +	    modeSelector.suspendEvents();
> +	    me.down('#modeSelector').setValue(mode);
> +	    modeSelector.resumeEvents();
> +	    me.down('#modeBox').setHidden(false);
> +	    me.down('#whatWarning').setHidden(me.oclass !== 'what');
>  	}
>      },
>  
> @@ -205,13 +216,62 @@ Ext.define('PMG.ObjectGroup', {
>  	me.dockedItems.push({
>  	    dock: 'top',
>  	    border: 1,
> -	    layout: 'anchor',
> +	    layout: 'hbox',
>  	    hidden: !!me.hideGroupInfo,
>  	    itemId: 'ogdata',
>  	    items: [
> +		{
> +		    xtype: 'container',
> +		    itemId: 'modeBox',
> +		    hidden: true,
> +		    width: 220,
> +		    padding: 10,
> +		    layout: {
> +			type: 'vbox',
> +			align: 'stretch',
> +		    },
> +		    items: [
> +			{
> +			    xtype: 'box',
> +			    html: `<b>${gettext("Match if")}</b>`,
> +			},
> +			{
> +			    xtype: 'pmgModeSelector',
> +			    itemId: 'modeSelector',
> +			    padding: '10 0 0 0',
> +			    listeners: {
> +				change: function(_field, value) {
> +				    let invert = value.startsWith('not');
> +				    let and = value.endsWith('all');
> +
> +				    let url = `${me.baseurl}/config`;

nit: I'd rather avoid the extra variable here, just an inline:

url: `${me.baseurl}/config`,

> +
> +				    Proxmox.Utils.API2Request({
> +					url,
> +					method: 'PUT',
> +					params: {
> +					    and: and ? 1 : 0,
> +					    invert: invert ? 1 : 0,

nit: while the separate variables have IMO a value here, the ternary
could be used on assignment to and/invert already, then one could just
use `param: { and, invert }` here (maybe not in a single line)

> +					},
> +					success: function() {
> +					    me.fireEvent('modeUpdate', me);
> +					},

tiny nit: could be an arrow fn:

success: () => me.fireEvent('modeUpdate', me),

> +				    });
> +				},
> +			    },
> +			},
> +			{
> +			    xtype: 'displayfield',
> +			    itemId: 'whatWarning',
> +			    hidden: true,
> +			    value: gettext("Caution: What Objects match per mail part, so be careful with any option besides 'Any matches'."),

Hmm, this is a bit hard to word better, but maybe something slightly adapted
like:

"Caution: 'What Objects' match each mail part separately, so be careful with any option besides 'Any matches'."

Also, we could hide this is 'Any matches' is selected, and making it span over
all columns would make it look better too. Maybe in some bottom docked bar,
as otherwise it can flow over the description.

btw. why does the left store reloads after every match selection change?
Maybe an explicit reload button there (simple icon without text) could be better
for that?


> +			    userCls: 'pmx-hint',
> +			},
> +		    ],
> +		},
>  		{
>  		    xtype: 'component',
> -		    anchor: '100%',
> +		    flex: 1,
>  		    itemId: 'oginfo',
>  		    style: { 'white-space': 'pre' },
>  		    padding: 10,
> diff --git a/js/ObjectGroupConfiguration.js b/js/ObjectGroupConfiguration.js
> index 1d72851..f59f5ed 100644
> --- a/js/ObjectGroupConfiguration.js
> +++ b/js/ObjectGroupConfiguration.js
> @@ -30,6 +30,7 @@ Ext.define('PMG.ObjectGroupConfiguration', {
>  
>  	var right = Ext.create('PMG.ObjectGroup', {
>  	    otype_list: me.otype_list,
> +	    oclass: me.ogclass,
>  	    border: false,
>  	    region: 'center',
>  	    listeners: {
> @@ -40,6 +41,9 @@ Ext.define('PMG.ObjectGroupConfiguration', {
>  			left.run_editor();
>  		    }
>  		},
> +		modeUpdate: function() {
> +		    left.reload();
> +		},

nit, could be an arrow fn:

modeUpdate: () => left.reload(),

>  	    },
>  	});
>  
> diff --git a/js/RuleInfo.js b/js/RuleInfo.js
> index c29c0ca..d53c1c5 100644
> --- a/js/RuleInfo.js
> +++ b/js/RuleInfo.js
> @@ -120,6 +120,8 @@ Ext.define('PMG.RuleInfo', {
>  			name: oc,
>  			oclass: oc,
>  			type: true,
> +			invert: ruledata[`${oc}-invert`],
> +			and: ruledata[`${oc}-and`],
>  			leaf: false,
>  			expanded: true,
>  			expandable: false,
> @@ -162,6 +164,25 @@ Ext.define('PMG.RuleInfo', {
>  	    return true;
>  	},
>  
> +	updateMode: function(field, value) {
> +	    let me = this;
> +	    let vm = me.getViewModel();
> +	    let oclass = field.getWidgetRecord().data.oclass;
> +
> +	    let params = {};
> +	    params[`${oclass}-invert`] = value.startsWith('not') ? 1 : 0;
> +	    params[`${oclass}-and`] = value.endsWith('all') ? 1 : 0;
> +
> +	    Proxmox.Utils.API2Request({
> +		url: `${vm.get('baseurl')}/config`,
> +		method: 'PUT',
> +		params,
> +		success: function() {
> +		    me.reload();
> +		},

nit, could be an arrow fn:

success: () => me.reload(),

> +	    });
> +	},
> +
>  	control: {
>  	    'treepanel[reference=usedobjects]': {
>  		drop: 'addDrop',
> @@ -169,6 +190,9 @@ Ext.define('PMG.RuleInfo', {
>  	    'tabpanel[reference=availobjects] > grid': {
>  		drop: 'removeDrop',
>  	    },
> +	    'pmgModeSelector': {
> +		change: 'updateMode',
> +	    },
>  	},
>      },
>  
> @@ -311,6 +335,26 @@ Ext.define('PMG.RuleInfo', {
>  		    },
>  		    flex: 1,
>  		},
> +		{
> +		    header: gettext('Match if'),
> +		    xtype: 'widgetcolumn',
> +		    width: 200,
> +		    widget: {
> +			xtype: 'pmgModeSelector',
> +		    },
> +		    onWidgetAttach: function(col, widget, rec) {
> +			if (rec.data.type && rec.data.oclass !== 'action') {
> +			    let mode = rec.data.invert ? 'not' : '';
> +			    mode += rec.data.and ? 'all' : 'any';
> +			    widget.suspendEvents();
> +			    widget.setValue(mode);
> +			    widget.resumeEvents();
> +			    widget.setHidden(false);
> +			} else {
> +			    widget.setHidden(true);
> +			}
> +		    },
> +		},
>  		{
>  		    text: '',
>  		    xtype: 'actioncolumn',




  reply	other threads:[~2024-02-21 18:31 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-21 12:24 [pmg-devel] [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 01/10] RuleCache: reorganize how we gather marks and spaminfo Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 02/10] api: refactor rule parameters Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 03/10] add objectgroup attributes and/invert Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 04/10] add rule attributes and/invert (for each relevant type) Dominik Csapak
2024-02-22  6:46   ` Thomas Lamprecht
2024-02-22  7:34     ` Dominik Csapak
2024-02-22  7:38       ` Thomas Lamprecht
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 05/10] RuleCache: load rule/objectgroup attributes from database Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 06/10] RuleCache: implement and/invert for when/from/to Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 07/10] MailQueue: return maximum AID Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 08/10] ModGroup: add possibility to explode to all targets Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 09/10] RuleCache: implement and/invert for what matches Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-api v2 10/10] pmgdb: extend dump output to include add/invert Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-docs v2 1/1] rule system: explain new and mode and invert flag Dominik Csapak
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-gui v2 1/2] rules: use tree panel instead of grouping feature of the grid Dominik Csapak
2024-02-21 17:42   ` Thomas Lamprecht
2024-02-21 12:24 ` [pmg-devel] [PATCH pmg-gui v2 2/2] rules/objects: add mode selector dropdown Dominik Csapak
2024-02-21 18:31   ` Thomas Lamprecht [this message]
2024-02-21 18:36 ` [pmg-devel] applied-partially: [PATCH pmg-api/docs/gui v2] implement and combination and inversion of groups and objects Stoiko Ivanov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8d4b2f87-1075-4e99-9809-b0c3499ab5e5@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pmg-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal