fix: send zigbee/zwave coordinator to pending inventory, not auto-canvas
The coordinator was special-cased in both mesh imports to auto-create a canvas Node, so it never appeared in the pending inventory and users could not approve/hide/type it like every other device. Remove the auto-placement: the coordinator now flows through the shared pending path (upsert into pending_devices with suggested_type zigbee_coordinator / zwave_coordinator). An already-approved coordinator Node still gets its properties refreshed on re-import via the shared approved-node path. The response's coordinator/coordinator_already_existed fields are retained (now always unset) for backward-compatible shape; the frontend already ignored them. Tests updated: coordinator lands in pending (counts include it), no Node auto-created, pending metadata carried, approved coordinator refresh + no pending re-list. ha-relevant: yes
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user