Re: [pve-devel] [PATCH proxmox-widget-toolkit 25/27] notification: matcher: add UI for matcher editing

[Dominik Csapak <d.csapak@proxmox.com>]

in generaly my biggest issue with this patch is that
the various functions/formulas/handler are way to far
away from the place they live/get used/etc.

in generaly i'd try to use the 'controller+viewmodel' consistently
so have a controller (where the logic (methods+handlers) live, the
view where the structure of the layout lives
(and if necessary the viewmodel where the data+formulas live)

otherwise not a very deep review, but some comments inline nonetheless:

On 11/7/23 11:18, Lukas Wagner wrote:
> This modifies the old filter edit window in the following ways:
>    - Split content into multiple panels
>      - Name and comment in the first tab
>      - Match rules in a tree-structure in the second tab
>      - Targets to notify in the third tab
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> 
> Notes:
>      The code binding the match rule tree structure to the editable fields
>      could definitely be a bit cleaner. I think this is the first time that
>      we have used such a pattern, so there there was much experimentation
>      needed to get this working.
>      I plan to revisit it and clean up a bit later, I wanted to get
>      the notification system changes on the list ASAP.
> 
>   src/window/NotificationMatcherEdit.js | 867 ++++++++++++++++++++++++--
>   1 file changed, 820 insertions(+), 47 deletions(-)
> 
> diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js
> index a014f3e..c6f0726 100644
> --- a/src/window/NotificationMatcherEdit.js
> +++ b/src/window/NotificationMatcherEdit.js
> @@ -1,6 +1,6 @@
> -Ext.define('Proxmox.panel.NotificationMatcherEditPanel', {
> +Ext.define('Proxmox.panel.NotificationMatcherGeneralPanel', {
>       extend: 'Proxmox.panel.InputPanel',
> -    xtype: 'pmxNotificationMatcherEditPanel',
> +    xtype: 'pmxNotificationMatcherGeneralPanel',
>       mixins: ['Proxmox.Mixin.CBind'],
>   
>       items: [
> @@ -15,53 +15,27 @@ Ext.define('Proxmox.panel.NotificationMatcherEditPanel', {
>   	    allowBlank: false,
>   	},
>   	{
> -	    xtype: 'proxmoxKVComboBox',
> -	    name: 'min-severity',
> -	    fieldLabel: gettext('Minimum Severity'),
> -	    value: null,
> +	    xtype: 'proxmoxtextfield',
> +	    name: 'comment',
> +	    fieldLabel: gettext('Comment'),
>   	    cbind: {
>   		deleteEmpty: '{!isCreate}',
>   	    },
> -	    comboItems: [
> -		['info', 'info'],
> -		['notice', 'notice'],
> -		['warning', 'warning'],
> -		['error', 'error'],
> -	    ],
> -	    triggers: {
> -		clear: {
> -		    cls: 'pmx-clear-trigger',
> -		    weight: -1,
> -		    hidden: false,
> -		    handler: function() {
> -			this.setValue('');
> -		    },
> -		},
> -	    },
> -	},
> -	{
> -	    xtype: 'proxmoxcheckbox',
> -	    fieldLabel: gettext('Invert match'),
> -	    name: 'invert-match',
> -	    uncheckedValue: 0,
> -	    defaultValue: 0,
> -	    cbind: {
> -		deleteDefaultValue: '{!isCreate}',
> -	    },
>   	},
> +    ],
> +});
> +
> +Ext.define('Proxmox.panel.NotificationMatcherTargetPanel', {
> +    extend: 'Proxmox.panel.InputPanel',
> +    xtype: 'pmxNotificationMatcherTargetPanel',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    items: [
>   	{
>   	    xtype: 'pmxNotificationTargetSelector',
>   	    name: 'target',
>   	    allowBlank: false,
>   	},
> -	{
> -	    xtype: 'proxmoxtextfield',
> -	    name: 'comment',
> -	    fieldLabel: gettext('Comment'),
> -	    cbind: {
> -		deleteEmpty: '{!isCreate}',
> -	    },
> -	},
>       ],
>   });
>   
> @@ -74,7 +48,7 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', {
>   	labelWidth: 120,
>       },
>   
> -    width: 500,
> +    width: 700,
>   
>       initComponent: function() {
>   	let me = this;
> @@ -97,12 +71,38 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', {
>   	me.subject = gettext('Notification Matcher');
>   
>   	Ext.apply(me, {
> -	    items: [{
> -		name: me.name,
> -		xtype: 'pmxNotificationMatcherEditPanel',
> -		isCreate: me.isCreate,
> -		baseUrl: me.baseUrl,
> -	    }],
> +	    bodyPadding: 0,
> +	    items: [
> +		{
> +		    xtype: 'tabpanel',
> +		    region: 'center',
> +		    layout: 'fit',
> +		    bodyPadding: 10,
> +		    items: [
> +			{
> +			    name: me.name,
> +			    title: gettext('General'),
> +			    xtype: 'pmxNotificationMatcherGeneralPanel',
> +			    isCreate: me.isCreate,
> +			    baseUrl: me.baseUrl,
> +			},
> +			{
> +			    name: me.name,
> +			    title: gettext('Match Rules'),
> +			    xtype: 'pmxNotificationMatchRulesEditPanel',
> +			    isCreate: me.isCreate,
> +			    baseUrl: me.baseUrl,
> +			},
> +			{
> +			    name: me.name,
> +			    title: gettext('Targets to notify'),
> +			    xtype: 'pmxNotificationMatcherTargetPanel',
> +			    isCreate: me.isCreate,
> +			    baseUrl: me.baseUrl,
> +			},
> +		    ],
> +		},
> +	    ],
>   	});
>   
>   	me.callParent();
> @@ -252,3 +252,776 @@ Ext.define('Proxmox.form.NotificationTargetSelector', {
>       },
>   
>   });
> +
> +Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
> +    extend: 'Proxmox.panel.InputPanel',
> +    xtype: 'pmxNotificationMatchRulesEditPanel',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    viewModel: {
> +	data: {
> +	    selectedRecord: null,
> +	    matchFieldType: 'exact',
> +	    matchFieldField: '',
> +	    matchFieldValue: '',
> +	    rootMode: 'all',
> +	},
> +
> +	formulas: {

i'm not really a big fan of this, basically i have two
complaints:

* there are too many formulas here
* they are very far away from where they are used
   (logically (a different component) as well as literally far away in the code)

you can inherit the viewmodels too, so you could probably move some formulas
in the child component where they are used?

also there is room for some improvement there:
(see further down)


> +	    nodeType: {
> +		get: function(get) {
> +		    let record = get('selectedRecord');
> +		    return record?.get('type');
> +		},
> +		set: function(value) {
> +		    let me = this;
> +		    let record = me.get('selectedRecord');
> +
> +		    let data;
> +
> +		    switch (value) {
> +			case 'match-severity':
> +			    data = {
> +				value: ['info', 'notice', 'warning', 'error'],
> +			    };
> +			    break;
> +			case 'match-field':
> +			    data = {
> +				type: 'exact',
> +				field: '',
> +				value: '',
> +			    };
> +			    break;
> +			case 'match-calendar':
> +			    data = {
> +				value: '',
> +			    };
> +			    break;
> +		    }
> +
> +		    let node = {
> +			type: value,
> +			data,
> +		    };
> +		    record.set(node);
> +		},
> +	    },
> +	    showMatchingMode: function(get) {
> +		let record = get('selectedRecord');
> +		if (!record) {
> +		    return false;
> +		}
> +		return record.isRoot();
> +	    },
> +	    showMatcherType: function(get) {
> +		let record = get('selectedRecord');
> +		if (!record) {
> +		    return false;
> +		}
> +		return !record.isRoot();
> +	    },

these two are inverts of each other, so with the '!' operator
there is no need for both

> +	    typeIsMatchField: {
> +		bind: {
> +		    bindTo: '{selectedRecord}',
> +		    deep: true,
> +		},
> +		get: function(record) {
> +		    return record?.get('type') === 'match-field';
> +		},
> +	    },
> +	    typeIsMatchSeverity: {
> +		bind: {
> +		    bindTo: '{selectedRecord}',
> +		    deep: true,
> +		},
> +		get: function(record) {
> +		    return record?.get('type') === 'match-severity';
> +		},
> +	    },
> +	    typeIsMatchCalendar: {
> +		bind: {
> +		    bindTo: '{selectedRecord}',
> +		    deep: true,
> +		},
> +		get: function(record) {
> +		    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');
> +		    record.set({
> +			data: {
> +			    ...currentData,
> +			    type: value,
> +			},
> +		    });
> +		},
> +		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,
> +			},
> +		    });
> +		},
> +		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 currentData = record.get('data');
> +		    record.set({
> +			data: {
> +			    ...currentData,
> +			    value: value,
> +			},
> +		    });
> +		},
> +		get: function(record) {
> +		    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}',
> +		    deep: true,
> +		},
> +		set: function(value) {
> +		    let me = this;
> +		    let record = me.get('selectedRecord');
> +		    let currentData = record.get('data');
> +		    record.set({
> +			data: {
> +			    ...currentData,
> +			    value,
> +			},
> +		    });
> +		},
> +		get: function(record) {
> +		    return record?.get('data')?.value;
> +		},
> +	    },
> +	    invertMatch: {
> +		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,
> +			    invert: value,
> +			},
> +		    });
> +		},
> +		get: function(record) {
> +		    return record?.get('data')?.invert;
> +		},
> +	    },


