diff --git a/backend/app/api/routes/zigbee.py b/backend/app/api/routes/zigbee.py index d59bfaf..d5d47cb 100644 --- a/backend/app/api/routes/zigbee.py +++ b/backend/app/api/routes/zigbee.py @@ -162,9 +162,11 @@ async def _persist_pending_import( # the pending inventory like every other device, so the user approves it # explicitly. Only the shared paths below run for it. - # If the device has already been approved as a canvas Node, refresh - # its properties on every canvas it sits on and skip creating a pending - # row (keeps approved devices out of pending/hidden modals on re-import). + # If the device has already been approved as a canvas Node, refresh its + # properties on every canvas it sits on. Still ensure the discovery + # inventory carries a row for it (status="approved") so it shows in the + # inventory list with an "In N canvas" badge — legacy auto-placed + # coordinators never got a pending row, which is why they went missing. existing_nodes = ( await db.execute( select(Node).where(Node.ieee_address == ieee).order_by(Node.id) @@ -175,6 +177,38 @@ async def _persist_pending_import( existing_node.properties = merge_zigbee_properties( existing_node.properties, props ) + inv = ( + await db.execute( + select(PendingDevice).where(PendingDevice.ieee_address == ieee) + ) + ).scalar_one_or_none() + if inv is None: + db.add( + PendingDevice( + ieee_address=ieee, + friendly_name=n.get("friendly_name"), + hostname=n.get("friendly_name"), + suggested_type=n.get("type"), + device_subtype=n.get("device_type"), + model=n.get("model"), + vendor=n.get("vendor"), + lqi=n.get("lqi"), + status="approved", + discovery_source="zigbee", + ) + ) + pending_created += 1 + else: + # Refresh metadata but never change the row's status (an approved + # device stays approved; a hidden one stays hidden). + inv.friendly_name = n.get("friendly_name") or inv.friendly_name + inv.suggested_type = n.get("type") or inv.suggested_type + inv.device_subtype = n.get("device_type") or inv.device_subtype + inv.model = n.get("model") or inv.model + inv.vendor = n.get("vendor") or inv.vendor + if n.get("lqi") is not None: + inv.lqi = n.get("lqi") + pending_updated += 1 continue result = await db.execute( diff --git a/backend/app/api/routes/zwave.py b/backend/app/api/routes/zwave.py index 9f20241..add3e9f 100644 --- a/backend/app/api/routes/zwave.py +++ b/backend/app/api/routes/zwave.py @@ -157,7 +157,10 @@ async def _persist_pending_import( # explicitly. Only the shared paths below run for it. # Already approved as a canvas Node → refresh props on every canvas it - # sits on, skip pending row. + # sits on. Still ensure the discovery inventory carries a row for it + # (status="approved") so it shows in the inventory list with an + # "In N canvas" badge — legacy auto-placed coordinators never got a + # pending row, which is why they went missing. existing_nodes = ( await db.execute( select(Node).where(Node.ieee_address == ieee).order_by(Node.id) @@ -168,6 +171,37 @@ async def _persist_pending_import( existing_node.properties = merge_zwave_properties( existing_node.properties, props ) + inv = ( + await db.execute( + select(PendingDevice).where(PendingDevice.ieee_address == ieee) + ) + ).scalar_one_or_none() + if inv is None: + db.add( + PendingDevice( + ieee_address=ieee, + friendly_name=n.get("friendly_name"), + hostname=n.get("friendly_name"), + suggested_type=n.get("type"), + device_subtype=n.get("device_type"), + model=n.get("model"), + vendor=n.get("vendor"), + lqi=n.get("lqi"), + status="approved", + discovery_source="zwave", + ) + ) + pending_created += 1 + else: + # Refresh metadata but never change the row's status. + inv.friendly_name = n.get("friendly_name") or inv.friendly_name + inv.suggested_type = n.get("type") or inv.suggested_type + inv.device_subtype = n.get("device_type") or inv.device_subtype + inv.model = n.get("model") or inv.model + inv.vendor = n.get("vendor") or inv.vendor + if n.get("lqi") is not None: + inv.lqi = n.get("lqi") + pending_updated += 1 continue result = await db.execute( diff --git a/backend/tests/test_zigbee_router.py b/backend/tests/test_zigbee_router.py index a6f2160..a2e9cda 100644 --- a/backend/tests/test_zigbee_router.py +++ b/backend/tests/test_zigbee_router.py @@ -429,19 +429,20 @@ async def test_persist_pending_import_sets_coordinator_pending_fields(db_session @pytest.mark.asyncio -async def test_persist_pending_import_skips_pending_for_approved_node( +async def test_persist_pending_import_backfills_inventory_for_approved_node( db_session, ) -> None: - """A device already approved as a canvas Node must not reappear in pending. - - Its properties must still be refreshed with the latest Vendor/Model/LQI. + """A device on canvas but missing its inventory row gets one backfilled + (status="approved"), so it shows in the inventory list. Node props still + refresh with the latest Vendor/Model/LQI. """ from sqlalchemy import select from app.api.routes.zigbee import _persist_pending_import from app.db.models import Node, PendingDevice - # Simulate: router was approved earlier → exists as a canvas Node. + # Simulate: router was approved earlier → exists as a canvas Node, but with + # no matching pending_devices row (e.g. a legacy auto-placed device). approved = Node( label="router_1", type="zigbee_router", @@ -458,13 +459,15 @@ async def test_persist_pending_import_skips_pending_for_approved_node( bumped[1]["lqi"] = 250 # new LQI from re-import await _persist_pending_import(db_session, bumped, _PENDING_EDGES) - # No PendingDevice row was created for the approved router. - pendings = ( + # An inventory row is backfilled as "approved" (it is on a canvas). + inv = ( await db_session.execute( select(PendingDevice).where(PendingDevice.ieee_address == "0xR1") ) - ).scalars().all() - assert pendings == [] + ).scalar_one() + assert inv.status == "approved" + assert inv.suggested_type == "zigbee_router" + assert inv.device_subtype == "Router" # Node properties got refreshed. refreshed = ( @@ -476,6 +479,37 @@ async def test_persist_pending_import_skips_pending_for_approved_node( assert all(p["visible"] is False for p in refreshed.properties) +@pytest.mark.asyncio +async def test_persist_pending_import_preserves_hidden_inventory_for_approved_node( + db_session, +) -> None: + """If the on-canvas device already has a hidden inventory row, re-import + refreshes its metadata but must NOT flip it back to approved/visible.""" + from sqlalchemy import select + + from app.api.routes.zigbee import _persist_pending_import + from app.db.models import Node, PendingDevice + + db_session.add(Node( + label="router_1", type="zigbee_router", status="online", + check_method="none", ieee_address="0xR1", services=[], properties=[], + )) + db_session.add(PendingDevice( + ieee_address="0xR1", friendly_name="router_1", suggested_type="zigbee_router", + device_subtype="Router", status="hidden", discovery_source="zigbee", + )) + await db_session.commit() + + await _persist_pending_import(db_session, _PENDING_NODES, _PENDING_EDGES) + + inv = ( + await db_session.execute( + select(PendingDevice).where(PendingDevice.ieee_address == "0xR1") + ) + ).scalar_one() + assert inv.status == "hidden" # stays hidden + + @pytest.mark.asyncio async def test_persist_pending_import_revives_orphaned_approved_device( db_session, @@ -611,14 +645,15 @@ async def test_persist_pending_import_preserves_user_visibility(db_session) -> N async def test_persist_pending_import_refreshes_approved_coordinator_node( db_session, ) -> None: - """An already-approved coordinator Node still gets its props refreshed on - re-import (via the shared approved-node path), and stays out of pending.""" + """An already-approved coordinator Node gets its props refreshed on + re-import, and a backfilled inventory row (approved) so it shows in the + inventory list — this is the legacy auto-placed coordinator scenario.""" from sqlalchemy import select from app.api.routes.zigbee import _persist_pending_import from app.db.models import Node, PendingDevice - # User previously approved the coordinator onto the canvas. + # Legacy: coordinator auto-placed as a canvas Node, no pending_devices row. db_session.add(Node( label="Coordinator", type="zigbee_coordinator", status="online", check_method="none", ieee_address="0xCOORD", services=[], properties=[], @@ -638,13 +673,14 @@ async def test_persist_pending_import_refreshes_approved_coordinator_node( assert keys["Model"] == "CC2652" by_key = {p["key"]: p for p in coord.properties} assert by_key["Vendor"]["visible"] is False - # Approved coordinator must NOT reappear in pending. - pendings = ( + # Inventory row backfilled as approved → now visible in the inventory list. + inv = ( await db_session.execute( select(PendingDevice).where(PendingDevice.ieee_address == "0xCOORD") ) - ).scalars().all() - assert pendings == [] + ).scalar_one() + assert inv.status == "approved" + assert inv.suggested_type == "zigbee_coordinator" @pytest.mark.asyncio diff --git a/backend/tests/test_zwave_router.py b/backend/tests/test_zwave_router.py index 5b275c9..3ccc442 100644 --- a/backend/tests/test_zwave_router.py +++ b/backend/tests/test_zwave_router.py @@ -374,7 +374,9 @@ async def test_persist_sets_coordinator_pending_fields(db_session) -> None: @pytest.mark.asyncio -async def test_persist_skips_pending_for_approved_node(db_session) -> None: +async def test_persist_backfills_inventory_for_approved_node(db_session) -> None: + """On-canvas device missing its inventory row gets one backfilled + (status="approved"); Node props still refresh.""" from sqlalchemy import select from app.api.routes.zwave import _persist_pending_import @@ -394,12 +396,13 @@ async def test_persist_skips_pending_for_approved_node(db_session) -> None: await _persist_pending_import(db_session, _PENDING_NODES, _PENDING_EDGES) - pendings = ( + inv = ( await db_session.execute( select(PendingDevice).where(PendingDevice.ieee_address == "zwave-0xh-2") ) - ).scalars().all() - assert pendings == [] + ).scalar_one() + assert inv.status == "approved" + assert inv.suggested_type == "zwave_router" refreshed = ( await db_session.execute(select(Node).where(Node.ieee_address == "zwave-0xh-2")) ).scalar_one()