Merge pull request #1122 from LibreQoE/fix_device_editor

Fix shaped device edit buttons for numeric device IDs

src/rust/lqosd/src/node_manager/js_build/src/config/shaped_device_identity.mjs

@@ -0,0 +1,19 @@
+export function shapedDeviceIdFromElement(element) {
+    return element?.getAttribute("data-device-id") || "";
+}
+
+export function shapedDeviceIdMatches(row, deviceId) {
+    return String(row?.device_id ?? "") === String(deviceId ?? "");
+}
+
+export function shapedDeviceRowForId(rows, deviceId) {
+    return rows.find((row) => shapedDeviceIdMatches(row, deviceId));
+}
+
+export function handleShapedDeviceActionClick(event, action) {
+    event.preventDefault();
+    const deviceId = shapedDeviceIdFromElement(event.currentTarget);
+    if (deviceId.length > 0) {
+        action(deviceId);
+    }
+}

src/rust/lqosd/src/node_manager/js_build/src/config/shaped_device_identity.test.mjs

@@ -0,0 +1,103 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import {
+    handleShapedDeviceActionClick,
+    shapedDeviceIdFromElement,
+    shapedDeviceIdMatches,
+    shapedDeviceRowForId,
+} from "./shaped_device_identity.mjs";
+
+function actionEvent(deviceId) {
+    return {
+        prevented: false,
+        currentTarget: {
+            getAttribute(name) {
+                return name === "data-device-id" ? deviceId : null;
+            },
+        },
+        preventDefault() {
+            this.prevented = true;
+        },
+    };
+}
+
+test("reads numeric data-device-id values as strings", () => {
+    const element = {
+        getAttribute(name) {
+            return name === "data-device-id" ? "123" : null;
+        },
+    };
+
+    assert.equal(shapedDeviceIdFromElement(element), "123");
+});
+
+test("preserves leading zeros in data-device-id values", () => {
+    const element = {
+        getAttribute(name) {
+            return name === "data-device-id" ? "00123" : null;
+        },
+    };
+
+    assert.equal(shapedDeviceIdFromElement(element), "00123");
+});
+
+test("matches shaped device rows by string-stable device id", () => {
+    assert.equal(shapedDeviceIdMatches({ device_id: "00123" }, "00123"), true);
+    assert.equal(shapedDeviceIdMatches({ device_id: 123 }, "123"), true);
+    assert.equal(shapedDeviceIdMatches({ device_id: "123" }, "00123"), false);
+});
+
+test("finds shaped device rows by string-stable device id", () => {
+    const rows = [
+        { device_id: "123", device_name: "numeric" },
+        { device_id: "00123", device_name: "leading zero" },
+    ];
+
+    assert.equal(shapedDeviceRowForId(rows, "123")?.device_name, "numeric");
+    assert.equal(shapedDeviceRowForId(rows, 123)?.device_name, "numeric");
+    assert.equal(shapedDeviceRowForId(rows, "00123")?.device_name, "leading zero");
+    assert.equal(shapedDeviceRowForId(rows, "456"), undefined);
+});
+
+test("passes clicked numeric device ids to action callbacks as strings", () => {
+    const event = actionEvent("123");
+    let received = null;
+
+    handleShapedDeviceActionClick(event, (deviceId) => {
+        received = deviceId;
+    });
+
+    assert.equal(event.prevented, true);
+    assert.equal(received, "123");
+});
+
+test("click action can select numeric and leading-zero rows", () => {
+    const rows = [
+        { device_id: "123", device_name: "numeric" },
+        { device_id: "00123", device_name: "leading zero" },
+    ];
+    const selected = [];
+
+    ["123", "00123"].forEach((deviceId) => {
+        const event = actionEvent(deviceId);
+        handleShapedDeviceActionClick(event, (clickedDeviceId) => {
+            selected.push(shapedDeviceRowForId(rows, clickedDeviceId)?.device_name);
+        });
+        assert.equal(event.prevented, true);
+    });
+
+    assert.deepEqual(selected, ["numeric", "leading zero"]);
+});
+
+test("does not invoke action callbacks for empty device ids", () => {
+    const event = actionEvent("");
+    let called = false;
+
+    handleShapedDeviceActionClick(event, () => {
+        called = true;
+    });
+
+    assert.equal(event.prevented, true);
+    assert.equal(called, false);
+});

src/rust/lqosd/src/node_manager/js_build/src/config_devices.js

@@ -10,6 +10,10 @@ import {
     updateShapedDevice,
     validNodeList,
 } from "./config/config_helper";
+import {
+    handleShapedDeviceActionClick,
+    shapedDeviceRowForId,
+} from "./config/shaped_device_identity.mjs";
 import { parseIpInput } from "./config/shaped_device_wire.mjs";
 
 let current_rows = [];
@@ -548,7 +552,7 @@ function collectModalDevice() {
 }
 
 function findCurrentRow(deviceId) {
-    return current_rows.find((row) => row.device_id === deviceId);
+    return shapedDeviceRowForId(current_rows, deviceId);
 }
 
 function openEditModal(deviceId) {
@@ -743,19 +747,11 @@ function start() {
     });
 
     $("#sdTableContainer").on("click", ".sd-edit", (event) => {
-        event.preventDefault();
-        const deviceId = $(event.currentTarget).data("device-id");
-        if (typeof deviceId === "string" && deviceId.length > 0) {
-            openEditModal(deviceId);
-        }
+        handleShapedDeviceActionClick(event, openEditModal);
     });
 
     $("#sdTableContainer").on("click", ".sd-delete", (event) => {
-        event.preventDefault();
-        const deviceId = $(event.currentTarget).data("device-id");
-        if (typeof deviceId === "string" && deviceId.length > 0) {
-            deleteDevice(deviceId);
-        }
+        handleShapedDeviceActionClick(event, deleteDevice);
     });
 
     $("#btnAddDevice").on("click", (event) => {

src/rust/lqosd/src/node_manager/js_build/test-node-manager.sh

@@ -14,6 +14,7 @@ node --test \
   "${SCRIPT_DIR}/src/circuit_rate_display.test.mjs" \
   "${SCRIPT_DIR}/src/circuit_packet_capture_dom.test.mjs" \
   "${SCRIPT_DIR}/src/tree_limit_reason.test.mjs" \
+  "${SCRIPT_DIR}/src/config/shaped_device_identity.test.mjs" \
   "${SCRIPT_DIR}/src/config/shaped_device_wire.test.mjs" \
   "${SCRIPT_DIR}/src/config/config_interface_mtu.test.mjs" \
   "${SCRIPT_DIR}/src/config/network_mode_storage.test.mjs" \