From 6690d8a5dd2fd1c858c72fd4338ca9132f0279ee Mon Sep 17 00:00:00 2001 From: josh Date: Fri, 17 Apr 2026 10:43:02 -0400 Subject: [PATCH] feat(parts): couple state and location (host vs bin) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DEPLOYED parts live on a host; every other state lives in a bin (or unassigned). Previously binId and hostId were independent nullable fields with no validation, so the Edit Part dialog could leave a DEPLOYED part with only a bin and no host — which silently dropped it from the repair problem-part picker. - Service: resolveLocation() helper enforces the invariant on create and update. On a state transition, update auto-clears the stale relation and emits LOCATION_CHANGED for the cleared side. - Zod: CreatePartRequest.superRefine rejects mismatched state/location up front; UpdatePartRequest rejects both-fields-set. - Web: PartFormDialog swaps a single Location field between Host combobox (DEPLOYED) and Bin combobox (others); switching State clears the opposite field. Parts list + detail render host first, then bin path, then Unassigned. - Tests: 9 new cases covering the invariant including the no-op guard so an unrelated PATCH on a DEPLOYED part doesn't touch hostId/binId. Co-Authored-By: Claude Opus 4.7 --- apps/api/src/services/parts.test.ts | 240 ++++++++++++++++++ apps/api/src/services/parts.ts | 59 ++++- .../src/components/parts/PartFormDialog.tsx | 141 +++++++--- apps/web/src/pages/PartDetail.tsx | 6 +- apps/web/src/pages/Parts.tsx | 8 + packages/shared/src/parts.ts | 30 ++- 6 files changed, 432 insertions(+), 52 deletions(-) create mode 100644 apps/api/src/services/parts.test.ts diff --git a/apps/api/src/services/parts.test.ts b/apps/api/src/services/parts.test.ts new file mode 100644 index 0000000..fbb54be --- /dev/null +++ b/apps/api/src/services/parts.test.ts @@ -0,0 +1,240 @@ +import { describe, expect, it, vi } from 'vitest'; +import type { Tx, Actor } from './types.js'; +import { create, update } from './parts.js'; + +const actor: Actor = { id: 'user-1', username: 'tech', role: 'ADMIN' }; + +const partModel = { + id: 'pm-1', + manufacturerId: 'mfr-1', + mpn: 'WD-1000', + eolDate: null, + notes: null, +}; + +// Current-row fixtures used by update tests. Only the fields the service reads are populated. +function sparePart(overrides: Partial> = {}) { + return { + id: 'p-1', + serialNumber: 'SN-1', + partModelId: 'pm-1', + manufacturerId: 'mfr-1', + state: 'SPARE', + binId: 'bin-1', + hostId: null, + categoryId: null, + price: null, + notes: null, + partModel: { ...partModel }, + manufacturer: { id: 'mfr-1', name: 'WD' }, + bin: null, + host: null, + category: null, + tags: [], + ...overrides, + }; +} + +function deployedPart(overrides: Partial> = {}) { + return sparePart({ + state: 'DEPLOYED', + binId: null, + hostId: 'host-1', + host: { id: 'host-1', name: 'rack-1', assetId: 'ASSET-001' }, + ...overrides, + }); +} + +describe('parts.create — state/location coupling', () => { + it('rejects DEPLOYED without a hostId', async () => { + const partCreate = vi.fn(); + const tx = { + partModel: { findUnique: async () => partModel }, + part: { create: partCreate }, + partEvent: { create: vi.fn() }, + } as unknown as Tx; + + await expect( + create( + tx, + { serialNumber: 'SN-1', partModelId: 'pm-1', state: 'DEPLOYED' }, + actor, + ), + ).rejects.toMatchObject({ status: 400 }); + expect(partCreate).not.toHaveBeenCalled(); + }); + + it('rejects DEPLOYED with both hostId and binId', async () => { + const partCreate = vi.fn(); + const tx = { + partModel: { findUnique: async () => partModel }, + part: { create: partCreate }, + partEvent: { create: vi.fn() }, + } as unknown as Tx; + + await expect( + create( + tx, + { + serialNumber: 'SN-1', + partModelId: 'pm-1', + state: 'DEPLOYED', + hostId: 'host-1', + binId: 'bin-1', + }, + actor, + ), + ).rejects.toMatchObject({ status: 400 }); + expect(partCreate).not.toHaveBeenCalled(); + }); + + it('rejects a non-DEPLOYED part that carries a hostId', async () => { + const partCreate = vi.fn(); + const tx = { + partModel: { findUnique: async () => partModel }, + part: { create: partCreate }, + partEvent: { create: vi.fn() }, + } as unknown as Tx; + + await expect( + create( + tx, + { serialNumber: 'SN-1', partModelId: 'pm-1', state: 'SPARE', hostId: 'host-1' }, + actor, + ), + ).rejects.toMatchObject({ status: 400 }); + expect(partCreate).not.toHaveBeenCalled(); + }); + + it('creates a DEPLOYED part with hostId and writes binId=null', async () => { + const created = sparePart({ + id: 'p-new', + state: 'DEPLOYED', + binId: null, + hostId: 'host-1', + host: { id: 'host-1', name: 'rack-1', assetId: 'ASSET-001' }, + }); + const partCreate = vi.fn(); + partCreate.mockResolvedValue(created); + const partEventCreate = vi.fn(); + const tx = { + partModel: { findUnique: async () => partModel }, + part: { + create: partCreate, + findUnique: async () => created, + }, + partEvent: { create: partEventCreate }, + } as unknown as Tx; + + const r = await create( + tx, + { + serialNumber: 'SN-1', + partModelId: 'pm-1', + state: 'DEPLOYED', + hostId: 'host-1', + }, + actor, + ); + expect(r.id).toBe('p-new'); + const callArgs = partCreate.mock.calls[0]![0] as { data: { binId: string | null; hostId: string | null } }; + expect(callArgs.data.binId).toBeNull(); + expect(callArgs.data.hostId).toBe('host-1'); + }); +}); + +describe('parts.update — state/location coupling', () => { + it('promoting SPARE→DEPLOYED with a hostId clears binId', async () => { + const current = sparePart({ binId: 'bin-1', hostId: null }); + const partUpdate = vi.fn(); + partUpdate.mockResolvedValue(sparePart({ state: 'DEPLOYED', binId: null, hostId: 'host-1' })); + const tx = { + part: { + findUnique: async () => current, + update: partUpdate, + }, + partEvent: { createMany: vi.fn() }, + partTag: { findMany: async () => [] }, + } as unknown as Tx; + + await update(tx, 'p-1', { state: 'DEPLOYED', hostId: 'host-1' }, actor); + + const call = partUpdate.mock.calls[0]![0] as { data: { bin?: unknown; host?: unknown } }; + expect(call.data.bin).toEqual({ disconnect: true }); + expect(call.data.host).toEqual({ connect: { id: 'host-1' } }); + }); + + it('demoting DEPLOYED→BROKEN with a binId clears hostId', async () => { + const current = deployedPart(); + const partUpdate = vi.fn(); + partUpdate.mockResolvedValue(sparePart({ state: 'BROKEN', binId: 'bin-2', hostId: null })); + const tx = { + part: { + findUnique: async () => current, + update: partUpdate, + }, + partEvent: { createMany: vi.fn() }, + partTag: { findMany: async () => [] }, + } as unknown as Tx; + + await update(tx, 'p-1', { state: 'BROKEN', binId: 'bin-2' }, actor); + + const call = partUpdate.mock.calls[0]![0] as { data: { bin?: unknown; host?: unknown } }; + expect(call.data.bin).toEqual({ connect: { id: 'bin-2' } }); + expect(call.data.host).toEqual({ disconnect: true }); + }); + + it('rejects a DEPLOYED transition when neither current nor input supplies a hostId', async () => { + const current = sparePart({ binId: 'bin-1', hostId: null }); + const partUpdate = vi.fn(); + const tx = { + part: { + findUnique: async () => current, + update: partUpdate, + }, + partEvent: { createMany: vi.fn() }, + } as unknown as Tx; + + await expect( + update(tx, 'p-1', { state: 'DEPLOYED' }, actor), + ).rejects.toMatchObject({ status: 400 }); + expect(partUpdate).not.toHaveBeenCalled(); + }); + + it('rejects DEPLOYED→DEPLOYED with a binId (bin not allowed on DEPLOYED)', async () => { + const current = deployedPart(); + const partUpdate = vi.fn(); + const tx = { + part: { + findUnique: async () => current, + update: partUpdate, + }, + partEvent: { createMany: vi.fn() }, + } as unknown as Tx; + + await expect( + update(tx, 'p-1', { binId: 'bin-2' }, actor), + ).rejects.toMatchObject({ status: 400 }); + expect(partUpdate).not.toHaveBeenCalled(); + }); + + it('no-op update (notes only) on a DEPLOYED part does not touch bin or host', async () => { + const current = deployedPart(); + const partUpdate = vi.fn(); + partUpdate.mockResolvedValue(deployedPart({ notes: 'ok' })); + const tx = { + part: { + findUnique: async () => current, + update: partUpdate, + }, + partEvent: { createMany: vi.fn() }, + partTag: { findMany: async () => [] }, + } as unknown as Tx; + + await update(tx, 'p-1', { notes: 'ok' }, actor); + + const call = partUpdate.mock.calls[0]![0] as { data: { bin?: unknown; host?: unknown } }; + expect(call.data.bin).toBeUndefined(); + expect(call.data.host).toBeUndefined(); + }); +}); diff --git a/apps/api/src/services/parts.ts b/apps/api/src/services/parts.ts index 6a173db..7efbc29 100644 --- a/apps/api/src/services/parts.ts +++ b/apps/api/src/services/parts.ts @@ -3,6 +3,7 @@ import type { CreatePartRequest, PaginationQuery, PartListQuery, + PartState as PartStateValue, UpdatePartRequest, } from '@vector/shared'; import { errors } from '../lib/http-error.js'; @@ -10,6 +11,29 @@ import * as partModelsSvc from './part-models.js'; import * as tagsSvc from './tags.js'; import type { Actor, Tx } from './types.js'; +// DEPLOYED parts live on a host; every other state lives in a bin (or is unassigned). +// This helper enforces the invariant on create/update and auto-clears the stale field on a +// state transition, so callers don't have to remember to null the opposite relation. +function resolveLocation( + state: PartStateValue, + input: { binId?: string | null; hostId?: string | null }, + current: { binId: string | null; hostId: string | null } = { binId: null, hostId: null }, +): { binId: string | null; hostId: string | null } { + if (state === 'DEPLOYED') { + const hostId = input.hostId !== undefined ? input.hostId : current.hostId; + if (!hostId) throw errors.badRequest('A deployed part must be assigned to a host'); + if (input.binId) { + throw errors.badRequest('A deployed part cannot also be in a storage bin'); + } + return { binId: null, hostId }; + } + if (input.hostId) { + throw errors.badRequest('Only deployed parts can be assigned to a host'); + } + const binId = input.binId !== undefined ? input.binId : current.binId; + return { binId, hostId: null }; +} + const partInclude = { manufacturer: true, partModel: true, @@ -123,6 +147,9 @@ export async function create( throw errors.badRequest('manufacturerId does not match the selected part model'); } + const state = input.state ?? 'SPARE'; + const location = resolveLocation(state, { binId: input.binId, hostId: input.hostId }); + try { const p = await tx.part.create({ data: { @@ -130,9 +157,9 @@ export async function create( partModelId, manufacturerId, price: input.price ?? null, - state: input.state ?? 'SPARE', - binId: input.binId ?? null, - hostId: input.hostId ?? null, + state, + binId: location.binId, + hostId: location.hostId, categoryId: input.categoryId ?? null, notes: input.notes ?? null, }, @@ -179,12 +206,24 @@ export async function update( } if (input.price !== undefined) data.price = input.price; if (input.state !== undefined) data.state = input.state; - if (input.binId !== undefined) { - data.bin = input.binId ? { connect: { id: input.binId } } : { disconnect: true }; - } - if (input.hostId !== undefined) { - data.host = input.hostId ? { connect: { id: input.hostId } } : { disconnect: true }; + + let nextBinId: string | null = current.binId; + let nextHostId: string | null = current.hostId; + const locationTouched = + input.state !== undefined || input.binId !== undefined || input.hostId !== undefined; + if (locationTouched) { + const nextState = input.state ?? (current.state as PartStateValue); + const resolved = resolveLocation( + nextState, + { binId: input.binId, hostId: input.hostId }, + { binId: current.binId, hostId: current.hostId }, + ); + nextBinId = resolved.binId; + nextHostId = resolved.hostId; + data.bin = resolved.binId ? { connect: { id: resolved.binId } } : { disconnect: true }; + data.host = resolved.hostId ? { connect: { id: resolved.hostId } } : { disconnect: true }; } + if (input.categoryId !== undefined) { data.category = input.categoryId ? { connect: { id: input.categoryId } } @@ -216,7 +255,7 @@ export async function update( newValue: input.state, }); } - if (input.binId !== undefined && input.binId !== current.binId) { + if (nextBinId !== current.binId) { events.push({ partId: part.id, userId, @@ -226,7 +265,7 @@ export async function update( newValue: binPath(part.bin), }); } - if (input.hostId !== undefined && input.hostId !== current.hostId) { + if (nextHostId !== current.hostId) { events.push({ partId: part.id, userId, diff --git a/apps/web/src/components/parts/PartFormDialog.tsx b/apps/web/src/components/parts/PartFormDialog.tsx index de70fda..a7acd62 100644 --- a/apps/web/src/components/parts/PartFormDialog.tsx +++ b/apps/web/src/components/parts/PartFormDialog.tsx @@ -30,6 +30,7 @@ import { } from '@vector/ui'; import { listManufacturers } from '../../lib/api/manufacturers.js'; import { listBins } from '../../lib/api/bins.js'; +import { listHosts } from '../../lib/api/hosts.js'; import { createPart, updatePart } from '../../lib/api/parts.js'; import type { Part } from '../../lib/api/types.js'; import { ApiRequestError } from '../../lib/api/client.js'; @@ -38,15 +39,26 @@ import { partStateOptions } from './PartStateBadge.js'; // Schema reflects the server's CreatePartRequest but keeps strings for the form, letting the // submit handler coerce to the network shape. -const PartFormSchema = z.object({ - serialNumber: z.string().min(1, 'Required').max(128), - mpn: z.string().min(1, 'Required').max(128), - manufacturerId: z.string().uuid('Select a manufacturer'), - state: PartState, - binId: z.string().optional(), // '' = none - price: z.string().optional(), // empty string = null - notes: z.string().max(4096).optional(), -}); +const PartFormSchema = z + .object({ + serialNumber: z.string().min(1, 'Required').max(128), + mpn: z.string().min(1, 'Required').max(128), + manufacturerId: z.string().uuid('Select a manufacturer'), + state: PartState, + binId: z.string().optional(), // '' = none + hostId: z.string().optional(), // '' = none + price: z.string().optional(), // empty string = null + notes: z.string().max(4096).optional(), + }) + .superRefine((v, ctx) => { + if (v.state === 'DEPLOYED' && !v.hostId) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'A deployed part must be assigned to a host', + path: ['hostId'], + }); + } + }); type PartFormValues = z.infer; const UNASSIGNED = '__none__'; @@ -69,6 +81,7 @@ export function PartFormDialog({ open, onOpenChange, part }: PartFormDialogProps manufacturerId: '', state: 'SPARE', binId: '', + hostId: '', price: '', notes: '', }, @@ -84,6 +97,7 @@ export function PartFormDialog({ open, onOpenChange, part }: PartFormDialogProps manufacturerId: part.manufacturerId, state: part.state, binId: part.binId ?? '', + hostId: part.hostId ?? '', price: part.price != null ? String(part.price) : '', notes: part.notes ?? '', } @@ -93,12 +107,15 @@ export function PartFormDialog({ open, onOpenChange, part }: PartFormDialogProps manufacturerId: '', state: 'SPARE', binId: '', + hostId: '', price: '', notes: '', }, ); }, [open, part, form]); + const watchedState = form.watch('state'); + const manufacturers = useQuery({ queryKey: queryKeys.manufacturers.list({ pageSize: 100 }), queryFn: () => listManufacturers({ pageSize: 100 }), @@ -108,17 +125,25 @@ export function PartFormDialog({ open, onOpenChange, part }: PartFormDialogProps const bins = useQuery({ queryKey: queryKeys.bins.list({ pageSize: 100 }), queryFn: () => listBins({ pageSize: 100 }), - enabled: open, + enabled: open && watchedState !== 'DEPLOYED', + }); + + const hosts = useQuery({ + queryKey: queryKeys.hosts.list({ pageSize: 100 }), + queryFn: () => listHosts({ pageSize: 100 }), + enabled: open && watchedState === 'DEPLOYED', }); const mutation = useMutation({ mutationFn: async (values: PartFormValues) => { + const deployed = values.state === 'DEPLOYED'; const payload = { serialNumber: values.serialNumber, mpn: values.mpn, manufacturerId: values.manufacturerId, state: values.state, - binId: values.binId ? values.binId : null, + binId: deployed ? null : values.binId ? values.binId : null, + hostId: deployed ? (values.hostId ? values.hostId : null) : null, price: values.price === '' ? null : Number(values.price), notes: values.notes ? values.notes : null, }; @@ -227,7 +252,16 @@ export function PartFormDialog({ open, onOpenChange, part }: PartFormDialogProps render={({ field }) => ( State - { + field.onChange(v); + // State and location are coupled: DEPLOYED lives on a host, all other + // states live in a bin. Clear the now-invalid field on transition. + if (v === 'DEPLOYED') form.setValue('binId', ''); + else form.setValue('hostId', ''); + }} + > @@ -260,34 +294,61 @@ export function PartFormDialog({ open, onOpenChange, part }: PartFormDialogProps /> - ( - - Location - - - - )} - /> + {watchedState === 'DEPLOYED' ? ( + ( + + Location (host) + + + + )} + /> + ) : ( + ( + + Location (bin) + + + + )} + /> + )} + {part.host.assetId} / {part.host.name} + + ) : part.bin?.fullPath ? ( {part.bin.fullPath} ) : ( Unassigned diff --git a/apps/web/src/pages/Parts.tsx b/apps/web/src/pages/Parts.tsx index eb6580f..18270b9 100644 --- a/apps/web/src/pages/Parts.tsx +++ b/apps/web/src/pages/Parts.tsx @@ -116,6 +116,14 @@ export default function Parts() { id: 'location', header: 'Location', cell: ({ row }) => { + const host = row.original.host; + if (host) { + return ( + + {host.assetId} / {host.name} + + ); + } const path = row.original.bin?.fullPath; return path ? ( {path} diff --git a/packages/shared/src/parts.ts b/packages/shared/src/parts.ts index 87fbf3d..ed1ae47 100644 --- a/packages/shared/src/parts.ts +++ b/packages/shared/src/parts.ts @@ -42,6 +42,30 @@ export const CreatePartRequest = z path: ['partModelId'], }); } + // State/location coupling: DEPLOYED parts live on a host; every other state lives in a bin. + const state = v.state ?? 'SPARE'; + if (state === 'DEPLOYED') { + if (!v.hostId) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'A deployed part must be assigned to a host', + path: ['hostId'], + }); + } + if (v.binId) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'A deployed part cannot also be in a storage bin', + path: ['binId'], + }); + } + } else if (v.hostId) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'Only deployed parts can be assigned to a host', + path: ['hostId'], + }); + } }); export type CreatePartRequest = z.infer; @@ -57,7 +81,11 @@ export const UpdatePartRequest = z categoryId: z.string().uuid().nullable().optional(), tagIds: z.array(z.string().uuid()).max(32).optional(), }) - .refine((v) => Object.keys(v).length > 0, { message: 'At least one field required' }); + .refine((v) => Object.keys(v).length > 0, { message: 'At least one field required' }) + .refine((v) => !(v.binId && v.hostId), { + message: 'A part cannot be assigned to both a host and a bin', + path: ['binId'], + }); export type UpdatePartRequest = z.infer; export const PartListQuery = PaginationQuery.extend({