feat: bulk approve/hide pending devices (#70)
- 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
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -165,9 +165,26 @@ function PendingDevicesPanel({ onNodeApproved, highlightId }: { onNodeApproved:
|
||||
const [devices, setDevices] = useState<PendingDevice[]>([])
|
||||
const [loading, setLoading] = useState(false)
|
||||
const [selected, setSelected] = useState<PendingDevice | null>(null)
|
||||
const [checkedIds, setCheckedIds] = useState<Set<string>>(new Set())
|
||||
const { addNode, scanEventTs } = useCanvasStore()
|
||||
const highlightRef = useRef<HTMLButtonElement>(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<string, string> = {}
|
||||
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:
|
||||
<>
|
||||
<div className="p-2">
|
||||
<div className="flex items-center justify-between mb-2">
|
||||
<span className="text-xs font-medium text-muted-foreground uppercase tracking-wider">Pending</span>
|
||||
<div className="flex items-center gap-1.5">
|
||||
{devices.length > 0 && (
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={allChecked}
|
||||
ref={(el) => { if (el) el.indeterminate = someChecked && !allChecked }}
|
||||
onChange={toggleAll}
|
||||
className="w-3 h-3 accent-[#00d4ff] cursor-pointer"
|
||||
title="Select all"
|
||||
/>
|
||||
)}
|
||||
<span className="text-xs font-medium text-muted-foreground uppercase tracking-wider">Pending</span>
|
||||
</div>
|
||||
<div className="flex items-center gap-1">
|
||||
<button onClick={load} className="text-muted-foreground hover:text-foreground p-0.5" title="Refresh">
|
||||
<RefreshCw size={12} />
|
||||
@@ -263,6 +338,22 @@ function PendingDevicesPanel({ onNodeApproved, highlightId }: { onNodeApproved:
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
{someChecked && (
|
||||
<div className="flex items-center gap-1 mb-2">
|
||||
<button
|
||||
onClick={handleBulkApprove}
|
||||
className="flex-1 text-[10px] py-1 px-2 rounded bg-[#39d353]/20 text-[#39d353] hover:bg-[#39d353]/30 transition-colors font-medium"
|
||||
>
|
||||
Approve ({checkedIds.size})
|
||||
</button>
|
||||
<button
|
||||
onClick={handleBulkHide}
|
||||
className="flex-1 text-[10px] py-1 px-2 rounded bg-[#8b949e]/20 text-[#8b949e] hover:bg-[#8b949e]/30 transition-colors font-medium"
|
||||
>
|
||||
Hide ({checkedIds.size})
|
||||
</button>
|
||||
</div>
|
||||
)}
|
||||
{loading && <Loader2 size={14} className="animate-spin text-muted-foreground mx-auto my-4" />}
|
||||
{!loading && devices.length === 0 && (
|
||||
<p className="text-xs text-muted-foreground text-center py-4">No pending devices</p>
|
||||
@@ -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]'}`}
|
||||
>
|
||||
<div className="flex items-center gap-1.5">
|
||||
<span className="w-1.5 h-1.5 rounded-full bg-[#e3b341] shrink-0" />
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={checkedIds.has(d.id)}
|
||||
onClick={(e) => toggleCheck(d.id, e)}
|
||||
onChange={() => {}}
|
||||
className="w-3 h-3 accent-[#00d4ff] cursor-pointer shrink-0"
|
||||
/>
|
||||
<span className="text-foreground truncate font-medium">{title}</span>
|
||||
</div>
|
||||
{showIpBelow && (
|
||||
|
||||
@@ -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(<Sidebar {...defaultProps} forceView="pending" />)
|
||||
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())
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user