diff --git a/backend/app/api/routes/zigbee.py b/backend/app/api/routes/zigbee.py index dd62039..d59bfaf 100644 --- a/backend/app/api/routes/zigbee.py +++ b/backend/app/api/routes/zigbee.py @@ -11,7 +11,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.api.deps import get_current_user from app.db.database import AsyncSessionLocal, get_db -from app.db.models import Design, Node, PendingDevice, PendingDeviceLink, ScanRun +from app.db.models import Node, PendingDevice, PendingDeviceLink, ScanRun from app.schemas.scan import ScanRunResponse from app.schemas.zigbee import ( ZigbeeCoordinatorOut, @@ -143,10 +143,8 @@ async def _persist_pending_import( # the by-IEEE lookups below resolve to a single row. await dedupe_nodes_by_ieee(db) - # Determine target design (use first design as fallback) - first_design = (await db.execute(select(Design).order_by(Design.created_at).limit(1))).scalar() - default_design_id = first_design.id if first_design else None - + # Coordinator is no longer auto-placed, so the response's coordinator fields + # stay unset — retained for backward-compatible response shape. coordinator_out: ZigbeeCoordinatorOut | None = None coordinator_existed = False pending_created = 0 @@ -160,44 +158,9 @@ async def _persist_pending_import( ieee, n.get("vendor"), n.get("model"), n.get("lqi") ) - if n.get("device_type") == "Coordinator": - # The coordinator may sit on several canvases (one Node per design). - # Refresh props on every matching node, not just one. - existing_nodes = ( - await db.execute( - select(Node).where(Node.ieee_address == ieee).order_by(Node.id) - ) - ).scalars().all() - if existing_nodes: - for existing_node in existing_nodes: - existing_node.properties = merge_zigbee_properties( - existing_node.properties, props - ) - first = existing_nodes[0] - coordinator_out = ZigbeeCoordinatorOut( - id=first.id, - label=first.label, - ieee_address=ieee, - ) - coordinator_existed = True - continue - label = n.get("friendly_name") or ieee - node = Node( - label=label, - type=n.get("type") or "zigbee_coordinator", - status="online", - check_method="none", - ieee_address=ieee, - services=[], - properties=props, - design_id=default_design_id, - ) - db.add(node) - await db.flush() - coordinator_out = ZigbeeCoordinatorOut( - id=node.id, label=label, ieee_address=ieee - ) - continue + # The coordinator is no longer auto-placed on the canvas — it flows to + # 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 diff --git a/backend/app/api/routes/zwave.py b/backend/app/api/routes/zwave.py index 56fc355..9f20241 100644 --- a/backend/app/api/routes/zwave.py +++ b/backend/app/api/routes/zwave.py @@ -11,7 +11,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.api.deps import get_current_user from app.db.database import AsyncSessionLocal, get_db -from app.db.models import Design, Node, PendingDevice, PendingDeviceLink, ScanRun +from app.db.models import Node, PendingDevice, PendingDeviceLink, ScanRun from app.schemas.scan import ScanRunResponse from app.schemas.zwave import ( ZwaveCoordinatorOut, @@ -139,9 +139,8 @@ async def _persist_pending_import( # the by-IEEE lookups below resolve cleanly. await dedupe_nodes_by_ieee(db) - first_design = (await db.execute(select(Design).order_by(Design.created_at).limit(1))).scalar() - default_design_id = first_design.id if first_design else None - + # Coordinator is no longer auto-placed, so the response's coordinator fields + # stay unset — retained for backward-compatible response shape. coordinator_out: ZwaveCoordinatorOut | None = None coordinator_existed = False pending_created = 0 @@ -153,44 +152,9 @@ async def _persist_pending_import( continue props = build_zwave_properties(ieee, n.get("vendor"), n.get("model")) - if n.get("type") == "zwave_coordinator": - # The coordinator may sit on several canvases (one Node per design). - # Refresh props on every matching node, not just one. - existing_nodes = ( - await db.execute( - select(Node).where(Node.ieee_address == ieee).order_by(Node.id) - ) - ).scalars().all() - if existing_nodes: - for existing_node in existing_nodes: - existing_node.properties = merge_zwave_properties( - existing_node.properties, props - ) - first = existing_nodes[0] - coordinator_out = ZwaveCoordinatorOut( - id=first.id, - label=first.label, - ieee_address=ieee, - ) - coordinator_existed = True - continue - label = n.get("friendly_name") or ieee - node = Node( - label=label, - type=n.get("type") or "zwave_coordinator", - status="online", - check_method="none", - ieee_address=ieee, - services=[], - properties=props, - design_id=default_design_id, - ) - db.add(node) - await db.flush() - coordinator_out = ZwaveCoordinatorOut( - id=node.id, label=label, ieee_address=ieee - ) - continue + # The coordinator is no longer auto-placed on the canvas — it flows to + # the pending inventory like every other device, so the user approves it + # 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. diff --git a/backend/tests/test_zigbee_router.py b/backend/tests/test_zigbee_router.py index 7467f15..a6f2160 100644 --- a/backend/tests/test_zigbee_router.py +++ b/backend/tests/test_zigbee_router.py @@ -338,20 +338,36 @@ async def test_import_pending_endpoint_creates_zigbee_scan_run( @pytest.mark.asyncio -async def test_persist_pending_import_creates_coordinator_and_pending( +async def test_persist_pending_import_coordinator_goes_to_pending( db_session, ) -> None: + """Coordinator is no longer auto-placed — it lands in pending like the rest.""" + from sqlalchemy import select + from app.api.routes.zigbee import _persist_pending_import + from app.db.models import Node, PendingDevice result = await _persist_pending_import(db_session, _PENDING_NODES, _PENDING_EDGES) assert result.device_count == 3 - assert result.pending_created == 2 + assert result.pending_created == 3 # coordinator included now assert result.pending_updated == 0 - assert result.coordinator is not None - assert result.coordinator.ieee_address == "0xCOORD" + assert result.coordinator is None # not auto-placed assert result.coordinator_already_existed is False assert result.links_recorded == 2 + # No canvas Node auto-created for the coordinator. + nodes = (await db_session.execute(select(Node))).scalars().all() + assert nodes == [] + # Coordinator sits in the pending inventory. + coord = ( + await db_session.execute( + select(PendingDevice).where(PendingDevice.ieee_address == "0xCOORD") + ) + ).scalar_one() + assert coord.status == "pending" + assert coord.suggested_type == "zigbee_coordinator" + assert coord.device_subtype == "Coordinator" + @pytest.mark.asyncio async def test_persist_pending_import_idempotent_updates_existing( @@ -366,8 +382,8 @@ async def test_persist_pending_import_idempotent_updates_existing( result = await _persist_pending_import(db_session, bumped, _PENDING_EDGES) assert result.pending_created == 0 - assert result.pending_updated == 2 - assert result.coordinator_already_existed is True + assert result.pending_updated == 3 # coordinator upserts too + assert result.coordinator_already_existed is False assert result.links_recorded == 2 @@ -389,12 +405,12 @@ async def test_persist_pending_import_replaces_links(db_session) -> None: @pytest.mark.asyncio -async def test_persist_pending_import_sets_coordinator_properties(db_session) -> None: - """Coordinator Node is created with IEEE/Vendor/Model/LQI in properties.""" +async def test_persist_pending_import_sets_coordinator_pending_fields(db_session) -> None: + """Coordinator lands in pending carrying its vendor/model metadata.""" from sqlalchemy import select from app.api.routes.zigbee import _persist_pending_import - from app.db.models import Node + from app.db.models import PendingDevice nodes_with_meta = [dict(n) for n in _PENDING_NODES] nodes_with_meta[0]["vendor"] = "TI" @@ -403,12 +419,13 @@ async def test_persist_pending_import_sets_coordinator_properties(db_session) -> await _persist_pending_import(db_session, nodes_with_meta, _PENDING_EDGES) coord = ( - await db_session.execute(select(Node).where(Node.ieee_address == "0xCOORD")) + await db_session.execute( + select(PendingDevice).where(PendingDevice.ieee_address == "0xCOORD") + ) ).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) + assert coord.vendor == "TI" + assert coord.model == "CC2652" + assert coord.suggested_type == "zigbee_coordinator" @pytest.mark.asyncio @@ -501,8 +518,8 @@ async def test_persist_pending_import_revives_orphaned_approved_device( ) ).scalar_one() assert revived.status == "pending" - # End device 0xE1 is brand new → created as pending; router was updated. - assert result.pending_created == 1 + # Coordinator + end device 0xE1 are brand new → created; router was revived. + assert result.pending_created == 2 assert result.pending_updated == 1 # It is now visible to the Pending list (status filter == "pending"). @@ -511,7 +528,7 @@ async def test_persist_pending_import_revives_orphaned_approved_device( select(PendingDevice).where(PendingDevice.status == "pending") ) ).scalars().all() - assert {p.ieee_address for p in listed} == {"0xR1", "0xE1"} + assert {p.ieee_address for p in listed} == {"0xCOORD", "0xR1", "0xE1"} @pytest.mark.asyncio @@ -591,15 +608,22 @@ async def test_persist_pending_import_preserves_user_visibility(db_session) -> N @pytest.mark.asyncio -async def test_persist_pending_import_refreshes_existing_coordinator_properties( +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.""" from sqlalchemy import select from app.api.routes.zigbee import _persist_pending_import - from app.db.models import Node + from app.db.models import Node, PendingDevice - await _persist_pending_import(db_session, _PENDING_NODES, _PENDING_EDGES) + # User previously approved the coordinator onto the canvas. + db_session.add(Node( + label="Coordinator", type="zigbee_coordinator", status="online", + check_method="none", ieee_address="0xCOORD", services=[], properties=[], + )) + await db_session.commit() bumped = [dict(n) for n in _PENDING_NODES] bumped[0]["vendor"] = "TI" @@ -612,10 +636,15 @@ 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 + # Approved coordinator must NOT reappear in pending. + pendings = ( + await db_session.execute( + select(PendingDevice).where(PendingDevice.ieee_address == "0xCOORD") + ) + ).scalars().all() + assert pendings == [] @pytest.mark.asyncio diff --git a/backend/tests/test_zwave_router.py b/backend/tests/test_zwave_router.py index 3aa7130..5b275c9 100644 --- a/backend/tests/test_zwave_router.py +++ b/backend/tests/test_zwave_router.py @@ -297,18 +297,31 @@ async def test_import_pending_requires_auth(client: AsyncClient) -> None: @pytest.mark.asyncio -async def test_persist_creates_coordinator_and_pending(db_session) -> None: +async def test_persist_coordinator_goes_to_pending(db_session) -> None: + """Coordinator is no longer auto-placed — it lands in pending like the rest.""" + from sqlalchemy import select + from app.api.routes.zwave import _persist_pending_import + from app.db.models import Node, PendingDevice result = await _persist_pending_import(db_session, _PENDING_NODES, _PENDING_EDGES) assert result.device_count == 3 - assert result.pending_created == 2 + assert result.pending_created == 3 # coordinator included now assert result.pending_updated == 0 - assert result.coordinator is not None - assert result.coordinator.ieee_address == "zwave-0xh-1" + assert result.coordinator is None # not auto-placed assert result.coordinator_already_existed is False assert result.links_recorded == 2 + nodes = (await db_session.execute(select(Node))).scalars().all() + assert nodes == [] + coord = ( + await db_session.execute( + select(PendingDevice).where(PendingDevice.ieee_address == "zwave-0xh-1") + ) + ).scalar_one() + assert coord.status == "pending" + assert coord.suggested_type == "zwave_coordinator" + @pytest.mark.asyncio async def test_persist_idempotent_updates_existing(db_session) -> None: @@ -319,8 +332,8 @@ async def test_persist_idempotent_updates_existing(db_session) -> None: bumped[1]["model"] = "ZW111" result = await _persist_pending_import(db_session, bumped, _PENDING_EDGES) assert result.pending_created == 0 - assert result.pending_updated == 2 - assert result.coordinator_already_existed is True + assert result.pending_updated == 3 # coordinator upserts too + assert result.coordinator_already_existed is False @pytest.mark.asyncio @@ -339,22 +352,25 @@ async def test_persist_replaces_links(db_session) -> None: @pytest.mark.asyncio -async def test_persist_sets_coordinator_properties(db_session) -> None: +async def test_persist_sets_coordinator_pending_fields(db_session) -> None: + """Coordinator lands in pending carrying its vendor/model metadata.""" from sqlalchemy import select from app.api.routes.zwave import _persist_pending_import - from app.db.models import Node + from app.db.models import PendingDevice nodes = [dict(n) for n in _PENDING_NODES] nodes[0]["vendor"] = "Aeotec" nodes[0]["model"] = "ZW090" await _persist_pending_import(db_session, nodes, _PENDING_EDGES) coord = ( - await db_session.execute(select(Node).where(Node.ieee_address == "zwave-0xh-1")) + await db_session.execute( + select(PendingDevice).where(PendingDevice.ieee_address == "zwave-0xh-1") + ) ).scalar_one() - keys = {p["key"]: p["value"] for p in coord.properties} - assert keys == {"Z-Wave ID": "zwave-0xh-1", "Vendor": "Aeotec", "Model": "ZW090"} - assert all(p["visible"] is False for p in coord.properties) + assert coord.vendor == "Aeotec" + assert coord.model == "ZW090" + assert coord.suggested_type == "zwave_coordinator" @pytest.mark.asyncio @@ -416,7 +432,8 @@ async def test_persist_revives_orphaned_approved_device(db_session) -> None: ) ).scalar_one() assert revived.status == "pending" - assert result.pending_created == 1 + # Coordinator + end device are brand new → created; router was revived. + assert result.pending_created == 2 assert result.pending_updated == 1