From c526d15ede286216ffcc52a197a4abad82c7d47d Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 14 Apr 2026 17:08:59 +0200 Subject: [PATCH 1/2] fix: distinguish loading from empty in Data tab to avoid empty-state flash Before: the Data tab panel read `topicsData` as `ComponentTopic[]` seeded with `[]`. When the user switched entities the parent cleared the array and then started a fetch; the child could render a full "no data" panel for the new entity before the fetch resolved, regardless of what the new entity actually had. Now: `topicsData` is `ComponentTopic[] | null`. The parent resets it to `null` at the start of every fetch (including the unsupported-type branches and on error), and sets it to an array once the response is in. The Data tab renders a skeleton for `null`, the normal list for a non-empty array, and the empty/fallback state for `[]`. This removes the need for a second in-flight request from the child component and gives the fetch effect a single source of truth. --- src/components/EntityDetailPanel.tsx | 57 +++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/src/components/EntityDetailPanel.tsx b/src/components/EntityDetailPanel.tsx index 6279c73..f598952 100644 --- a/src/components/EntityDetailPanel.tsx +++ b/src/components/EntityDetailPanel.tsx @@ -94,7 +94,7 @@ interface ComponentTabContentProps { hasTopicsInfo: boolean; selectEntity: (path: string) => void; entityType: SovdResourceEntityType; - topicsData: ComponentTopic[]; + topicsData: ComponentTopic[] | null; } function ComponentTabContent({ @@ -122,14 +122,22 @@ function ComponentTabContent({ } /** - * Data tab content - shows data items + * Data tab content - shows data items. + * + * `topicsData` uses a three-state convention: + * - `null` -> parent fetch is still in flight, render a skeleton + * - `[]` -> fetch completed with no items, fall through to topicsInfo / empty state + * - `[...]` -> render the list + * + * This lets the tab distinguish "loading" from "loaded empty" without a + * second self-fetch or a duplicate network request. */ interface DataTabContentProps { selectedPath: string; selectedEntity: NonNullable; hasTopicsInfo: boolean; selectEntity: (path: string) => void; - topicsData: ComponentTopic[]; + topicsData: ComponentTopic[] | null; } function DataTabContent({ @@ -139,9 +147,30 @@ function DataTabContent({ selectEntity, topicsData, }: DataTabContentProps) { - // Use topicsData from props (fetched via API), or fall back to selectedEntity.topics - const topics = topicsData.length > 0 ? topicsData : (selectedEntity.topics as ComponentTopic[] | undefined); - const hasTopics = topics && topics.length > 0; + if (topicsData === null) { + return ( + + +
+ + Data +
+ + +
+ {[0, 1, 2, 3].map((i) => ( +
+ ))} +
+ + + ); + } + + // topicsData is loaded at this point. Prefer it over the entity-level fallback + // so we never surface stale `selectedEntity.topics` when the fresh fetch is empty. + const topics = topicsData.length > 0 ? topicsData : (selectedEntity.topics as ComponentTopic[] | undefined) || []; + const hasTopics = topics.length > 0; if (hasTopics) { return ( @@ -344,8 +373,11 @@ export function EntityDetailPanel({ onConnectClick, viewMode = 'entity', onEntit faults: 0, logs: 0, }); - // Store fetched topics data for the Data tab - const [topicsData, setTopicsData] = useState([]); + // Store fetched topics data for the Data tab. `null` means "not yet loaded + // for the current entity" so the Data tab can render a skeleton instead of + // an empty-state flash while the fetch is in flight. `[]` means "loaded, + // no items". `[...]` means "loaded with items". + const [topicsData, setTopicsData] = useState(null); const { selectedPath, @@ -394,6 +426,11 @@ export function EntityDetailPanel({ onConnectClick, viewMode = 'entity', onEntit logs: 0, }; const doFetchResourceCounts = async () => { + // Mark topicsData as "not loaded yet for the current entity" so the + // Data tab renders a skeleton instead of an empty-state flash while + // the fetch is in flight. Any previous entity's data is discarded. + setTopicsData(null); + if (!selectedEntity) { setResourceCounts(emptyCounts); setTopicsData([]); @@ -433,7 +470,9 @@ 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 { - // Silently handle errors - counts will stay at 0 + // On unexpected failure fall back to "loaded empty" so the UI + // doesn't get stuck showing the skeleton forever. + setTopicsData([]); } }; From af06e6b730d8e772eb1de406624960dc596bde86 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 14 Apr 2026 17:12:07 +0200 Subject: [PATCH 2/2] fix: defensive timestamp parsing in transformFault Accept unix-seconds numbers, ISO 8601 strings, and fall back to the current time for zero, negative, missing, or otherwise invalid values instead of letting `new Date()` produce "Invalid Date" in the UI. Widen `RawFaultItem.first_occurred` to `number | string | null | undefined` so the type reflects the shapes the transform actually handles and the fallback branches are not unreachable at the type level. --- src/lib/transforms.test.ts | 54 +++++++++++++++++++++++++++++++++++++- src/lib/transforms.ts | 27 ++++++++++++++++--- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/lib/transforms.test.ts b/src/lib/transforms.test.ts index 3d9de3b..fbd2041 100644 --- a/src/lib/transforms.test.ts +++ b/src/lib/transforms.test.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { unwrapItems, transformFault, @@ -89,6 +89,58 @@ describe('transformFault', () => { expect(result.timestamp).toBe(new Date(1700000000 * 1000).toISOString()); }); + describe('timestamp defensive parsing', () => { + // All fallback branches must return an ISO string close to "now". + // Asserting recency (not just "doesn't throw") actually verifies the + // fallback fired and produced a sane value. + const expectRecent = (iso: string) => { + const ts = new Date(iso).getTime(); + const now = Date.now(); + expect(ts).toBeGreaterThan(now - 5000); + expect(ts).toBeLessThanOrEqual(now); + }; + + // Suppress the dev-tools breadcrumb console.warn in fallback cases. + let warnSpy: ReturnType; + beforeEach(() => { + warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => { + warnSpy.mockRestore(); + }); + + it('falls back to current time when first_occurred is 0', () => { + const result = transformFault(makeFaultInput({ first_occurred: 0 })); + expectRecent(result.timestamp); + expect(warnSpy).toHaveBeenCalledTimes(1); + }); + + it('falls back to current time when first_occurred is negative', () => { + const result = transformFault(makeFaultInput({ first_occurred: -1 })); + expectRecent(result.timestamp); + expect(warnSpy).toHaveBeenCalledTimes(1); + }); + + it('parses ISO 8601 string first_occurred', () => { + const iso = '2026-04-13T10:00:00.000Z'; + const result = transformFault(makeFaultInput({ first_occurred: iso })); + expect(result.timestamp).toBe(iso); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it('falls back to current time when first_occurred is an invalid string', () => { + const result = transformFault(makeFaultInput({ first_occurred: 'not-a-date' })); + expectRecent(result.timestamp); + expect(warnSpy).toHaveBeenCalledTimes(1); + }); + + it('falls back to current time when first_occurred is missing', () => { + const result = transformFault(makeFaultInput({ first_occurred: undefined })); + expectRecent(result.timestamp); + expect(warnSpy).toHaveBeenCalledTimes(1); + }); + }); + it('defaults entity_type to "app" when not in raw data', () => { const result = transformFault(makeFaultInput()); expect(result.entity_type).toBe('app'); diff --git a/src/lib/transforms.ts b/src/lib/transforms.ts index 152ee86..5422228 100644 --- a/src/lib/transforms.ts +++ b/src/lib/transforms.ts @@ -64,8 +64,10 @@ export interface RawFaultItem { severity: number; severity_label: string; status: string; - first_occurred: number; - last_occurred?: number; + /** 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; + last_occurred?: number | string | null; occurrence_count?: number; reporting_sources?: string[]; /** Entity type if provided by the gateway (not currently included in @@ -127,7 +129,26 @@ export function transformFault(apiFault: RawFaultItem): Fault { message: apiFault.description, severity, status, - timestamp: new Date(apiFault.first_occurred * 1000).toISOString(), + timestamp: (() => { + try { + if (typeof apiFault.first_occurred === 'number' && apiFault.first_occurred > 0) { + return new Date(apiFault.first_occurred * 1000).toISOString(); + } + if (typeof apiFault.first_occurred === 'string') { + const parsed = new Date(apiFault.first_occurred); + if (!Number.isNaN(parsed.getTime())) { + return parsed.toISOString(); + } + } + } catch { + // fall through to the warn + fallback below + } + // Log a breadcrumb so operators correlating with syslog can tell + // the timestamp was fabricated by the UI and not reported by the + // gateway. Dev tools only; fallback keeps rendering alive. + console.warn('[transformFault] invalid first_occurred, falling back to now:', apiFault.first_occurred); + return new Date().toISOString(); + })(), entity_id, entity_type, parameters: {