fix(inventory): read a device's curated type on its inventory card
A device documented straight on a canvas carries a `label` and a `type` and no discovery guess at all — no scan ever saw it. The card read `suggested_type` alone, so a printer drawn on a canvas fell back to 'generic': unnamed, filed under no type, and drawn with a plain circle. Two faults compounded: - The card decided the kind from `suggested_type`, ignoring the curated `type`. That also meant retyping a scanned device in the detail modal changed nothing here, and approving it still built a node of the type the scanner had guessed. - It carried its own table of eleven icons, unrelated to the ~40 NodeType values, so anything outside that list drew a circle even with the right type in hand. `deviceType()` now resolves `type` before `suggested_type` for the icon, the badge, the type filter and both approve paths, and the icons come from the shared `NODE_TYPE_DEFAULT_ICONS` — one map, the same one the canvas and the detail modal use. `deviceLabel` prefers the curated `label` for the same reason, otherwise a canvas-created device shows as its IP. ha-relevant: maybe
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
import { useState, useEffect, useCallback, useRef, useMemo } from 'react'
|
||||
import {
|
||||
Globe, Router, Server, Layers, Box, Container, HardDrive, Cpu, Wifi, Circle, Network,
|
||||
Server, Layers,
|
||||
Search, RefreshCw, X, CheckCircle2, EyeOff, Trash2, Loader2, ServerCog,
|
||||
} from 'lucide-react'
|
||||
import { Dialog, DialogClose, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog'
|
||||
@@ -9,6 +9,7 @@ import { useCanvasStore } from '@/stores/canvasStore'
|
||||
import { useDesignStore } from '@/stores/designStore'
|
||||
import { useThemeStore } from '@/stores/themeStore'
|
||||
import { resolveNodeColors } from '@/utils/nodeColors'
|
||||
import { NODE_TYPE_DEFAULT_ICONS } from '@/utils/nodeIcons'
|
||||
import { toast } from 'sonner'
|
||||
import { InventoryDeviceModal, type InventoryEntry } from '@/components/modals/InventoryDeviceModal'
|
||||
import type { NodeType, ServiceInfo } from '@/types'
|
||||
@@ -72,25 +73,20 @@ function serviceColor(port: number | null | undefined, category?: string | null)
|
||||
return '#8b949e'
|
||||
}
|
||||
|
||||
const TYPE_ICONS: Record<string, React.ElementType> = {
|
||||
isp: Globe,
|
||||
router: Router,
|
||||
server: Server,
|
||||
proxmox: Layers,
|
||||
vm: Box,
|
||||
lxc: Container,
|
||||
nas: HardDrive,
|
||||
iot: Cpu,
|
||||
ap: Wifi,
|
||||
switch: Network,
|
||||
generic: Circle,
|
||||
}
|
||||
|
||||
type SourceFilter = 'all' | SourceBucket
|
||||
type StatusFilter = 'pending' | 'hidden'
|
||||
|
||||
const COMMON_PORTS = new Set([22, 80, 443])
|
||||
|
||||
/**
|
||||
* What the device is. The curated `type` wins over the discovery guess: a
|
||||
* device drawn on a canvas only ever has the former, and an edit that retypes a
|
||||
* scanned row must not keep reading the scanner's opinion.
|
||||
*/
|
||||
function deviceType(d: InventoryEntry): NodeType | null {
|
||||
return ((d.type ?? d.suggested_type) as NodeType) ?? null
|
||||
}
|
||||
|
||||
function specialServiceName(d: InventoryEntry): string | undefined {
|
||||
const candidates = (d.services ?? []).filter(
|
||||
(s) => s.category != null && s.port != null && !COMMON_PORTS.has(s.port) && s.service_name,
|
||||
@@ -101,7 +97,8 @@ function specialServiceName(d: InventoryEntry): string | undefined {
|
||||
}
|
||||
|
||||
function deviceLabel(d: InventoryEntry): string {
|
||||
return d.friendly_name ?? d.hostname ?? specialServiceName(d) ?? d.ip ?? d.ieee_address ?? 'device'
|
||||
// Same precedence as the detail modal — the name the user gave it first.
|
||||
return d.label ?? d.friendly_name ?? d.hostname ?? specialServiceName(d) ?? d.ip ?? d.ieee_address ?? 'device'
|
||||
}
|
||||
|
||||
// Pull the duplicate-conflict payload out of a 409 approve response, if that's
|
||||
@@ -182,7 +179,7 @@ export function DeviceInventoryModal({ open, onClose, highlightId, initialStatus
|
||||
|
||||
const distinctTypes = useMemo(() => {
|
||||
const set = new Set<string>()
|
||||
devices.forEach((d) => { if (d.suggested_type) set.add(d.suggested_type) })
|
||||
devices.forEach((d) => { const t = deviceType(d); if (t) set.add(t) })
|
||||
return [...set].sort()
|
||||
}, [devices])
|
||||
|
||||
@@ -190,7 +187,7 @@ export function DeviceInventoryModal({ open, onClose, highlightId, initialStatus
|
||||
const q = search.trim().toLowerCase()
|
||||
return devices.filter((d) => {
|
||||
if (sourceFilter !== 'all' && !sourceBuckets(d).has(sourceFilter)) return false
|
||||
if (typeFilter !== 'all' && d.suggested_type !== typeFilter) return false
|
||||
if (typeFilter !== 'all' && deviceType(d) !== typeFilter) return false
|
||||
// Inventory-only: optionally hide devices already placed on a canvas.
|
||||
if (statusFilter === 'pending' && !showOnCanvas && (d.canvas_count ?? 0) > 0) return false
|
||||
if (withServicesOnly && (d.services?.length ?? 0) === 0) return false
|
||||
@@ -302,7 +299,7 @@ export function DeviceInventoryModal({ open, onClose, highlightId, initialStatus
|
||||
return
|
||||
}
|
||||
const fallbackLabel = deviceLabel(device)
|
||||
const type = (device.suggested_type ?? 'generic') as NodeType
|
||||
const type = deviceType(device) ?? 'generic'
|
||||
const zwave = isZwaveType(type)
|
||||
const wireless = isZigbeeType(type) || zwave
|
||||
const properties = zwave
|
||||
@@ -405,7 +402,7 @@ export function DeviceInventoryModal({ open, onClose, highlightId, initialStatus
|
||||
approvedDevices.forEach((d, i) => {
|
||||
const nodeId = deviceToNode[d.id]
|
||||
if (!nodeId) return
|
||||
const type = (d.suggested_type ?? 'generic') as NodeType
|
||||
const type = deviceType(d) ?? 'generic'
|
||||
const zwave = isZwaveType(type)
|
||||
const wireless = isZigbeeType(type) || zwave
|
||||
addNode({
|
||||
@@ -827,8 +824,8 @@ interface DeviceCardProps {
|
||||
|
||||
function DeviceCard({ device, selected, selectMode, highlighted, onClick, cardRef }: DeviceCardProps) {
|
||||
const sources = orderedSources(device)
|
||||
const roleType = (device.suggested_type ?? 'generic') as NodeType
|
||||
const Icon = TYPE_ICONS[roleType] ?? Circle
|
||||
const roleType = deviceType(device) ?? 'generic'
|
||||
const Icon = NODE_TYPE_DEFAULT_ICONS[roleType] ?? NODE_TYPE_DEFAULT_ICONS.generic
|
||||
const activeTheme = useThemeStore((s) => s.activeTheme)
|
||||
// Colour the role badge with the same accent the node uses on the canvas
|
||||
// (from the active theme / style section), instead of a flat grey.
|
||||
@@ -906,12 +903,12 @@ function DeviceCard({ device, selected, selectMode, highlighted, onClick, cardRe
|
||||
{SOURCE_META[s].label}
|
||||
</span>
|
||||
))}
|
||||
{device.suggested_type && (
|
||||
{roleType !== 'generic' && (
|
||||
<span
|
||||
className="text-[9px] font-mono px-1.5 py-0.5 rounded uppercase tracking-wider"
|
||||
style={{ background: `${roleColor}22`, color: roleColor }}
|
||||
>
|
||||
{device.suggested_type}
|
||||
{roleType}
|
||||
</span>
|
||||
)}
|
||||
{device.lqi != null && (
|
||||
|
||||
@@ -693,3 +693,70 @@ describe('DeviceInventoryModal', () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* A device documented straight on a canvas carries a curated `label` and `type`
|
||||
* and no discovery guess at all — the card has to read those, or a printer
|
||||
* shows up unnamed with the generic circle.
|
||||
*/
|
||||
describe('DeviceInventoryModal — the curated type wins over the discovery guess', () => {
|
||||
const DEVICE_CANVAS = {
|
||||
id: 'dev-printer',
|
||||
ip: '192.168.1.77',
|
||||
hostname: null,
|
||||
mac: null,
|
||||
os: null,
|
||||
services: [],
|
||||
label: 'Office printer',
|
||||
type: 'printer',
|
||||
suggested_type: null,
|
||||
status: 'approved',
|
||||
discovery_source: 'canvas',
|
||||
discovery_sources: ['canvas'],
|
||||
discovered_at: '2026-02-01T00:00:00Z',
|
||||
}
|
||||
|
||||
it('names the card from the curated label', async () => {
|
||||
mockPending.mockResolvedValue({ data: [DEVICE_CANVAS] })
|
||||
render(<DeviceInventoryModal {...baseProps} />)
|
||||
await waitFor(() => expect(screen.getByTestId('pending-card-dev-printer')).toBeInTheDocument())
|
||||
expect(screen.getByText('Office printer')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('badges the curated type', async () => {
|
||||
mockPending.mockResolvedValue({ data: [DEVICE_CANVAS] })
|
||||
render(<DeviceInventoryModal {...baseProps} />)
|
||||
await waitFor(() => expect(screen.getByTestId('pending-card-dev-printer')).toBeInTheDocument())
|
||||
expect(within(screen.getByTestId('pending-card-dev-printer')).getByText('printer')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('prefers it over a stale discovery guess', async () => {
|
||||
mockPending.mockResolvedValue({ data: [{ ...DEVICE_CANVAS, suggested_type: 'server' }] })
|
||||
render(<DeviceInventoryModal {...baseProps} />)
|
||||
await waitFor(() => expect(screen.getByTestId('pending-card-dev-printer')).toBeInTheDocument())
|
||||
const card = within(screen.getByTestId('pending-card-dev-printer'))
|
||||
expect(card.getByText('printer')).toBeInTheDocument()
|
||||
expect(card.queryByText('server')).toBeNull()
|
||||
})
|
||||
|
||||
it('filters on it', async () => {
|
||||
mockPending.mockResolvedValue({ data: [DEVICE_CANVAS, DEVICE_IP] })
|
||||
render(<DeviceInventoryModal {...baseProps} />)
|
||||
await waitFor(() => expect(screen.getByTestId('pending-card-dev-printer')).toBeInTheDocument())
|
||||
|
||||
fireEvent.change(screen.getByLabelText('Type filter'), { target: { value: 'printer' } })
|
||||
expect(screen.getByTestId('pending-card-dev-printer')).toBeInTheDocument()
|
||||
expect(screen.queryByTestId('pending-card-dev-a')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('approves onto a node of that type', async () => {
|
||||
mockPending.mockResolvedValue({ data: [DEVICE_CANVAS] })
|
||||
render(<DeviceInventoryModal {...baseProps} />)
|
||||
await waitFor(() => expect(screen.getByTestId('pending-card-dev-printer')).toBeInTheDocument())
|
||||
|
||||
fireEvent.click(screen.getByTestId('pending-card-dev-printer'))
|
||||
fireEvent.click(screen.getByTestId('do-approve'))
|
||||
await waitFor(() => expect(mockApprove).toHaveBeenCalled())
|
||||
expect(mockApprove.mock.calls[0][1]).toMatchObject({ type: 'printer', label: 'Office printer' })
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user