fix: bulk-approve places every selected device onto the active canvas
Bulk-approve filtered status=='pending', so a device already approved onto another canvas (status is global, canvas membership is per-design) — or whose node was later deleted — was silently skipped. Selecting 64 devices on an empty canvas produced only the ~28 still pending. Approve now places a node on the target design for any selected, non-hidden device that isn't already on that design (deduped by ip/ieee_address, including within the batch). Returned device_ids/node_ids stay index-aligned so the client places them all. ha-relevant: yes
This commit is contained in:
@@ -308,15 +308,38 @@ async def bulk_approve_devices(
|
|||||||
first_design = (await db.execute(select(Design).order_by(Design.created_at).limit(1))).scalar()
|
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
|
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(
|
result = await db.execute(
|
||||||
select(PendingDevice).where(
|
select(PendingDevice).where(
|
||||||
PendingDevice.id.in_(payload.device_ids),
|
PendingDevice.id.in_(payload.device_ids),
|
||||||
PendingDevice.status == "pending",
|
PendingDevice.status != "hidden",
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
devices = result.scalars().all()
|
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] = []
|
created_nodes: list[Node] = []
|
||||||
|
approved_devices: list[PendingDevice] = []
|
||||||
for device in devices:
|
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"
|
device.status = "approved"
|
||||||
node_type = device.suggested_type or "generic"
|
node_type = device.suggested_type or "generic"
|
||||||
is_wireless = _is_wireless(node_type)
|
is_wireless = _is_wireless(node_type)
|
||||||
@@ -339,12 +362,20 @@ async def bulk_approve_devices(
|
|||||||
)
|
)
|
||||||
db.add(node)
|
db.add(node)
|
||||||
created_nodes.append(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
|
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]
|
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]] = []
|
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))
|
all_edges.extend(await _resolve_pending_links_for_ieee(db, device.ieee_address))
|
||||||
|
|
||||||
await db.commit()
|
await db.commit()
|
||||||
|
|||||||
@@ -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)
|
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
|
@pytest.fixture
|
||||||
async def zigbee_pending_device(db_session):
|
async def zigbee_pending_device(db_session):
|
||||||
device = PendingDevice(
|
device = PendingDevice(
|
||||||
|
|||||||
Reference in New Issue
Block a user