diff --git a/backend/app/api/routes/scan.py b/backend/app/api/routes/scan.py index 7f0da05..5377cb6 100644 --- a/backend/app/api/routes/scan.py +++ b/backend/app/api/routes/scan.py @@ -16,8 +16,28 @@ from app.schemas.nodes import NodeCreate from app.schemas.scan import PendingDeviceResponse, ScanRunResponse from app.services.scanner import DeepScanOptions, _valid_port_range, request_cancel, run_scan from app.services.zigbee_service import build_zigbee_properties +from app.services.zwave_service import build_zwave_properties _ZIGBEE_TYPES = {"zigbee_coordinator", "zigbee_router", "zigbee_enddevice"} +_ZWAVE_TYPES = {"zwave_coordinator", "zwave_router", "zwave_enddevice"} + + +def _is_wireless(node_type: str | None) -> bool: + """Zigbee + Z-Wave mesh devices share online status / no ICMP check.""" + return node_type in _ZIGBEE_TYPES or node_type in _ZWAVE_TYPES + + +def _wireless_properties( + node_type: str | None, + ieee: str | None, + vendor: str | None, + model: str | None, + lqi: int | None, +) -> list[dict[str, Any]]: + """Build the right property rows for a mesh device (Z-Wave has no LQI).""" + if node_type in _ZWAVE_TYPES: + return build_zwave_properties(ieee, vendor, model) + return build_zigbee_properties(ieee, vendor, model, lqi) def build_mac_property(mac: str | None) -> list[dict[str, Any]]: @@ -50,6 +70,10 @@ def merge_mac_property( class BulkActionRequest(BaseModel): device_ids: list[str] + # Target design for approved nodes. Falls back to the first design when + # omitted (keeps older clients working), but the UI should send the active + # design so approved devices land on the canvas the user is looking at. + design_id: str | None = None def _check_port_ranges(v: list[str]) -> list[str]: @@ -248,9 +272,11 @@ async def bulk_approve_devices( db: AsyncSession = Depends(get_db), _: str = Depends(get_current_user), ) -> dict[str, Any]: - # 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 + # Target the design the user is on; fall back to the first design. + default_design_id = payload.design_id + if default_design_id is None: + 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 result = await db.execute( select(PendingDevice).where( @@ -263,22 +289,22 @@ async def bulk_approve_devices( for device in devices: device.status = "approved" node_type = device.suggested_type or "generic" - is_zigbee = node_type in _ZIGBEE_TYPES + is_wireless = _is_wireless(node_type) node = Node( label=device.hostname or device.friendly_name or device.ip or "device", type=node_type, ip=device.ip, mac=device.mac, hostname=device.hostname, - status="online" if is_zigbee else "unknown", + status="online" if is_wireless 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 build_mac_property(device.mac), + properties=_wireless_properties( + node_type, device.ieee_address, device.vendor, device.model, device.lqi + ) if is_wireless else build_mac_property(device.mac), # 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="none" if is_zigbee else ("ping" if device.ip else None), + check_method="none" if is_wireless else ("ping" if device.ip else None), design_id=default_design_id, ) db.add(node) @@ -375,7 +401,7 @@ async def approve_device( if device.status != "pending": raise HTTPException(status_code=409, detail="Device already processed") device.status = "approved" - _is_zigbee = node_data.type in _ZIGBEE_TYPES + wireless = _is_wireless(node_data.type) # Prefer the MAC discovered during the scan (stored on the pending device); # fall back to whatever the approve payload carried. _mac = device.mac or node_data.mac @@ -385,14 +411,14 @@ async def approve_device( ip=node_data.ip, mac=_mac, hostname=node_data.hostname, - status="online" if _is_zigbee else node_data.status, + status="online" if wireless 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 merge_mac_property(node_data.properties, _mac), - 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, + properties=_wireless_properties( + node_data.type, device.ieee_address, device.vendor, device.model, device.lqi + ) if wireless else merge_mac_property(node_data.properties, _mac), + check_method="none" if wireless else (node_data.check_method or ("ping" if node_data.ip else None)), + check_target=None if wireless else node_data.check_target, design_id=node_design_id, ) db.add(node) diff --git a/backend/tests/test_scan.py b/backend/tests/test_scan.py index dc6b518..b9213c8 100644 --- a/backend/tests/test_scan.py +++ b/backend/tests/test_scan.py @@ -1357,3 +1357,76 @@ async def test_update_scan_config_persists_deep_scan(client: AsyncClient, header ) assert res.status_code == 200 assert saved == {"http_ranges": ["9000-9100"], "probe": True} + + +# --- Z-Wave approve: active design targeting + wireless props (regression) --- + +@pytest.mark.asyncio +async def test_bulk_approve_targets_requested_design(client, headers, db_session): + """bulk-approve must place nodes on the design_id sent by the UI, not the + first design — otherwise approved devices land on the wrong canvas.""" + first = await _add_design(db_session, "Default") # first design (fallback) + active = await _add_design(db_session, "zwave") # the design the user is on + dev = PendingDevice( + id=str(uuid.uuid4()), + ieee_address="zwave-H-2", + friendly_name="Living Room Plug", + suggested_type="zwave_router", + device_subtype="Router", + vendor="Aeotec", + model="ZW096", + status="pending", + discovery_source="zwave", + ) + db_session.add(dev) + await db_session.commit() + + res = await client.post( + "/api/v1/scan/pending/bulk-approve", + json={"device_ids": [dev.id], "design_id": active}, + headers=headers, + ) + assert res.status_code == 200 + assert res.json()["approved"] == 1 + + node = ( + await db_session.execute(select(Node).where(Node.ieee_address == "zwave-H-2")) + ).scalar_one() + assert node.design_id == active + assert node.design_id != first + # Z-Wave device → online + Z-Wave property rows, no ICMP check. + assert node.status == "online" + assert node.check_method == "none" + assert {p["key"] for p in node.properties} == {"Z-Wave ID", "Vendor", "Model"} + + +@pytest.mark.asyncio +async def test_single_approve_zwave_sets_wireless_fields(client, headers, db_session): + active = await _add_design(db_session, "zwave") + dev = PendingDevice( + id=str(uuid.uuid4()), + ieee_address="zwave-H-9", + friendly_name="Door Sensor", + suggested_type="zwave_enddevice", + vendor="Aeotec", + model="ZW120", + status="pending", + discovery_source="zwave", + ) + db_session.add(dev) + await db_session.commit() + + res = await client.post( + f"/api/v1/scan/pending/{dev.id}/approve", + json={"label": "Door Sensor", "type": "zwave_enddevice", "design_id": active}, + headers=headers, + ) + assert res.status_code == 200 + + node = ( + await db_session.execute(select(Node).where(Node.ieee_address == "zwave-H-9")) + ).scalar_one() + assert node.design_id == active + assert node.status == "online" + assert node.check_method == "none" + assert any(p["key"] == "Z-Wave ID" for p in node.properties) diff --git a/frontend/src/api/__tests__/client.test.ts b/frontend/src/api/__tests__/client.test.ts index f5a129d..557c56d 100644 --- a/frontend/src/api/__tests__/client.test.ts +++ b/frontend/src/api/__tests__/client.test.ts @@ -186,7 +186,9 @@ describe('api/client', () => { mod.scanApi.ignore('d1') expect(api.post).toHaveBeenCalledWith('/scan/pending/d1/ignore') mod.scanApi.bulkApprove(['a', 'b']) - expect(api.post).toHaveBeenCalledWith('/scan/pending/bulk-approve', { device_ids: ['a', 'b'] }) + expect(api.post).toHaveBeenCalledWith('/scan/pending/bulk-approve', { device_ids: ['a', 'b'], design_id: undefined }) + mod.scanApi.bulkApprove(['a'], 'design-9') + expect(api.post).toHaveBeenCalledWith('/scan/pending/bulk-approve', { device_ids: ['a'], design_id: 'design-9' }) mod.scanApi.bulkHide(['a']) expect(api.post).toHaveBeenCalledWith('/scan/pending/bulk-hide', { device_ids: ['a'] }) mod.scanApi.restore('d1') diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 45a5958..a21755e 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -81,7 +81,7 @@ export const scanApi = { }>(`/scan/pending/${id}/approve`, nodeData), hide: (id: string) => api.post(`/scan/pending/${id}/hide`), ignore: (id: string) => api.post(`/scan/pending/${id}/ignore`), - bulkApprove: (ids: string[]) => + bulkApprove: (ids: string[], designId?: string | null) => api.post<{ approved: number node_ids: string[] @@ -89,7 +89,7 @@ export const scanApi = { edges_created: number edges: { id: string; source: string; target: string }[] skipped: number - }>('/scan/pending/bulk-approve', { device_ids: ids }), + }>('/scan/pending/bulk-approve', { device_ids: ids, design_id: designId ?? undefined }), bulkHide: (ids: string[]) => api.post<{ hidden: number; skipped: number }>('/scan/pending/bulk-hide', { device_ids: ids }), restore: (id: string) => api.post<{ restored: boolean; device_id: string }>(`/scan/pending/${id}/restore`), bulkRestore: (ids: string[]) => api.post<{ restored: number; skipped: number }>('/scan/pending/bulk-restore', { device_ids: ids }), diff --git a/frontend/src/components/modals/PendingDevicesModal.tsx b/frontend/src/components/modals/PendingDevicesModal.tsx index 933fa60..b458fea 100644 --- a/frontend/src/components/modals/PendingDevicesModal.tsx +++ b/frontend/src/components/modals/PendingDevicesModal.tsx @@ -6,6 +6,7 @@ import { import { Dialog, DialogClose, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog' import { scanApi } from '@/api/client' import { useCanvasStore } from '@/stores/canvasStore' +import { useDesignStore } from '@/stores/designStore' import { toast } from 'sonner' import { PendingDeviceModal, type PendingDevice } from '@/components/modals/PendingDeviceModal' import type { NodeType, ServiceInfo } from '@/types' @@ -127,6 +128,7 @@ export function PendingDevicesModal({ open, onClose, highlightId, initialStatus // Optionally restrict to devices that have at least one detected service. const [withServicesOnly, setWithServicesOnly] = useState(false) const { addNode, scanEventTs } = useCanvasStore() + const activeDesignId = useDesignStore((s) => s.activeDesignId) const highlightRef = useRef(null) const load = useCallback(async () => { @@ -283,6 +285,8 @@ export function PendingDevicesModal({ open, onClose, highlightId, initialStatus status: wireless ? 'online' : 'unknown', services: (device.services ?? []) as ServiceInfo[], properties, + // Approve onto the design the user is viewing, not the first design. + design_id: activeDesignId ?? undefined, } const res = await scanApi.approve(device.id, nodeData) const nodeId = res.data.node_id @@ -327,7 +331,7 @@ export function PendingDevicesModal({ open, onClose, highlightId, initialStatus const ids = [...selectedIds] if (ids.length === 0) return try { - const res = await scanApi.bulkApprove(ids) + const res = await scanApi.bulkApprove(ids, activeDesignId) const deviceToNode: Record = {} res.data.device_ids.forEach((did, i) => { deviceToNode[did] = res.data.node_ids[i] }) const approvedDevices = devices.filter((d) => ids.includes(d.id)) diff --git a/frontend/src/components/modals/__tests__/PendingDevicesModal.test.tsx b/frontend/src/components/modals/__tests__/PendingDevicesModal.test.tsx index f988d1d..92baef6 100644 --- a/frontend/src/components/modals/__tests__/PendingDevicesModal.test.tsx +++ b/frontend/src/components/modals/__tests__/PendingDevicesModal.test.tsx @@ -213,7 +213,7 @@ describe('PendingDevicesModal', () => { fireEvent.click(screen.getByTestId('pending-card-dev-a')) fireEvent.click(screen.getByTestId('pending-card-dev-b')) fireEvent.click(screen.getByRole('button', { name: /Approve \(2\)/ })) - await waitFor(() => expect(mockBulkApprove).toHaveBeenCalledWith(['dev-a', 'dev-b'])) + await waitFor(() => expect(mockBulkApprove).toHaveBeenCalledWith(['dev-a', 'dev-b'], null)) }) it('bulk approve carries the scanned MAC onto the canvas node (#168)', async () => { @@ -290,7 +290,7 @@ describe('PendingDevicesModal', () => { fireEvent.click(screen.getByRole('button', { name: 'Select mode' })) fireEvent.click(screen.getByTestId('pending-card-dev-a')) fireEvent.keyDown(window, { key: 'Enter' }) - await waitFor(() => expect(mockBulkApprove).toHaveBeenCalledWith(['dev-a'])) + await waitFor(() => expect(mockBulkApprove).toHaveBeenCalledWith(['dev-a'], null)) expect(mockBulkRestore).not.toHaveBeenCalled() })