From 0193f933ceab5176633b0cb62023ae902fec722b Mon Sep 17 00:00:00 2001 From: Pouzor Date: Sun, 19 Apr 2026 14:10:08 +0200 Subject: [PATCH] feat: bulk approve/hide pending devices (#70) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Backend: POST /scan/pending/bulk-approve and /scan/pending/bulk-hide endpoints (registered before dynamic routes to avoid conflict); bulk-approve response includes device_ids for frontend mapping - Frontend: PendingDevicesPanel gains per-row checkboxes, select-all, and a bulk action bar (Approve N / Hide N) that appears when ≥1 device is selected - Tests: 6 new backend API tests + 7 new frontend UI tests for bulk selection flows --- backend/app/api/routes/scan.py | 59 ++++++++++ backend/tests/test_scan.py | 98 ++++++++++++++++ frontend/src/api/client.ts | 2 + frontend/src/components/panels/Sidebar.tsx | 103 ++++++++++++++++- .../panels/__tests__/Sidebar.test.tsx | 106 ++++++++++++++++++ 5 files changed, 365 insertions(+), 3 deletions(-) diff --git a/backend/app/api/routes/scan.py b/backend/app/api/routes/scan.py index 6f3dcb2..1e4c86c 100644 --- a/backend/app/api/routes/scan.py +++ b/backend/app/api/routes/scan.py @@ -17,6 +17,10 @@ from app.schemas.scan import PendingDeviceResponse, ScanRunResponse from app.services.scanner import request_cancel, run_scan +class BulkActionRequest(BaseModel): + device_ids: list[str] + + class ScanConfig(BaseModel): ranges: list[str] @@ -99,6 +103,61 @@ async def list_hidden(db: AsyncSession = Depends(get_db), _: str = Depends(get_c return list(result.scalars().all()) +@router.post("/pending/bulk-approve", response_model=dict) +async def bulk_approve_devices( + payload: BulkActionRequest, + db: AsyncSession = Depends(get_db), + _: str = Depends(get_current_user), +) -> dict[str, Any]: + result = await db.execute( + select(PendingDevice).where( + PendingDevice.id.in_(payload.device_ids), + PendingDevice.status == "pending", + ) + ) + devices = result.scalars().all() + node_ids: list[str] = [] + for device in devices: + device.status = "approved" + node = Node( + label=device.hostname or device.ip, + type=device.suggested_type or "generic", + ip=device.ip, + hostname=device.hostname, + status="unknown", + services=device.services or [], + ) + db.add(node) + node_ids.append(node.id) + await db.commit() + approved_device_ids = [d.id for d in devices] + return { + "approved": len(node_ids), + "node_ids": node_ids, + "device_ids": approved_device_ids, + "skipped": len(payload.device_ids) - len(node_ids), + } + + +@router.post("/pending/bulk-hide", response_model=dict) +async def bulk_hide_devices( + payload: BulkActionRequest, + db: AsyncSession = Depends(get_db), + _: str = Depends(get_current_user), +) -> dict[str, Any]: + result = await db.execute( + select(PendingDevice).where( + PendingDevice.id.in_(payload.device_ids), + PendingDevice.status == "pending", + ) + ) + devices = result.scalars().all() + for device in devices: + device.status = "hidden" + await db.commit() + return {"hidden": len(devices), "skipped": len(payload.device_ids) - len(devices)} + + @router.post("/pending/{device_id}/approve", response_model=dict) async def approve_device( device_id: str, diff --git a/backend/tests/test_scan.py b/backend/tests/test_scan.py index 9540370..5e76d70 100644 --- a/backend/tests/test_scan.py +++ b/backend/tests/test_scan.py @@ -444,3 +444,101 @@ async def test_run_scan_updates_existing_pending_device(db_session: AsyncSession # Services and hostname should be updated assert device.hostname == "myhost.lan" assert any(s["port"] == 8096 for s in device.services) + + +# --- Bulk approve --- + +@pytest.fixture +async def two_pending_devices(db_session): + devices = [] + for i in range(2): + d = PendingDevice( + id=str(uuid.uuid4()), + ip=f"192.168.1.{10 + i}", + mac=None, + hostname=f"host-{i}", + os=None, + services=[], + suggested_type="generic", + status="pending", + ) + db_session.add(d) + devices.append(d) + await db_session.commit() + for d in devices: + await db_session.refresh(d) + return devices + + +@pytest.mark.asyncio +async def test_bulk_approve_approves_devices(client: AsyncClient, headers, two_pending_devices): + ids = [d.id for d in two_pending_devices] + res = await client.post("/api/v1/scan/pending/bulk-approve", json={"device_ids": ids}, headers=headers) + assert res.status_code == 200 + data = res.json() + assert data["approved"] == 2 + assert len(data["node_ids"]) == 2 + assert len(data["device_ids"]) == 2 + assert data["skipped"] == 0 + # Pending list should now be empty + pending_res = await client.get("/api/v1/scan/pending", headers=headers) + assert pending_res.json() == [] + + +@pytest.mark.asyncio +async def test_bulk_approve_skips_already_approved(client: AsyncClient, headers, two_pending_devices): + ids = [d.id for d in two_pending_devices] + # Approve first device individually first + await client.post( + f"/api/v1/scan/pending/{ids[0]}/approve", + json={"label": "h", "type": "generic", "ip": "192.168.1.10", "status": "unknown", "services": []}, + headers=headers, + ) + # Bulk approve both — first one is already approved (not pending), should be skipped + res = await client.post("/api/v1/scan/pending/bulk-approve", json={"device_ids": ids}, headers=headers) + assert res.status_code == 200 + data = res.json() + assert data["approved"] == 1 + assert data["skipped"] == 1 + + +@pytest.mark.asyncio +async def test_bulk_approve_requires_auth(client: AsyncClient, two_pending_devices): + ids = [d.id for d in two_pending_devices] + res = await client.post("/api/v1/scan/pending/bulk-approve", json={"device_ids": ids}) + assert res.status_code == 401 + + +# --- Bulk hide --- + +@pytest.mark.asyncio +async def test_bulk_hide_hides_devices(client: AsyncClient, headers, two_pending_devices): + ids = [d.id for d in two_pending_devices] + res = await client.post("/api/v1/scan/pending/bulk-hide", json={"device_ids": ids}, headers=headers) + assert res.status_code == 200 + data = res.json() + assert data["hidden"] == 2 + assert data["skipped"] == 0 + # Should appear in hidden list + hidden_res = await client.get("/api/v1/scan/hidden", headers=headers) + assert len(hidden_res.json()) == 2 + + +@pytest.mark.asyncio +async def test_bulk_hide_skips_non_pending(client: AsyncClient, headers, two_pending_devices): + ids = [d.id for d in two_pending_devices] + # Hide first device individually first + await client.post(f"/api/v1/scan/pending/{ids[0]}/hide", headers=headers) + # Bulk hide both — first is already hidden (not pending anymore) + res = await client.post("/api/v1/scan/pending/bulk-hide", json={"device_ids": ids}, headers=headers) + assert res.status_code == 200 + data = res.json() + assert data["hidden"] == 1 + assert data["skipped"] == 1 + + +@pytest.mark.asyncio +async def test_bulk_hide_requires_auth(client: AsyncClient, two_pending_devices): + ids = [d.id for d in two_pending_devices] + res = await client.post("/api/v1/scan/pending/bulk-hide", json={"device_ids": ids}) + assert res.status_code == 401 diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 6f9d808..62aed11 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -60,6 +60,8 @@ export const scanApi = { approve: (id: string, nodeData: object) => api.post(`/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[]) => api.post<{ approved: number; node_ids: string[]; device_ids: string[]; skipped: number }>('/scan/pending/bulk-approve', { device_ids: ids }), + bulkHide: (ids: string[]) => api.post<{ hidden: number; skipped: number }>('/scan/pending/bulk-hide', { device_ids: ids }), stop: (runId: string) => api.post(`/scan/${runId}/stop`), getConfig: () => api.get<{ ranges: string[] }>('/scan/config'), saveConfig: (data: { ranges: string[] }) => api.post('/scan/config', data), diff --git a/frontend/src/components/panels/Sidebar.tsx b/frontend/src/components/panels/Sidebar.tsx index eb77c46..a8f7688 100644 --- a/frontend/src/components/panels/Sidebar.tsx +++ b/frontend/src/components/panels/Sidebar.tsx @@ -165,9 +165,26 @@ function PendingDevicesPanel({ onNodeApproved, highlightId }: { onNodeApproved: const [devices, setDevices] = useState([]) const [loading, setLoading] = useState(false) const [selected, setSelected] = useState(null) + const [checkedIds, setCheckedIds] = useState>(new Set()) const { addNode, scanEventTs } = useCanvasStore() const highlightRef = useRef(null) + const allChecked = devices.length > 0 && checkedIds.size === devices.length + const someChecked = checkedIds.size > 0 + + const toggleCheck = (id: string, e: React.MouseEvent) => { + e.stopPropagation() + setCheckedIds((prev) => { + const next = new Set(prev) + if (next.has(id)) next.delete(id); else next.add(id) + return next + }) + } + + const toggleAll = () => { + setCheckedIds(allChecked ? new Set() : new Set(devices.map((d) => d.id))) + } + const load = useCallback(async () => { setLoading(true) try { @@ -184,12 +201,58 @@ function PendingDevicesPanel({ onNodeApproved, highlightId }: { onNodeApproved: try { await scanApi.clearPending() setDevices([]) + setCheckedIds(new Set()) toast.success('Pending devices cleared') } catch { toast.error('Failed to clear pending devices') } } + const handleBulkApprove = async () => { + const ids = [...checkedIds] + try { + const res = await scanApi.bulkApprove(ids) + 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)) + approvedDevices.forEach((d, i) => { + const nodeId = deviceToNode[d.id] + if (!nodeId) return + addNode({ + id: nodeId, + type: (d.suggested_type ?? 'generic') as import('@/types').NodeType, + position: { x: 400 + (i % 4) * 160, y: 300 + Math.floor(i / 4) * 100 }, + data: { + label: d.hostname ?? d.ip, + type: (d.suggested_type ?? 'generic') as import('@/types').NodeType, + ip: d.ip, + hostname: d.hostname ?? undefined, + status: 'unknown' as const, + services: (d.services ?? []) as import('@/types').ServiceInfo[], + }, + }) + onNodeApproved(nodeId) + }) + setDevices((prev) => prev.filter((d) => !ids.includes(d.id))) + setCheckedIds(new Set()) + toast.success(`Approved ${res.data.approved} device${res.data.approved !== 1 ? 's' : ''}`) + } catch { + toast.error('Failed to bulk approve devices') + } + } + + const handleBulkHide = async () => { + const ids = [...checkedIds] + try { + const res = await scanApi.bulkHide(ids) + setDevices((prev) => prev.filter((d) => !ids.includes(d.id))) + setCheckedIds(new Set()) + toast.success(`Hidden ${res.data.hidden} device${res.data.hidden !== 1 ? 's' : ''}`) + } catch { + toast.error('Failed to bulk hide devices') + } + } + useEffect(() => { load() }, [load]) useEffect(() => { @@ -251,7 +314,19 @@ function PendingDevicesPanel({ onNodeApproved, highlightId }: { onNodeApproved: <>
- Pending +
+ {devices.length > 0 && ( + { if (el) el.indeterminate = someChecked && !allChecked }} + onChange={toggleAll} + className="w-3 h-3 accent-[#00d4ff] cursor-pointer" + title="Select all" + /> + )} + Pending +
+ {someChecked && ( +
+ + +
+ )} {loading && } {!loading && devices.length === 0 && (

No pending devices

@@ -288,10 +379,16 @@ function PendingDevicesPanel({ onNodeApproved, highlightId }: { onNodeApproved: key={d.id} ref={isHighlighted ? highlightRef : null} onClick={() => setSelected(d)} - className={`w-full mb-1.5 p-2 rounded-md text-xs text-left transition-colors border ${isHighlighted ? 'bg-[#2d3748] border-[#e3b341]' : 'bg-[#21262d] border-transparent hover:bg-[#30363d] hover:border-[#30363d]'}`} + className={`w-full mb-1.5 p-2 rounded-md text-xs text-left transition-colors border ${isHighlighted ? 'bg-[#2d3748] border-[#e3b341]' : checkedIds.has(d.id) ? 'bg-[#21262d] border-[#00d4ff]/40' : 'bg-[#21262d] border-transparent hover:bg-[#30363d] hover:border-[#30363d]'}`} >
- + toggleCheck(d.id, e)} + onChange={() => {}} + className="w-3 h-3 accent-[#00d4ff] cursor-pointer shrink-0" + /> {title}
{showIpBelow && ( diff --git a/frontend/src/components/panels/__tests__/Sidebar.test.tsx b/frontend/src/components/panels/__tests__/Sidebar.test.tsx index ddfbbbb..a63d0ed 100644 --- a/frontend/src/components/panels/__tests__/Sidebar.test.tsx +++ b/frontend/src/components/panels/__tests__/Sidebar.test.tsx @@ -9,6 +9,9 @@ import type { NodeData } from '@/types' vi.mock('@/stores/canvasStore') +const mockBulkApprove = vi.fn() +const mockBulkHide = vi.fn() + vi.mock('@/api/client', () => ({ scanApi: { trigger: vi.fn().mockResolvedValue({}), @@ -16,6 +19,12 @@ vi.mock('@/api/client', () => ({ hidden: vi.fn().mockResolvedValue({ data: [] }), runs: vi.fn().mockResolvedValue({ data: [] }), stop: vi.fn().mockResolvedValue({}), + clearPending: vi.fn().mockResolvedValue({}), + approve: vi.fn().mockResolvedValue({ data: { approved: true, node_id: 'new-node-1' } }), + hide: vi.fn().mockResolvedValue({ data: { hidden: true } }), + ignore: vi.fn().mockResolvedValue({ data: { ignored: true } }), + bulkApprove: (...args: unknown[]) => mockBulkApprove(...args), + bulkHide: (...args: unknown[]) => mockBulkHide(...args), }, settingsApi: { get: vi.fn().mockResolvedValue({ data: { interval_seconds: 60 } }), @@ -259,3 +268,100 @@ describe('Sidebar', () => { expect(screen.queryByText('Status check interval (s)')).not.toBeInTheDocument() }) }) + +// ── PendingDevicesPanel — bulk select ───────────────────────────────────────── + +const DEVICE_A = { + id: 'dev-a', + ip: '192.168.1.10', + hostname: 'host-a', + mac: null, + os: null, + services: [], + suggested_type: 'generic', + status: 'pending', + discovery_source: 'arp', +} + +const DEVICE_B = { + id: 'dev-b', + ip: '192.168.1.11', + hostname: 'host-b', + mac: null, + os: null, + services: [], + suggested_type: 'generic', + status: 'pending', + discovery_source: 'arp', +} + +describe('PendingDevicesPanel — bulk select', () => { + beforeEach(() => { + mockStore() + vi.clearAllMocks() + mockBulkApprove.mockResolvedValue({ + data: { approved: 2, node_ids: ['n1', 'n2'], device_ids: ['dev-a', 'dev-b'], skipped: 0 }, + }) + mockBulkHide.mockResolvedValue({ data: { hidden: 2, skipped: 0 } }) + }) + + async function renderWithDevices() { + const { scanApi } = await import('@/api/client') + vi.mocked(scanApi.pending).mockResolvedValue({ data: [DEVICE_A, DEVICE_B] } as never) + render() + await waitFor(() => expect(screen.getByText('host-a')).toBeInTheDocument()) + } + + it('renders checkboxes for each device', async () => { + await renderWithDevices() + const checkboxes = screen.getAllByRole('checkbox') + // select-all + 2 device checkboxes + expect(checkboxes.length).toBe(3) + }) + + it('shows bulk action bar when a device is checked', async () => { + await renderWithDevices() + const [, firstDeviceCheckbox] = screen.getAllByRole('checkbox') + fireEvent.click(firstDeviceCheckbox) + await waitFor(() => expect(screen.getByText(/Approve \(1\)/)).toBeInTheDocument()) + expect(screen.getByText(/Hide \(1\)/)).toBeInTheDocument() + }) + + it('hides bulk action bar when no device is checked', async () => { + await renderWithDevices() + expect(screen.queryByText(/Approve \(/)).not.toBeInTheDocument() + }) + + it('select-all checks all devices', async () => { + await renderWithDevices() + const [selectAll] = screen.getAllByRole('checkbox') + fireEvent.click(selectAll) + await waitFor(() => expect(screen.getByText(/Approve \(2\)/)).toBeInTheDocument()) + }) + + it('select-all unchecks all when all are selected', async () => { + await renderWithDevices() + const [selectAll] = screen.getAllByRole('checkbox') + fireEvent.click(selectAll) // select all + fireEvent.click(selectAll) // deselect all + await waitFor(() => expect(screen.queryByText(/Approve \(/)).not.toBeInTheDocument()) + }) + + it('calls bulkApprove with checked ids and removes devices from list', async () => { + await renderWithDevices() + const [selectAll] = screen.getAllByRole('checkbox') + fireEvent.click(selectAll) + fireEvent.click(screen.getByText(/Approve \(2\)/)) + await waitFor(() => expect(mockBulkApprove).toHaveBeenCalledWith(['dev-a', 'dev-b'])) + await waitFor(() => expect(screen.queryByText('host-a')).not.toBeInTheDocument()) + }) + + it('calls bulkHide with checked ids and removes devices from list', async () => { + await renderWithDevices() + const [selectAll] = screen.getAllByRole('checkbox') + fireEvent.click(selectAll) + fireEvent.click(screen.getByText(/Hide \(2\)/)) + await waitFor(() => expect(mockBulkHide).toHaveBeenCalledWith(['dev-a', 'dev-b'])) + await waitFor(() => expect(screen.queryByText('host-b')).not.toBeInTheDocument()) + }) +})