From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 2EEFBEA8A for ; Wed, 19 Jul 2023 15:26:08 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 07E90808E for ; Wed, 19 Jul 2023 15:25:38 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 19 Jul 2023 15:25:36 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 9FC2B41055; Wed, 19 Jul 2023 15:25:36 +0200 (CEST) Message-ID: Date: Wed, 19 Jul 2023 15:25:34 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Proxmox VE development discussion , Lukas Wagner References: <20230717150051.710464-1-l.wagner@proxmox.com> <20230717150051.710464-62-l.wagner@proxmox.com> From: Dominik Csapak In-Reply-To: <20230717150051.710464-62-l.wagner@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.016 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [data.name, me.name] Subject: Re: [pve-devel] [PATCH v3 proxmox-widget-toolkit 61/66] notification: add gui for sendmail notification endpoints X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Jul 2023 13:26:08 -0000 some comments/nits inline: On 7/17/23 17:00, Lukas Wagner wrote: > Signed-off-by: Lukas Wagner > --- > src/Makefile | 4 + > src/Schema.js | 8 ++ > src/data/model/NotificationConfig.js | 8 ++ > src/panel/NotificationConfigView.js | 192 +++++++++++++++++++++++++++ > src/panel/SendmailEditPanel.js | 140 +++++++++++++++++++ > src/window/EndpointEditBase.js | 50 +++++++ > 6 files changed, 402 insertions(+) > create mode 100644 src/data/model/NotificationConfig.js > create mode 100644 src/panel/NotificationConfigView.js > create mode 100644 src/panel/SendmailEditPanel.js > create mode 100644 src/window/EndpointEditBase.js > > diff --git a/src/Makefile b/src/Makefile > index baa90ec..e83038f 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -22,6 +22,7 @@ JSSRC= \ > data/ObjectStore.js \ > data/RRDStore.js \ > data/TimezoneStore.js \ > + data/model/NotificationConfig.js \ > data/model/Realm.js \ > data/model/Certificates.js \ > data/model/ACME.js \ > @@ -59,6 +60,7 @@ JSSRC= \ > panel/InfoWidget.js \ > panel/LogView.js \ > panel/NodeInfoRepoStatus.js \ > + panel/NotificationConfigView.js \ > panel/JournalView.js \ > panel/PermissionView.js \ > panel/PruneKeepPanel.js \ > @@ -68,6 +70,7 @@ JSSRC= \ > panel/ACMEAccount.js \ > panel/ACMEPlugin.js \ > panel/ACMEDomains.js \ > + panel/SendmailEditPanel.js \ > panel/StatusView.js \ > panel/TfaView.js \ > panel/NotesView.js \ > @@ -83,6 +86,7 @@ JSSRC= \ > window/ACMEAccount.js \ > window/ACMEPluginEdit.js \ > window/ACMEDomains.js \ > + window/EndpointEditBase.js \ > window/FileBrowser.js \ > window/AuthEditBase.js \ > window/AuthEditOpenId.js \ > diff --git a/src/Schema.js b/src/Schema.js > index b247b1e..99bb3fa 100644 > --- a/src/Schema.js > +++ b/src/Schema.js > @@ -37,6 +37,14 @@ Ext.define('Proxmox.Schema', { // a singleton > } > }, > > + notificationEndpointTypes: { > + sendmail: { > + name: gettext('Sendmail'), > + ipanel: 'pmxSendmailEditPanel', > + iconCls: 'fa-envelope-o', > + }, > + }, > + > pxarFileTypes: { > b: { icon: 'cube', label: gettext('Block Device') }, > c: { icon: 'tty', label: gettext('Character Device') }, > diff --git a/src/data/model/NotificationConfig.js b/src/data/model/NotificationConfig.js > new file mode 100644 > index 0000000..c2ce843 > --- /dev/null > +++ b/src/data/model/NotificationConfig.js > @@ -0,0 +1,8 @@ > +Ext.define('proxmox-notification-endpoints', { > + extend: 'Ext.data.Model', > + fields: ['name', 'type', 'comment'], > + proxy: { > + type: 'proxmox', > + }, > + idProperty: 'name', > +}); > diff --git a/src/panel/NotificationConfigView.js b/src/panel/NotificationConfigView.js > new file mode 100644 > index 0000000..f6e6a8b > --- /dev/null > +++ b/src/panel/NotificationConfigView.js > @@ -0,0 +1,192 @@ > +Ext.define('Proxmox.panel.NotificationConfigView', { > + extend: 'Ext.panel.Panel', > + alias: 'widget.pmxNotificationConfigView', > + mixins: ['Proxmox.Mixin.CBind'], > + layout: { > + type: 'border', > + }, > + > + items: [ > + { > + region: 'center', > + border: false, > + xtype: 'pmxNotificationEndpointView', > + cbind: { > + baseUrl: '{baseUrl}', > + }, > + }, > + ], > +}); > + > +Ext.define('Proxmox.panel.NotificationEndpointView', { > + extend: 'Ext.grid.Panel', > + alias: 'widget.pmxNotificationEndpointView', > + > + title: gettext('Notification Targets'), > + > + controller: { > + xclass: 'Ext.app.ViewController', > + > + openEditWindow: function(endpointType, endpoint) { > + let me = this; > + > + if (endpoint === 'mail-to-root') { > + return; > + } > + > + Ext.create('Proxmox.window.EndpointEditBase', { > + baseUrl: me.getView().baseUrl, > + type: endpointType, > + > + name: endpoint, > + listeners: { > + destroy: () => me.reload(), > + }, > + }).show(); you could use 'autoShow: true' instead > + }, > + > + openEditForSelectedItem: function() { > + let me = this; > + let view = me.getView(); > + > + let selection = view.getSelection(); > + if (selection.length < 1) return; this is against your style guide, please don't use single line if statements. > + let endpointName = selection[0].data.name; > + let type = selection[0].data.type; > + > + me.openEditWindow(type, endpointName); for this, you don't really need to extract it into seperate variables, simply call it with me.openEditWindow(selection[0].data.type, selection[0].data.name); should still fit in one line ;) > + }, > + > + reload: function() { > + let me = this; > + let view = me.getView(); > + view.getStore().rstore.load(); > + }, > + > + testEndpoint: function() { > + let me = this; > + let view = me.getView(); > + > + let selection = view.getSelection(); > + if (selection.length < 1) return; again style issue > + let target = selection[0].data.name; > + > + Ext.Msg.confirm( > + gettext("Notification Target Test"), > + gettext(`Do you want to send a test notification to '${target}'?`), > + function(decision) { > + if (decision !== "yes") { > + return; > + } > + > + Proxmox.Utils.API2Request({ > + method: 'POST', > + url: `${view.baseUrl}/targets/${target}/test`, > + > + success: function(response, opt) { > + Ext.Msg.show({ > + title: gettext('Notification Target Test'), > + message: gettext(`Sent test notification to '${target}'.`), > + buttons: Ext.Msg.OK, > + icon: Ext.Msg.INFO, > + }); > + }, > + autoErrorAlert: true, > + }); > + }); > + }, > + }, > + > + listeners: { > + itemdblclick: 'openEditForSelectedItem', > + activate: 'reload', > + }, > + > + emptyText: gettext('No notification targets configured'), > + > + columns: [ > + { > + dataIndex: 'name', > + text: gettext('Target Name'), > + renderer: Ext.String.htmlEncode, > + flex: 1, > + }, > + { > + dataIndex: 'type', > + text: gettext('Type'), > + renderer: Ext.String.htmlEncode, > + flex: 1, > + }, > + { > + dataIndex: 'comment', > + text: gettext('Comment'), > + renderer: Ext.String.htmlEncode, > + flex: 1, > + }, > + ], > + > + store: { > + type: 'diff', > + autoDestroy: true, > + autoDestroyRstore: true, > + rstore: { > + type: 'update', > + storeid: 'proxmox-notification-endpoints', > + model: 'proxmox-notification-endpoints', > + autoStart: true, > + }, > + sorters: 'name', > + }, > + > + initComponent: function() { > + let me = this; > + > + let menuItems = []; > + for (const [endpointType, config] of Object.entries( > + Proxmox.Schema.notificationEndpointTypes).sort()) { > + menuItems.push({ > + text: config.name, > + iconCls: 'fa fa-fw ' + (config.iconCls || 'fa-bell-o'), > + handler: () => me.controller.openEditWindow(endpointType), > + }); > + } > + > + Ext.apply(me, { > + tbar: [ > + { > + text: gettext('Add'), > + menu: menuItems, > + }, > + { > + xtype: 'proxmoxButton', > + text: gettext('Modify'), > + handler: 'openEditForSelectedItem', > + enableFn: rec => rec.data.name !== 'mail-to-root', > + disabled: true, > + }, > + { > + xtype: 'proxmoxStdRemoveButton', > + callback: 'reload', > + enableFn: rec => rec.data.name !== 'mail-to-root', > + getUrl: function(rec) { > + if (rec.data.type === 'group') { > + return `${me.baseUrl}/groups/${rec.getId()}`; > + } > + > + return `${me.baseUrl}/endpoints/${rec.data.type}/${rec.getId()}`; > + }, > + }, > + '-', > + { > + xtype: 'proxmoxButton', > + text: gettext('Test'), > + handler: 'testEndpoint', > + disabled: true, > + }, > + ], > + }); > + > + me.callParent(); > + me.store.rstore.proxy.setUrl(`/api2/json/${me.baseUrl}/targets`); would it maybe nicer in this case to have the store defined in the initcomponent before the callParent to be able to inject it there? no hard feelings though > + }, > +}); > diff --git a/src/panel/SendmailEditPanel.js b/src/panel/SendmailEditPanel.js > new file mode 100644 > index 0000000..9444a8c > --- /dev/null > +++ b/src/panel/SendmailEditPanel.js > @@ -0,0 +1,140 @@ > +Ext.define('Proxmox.panel.SendmailEditPanel', { > + extend: 'Proxmox.panel.InputPanel', > + xtype: 'pmxSendmailEditPanel', > + mixins: ['Proxmox.Mixin.CBind'], > + > + type: 'sendmail', > + > + columnT: [ if you only need need one column, why not simply use items/advancedItems and increase the width of the window/inputpanel? > + { > + xtype: 'pmxDisplayEditField', > + name: 'name', > + cbind: { > + value: '{name}', > + editable: '{isCreate}', > + }, > + fieldLabel: gettext('Endpoint Name'), > + allowBlank: false, > + }, > + { > + xtype: 'pmxUserSelector', > + name: 'mailto-user', > + reference: 'mailto-user', > + multiSelect: true, > + allowBlank: true, > + editable: false, > + skipEmptyText: true, > + fieldLabel: gettext('User(s)'), > + cbind: { > + deleteEmpty: '{!isCreate}', > + }, > + validator: function(value) { > + let up = this.up('pmxSendmailEditPanel'); > + let other = up.down('[name=mailto]'); > + > + if (!value && !other.getValue()) { > + return gettext('Either mailto or mailto-user must be set'); > + } > + > + return true; > + }, the validator/gettext could be factored out since it's basically the same as below, but it's ok this way if thats too much work > + listConfig: { > + width: 600, > + columns: [ > + { > + header: gettext('User'), > + sortable: true, > + dataIndex: 'userid', > + renderer: Ext.String.htmlEncode, > + flex: 1, > + }, > + { > + header: gettext('E-Mail'), > + sortable: true, > + dataIndex: 'email', > + renderer: Ext.String.htmlEncode, > + flex: 1, > + }, > + { > + header: gettext('Comment'), > + sortable: false, > + dataIndex: 'comment', > + renderer: Ext.String.htmlEncode, > + flex: 1, > + }, > + ], > + }, > + }, > + { > + xtype: 'proxmoxtextfield', > + fieldLabel: gettext('Additional Recipient(s)'), > + name: 'mailto', > + reference: 'mailto', > + allowBlank: true, > + cbind: { > + deleteEmpty: '{!isCreate}', > + }, > + autoEl: { > + tag: 'div', > + 'data-qtip': gettext( > + 'Multiple recipients must be separated by spaces, commas or semicolons', > + ), > + }, > + validator: function(value) { > + let up = this.up('pmxSendmailEditPanel'); > + let other = up.down('[name=mailto-user]'); > + > + if (!value && !other.getValue().length) { > + return gettext('Either mailto or mailto-user must be set'); > + } > + > + return true; > + }, > + }, > + ], > + > + column1: [], > + > + column2: [], > + > + columnB: [ why in columnB instead of columnT if it's the only one in use? > + { > + xtype: 'proxmoxtextfield', > + name: 'comment', > + fieldLabel: gettext('Comment'), > + cbind: { > + deleteEmpty: '{!isCreate}', > + }, > + }, > + ], > + advancedColumnB: [ > + { > + xtype: 'proxmoxtextfield', > + fieldLabel: gettext('Author'), > + name: 'author', > + allowBlank: true, > + emptyText: gettext('Proxmox VE'), > + cbind: { > + deleteEmpty: '{!isCreate}', > + }, > + }, > + { > + xtype: 'proxmoxtextfield', > + fieldLabel: gettext('From Address'), > + name: 'from-address', > + allowBlank: true, > + emptyText: gettext('Defaults to datacenter configuration, or root@$hostname'), > + cbind: { > + deleteEmpty: '{!isCreate}', > + }, > + }, > + ], > + > + onGetValues: (values) => { > + if (values.mailto) { > + values.mailto = values.mailto.split(/[\s,;]+/); > + } > + return values; > + }, > +}); > + > diff --git a/src/window/EndpointEditBase.js b/src/window/EndpointEditBase.js > new file mode 100644 > index 0000000..81e5951 > --- /dev/null > +++ b/src/window/EndpointEditBase.js > @@ -0,0 +1,50 @@ > +Ext.define('Proxmox.window.EndpointEditBase', { > + extend: 'Proxmox.window.Edit', > + > + isAdd: true, > + > + fieldDefaults: { > + labelWidth: 120, > + }, > + > + initComponent: function() { > + let me = this; > + > + me.isCreate = !me.name; > + > + if (!me.baseUrl) { > + throw "baseUrl not set"; > + } > + > + me.url = `/api2/extjs${me.baseUrl}/endpoints/${me.type}`; > + > + if (me.isCreate) { > + me.method = 'POST'; > + } else { > + me.url += `/${me.name}`; > + me.method = 'PUT'; > + } > + > + let endpointConfig = Proxmox.Schema.notificationEndpointTypes[me.type]; > + if (!endpointConfig) { > + throw 'unknown endpoint type'; > + } > + > + me.subject = endpointConfig.name; > + > + Ext.apply(me, { > + items: [{ > + name: me.name, > + xtype: endpointConfig.ipanel, > + isCreate: me.isCreate, > + type: me.type, > + }], > + }); > + > + me.callParent(); > + > + if (!me.isCreate) { > + me.load(); > + } > + }, > +});