From 5f87c64dcf7430a0c72280f32538555ad39179ea Mon Sep 17 00:00:00 2001 From: Pouzor Date: Thu, 14 May 2026 01:10:45 +0200 Subject: [PATCH] fix(zigbee): default IEEE/Vendor/Model/LQI props to hidden, preserve user visibility - New zigbee props (approve + first-time re-import) ship with visible=false so the canvas card stays clean. User opts in from the right panel. - On re-import of an already-approved node, merge instead of overwrite: keys already present keep their visible flag (and any user-edited value is replaced with the freshly imported one), brand-new keys are appended hidden. Non-zigbee custom properties are preserved untouched. --- backend/app/api/routes/zigbee.py | 15 ++++++-- backend/app/services/zigbee_service.py | 33 ++++++++++++++-- backend/tests/test_zigbee_router.py | 53 ++++++++++++++++++++++++++ frontend/src/utils/zigbeeProperties.ts | 8 ++-- 4 files changed, 98 insertions(+), 11 deletions(-) diff --git a/backend/app/api/routes/zigbee.py b/backend/app/api/routes/zigbee.py index 2fd9f9a..1379d23 100644 --- a/backend/app/api/routes/zigbee.py +++ b/backend/app/api/routes/zigbee.py @@ -23,7 +23,12 @@ from app.schemas.zigbee import ( ZigbeeTestConnectionRequest, ZigbeeTestConnectionResponse, ) -from app.services.zigbee_service import build_zigbee_properties, fetch_networkmap, test_mqtt_connection +from app.services.zigbee_service import ( + build_zigbee_properties, + fetch_networkmap, + merge_zigbee_properties, + test_mqtt_connection, +) logger = logging.getLogger(__name__) router = APIRouter() @@ -150,7 +155,9 @@ async def _persist_pending_import( existing = await db.execute(select(Node).where(Node.ieee_address == ieee)) existing_node = existing.scalar_one_or_none() if existing_node: - existing_node.properties = props + existing_node.properties = merge_zigbee_properties( + existing_node.properties, props + ) coordinator_out = ZigbeeCoordinatorOut( id=existing_node.id, label=existing_node.label, @@ -183,7 +190,9 @@ async def _persist_pending_import( ) existing_node = existing_node_q.scalar_one_or_none() if existing_node: - existing_node.properties = props + existing_node.properties = merge_zigbee_properties( + existing_node.properties, props + ) continue result = await db.execute( diff --git a/backend/app/services/zigbee_service.py b/backend/app/services/zigbee_service.py index faced11..d09e255 100644 --- a/backend/app/services/zigbee_service.py +++ b/backend/app/services/zigbee_service.py @@ -68,19 +68,44 @@ def build_zigbee_properties( Only includes a row when the value is non-empty. Shape matches the frontend ``NodeProperty`` type: ``{key, value, icon, visible}``. + + New props default to ``visible=False`` — users opt in to showing them on + the canvas card from the right panel. """ props: list[dict[str, Any]] = [] if ieee: - props.append({"key": "IEEE", "value": ieee, "icon": None, "visible": True}) + props.append({"key": "IEEE", "value": ieee, "icon": None, "visible": False}) if vendor: - props.append({"key": "Vendor", "value": vendor, "icon": None, "visible": True}) + props.append({"key": "Vendor", "value": vendor, "icon": None, "visible": False}) if model: - props.append({"key": "Model", "value": model, "icon": None, "visible": True}) + props.append({"key": "Model", "value": model, "icon": None, "visible": False}) if lqi is not None: - props.append({"key": "LQI", "value": str(lqi), "icon": None, "visible": True}) + props.append({"key": "LQI", "value": str(lqi), "icon": None, "visible": False}) return props +def merge_zigbee_properties( + existing: list[dict[str, Any]] | None, + new_props: list[dict[str, Any]], +) -> list[dict[str, Any]]: + """Merge fresh zigbee props into an existing property list. + + For keys already present: update ``value`` but preserve the user's + ``visible`` choice. New keys are appended with whatever visibility the + caller gave them (hidden by default per ``build_zigbee_properties``). + Non-zigbee custom properties are preserved untouched. + """ + out = [dict(p) for p in (existing or [])] + by_key = {p.get("key"): p for p in out} + for np in new_props: + key = np.get("key") + if key in by_key: + by_key[key]["value"] = np.get("value") + else: + out.append(dict(np)) + return out + + def _z2m_type_to_homelable(device_type: str) -> str: """Map a Z2M device type string to a homelable node type.""" mapping = { diff --git a/backend/tests/test_zigbee_router.py b/backend/tests/test_zigbee_router.py index 04ca15a..b24e0b3 100644 --- a/backend/tests/test_zigbee_router.py +++ b/backend/tests/test_zigbee_router.py @@ -407,6 +407,8 @@ async def test_persist_pending_import_sets_coordinator_properties(db_session) -> ).scalar_one() keys = {p["key"]: p["value"] for p in coord.properties} assert keys == {"IEEE": "0xCOORD", "Vendor": "TI", "Model": "CC2652"} + # New zigbee props default to hidden — user opts in from the right panel. + assert all(p["visible"] is False for p in coord.properties) @pytest.mark.asyncio @@ -453,6 +455,53 @@ async def test_persist_pending_import_skips_pending_for_approved_node( ).scalar_one() keys = {p["key"]: p["value"] for p in refreshed.properties} assert keys == {"IEEE": "0xR1", "Vendor": "TI", "Model": "CC2530", "LQI": "250"} + # Brand-new props on an existing Node start hidden. + assert all(p["visible"] is False for p in refreshed.properties) + + +@pytest.mark.asyncio +async def test_persist_pending_import_preserves_user_visibility(db_session) -> None: + """If user has already made props visible, re-import must not flip them back.""" + from sqlalchemy import select + + from app.api.routes.zigbee import _persist_pending_import + from app.db.models import Node + + approved = Node( + label="router_1", + type="zigbee_router", + status="online", + check_method="none", + ieee_address="0xR1", + services=[], + properties=[ + {"key": "IEEE", "value": "0xR1", "icon": None, "visible": True}, + {"key": "Vendor", "value": "TI", "icon": None, "visible": True}, + {"key": "Custom", "value": "kept", "icon": None, "visible": True}, + ], + ) + db_session.add(approved) + await db_session.commit() + + bumped = [dict(n) for n in _PENDING_NODES] + bumped[1]["lqi"] = 99 + bumped[1]["model"] = "CC2530" + await _persist_pending_import(db_session, bumped, _PENDING_EDGES) + + refreshed = ( + await db_session.execute(select(Node).where(Node.ieee_address == "0xR1")) + ).scalar_one() + by_key = {p["key"]: p for p in refreshed.properties} + # Existing keys keep their visibility (True). + assert by_key["IEEE"]["visible"] is True + assert by_key["Vendor"]["visible"] is True + # New key arrives hidden. + assert by_key["Model"]["visible"] is False + assert by_key["LQI"]["visible"] is False + assert by_key["LQI"]["value"] == "99" + # Non-zigbee user-added prop is preserved untouched. + assert by_key["Custom"]["value"] == "kept" + assert by_key["Custom"]["visible"] is True @pytest.mark.asyncio @@ -477,6 +526,10 @@ async def test_persist_pending_import_refreshes_existing_coordinator_properties( keys = {p["key"]: p["value"] for p in coord.properties} assert keys["Vendor"] == "TI" assert keys["Model"] == "CC2652" + # Newly added keys on re-import default to hidden. + by_key = {p["key"]: p for p in coord.properties} + assert by_key["Vendor"]["visible"] is False + assert by_key["Model"]["visible"] is False @pytest.mark.asyncio diff --git a/frontend/src/utils/zigbeeProperties.ts b/frontend/src/utils/zigbeeProperties.ts index 6fdc4ef..92dd8fc 100644 --- a/frontend/src/utils/zigbeeProperties.ts +++ b/frontend/src/utils/zigbeeProperties.ts @@ -15,9 +15,9 @@ export function buildZigbeeProperties(input: { lqi?: number | null }): NodeProperty[] { const props: NodeProperty[] = [] - if (input.ieee_address) props.push({ key: 'IEEE', value: input.ieee_address, icon: null, visible: true }) - if (input.vendor) props.push({ key: 'Vendor', value: input.vendor, icon: null, visible: true }) - if (input.model) props.push({ key: 'Model', value: input.model, icon: null, visible: true }) - if (input.lqi != null) props.push({ key: 'LQI', value: String(input.lqi), icon: null, visible: true }) + if (input.ieee_address) props.push({ key: 'IEEE', value: input.ieee_address, icon: null, visible: false }) + if (input.vendor) props.push({ key: 'Vendor', value: input.vendor, icon: null, visible: false }) + if (input.model) props.push({ key: 'Model', value: input.model, icon: null, visible: false }) + if (input.lqi != null) props.push({ key: 'LQI', value: String(input.lqi), icon: null, visible: false }) return props }