diff --git a/backend/app/api/routes/scan.py b/backend/app/api/routes/scan.py index 4178e15..33c03df 100644 --- a/backend/app/api/routes/scan.py +++ b/backend/app/api/routes/scan.py @@ -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, ) diff --git a/backend/app/api/routes/zigbee.py b/backend/app/api/routes/zigbee.py index 2afbec3..2fd9f9a 100644 --- a/backend/app/api/routes/zigbee.py +++ b/backend/app/api/routes/zigbee.py @@ -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) ) diff --git a/backend/app/services/zigbee_service.py b/backend/app/services/zigbee_service.py index 4b6222a..faced11 100644 --- a/backend/app/services/zigbee_service.py +++ b/backend/app/services/zigbee_service.py @@ -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 = { diff --git a/backend/tests/test_scan.py b/backend/tests/test_scan.py index 46abcdc..6b4b640 100644 --- a/backend/tests/test_scan.py +++ b/backend/tests/test_scan.py @@ -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.""" diff --git a/backend/tests/test_zigbee_router.py b/backend/tests/test_zigbee_router.py index 2fc463d..04ca15a 100644 --- a/backend/tests/test_zigbee_router.py +++ b/backend/tests/test_zigbee_router.py @@ -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( diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 4c82fe8..fa561f6 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -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 }))} /> ) => void initial?: Partial 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>(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 ( !o && onClose()}> @@ -325,31 +320,6 @@ export function NodeModal({ open, onClose, onSubmit, initial, title = 'Add Node' )} - {/* Parent container */} - {form.type !== 'groupRect' && form.type !== 'group' && filteredParentNodes.length > 0 && ( -
- - -
- )} - {/* Container mode */} {CONTAINER_MODE_TYPES.includes((form.type ?? 'generic') as NodeType) && (
diff --git a/frontend/src/components/modals/__tests__/NodeModal.test.tsx b/frontend/src/components/modals/__tests__/NodeModal.test.tsx index bcc958c..ca74316 100644 --- a/frontend/src/components/modals/__tests__/NodeModal.test.tsx +++ b/frontend/src/components/modals/__tests__/NodeModal.test.tsx @@ -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() })