diff --git a/.github/skills/code-standards/SKILL.md b/.github/skills/code-standards/SKILL.md index 37e6932b6..faf583d6f 100644 --- a/.github/skills/code-standards/SKILL.md +++ b/.github/skills/code-standards/SKILL.md @@ -74,6 +74,18 @@ Use sanitizers from `server/helper.py` before storing user input. MAC addresses - Everything is already writable - If permissions needed, fix `.devcontainer/scripts/setup.sh` +## Test Helpers — No Duplicate Mocks + +Reuse shared mocks and factories from `test/db_test_helpers.py`. Never redefine `DummyDB`, `make_db`, or inline DDL in individual test files. + +```python +import sys, os +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) +from db_test_helpers import make_db, DummyDB, insert_device, minutes_ago +``` + +If a helper you need doesn't exist yet, add it to `db_test_helpers.py` — not locally in the test file. + ## Path Hygiene - Use environment variables for runtime paths diff --git a/front/devices.php b/front/devices.php index 19095e527..e08fdbc4d 100755 --- a/front/devices.php +++ b/front/devices.php @@ -436,22 +436,28 @@ function initFilters() { if (existingFilter) { // Add the unique columnValue to options if not already present - if (!existingFilter.options.includes(entry.columnValue)) { - existingFilter.options.push(entry.columnValue); + if (!existingFilter.options.some(opt => opt.value === entry.columnValue)) { + existingFilter.options.push({ + value: entry.columnValue, + label: entry.columnLabel || entry.columnValue + }); } } else { // Create a new filter entry transformed.filters.push({ column: entry.columnName, headerKey: entry.columnHeaderStringKey, - options: [entry.columnValue] + options: [{ + value: entry.columnValue, + label: entry.columnLabel || entry.columnValue + }] }); } }); - // Sort options alphabetically for better readability + // Sort options alphabetically by label for better readability transformed.filters.forEach(filter => { - filter.options.sort(); + filter.options.sort((a, b) => a.label.localeCompare(b.label)); }); // Output the result diff --git a/front/php/components/devices_filters.php b/front/php/components/devices_filters.php index 0a376b4f0..9f36d7922 100755 --- a/front/php/components/devices_filters.php +++ b/front/php/components/devices_filters.php @@ -10,9 +10,18 @@ function renderFilterDropdown($headerKey, $columnName, $values) { // Generate dropdown options $optionsHtml = ''; // Default "All" option - foreach ($values as $value) { - $escapedValue = htmlspecialchars($value); - $optionsHtml .= ''; + foreach ($values as $item) { + // Support both {value, label} objects and plain strings (backward compat) + if (is_array($item)) { + $val = $item['value'] ?? ''; + $label = $item['label'] ?? $val; + } else { + $val = $item; + $label = $item; + } + $escapedValue = htmlspecialchars($val); + $escapedLabel = htmlspecialchars($label); + $optionsHtml .= ''; } // Generate the dropdown HTML diff --git a/server/const.py b/server/const.py index 1e554c9b8..9762ed69d 100755 --- a/server/const.py +++ b/server/const.py @@ -68,42 +68,46 @@ """ sql_appevents = """select * from AppEvents order by dateTimeCreated desc""" -sql_devices_filters = """ - SELECT DISTINCT 'devSite' AS columnName, devSite AS columnValue - FROM Devices WHERE devSite NOT IN ('', 'null') AND devSite IS NOT NULL +sql_devices_filters = f""" + SELECT DISTINCT 'devSite' AS columnName, devSite AS columnValue, devSite AS columnLabel + FROM Devices WHERE devSite NOT IN ({NULL_EQUIVALENTS_SQL}) AND devSite IS NOT NULL UNION - SELECT DISTINCT 'devSourcePlugin' AS columnName, devSourcePlugin AS columnValue - FROM Devices WHERE devSourcePlugin NOT IN ('', 'null') AND devSourcePlugin IS NOT NULL + SELECT DISTINCT 'devSourcePlugin' AS columnName, devSourcePlugin AS columnValue, devSourcePlugin AS columnLabel + FROM Devices WHERE devSourcePlugin NOT IN ({NULL_EQUIVALENTS_SQL}) AND devSourcePlugin IS NOT NULL UNION - SELECT DISTINCT 'devOwner' AS columnName, devOwner AS columnValue - FROM Devices WHERE devOwner NOT IN ('', 'null') AND devOwner IS NOT NULL + SELECT DISTINCT 'devOwner' AS columnName, devOwner AS columnValue, devOwner AS columnLabel + FROM Devices WHERE devOwner NOT IN ({NULL_EQUIVALENTS_SQL}) AND devOwner IS NOT NULL UNION - SELECT DISTINCT 'devType' AS columnName, devType AS columnValue - FROM Devices WHERE devType NOT IN ('', 'null') AND devType IS NOT NULL + SELECT DISTINCT 'devType' AS columnName, devType AS columnValue, devType AS columnLabel + FROM Devices WHERE devType NOT IN ({NULL_EQUIVALENTS_SQL}) AND devType IS NOT NULL UNION - SELECT DISTINCT 'devGroup' AS columnName, devGroup AS columnValue - FROM Devices WHERE devGroup NOT IN ('', 'null') AND devGroup IS NOT NULL + SELECT DISTINCT 'devGroup' AS columnName, devGroup AS columnValue, devGroup AS columnLabel + FROM Devices WHERE devGroup NOT IN ({NULL_EQUIVALENTS_SQL}) AND devGroup IS NOT NULL UNION - SELECT DISTINCT 'devLocation' AS columnName, devLocation AS columnValue - FROM Devices WHERE devLocation NOT IN ('', 'null') AND devLocation IS NOT NULL + SELECT DISTINCT 'devLocation' AS columnName, devLocation AS columnValue, devLocation AS columnLabel + FROM Devices WHERE devLocation NOT IN ({NULL_EQUIVALENTS_SQL}) AND devLocation IS NOT NULL UNION - SELECT DISTINCT 'devVendor' AS columnName, devVendor AS columnValue - FROM Devices WHERE devVendor NOT IN ('', 'null') AND devVendor IS NOT NULL + SELECT DISTINCT 'devVendor' AS columnName, devVendor AS columnValue, devVendor AS columnLabel + FROM Devices WHERE devVendor NOT IN ({NULL_EQUIVALENTS_SQL}) AND devVendor IS NOT NULL UNION - SELECT DISTINCT 'devSyncHubNode' AS columnName, devSyncHubNode AS columnValue - FROM Devices WHERE devSyncHubNode NOT IN ('', 'null') AND devSyncHubNode IS NOT NULL + SELECT DISTINCT 'devSyncHubNode' AS columnName, devSyncHubNode AS columnValue, devSyncHubNode AS columnLabel + FROM Devices WHERE devSyncHubNode NOT IN ({NULL_EQUIVALENTS_SQL}) AND devSyncHubNode IS NOT NULL UNION - SELECT DISTINCT 'devVlan' AS columnName, devVlan AS columnValue - FROM Devices WHERE devVlan NOT IN ('', 'null') AND devVlan IS NOT NULL + SELECT DISTINCT 'devVlan' AS columnName, devVlan AS columnValue, devVlan AS columnLabel + FROM Devices WHERE devVlan NOT IN ({NULL_EQUIVALENTS_SQL}) AND devVlan IS NOT NULL UNION - SELECT DISTINCT 'devParentMAC' AS columnName, devParentMAC AS columnValue - FROM Devices WHERE devParentMAC NOT IN ('', 'null') AND devParentMAC IS NOT NULL + SELECT 'devParentMAC' AS columnName, d.devParentMAC AS columnValue, + COALESCE(p.devName, d.devParentMAC) AS columnLabel + FROM Devices d + LEFT JOIN Devices p ON LOWER(p.devMac) = LOWER(d.devParentMAC) + WHERE d.devParentMAC NOT IN ({NULL_EQUIVALENTS_SQL}) AND d.devParentMAC IS NOT NULL + GROUP BY d.devParentMAC COLLATE NOCASE UNION - SELECT DISTINCT 'devParentRelType' AS columnName, devParentRelType AS columnValue - FROM Devices WHERE devParentRelType NOT IN ('', 'null') AND devParentRelType IS NOT NULL + SELECT DISTINCT 'devParentRelType' AS columnName, devParentRelType AS columnValue, devParentRelType AS columnLabel + FROM Devices WHERE devParentRelType NOT IN ({NULL_EQUIVALENTS_SQL}) AND devParentRelType IS NOT NULL UNION - SELECT DISTINCT 'devSSID' AS columnName, devSSID AS columnValue - FROM Devices WHERE devSSID NOT IN ('', 'null') AND devSSID IS NOT NULL + SELECT DISTINCT 'devSSID' AS columnName, devSSID AS columnValue, devSSID AS columnLabel + FROM Devices WHERE devSSID NOT IN ({NULL_EQUIVALENTS_SQL}) AND devSSID IS NOT NULL ORDER BY columnName; """ diff --git a/server/scan/device_handling.py b/server/scan/device_handling.py index 63673eda9..1029caa8d 100755 --- a/server/scan/device_handling.py +++ b/server/scan/device_handling.py @@ -5,7 +5,7 @@ from helper import get_setting_value, check_IP_format from utils.datetime_utils import timeNowUTC, normalizeTimeStamp from logger import mylog, Logger -from const import vendorsPath, vendorsPathNewest, sql_generateGuid, NULL_EQUIVALENTS +from const import vendorsPath, vendorsPathNewest, sql_generateGuid, NULL_EQUIVALENTS, NULL_EQUIVALENTS_SQL from models.device_instance import DeviceInstance from scan.name_resolution import NameResolver from scan.device_heuristics import guess_icon, guess_type @@ -240,6 +240,29 @@ def update_devLastConnection_from_CurrentScan(db): """) +def update_sync_hub_node(db): + """ + Backfill devSyncHubNode with SYNC_node_name for devices where it is empty. + Mirrors the fallback already used in create_new_devices. + """ + sql = db.sql + node_name = str(get_setting_value("SYNC_node_name") or "").strip() + + if not node_name: + return + + sql.execute( + f""" + UPDATE Devices + SET devSyncHubNode = ? + WHERE COALESCE(LOWER(TRIM(devSyncHubNode)), '') IN ({NULL_EQUIVALENTS_SQL}) + """, + (node_name,), + ) + + db.commitDB() + + def update_devices_data_from_scan(db): sql = db.sql @@ -378,11 +401,11 @@ def update_icons_and_types(db): if get_setting_value("NEWDEV_replace_preset_icon"): query = f"""SELECT * FROM Devices - WHERE devIcon in ('', 'null', '{default_icon}') + WHERE devIcon in ({NULL_EQUIVALENTS_SQL}, '{default_icon}') OR devIcon IS NULL""" else: - query = """SELECT * FROM Devices - WHERE devIcon in ('', 'null') + query = f"""SELECT * FROM Devices + WHERE devIcon in ({NULL_EQUIVALENTS_SQL}) OR devIcon IS NULL""" for device in sql.execute(query): @@ -406,8 +429,8 @@ def update_icons_and_types(db): # Guess Type recordsToUpdate = [] - query = """SELECT * FROM Devices - WHERE devType in ('', 'null') + query = f"""SELECT * FROM Devices + WHERE devType in ({NULL_EQUIVALENTS_SQL}) OR devType IS NULL""" default_type = get_setting_value("NEWDEV_devType") @@ -529,7 +552,7 @@ def save_scanned_devices(db): def print_scan_stats(db): sql = db.sql # TO-DO - query = """ + query = f""" SELECT (SELECT COUNT(*) FROM CurrentScan) AS devices_detected, (SELECT COUNT(*) FROM CurrentScan WHERE NOT EXISTS (SELECT 1 FROM Devices WHERE devMac = scanMac)) AS new_devices, @@ -544,7 +567,7 @@ def print_scan_stats(db): (SELECT COUNT(*) FROM Devices, CurrentScan WHERE devMac = scanMac AND scanLastIP IS NOT NULL - AND scanLastIP NOT IN ('', 'null', '(unknown)', '(Unknown)') + AND scanLastIP NOT IN ({NULL_EQUIVALENTS_SQL}) AND scanLastIP <> COALESCE(devPrimaryIPv4, '') AND scanLastIP <> COALESCE(devPrimaryIPv6, '') AND scanLastIP <> COALESCE(devLastIP, '') diff --git a/server/scan/session_events.py b/server/scan/session_events.py index f39e791fe..ca35c1fe1 100755 --- a/server/scan/session_events.py +++ b/server/scan/session_events.py @@ -4,6 +4,7 @@ save_scanned_devices, exclude_ignored_devices, update_devices_data_from_scan, + update_sync_hub_node, update_vendors_from_mac, update_icons_and_types, update_devPresentLastScan_based_on_force_status, @@ -62,6 +63,10 @@ def process_scan(db): mylog("verbose", "[Process Scan] Updating Devices Info") update_devices_data_from_scan(db) + # Backfill devSyncHubNode for devices where it is empty + mylog("verbose", "[Process Scan] Updating Sync Hub Node") + update_sync_hub_node(db) + # Last Connection Time stamp from CurrentScan mylog("verbose", "[Process Scan] Updating devLastConnection from CurrentScan") update_devLastConnection_from_CurrentScan(db) diff --git a/test/scan/test_sync_hub_node_backfill.py b/test/scan/test_sync_hub_node_backfill.py new file mode 100644 index 000000000..f32fcdacf --- /dev/null +++ b/test/scan/test_sync_hub_node_backfill.py @@ -0,0 +1,90 @@ +"""Tests for update_sync_hub_node backfill.""" + +import sys +import os +from unittest.mock import patch + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) +from db_test_helpers import make_db, DummyDB # noqa: E402 + +from server.scan import device_handling + + +def _make_db(devices): + """Create an in-memory DB with full schema and seed rows.""" + conn = make_db() + cur = conn.cursor() + cur.executemany( + "INSERT INTO Devices (devMac, devSyncHubNode) VALUES (?, ?)", + devices, + ) + conn.commit() + return conn + + +def _read_nodes(conn): + """Return a dict of devMac -> devSyncHubNode.""" + return { + row["devMac"]: row["devSyncHubNode"] + for row in conn.execute("SELECT devMac, devSyncHubNode FROM Devices") + } + + +@patch.object(device_handling, "get_setting_value", return_value="MyNode") +def test_backfill_empty_values(mock_setting): + """Empty and null devSyncHubNode should be backfilled with SYNC_node_name.""" + conn = _make_db([ + ("AA:AA:AA:AA:AA:01", ""), + ("AA:AA:AA:AA:AA:02", None), + ("AA:AA:AA:AA:AA:03", "null"), + ]) + + device_handling.update_sync_hub_node(DummyDB(conn)) + nodes = _read_nodes(conn) + + assert nodes["AA:AA:AA:AA:AA:01"] == "MyNode" + assert nodes["AA:AA:AA:AA:AA:02"] == "MyNode" + assert nodes["AA:AA:AA:AA:AA:03"] == "MyNode" + + +@patch.object(device_handling, "get_setting_value", return_value="MyNode") +def test_no_overwrite_existing(mock_setting): + """Devices with a real devSyncHubNode should not be overwritten.""" + conn = _make_db([ + ("AA:AA:AA:AA:AA:01", "RemoteNode"), + ("AA:AA:AA:AA:AA:02", ""), + ]) + + device_handling.update_sync_hub_node(DummyDB(conn)) + nodes = _read_nodes(conn) + + assert nodes["AA:AA:AA:AA:AA:01"] == "RemoteNode" + assert nodes["AA:AA:AA:AA:AA:02"] == "MyNode" + + +@patch.object(device_handling, "get_setting_value", return_value="") +def test_noop_when_setting_empty(mock_setting): + """No updates when SYNC_node_name is empty.""" + conn = _make_db([ + ("AA:AA:AA:AA:AA:01", ""), + ("AA:AA:AA:AA:AA:02", None), + ]) + + device_handling.update_sync_hub_node(DummyDB(conn)) + nodes = _read_nodes(conn) + + assert nodes["AA:AA:AA:AA:AA:01"] == "" + assert nodes["AA:AA:AA:AA:AA:02"] is None + + +@patch.object(device_handling, "get_setting_value", return_value=None) +def test_noop_when_setting_none(mock_setting): + """No updates when SYNC_node_name is None.""" + conn = _make_db([ + ("AA:AA:AA:AA:AA:01", ""), + ]) + + device_handling.update_sync_hub_node(DummyDB(conn)) + nodes = _read_nodes(conn) + + assert nodes["AA:AA:AA:AA:AA:01"] == ""