From 5b5eabf5dbe80c42b4649da5f153a71e7aaa2873 Mon Sep 17 00:00:00 2001 From: Pouzor Date: Tue, 7 Jul 2026 10:32:47 +0200 Subject: [PATCH] fix: keep mesh/cluster links so edges resolve onto a second canvas Approving the same zigbee/zwave/proxmox devices onto a second design placed the nodes but drew no edges. _resolve_pending_links_for_ieee deleted each pending_device_link after materializing its edge, so the first approve consumed the whole topology and later approves had nothing to resolve. Links are topology, not one-shot: every importer wipes+reinserts its link set on each import, so they can safely persist across approvals. - keep the link rows (drop both db.delete(link) calls) - scope resolution to the target design (Node.ieee_address + design_id) so a re-approve links that canvas's nodes, not another's - thread design_id through all three approve call sites Existing links deleted by the old code do not come back on their own; a re-import repopulates them. ha-relevant: maybe --- backend/app/api/routes/scan.py | 47 +++++++++++++++++----------- backend/tests/test_proxmox_router.py | 38 +++++++++++++++++++++- backend/tests/test_scan.py | 6 ++-- 3 files changed, 69 insertions(+), 22 deletions(-) diff --git a/backend/app/api/routes/scan.py b/backend/app/api/routes/scan.py index 8198c3c..49e1db1 100644 --- a/backend/app/api/routes/scan.py +++ b/backend/app/api/routes/scan.py @@ -393,7 +393,9 @@ async def bulk_approve_devices( all_edges: list[dict[str, str]] = [] for device in approved_devices: - all_edges.extend(await _resolve_pending_links_for_ieee(db, device.ieee_address)) + all_edges.extend( + await _resolve_pending_links_for_ieee(db, device.ieee_address, default_design_id) + ) await db.commit() return { @@ -500,7 +502,9 @@ async def approve_device( device.vendor, device.model, device.lqi, ) if wireless else [], ) - edges = await _resolve_pending_links_for_ieee(db, device.ieee_address) + edges = await _resolve_pending_links_for_ieee( + db, device.ieee_address, node_design_id + ) await db.commit() return { "approved": True, @@ -539,7 +543,7 @@ async def approve_device( await db.flush() node_id = node.id - edges = await _resolve_pending_links_for_ieee(db, device.ieee_address) + edges = await _resolve_pending_links_for_ieee(db, device.ieee_address, node_design_id) await db.commit() return { @@ -573,14 +577,17 @@ async def _is_proxmox_cluster_member(db: AsyncSession, ieee: str | None) -> bool async def _resolve_pending_links_for_ieee( - db: AsyncSession, ieee: str | None + db: AsyncSession, ieee: str | None, design_id: str | None ) -> list[dict[str, str]]: - """Materialize edges for any pending_device_links involving ``ieee``. + """Materialize edges for any pending_device_links involving ``ieee`` on the + canvas identified by ``design_id``. - For each link where the other endpoint already exists as a canvas Node - (matched by ``Node.ieee_address``), create the Edge and drop the link - row. Links where the other endpoint is still pending are kept so they - can resolve when that endpoint is approved later. + For each link where the other endpoint already exists as a Node *on this + design* (matched by ``Node.ieee_address`` + ``Node.design_id``), create the + Edge. Links are **never** deleted here: they describe the discovered mesh / + cluster topology and are wiped+reinserted wholesale on the next import + (zigbee/zwave/proxmox). Keeping them lets the same devices be re-approved + onto a second canvas with their edges intact. """ if not ieee: return [] @@ -595,14 +602,19 @@ async def _resolve_pending_links_for_ieee( if not links: return [] - # Map every relevant ieee → Node (single query). + # Map every relevant ieee → Node *on the target design* (single query). + # Scoping by design is what makes a re-approve onto a second canvas link the + # nodes of THAT canvas, not stale nodes left on another one. other_ieees = { link.target_ieee if link.source_ieee == ieee else link.source_ieee for link in links } other_ieees.add(ieee) nodes_q = await db.execute( - select(Node).where(Node.ieee_address.in_(other_ieees)) + select(Node).where( + Node.ieee_address.in_(other_ieees), + Node.design_id == design_id, + ) ) by_ieee = {n.ieee_address: n for n in nodes_q.scalars().all() if n.ieee_address} @@ -633,15 +645,13 @@ async def _resolve_pending_links_for_ieee( src_id, tgt_id = self_node.id, other_node.id else: src_id, tgt_id = other_node.id, self_node.id - # Skip if either direction already exists. + # Skip if either direction already exists on this design (re-approve or + # a manually drawn link). The link row is kept for other designs. if (src_id, tgt_id) in existing_pairs or (tgt_id, src_id) in existing_pairs: - await db.delete(link) continue - # Use the source node's design_id for the edge - edge_design_id = self_node.design_id if self_node else None - if edge_design_id is None: - first = (await db.execute(select(Design).order_by(Design.created_at).limit(1))).scalar() - edge_design_id = first.id if first else None + # Edge lands on the design we're approving into (the nodes above are + # already scoped to it). + edge_design_id = design_id or (self_node.design_id if self_node else None) # Edge shape by link source. Handle IDs are the *bare* slot-0 side names # (the canonical stored form — the save path normalizes '-t' → the # bare source id, and React Flow resolves the bare id to that side). A @@ -677,7 +687,6 @@ async def _resolve_pending_links_for_ieee( "source_handle": src_handle, "target_handle": tgt_handle, }) - await db.delete(link) return created diff --git a/backend/tests/test_proxmox_router.py b/backend/tests/test_proxmox_router.py index b9fe684..e89d280 100644 --- a/backend/tests/test_proxmox_router.py +++ b/backend/tests/test_proxmox_router.py @@ -389,7 +389,7 @@ async def test_cluster_link_resolves_to_cluster_edge(db_session) -> None: )) await db_session.commit() - await _resolve_pending_links_for_ieee(db_session, "pve-node-a") + await _resolve_pending_links_for_ieee(db_session, "pve-node-a", design.id) edge = (await db_session.execute(select(Edge))).scalars().one() assert edge.type == "cluster" @@ -399,6 +399,42 @@ async def test_cluster_link_resolves_to_cluster_edge(db_session) -> None: assert edge.target_handle == "left" +@pytest.mark.asyncio +async def test_link_survives_and_resolves_onto_second_design(db_session) -> None: + # Regression: approving the same mesh devices onto a second canvas must also + # get their edges. The link row is topology, not one-shot — resolving it on + # design A must not consume it, so design B resolves too. + da = Design(id=str(uuid.uuid4()), name="a") + db_ = Design(id=str(uuid.uuid4()), name="b") + db_session.add_all([da, db_]) + # Same two devices placed on BOTH designs (one Node per canvas). + for d in (da, db_): + db_session.add_all([ + Node(id=str(uuid.uuid4()), type="iot", label="x", ieee_address="0xAAAA", + status="online", pos_x=0, pos_y=0, design_id=d.id), + Node(id=str(uuid.uuid4()), type="iot", label="y", ieee_address="0xBBBB", + status="online", pos_x=0, pos_y=0, design_id=d.id), + ]) + db_session.add(PendingDeviceLink( + id=str(uuid.uuid4()), source_ieee="0xAAAA", target_ieee="0xBBBB", + discovery_source="zigbee", + )) + await db_session.commit() + + first = await _resolve_pending_links_for_ieee(db_session, "0xAAAA", da.id) + second = await _resolve_pending_links_for_ieee(db_session, "0xAAAA", db_.id) + + assert len(first) == 1 + assert len(second) == 1 # would be 0 if the link were consumed on design A + # One iot edge per design, each between that design's own nodes. + edges = (await db_session.execute(select(Edge))).scalars().all() + assert len(edges) == 2 + assert {e.design_id for e in edges} == {da.id, db_.id} + assert all(e.type == "iot" for e in edges) + # The link row is still present for any further design. + assert (await db_session.execute(select(PendingDeviceLink))).scalars().one() + + @pytest.mark.asyncio async def test_persist_never_deletes(db_session) -> None: await _persist_pending_import(db_session, [_guest_node(101, "10.0.0.5")], []) diff --git a/backend/tests/test_scan.py b/backend/tests/test_scan.py index 8e7834d..4af6a90 100644 --- a/backend/tests/test_scan.py +++ b/backend/tests/test_scan.py @@ -1375,7 +1375,9 @@ async def test_approve_zigbee_skips_when_other_endpoint_still_pending( async def test_approve_zigbee_resolves_link_after_second_approval( client: AsyncClient, headers, db_session ): - """First approval keeps link; second approval creates the edge.""" + """First approval keeps link (other endpoint pending); second approval + creates the edge. The link row is retained afterwards so the same pair can + be re-approved onto another canvas — it's topology, wiped only on reimport.""" from sqlalchemy import select from app.db.models import Edge, PendingDevice, PendingDeviceLink @@ -1408,7 +1410,7 @@ async def test_approve_zigbee_resolves_link_after_second_approval( edges = (await db_session.execute(select(Edge))).scalars().all() assert len(edges) == 1 links = (await db_session.execute(select(PendingDeviceLink))).scalars().all() - assert links == [] # consumed + assert len(links) == 1 # retained for re-approval onto other canvases # --- Deep scan: trigger overrides + config persistence ---