all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH manager] ui: sdn: fabrics: node edit: prevent adding multiple ipv6 columns
@ 2026-05-20 13:12 Dominik Csapak
  2026-05-20 13:17 ` Dominik Csapak
  2026-05-20 13:26 ` superseded: " Dominik Csapak
  0 siblings, 2 replies; 3+ messages in thread
From: Dominik Csapak @ 2026-05-20 13:12 UTC (permalink / raw)
  To: pve-devel

modifying a class member is always dangerous, since it modifies the
object on the original class prototype, so pushing into it happens on
every instance creation. This means every time 'Add Node' is clicked
for a fabric with ipv6 support another ipv6 column is added.

To prevent this, create the list of common columns in initComponent
and push into that instead.

Slightly changes the handling of additionalColumns so we don't have to
copy the common columns with 'concat'.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/sdn/fabrics/InterfacePanel.js | 126 +++++++++++----------
 1 file changed, 65 insertions(+), 61 deletions(-)

diff --git a/www/manager6/sdn/fabrics/InterfacePanel.js b/www/manager6/sdn/fabrics/InterfacePanel.js
index 5632104a..30c43ddf 100644
--- a/www/manager6/sdn/fabrics/InterfacePanel.js
+++ b/www/manager6/sdn/fabrics/InterfacePanel.js
@@ -15,65 +15,6 @@ Ext.define('PVE.sdn.Fabric.InterfacePanel', {
 
     maxHeight: 500,
 
-    commonColumns: [
-        {
-            text: gettext('Status'),
-            dataIndex: 'status',
-            width: 30,
-            renderer: function (value, metaData, record) {
-                let me = this;
-
-                let warning;
-                let nodeInterface = me.nodeInterfaces[record.data.name];
-
-                if (!nodeInterface) {
-                    warning = gettext('Interface does not exist on node');
-                } else if (
-                    (nodeInterface.ip && record.data.ip) ||
-                    (nodeInterface.ip6 && record.data.ip6)
-                ) {
-                    warning = gettext(
-                        'Interface already has an address configured in /etc/network/interfaces',
-                    );
-                } else if (nodeInterface.ip || nodeInterface.ip6) {
-                    warning = gettext(
-                        'Configure the IP in the fabric, instead of /etc/network/interfaces',
-                    );
-                }
-
-                if (warning) {
-                    metaData.tdAttr = `data-qtip="${Ext.htmlEncode(Ext.htmlEncode(warning))}"`;
-                    return `<i class="fa warning fa-warning"></i>`;
-                }
-
-                return '';
-            },
-        },
-        {
-            text: gettext('Name'),
-            dataIndex: 'name',
-            flex: 2,
-        },
-        {
-            text: gettext('Type'),
-            dataIndex: 'type',
-            flex: 1,
-        },
-        {
-            text: gettext('IP'),
-            xtype: 'widgetcolumn',
-            dataIndex: 'ip',
-            flex: 1,
-            widget: {
-                xtype: 'proxmoxtextfield',
-                isFormField: false,
-                bind: {
-                    disabled: '{record.isDisabled}',
-                },
-            },
-        },
-    ],
-
     additionalColumns: [],
 
     controller: {
@@ -110,8 +51,67 @@ Ext.define('PVE.sdn.Fabric.InterfacePanel', {
     initComponent: function () {
         let me = this;
 
+        let columns = [
+            {
+                text: gettext('Status'),
+                dataIndex: 'status',
+                width: 30,
+                renderer: function (value, metaData, record) {
+                    let me = this;
+
+                    let warning;
+                    let nodeInterface = me.nodeInterfaces[record.data.name];
+
+                    if (!nodeInterface) {
+                        warning = gettext('Interface does not exist on node');
+                    } else if (
+                        (nodeInterface.ip && record.data.ip) ||
+                        (nodeInterface.ip6 && record.data.ip6)
+                    ) {
+                        warning = gettext(
+                            'Interface already has an address configured in /etc/network/interfaces',
+                        );
+                    } else if (nodeInterface.ip || nodeInterface.ip6) {
+                        warning = gettext(
+                            'Configure the IP in the fabric, instead of /etc/network/interfaces',
+                        );
+                    }
+
+                    if (warning) {
+                        metaData.tdAttr = `data-qtip="${Ext.htmlEncode(Ext.htmlEncode(warning))}"`;
+                        return `<i class="fa warning fa-warning"></i>`;
+                    }
+
+                    return '';
+                },
+            },
+            {
+                text: gettext('Name'),
+                dataIndex: 'name',
+                flex: 2,
+            },
+            {
+                text: gettext('Type'),
+                dataIndex: 'type',
+                flex: 1,
+            },
+            {
+                text: gettext('IP'),
+                xtype: 'widgetcolumn',
+                dataIndex: 'ip',
+                flex: 1,
+                widget: {
+                    xtype: 'proxmoxtextfield',
+                    isFormField: false,
+                    bind: {
+                        disabled: '{record.isDisabled}',
+                    },
+                },
+            },
+        ];
+
         if (me.hasIpv6Support) {
-            me.commonColumns.push({
+            columns.push({
                 text: gettext('IPv6'),
                 xtype: 'widgetcolumn',
                 dataIndex: 'ip6',
@@ -126,6 +126,10 @@ Ext.define('PVE.sdn.Fabric.InterfacePanel', {
             });
         }
 
+        if (me.additionalColumns.length > 0) {
+            columns.push(...me.additionalColumns);
+        }
+
         Ext.apply(me, {
             store: Ext.create('Ext.data.Store', {
                 model: 'Pve.sdn.Interface',
@@ -134,7 +138,7 @@ Ext.define('PVE.sdn.Fabric.InterfacePanel', {
                     direction: 'ASC',
                 },
             }),
-            columns: me.commonColumns.concat(me.additionalColumns),
+            columns,
         });
 
         me.callParent();
-- 
2.47.3





^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH manager] ui: sdn: fabrics: node edit: prevent adding multiple ipv6 columns
  2026-05-20 13:12 [PATCH manager] ui: sdn: fabrics: node edit: prevent adding multiple ipv6 columns Dominik Csapak
@ 2026-05-20 13:17 ` Dominik Csapak
  2026-05-20 13:26 ` superseded: " Dominik Csapak
  1 sibling, 0 replies; 3+ messages in thread
From: Dominik Csapak @ 2026-05-20 13:17 UTC (permalink / raw)
  To: pve-devel

disregard this, we access the common columns from elsewhere too and that
breaks with this patch. sending a v2 shortly

On 5/20/26 3:13 PM, Dominik Csapak wrote:
> modifying a class member is always dangerous, since it modifies the
> object on the original class prototype, so pushing into it happens on
> every instance creation. This means every time 'Add Node' is clicked
> for a fabric with ipv6 support another ipv6 column is added.
> 
> To prevent this, create the list of common columns in initComponent
> and push into that instead.
> 
> Slightly changes the handling of additionalColumns so we don't have to
> copy the common columns with 'concat'.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>   www/manager6/sdn/fabrics/InterfacePanel.js | 126 +++++++++++----------
>   1 file changed, 65 insertions(+), 61 deletions(-)
> 
> diff --git a/www/manager6/sdn/fabrics/InterfacePanel.js b/www/manager6/sdn/fabrics/InterfacePanel.js
> index 5632104a..30c43ddf 100644
> --- a/www/manager6/sdn/fabrics/InterfacePanel.js
> +++ b/www/manager6/sdn/fabrics/InterfacePanel.js
> @@ -15,65 +15,6 @@ Ext.define('PVE.sdn.Fabric.InterfacePanel', {
>   
>       maxHeight: 500,
>   
> -    commonColumns: [
> -        {
> -            text: gettext('Status'),
> -            dataIndex: 'status',
> -            width: 30,
> -            renderer: function (value, metaData, record) {
> -                let me = this;
> -
> -                let warning;
> -                let nodeInterface = me.nodeInterfaces[record.data.name];
> -
> -                if (!nodeInterface) {
> -                    warning = gettext('Interface does not exist on node');
> -                } else if (
> -                    (nodeInterface.ip && record.data.ip) ||
> -                    (nodeInterface.ip6 && record.data.ip6)
> -                ) {
> -                    warning = gettext(
> -                        'Interface already has an address configured in /etc/network/interfaces',
> -                    );
> -                } else if (nodeInterface.ip || nodeInterface.ip6) {
> -                    warning = gettext(
> -                        'Configure the IP in the fabric, instead of /etc/network/interfaces',
> -                    );
> -                }
> -
> -                if (warning) {
> -                    metaData.tdAttr = `data-qtip="${Ext.htmlEncode(Ext.htmlEncode(warning))}"`;
> -                    return `<i class="fa warning fa-warning"></i>`;
> -                }
> -
> -                return '';
> -            },
> -        },
> -        {
> -            text: gettext('Name'),
> -            dataIndex: 'name',
> -            flex: 2,
> -        },
> -        {
> -            text: gettext('Type'),
> -            dataIndex: 'type',
> -            flex: 1,
> -        },
> -        {
> -            text: gettext('IP'),
> -            xtype: 'widgetcolumn',
> -            dataIndex: 'ip',
> -            flex: 1,
> -            widget: {
> -                xtype: 'proxmoxtextfield',
> -                isFormField: false,
> -                bind: {
> -                    disabled: '{record.isDisabled}',
> -                },
> -            },
> -        },
> -    ],
> -
>       additionalColumns: [],
>   
>       controller: {
> @@ -110,8 +51,67 @@ Ext.define('PVE.sdn.Fabric.InterfacePanel', {
>       initComponent: function () {
>           let me = this;
>   
> +        let columns = [
> +            {
> +                text: gettext('Status'),
> +                dataIndex: 'status',
> +                width: 30,
> +                renderer: function (value, metaData, record) {
> +                    let me = this;
> +
> +                    let warning;
> +                    let nodeInterface = me.nodeInterfaces[record.data.name];
> +
> +                    if (!nodeInterface) {
> +                        warning = gettext('Interface does not exist on node');
> +                    } else if (
> +                        (nodeInterface.ip && record.data.ip) ||
> +                        (nodeInterface.ip6 && record.data.ip6)
> +                    ) {
> +                        warning = gettext(
> +                            'Interface already has an address configured in /etc/network/interfaces',
> +                        );
> +                    } else if (nodeInterface.ip || nodeInterface.ip6) {
> +                        warning = gettext(
> +                            'Configure the IP in the fabric, instead of /etc/network/interfaces',
> +                        );
> +                    }
> +
> +                    if (warning) {
> +                        metaData.tdAttr = `data-qtip="${Ext.htmlEncode(Ext.htmlEncode(warning))}"`;
> +                        return `<i class="fa warning fa-warning"></i>`;
> +                    }
> +
> +                    return '';
> +                },
> +            },
> +            {
> +                text: gettext('Name'),
> +                dataIndex: 'name',
> +                flex: 2,
> +            },
> +            {
> +                text: gettext('Type'),
> +                dataIndex: 'type',
> +                flex: 1,
> +            },
> +            {
> +                text: gettext('IP'),
> +                xtype: 'widgetcolumn',
> +                dataIndex: 'ip',
> +                flex: 1,
> +                widget: {
> +                    xtype: 'proxmoxtextfield',
> +                    isFormField: false,
> +                    bind: {
> +                        disabled: '{record.isDisabled}',
> +                    },
> +                },
> +            },
> +        ];
> +
>           if (me.hasIpv6Support) {
> -            me.commonColumns.push({
> +            columns.push({
>                   text: gettext('IPv6'),
>                   xtype: 'widgetcolumn',
>                   dataIndex: 'ip6',
> @@ -126,6 +126,10 @@ Ext.define('PVE.sdn.Fabric.InterfacePanel', {
>               });
>           }
>   
> +        if (me.additionalColumns.length > 0) {
> +            columns.push(...me.additionalColumns);
> +        }
> +
>           Ext.apply(me, {
>               store: Ext.create('Ext.data.Store', {
>                   model: 'Pve.sdn.Interface',
> @@ -134,7 +138,7 @@ Ext.define('PVE.sdn.Fabric.InterfacePanel', {
>                       direction: 'ASC',
>                   },
>               }),
> -            columns: me.commonColumns.concat(me.additionalColumns),
> +            columns,
>           });
>   
>           me.callParent();





^ permalink raw reply	[flat|nested] 3+ messages in thread

* superseded: [PATCH manager] ui: sdn: fabrics: node edit: prevent adding multiple ipv6 columns
  2026-05-20 13:12 [PATCH manager] ui: sdn: fabrics: node edit: prevent adding multiple ipv6 columns Dominik Csapak
  2026-05-20 13:17 ` Dominik Csapak
@ 2026-05-20 13:26 ` Dominik Csapak
  1 sibling, 0 replies; 3+ messages in thread
From: Dominik Csapak @ 2026-05-20 13:26 UTC (permalink / raw)
  To: pve-devel

superseded by v2:
https://lore.proxmox.com/pve-devel/20260520132533.3060077-1-d.csapak@proxmox.com/T/#u




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-20 13:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-20 13:12 [PATCH manager] ui: sdn: fabrics: node edit: prevent adding multiple ipv6 columns Dominik Csapak
2026-05-20 13:17 ` Dominik Csapak
2026-05-20 13:26 ` superseded: " Dominik Csapak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal