diff --git a/backend/app/api/routes/scan.py b/backend/app/api/routes/scan.py index 13c2b31..0ef6398 100644 --- a/backend/app/api/routes/scan.py +++ b/backend/app/api/routes/scan.py @@ -308,15 +308,38 @@ async def bulk_approve_devices( 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 + # Accept every selected device that isn't user-hidden. We intentionally do NOT + # filter on status == "pending": a device's status is global, but canvas + # membership is per-design. A device approved onto another canvas (or whose + # node was later deleted) must still be placeable on THIS design. Duplicates + # are guarded per-design below, not by the global status flag. result = await db.execute( select(PendingDevice).where( PendingDevice.id.in_(payload.device_ids), - PendingDevice.status == "pending", + PendingDevice.status != "hidden", ) ) devices = result.scalars().all() + + # What already sits on the target canvas, so we skip devices already placed + # here (by ip or ieee_address) instead of creating duplicate nodes. + existing = ( + await db.execute( + select(Node.ip, Node.ieee_address).where(Node.design_id == default_design_id) + ) + ).all() + placed_ips = {ip for ip, _ in existing if ip} + placed_ieee = {ieee for _, ieee in existing if ieee} + created_nodes: list[Node] = [] + approved_devices: list[PendingDevice] = [] for device in devices: + already_here = ( + (device.ip is not None and device.ip in placed_ips) + or (device.ieee_address is not None and device.ieee_address in placed_ieee) + ) + if already_here: + continue device.status = "approved" node_type = device.suggested_type or "generic" is_wireless = _is_wireless(node_type) @@ -339,12 +362,20 @@ async def bulk_approve_devices( ) db.add(node) created_nodes.append(node) + approved_devices.append(device) + # Track within this batch so a duplicate selection (same ip/ieee) is not + # placed twice on the same canvas. + if device.ip: + placed_ips.add(device.ip) + if device.ieee_address: + placed_ieee.add(device.ieee_address) await db.flush() # populates node.id from Python-side default before reading + # node_ids and approved_device_ids stay index-aligned for the client's mapping. node_ids = [n.id for n in created_nodes] - approved_device_ids = [d.id for d in devices] + approved_device_ids = [d.id for d in approved_devices] all_edges: list[dict[str, str]] = [] - for device in devices: + for device in approved_devices: all_edges.extend(await _resolve_pending_links_for_ieee(db, device.ieee_address)) await db.commit() diff --git a/backend/tests/test_scan.py b/backend/tests/test_scan.py index 0f09fbe..3b05eb0 100644 --- a/backend/tests/test_scan.py +++ b/backend/tests/test_scan.py @@ -792,6 +792,75 @@ async def test_bulk_approve_approves_devices(client: AsyncClient, headers, two_p assert all(d["status"] == "approved" for d in inventory) +@pytest.mark.asyncio +async def test_bulk_approve_places_already_approved_device_on_another_design( + client: AsyncClient, headers, db_session, two_pending_devices +): + """Regression: a device already approved (status='approved', e.g. placed on + another canvas) must still get a node on the design being approved onto. + + Previously bulk-approve filtered status=='pending', so selecting an + already-approved device created no node — the user saw fewer nodes than + they selected.""" + ids = [d.id for d in two_pending_devices] + design_a = await _add_design(db_session, "Canvas A") + design_b = await _add_design(db_session, "Canvas B") + + # Approve both onto design A. + res_a = await client.post( + "/api/v1/scan/pending/bulk-approve", + json={"device_ids": ids, "design_id": design_a}, + headers=headers, + ) + assert res_a.json()["approved"] == 2 + + # Re-approve the same (now status='approved') devices onto design B. + res_b = await client.post( + "/api/v1/scan/pending/bulk-approve", + json={"device_ids": ids, "design_id": design_b}, + headers=headers, + ) + data_b = res_b.json() + assert data_b["approved"] == 2, "already-approved devices must place onto the new canvas" + assert data_b["skipped"] == 0 + + # Two nodes now exist on each design. + from app.db.models import Node as NodeModel + nodes_b = ( + await db_session.execute(select(NodeModel).where(NodeModel.design_id == design_b)) + ).scalars().all() + assert len(nodes_b) == 2 + + +@pytest.mark.asyncio +async def test_bulk_approve_skips_device_already_on_target_design( + client: AsyncClient, headers, db_session, two_pending_devices +): + """A device already on the target canvas (same ip) is not placed twice.""" + ids = [d.id for d in two_pending_devices] + design = await _add_design(db_session, "Canvas") + # First device already sits on the canvas (matched by ip). + db_session.add(_node(design, ip="192.168.1.10")) + await db_session.commit() + + res = await client.post( + "/api/v1/scan/pending/bulk-approve", + json={"device_ids": ids, "design_id": design}, + headers=headers, + ) + data = res.json() + assert data["approved"] == 1 # only the second device (192.168.1.11) + assert data["skipped"] == 1 + + from app.db.models import Node as NodeModel + nodes = ( + await db_session.execute(select(NodeModel).where(NodeModel.design_id == design)) + ).scalars().all() + # The pre-existing node plus the one newly approved — no duplicate for .10. + assert len(nodes) == 2 + assert sorted(n.ip for n in nodes) == ["192.168.1.10", "192.168.1.11"] + + @pytest.fixture async def zigbee_pending_device(db_session): device = PendingDevice(