On 2024-07-26 14:22, Shannon Sterz wrote:
On Wed Jul 3, 2024 at 10:01 AM CEST, Aaron Lauterer wrote:
The new optional bridge_vids field allows to set that property via the
GUI. Since the backend needs to support it, the field needs to be
explicitly enabled.
For now, Proxmox VE (PVE) is the use case.
Signed-off-by: Aaron Lauterer <a.laute...@proxmox.com>
---
changes since v2:
* added validation code following how it is implemented in the API
src/node/NetworkEdit.js | 62 +++++++++++++++++++++++++++++++++++++++++
src/node/NetworkView.js | 5 ++++
2 files changed, 67 insertions(+)
diff --git a/src/node/NetworkEdit.js b/src/node/NetworkEdit.js
index 27c1baf..8c1b135 100644
--- a/src/node/NetworkEdit.js
+++ b/src/node/NetworkEdit.js
@@ -2,6 +2,9 @@ Ext.define('Proxmox.node.NetworkEdit', {
extend: 'Proxmox.window.Edit',
alias: ['widget.proxmoxNodeNetworkEdit'],
+ // Enable to show the VLAN ID field
+ bridge_set_vids: false,
+
initComponent: function() {
let me = this;
@@ -57,11 +60,67 @@ Ext.define('Proxmox.node.NetworkEdit', {
}
if (me.iftype === 'bridge') {
+ let vids = Ext.create('Ext.form.field.Text', {
+ fieldLabel: gettext('Bridge VIDS'),
+ name: 'bridge_vids',
+ emptyText: '2-4094',
+ disabled: true,
+ autoEl: {
+ tag: 'div',
+ 'data-qtip': gettext('Space-separated list of VLANs and
ranges, for example: 2 4 100-200'),
+ },
+ validator: function(value) {
+ if (!value) { return true; } // Empty
nit: our js style guide state that single line ifs should be avoided
+ let result = true;
+
+ let vid_list = value.split(' ');
+
+ let checkVid = function(tag) {
+ if (tag < 2 || tag > 4094) {
+ return `not a valid VLAN ID '${tag}'`;
+ }
+ return true;
+ };
+
+ for (const vid of vid_list) {
+ if (!vid) {
+ continue;
+ }
+ let res = vid.match(/^(\d+)([-](\d+))?$/i);
nit: see my comment for patch 2.
+ if (!res) {
+ return `not a valid VLAN configuration '${vid}'`;
+ }
+ let start = Number(res[1]);
+ let end = Number(res[3]);
+
+ if (start) {
+ result = checkVid(start);
+ if (result !== true) { return result; }
same here...
+ }
+ if (end) {
+ result = checkVid(end);
+ if (result !== true) { return result; }
and here. it might be more elegant to do this:
let invalidVid = function(tag) {
if (!isNaN(tag) && (tag < 2 || tag > 4094)) {
return `not a valid VLAN ID '${tag}'`;
}
return false;
};
// [..]
if (res = invalidVid(start)) {
return res;
}
if (res = invalidVid(end)) {
return res;
}
`Number(res[n])` should return `NaN` if `res[n]` is undefined, this
shouldn't be the case for start ever but is relevant for `end`.
Thanks for the overall review and these suggestions!
The above example won't work exactly like that, as eslint will throw an
error due to assignments inside the if-clause.
I opted for
res = invalidVid(start);
if (res) {
return res;
}
Not quite as succinct, but it does the same job and adheres to our
eslint rules.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
