feat(zigbee): populate IEEE/Vendor/Model/LQI properties on approve and re-import
- Approval (single + bulk) of zigbee pending devices now writes IEEE, Vendor, Model, LQI into Node.properties so they show in the right panel. - Zigbee re-import refreshes properties on existing canvas Nodes and skips creating a pending row when the device was already approved — keeps approved devices out of pending/hidden modals. - Coordinator Node also receives the same properties on first creation and on re-import. - Remove the Parent Container selector from the add/edit node modal.
This commit is contained in:
@@ -15,6 +15,9 @@ from app.db.models import Edge, Node, PendingDevice, PendingDeviceLink, ScanRun
|
||||
from app.schemas.nodes import NodeCreate
|
||||
from app.schemas.scan import PendingDeviceResponse, ScanRunResponse
|
||||
from app.services.scanner import request_cancel, run_scan
|
||||
from app.services.zigbee_service import build_zigbee_properties
|
||||
|
||||
_ZIGBEE_TYPES = {"zigbee_coordinator", "zigbee_router", "zigbee_enddevice"}
|
||||
|
||||
|
||||
class BulkActionRequest(BaseModel):
|
||||
@@ -125,17 +128,22 @@ async def bulk_approve_devices(
|
||||
created_nodes: list[Node] = []
|
||||
for device in devices:
|
||||
device.status = "approved"
|
||||
node_type = device.suggested_type or "generic"
|
||||
is_zigbee = node_type in _ZIGBEE_TYPES
|
||||
node = Node(
|
||||
label=device.hostname or device.friendly_name or device.ip or "device",
|
||||
type=device.suggested_type or "generic",
|
||||
type=node_type,
|
||||
ip=device.ip,
|
||||
hostname=device.hostname,
|
||||
status="unknown",
|
||||
status="online" if is_zigbee else "unknown",
|
||||
services=device.services or [],
|
||||
ieee_address=device.ieee_address,
|
||||
properties=build_zigbee_properties(
|
||||
device.ieee_address, device.vendor, device.model, device.lqi
|
||||
) if is_zigbee else [],
|
||||
# Default to ping so the status checker actually polls the new node.
|
||||
# Without this the scheduler skips it (check_method NULL → no check).
|
||||
check_method="ping" if device.ip else None,
|
||||
check_method="none" if is_zigbee else ("ping" if device.ip else None),
|
||||
)
|
||||
db.add(node)
|
||||
created_nodes.append(node)
|
||||
@@ -225,8 +233,7 @@ async def approve_device(
|
||||
if device.status != "pending":
|
||||
raise HTTPException(status_code=409, detail="Device already processed")
|
||||
device.status = "approved"
|
||||
_zigbee_types = {"zigbee_coordinator", "zigbee_router", "zigbee_enddevice"}
|
||||
_is_zigbee = node_data.type in _zigbee_types
|
||||
_is_zigbee = node_data.type in _ZIGBEE_TYPES
|
||||
node = Node(
|
||||
label=node_data.label,
|
||||
type=node_data.type,
|
||||
@@ -235,6 +242,9 @@ async def approve_device(
|
||||
status="online" if _is_zigbee else node_data.status,
|
||||
services=node_data.services or [],
|
||||
ieee_address=device.ieee_address,
|
||||
properties=build_zigbee_properties(
|
||||
device.ieee_address, device.vendor, device.model, device.lqi
|
||||
) if _is_zigbee else (node_data.properties or []),
|
||||
check_method="none" if _is_zigbee else (node_data.check_method or ("ping" if node_data.ip else None)),
|
||||
check_target=None if _is_zigbee else node_data.check_target,
|
||||
)
|
||||
|
||||
@@ -23,7 +23,7 @@ from app.schemas.zigbee import (
|
||||
ZigbeeTestConnectionRequest,
|
||||
ZigbeeTestConnectionResponse,
|
||||
)
|
||||
from app.services.zigbee_service import fetch_networkmap, test_mqtt_connection
|
||||
from app.services.zigbee_service import build_zigbee_properties, fetch_networkmap, test_mqtt_connection
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
router = APIRouter()
|
||||
@@ -142,10 +142,15 @@ async def _persist_pending_import(
|
||||
ieee = n.get("ieee_address")
|
||||
if not ieee:
|
||||
continue
|
||||
props = build_zigbee_properties(
|
||||
ieee, n.get("vendor"), n.get("model"), n.get("lqi")
|
||||
)
|
||||
|
||||
if n.get("device_type") == "Coordinator":
|
||||
existing = await db.execute(select(Node).where(Node.ieee_address == ieee))
|
||||
existing_node = existing.scalar_one_or_none()
|
||||
if existing_node:
|
||||
existing_node.properties = props
|
||||
coordinator_out = ZigbeeCoordinatorOut(
|
||||
id=existing_node.id,
|
||||
label=existing_node.label,
|
||||
@@ -161,6 +166,7 @@ async def _persist_pending_import(
|
||||
check_method="none",
|
||||
ieee_address=ieee,
|
||||
services=[],
|
||||
properties=props,
|
||||
)
|
||||
db.add(node)
|
||||
await db.flush()
|
||||
@@ -169,6 +175,17 @@ async def _persist_pending_import(
|
||||
)
|
||||
continue
|
||||
|
||||
# If the device has already been approved as a canvas Node, refresh
|
||||
# its properties and skip creating a pending row (keeps approved
|
||||
# devices out of pending/hidden modals on re-import).
|
||||
existing_node_q = await db.execute(
|
||||
select(Node).where(Node.ieee_address == ieee)
|
||||
)
|
||||
existing_node = existing_node_q.scalar_one_or_none()
|
||||
if existing_node:
|
||||
existing_node.properties = props
|
||||
continue
|
||||
|
||||
result = await db.execute(
|
||||
select(PendingDevice).where(PendingDevice.ieee_address == ieee)
|
||||
)
|
||||
|
||||
@@ -58,6 +58,29 @@ def _build_tls_context(insecure: bool) -> ssl.SSLContext:
|
||||
return ctx
|
||||
|
||||
|
||||
def build_zigbee_properties(
|
||||
ieee: str | None,
|
||||
vendor: str | None,
|
||||
model: str | None,
|
||||
lqi: int | None,
|
||||
) -> list[dict[str, Any]]:
|
||||
"""Build a NodeProperty list for a Zigbee device (IEEE, Vendor, Model, LQI).
|
||||
|
||||
Only includes a row when the value is non-empty. Shape matches the
|
||||
frontend ``NodeProperty`` type: ``{key, value, icon, visible}``.
|
||||
"""
|
||||
props: list[dict[str, Any]] = []
|
||||
if ieee:
|
||||
props.append({"key": "IEEE", "value": ieee, "icon": None, "visible": True})
|
||||
if vendor:
|
||||
props.append({"key": "Vendor", "value": vendor, "icon": None, "visible": True})
|
||||
if model:
|
||||
props.append({"key": "Model", "value": model, "icon": None, "visible": True})
|
||||
if lqi is not None:
|
||||
props.append({"key": "LQI", "value": str(lqi), "icon": None, "visible": True})
|
||||
return props
|
||||
|
||||
|
||||
def _z2m_type_to_homelable(device_type: str) -> str:
|
||||
"""Map a Z2M device type string to a homelable node type."""
|
||||
mapping = {
|
||||
|
||||
@@ -528,6 +528,87 @@ async def test_bulk_approve_approves_devices(client: AsyncClient, headers, two_p
|
||||
assert pending_res.json() == []
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def zigbee_pending_device(db_session):
|
||||
device = PendingDevice(
|
||||
id=str(uuid.uuid4()),
|
||||
ip=None,
|
||||
mac=None,
|
||||
hostname=None,
|
||||
friendly_name="bulb_1",
|
||||
services=[],
|
||||
suggested_type="zigbee_enddevice",
|
||||
device_subtype="EndDevice",
|
||||
ieee_address="0xABCDEF",
|
||||
vendor="IKEA",
|
||||
model="TRADFRI",
|
||||
lqi=180,
|
||||
status="pending",
|
||||
discovery_source="zigbee",
|
||||
)
|
||||
db_session.add(device)
|
||||
await db_session.commit()
|
||||
await db_session.refresh(device)
|
||||
return device
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_approve_zigbee_device_populates_properties(
|
||||
client: AsyncClient, headers, zigbee_pending_device, db_session
|
||||
):
|
||||
"""Approving a zigbee device must populate IEEE/Vendor/Model/LQI in properties."""
|
||||
from sqlalchemy import select
|
||||
|
||||
from app.db.models import Node as NodeModel
|
||||
payload = {
|
||||
"label": "bulb_1",
|
||||
"type": "zigbee_enddevice",
|
||||
"status": "online",
|
||||
"services": [],
|
||||
"check_method": "none",
|
||||
}
|
||||
res = await client.post(
|
||||
f"/api/v1/scan/pending/{zigbee_pending_device.id}/approve",
|
||||
json=payload,
|
||||
headers=headers,
|
||||
)
|
||||
assert res.status_code == 200
|
||||
node = (
|
||||
await db_session.execute(select(NodeModel).where(NodeModel.ieee_address == "0xABCDEF"))
|
||||
).scalar_one()
|
||||
keys = {p["key"]: p["value"] for p in node.properties}
|
||||
assert keys == {
|
||||
"IEEE": "0xABCDEF",
|
||||
"Vendor": "IKEA",
|
||||
"Model": "TRADFRI",
|
||||
"LQI": "180",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bulk_approve_zigbee_populates_properties(
|
||||
client: AsyncClient, headers, zigbee_pending_device, db_session
|
||||
):
|
||||
from sqlalchemy import select
|
||||
|
||||
from app.db.models import Node as NodeModel
|
||||
res = await client.post(
|
||||
"/api/v1/scan/pending/bulk-approve",
|
||||
json={"device_ids": [zigbee_pending_device.id]},
|
||||
headers=headers,
|
||||
)
|
||||
assert res.status_code == 200
|
||||
node = (
|
||||
await db_session.execute(select(NodeModel).where(NodeModel.ieee_address == "0xABCDEF"))
|
||||
).scalar_one()
|
||||
keys = {p["key"]: p["value"] for p in node.properties}
|
||||
assert keys["IEEE"] == "0xABCDEF"
|
||||
assert keys["Vendor"] == "IKEA"
|
||||
assert keys["Model"] == "TRADFRI"
|
||||
assert keys["LQI"] == "180"
|
||||
assert node.check_method == "none"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bulk_approve_sets_default_check_method(client: AsyncClient, headers, two_pending_devices, db_session):
|
||||
"""Approved devices with an IP must default to ping; otherwise scheduler skips them."""
|
||||
|
||||
@@ -388,6 +388,97 @@ async def test_persist_pending_import_replaces_links(db_session) -> None:
|
||||
assert (rows[0].source_ieee, rows[0].target_ieee) == ("0xCOORD", "0xR1")
|
||||
|
||||
|
||||
@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."""
|
||||
from sqlalchemy import select
|
||||
|
||||
from app.api.routes.zigbee import _persist_pending_import
|
||||
from app.db.models import Node
|
||||
|
||||
nodes_with_meta = [dict(n) for n in _PENDING_NODES]
|
||||
nodes_with_meta[0]["vendor"] = "TI"
|
||||
nodes_with_meta[0]["model"] = "CC2652"
|
||||
|
||||
await _persist_pending_import(db_session, nodes_with_meta, _PENDING_EDGES)
|
||||
|
||||
coord = (
|
||||
await db_session.execute(select(Node).where(Node.ieee_address == "0xCOORD"))
|
||||
).scalar_one()
|
||||
keys = {p["key"]: p["value"] for p in coord.properties}
|
||||
assert keys == {"IEEE": "0xCOORD", "Vendor": "TI", "Model": "CC2652"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_persist_pending_import_skips_pending_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.
|
||||
"""
|
||||
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.
|
||||
approved = Node(
|
||||
label="router_1",
|
||||
type="zigbee_router",
|
||||
status="online",
|
||||
check_method="none",
|
||||
ieee_address="0xR1",
|
||||
services=[],
|
||||
properties=[],
|
||||
)
|
||||
db_session.add(approved)
|
||||
await db_session.commit()
|
||||
|
||||
bumped = [dict(n) for n in _PENDING_NODES]
|
||||
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 = (
|
||||
await db_session.execute(
|
||||
select(PendingDevice).where(PendingDevice.ieee_address == "0xR1")
|
||||
)
|
||||
).scalars().all()
|
||||
assert pendings == []
|
||||
|
||||
# Node properties got refreshed.
|
||||
refreshed = (
|
||||
await db_session.execute(select(Node).where(Node.ieee_address == "0xR1"))
|
||||
).scalar_one()
|
||||
keys = {p["key"]: p["value"] for p in refreshed.properties}
|
||||
assert keys == {"IEEE": "0xR1", "Vendor": "TI", "Model": "CC2530", "LQI": "250"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_persist_pending_import_refreshes_existing_coordinator_properties(
|
||||
db_session,
|
||||
) -> None:
|
||||
from sqlalchemy import select
|
||||
|
||||
from app.api.routes.zigbee import _persist_pending_import
|
||||
from app.db.models import Node
|
||||
|
||||
await _persist_pending_import(db_session, _PENDING_NODES, _PENDING_EDGES)
|
||||
|
||||
bumped = [dict(n) for n in _PENDING_NODES]
|
||||
bumped[0]["vendor"] = "TI"
|
||||
bumped[0]["model"] = "CC2652"
|
||||
await _persist_pending_import(db_session, bumped, _PENDING_EDGES)
|
||||
|
||||
coord = (
|
||||
await db_session.execute(select(Node).where(Node.ieee_address == "0xCOORD"))
|
||||
).scalar_one()
|
||||
keys = {p["key"]: p["value"] for p in coord.properties}
|
||||
assert keys["Vendor"] == "TI"
|
||||
assert keys["Model"] == "CC2652"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_import_pending_requires_auth(client: AsyncClient) -> None:
|
||||
res = await client.post(
|
||||
|
||||
@@ -552,9 +552,6 @@ export default function App() {
|
||||
onClose={() => setAddNodeOpen(false)}
|
||||
onSubmit={handleAddNode}
|
||||
title="Add Node"
|
||||
parentContainerNodes={nodes
|
||||
.filter((n) => CONTAINER_MODE_TYPES.has(n.data.type) && n.data.container_mode)
|
||||
.map((n) => ({ id: n.id, label: n.data.label, nodeType: n.data.type }))}
|
||||
/>
|
||||
|
||||
{/* key forces re-mount when editing a different node, resetting form state */}
|
||||
@@ -565,9 +562,6 @@ export default function App() {
|
||||
onSubmit={handleUpdateNode}
|
||||
initial={editNode?.data}
|
||||
title="Edit Node"
|
||||
parentContainerNodes={nodes
|
||||
.filter((n) => n.id !== editNodeId && CONTAINER_MODE_TYPES.has(n.data.type) && n.data.container_mode)
|
||||
.map((n) => ({ id: n.id, label: n.data.label, nodeType: n.data.type }))}
|
||||
/>
|
||||
|
||||
<EdgeModal
|
||||
|
||||
@@ -54,12 +54,11 @@ interface NodeModalProps {
|
||||
onSubmit: (data: Partial<NodeData>) => void
|
||||
initial?: Partial<NodeData>
|
||||
title?: string
|
||||
parentContainerNodes?: { id: string; label: string; nodeType?: NodeType }[]
|
||||
}
|
||||
|
||||
// NodeModal is always mounted with a key that changes on open/edit, so useState
|
||||
// initial value is enough - no need for a reset effect.
|
||||
export function NodeModal({ open, onClose, onSubmit, initial, title = 'Add Node', parentContainerNodes = [] }: NodeModalProps) {
|
||||
export function NodeModal({ open, onClose, onSubmit, initial, title = 'Add Node' }: NodeModalProps) {
|
||||
const merged = { ...DEFAULT_DATA, ...initial }
|
||||
if (ZIGBEE_TYPES.includes((merged.type ?? '') as NodeType)) merged.check_method = 'none'
|
||||
const [form, setForm] = useState<Partial<NodeData>>(merged)
|
||||
@@ -94,10 +93,6 @@ export function NodeModal({ open, onClose, onSubmit, initial, title = 'Add Node'
|
||||
onClose()
|
||||
}
|
||||
|
||||
const filteredParentNodes = form.type === 'docker_container'
|
||||
? parentContainerNodes.filter((n) => n.nodeType === 'docker_host')
|
||||
: parentContainerNodes
|
||||
|
||||
return (
|
||||
<Dialog open={open} onOpenChange={(o) => !o && onClose()}>
|
||||
<DialogContent className="bg-[#161b22] border-[#30363d] text-foreground max-w-md max-h-[90vh] overflow-y-auto">
|
||||
@@ -325,31 +320,6 @@ export function NodeModal({ open, onClose, onSubmit, initial, title = 'Add Node'
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Parent container */}
|
||||
{form.type !== 'groupRect' && form.type !== 'group' && filteredParentNodes.length > 0 && (
|
||||
<div className="flex flex-col gap-1.5 col-span-2">
|
||||
<Label className="text-xs text-muted-foreground">Parent Container</Label>
|
||||
<Select
|
||||
value={form.parent_id ?? 'none'}
|
||||
onValueChange={(v) => set('parent_id', v === 'none' ? undefined : v)}
|
||||
>
|
||||
<SelectTrigger className={`bg-[#21262d] border-[#30363d] text-sm h-8 cursor-pointer ${modalStyles['modal-interactive']} ${modalStyles['modal-radius']}`} aria-label="Parent container selector">
|
||||
<SelectValue placeholder="None (standalone)">
|
||||
{form.parent_id
|
||||
? (filteredParentNodes.find((n) => n.id === form.parent_id)?.label ?? 'None (standalone)')
|
||||
: 'None (standalone)'}
|
||||
</SelectValue>
|
||||
</SelectTrigger>
|
||||
<SelectContent className="bg-[#21262d] border-[#30363d]">
|
||||
<SelectItem value="none" className="text-sm">None (standalone)</SelectItem>
|
||||
{filteredParentNodes.map((n) => (
|
||||
<SelectItem key={n.id} value={n.id} className="text-sm">{n.label}</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Container mode */}
|
||||
{CONTAINER_MODE_TYPES.includes((form.type ?? 'generic') as NodeType) && (
|
||||
<div className="flex items-center justify-between col-span-2 py-1">
|
||||
|
||||
@@ -311,47 +311,15 @@ describe('NodeModal', () => {
|
||||
expect(screen.queryByText('Reset to defaults')).toBeNull()
|
||||
})
|
||||
|
||||
// ── Parent Proxmox (vm / lxc only) ───────────────────────────────────
|
||||
// ── Parent Container selector removed ────────────────────────────────
|
||||
|
||||
const parentContainerVisibleTypes = ['proxmox', 'vm', 'lxc', 'docker_host', 'isp', 'router', 'switch', 'server', 'nas', 'ap', 'printer', 'iot', 'camera', 'cpl', 'computer', 'generic'] as const
|
||||
const parentContainerHiddenTypes = ['groupRect', 'group'] as const
|
||||
|
||||
it.each(parentContainerVisibleTypes)('shows Parent Container for %s type when options are provided', (type) => {
|
||||
renderModal({
|
||||
initial: { ...BASE, type },
|
||||
parentContainerNodes: [{ id: 'c1', label: 'Container 01' }],
|
||||
})
|
||||
expect(screen.getByText('Parent Container')).toBeDefined()
|
||||
expect(screen.getByText('Container 01')).toBeDefined()
|
||||
})
|
||||
|
||||
it.each(parentContainerHiddenTypes)('hides Parent Container for %s type even when options are provided', (type) => {
|
||||
renderModal({ initial: { ...BASE, type }, parentContainerNodes: [{ id: 'c1', label: 'Container 01' }] })
|
||||
it('does not render the Parent Container selector', () => {
|
||||
renderModal({ initial: BASE })
|
||||
expect(screen.queryByText('Parent Container')).toBeNull()
|
||||
})
|
||||
|
||||
it.each(parentContainerVisibleTypes)('hides Parent Container for %s type when no container options are available', (type) => {
|
||||
renderModal({ initial: { ...BASE, type } })
|
||||
expect(screen.queryByText('Parent Container')).toBeNull()
|
||||
})
|
||||
|
||||
it('docker_container shows only docker_host parents', () => {
|
||||
renderModal({
|
||||
initial: { ...BASE, type: 'docker_container' },
|
||||
parentContainerNodes: [
|
||||
{ id: 'h1', label: 'My Docker Host', nodeType: 'docker_host' },
|
||||
{ id: 'p1', label: 'My Proxmox', nodeType: 'proxmox' },
|
||||
],
|
||||
})
|
||||
expect(screen.getByText('My Docker Host')).toBeDefined()
|
||||
expect(screen.queryByText('My Proxmox')).toBeNull()
|
||||
})
|
||||
|
||||
it('docker_container hides Parent Container when no docker_host is available', () => {
|
||||
renderModal({
|
||||
initial: { ...BASE, type: 'docker_container' },
|
||||
parentContainerNodes: [{ id: 'p1', label: 'My Proxmox', nodeType: 'proxmox' }],
|
||||
})
|
||||
it('does not render Parent Container for docker_container either', () => {
|
||||
renderModal({ initial: { ...BASE, type: 'docker_container' } })
|
||||
expect(screen.queryByText('Parent Container')).toBeNull()
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user