Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions src/components/DataPanel.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// Copyright 2026 bburda
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
Comment thread
bburda marked this conversation as resolved.
Outdated

import { describe, it, expect, vi } from 'vitest';
import { render, screen } from '@testing-library/react';
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 })),
}));

vi.mock('@/components/TopicPublishForm', () => ({
TopicPublishForm: () => <div data-testid="topic-publish-form" />,
}));

vi.mock('@/components/JsonFormViewer', () => ({
JsonFormViewer: () => <div data-testid="json-form-viewer" />,
}));

function makeTopic(overrides: Partial<ComponentTopic> = {}): 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(<DataPanel topic={makeTopic({ access: 'write', data: 0 })} entityId="motor" />);
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(<DataPanel topic={makeTopic({ access: 'readwrite', data: false })} entityId="motor" />);
expect(screen.getByText('Write Value')).toBeInTheDocument();
});

it('hides write form when access is read regardless of data or type', () => {
render(
<DataPanel
topic={makeTopic({ access: 'read', type: 'sensor_msgs/msg/Temperature', data: { value: 23 } })}
entityId="motor"
/>
);
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(<DataPanel topic={makeTopic({ type: 'std_msgs/msg/String', data: null })} entityId="motor" />);
expect(screen.getByText('Publish Message')).toBeInTheDocument();
});

it('hides write form when access is absent and there is no type hint and no value', () => {
render(<DataPanel topic={makeTopic({ data: null })} entityId="motor" />);
expect(screen.queryByText('Publish Message')).not.toBeInTheDocument();
expect(screen.queryByText('Write Value')).not.toBeInTheDocument();
});
});
24 changes: 20 additions & 4 deletions src/components/DataPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,23 @@ export function DataPanel({

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;
Comment thread
bburda marked this conversation as resolved.
Outdated
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) {
Expand Down Expand Up @@ -270,10 +286,10 @@ export function DataPanel({
)}
</div>

{/* Publish Section */}
{canPublish && (
{/* Write/Publish Section */}
{canWrite && (
<div className="border-t pt-4 space-y-2">
<span className="text-sm font-medium">Publish Message</span>
<span className="text-sm font-medium">{writeSectionLabel}</span>
<TopicPublishForm
topic={topic}
entityId={entityId}
Expand Down
17 changes: 15 additions & 2 deletions src/components/EntityDetailPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,12 @@ export function EntityDetailPanel({ onConnectClick, viewMode = 'entity', onEntit
faults: 0,
logs: 0,
};
// Guard against late results from a previous entity overwriting the
// current entity's state. The cleanup aborts in-flight requests AND
// flips `cancelled` so any setState calls after the entity changed
// become no-ops (covers transforms that ignore the abort signal).
const controller = new AbortController();
let cancelled = false;
const doFetchResourceCounts = async () => {
// Mark topicsData as "not loaded yet for the current entity" so the
Comment thread
bburda marked this conversation as resolved.
// Data tab renders a skeleton instead of an empty-state flash while
Expand Down Expand Up @@ -459,24 +465,31 @@ 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);

// 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([]);
}
};

doFetchResourceCounts();
return () => {
cancelled = true;
controller.abort();
};
}, [selectedEntity, prefetchResourceCounts, fetchEntityData]);

const handleCopyEntity = async () => {
Expand Down
39 changes: 31 additions & 8 deletions src/lib/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,8 @@ export interface AppState {
getFunctionHosts: (functionId: string) => Promise<unknown[]>;
prefetchResourceCounts: (
entityType: SovdResourceEntityType,
entityId: string
entityId: string,
signal?: AbortSignal
) => Promise<{ data: number; operations: number; configurations: number; faults: number }>;
}

Expand Down Expand Up @@ -2132,32 +2133,54 @@ export const useAppStore = create<AppState>()(
return data ? unwrapItems<unknown>(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 };

// Note: data count is NOT fetched here to avoid a duplicate request.
// 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 = <T>(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,
};
},

Expand Down
98 changes: 98 additions & 0 deletions src/lib/transforms.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
transformDataResponse,
transformConfigurationsResponse,
transformBulkDataDescriptor,
type RawFaultItem,
} from './transforms';

// =============================================================================
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -324,6 +382,22 @@ describe('transformFaultsResponse', () => {
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);
const codes = result.items.map((f) => f.code);
expect(new Set(codes).size).toBe(3);
for (const code of codes) {
expect(code).toMatch(/^unknown-/);
}
});
});

// =============================================================================
Expand Down Expand Up @@ -558,6 +632,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();
});
});
});

Expand Down
Loading
Loading