[pmg-devel] applied: [PATCH pmg-gui v2] fix #3284: improve display & editing of `dnsbl_sites`

[Stoiko Ivanov <s.ivanov@proxmox.com>]

Thanks for tackling this, and the fast iteration!
gave it another spin on my testsetup - and it looked good to me.

applied!

On Mon, 22 Sep 2025 14:09:24 +0200
Dominik Csapak <d.csapak@proxmox.com> wrote:

> Improve the display of the dnsbl_sites property by splitting on our
> usual characters (',', ';', ' ') and put one entry per line.
> For this to better work, put the DNSBL options at the bottom.
> 
> To improve the editing of this setting, introduce a DNSBLSitesGrid,
> inspired from the tag color override grid from PVE. (There might be some
> code to be shared with that, but the most interesting parts are different:
> get/setValue, columns, etc.).
> 
> This allows to add/edit/remove single entries from the list much easier.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v1:
> * as discussed with Stoiko off-list: added emptytext to the 'filter' and
>   'weight' textfields
> 
>  js/DNSBLSitesField.js  | 207 +++++++++++++++++++++++++++++++++++++++++
>  js/MailProxyOptions.js |  46 ++++++---
>  js/Makefile            |   1 +
>  3 files changed, 242 insertions(+), 12 deletions(-)
>  create mode 100644 js/DNSBLSitesField.js
> 
> diff --git a/js/DNSBLSitesField.js b/js/DNSBLSitesField.js
> new file mode 100644
> index 0000000..c17de8b
> --- /dev/null
> +++ b/js/DNSBLSitesField.js
> @@ -0,0 +1,207 @@
> +Ext.define('PMG.form.DNSBLSitesGrid', {
> +    extend: 'Ext.grid.Panel',
> +    alias: 'widget.pmgDnsblSitesGrid',
> +
> +    mixins: ['Ext.form.field.Field'],
> +
> +    allowBlank: true,
> +    selectAll: false,
> +    isFormField: true,
> +    deleteEmpty: false,
> +    selModel: 'checkboxmodel',
> +
> +    config: {
> +        deleteEmpty: false,
> +    },
> +
> +    emptyText: gettext('No Sites defined'),
> +    viewConfig: {
> +        deferEmptyText: false,
> +    },
> +
> +    setValue: function (value) {
> +        let me = this;
> +        let sites = value ?? '';
> +        if (!sites) {
> +            me.getStore().removeAll();
> +            me.checkChange();
> +            return me;
> +        }
> +        let matcher = /^(.*?)(?:=(.*?))?(?:\*(.*))?$/;
> +        let entries = (sites.split(/[;, ]/) || [])
> +            .filter((s) => !!s)
> +            .map((entry) => {
> +                let [, site, filter, weight] = matcher.exec(entry);
> +                return {
> +                    site,
> +                    filter,
> +                    weight,
> +                };
> +            });
> +        me.getStore().setData(entries);
> +        me.checkChange();
> +        return me;
> +    },
> +
> +    getValue: function () {
> +        let me = this;
> +        let values = [];
> +        me.getStore().each((rec) => {
> +            let val = rec.data.site;
> +            if (rec.data.filter) {
> +                val += `=${rec.data.filter}`;
> +            }
> +            if (rec.data.weight) {
> +                val += `*${rec.data.weight}`;
> +            }
> +            values.push(val);
> +        });
> +        return values.join(';');
> +    },
> +
> +    getErrors: function (value) {
> +        let me = this;
> +        let emptySite = false;
> +        me.getStore().each((rec) => {
> +            if (!rec.data.site) {
> +                emptySite = true;
> +            }
> +        });
> +        let errors = [];
> +        if (emptySite) {
> +            errors.push(gettext('Site must not be empty.'));
> +        }
> +        return errors;
> +    },
> +
> +    // override framework function to implement deleteEmpty behaviour
> +    getSubmitData: function () {
> +        let me = this,
> +            data = null,
> +            val;
> +        if (!me.disabled && me.submitValue) {
> +            val = me.getValue();
> +            if (val !== null && val !== '') {
> +                data = {};
> +                data[me.getName()] = val;
> +            } else if (me.getDeleteEmpty()) {
> +                data = {};
> +                data.delete = me.getName();
> +            }
> +        }
> +        return data;
> +    },
> +
> +    controller: {
> +        xclass: 'Ext.app.ViewController',
> +
> +        addLine: function () {
> +            let me = this;
> +            me.getView().getStore().add({
> +                site: '',
> +                filter: '',
> +                weight: '',
> +            });
> +        },
> +
> +        removeSelection: function () {
> +            let me = this;
> +            let view = me.getView();
> +            let selection = view.getSelection();
> +            if (selection === undefined) {
> +                return;
> +            }
> +
> +            selection.forEach((sel) => {
> +                view.getStore().remove(sel);
> +            });
> +            view.checkChange();
> +        },
> +
> +        fieldChange: function (field, newValue, oldValue) {
> +            let me = this;
> +            let view = me.getView();
> +            let rec = field.getWidgetRecord();
> +            if (!rec) {
> +                return;
> +            }
> +            let column = field.getWidgetColumn();
> +            rec.set(column.dataIndex, newValue);
> +            view.checkChange();
> +        },
> +    },
> +
> +    tbar: [
> +        {
> +            text: gettext('Add'),
> +            handler: 'addLine',
> +        },
> +        {
> +            xtype: 'proxmoxButton',
> +            text: gettext('Remove'),
> +            handler: 'removeSelection',
> +            disabled: true,
> +        },
> +    ],
> +
> +    columns: [
> +        {
> +            header: gettext('Site'),
> +            dataIndex: 'site',
> +            xtype: 'widgetcolumn',
> +            widget: {
> +                xtype: 'proxmoxtextfield',
> +                isFormField: false,
> +                allowBlank: false,
> +                listeners: {
> +                    change: 'fieldChange',
> +                },
> +            },
> +            flex: 1,
> +        },
> +        {
> +            header: gettext('Filter'),
> +            xtype: 'widgetcolumn',
> +            flex: 1,
> +            dataIndex: 'filter',
> +            widget: {
> +                xtype: 'proxmoxtextfield',
> +                emptyText: Proxmox.Utils.noneText,
> +                allowBlank: true,
> +                isFormField: false,
> +                listeners: {
> +                    change: 'fieldChange',
> +                },
> +            },
> +        },
> +        {
> +            header: gettext('Weight'),
> +            xtype: 'widgetcolumn',
> +            flex: 1,
> +            dataIndex: 'weight',
> +            widget: {
> +                xtype: 'proxmoxintegerfield',
> +                emptyText: '1',
> +                allowBlank: true,
> +                isFormField: false,
> +                listeners: {
> +                    change: 'fieldChange',
> +                },
> +            },
> +        },
> +    ],
> +
> +    store: {
> +        listeners: {
> +            update: function () {
> +                this.commitChanges();
> +            },
> +        },
> +    },
> +
> +    initComponent: function () {
> +        let me = this;
> +        me.callParent();
> +        me.initField();
> +    },
> +});
> diff --git a/js/MailProxyOptions.js b/js/MailProxyOptions.js
> index 7d0d34e..26903fa 100644
> --- a/js/MailProxyOptions.js
> +++ b/js/MailProxyOptions.js
> @@ -19,18 +19,6 @@ Ext.define('PMG.MailProxyOptions', {
>  
>          me.add_boolean_row('helotests', gettext('SMTP HELO checks'));
>  
> -        me.add_text_row('dnsbl_sites', gettext('DNSBL Sites'), {
> -            deleteEmpty: true,
> -            defaultValue: Proxmox.Utils.noneText,
> -            renderer: Ext.htmlEncode,
> -        });
> -
> -        me.add_integer_row('dnsbl_threshold', gettext('DNSBL Threshold'), {
> -            deleteEmpty: true,
> -            defaultValue: 1,
> -            minValue: 0,
> -        });
> -
>          var render_verifyreceivers = function (value) {
>              if (value === undefined || value === '__default__') {
>                  return Proxmox.Utils.noText;
> @@ -107,6 +95,40 @@ Ext.define('PMG.MailProxyOptions', {
>  
>          me.add_boolean_row('before_queue_filtering', gettext('Before Queue Filtering'));
>  
> +        me.add_integer_row('dnsbl_threshold', gettext('DNSBL Threshold'), {
> +            deleteEmpty: true,
> +            defaultValue: 1,
> +            minValue: 0,
> +        });
> +
> +        me.rows.dnsbl_sites = {
> +            required: true,
> +            header: gettext('DNSBL Sites'),
> +            renderer: function () {
> +                return (me.getObjectValue('dnsbl_sites') ?? '')
> +                    .split(/[;, ]/)
> +                    .filter((s) => !!s)
> +                    .map((site) => Ext.htmlEncode(site))
> +                    .join('<br>');
> +            },
> +            editor: {
> +                xtype: 'proxmoxWindowEdit',
> +                subject: gettext('DNSBL Sites'),
> +                fieldDefaults: {
> +                    labelWidth: 100,
> +                },
> +                width: 600,
> +                height: 400,
> +                items: [
> +                    {
> +                        xtype: 'pmgDnsblSitesGrid',
> +                        name: 'dnsbl_sites',
> +                        deleteEmpty: true,
> +                    },
> +                ],
> +            },
> +        };
> +
>          var baseurl = '/config/mail';
>  
>          me.selModel = Ext.create('Ext.selection.RowModel', {});
> diff --git a/js/Makefile b/js/Makefile
> index 6e51b8c..2ffd44c 100644
> --- a/js/Makefile
> +++ b/js/Makefile
> @@ -51,6 +51,7 @@ JSSRC=							\
>  	PBSRemoteEdit.js				\
>  	PBSConfig.js					\
>  	SystemConfiguration.js				\
> +	DNSBLSitesField.js				\
>  	MailProxyRelaying.js				\
>  	MailProxyPorts.js				\
>  	MailProxyOptions.js				\



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