diff --git a/apps/docs/src/content/docs/features/devices.mdx b/apps/docs/src/content/docs/features/devices.mdx index a009b89efe..76421444a6 100644 --- a/apps/docs/src/content/docs/features/devices.mdx +++ b/apps/docs/src/content/docs/features/devices.mdx @@ -1,11 +1,11 @@ --- title: Devices -description: Work with the device list and device Info tab — customizable columns, filters, VPN presence, and battery status. +description: Work with the device list and device Info tab — customizable columns, filters, VPN presence, battery status, and manual (hand-entered) assets. --- import { Aside } from '@astrojs/starlight/components'; -The **Devices** page is the home view for every managed endpoint — servers, workstations, laptops, and discovered network devices across all the organizations you can access. This page covers the device list itself (columns and filters) and the device detail **Info** tab, including VPN presence and battery status. +The **Devices** page is the home view for every managed endpoint — servers, workstations, laptops, discovered network devices, and hand-entered manual assets across all the organizations you can access. This page covers the device list itself (columns and filters) and the device detail **Info** tab, including VPN presence and battery status. --- @@ -24,6 +24,7 @@ The list shows a compact set of columns by default — hostname, class, organiza - **WAN IP** and **LAN IP** — the device's public (egress) address as seen from the server, and its local network interface address. Both are sortable; see [IP History](/features/ip-history/) for the timeline of address changes. - **Tags, last logged-in user, uptime, enrollment date** - **Desktop access** and **reliability score** +- **Serial, asset tag, location** — inventory fields for manual assets (asset tag and location) and, where reported, serial number (manual assets and agent devices); see [Manual Assets](#manual-assets). Your column selection and order are remembered per browser, so each technician can tailor the list to their workflow. Most columns are sortable by clicking the header. @@ -33,11 +34,49 @@ Above the list you can: - **Search** by display name or hostname. - Build **structured filters** with the filter toolbar — status, OS, role, organization, site, group, hardware attributes, and more, combinable into saved filter conditions. -- Switch the **class facet** between All, Agent (endpoints running the Breeze agent), and Network (devices found by network discovery). +- Switch the **class facet** between All, Agent (endpoints running the Breeze agent), Network (devices found by network discovery), and Manual (hand-entered assets — see [Manual Assets](#manual-assets)). - Filter by **VPN** when the VPN column is enabled (see below). --- +## Manual Assets + +Not everything an MSP is responsible for runs an agent or answers a ping. A spare laptop in a drawer, a desk phone, a non-networked label printer, a loaner tablet out with a field tech — a **manual asset** records that equipment as plain inventory so it shows up in the same unified Devices list as everything else. + +### Discovered asset vs. manual asset + +The rule is simple: **does it have a network identity?** + +- **Has an IP address, hostname, or URL** — it's a discovered network asset (the **Network** class), found automatically by network discovery scanning and eligible for monitors, alerts, and topology. See [Network Discovery](/features/discovery/) if that's what you're looking for. +- **Does not** — it's a manual asset. You type it in by hand, and it carries no reachability at all: it was never "online" or "offline", so its **Status** column shows **Unknown** rather than a misleading Offline. + +Manual assets deliberately carry no IP, MAC address, monitoring, alerts, or remote access — those are all agent/network concepts a hand-entered row cannot answer. + +### Adding a manual asset + +From the Devices page, open the **Add** menu next to the device list and choose **Add asset manually…** (the other item, **Install agent…**, is the existing agent-enrollment flow — the two share one menu since they're both ways of adding something to your fleet). Fill in: + +- **Site** (required) — defaults to the organization's only site when it has just one. +- **Name** (required) — the label that appears everywhere else in the list. +- **Asset type** — the same device-type list (printer, workstation, phone, etc.) discovered devices use. +- **Manufacturer, model, serial number, asset tag, location** — free-text inventory fields. Location is a free-text field within the site ("Closet B, shelf 2"), not a structured address. +- **Assigned to** — an organization contact (not a Breeze technician login) responsible for the asset. +- **Tags** and **notes**. + +Serial numbers are not required to be unique — the same manufacturer's serial format can legitimately repeat across vendors. If you enter a serial that already exists in the organization, the form shows a non-blocking warning; it does not stop you from saving. + +### Editing, linking, and deleting + +Select a manual asset's row to reopen the same modal for editing. From there you can also **link** the record to an agent device or a discovered network asset once one shows up for that physical machine — for example, after IT installs the agent on a spare laptop that had been tracked manually. Linking is reversible (**Unlink** restores the manual record to the list) and never merges data destructively: the manual record's inventory fields (serial, asset tag, location, assigned contact, notes) stay attached and surface alongside the linked device or asset. A linked manual asset drops out of the Manual segment — the fleet is never double-counted — until you unlink it. + +**Delete** permanently removes a manual asset record (no agent to uninstall, so there's no separate "remove" step). It's available from the row actions and, for a manual-only selection, from the bulk actions menu. + +### Filtering and search + +The **Manual** segment (and its Devices-list count) is independent of network discovery being enabled — a manual asset still shows up even in an environment with no network-discovered devices at all. Search matches a manual asset's name, serial number, and asset tag. Structured filters apply only where they make sense for a manual asset: name, tags, asset type, organization, site, manufacturer, model, and serial number are supported; anything that depends on reachability or an agent (status, IP/MAC, last-seen, OS, agent version, metrics) is reported as *not applicable* rather than silently hiding the row without explanation. + +--- + ## Billing Coverage The device **Overview** tab carries a **Billing** card answering "which contract line bills this device?". It needs **Contracts read** access and a partner-scoped login, so techs without billing access do not see it at all. It shows one of four things: diff --git a/apps/web/src/components/devices/DeviceCard.test.tsx b/apps/web/src/components/devices/DeviceCard.test.tsx index 0277223d20..2ddae84cfd 100644 --- a/apps/web/src/components/devices/DeviceCard.test.tsx +++ b/apps/web/src/components/devices/DeviceCard.test.tsx @@ -91,3 +91,54 @@ describe('DeviceCard sr-only status text', () => { expect(screen.queryByText('Decommissioned')).not.toBeInTheDocument(); }); }); + +// #4622 W04: this card had no `deviceClass` handling for 'manual' at all when +// the class was first introduced, so the grid offered the FULL agent kebab +// (Terminal/Run Script/Reboot/Decommission/Permanent Delete) on a manual +// asset's foreign `manual_assets.id` — the same #4014 failure class the +// network arm was already fixed for. Locks in the fix: Edit/Delete only, no +// metrics fetch. +describe('DeviceCard manual asset class (#4622 W04)', () => { + const manualDevice: Device = { + ...baseDevice, + id: 'manual-1', + hostname: 'spare-laptop', + deviceClass: 'manual', + assetType: 'workstation', + status: 'unknown', + }; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('never fires the agent metrics-history request for a manual row', async () => { + render(); + // Give any accidental effect a tick to fire before asserting its absence. + await Promise.resolve(); + expect(fetchWithAuthMock).not.toHaveBeenCalled(); + }); + + it('offers Edit and Delete, never the agent actions menu, for a manual row', () => { + const onClick = vi.fn(); + const onAction = vi.fn(); + render(); + + expect(screen.getByTestId('device-manual-1-edit-manual')).toBeInTheDocument(); + expect(screen.getByTestId('device-manual-1-delete-manual')).toBeInTheDocument(); + expect(screen.queryByTestId('device-manual-1-actions-menu')).not.toBeInTheDocument(); + expect(screen.queryByTestId('device-manual-1-open-network')).not.toBeInTheDocument(); + + screen.getByTestId('device-manual-1-edit-manual').click(); + expect(onClick).toHaveBeenCalledWith(manualDevice); + + screen.getByTestId('device-manual-1-delete-manual').click(); + expect(onAction).toHaveBeenCalledWith('delete-manual', manualDevice); + }); + + it('renders the Unknown status chip, never Offline, and no CPU/RAM reading', () => { + render(); + expect(screen.getByText('Unknown')).toBeInTheDocument(); + expect(screen.queryByText('Offline')).not.toBeInTheDocument(); + }); +}); diff --git a/apps/web/src/components/devices/DeviceCard.tsx b/apps/web/src/components/devices/DeviceCard.tsx index af1fd5b9b7..b287c83535 100644 --- a/apps/web/src/components/devices/DeviceCard.tsx +++ b/apps/web/src/components/devices/DeviceCard.tsx @@ -3,6 +3,7 @@ import { Monitor, MoreVertical, Network, + Package, Terminal, RotateCcw, FileCode, @@ -162,12 +163,21 @@ export default function DeviceCard({ // row and 404s. The list row already collapses to a single "View" // (DeviceList.tsx); the grid card mirrors that treatment exactly, reusing the // same `deviceList.view` copy and `-open-network` test id. - const isNetwork = (device.deviceClass ?? "agent") === "network"; + // + // #4622 W04: a manual asset's `id` is a `manual_assets.id`, the same foreign- + // id problem as network — the fix here mirrors DeviceList.tsx's manual row + // Actions cell (Edit + Delete instead of the agent kebab) rather than the + // network arm's single "View", since a manual asset IS editable, just not + // through the agent action funnel. + const deviceClass = device.deviceClass ?? "agent"; + const isNetwork = deviceClass === "network"; + const isManual = deviceClass === "manual"; useEffect(() => { - // A discovered asset has no agent and no metric history; firing the request - // anyway is a guaranteed 404 on every card mount. - if (isNetwork) return; + // A discovered asset or a manual asset has no agent and no metric + // history; firing the request anyway is a guaranteed 404 on every card + // mount. + if (isNetwork || isManual) return; let isCancelled = false; @@ -205,7 +215,7 @@ export default function DeviceCard({ return () => { isCancelled = true; }; - }, [device.id, isNetwork]); + }, [device.id, isNetwork, isManual]); const cpuHistory = historyState === "ready" ? metricHistory.map((point) => point.cpu) : []; @@ -245,6 +255,9 @@ export default function DeviceCard({ // to the generic monitor glyph — the same one a workstation gets. // The list uses a Network glyph on these rows; match it. + ) : isManual ? ( + // Same Package glyph DeviceList's class badge uses for manual rows. + ) : ( osIcons[device.os] || )} @@ -273,12 +286,56 @@ export default function DeviceCard({ {t("deviceList.network")} + ) : isManual ? ( + + + {t("deviceList.manual")} + ) : (

