diff --git a/src/components/DataPanel.test.tsx b/src/components/DataPanel.test.tsx new file mode 100644 index 0000000..5994256 --- /dev/null +++ b/src/components/DataPanel.test.tsx @@ -0,0 +1,119 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { DataPanel } from './DataPanel'; +import type { ComponentTopic } from '@/lib/types'; + +vi.mock('@/lib/store', () => ({ + useAppStore: vi.fn((selector: (s: { isConnected: boolean }) => unknown) => selector({ isConnected: true })), +})); + +// Capture the latest `initialValue` so tests can assert what the form would +// receive when the user clicks "Copy to Publish". +const publishFormInitialValues: unknown[] = []; +vi.mock('@/components/TopicPublishForm', () => ({ + TopicPublishForm: ({ initialValue }: { initialValue: unknown }) => { + publishFormInitialValues.push(initialValue); + return
; + }, +})); + +vi.mock('@/components/JsonFormViewer', () => ({ + JsonFormViewer: () =>
, +})); + +beforeEach(() => { + publishFormInitialValues.length = 0; +}); + +function makeTopic(overrides: Partial = {}): ComponentTopic { + return { + topic: '/test/data', + timestamp: 0, + data: null, + status: 'data', + ...overrides, + }; +} + +describe('DataPanel canWrite', () => { + it('shows write form when access is write even if data is 0 and no type is present', () => { + // Regression for the falsy-scalar bug: a counter reading of exactly 0 + // with an explicit `access: 'write'` must not hide the write section. + render(); + expect(screen.getByText('Write Value')).toBeInTheDocument(); + expect(screen.getByTestId('topic-publish-form')).toBeInTheDocument(); + }); + + it('shows write form when access is readwrite even if data is false and no type is present', () => { + render(); + expect(screen.getByText('Write Value')).toBeInTheDocument(); + }); + + it('hides write form when access is read regardless of data or type', () => { + render( + + ); + expect(screen.queryByText('Write Value')).not.toBeInTheDocument(); + expect(screen.queryByText('Publish Message')).not.toBeInTheDocument(); + }); + + it('falls back to typed-topic heuristic when access is absent', () => { + render(); + expect(screen.getByText('Publish Message')).toBeInTheDocument(); + }); + + it('hides write form when access is absent and there is no type hint and no value', () => { + render(); + expect(screen.queryByText('Publish Message')).not.toBeInTheDocument(); + expect(screen.queryByText('Write Value')).not.toBeInTheDocument(); + }); +}); + +describe('DataPanel publishValue initialization', () => { + it('preserves a scalar zero data value as the initial publish value when no default is set', () => { + // `||` would have collapsed `0` to `{}`; `??` keeps the falsy scalar. + render(); + expect(publishFormInitialValues.at(-1)).toBe(0); + }); + + it('prefers type_info.default_value over data when both are present', () => { + render( + }, + })} + entityId="motor" + /> + ); + expect(publishFormInitialValues.at(-1)).toBe(42); + }); +}); + +describe('DataPanel Copy to Publish', () => { + it('copies a scalar zero value from the last received data into the publish form', async () => { + // With the previous truthy guard, clicking Copy did nothing for a + // valid reading of exactly 0 (or false / ''). Presence check fixes it. + render( + }, + })} + entityId="motor" + /> + ); + + expect(publishFormInitialValues.at(-1)).toBe(42); + + await userEvent.click(screen.getByRole('button', { name: /copy to publish/i })); + + expect(publishFormInitialValues.at(-1)).toBe(0); + }); +}); diff --git a/src/components/DataPanel.tsx b/src/components/DataPanel.tsx index 584eebc..5c553e4 100644 --- a/src/components/DataPanel.tsx +++ b/src/components/DataPanel.tsx @@ -197,14 +197,34 @@ export function DataPanel({ isRefreshing = false, onRefresh, }: DataPanelProps) { - const [publishValue, setPublishValue] = useState(topic.type_info?.default_value || topic.data || {}); + // Use nullish coalescing so legitimate falsy scalars (0, false, '') are + // preserved as the initial publish value instead of collapsing to `{}`. + const [publishValue, setPublishValue] = useState(topic.type_info?.default_value ?? topic.data ?? {}); const isConnected = useAppStore((state) => state.isConnected); const hasData = topic.status === 'data' && topic.data !== null && topic.data !== undefined; - const canPublish = isConnected && !!(topic.type || topic.type_info || topic.data); + // `access` is the explicit per-item write capability; when present it + // overrides the legacy "any typed topic is publishable" heuristic so a + // read-only data item never surfaces a write form. Falsy scalar values + // (0, false, '') count as present - checking truthiness of `topic.data` + // would incorrectly hide the write form for e.g. a counter reading 0. + const hasTypeHint = !!(topic.type || topic.type_info); + const hasValuePresent = topic.data !== null && topic.data !== undefined; + const canWrite = + isConnected && + (topic.access === 'write' || + topic.access === 'readwrite' || + (topic.access !== 'read' && (hasTypeHint || hasValuePresent))); + // Use "Write Value" when the gateway told us this is a writable scalar + // (access === 'write' / 'readwrite'); fall back to "Publish Message" for + // streaming topics where the operation really is a publish. + const writeSectionLabel = + topic.access === 'write' || topic.access === 'readwrite' ? 'Write Value' : 'Publish Message'; const handleCopyFromLast = () => { - if (topic.data) { + // Presence check, not truthiness, so a reported value of exactly 0 + // (or false / empty string) still copies into the publish form. + if (topic.data !== null && topic.data !== undefined) { setPublishValue(JSON.parse(JSON.stringify(topic.data))); } }; @@ -270,10 +290,10 @@ export function DataPanel({ )}
- {/* Publish Section */} - {canPublish && ( + {/* Write/Publish Section */} + {canWrite && (
- Publish Message + {writeSectionLabel} { // Mark topicsData as "not loaded yet for the current entity" so the // Data tab renders a skeleton instead of an empty-state flash while @@ -459,10 +465,12 @@ export function EntityDetailPanel({ onConnectClick, viewMode = 'entity', onEntit try { // Fetch resource counts and data in parallel const [counts, dataRes] = await Promise.all([ - prefetchResourceCounts(entityType, entityId), - fetchEntityData(entityType, entityId).catch(() => [] as ComponentTopic[]), + prefetchResourceCounts(entityType, entityId, controller.signal), + fetchEntityData(entityType, entityId, controller.signal).catch(() => [] as ComponentTopic[]), ]); + if (cancelled) return; + // Store the fetched data for the Data tab const fetchedData = Array.isArray(dataRes) ? dataRes : []; setTopicsData(fetchedData); @@ -470,6 +478,7 @@ export function EntityDetailPanel({ onConnectClick, viewMode = 'entity', onEntit // Use the already-fetched data length instead of a separate request setResourceCounts({ ...counts, data: fetchedData.length, logs: 0 }); } catch { + if (cancelled) return; // On unexpected failure fall back to "loaded empty" so the UI // doesn't get stuck showing the skeleton forever. setTopicsData([]); @@ -477,6 +486,10 @@ export function EntityDetailPanel({ onConnectClick, viewMode = 'entity', onEntit }; doFetchResourceCounts(); + return () => { + cancelled = true; + controller.abort(); + }; }, [selectedEntity, prefetchResourceCounts, fetchEntityData]); const handleCopyEntity = async () => { diff --git a/src/lib/store.ts b/src/lib/store.ts index cc8a462..54f8264 100644 --- a/src/lib/store.ts +++ b/src/lib/store.ts @@ -213,7 +213,8 @@ export interface AppState { getFunctionHosts: (functionId: string) => Promise; prefetchResourceCounts: ( entityType: SovdResourceEntityType, - entityId: string + entityId: string, + signal?: AbortSignal ) => Promise<{ data: number; operations: number; configurations: number; faults: number }>; } @@ -2132,7 +2133,11 @@ export const useAppStore = create()( return data ? unwrapItems(data) : []; }, - prefetchResourceCounts: async (entityType: SovdResourceEntityType, entityId: string) => { + prefetchResourceCounts: async ( + entityType: SovdResourceEntityType, + entityId: string, + signal?: AbortSignal + ) => { const { client } = get(); if (!client) return { data: 0, operations: 0, configurations: 0, faults: 0 }; @@ -2140,24 +2145,42 @@ export const useAppStore = create()( // The caller (EntityDetailPanel) already fetches entity data via fetchEntityData // and overrides counts.data with the result length. const [opsRes, configRes, faultsRes] = await Promise.all([ - getEntityOperations(client, entityType, entityId).catch(() => ({ + getEntityOperations(client, entityType, entityId, signal).catch(() => ({ data: undefined, error: undefined, })), - getEntityConfigurations(client, entityType, entityId).catch(() => ({ + getEntityConfigurations(client, entityType, entityId, signal).catch(() => ({ + data: undefined, + error: undefined, + })), + getEntityFaults(client, entityType, entityId, signal).catch(() => ({ data: undefined, error: undefined, })), - getEntityFaults(client, entityType, entityId).catch(() => ({ data: undefined, error: undefined })), ]); + // Isolate each transform call: a malformed payload from one + // resource (e.g. UDS DTC faults with non-canonical schema) + // must not crash the others or the caller's Promise.all. + const safeCount = (fn: () => T, fallback: T): T => { + try { + return fn(); + } catch { + return fallback; + } + }; return { data: 0, - operations: opsRes.data ? unwrapItems(opsRes.data).length : 0, + operations: opsRes.data ? safeCount(() => unwrapItems(opsRes.data).length, 0) : 0, configurations: configRes.data - ? transformConfigurationsResponse(configRes.data, entityId).parameters.length + ? safeCount( + () => transformConfigurationsResponse(configRes.data, entityId).parameters.length, + 0 + ) + : 0, + faults: faultsRes.data + ? safeCount(() => transformFaultsResponse(faultsRes.data).items.length, 0) : 0, - faults: faultsRes.data ? transformFaultsResponse(faultsRes.data).items.length : 0, }; }, diff --git a/src/lib/transforms.test.ts b/src/lib/transforms.test.ts index 1aa86d5..9df43e5 100644 --- a/src/lib/transforms.test.ts +++ b/src/lib/transforms.test.ts @@ -21,6 +21,7 @@ import { transformDataResponse, transformConfigurationsResponse, transformBulkDataDescriptor, + type RawFaultItem, } from './transforms'; // ============================================================================= @@ -270,6 +271,63 @@ describe('transformFault', () => { const result = transformFault(makeFaultInput({ status: 'UNKNOWN_STATUS' })); expect(result.status).toBe('active'); }); + + it('reads aggregatedStatus when status is an object', () => { + const result = transformFault(makeFaultInput({ status: { aggregatedStatus: 'active', extra: '1' } })); + expect(result.status).toBe('active'); + }); + + it('maps object aggregatedStatus "passive" to pending', () => { + const result = transformFault(makeFaultInput({ status: { aggregatedStatus: 'passive' } })); + expect(result.status).toBe('pending'); + }); + + it('does not throw when status is null', () => { + const result = transformFault(makeFaultInput({ status: null })); + expect(result.status).toBe('active'); + }); + + it('does not throw when status is missing', () => { + const result = transformFault(makeFaultInput({ status: undefined })); + expect(result.status).toBe('active'); + }); + }); + + describe('field aliases', () => { + it('accepts code as a fallback for fault_code', () => { + const result = transformFault({ + code: 'ALT_CODE', + description: 'x', + severity: 1, + severity_label: 'warn', + status: 'CONFIRMED', + first_occurred: 1700000000, + } as unknown as RawFaultItem); + expect(result.code).toBe('ALT_CODE'); + }); + + it('accepts fault_name as a fallback for description', () => { + const result = transformFault({ + fault_code: 'F1', + fault_name: 'Alternative description', + severity: 1, + severity_label: 'warn', + status: 'CONFIRMED', + first_occurred: 1700000000, + } as unknown as RawFaultItem); + expect(result.message).toBe('Alternative description'); + }); + + it('does not throw when severity is undefined', () => { + const result = transformFault({ + fault_code: 'F1', + description: 'x', + severity_label: 'warn', + status: 'CONFIRMED', + first_occurred: 1700000000, + } as unknown as RawFaultItem); + expect(result.severity).toBe('warning'); + }); }); it('includes occurrence metadata in parameters', () => { @@ -324,6 +382,38 @@ describe('transformFaultsResponse', () => { expect(result.items).toEqual([]); expect(result.count).toBe(0); }); + + it('returns empty items when items field is a non-array truthy value', () => { + // `{ items: {} }` and similar would throw on `.map` if we trusted + // truthiness alone; transformFaultsResponse must accept unknown input. + for (const bad of [{}, 'string', 42, true]) { + const result = transformFaultsResponse({ items: bad }); + expect(result.items).toEqual([]); + expect(result.count).toBe(0); + } + }); + + it('assigns distinct synthetic codes when fault_code and code are missing', () => { + // Collisions on a literal 'unknown' code would collapse store dedup + // (keyed by code+entity_id) and produce duplicate React keys in the + // faults lists. The synthetic fallback must keep each fault distinct. + const faultWithoutCode = { description: 'drift', severity: 2, reporting_sources: [] }; + const result = transformFaultsResponse({ + items: [faultWithoutCode, faultWithoutCode, faultWithoutCode], + }); + expect(result.items).toHaveLength(3); + expect(result.items.map((f) => f.code)).toEqual(['unknown-0', 'unknown-1', 'unknown-2']); + }); + + it('returns stable synthetic codes across repeated calls so store dedup can match', () => { + // A non-deterministic synthetic code would make the same code-less + // fault look "changed" on every poll, forcing re-render and resetting + // React keys and expand state each refresh. + const faultWithoutCode = { description: 'drift', severity: 2, reporting_sources: [] }; + const first = transformFaultsResponse({ items: [faultWithoutCode, faultWithoutCode] }); + const second = transformFaultsResponse({ items: [faultWithoutCode, faultWithoutCode] }); + expect(first.items.map((f) => f.code)).toEqual(second.items.map((f) => f.code)); + }); }); // ============================================================================= @@ -558,6 +648,30 @@ describe('transformDataResponse', () => { const result = transformDataResponse({ items: [raw] }); expect(result[0]?.uniqueKey).toBe('x:publish'); }); + + it('passes through access="read"', () => { + const raw = { id: 'x', name: 'x', 'x-medkit': { access: 'read' } }; + const result = transformDataResponse({ items: [raw] }); + expect(result[0]?.access).toBe('read'); + }); + + it('passes through access="readwrite"', () => { + const raw = { id: 'x', name: 'x', 'x-medkit': { access: 'readwrite' } }; + const result = transformDataResponse({ items: [raw] }); + expect(result[0]?.access).toBe('readwrite'); + }); + + it('lowercases access for case-insensitive matching', () => { + const raw = { id: 'x', name: 'x', 'x-medkit': { access: 'WRITE' } }; + const result = transformDataResponse({ items: [raw] }); + expect(result[0]?.access).toBe('write'); + }); + + it('drops unrecognised access values', () => { + const raw = { id: 'x', name: 'x', 'x-medkit': { access: 'execute' } }; + const result = transformDataResponse({ items: [raw] }); + expect(result[0]?.access).toBeUndefined(); + }); }); }); diff --git a/src/lib/transforms.ts b/src/lib/transforms.ts index d8ff805..103cb6d 100644 --- a/src/lib/transforms.ts +++ b/src/lib/transforms.ts @@ -59,14 +59,21 @@ export function unwrapItems(response: unknown): T[] { * Raw fault item shape returned by the gateway faults endpoints. */ export interface RawFaultItem { - fault_code: string; - description: string; - severity: number; - severity_label: string; - status: string; + /** Canonical SOVD identifier. `code` accepted as a fallback for backends + * that have not yet aligned on the SOVD field name. */ + fault_code?: string; + code?: string; + /** Human-readable description. `fault_name` accepted as a fallback. */ + description?: string; + fault_name?: string; + severity?: number; + severity_label?: string; + /** Canonical value is a string. An object form with `aggregatedStatus` + * is also accepted to keep the UI from crashing on payload drift. */ + status?: string | { aggregatedStatus?: string; [key: string]: unknown } | null; /** Accepted as unix seconds (number), ISO 8601 string, or missing/invalid; * `transformFault` normalises all of these to an ISO timestamp. */ - first_occurred: number | string | null | undefined; + first_occurred?: number | string | null; last_occurred?: number | string | null; occurrence_count?: number; reporting_sources?: string[]; @@ -87,25 +94,33 @@ export interface RawFaultItem { * - `first_occurred` (unix seconds) → `timestamp` (ISO 8601) * - `reporting_sources[0]` last path segment → `entity_id` */ -export function transformFault(apiFault: RawFaultItem): Fault { +export function transformFault(apiFault: RawFaultItem, syntheticIdSuffix?: string): Fault { // Map severity number/label to FaultSeverity. // Label check takes priority over numeric value; critical is checked first. let severity: FaultSeverity = 'info'; - const label = apiFault.severity_label?.toLowerCase() || ''; - if (label === 'critical' || apiFault.severity >= 3) { + const severityNum = typeof apiFault.severity === 'number' ? apiFault.severity : 0; + const label = typeof apiFault.severity_label === 'string' ? apiFault.severity_label.toLowerCase() : ''; + if (label === 'critical' || severityNum >= 3) { severity = 'critical'; - } else if (label === 'error' || apiFault.severity === 2) { + } else if (label === 'error' || severityNum === 2) { severity = 'error'; - } else if (label === 'warn' || label === 'warning' || apiFault.severity === 1) { + } else if (label === 'warn' || label === 'warning' || severityNum === 1) { severity = 'warning'; } - // Map API status string to FaultStatusValue. + // Map API status to FaultStatusValue. Accept either a string (canonical SOVD) + // or an object with an `aggregatedStatus` field (UDS DTC-style backends). + let apiStatus = ''; + if (typeof apiFault.status === 'string') { + apiStatus = apiFault.status.toLowerCase(); + } else if (apiFault.status && typeof apiFault.status === 'object') { + const agg = (apiFault.status as { aggregatedStatus?: unknown }).aggregatedStatus; + if (typeof agg === 'string') apiStatus = agg.toLowerCase(); + } let status: FaultStatusValue = 'active'; - const apiStatus = apiFault.status?.toLowerCase() || ''; if (apiStatus === 'confirmed' || apiStatus === 'active') { status = 'active'; - } else if (apiStatus === 'pending' || apiStatus === 'prefailed') { + } else if (apiStatus === 'pending' || apiStatus === 'prefailed' || apiStatus === 'passive') { status = 'pending'; } else if (apiStatus === 'cleared' || apiStatus === 'resolved') { status = 'cleared'; @@ -124,9 +139,21 @@ export function transformFault(apiFault: RawFaultItem): Fault { // faults are always reported by ROS 2 nodes which map to apps. const entity_type = apiFault.entity_type || 'app'; + // When both canonical `fault_code` and the `code` alias are missing, a bare + // literal 'unknown' collides with every other code-less fault in the same + // response: that collapses the store's dedup (keyed by code + entity_id), + // duplicates React keys in the faults lists, and ties the expand-state + // Sets together so clicking one row toggles them all. The optional + // `syntheticIdSuffix` (typically the array index supplied by + // `transformFaultsResponse`) makes each code-less fault distinct within a + // response while staying stable across polls, so the store can still dedup + // on the next refresh instead of churning on every fetch. + const syntheticCode = syntheticIdSuffix !== undefined ? `unknown-${syntheticIdSuffix}` : 'unknown'; + const code = apiFault.fault_code ?? apiFault.code ?? syntheticCode; + return { - code: apiFault.fault_code, - message: apiFault.description, + code, + message: apiFault.description ?? apiFault.fault_name ?? '', severity, status, timestamp: (() => { @@ -177,7 +204,11 @@ interface RawFaultsResponse { export function transformFaultsResponse(rawData: unknown): ListFaultsResponse { if (!rawData || typeof rawData !== 'object') return { items: [], count: 0 }; const data = rawData as RawFaultsResponse; - const items = (data.items || []).map((f) => transformFault(f as RawFaultItem)); + // Guard against non-array `items` (e.g. `{}` or a string) - the `|| []` + // fallback only catches nullish values, so any other truthy non-array + // would crash `.map`. + const rawItems = Array.isArray(data.items) ? data.items : []; + const items = rawItems.map((f, idx) => transformFault(f as RawFaultItem, String(idx))); return { items, count: data['x-medkit']?.count ?? items.length }; } @@ -328,6 +359,9 @@ export function transformDataResponse(rawData: unknown): ComponentTopic[] { // `direction` above. const typeLabel = xm?.ros2?.type ?? xm?.type; const hasValue = item.value !== undefined; + const rawAccess = typeof xm?.access === 'string' ? xm.access.toLowerCase() : undefined; + const access: 'read' | 'write' | 'readwrite' | undefined = + rawAccess === 'read' || rawAccess === 'write' || rawAccess === 'readwrite' ? rawAccess : undefined; return { topic: topicName, timestamp: Date.now(), @@ -344,6 +378,7 @@ export function transformDataResponse(rawData: unknown): ComponentTopic[] { isPublisher: direction === 'publish' || direction === 'both' || direction === 'output', isSubscriber: direction === 'subscribe' || direction === 'both' || direction === 'input', uniqueKey: direction ? `${topicName}:${direction}` : topicName, + access, }; }); } diff --git a/src/lib/types.ts b/src/lib/types.ts index ab28f78..b874fba 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -214,6 +214,10 @@ export interface ComponentTopic { isSubscriber?: boolean; /** Unique key combining topic and direction for React key */ uniqueKey?: string; + /** Access mode from `x-medkit.access`. `'read'` hides the write section, + * `'write'` / `'readwrite'` enable it. Absent means "no constraint" (the + * legacy ROS 2 behaviour where any topic with a known type may publish). */ + access?: 'read' | 'write' | 'readwrite'; } /**