for all the 'matchXY' you could probably simply use a change handler
instead of writing out the setter for each manually

(the get can be ok, but for those i guess you wouldn't need a formula
but coud use '{selectedRecord.data.value}' or '{selectedRecord.data.field}', no?

even if they are really needed, they are basically all the same and could
be generated quite easily IMHO. not only would it make much shorter,
but would catch copy & paste errors


> +	},
> +    },
> +
> +    column1: [
> +	{
> +	    xtype: 'pmxNotificationMatchRuleTree',
> +	    cbind: {
> +		isCreate: '{isCreate}',
> +	    },
> +	},
> +    ],
> +    column2: [
> +	{
> +	    xtype: 'pmxNotificationMatchRuleSettings',
> +	},
> +
> +    ],
> +
> +    onGetValues: function(values) {
> +	let me = this;
> +
> +	let deleteArrayIfEmtpy = (field) => {
> +	    if (Ext.isArray(values[field])) {
> +		if (values[field].length === 0) {
> +		    delete values[field];
> +		    if (!me.isCreate) {
> +			Proxmox.Utils.assemble_field_data(values, { 'delete': field });
> +		    }
> +		}
> +	    }
> +	};
> +	deleteArrayIfEmtpy('match-field');
> +	deleteArrayIfEmtpy('match-severity');
> +	deleteArrayIfEmtpy('match-calendar');
> +
> +	return values;
> +    },
> +});
> +
> +Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
> +    extend: 'Ext.panel.Panel',
> +    xtype: 'pmxNotificationMatchRuleTree',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +    border: false,
> +
> +    getNodeTextAndIcon: function(type, data) {
> +	let text;
> +	let iconCls;
> +
> +	switch (type) {
> +	    case 'match-severity': {
> +		let v = data.value.join(', ');
> +		text = Ext.String.format(gettext("Match severity: {0}"), v);
> +		iconCls = 'fa fa-exclamation';
> +	    } break;
> +	    case 'match-field': {
> +		let field = data.field;
> +		let value = data.value;
> +		text = Ext.String.format(gettext("Match field: {0}={1}"), field, value);
> +		iconCls = 'fa fa-cube';
> +	    } break;
> +	    case 'match-calendar': {
> +		let v = data.value;
> +		text = Ext.String.format(gettext("Match calendar: {0}"), v);
> +		iconCls = 'fa fa-calendar-o';
> +	    } break;
> +	    case 'mode':
> +		if (data.value === 'all') {
> +		    text = gettext("All");
> +		} else if (data.value === 'any') {
> +		    text = gettext("Any");
> +		}
> +		if (data.invert) {
> +		    text = `!${text}`;
> +		}
> +		iconCls = 'fa fa-filter';
> +
> +		break;
> +	}
> +
> +	return [text, iconCls];
> +    },
> +
> +    initComponent: function() {
> +	let me = this;
> +
> +	let treeStore = Ext.create('Ext.data.TreeStore', {
> +	    root: {
> +		expanded: true,
> +		expandable: false,
> +		text: '',
> +		type: 'mode',
> +		data: {
> +		    value: 'all',
> +		    invert: false,
> +		},
> +		children: [],
> +		iconCls: 'fa fa-filter',
> +	    },
> +	});
> +
> +	let realMatchFields = Ext.create({
> +	    xtype: 'hiddenfield',
> +	    setValue: function(value) {
> +		this.value = value;
> +		this.checkChange();
> +	    },
> +	    getValue: function() {
> +		return this.value;
> +	    },
> +	    getSubmitValue: function() {
> +		let value = this.value;
> +		if (!value) {
> +		    value = [];
> +		}
> +		return value;
> +	    },
> +	    name: 'match-field',
> +	});
> +
> +	let realMatchSeverity = Ext.create({
> +	    xtype: 'hiddenfield',
> +	    setValue: function(value) {
> +		this.value = value;
> +		this.checkChange();
> +	    },
> +	    getValue: function() {
> +		return this.value;
> +	    },
> +	    getSubmitValue: function() {
> +		let value = this.value;
> +		if (!value) {
> +		    value = [];
> +		}
> +		return value;
> +	    },
> +	    name: 'match-severity',
> +	});
> +
> +	let realMode = Ext.create({
> +	    xtype: 'hiddenfield',
> +	    name: 'mode',
> +	    setValue: function(value) {
> +		this.value = value;
> +		this.checkChange();
> +	    },
> +	    getValue: function() {
> +		return this.value;
> +	    },
> +	    getSubmitValue: function() {
> +		let value = this.value;
> +		return value;
> +	    },
> +	});
> +
> +	let realMatchCalendar = Ext.create({
> +	    xtype: 'hiddenfield',
> +	    name: 'match-calendar',
> +
> +	    setValue: function(value) {
> +		this.value = value;
> +		this.checkChange();
> +	    },
> +	    getValue: function() {
> +		return this.value;
> +	    },
> +	    getSubmitValue: function() {
> +		let value = this.value;
> +		return value;
> +	    },
> +	});

are these really all necessary? especially the lines

getSubmitValue: function{
    let value = this.value;
    return value;
}

looks weird. submitValue does basically the same already?
(+ processrawvalue, which does nothing by defaut, +getRawvalue)

if not, a

getSubmitValue: function() {
      return this.value;
}

would be shorter (return this.value || []; for the array case)

IMHO having such fields defined inline can be ok, but
not if there are many of those, then i'd rather prefer
having the functionality split out into a class (or helper)
and reuse that

> +
> +	let realInvertMatch = Ext.create({
> +	    xtype: 'proxmoxcheckbox',
> +	    name: 'invert-match',
> +	    hidden: true,
> +	    deleteEmpty: !me.isCreate,
> +	});
> +
> +	let storeChanged = function(store) {
> +	    store.suspendEvent('datachanged');
> +
> +	    let matchFieldStmts = [];
> +	    let matchSeverityStmts = [];
> +	    let matchCalendarStmts = [];
> +	    let modeStmt = 'all';
> +	    let invertMatchStmt = false;
> +
> +	    store.each(function(model) {
> +		let type = model.get('type');
> +		let data = model.get('data');
> +
> +		switch (type) {
> +		    case 'match-field':
> +			matchFieldStmts.push(`${data.type}:${data.field}=${data.value}`);
> +			break;
> +		    case 'match-severity':
> +			matchSeverityStmts.push(data.value.join(','));
> +			break;
> +		    case 'match-calendar':
> +			matchCalendarStmts.push(data.value);
> +			break;
> +		    case 'mode':
> +			modeStmt = data.value;
> +			invertMatchStmt = data.invert;
> +			break;
> +		}
> +
> +		let [text, iconCls] = me.getNodeTextAndIcon(type, data);
> +		model.set({
> +		    text,
> +		    iconCls,
> +		});
> +	    });
> +
> +	    realMatchFields.suspendEvent('change');
> +	    realMatchFields.setValue(matchFieldStmts);
> +	    realMatchFields.resumeEvent('change');
> +
> +	    realMatchCalendar.suspendEvent('change');
> +	    realMatchCalendar.setValue(matchCalendarStmts);
> +	    realMatchCalendar.resumeEvent('change');
> +
> +	    realMode.suspendEvent('change');
> +	    realMode.setValue(modeStmt);
> +	    realMode.resumeEvent('change');
> +
> +	    realInvertMatch.suspendEvent('change');
> +	    realInvertMatch.setValue(invertMatchStmt);
> +	    realInvertMatch.resumeEvent('change');
> +
> +	    realMatchSeverity.suspendEvent('change');
> +	    realMatchSeverity.setValue(matchSeverityStmts);
> +	    realMatchSeverity.resumeEvent('change');

wouldn't it be much easier and shorter to do:

[realmatchfields,realmatchcalender,...].forEach(field => field.suspendEvent(...));

(and same for resume? or is the separation here necessary?

> +
> +	    store.resumeEvent('datachanged');
> +	};
> +
> +	realMatchFields.addListener('change', function(field, value) {
> +	    let parseMatchField = function(filter) {
> +		let [, type, matchedField, matchedValue] =
> +		    filter.match(/^(?:(regex|exact):)?([A-Za-z0-9_][A-Za-z0-9._-]*)=(.+)$/);
> +		if (type === undefined) {
> +		    type = "exact";
> +		}
> +		return {
> +		    type: 'match-field',
> +		    data: {
> +			type,
> +			field: matchedField,
> +			value: matchedValue,
> +		    },
> +		    leaf: true,
> +		};
> +	    };
> +
> +	    for (let node of treeStore.queryBy(
> +		record => record.get('type') === 'match-field',
> +	    ).getRange()) {
> +		node.remove(true);
> +	    }
> +
> +	    let records = value.map(parseMatchField);
> +
> +	    let rootNode = treeStore.getRootNode();
> +
> +	    for (let record of records) {
> +		rootNode.appendChild(record);
> +	    }
> +	});
> +
> +	realMatchSeverity.addListener('change', function(field, value) {
> +	    let parseSeverity = function(severities) {
> +		return {
> +		    type: 'match-severity',
> +		    data: {
> +			value: severities.split(','),
> +		    },
> +		    leaf: true,
> +		};
> +	    };
> +
> +	    for (let node of treeStore.queryBy(
> +		record => record.get('type') === 'match-severity').getRange()) {
> +		node.remove(true);
> +	    }
> +
> +	    let records = value.map(parseSeverity);
> +	    let rootNode = treeStore.getRootNode();
> +
> +	    for (let record of records) {
> +		rootNode.appendChild(record);
> +	    }
> +	});
> +
> +	realMatchCalendar.addListener('change', function(field, value) {
> +	    let parseCalendar = function(timespan) {
> +		return {
> +		    type: 'match-calendar',
> +		    data: {
> +			value: timespan,
> +		    },
> +		    leaf: true,
> +		};
> +	    };
> +
> +	    for (let node of treeStore.queryBy(
> +		record => record.get('type') === 'match-calendar').getRange()) {
> +		node.remove(true);
> +	    }
> +
> +	    let records = value.map(parseCalendar);
> +	    let rootNode = treeStore.getRootNode();
> +
> +	    for (let record of records) {
> +		rootNode.appendChild(record);
> +	    }
> +	});
> +
> +	realMode.addListener('change', function(field, value) {
> +	    let data = treeStore.getRootNode().get('data');
> +	    treeStore.getRootNode().set('data', {
> +		...data,
> +		value,
> +	    });
> +	});
> +
> +	realInvertMatch.addListener('change', function(field, value) {
> +	    let data = treeStore.getRootNode().get('data');
> +	    treeStore.getRootNode().set('data', {
> +		...data,
> +		invert: value,
> +	    });
> +	});


i think you should either define them inline with the fields,
or go all in to the controller method, and define them in a controller
and the fields declarative

> +
> +	treeStore.addListener('datachanged', storeChanged);
> +
> +	let treePanel = Ext.create({
> +	    xtype: 'treepanel',
> +	    store: treeStore,
> +	    minHeight: 300,
> +	    maxHeight: 300,
> +	    scrollable: true,
> +
> +	    bind: {
> +		selection: '{selectedRecord}',
> +	    },
> +	});
> +
> +	let addNode = function() {
> +	    let node = {
> +		type: 'match-field',
> +		data: {
> +		    type: 'exact',
> +		    field: '',
> +		    value: '',
> +		},
> +		leaf: true,
> +	    };
> +	    treeStore.getRootNode().appendChild(node);
> +	    treePanel.setSelection(treeStore.getRootNode().lastChild);
> +	};
> +
> +	let deleteNode = function() {
> +	    let selection = treePanel.getSelection();
> +	    for (let selected of selection) {
> +		if (!selected.isRoot()) {
> +		    selected.remove(true);
> +		}
> +	    }
> +	};
> +
> +	Ext.apply(me, {
> +	    items: [
> +		realMatchFields,
> +		realMode,
> +		realMatchSeverity,
> +		realInvertMatch,
> +		realMatchCalendar,
> +		treePanel,
> +		{
> +		    xtype: 'button',
> +		    margin: '5 5 5 0',
> +		    text: gettext('Add'),
> +		    iconCls: 'fa fa-plus-circle',
> +		    handler: addNode,
> +		},
> +		{
> +		    xtype: 'button',
> +		    margin: '5 5 5 0',
> +		    text: gettext('Remove'),
> +		    iconCls: 'fa fa-minus-circle',
> +		    handler: deleteNode,
> +		},
> +	    ],
> +	});
> +	me.callParent();
> +    },
> +});
> +
> +Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
> +    extend: 'Ext.panel.Panel',
> +    xtype: 'pmxNotificationMatchRuleSettings',
> +    border: false,
> +
> +    items: [
> +	{
> +	    xtype: 'proxmoxKVComboBox',
> +	    name: 'mode',
> +	    fieldLabel: gettext('Match if'),
> +	    allowBlank: false,
> +	    isFormField: false,
> +
> +	    comboItems: [
> +		['all', gettext('All rules match')],
> +		['any', gettext('Any rule matches')],
> +	    ],
> +	    bind: {
> +		hidden: '{!showMatchingMode}',
> +		disabled: '{!showMatchingMode}',
> +		value: '{rootMode}',
> +	    },
> +	},
> +	{
> +	    xtype: 'proxmoxcheckbox',
> +	    fieldLabel: gettext('Invert match'),
> +	    isFormField: false,
> +	    uncheckedValue: 0,
> +	    defaultValue: 0,
> +	    bind: {
> +		hidden: '{!showMatchingMode}',
> +		disabled: '{!showMatchingMode}',
> +		value: '{invertMatch}',
> +	    },
> +
> +	},
> +	{
> +	    xtype: 'proxmoxKVComboBox',
> +	    fieldLabel: gettext('Node type'),
> +	    isFormField: false,
> +	    allowBlank: false,
> +
> +	    bind: {
> +		value: '{nodeType}',
> +		hidden: '{!showMatcherType}',
> +		disabled: '{!showMatcherType}',
> +	    },
> +
> +	    comboItems: [
> +		['match-field', gettext('Match Field')],
> +		['match-severity', gettext('Match Severity')],
> +		['match-calendar', gettext('Match Calendar')],
> +	    ],
> +	},
> +	{
> +	    fieldLabel: 'Match Type',
> +	    xtype: 'proxmoxKVComboBox',
> +	    reference: 'type',
> +	    isFormField: false,
> +	    allowBlank: false,
> +	    submitValue: false,
> +
> +	    bind: {
> +		hidden: '{!typeIsMatchField}',
> +		disabled: '{!typeIsMatchField}',
> +		value: '{matchFieldType}',
> +	    },
> +
> +	    comboItems: [
> +		['exact', gettext('Exact')],
> +		['regex', gettext('Regex')],
> +	    ],
> +	},
> +	{
> +	    fieldLabel: gettext('Field'),
> +	    xtype: 'textfield',
> +	    isFormField: false,
> +	    submitValue: false,
> +	    bind: {
> +		hidden: '{!typeIsMatchField}',
> +		disabled: '{!typeIsMatchField}',
> +		value: '{matchFieldField}',
> +	    },
> +	},
> +	{
> +	    fieldLabel: gettext('Value'),
> +	    xtype: 'textfield',
> +	    isFormField: false,
> +	    submitValue: false,
> +	    allowBlank: false,
> +	    bind: {
> +		hidden: '{!typeIsMatchField}',
> +		disabled: '{!typeIsMatchField}',
> +		value: '{matchFieldValue}',
> +	    },
> +	},
> +	{
> +	    xtype: 'proxmoxKVComboBox',
> +	    fieldLabel: gettext('Severities to match'),
> +	    isFormField: false,
> +	    allowBlank: true,
> +	    multiSelect: true,
> +
> +	    bind: {
> +		value: '{matchSeverityValue}',
> +		hidden: '{!typeIsMatchSeverity}',
> +		disabled: '{!typeIsMatchSeverity}',
> +	    },
> +
> +	    comboItems: [
> +		['info', gettext('Info')],
> +		['notice', gettext('Notice')],
> +		['warning', gettext('Warning')],
> +		['error', gettext('Error')],
> +	    ],
> +	},
> +	{
> +	    xtype: 'proxmoxKVComboBox',
> +	    fieldLabel: gettext('Timespan to match'),
> +	    isFormField: false,
> +	    allowBlank: false,
> +	    editable: true,
> +	    displayField: 'key',
> +
> +	    bind: {
> +		value: '{matchCalendarValue}',
> +		hidden: '{!typeIsMatchCalendar}',
> +		disabled: '{!typeIsMatchCalender}',
> +	    },
> +
> +	    comboItems: [
> +		['mon 8-12', ''],
> +		['tue..fri,sun 0:00-23:59', ''],
> +	    ],
> +	},
> +    ],
> +});


wouldn't it make sense to have multiple panels depending on the type, so that the various
matchers are separated, and simply show only the one thats currently selected?