{device.osVersion}

)} - {isNetwork ? ( + {isManual ? ( + // Mirrors DeviceList.tsx's manual row Actions cell: Edit (opens the + // add/edit modal via onClick, same as the list) and Delete (routes + // through onAction("delete-manual", ...) into the same confirm- + // gated funnel as the list row and the bulk bar). +
+ + +
+ ) : isNetwork ? ( // Mirrors DeviceList.tsx's network row: the whole action surface // collapses to one "View", which opens the read-only network detail // page (DevicesPage.handleSelectDevice routes `network` there). @@ -431,11 +488,12 @@ export default function DeviceCard({ )} - {isNetwork ? ( - // A discovered asset reports no CPU/RAM — DevicesPage fills those - // fields with a placeholder 0, which would render as a confident - // "0%" reading for a printer. The list already shows "—" in those - // columns for exactly this reason (DeviceList.tsx agentCell). + {isNetwork || isManual ? ( + // A discovered asset or a manual asset reports no CPU/RAM — + // DevicesPage fills those fields with a placeholder 0, which would + // render as a confident "0%" reading for a printer or a spare laptop. + // The list already shows "—" in those columns for exactly this reason + // (DeviceList.tsx agentCell).
{(["CPU", "RAM"] as const).map((label) => (
{ - const counts = { all: 12, agent: 10, network: 2 }; + const counts = { all: 15, agent: 10, network: 2, manual: 3 }; - it('renders the three segments with their counts', () => { + it('renders the four segments with their counts', () => { render( {}} />); expect(screen.getByTestId('device-class-segment-all')).toHaveTextContent('All'); - expect(screen.getByTestId('device-class-segment-all')).toHaveTextContent('12'); + expect(screen.getByTestId('device-class-segment-all')).toHaveTextContent('15'); expect(screen.getByTestId('device-class-segment-agent')).toHaveTextContent('10'); expect(screen.getByTestId('device-class-segment-network')).toHaveTextContent('2'); + expect(screen.getByTestId('device-class-segment-manual')).toHaveTextContent('3'); }); it('marks the active segment as pressed', () => { diff --git a/apps/web/src/components/devices/DeviceClassSegment.tsx b/apps/web/src/components/devices/DeviceClassSegment.tsx index ff9dcb5965..ddc1fa52da 100644 --- a/apps/web/src/components/devices/DeviceClassSegment.tsx +++ b/apps/web/src/components/devices/DeviceClassSegment.tsx @@ -1,4 +1,4 @@ -import { Cpu, LayoutGrid, Network } from "lucide-react"; +import { Cpu, LayoutGrid, Network, Package } from "lucide-react"; import type { ComponentType } from "react"; import type { DeviceClassFilter } from "./deviceClassFilter"; import { useTranslation } from "react-i18next"; @@ -6,12 +6,13 @@ import "../../lib/i18n"; type DeviceClassSegmentProps = { value: DeviceClassFilter; - counts: { all: number; agent: number; network: number }; + counts: { all: number; agent: number; network: number; manual: number }; onChange: (value: DeviceClassFilter) => void; }; -// Icons mirror the Class column in DeviceList (Cpu = agent, Network = network) -// so the segment and the per-row badge read as the same vocabulary. +// Icons mirror the Class column in DeviceList (Cpu = agent, Network = network, +// Package = manual) so the segment and the per-row badge read as the same +// vocabulary. const SEGMENTS: Array<{ id: DeviceClassFilter; labelKey: string; @@ -24,6 +25,11 @@ const SEGMENTS: Array<{ labelKey: "deviceClassSegment.segments.network", icon: Network, }, + { + id: "manual", + labelKey: "deviceClassSegment.segments.manual", + icon: Package, + }, ]; /** diff --git a/apps/web/src/components/devices/DeviceCompare.tsx b/apps/web/src/components/devices/DeviceCompare.tsx index 24d7860743..21bedc387b 100644 --- a/apps/web/src/components/devices/DeviceCompare.tsx +++ b/apps/web/src/components/devices/DeviceCompare.tsx @@ -668,6 +668,9 @@ export default function DeviceCompare({ timezone }: DeviceCompareProps = {}) { quarantined: t("deviceCompare.status.quarantined"), updating: t("deviceCompare.status.updating"), pending: t("deviceCompare.status.pending"), + // No compare entry point exists for a manual asset (#4622 W04) and the + // `unknown` status is only produced for unprobed network rows (#5213) — + // kept for the shared DeviceStatus exhaustiveness. unknown: t("deviceCompare.status.unknown"), }; const osLabels: Record = { diff --git a/apps/web/src/components/devices/DeviceDetails.tsx b/apps/web/src/components/devices/DeviceDetails.tsx index 8bc639d256..640f143011 100644 --- a/apps/web/src/components/devices/DeviceDetails.tsx +++ b/apps/web/src/components/devices/DeviceDetails.tsx @@ -159,6 +159,9 @@ const statusLabels: Record = { quarantined: "Quarantined", updating: "Updating", pending: "Pending", + // No detail page exists for a manual asset in v1 (#4622 W04 spec), and + // `unknown` is only produced for unprobed network rows (#5213), so this + // never renders today — kept for the shared DeviceStatus exhaustiveness. unknown: "Unknown", }; @@ -951,11 +954,10 @@ export default function DeviceDetails({ {activeTab === "backup" && ( diff --git a/apps/web/src/components/devices/DeviceList.test.tsx b/apps/web/src/components/devices/DeviceList.test.tsx index 2e3f3e36cb..7182d9b823 100644 --- a/apps/web/src/components/devices/DeviceList.test.tsx +++ b/apps/web/src/components/devices/DeviceList.test.tsx @@ -546,8 +546,9 @@ describe('DeviceList — sortable columns (every column sorts on header click)', ); // Columns adapt to the classes on screen (Class/Type only render when a - // network row is present; OS/CPU/… only when an agent row is), so render - // one of each to bring the whole catalog out. + // non-agent row is present; OS/CPU/… only when an agent row is; + // assetTag/location only when a manual row is, #4622 W04), so render one + // of each to bring the whole catalog out. const networkRow: Device = { ...baseDevice, id: 'b1b1b1b1-0000-0000-0000-0000000000b1', @@ -555,7 +556,15 @@ describe('DeviceList — sortable columns (every column sorts on header click)', deviceClass: 'network', assetType: 'printer', }; - const { container } = render(); + const manualRow: Device = { + ...baseDevice, + id: 'c1c1c1c1-0000-0000-0000-0000000000c1', + hostname: 'spare-laptop', + deviceClass: 'manual', + assetType: 'workstation', + status: 'unknown', + }; + const { container } = render(); const headers = Array.from(container.querySelectorAll('thead th')); // First (checkbox) and last (Actions) are structural; everything between @@ -1454,9 +1463,16 @@ describe('DeviceList — bulk actions are all classified by the status gate (#24 * Clicking an item closes the menu and clears the selection, so each button is * driven from a fresh render and identified by index within the live menu. */ + // Buttons DISABLED in the live menu — e.g. bulk-delete-manual on an + // all-agent selection (#4622 W04: manual-only, correctly inert here) — + // cannot emit anything by construction and are excluded from the + // enumeration; a disabled button is not a class this contract governs. + const enabledButtons = (container: HTMLElement) => + within(container).getAllByRole('button').filter((b) => !b.hasAttribute('disabled')); + function emittedBulkActions(): string[] { const probe = render(); - const buttonCount = within(openBulkMenu()).getAllByRole('button').length; + const buttonCount = enabledButtons(openBulkMenu()).length; probe.unmount(); expect(buttonCount).toBeGreaterThan(0); // menu must actually render items @@ -1464,7 +1480,7 @@ describe('DeviceList — bulk actions are all classified by the status gate (#24 for (let i = 0; i < buttonCount; i++) { const onBulkAction = vi.fn(); const view = render(); - const buttons = within(openBulkMenu()).getAllByRole('button'); + const buttons = enabledButtons(openBulkMenu()); fireEvent.click(buttons[i]!); expect(onBulkAction).toHaveBeenCalledTimes(1); emitted.push(onBulkAction.mock.calls[0]![0] as string); @@ -1492,7 +1508,7 @@ describe('DeviceList — bulk actions are all classified by the status gate (#24 const probe = render( , ); - const buttonCount = within(openRemovedBulkMenu()).getAllByRole('button').length; + const buttonCount = enabledButtons(openRemovedBulkMenu()).length; probe.unmount(); expect(buttonCount).toBeGreaterThan(0); @@ -1502,7 +1518,7 @@ describe('DeviceList — bulk actions are all classified by the status gate (#24 const view = render( , ); - const buttons = within(openRemovedBulkMenu()).getAllByRole('button'); + const buttons = enabledButtons(openRemovedBulkMenu()); fireEvent.click(buttons[i]!); expect(onBulkAction).toHaveBeenCalledTimes(1); emitted.push(onBulkAction.mock.calls[0]![0] as string); @@ -1612,6 +1628,73 @@ describe('DeviceList — bulk actions are all classified by the status gate (#24 }); }); +describe('DeviceList — manual asset bulk delete + row actions (#4622 W04)', () => { + const manualDevice = (extra: Partial = {}): Device => ({ + ...baseDevice, + id: 'c1111111-1111-1111-1111-111111111111', + hostname: 'spare-laptop', + deviceClass: 'manual', + assetType: 'workstation', + status: 'unknown', + ...extra, + }); + + it('offers Edit and Delete (not View) on a manual row, wired to onSelect/onAction', () => { + const onSelect = vi.fn(); + const onAction = vi.fn(); + const device = manualDevice(); + render(); + + fireEvent.click(screen.getByTestId(`device-${device.id}-edit-manual`)); + expect(onSelect).toHaveBeenCalledWith(device); + + fireEvent.click(screen.getByTestId(`device-${device.id}-delete-manual`)); + expect(onAction).toHaveBeenCalledWith('delete-manual', device); + + expect(screen.queryByTestId(`device-${device.id}-open-network`)).not.toBeInTheDocument(); + expect(screen.queryByTestId(`device-${device.id}-actions-menu`)).not.toBeInTheDocument(); + }); + + it('bulk-delete-manual is disabled with no manual row selected, enabled with one', () => { + const agentA = { ...baseDevice, id: 'a1111111-1111-1111-1111-111111111111' }; + const manual = manualDevice(); + render(); + + fireEvent.click(screen.getByLabelText('Select all devices on this page')); + fireEvent.click(screen.getByRole('button', { name: /bulk actions/i })); + const menu = screen.getByTestId('bulk-actions-menu'); + const deleteManualButton = within(menu).getByTestId('bulk-delete-manual'); + // Mixed selection (1 agent + 1 manual): eligible for the manual-only + // action, and the "N of M eligible" suffix reports the split honestly. + expect(deleteManualButton).not.toBeDisabled(); + expect(deleteManualButton.textContent).toMatch(/1 of 2/); + }); + + it('bulk-delete-manual stays disabled for an agent-only selection', () => { + const agentA = { ...baseDevice, id: 'a1111111-1111-1111-1111-111111111111' }; + const agentB = { ...baseDevice, id: 'a2222222-2222-2222-2222-222222222222' }; + render(); + + fireEvent.click(screen.getByLabelText('Select all devices on this page')); + fireEvent.click(screen.getByRole('button', { name: /bulk actions/i })); + const menu = screen.getByTestId('bulk-actions-menu'); + expect(within(menu).getByTestId('bulk-delete-manual')).toBeDisabled(); + }); + + it('fires onBulkAction("delete-manual", ...) for a manual-only selection', () => { + const onBulkAction = vi.fn(); + const manualA = manualDevice({ id: 'c1111111-1111-1111-1111-111111111111' }); + const manualB = manualDevice({ id: 'c2222222-2222-2222-2222-222222222222', hostname: 'desk-phone' }); + render(); + + fireEvent.click(screen.getByLabelText('Select all devices on this page')); + fireEvent.click(screen.getByRole('button', { name: /bulk actions/i })); + fireEvent.click(within(screen.getByTestId('bulk-actions-menu')).getByTestId('bulk-delete-manual')); + + expect(onBulkAction).toHaveBeenCalledWith('delete-manual', [manualA, manualB]); + }); +}); + describe('classifyBulkSelection (#2787)', () => { it('reports removed only when EVERY selected device is removed', () => { expect(classifyBulkSelection(['decommissioned'])).toBe('removed'); diff --git a/apps/web/src/components/devices/DeviceList.tsx b/apps/web/src/components/devices/DeviceList.tsx index f8657e5544..8e3674b312 100644 --- a/apps/web/src/components/devices/DeviceList.tsx +++ b/apps/web/src/components/devices/DeviceList.tsx @@ -19,6 +19,7 @@ import { Zap, Columns3, Network, + Package, Cpu, Battery, BatteryCharging, @@ -114,20 +115,29 @@ export type DeviceStatus = | "quarantined" | "updating" | "pending" - // #5213 — a manual network asset that no scan has ever reached. Distinct - // from 'offline': that is a reachability claim (a probe failed); this one - // makes no claim at all, because no probe has ever run. + /** + * No reachability claim at all. Produced two ways: a manual asset (#4622 + * W04) that was hand-typed and never probed, and a manual network asset + * (#5213) that no scan has ever reached. Distinct from 'offline', which is + * a reachability claim (a probe ran and failed). Rendered as its own + * neutral "Unknown" chip; claiming "offline" for a printer that was never + * online is the kind of small lie that makes an inventory list + * untrustworthy. + */ | "unknown"; export type OSType = "windows" | "macos" | "linux"; /** - * Presentation-level discriminator for the unified Devices list (#1322). + * Presentation-level discriminator for the unified Devices list (#1322, #4622). * `agent` = an enrolled endpoint running the Go agent (devices table). * `network` = a discovered network device (printer/router/switch/…) from * discovered_assets that is approved and not linked to an agent. Agent-only * columns (CPU/RAM, agent version, OS build) render blank for `network` rows. + * `manual` = a hand-entered, non-networked inventory row (manual_assets) — a + * spare laptop, a desk phone, a non-networked printer. Carries no network + * identity and no reachability at all (see the `unknown` DeviceStatus above). */ -export type DeviceClass = "agent" | "network"; +export type DeviceClass = "agent" | "network" | "manual"; export type Device = { id: string; @@ -141,6 +151,28 @@ export type Device = { responseTimeMs?: number | null; /** Whether SNMP/network monitoring is configured for a network device. */ monitoringEnabled?: boolean; + /** + * Manual-asset-only fields (#4622 W04) — null/undefined for agent and + * network rows. `serialNumber` doubles as the warranty key and is also + * surfaced (read-only) for agent rows via `hardware` in a future column; + * `assetTag` and `location` are manual-only. + */ + serialNumber?: string | null; + assetTag?: string | null; + location?: string | null; + /** An org `contacts` row id — the person holding this manual asset. */ + assignedContactId?: string | null; + /** + * Reversible promotion links (#4622 W04, manual assets only): set when an + * agent was later installed on this physical asset, or a scan later found + * it. A linked manual asset drops out of `GET /devices/manual`, so these + * never appear on a manual row rendered from the list fetch — only on the + * row passed into the edit modal, which fetches the asset directly. + */ + linkedDeviceId?: string | null; + linkedDiscoveredAssetId?: string | null; + /** Free-text notes (manual assets only, #4622 W04). */ + notes?: string | null; hostname: string; os: OSType; osVersion: string; @@ -271,6 +303,12 @@ export type Device = { cpuCores?: number; ramTotalMb?: number; diskTotalGb?: number; + /** + * device_hardware.serial_number, when the API sends it. Read by the + * opt-in Serial column (#4622 W04) so an agent row's serial can sit + * beside a manual asset's `serialNumber` in the same column. + */ + serialNumber?: string; }; /** * Headline device reliability score (0-100) from the existing @@ -328,14 +366,32 @@ export type Device = { url?: string | null; }; -// Columns that only make sense for the network arm (#1322); hidden unless -// networkDevicesEnabled. Module-level so it isn't reallocated each render. -const NETWORK_ONLY_COLUMNS: ReadonlySet = new Set([ +// Columns that only make sense for a non-agent row (#1322, #4622): the class +// discriminator itself and its asset type. Hidden entirely when the network +// arm's flag is off AND no manual asset is present — the manual arm has no +// flag (it's independent of PUBLIC_ENABLE_NETWORK_DEVICES_IN_LIST), so these +// columns must not stay gated on the network-only flag once a manual row +// exists. Module-level so it isn't reallocated each render. +const NON_AGENT_COLUMNS: ReadonlySet = new Set([ "class", "type", // #5213 — provenance (scan | unifi | manual). Agent rows have no source. "source", ]); +// Columns meaningful only for a hand-entered manual asset (#4622 W04) — no +// analogue on an agent or a discovered network device. +const MANUAL_ONLY_COLUMNS: ReadonlySet = new Set([ + "assetTag", + "location", +]); +// `serial` is opt-in for BOTH agent (device_hardware.serial_number) and +// manual rows, but meaningless for a discovered network device (discovered_assets +// has no serial column by design — see the design spec). It steps aside only +// when the visible fleet is purely network, mirroring AGENT_ONLY_COLUMNS' +// dash-avoidance rule below rather than joining either fixed set. +const NETWORK_EXCLUDED_COLUMNS: ReadonlySet = new Set([ + "serial", +]); // Columns that only ever carry data for agent-managed endpoints. When the rows // on screen are all network devices (Network facet, or a network-only fleet) // these would render as solid columns of dashes, so they step aside — the @@ -548,10 +604,11 @@ const statusSortRank: Record = { maintenance: 3, quarantined: 4, offline: 5, - // #5213 — "no probe has run yet" ranks alongside decommissioned: neither - // is an operationally live state worth surfacing above offline/quarantined. - unknown: 6, - decommissioned: 7, + decommissioned: 6, + // "No probe has ever run" is not a worse operational state than offline — + // it's a different axis entirely (#4622 W04, #5213). Sorts last, after + // decommissioned. + unknown: 7, }; // Single shared collator for every string sort in this list. `numeric` keeps @@ -582,16 +639,21 @@ function serverHost(raw: string | null | undefined): string | null { // numeric collation (host-2 < host-10, agent 0.9.x < 0.10.x). const sortValue: Record string | number | null> = { hostname: (d) => d.displayName || d.hostname, - // Unified-list columns (#1322): sort by the same value the cell renders so - // header sort stays consistent with every other column (#1284 invariant). - class: (d) => - (d.deviceClass ?? "agent") === "network" ? "Network" : "Agent", - // Type renders only for network rows now (#1386); agent rows show a dash, so - // they sort as blanks-last (null) to match the cell — the #1284 invariant. + // Unified-list columns (#1322, #4622): sort by the same value the cell + // renders so header sort stays consistent with every other column (#1284 + // invariant). Three-way now — a two-branch ternary here would silently + // fold manual rows into whichever branch is the `else`. + class: (d) => { + const cls = d.deviceClass ?? "agent"; + return cls === "manual" ? "Manual" : cls === "network" ? "Network" : "Agent"; + }, + // Type renders for any non-agent row (network or manual, #1386, #4622); + // agent rows show a dash, so they sort as blanks-last (null) to match the + // cell — the #1284 invariant. type: (d) => - (d.deviceClass ?? "agent") === "network" - ? getDeviceRoleLabel(d.assetType ?? "unknown") - : null, + (d.deviceClass ?? "agent") === "agent" + ? null + : getDeviceRoleLabel(d.assetType ?? "unknown"), organization: (d) => d.orgName || null, site: (d) => d.siteName || null, // A network row has no OS (the cell renders a dash), so it must sort as a @@ -600,10 +662,12 @@ const sortValue: Record string | number | null> = { osVersion: (d) => formatDeviceOsVersion(d.os, d.osVersion) || null, osBuild: (d) => d.osBuild || null, architecture: (d) => d.architecture || null, - // Role renders only for agent rows now (#1386); network rows show a dash and - // sort blanks-last (null) to match the cell — the #1284 invariant. + // Role renders only for agent rows now (#1386, #4622); network AND manual + // rows show a dash and sort blanks-last (null) to match the cell — the + // #1284 invariant. `!== "agent"`, not `=== "network"`: a two-branch ternary + // here would silently fold manual into the "has a role" branch. role: (d) => - (d.deviceClass ?? "agent") === "network" + (d.deviceClass ?? "agent") !== "agent" ? null : getDeviceRoleLabel(d.deviceRole ?? "unknown"), isHeadless: (d) => @@ -612,16 +676,16 @@ const sortValue: Record string | number | null> = { // false/absent renders as a dash (see the cell), so it maps to null like // isHeadless — keeping the blanks-last invariant consistent for booleans. pendingReboot: (d) => (d.pendingReboot ? 1 : null), - // Network rows carry a placeholder 0 but render a dash — sort them as - // blanks so a CPU/RAM sort actually moves the agent rows. + // Network/manual rows carry a placeholder 0 but render a dash — sort them + // as blanks so a CPU/RAM sort actually moves the agent rows. cpu: (d) => - (d.deviceClass ?? "agent") === "network" + (d.deviceClass ?? "agent") !== "agent" ? null : d.status === "online" ? d.cpuPercent : null, ram: (d) => - (d.deviceClass ?? "agent") === "network" + (d.deviceClass ?? "agent") !== "agent" ? null : d.status === "online" ? d.ramPercent @@ -668,6 +732,13 @@ const sortValue: Record string | number | null> = { const vpns = vpnList(d.activeVpns); return vpns.length > 0 ? getVpnProviderLabel(vpns[0].provider) : null; }, + // Manual-asset inventory columns (#4622 W04); mirrors the cells above. + serial: (d) => { + const cls = d.deviceClass ?? "agent"; + return (cls === "manual" ? d.serialNumber : cls === "agent" ? d.hardware?.serialNumber : null) || null; + }, + assetTag: (d) => ((d.deviceClass ?? "agent") === "manual" ? d.assetTag || null : null), + location: (d) => ((d.deviceClass ?? "agent") === "manual" ? d.location || null : null), // #5213 — network-only, like class/type above; agent rows sort blanks-last. source: (d) => (d.deviceClass ?? "agent") === "network" ? (d.source ?? null) : null, @@ -1083,21 +1154,45 @@ export default function DeviceList({ () => selectedDevices.filter((d) => (d.deviceClass ?? "agent") === "agent").length, [selectedDevices], ); - const selectedNetworkCount = selectedDevices.length - selectedAgentCount; + // Per-class tally (#4622 W04) so the composition line below can say "N + // agent, N network, N manual" honestly instead of folding manual selections + // into a "network" bucket that no longer means only network. + const selectedNetworkCount = useMemo( + () => selectedDevices.filter((d) => (d.deviceClass ?? "agent") === "network").length, + [selectedDevices], + ); + const selectedManualCount = useMemo( + () => selectedDevices.filter((d) => (d.deviceClass ?? "agent") === "manual").length, + [selectedDevices], + ); + const selectedNonAgentCount = selectedNetworkCount + selectedManualCount; // Agent-only bulk actions: disabled outright when no agent is selected, // annotated with the eligible count on a mixed selection — the request - // funnel in DevicesPage still refuses network rows, this just says so - // before the click instead of after. + // funnel in DevicesPage still refuses network/manual rows, this just says + // so before the click instead of after. const agentOnlyDisabled = selectedAgentCount === 0; const agentOnlyTitle = agentOnlyDisabled ? t("deviceList.agentOnlyBulkAction") : undefined; const agentOnlySuffix = - selectedNetworkCount > 0 && selectedAgentCount > 0 ? ( + selectedNonAgentCount > 0 && selectedAgentCount > 0 ? ( ({t("deviceList.eligibleOfSelected", { count: selectedAgentCount, total: selectedIds.size })}) ) : null; + // Manual-only bulk action (Delete, #4622 W04): the mirror image of the + // agent-only gating above — disabled with zero manual rows selected, + // annotated with the eligible count on a mixed selection. + const manualOnlyDisabled = selectedManualCount === 0; + const manualOnlyTitle = manualOnlyDisabled + ? t("deviceList.manualOnlyBulkAction") + : undefined; + const manualOnlySuffix = + selectedManualCount > 0 && selectedManualCount < selectedIds.size ? ( + + ({t("deviceList.eligibleOfSelected", { count: selectedManualCount, total: selectedIds.size })}) + + ) : null; const handleSelectAll = (checked: boolean) => { if (checked) { @@ -1153,18 +1248,14 @@ export default function DeviceList({ const fleetFromStore = useOrgStore((s) => !s.currentOrgId && s.allOrgs); const isFleetView = !forceSingleOrg && fleetFromStore; - // The Class/Type columns belong to the network arm (#1322); hide them - // entirely when the feature flag is off so the list is the agent-only view. - const isColumnAvailable = (id: ColumnId) => - (networkDevicesEnabled || !NETWORK_ONLY_COLUMNS.has(id)) && - (id !== "organization" || isFleetView); - // Which classes are actually on screen — drives the class-adaptive column // set below (a Network-only view has no use for OS/CPU/RAM; an agent-only - // view has no use for Class/Type). + // view has no use for Class/Type). The manual arm carries no feature flag + // (independent of PUBLIC_ENABLE_NETWORK_DEVICES_IN_LIST), so `hasManualRows` + // is never gated on `networkDevicesEnabled`. const hasAgentRows = useMemo( () => - !networkDevicesEnabled || + (!networkDevicesEnabled && !devices.some((d) => (d.deviceClass ?? "agent") === "manual")) || devices.some((d) => (d.deviceClass ?? "agent") === "agent"), [networkDevicesEnabled, devices], ); @@ -1174,6 +1265,18 @@ export default function DeviceList({ devices.some((d) => (d.deviceClass ?? "agent") === "network"), [networkDevicesEnabled, devices], ); + const hasManualRows = useMemo( + () => devices.some((d) => (d.deviceClass ?? "agent") === "manual"), + [devices], + ); + + // The Class/Type columns belong to the non-agent arms (#1322, #4622); hide + // them entirely only when NEITHER the network flag is on NOR a manual row + // exists, so the list can be a pure agent-only view. + const isColumnAvailable = (id: ColumnId) => + (networkDevicesEnabled || hasManualRows || !NON_AGENT_COLUMNS.has(id)) && + (id !== "organization" || isFleetView); + // The VPN facet is agent-only; never leave it narrowing an all-network view // after its control has gone (the critique's "unmounted filter" dead end). useEffect(() => { @@ -1182,7 +1285,9 @@ export default function DeviceList({ const classAllowsColumn = (id: ColumnId) => (hasAgentRows || !AGENT_ONLY_COLUMNS.has(id)) && - (hasNetworkRows || !NETWORK_ONLY_COLUMNS.has(id)); + (hasNetworkRows || hasManualRows || !NON_AGENT_COLUMNS.has(id)) && + (hasManualRows || !MANUAL_ONLY_COLUMNS.has(id)) && + (hasAgentRows || hasManualRows || !NETWORK_EXCLUDED_COLUMNS.has(id)); // Effective render sequence: user-chosen order, filtered to visible. // Checkbox and Actions are rendered separately as the first/last cells. @@ -1347,10 +1452,11 @@ export default function DeviceList({ {t("deviceList.notApplicable")} ); - // Agent-only columns render "—" for network devices (#1322): the - // attribute doesn't exist for a printer/router, so don't imply 0/blank. + // Agent-only columns render "—" for network AND manual rows (#1322, #4622): + // the attribute doesn't exist for a printer/router or a hand-entered asset, + // so don't imply 0/blank. const agentCell = (device: Device, node: React.ReactNode): React.ReactNode => - (device.deviceClass ?? "agent") === "network" ? dash : node; + (device.deviceClass ?? "agent") !== "agent" ? dash : node; const columnDefs: Record< ColumnId, { header: () => React.ReactNode; cell: (device: Device) => React.ReactNode } @@ -1385,27 +1491,33 @@ export default function DeviceList({ cell: (device) => { const deviceClass = device.deviceClass ?? "agent"; const isNetwork = deviceClass === "network"; + const isManual = deviceClass === "manual"; + const badgeClass = isManual + ? "bg-warning/15 text-warning border-warning/30" + : isNetwork + ? "bg-info/15 text-info border-info/30" + : "bg-primary/10 text-primary border-primary/30"; + const title = isManual + ? t("deviceList.manualAsset") + : isNetwork + ? t("deviceList.networkDiscoveredDevice") + : t("deviceList.agentManagedEndpoint"); + const label = isManual ? t("deviceList.manual") : isNetwork ? t("deviceList.network") : t("deviceList.agent"); return ( - {isNetwork ? ( + {isManual ? ( + + ) : isNetwork ? ( ) : ( )} - {isNetwork ? t("deviceList.network") : t("deviceList.agent")} + {label} ); @@ -1414,12 +1526,13 @@ export default function DeviceList({ type: { header: () => sortHeader("type", t("deviceList.tableColumns.type"), t("deviceList.sortBy.type")), cell: (device) => { - // Type is the asset_type of a *network-discovered* device (printer, - // switch, NAS…). For agent rows the equivalent question — what kind of + // Type is the asset_type of a *non-agent* row (printer, switch, NAS… + // for network; the same discovered_asset_type enum for a manual + // asset). For agent rows the equivalent question — what kind of // endpoint is this — is answered by the Role column, so Type renders a // dash rather than echoing deviceRole and duplicating Role side by // side (#1386). Role and Type are complementary axes, one per class. - if ((device.deviceClass ?? "agent") !== "network") { + if ((device.deviceClass ?? "agent") === "agent") { return ( {dash} @@ -2106,6 +2219,43 @@ export default function DeviceList({ ); }, }, + // Manual-asset inventory columns (#4622 W04). `serial` is shared with + // agent rows (device_hardware.serial_number, once the API sends it); + // `assetTag`/`location` exist only for a manual asset. + serial: { + header: () => sortHeader("serial", t("deviceList.tableColumns.serial"), t("deviceList.sortBy.serial")), + cell: (device) => { + const cls = device.deviceClass ?? "agent"; + const value = cls === "manual" ? device.serialNumber : cls === "agent" ? device.hardware?.serialNumber : null; + return ( + + {value ? {value} : dash} + + ); + }, + }, + assetTag: { + header: () => sortHeader("assetTag", t("deviceList.tableColumns.assetTag"), t("deviceList.sortBy.assetTag")), + cell: (device) => { + const value = (device.deviceClass ?? "agent") === "manual" ? device.assetTag : null; + return ( + + {value ? {value} : dash} + + ); + }, + }, + location: { + header: () => sortHeader("location", t("deviceList.tableColumns.location"), t("deviceList.sortBy.location")), + cell: (device) => { + const value = (device.deviceClass ?? "agent") === "manual" ? device.location : null; + return ( + + {value ? {value} : dash} + + ); + }, + }, }; // Bulk-menu Compare item. DeviceCompare accepts at most COMPARE_MAX_DEVICES, @@ -2325,10 +2475,16 @@ export default function DeviceList({
{selectedIds.size} {t("deviceList.selected")} - {selectedNetworkCount > 0 && ( + {selectedNonAgentCount > 0 && ( {" · "} - {t("deviceList.selectedComposition", { agent: selectedAgentCount, network: selectedNetworkCount })} + {selectedManualCount > 0 + ? t("deviceList.selectedCompositionManual", { + agent: selectedAgentCount, + network: selectedNetworkCount, + manual: selectedManualCount, + }) + : t("deviceList.selectedComposition", { agent: selectedAgentCount, network: selectedNetworkCount })} )} @@ -2444,6 +2600,21 @@ export default function DeviceList({ > {t("deviceList.decommissionSelected")} {agentOnlySuffix} + + {/* Manual assets (#4622 W04): Delete is the ONLY bulk action + they're eligible for in v1. Disabled outright with no + manual row selected; annotated on a mixed selection so a + non-manual row is visibly skipped, never silently. */} + )} @@ -2622,7 +2793,37 @@ export default function DeviceList({ className="px-3 py-3 text-sm" onClick={(e) => e.stopPropagation()} > - {(device.deviceClass ?? "agent") === "network" ? ( + {(device.deviceClass ?? "agent") === "manual" ? ( + // A manual asset has no agent and no detail page in v1 + // (spec: per-asset detail pages are #1424's territory). + // Edit opens the same add/edit modal via onSelect; + // Delete is the one bulk-eligible action for this + // class, offered per-row too. +
+ + +
+ ) : (device.deviceClass ?? "agent") === "network" ? ( // Network devices have no agent — none of the remote // actions (desktop/terminal/scripts/reboot) apply. // View opens the network device page (/devices/network/:id). diff --git a/apps/web/src/components/devices/DevicesPage.test.tsx b/apps/web/src/components/devices/DevicesPage.test.tsx index 3ee3368ab9..f7292f75ec 100644 --- a/apps/web/src/components/devices/DevicesPage.test.tsx +++ b/apps/web/src/components/devices/DevicesPage.test.tsx @@ -5,7 +5,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import DevicesPage from './DevicesPage'; import { fetchWithAuth } from '../../stores/auth'; -import { fetchAllDevices, fetchAllNetworkDevices } from '../../lib/devicesFetch'; +import { fetchAllDevices, fetchAllNetworkDevices, fetchAllManualAssets } from '../../lib/devicesFetch'; import { navigateTo } from '@/lib/navigation'; // Feature flags are evaluated at module load, so expose a mutable holder we can @@ -24,11 +24,16 @@ vi.mock('@/lib/featureFlags', () => flagState); vi.mock('../../stores/auth', () => ({ fetchWithAuth: vi.fn(), + handleSessionExpired: vi.fn(), })); vi.mock('../../lib/devicesFetch', () => ({ fetchAllDevices: vi.fn(), fetchAllNetworkDevices: vi.fn(), + // Manual arm (#4622 W04) — carries no feature flag, so it's fetched on + // every render; defaults to empty so existing agent/network assertions are + // unaffected. + fetchAllManualAssets: vi.fn(), })); vi.mock('../../hooks/useEventStream', () => ({ @@ -343,6 +348,8 @@ beforeEach(() => { // Network arm (#1322) defaults to empty so existing assertions over the // agent fleet are unaffected. vi.mocked(fetchAllNetworkDevices).mockResolvedValue({ data: [], total: 0, pagesWalked: 1 } as never); + // Manual arm (#4622 W04) defaults to empty for the same reason. + vi.mocked(fetchAllManualAssets).mockResolvedValue({ data: [], total: 0, pagesWalked: 1 } as never); vi.mocked(fetchWithAuth).mockImplementation(async (url: string) => { if (url.startsWith('/filters/preview')) { diff --git a/apps/web/src/components/devices/DevicesPage.tsx b/apps/web/src/components/devices/DevicesPage.tsx index 3c44e0da9d..3500f501fc 100644 --- a/apps/web/src/components/devices/DevicesPage.tsx +++ b/apps/web/src/components/devices/DevicesPage.tsx @@ -15,6 +15,7 @@ import DeviceSettingsModal from './DeviceSettingsModal'; import RemoveDeviceDialog from './RemoveDeviceDialog'; import { BulkPurgeDialog } from './BulkPurgeDialog'; import AddDeviceModal from './AddDeviceModal'; +import ManualAssetModal from './ManualAssetModal'; import AddNetworkAssetModal from './AddNetworkAssetModal'; import RmmCustomFieldImport from './RmmCustomFieldImport'; import CreateGroupModal from './CreateGroupModal'; @@ -32,8 +33,9 @@ import { writeDeviceClassToHash, type DeviceClassFilter, } from './deviceClassFilter'; -import { fetchWithAuth } from '../../stores/auth'; -import { fetchAllDevices, fetchAllNetworkDevices } from '../../lib/devicesFetch'; +import { fetchWithAuth, handleSessionExpired } from '../../stores/auth'; +import { runAction } from '../../lib/runAction'; +import { fetchAllDevices, fetchAllNetworkDevices, fetchAllManualAssets } from '../../lib/devicesFetch'; import { useOrgStore } from '../../stores/orgStore'; import { useOrgScope } from '@/hooks/useOrgScope'; import { OrgLoadFailedState } from '../shared/OrgLoadFailedState'; @@ -51,7 +53,7 @@ import ProgressBar from '../shared/ProgressBar'; import { ConfirmDialog } from '../shared/ConfirmDialog'; import { scopeConfirmMessage } from '@/lib/scopeConfirmMessage'; import { DECOMMISSION_BLOCKED_BULK_ACTIONS, isCommandQueueable } from './bulkActionGating'; -import { matchesMergedListFilters, sortByDisplayName, summarizeHiddenNetworkDevices, VPN_FACET_FIELD } from './mergedListFilter'; +import { matchesMergedListFilters, sortByDisplayName, summarizeHiddenNonAgentDevices, VPN_FACET_FIELD } from './mergedListFilter'; import { COLUMN_LABELS } from './columnVisibility'; import { FILTER_FIELDS } from '../filters/filterFields'; import { asList } from '@/lib/asList'; @@ -85,6 +87,9 @@ type Org = { type Site = { id: string; name: string; + /** Present on every `/orgs/sites` row; declared so the manual-asset modal + * (#4622 W04) can filter to the selected org's sites. */ + orgId?: string; }; type DeviceGroup = { @@ -125,21 +130,138 @@ function summarizeFailedDevices(names: string[]): string { // irreversible operation on one click — and unlike `decommission` there is no // Restore afterwards. The 5s undo toast is not a substitute for a gate: it // starts a countdown the operator has to NOTICE to stop. -const CONFIRM_REQUIRED_ACTIONS = new Set(['reboot', 'reboot_safe_mode', 'shutdown', 'decommission', 'permanent-delete']); +const CONFIRM_REQUIRED_ACTIONS = new Set(['reboot', 'reboot_safe_mode', 'shutdown', 'decommission', 'permanent-delete', 'delete-manual']); // ConfirmDialog encodes severity by SHAPE as well as colour (stop-octagon vs // caution-triangle), so the grading has to match the detail page rather than // drift from it: DeviceActions.tsx marks shutdown and decommission -// `destructive` and every other confirm `warning`. -const DESTRUCTIVE_CONFIRM_ACTIONS = new Set(['shutdown', 'decommission', 'permanent-delete']); +// `destructive` and every other confirm `warning`. A manual-asset delete +// (#4622 W04) is a hard delete with no undo, so it's graded the same as +// permanent-delete. +const DESTRUCTIVE_CONFIRM_ACTIONS = new Set(['shutdown', 'decommission', 'permanent-delete', 'delete-manual']); // The command name is snake_case / kebab-case; the locale keys are camelCase. const CONFIRM_KEY_OVERRIDES: Record = { reboot_safe_mode: 'rebootSafeMode', 'permanent-delete': 'permanentDelete', + 'delete-manual': 'deleteManual', }; const confirmKeyFor = (action: string): string => CONFIRM_KEY_OVERRIDES[action] ?? action; +/** + * The Devices page's "Add" control (#4622 W04). What was a single "Install + * agent" button (opens AddDeviceModal — enrollment, creates no row) is now a + * split menu: *Install agent…* (unchanged) and *Add asset manually…* (new, + * opens ManualAssetModal). Used in both the header and the empty-state + * duplicate so the two never drift. + * + * The THIRD item — *Add network asset…* (#5213 W02, hand-entered + * discovered_assets rows, opens AddNetworkAssetModal) — landed on `main` in + * parallel as its own inline split menu; this merge folds it into this shared + * component so there is exactly ONE Add control on the page, and so the two + * instances no longer share a single open/closed flag (main's did, which made + * the header and empty-state menus open together). + */ +function AddAssetMenu({ + onInstallAgent, + onAddManualAsset, + onAddNetworkAsset, + variant, + testIdPrefix, +}: { + onInstallAgent: () => void; + onAddManualAsset: () => void; + onAddNetworkAsset: () => void; + variant: 'primary' | 'secondary'; + /** Distinguishes the header instance from the empty-state duplicate for testids. */ + testIdPrefix: string; +}) { + const { t } = useTranslation('devices'); + const [open, setOpen] = useState(false); + const containerRef = useRef(null); + + useEffect(() => { + if (!open) return; + const onDocPointerDown = (e: MouseEvent) => { + if (containerRef.current && !containerRef.current.contains(e.target as Node)) setOpen(false); + }; + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') setOpen(false); + }; + document.addEventListener('mousedown', onDocPointerDown); + document.addEventListener('keydown', onKeyDown); + return () => { + document.removeEventListener('mousedown', onDocPointerDown); + document.removeEventListener('keydown', onKeyDown); + }; + }, [open]); + + return ( +
+ + {open && ( +
+ + + +
+ )} +
+ ); +} + export default function DevicesPage() { const { t } = useTranslation('devices'); // Org scope is shown by the always-visible top-bar switcher (scope pill + @@ -190,14 +312,18 @@ export default function DevicesPage() { // The three hash-seeded states below adopt the hash post-mount via // useHashState so the first client render matches the SSR markup (#2421). const [showAddDevice, setShowAddDevice] = useHashState(false, (h) => (h === 'add-device' ? true : undefined)); - // Manual network asset (#5213 W02) — the second item of the header "Add" - // split menu below. Built as its own hash entry (not nested under - // showAddDevice) so a future third item (#4622's "Add asset manually…") - // slots in the same way without restructuring this state. + // Manual asset add/edit modal (#4622 W04). Add is hash-seeded like every + // other modal on this page; editing an EXISTING asset carries the full row + // (needed for the form fields) so it is plain component state, not hash — + // the hash only needs to say "the add flow is open", never which asset. + const [showAddManualAsset, setShowAddManualAsset] = useHashState(false, (h) => (h === 'add-manual-asset' ? true : undefined)); + const [editingManualAsset, setEditingManualAsset] = useState(null); + // Manual network asset (#5213 W02) — the third item of the shared + // AddAssetMenu below. Its own hash entry (not nested under showAddDevice), + // same shape as the manual-asset flow above. const [showAddNetworkAsset, setShowAddNetworkAsset] = useHashState(false, (h) => h === 'add-network-asset' ? true : undefined, ); - const [addMenuOpen, setAddMenuOpen] = useState(false); // "Import from another RMM" (#3257 W09): the wizard owns its OWN step hash // (#import-definitions / #import-values) internally, so this only tracks // whether either of those hashes means the wizard is open at all. @@ -232,6 +358,10 @@ export default function DevicesPage() { // #3987: bulk Remove asks the agent question ONCE for the whole selection, // then runBulkRemove runs the per-device DELETE loop with that one answer. const [pendingBulkRemove, setPendingBulkRemove] = useState(null); + // Manual asset bulk delete (#4622 W04): hard delete, no undo, so it asks + // once for the whole (already manual-only) selection before the per-item + // DELETE loop in runBulkDeleteManual. + const [pendingBulkDeleteManual, setPendingBulkDeleteManual] = useState(null); // #2787: bulk Delete permanently. The dialog asks for the count to be typed; // runBulkPurge then starts the async job and polls it. const [pendingBulkPurge, setPendingBulkPurge] = useState(null); @@ -404,13 +534,14 @@ export default function DevicesPage() { () => filterDevicesByClass(devices, deviceClassFilter), [devices, deviceClassFilter] ); - // Network rows the active filters hide only because they ask about agent-only - // things (filter fields like patches/alerts/metrics, or the VPN facet). - // Scoped to the chosen class so the notice matches what the tech is looking - // at; rows already hidden by search or the decommissioned rule are not - // counted (see summarizeHiddenNetworkDevices). + // Non-agent rows (network OR manual, #4622) the active filters hide only + // because they ask about agent-only things (filter fields like + // patches/alerts/metrics, or the VPN facet). Scoped to the chosen class so + // the notice matches what the tech is looking at; rows already hidden by + // search or the decommissioned rule are not counted (see + // summarizeHiddenNonAgentDevices). const hiddenNetwork = useMemo( - () => summarizeHiddenNetworkDevices(classFilteredDevices, listFilterContext), + () => summarizeHiddenNonAgentDevices(classFilteredDevices, listFilterContext), [classFilteredDevices, listFilterContext] ); const hiddenNetworkFieldLabels = useMemo( @@ -420,6 +551,15 @@ export default function DevicesPage() { .join(', '), [hiddenNetwork.fields] ); + // Which non-agent class(es) contributed to the count above, so the notice + // can say "3 manual assets hidden" instead of defaulting every non-agent + // row to "network devices" (#4622 W04). + const hiddenNonAgentClassLabel = useMemo(() => { + if (hiddenNetwork.classes.length === 1) { + return t(/* i18n-dynamic */ `devicesPage.hiddenNonAgentClassLabel.${hiddenNetwork.classes[0]}`); + } + return t('devicesPage.hiddenNonAgentClassLabel.mixed'); + }, [hiddenNetwork.classes, t]); // Rows the filters admit, before the hidden-by-default decommissioned rule — // the removed-hint counts come from this set so they only promise rows // "show" can actually reveal (#2251/#5023). Same shared predicate as the @@ -469,7 +609,7 @@ export default function DevicesPage() { // `signal` is wired by the mount useEffect's AbortController so a // navigate-away mid-walk stops the next page request and prevents // setState on an unmounted component (#778 review). - const [devicesResult, networkResult, orgsResponse, sitesResponse, groupsResponse] = await Promise.all([ + const [devicesResult, networkResult, manualResult, orgsResponse, sitesResponse, groupsResponse] = await Promise.all([ fetchAllDevices({ includeDecommissioned: true, signal, @@ -503,6 +643,18 @@ export default function DevicesPage() { return { data: [], total: 0, pagesWalked: 0 }; }) : Promise.resolve({ data: [], total: 0, pagesWalked: 0 }), + // Manual arm of the unified list (#4622 W04) — hand-entered inventory + // rows with no network identity. Carries NO feature flag: fetched + // unconditionally, independent of ENABLE_NETWORK_DEVICES_IN_LIST, so + // an org's manual assets show up even with the network arm off. Same + // best-effort degrade-to-empty semantics as the network arm above; a + // 401 is a real auth failure and is re-thrown, never masked. + fetchAllManualAssets({ signal }).catch((err) => { + if (err instanceof Error && err.name === 'AbortError') throw err; + if (err instanceof Response && err.status === 401) throw err; + console.warn('Failed to fetch manual assets:', err); + return { data: [], total: 0, pagesWalked: 0 }; + }), fetchWithAuth('/orgs', { signal }), fetchWithAuth('/orgs/sites', { signal }), fetchWithAuth('/device-groups?includeMemberships=true', { signal }).catch((err) => { @@ -651,7 +803,45 @@ export default function DevicesPage() { url: typeof d.url === 'string' ? d.url : null, })); - const allTransformed = [...transformedDevices, ...transformedNetworkDevices]; + // Manual arm (#4622 W04): normalize manual_assets rows into the same + // Device shape. No network identity, no reachability — the API already + // sends status: 'unknown', never a fabricated 'offline'. + const transformedManualAssets: Device[] = manualResult.data.map((d: Record) => ({ + id: d.id as string, + deviceClass: 'manual' as const, + assetType: (d.assetType as DeviceRole | undefined) ?? 'unknown', + hostname: (d.hostname ?? t('devicesPage.unknownDevice')) as string, + displayName: typeof d.displayName === 'string' ? d.displayName : undefined, + os: '' as OSType, + osVersion: '', + status: (d.status as DeviceStatus | undefined) ?? 'unknown', + cpuPercent: 0, + ramPercent: 0, + lastSeen: (d.lastSeenAt ?? '') as string, + orgId: (d.orgId ?? '') as string, + orgName: '', + siteId: (d.siteId ?? '') as string, + siteName: '', + agentVersion: '', + watchdogVersion: null, + wanIp: null, + lanIp: null, + macAddress: null, + tags: (d.tags ?? []) as string[], + manufacturer: (d.manufacturer ?? null) as string | null, + model: (d.model ?? null) as string | null, + serialNumber: (d.serialNumber ?? null) as string | null, + assetTag: (d.assetTag ?? null) as string | null, + location: (d.location ?? null) as string | null, + assignedContactId: (d.assignedContactId ?? null) as string | null, + linkedDeviceId: (d.linkedDeviceId ?? null) as string | null, + linkedDiscoveredAssetId: (d.linkedDiscoveredAssetId ?? null) as string | null, + notes: (d.notes ?? null) as string | null, + monitoringEnabled: false, + enrolledAt: d.enrolledAt as string | undefined, + })); + + const allTransformed = [...transformedDevices, ...transformedNetworkDevices, ...transformedManualAssets]; // Fetch orgs for org name lookup let orgsList: Org[] = []; @@ -831,6 +1021,13 @@ export default function DevicesPage() { }, [advancedFilter, filtersV2]); const handleSelectDevice = (device: Device) => { + // A manual asset has no detail page in v1 (#4622 W04 — #1424 owns the + // full three-class detail-page story); it edits in the same modal it was + // created from. + if ((device.deviceClass ?? 'agent') === 'manual') { + setEditingManualAsset(device); + return; + } // Network-discovered assets get a native, read-only detail/overview page in // the Devices section (#1424 slice 2) instead of bouncing out to Discovery. if ((device.deviceClass ?? 'agent') === 'network') { @@ -907,15 +1104,23 @@ export default function DevicesPage() { // row and 404s. handleBulkAction has filtered these out since #1322. // // Stated as an invariant rather than as a claim about today's UI: this - // funnel must refuse network rows on its own, because nothing guarantees - // that every present and future caller hides the actions first. #4014 was - // exactly that failure — DeviceList hid them, DeviceCard did not, and the - // handler trusted its callers. A guard here cannot be re-opened by adding - // a third surface. + // funnel must refuse network AND manual rows on its own, because nothing + // guarantees that every present and future caller hides the actions + // first. #4014 was exactly that failure — DeviceList hid them, DeviceCard + // did not, and the handler trusted its callers. A guard here cannot be + // re-opened by adding a third surface — which is precisely what adding + // the manual class did, so the guard is widened alongside it. A manual + // asset's `id` is a `manual_assets.id`, same foreign-id problem as + // network. `delete-manual` is the one manual-eligible action and is + // carved out explicitly rather than by omission. if ((device.deviceClass ?? 'agent') === 'network') { showToast({ type: 'error', message: t('devicesPage.toasts.agentOnlyAction') }); return; } + if ((device.deviceClass ?? 'agent') === 'manual' && action !== 'delete-manual') { + showToast({ type: 'error', message: t('devicesPage.toasts.agentOnlyActionManual') }); + return; + } if (CONFIRM_REQUIRED_ACTIONS.has(action)) { setPendingDeviceAction({ action, device }); return; @@ -1121,6 +1326,28 @@ export default function DevicesPage() { break; } + // Manual asset delete (#4622 W04) — hard delete, no undo (the API's + // own contract; unlike permanent-delete's device-purge flow, there is + // no soft-decommission step in between for a manual row). The route + // carries `requireMfa()` like every manual-asset mutator, so the + // MFA_REQUIRED 403 gets the same friendly copy other MFA-gated device + // mutations use (ArchiveOrgModal/MergeOrgModal precedent). + case 'delete-manual': { + try { + await runAction({ + request: () => fetchWithAuth(`/devices/manual/${device.id}`, { method: 'DELETE' }), + errorFallback: t('devicesPage.toasts.deleteManualFailed', { hostname: device.hostname }), + friendly: (code) => (code === 'MFA_REQUIRED' ? t('devicesPage.toasts.mfaRequired') : undefined), + onUnauthorized: handleSessionExpired, + successMessage: t('devicesPage.toasts.manualDeleted', { hostname: device.hostname }), + }); + await refreshDevices(); + } catch { + // runAction already toasted (or handled the 401 redirect). + } + break; + } + default: showToast({ type: 'error', message: t('devicesPage.toasts.unknownAction', { action }) }); } @@ -1209,6 +1436,25 @@ export default function DevicesPage() { const handleBulkAction = async (action: string, allSelectedDevices: Device[]) => { if (actionInProgress || allSelectedDevices.length === 0) return; + // Manual asset delete (#4622 W04) is the mirror image of every action + // below: it is MANUAL-only, so it must be handled BEFORE the agent-only + // allowlist a few lines down strips every manual row out. Skipped + // (non-manual) rows are reported the same "N of M eligible" way the + // agent-only actions already do, never silently. + if (action === 'delete-manual') { + const manualOnly = allSelectedDevices.filter(d => (d.deviceClass ?? 'agent') === 'manual'); + const skippedCount = allSelectedDevices.length - manualOnly.length; + if (manualOnly.length === 0) { + showToast({ type: 'error', message: t('devicesPage.toasts.manualOnlyAction') }); + return; + } + if (skippedCount > 0) { + showToast({ type: 'warning', message: t('devicesPage.toasts.manualSkipped', { count: skippedCount }) }); + } + setPendingBulkDeleteManual(manualOnly); + return; + } + // Every bulk action below talks to an enrolled agent (reboot/shutdown/lock, // maintenance, decommission, wake, run-script, deploy-software). A network // row's `id` is a `discovered_assets.id`, NOT a `devices.id` — feeding it @@ -1216,8 +1462,14 @@ export default function DevicesPage() { // and an unhandled throw mid-loop would silently skip every real device // after it. So drop network rows up front for these actions and tell the // user, rather than letting them flow into the per-device loops (#1322). + // Explicit agent-only allowlist (#4622 W04): `=== 'agent'` already drops + // BOTH network and manual rows correctly (the class union now has three + // members) — the toast wording below is what needs to stay honest, since + // "N network devices skipped" would mislabel a skipped manual asset. const selectedDevices = allSelectedDevices.filter(d => (d.deviceClass ?? 'agent') === 'agent'); - const skippedNetworkCount = allSelectedDevices.length - selectedDevices.length; + const skippedManualCount = allSelectedDevices.filter(d => (d.deviceClass ?? 'agent') === 'manual').length; + const skippedNonAgentCount = allSelectedDevices.length - selectedDevices.length; + const skippedNetworkCount = skippedNonAgentCount - skippedManualCount; if (selectedDevices.length === 0) { showToast({ type: 'error', @@ -1225,10 +1477,12 @@ export default function DevicesPage() { }); return; } - if (skippedNetworkCount > 0) { + if (skippedNonAgentCount > 0) { showToast({ type: 'warning', - message: t('devicesPage.toasts.networkSkipped', { count: skippedNetworkCount }), + message: skippedManualCount > 0 + ? t('devicesPage.toasts.nonAgentSkipped', { count: skippedNonAgentCount }) + : t('devicesPage.toasts.networkSkipped', { count: skippedNetworkCount }), }); } @@ -1675,6 +1929,68 @@ export default function DevicesPage() { } }; + // Manual asset bulk delete (#4622 W04). No bulk API route exists for this — + // a per-item DELETE loop, same shape as maintenance-off above: one item's + // failure must not abort the batch or silently skip everything after it. + // Each mutator carries requireMfa(), so a session missing MFA fails every + // item with the same MFA_REQUIRED code — collected once into its own + // friendly toast rather than repeated per-device. + const runBulkDeleteManual = async (selectedDevices: Device[]) => { + if (selectedDevices.length === 0) return; + setActionInProgress(true); + let mfaBlocked = false; + const failed: string[] = []; + try { + for (const device of selectedDevices) { + try { + const resp = await fetchWithAuth(`/devices/manual/${device.id}`, { method: 'DELETE' }); + if (resp.status === 401) { + handleSessionExpired(); + return; + } + if (!resp.ok) { + const body = await resp.json().catch(() => null); + if (resp.status === 403 && body && (body as { code?: string }).code === 'MFA_REQUIRED') { + mfaBlocked = true; + } + failed.push(device.hostname || device.id); + } + } catch (err) { + // Logged (not silent) — MFA detection above only inspects the + // response shape, so a network failure or a non-JSON error body + // would otherwise be invisible beyond "one of N failed". + console.warn(`[DevicesPage] delete-manual failed for ${device.id}:`, err); + failed.push(device.hostname || device.id); + } + } + const succeeded = selectedDevices.length - failed.length; + if (mfaBlocked) { + showToast({ type: 'error', message: t('devicesPage.toasts.mfaRequired') }); + } else if (failed.length === 0) { + showToast({ type: 'success', message: t('devicesPage.toasts.bulkManualDeleted', { count: succeeded }) }); + } else if (succeeded === 0) { + showToast({ + type: 'error', + message: t('devicesPage.toasts.bulkManualDeleteAllFailed', { count: failed.length, devices: summarizeFailedDevices(failed) }), + }); + } else { + showToast({ + type: 'error', + message: t('devicesPage.toasts.bulkManualDeleteSomeFailed', { succeeded, failed: failed.length, devices: summarizeFailedDevices(failed) }), + }); + } + await refreshDevices(); + } catch (err) { + // Mirrors runBulkAction/runBulkPurge/runBulkRemove above: anything that + // throws AFTER the per-item loop (t(), summarizeFailedDevices(), + // refreshDevices()) must still surface a toast, or a hard, no-undo + // delete of several assets reports nothing at all to the operator. + showToast({ type: 'error', message: err instanceof Error ? err.message : t('devicesPage.toasts.bulkActionFailed', { action: 'delete-manual' }) }); + } finally { + setActionInProgress(false); + } + }; + // The org context itself failed to load (#4147 review). Falling through would // fetch with no orgId, and the API reads an absent orgId as "every accessible // org" — so a transient /orgs/organizations failure would quietly render a @@ -1757,7 +2073,7 @@ export default function DevicesPage() {
-

{t('devicesPage.title')}

+

{t('devicesPage.title')}

{t('devicesPage.subtitle')}

@@ -1798,52 +2114,16 @@ export default function DevicesPage() { > {t('devicesPage.importFromRmm')} - {/* Add split menu (#5213 W02): "Install agent…" is the pre-existing - AddDeviceModal flow; "Add network asset…" is new. Built as a - menu (not a single button) so a third item — #4622's "Add asset - manually…" — slots in later without restructuring this block. */} -
- - {addMenuOpen && ( -
- - -
- )} -
+ setShowAddDevice(true)} + onAddManualAsset={() => setShowAddManualAsset(true)} + onAddNetworkAsset={() => { + window.location.hash = 'add-network-asset'; + setShowAddNetworkAsset(true); + }} + />
@@ -1869,11 +2149,16 @@ export default function DevicesPage() { /> )} - {/* Class segment (#1424) — only meaningful when the merged list carries - both arms; hidden entirely in the agent-only (flag-off) view. Narrows - both views: the table via classFilteredDevices, the grid via + {/* Class segment (#1424, #4622) — only meaningful when the merged list + carries more than the agent arm; hidden entirely in the pure + agent-only view. The manual arm carries NO feature flag (independent + of ENABLE_NETWORK_DEVICES_IN_LIST by design), so this must show + whenever EITHER the network flag is on OR a manual asset exists — + gating on the network flag alone would hide the Manual segment (and + its count) on an org that has manual assets but the network arm off. + Narrows both views: the table via classFilteredDevices, the grid via gridDevices (same class rule over the filtered fleet). */} - {ENABLE_NETWORK_DEVICES_IN_LIST && ( + {(ENABLE_NETWORK_DEVICES_IN_LIST || deviceClassCounts.manual > 0) && (
{hiddenNetwork.count > 0 && (

- {t('devicesPage.hiddenNetworkNotice', { count: hiddenNetwork.count, fields: hiddenNetworkFieldLabels })} + {t('devicesPage.hiddenNonAgentNotice', { + count: hiddenNetwork.count, + class: hiddenNonAgentClassLabel, + fields: hiddenNetworkFieldLabels, + })}

)}
@@ -1906,46 +2195,16 @@ export default function DevicesPage() { {t('devicesPage.emptyDescription')}

-
- - {addMenuOpen && ( -
- - -
- )} -
+ setShowAddDevice(true)} + onAddManualAsset={() => setShowAddManualAsset(true)} + onAddNetworkAsset={() => { + window.location.hash = 'add-network-asset'; + setShowAddNetworkAsset(true); + }} + /> {t('devicesPage.viewInstallationGuide')} @@ -2035,6 +2294,21 @@ export default function DevicesPage() { onCreated={() => { void refreshDevices(); }} /> + { + setShowAddManualAsset(false); + setEditingManualAsset(null); + if (window.location.hash === '#add-manual-asset') window.location.hash = ''; + }} + onSaved={refreshDevices} + organizationId={orgScope.status === 'resolved' && orgScope.scope !== 'all' ? orgScope.orgId : null} + orgs={orgs} + sites={sites} + existing={editingManualAsset} + linkableDevices={devices.filter((d) => (d.deviceClass ?? 'agent') !== 'manual')} + /> + {showRmmImport && ( )} + {pendingBulkDeleteManual && ( + setPendingBulkDeleteManual(null)} + onConfirm={() => { + const targets = pendingBulkDeleteManual; + setPendingBulkDeleteManual(null); + void runBulkDeleteManual(targets); + }} + title={t('deviceActions.confirm.bulkDeleteManual.title', { count: pendingBulkDeleteManual.length })} + message={t('deviceActions.confirm.bulkDeleteManual.message', { count: pendingBulkDeleteManual.length })} + confirmLabel={t('deviceActions.confirm.bulkDeleteManual.confirm')} + variant="destructive" + isLoading={actionInProgress} + confirmTestId="confirm-bulk-delete-manual" + /> + )} + {pendingBulkPurge && ( ({ + fetchWithAuth: vi.fn(), + handleSessionExpired: vi.fn(), +})); + +vi.mock('../shared/Toast', () => ({ showToast: vi.fn() })); + +const fetchMock = vi.mocked(fetchWithAuth); +const sessionExpiredMock = vi.mocked(handleSessionExpired); +const toastMock = vi.mocked(showToast); + +const jsonResponse = (payload: unknown, ok = true, status = ok ? 200 : 500): Response => + ({ ok, status, statusText: ok ? 'OK' : 'ERROR', json: vi.fn().mockResolvedValue(payload) }) as unknown as Response; + +const ORG_ID = 'aaaaaaaa-1111-1111-1111-111111111111'; +const SITE_ID = 'bbbbbbbb-1111-1111-1111-111111111111'; + +const ORGS = [{ id: ORG_ID, name: 'Acme Corp' }]; +const ONE_SITE = [{ id: SITE_ID, name: 'HQ', orgId: ORG_ID }]; +const TWO_SITES = [ + { id: SITE_ID, name: 'HQ', orgId: ORG_ID }, + { id: 'cccccccc-1111-1111-1111-111111111111', name: 'Branch', orgId: ORG_ID }, +]; + +const EXISTING: Device = { + id: 'dddddddd-1111-1111-1111-111111111111', + deviceClass: 'manual', + hostname: 'spare-laptop', + displayName: 'Spare Laptop', + os: '' as Device['os'], + osVersion: '', + status: 'unknown', + cpuPercent: 0, + ramPercent: 0, + lastSeen: '', + orgId: ORG_ID, + orgName: 'Acme Corp', + siteId: SITE_ID, + siteName: 'HQ', + agentVersion: '', + tags: ['loaner'], + serialNumber: 'SN-1', + assetTag: 'TAG-1', + location: 'Closet B', + linkedDeviceId: null, + linkedDiscoveredAssetId: null, + notes: 'Some notes', +}; + +function renderModal(overrides?: Partial[0]>) { + const onClose = overrides?.onClose ?? vi.fn(); + const onSaved = overrides?.onSaved ?? vi.fn(); + render( + , + ); + return { onClose, onSaved }; +} + +beforeEach(() => { + fetchMock.mockReset(); + sessionExpiredMock.mockReset(); + toastMock.mockReset(); + fetchMock.mockImplementation(async (input: string | URL | Request) => { + const url = String(input); + if (url.includes('/contacts')) return jsonResponse({ data: [] }); + return jsonResponse({ id: 'new-id' }, true, 201); + }); +}); + +describe('ManualAssetModal — create', () => { + it('Name is required — Create stays disabled until a name is entered', () => { + renderModal(); + expect(screen.getByTestId('manual-asset-submit')).toBeDisabled(); + fireEvent.change(screen.getByTestId('manual-asset-name'), { target: { value: 'Spare phone' } }); + expect(screen.getByTestId('manual-asset-submit')).not.toBeDisabled(); + }); + + it('defaults Site to the org\'s only site when there is exactly one', () => { + renderModal({ sites: ONE_SITE }); + expect(screen.getByTestId('manual-asset-site')).toHaveValue(SITE_ID); + }); + + it('leaves Site unselected when the org has more than one', () => { + renderModal({ sites: TWO_SITES }); + expect(screen.getByTestId('manual-asset-site')).toHaveValue(''); + }); + + it('submit POSTs to /devices/manual via runAction (fetchWithAuth)', async () => { + renderModal({ sites: ONE_SITE }); + fireEvent.change(screen.getByTestId('manual-asset-name'), { target: { value: 'Spare phone' } }); + fireEvent.click(screen.getByTestId('manual-asset-submit')); + + await waitFor(() => { + const call = fetchMock.mock.calls.find(([u]) => String(u) === '/devices/manual'); + expect(call).toBeDefined(); + }); + const [, init] = fetchMock.mock.calls.find(([u]) => String(u) === '/devices/manual')!; + expect(init?.method).toBe('POST'); + const body = JSON.parse(String(init?.body)); + expect(body).toMatchObject({ orgId: ORG_ID, siteId: SITE_ID, name: 'Spare phone' }); + }); + + it('a DUPLICATE_SERIAL warning renders non-blockingly and does not prevent the create from completing', async () => { + fetchMock.mockImplementation(async (input: string | URL | Request) => { + const url = String(input); + if (url.includes('/contacts')) return jsonResponse({ data: [] }); + return jsonResponse( + { id: 'new-id', warnings: [{ code: 'DUPLICATE_SERIAL', message: 'An asset with this serial number already exists in this organization' }] }, + true, + 201, + ); + }); + const { onSaved } = renderModal({ sites: ONE_SITE }); + fireEvent.change(screen.getByTestId('manual-asset-name'), { target: { value: 'Spare phone' } }); + fireEvent.click(screen.getByTestId('manual-asset-submit')); + + await waitFor(() => expect(onSaved).toHaveBeenCalled()); + expect(screen.getByTestId('manual-asset-duplicate-warning')).toHaveTextContent(/already exists/); + // Non-blocking: the create succeeded (onSaved fired) and the modal stays + // open on the warning rather than closing — verified by the input still + // being present. + expect(screen.getByTestId('manual-asset-name')).toBeInTheDocument(); + }); +}); + +describe('ManualAssetModal — edit + link control', () => { + it('the link control is on the modal\'s primary surface, not behind a tab', () => { + renderModal({ existing: EXISTING, sites: ONE_SITE }); + // No tab-switch interaction — the section is visible immediately. + expect(screen.getByTestId('manual-asset-link-section')).toBeVisible(); + expect(screen.getByTestId('manual-asset-link-device-select')).toBeInTheDocument(); + }); + + it('shows "Linked to" and an Unlink button when already linked', () => { + const linkedTarget: Device = { ...EXISTING, id: 'agent-1', deviceClass: 'agent', hostname: 'win-box', displayName: undefined }; + renderModal({ + existing: { ...EXISTING, linkedDeviceId: 'agent-1' }, + sites: ONE_SITE, + linkableDevices: [linkedTarget], + }); + expect(screen.getByText(/win-box/)).toBeInTheDocument(); + expect(screen.getByTestId('manual-asset-unlink-button')).toBeInTheDocument(); + }); + + it('pre-fills the form from the existing asset', () => { + renderModal({ existing: EXISTING, sites: ONE_SITE }); + expect(screen.getByTestId('manual-asset-name')).toHaveValue('Spare Laptop'); + expect(screen.getByTestId('manual-asset-serial')).toHaveValue('SN-1'); + expect(screen.getByTestId('manual-asset-tag')).toHaveValue('TAG-1'); + expect(screen.getByTestId('manual-asset-location')).toHaveValue('Closet B'); + }); +}); diff --git a/apps/web/src/components/devices/ManualAssetModal.tsx b/apps/web/src/components/devices/ManualAssetModal.tsx new file mode 100644 index 0000000000..24ab523e35 --- /dev/null +++ b/apps/web/src/components/devices/ManualAssetModal.tsx @@ -0,0 +1,644 @@ +import { useEffect, useMemo, useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import '../../lib/i18n'; +import { Dialog } from '../shared/Dialog'; +import { showToast } from '../shared/Toast'; +import { fetchWithAuth, handleSessionExpired } from '../../stores/auth'; +import { runAction, ActionError } from '../../lib/runAction'; +import { asList } from '@/lib/asList'; +import { DEVICE_ROLES, getDeviceRoleLabel, getDeviceRoleIcon, type DeviceRole } from '@/lib/deviceRoles'; +import type { Device } from './DeviceList'; + +export interface ManualAssetOrgOption { + id: string; + name: string; +} + +export interface ManualAssetSiteOption { + id: string; + name: string; + orgId?: string; +} + +interface ManualAssetContact { + id: string; + name: string | null; + email: string | null; +} + +export interface ManualAssetModalProps { + isOpen: boolean; + onClose: () => void; + /** Fired once the create/update/link/unlink succeeds — the caller refetches. */ + onSaved: () => void; + /** + * Fixed org context (the page is scoped to one org), or `null` in fleet + * view — in which case the operator must choose an org from `orgs` before + * anything else on the form is usable. + */ + organizationId: string | null; + orgs: ManualAssetOrgOption[]; + sites: ManualAssetSiteOption[]; + /** Non-null when editing an existing manual asset; null when creating. */ + existing: Device | null; + /** + * Every non-manual row in the currently-loaded fleet (agent + network), + * used to populate the link pickers and to resolve an already-linked + * target's display name. Manual assets are never link targets themselves. + */ + linkableDevices: Device[]; +} + +type FormState = { + orgId: string; + siteId: string; + name: string; + assetType: DeviceRole; + manufacturer: string; + model: string; + serialNumber: string; + assetTag: string; + location: string; + assignedContactId: string; + assignedContactLabel: string; + tags: string; + notes: string; +}; + +const emptyForm = (orgId: string): FormState => ({ + orgId, + siteId: '', + name: '', + assetType: 'unknown', + manufacturer: '', + model: '', + serialNumber: '', + assetTag: '', + location: '', + assignedContactId: '', + assignedContactLabel: '', + tags: '', + notes: '', +}); + +/** + * Add/edit modal for a manual (hand-entered, non-networked) inventory asset + * (#4622 W04). Every mutation goes through `runAction` — the API gates every + * manual-asset mutator behind `requireMfa()`, so a 403 `MFA_REQUIRED` is an + * expected outcome here, not a bug, and gets the same friendly copy other + * MFA-gated device mutations use (ArchiveOrgModal/MergeOrgModal precedent). + * + * The link control lives here, on the modal's primary surface, deliberately — + * #3451 records that today's answer (buried in a tab on + * /devices/network/:id) is the mistake to not repeat. + */ +export default function ManualAssetModal({ + isOpen, + onClose, + onSaved, + organizationId, + orgs, + sites, + existing, + linkableDevices, +}: ManualAssetModalProps) { + const { t } = useTranslation('devices'); + const isEdit = existing != null; + + const [form, setForm] = useState(() => emptyForm(organizationId ?? '')); + const [submitting, setSubmitting] = useState(false); + const [duplicateWarning, setDuplicateWarning] = useState(null); + + const [contacts, setContacts] = useState([]); + const [contactsLoadedForOrg, setContactsLoadedForOrg] = useState(null); + const [contactQuery, setContactQuery] = useState(''); + const [contactPickerOpen, setContactPickerOpen] = useState(false); + + const [linkTargetId, setLinkTargetId] = useState(''); + const [linkBusy, setLinkBusy] = useState(false); + + // Seed the form whenever the modal opens (create) or the edited row changes. + useEffect(() => { + if (!isOpen) return; + if (existing) { + setForm({ + orgId: existing.orgId, + siteId: existing.siteId, + name: existing.displayName || existing.hostname, + assetType: (existing.assetType as DeviceRole | undefined) ?? 'unknown', + manufacturer: existing.manufacturer ?? '', + model: existing.model ?? '', + serialNumber: existing.serialNumber ?? '', + assetTag: existing.assetTag ?? '', + location: existing.location ?? '', + assignedContactId: existing.assignedContactId ?? '', + assignedContactLabel: '', + tags: (existing.tags ?? []).join(', '), + notes: existing.notes ?? '', + }); + } else { + setForm(emptyForm(organizationId ?? '')); + } + setDuplicateWarning(null); + setLinkTargetId(''); + }, [isOpen, existing, organizationId]); + + const sitesForOrg = useMemo( + () => sites.filter((s) => !form.orgId || s.orgId === form.orgId), + [sites, form.orgId], + ); + + // Site defaults to the org's only site when there is exactly one (spec). + // Only auto-applies on CREATE — an edit's siteId is the asset's own truth. + useEffect(() => { + if (isEdit) return; + if (form.siteId) return; + if (sitesForOrg.length === 1) { + setForm((f) => ({ ...f, siteId: sitesForOrg[0]!.id })); + } + }, [isEdit, form.siteId, sitesForOrg]); + + // Load the selected org's contacts once per org (no server-side search on + // this route today — filtered client-side as the operator types). + useEffect(() => { + if (!form.orgId || contactsLoadedForOrg === form.orgId) return; + let cancelled = false; + (async () => { + try { + const resp = await fetchWithAuth(`/orgs/organizations/${form.orgId}/contacts?limit=200`); + if (!resp.ok) { + // Best-effort, non-blocking field: the picker just stays empty + // rather than blocking the whole form. Logged (not silent) so a + // systemic auth/permission failure on this route is still + // debuggable instead of looking identical to "org has no contacts". + console.warn('[ManualAssetModal] failed to load org contacts:', resp.status); + return; + } + const data = await resp.json(); + if (!cancelled) { + setContacts(asList(data)); + setContactsLoadedForOrg(form.orgId); + } + } catch (err) { + console.warn('[ManualAssetModal] failed to load org contacts:', err); + } + })(); + return () => { + cancelled = true; + }; + }, [form.orgId, contactsLoadedForOrg]); + + const filteredContacts = useMemo(() => { + const q = contactQuery.trim().toLowerCase(); + if (!q) return contacts.slice(0, 20); + return contacts.filter((c) => (c.name ?? '').toLowerCase().includes(q) || (c.email ?? '').toLowerCase().includes(q)).slice(0, 20); + }, [contacts, contactQuery]); + + const canSubmit = form.orgId.length > 0 && form.siteId.length > 0 && form.name.trim().length > 0 && !submitting; + + const mfaFriendly = (code: string) => (code === 'MFA_REQUIRED' ? t('manualAssetModal.errors.mfaRequired') : undefined); + + const handleSubmit = async () => { + if (!canSubmit) return; + setSubmitting(true); + setDuplicateWarning(null); + const tags = form.tags.split(',').map((s) => s.trim()).filter(Boolean); + const body: Record = { + siteId: form.siteId, + name: form.name.trim(), + assetType: form.assetType, + manufacturer: form.manufacturer.trim() || null, + model: form.model.trim() || null, + serialNumber: form.serialNumber.trim() || null, + assetTag: form.assetTag.trim() || null, + location: form.location.trim() || null, + assignedContactId: form.assignedContactId || null, + notes: form.notes.trim() || null, + tags, + }; + if (!isEdit) body.orgId = form.orgId; + + // The mutation itself and its post-success side effects (onSaved/onClose) + // are deliberately NOT in the same try/catch: a throw from either side + // effect must never be re-labeled as "create/update failed" when the + // asset was in fact saved — runAction's own success toast already fired. + let result: { warnings?: { code: string; message: string }[] } | undefined; + try { + result = await runAction<{ warnings?: { code: string; message: string }[] }>({ + request: () => + fetchWithAuth(isEdit ? `/devices/manual/${existing!.id}` : '/devices/manual', { + method: isEdit ? 'PATCH' : 'POST', + body: JSON.stringify(body), + }), + errorFallback: isEdit ? t('manualAssetModal.errors.updateFailed') : t('manualAssetModal.errors.createFailed'), + friendly: mfaFriendly, + onUnauthorized: handleSessionExpired, + successMessage: isEdit ? t('manualAssetModal.toasts.updated') : t('manualAssetModal.toasts.created'), + }); + } catch (err) { + if (!(err instanceof ActionError)) { + showToast({ type: 'error', message: isEdit ? t('manualAssetModal.errors.updateFailed') : t('manualAssetModal.errors.createFailed') }); + } + setSubmitting(false); + return; + } + setSubmitting(false); + // Non-blocking: the create succeeded either way (spec — duplicate serial + // is a soft hint, never a rejection). Surfaced as its own toast so it + // doesn't get lost inside the success message. + const dup = result.warnings?.find((w) => w.code === 'DUPLICATE_SERIAL'); + if (dup) { + setDuplicateWarning(dup.message); + showToast({ type: 'warning', message: dup.message }); + } + try { + onSaved(); + if (!dup) onClose(); + } catch (err) { + // The save genuinely succeeded — a bug in the caller's refresh/close + // handler must not read back as a failed save. Logged, not silent. + console.error('[ManualAssetModal] onSaved/onClose threw after a successful save', err); + } + }; + + const handleLink = async (kind: 'device' | 'asset') => { + if (!existing || !linkTargetId || linkBusy) return; + setLinkBusy(true); + try { + await runAction({ + request: () => + fetchWithAuth(`/devices/manual/${existing.id}/link`, { + method: 'POST', + body: JSON.stringify(kind === 'device' ? { deviceId: linkTargetId } : { discoveredAssetId: linkTargetId }), + }), + errorFallback: t('manualAssetModal.errors.linkFailed'), + friendly: mfaFriendly, + onUnauthorized: handleSessionExpired, + successMessage: t('manualAssetModal.toasts.linked'), + }); + } catch { + // runAction already toasted (or handled the 401 redirect). + return; + } finally { + setLinkBusy(false); + } + // The link genuinely succeeded — a throw here must not read back as a + // failed link (runAction's own catch above already handled that case). + try { + onSaved(); + onClose(); + } catch (err) { + console.error('[ManualAssetModal] onSaved/onClose threw after a successful link', err); + } + }; + + const handleUnlink = async () => { + if (!existing || linkBusy) return; + setLinkBusy(true); + try { + await runAction({ + request: () => fetchWithAuth(`/devices/manual/${existing.id}/link`, { method: 'DELETE' }), + errorFallback: t('manualAssetModal.errors.unlinkFailed'), + friendly: mfaFriendly, + onUnauthorized: handleSessionExpired, + successMessage: t('manualAssetModal.toasts.unlinked'), + }); + } catch { + // runAction already toasted (or handled the 401 redirect). + return; + } finally { + setLinkBusy(false); + } + try { + onSaved(); + } catch (err) { + console.error('[ManualAssetModal] onSaved threw after a successful unlink', err); + } + }; + + if (!isOpen) return null; + + const linkedTarget = existing?.linkedDeviceId + ? linkableDevices.find((d) => d.id === existing.linkedDeviceId) + : existing?.linkedDiscoveredAssetId + ? linkableDevices.find((d) => d.id === existing.linkedDiscoveredAssetId) + : null; + const isLinked = !!(existing?.linkedDeviceId || existing?.linkedDiscoveredAssetId); + const linkCandidateDevices = linkableDevices.filter((d) => (d.deviceClass ?? 'agent') === 'agent' && d.orgId === form.orgId); + const linkCandidateAssets = linkableDevices.filter((d) => (d.deviceClass ?? 'agent') === 'network' && d.orgId === form.orgId); + + return ( + +
+

+ {isEdit ? t('manualAssetModal.editTitle') : t('manualAssetModal.addTitle')} +

+ + {/* Link control — the modal's PRIMARY surface (#3451: not a tab). */} + {isEdit && ( +
+ {isLinked ? ( +
+ + {t('manualAssetModal.linkedTo', { name: linkedTarget?.displayName || linkedTarget?.hostname || t('manualAssetModal.linkedUnknown') })} + + +
+ ) : ( +
+

{t('manualAssetModal.linkSectionTitle')}

+
+ + +
+
+ + +
+
+ )} +
+ )} + + {duplicateWarning && ( +

+ {duplicateWarning} +

+ )} + +
+
+ + {isEdit || organizationId ? ( + o.id === form.orgId)?.name ?? form.orgId} + className="w-full rounded-md border bg-muted px-3 py-2 text-sm text-muted-foreground" + /> + ) : ( + + )} +
+ +
+ + +
+ +
+ + setForm((f) => ({ ...f, name: e.target.value }))} + placeholder={t('manualAssetModal.fields.namePlaceholder')} + className="w-full rounded-md border bg-background px-3 py-2 text-sm" + /> +
+ +
+ + +
+ +
+ {(() => { + const Icon = getDeviceRoleIcon(form.assetType); + return
+ +
+ + setForm((f) => ({ ...f, manufacturer: e.target.value }))} + className="w-full rounded-md border bg-background px-3 py-2 text-sm" + /> +
+ +
+ + setForm((f) => ({ ...f, model: e.target.value }))} + className="w-full rounded-md border bg-background px-3 py-2 text-sm" + /> +
+ +
+ + setForm((f) => ({ ...f, serialNumber: e.target.value }))} + className="w-full rounded-md border bg-background px-3 py-2 text-sm" + /> +
+ +
+ + setForm((f) => ({ ...f, assetTag: e.target.value }))} + className="w-full rounded-md border bg-background px-3 py-2 text-sm" + /> +
+ +
+ + setForm((f) => ({ ...f, location: e.target.value }))} + placeholder={t('manualAssetModal.fields.locationPlaceholder')} + className="w-full rounded-md border bg-background px-3 py-2 text-sm" + /> +
+ +
+ + { + setContactQuery(e.target.value); + setContactPickerOpen(true); + setForm((f) => ({ ...f, assignedContactId: '', assignedContactLabel: '' })); + }} + onFocus={() => setContactPickerOpen(true)} + placeholder={t('manualAssetModal.fields.assignedContactPlaceholder')} + className="w-full rounded-md border bg-background px-3 py-2 text-sm" + /> + {contactPickerOpen && filteredContacts.length > 0 && ( +
+ {filteredContacts.map((c) => ( + + ))} +
+ )} +
+ +
+ + setForm((f) => ({ ...f, tags: e.target.value }))} + placeholder={t('manualAssetModal.fields.tagsPlaceholder')} + className="w-full rounded-md border bg-background px-3 py-2 text-sm" + /> +
+ +
+ +