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
This commit is contained in:
@@ -393,7 +393,9 @@ async def bulk_approve_devices(
|
|||||||
|
|
||||||
all_edges: list[dict[str, str]] = []
|
all_edges: list[dict[str, str]] = []
|
||||||
for device in approved_devices:
|
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()
|
await db.commit()
|
||||||
return {
|
return {
|
||||||
@@ -500,7 +502,9 @@ async def approve_device(
|
|||||||
device.vendor, device.model, device.lqi,
|
device.vendor, device.model, device.lqi,
|
||||||
) if wireless else [],
|
) 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()
|
await db.commit()
|
||||||
return {
|
return {
|
||||||
"approved": True,
|
"approved": True,
|
||||||
@@ -539,7 +543,7 @@ async def approve_device(
|
|||||||
await db.flush()
|
await db.flush()
|
||||||
node_id = node.id
|
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()
|
await db.commit()
|
||||||
return {
|
return {
|
||||||
@@ -573,14 +577,17 @@ async def _is_proxmox_cluster_member(db: AsyncSession, ieee: str | None) -> bool
|
|||||||
|
|
||||||
|
|
||||||
async def _resolve_pending_links_for_ieee(
|
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]]:
|
) -> 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
|
For each link where the other endpoint already exists as a Node *on this
|
||||||
(matched by ``Node.ieee_address``), create the Edge and drop the link
|
design* (matched by ``Node.ieee_address`` + ``Node.design_id``), create the
|
||||||
row. Links where the other endpoint is still pending are kept so they
|
Edge. Links are **never** deleted here: they describe the discovered mesh /
|
||||||
can resolve when that endpoint is approved later.
|
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:
|
if not ieee:
|
||||||
return []
|
return []
|
||||||
@@ -595,14 +602,19 @@ async def _resolve_pending_links_for_ieee(
|
|||||||
if not links:
|
if not links:
|
||||||
return []
|
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 = {
|
other_ieees = {
|
||||||
link.target_ieee if link.source_ieee == ieee else link.source_ieee
|
link.target_ieee if link.source_ieee == ieee else link.source_ieee
|
||||||
for link in links
|
for link in links
|
||||||
}
|
}
|
||||||
other_ieees.add(ieee)
|
other_ieees.add(ieee)
|
||||||
nodes_q = await db.execute(
|
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}
|
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
|
src_id, tgt_id = self_node.id, other_node.id
|
||||||
else:
|
else:
|
||||||
src_id, tgt_id = other_node.id, self_node.id
|
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:
|
if (src_id, tgt_id) in existing_pairs or (tgt_id, src_id) in existing_pairs:
|
||||||
await db.delete(link)
|
|
||||||
continue
|
continue
|
||||||
# Use the source node's design_id for the edge
|
# Edge lands on the design we're approving into (the nodes above are
|
||||||
edge_design_id = self_node.design_id if self_node else None
|
# already scoped to it).
|
||||||
if edge_design_id is None:
|
edge_design_id = design_id or (self_node.design_id if self_node else 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 shape by link source. Handle IDs are the *bare* slot-0 side names
|
# Edge shape by link source. Handle IDs are the *bare* slot-0 side names
|
||||||
# (the canonical stored form — the save path normalizes '<side>-t' → the
|
# (the canonical stored form — the save path normalizes '<side>-t' → the
|
||||||
# bare source id, and React Flow resolves the bare id to that side). A
|
# 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,
|
"source_handle": src_handle,
|
||||||
"target_handle": tgt_handle,
|
"target_handle": tgt_handle,
|
||||||
})
|
})
|
||||||
await db.delete(link)
|
|
||||||
|
|
||||||
return created
|
return created
|
||||||
|
|
||||||
|
|||||||
@@ -389,7 +389,7 @@ async def test_cluster_link_resolves_to_cluster_edge(db_session) -> None:
|
|||||||
))
|
))
|
||||||
await db_session.commit()
|
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()
|
edge = (await db_session.execute(select(Edge))).scalars().one()
|
||||||
assert edge.type == "cluster"
|
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"
|
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
|
@pytest.mark.asyncio
|
||||||
async def test_persist_never_deletes(db_session) -> None:
|
async def test_persist_never_deletes(db_session) -> None:
|
||||||
await _persist_pending_import(db_session, [_guest_node(101, "10.0.0.5")], [])
|
await _persist_pending_import(db_session, [_guest_node(101, "10.0.0.5")], [])
|
||||||
|
|||||||
@@ -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(
|
async def test_approve_zigbee_resolves_link_after_second_approval(
|
||||||
client: AsyncClient, headers, db_session
|
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 sqlalchemy import select
|
||||||
|
|
||||||
from app.db.models import Edge, PendingDevice, PendingDeviceLink
|
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()
|
edges = (await db_session.execute(select(Edge))).scalars().all()
|
||||||
assert len(edges) == 1
|
assert len(edges) == 1
|
||||||
links = (await db_session.execute(select(PendingDeviceLink))).scalars().all()
|
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 ---
|
# --- Deep scan: trigger overrides + config persistence ---
|
||||||
|
|||||||
Reference in New Issue
Block a user