diff --git a/apps/desktop/src/routes/editor/Editor.tsx b/apps/desktop/src/routes/editor/Editor.tsx index f53a293e62..a863721047 100644 --- a/apps/desktop/src/routes/editor/Editor.tsx +++ b/apps/desktop/src/routes/editor/Editor.tsx @@ -54,6 +54,7 @@ import { import { EditorErrorScreen } from "./EditorErrorScreen"; import { Header } from "./Header"; import { ImportProgress } from "./ImportProgress"; +import { deriveImportStatus, deriveRawImportStatus } from "./import-status"; import { PlayerContent } from "./Player"; import { Timeline } from "./Timeline"; import { Dialog, DialogContent, EditorButton, Input, Subfield } from "./ui"; @@ -148,20 +149,12 @@ export function Editor() { refetchOnReconnect: false, })); - const rawImportStatus = createMemo(() => { - const meta = rawMetaQuery.data; - if (!meta) return "loading" as const; - if ( - "status" in meta && - meta.status && - typeof meta.status === "object" && - "status" in meta.status && - meta.status.status === "InProgress" - ) { - return "importing" as const; - } - return "ready" as const; - }); + const rawImportStatus = createMemo(() => + deriveRawImportStatus({ + data: rawMetaQuery.data, + isError: rawMetaQuery.isError, + }), + ); const [lockedToImporting, setLockedToImporting] = createSignal(false); @@ -171,10 +164,8 @@ export function Editor() { } }); - const importStatus = () => { - if (lockedToImporting()) return "importing" as const; - return rawImportStatus(); - }; + const importStatus = () => + deriveImportStatus(rawImportStatus(), lockedToImporting()); const [importAborted, setImportAborted] = createSignal(false); @@ -211,6 +202,12 @@ export function Editor() { } > + + + diff --git a/apps/desktop/src/routes/editor/import-status.test.ts b/apps/desktop/src/routes/editor/import-status.test.ts new file mode 100644 index 0000000000..82599f509f --- /dev/null +++ b/apps/desktop/src/routes/editor/import-status.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; + +import { deriveImportStatus, deriveRawImportStatus } from "./import-status"; + +describe("editor import status", () => { + it("reports loading while the metadata query is still in flight", () => { + expect(deriveRawImportStatus({ data: undefined, isError: false })).toBe( + "loading", + ); + }); + + it("reports ready once metadata arrives", () => { + expect( + deriveRawImportStatus({ data: { status: null }, isError: false }), + ).toBe("ready"); + }); + + it("reports importing while an import is in progress", () => { + expect( + deriveRawImportStatus({ + data: { status: { status: "InProgress" } }, + isError: false, + }), + ).toBe("importing"); + }); + + // #1812: a failed query also has no data. Before the fix this returned + // "loading", so an unreadable recording-meta.json left the editor on the + // skeleton with no error and no way forward. + it("reports error when the metadata query fails", () => { + expect(deriveRawImportStatus({ data: undefined, isError: true })).toBe( + "error", + ); + }); + + it("still reports error when a stale success payload is present", () => { + expect( + deriveRawImportStatus({ data: { status: null }, isError: true }), + ).toBe("error"); + }); + + it("keeps showing the import screen while the lock is held", () => { + expect(deriveImportStatus("loading", true)).toBe("importing"); + }); + + it("lets an error override the importing lock", () => { + expect(deriveImportStatus("error", true)).toBe("error"); + }); + + it("passes through the raw status when nothing is latched", () => { + expect(deriveImportStatus("ready", false)).toBe("ready"); + expect(deriveImportStatus("loading", false)).toBe("loading"); + }); +}); diff --git a/apps/desktop/src/routes/editor/import-status.ts b/apps/desktop/src/routes/editor/import-status.ts new file mode 100644 index 0000000000..272d44bec0 --- /dev/null +++ b/apps/desktop/src/routes/editor/import-status.ts @@ -0,0 +1,48 @@ +/** + * Which screen the editor should show while a recording's metadata loads. + * + * Extracted from `Editor.tsx` so the state machine can be tested without a + * running editor. The case that matters is `error`: `getRecordingMetaByPath` + * returns `Result` and fails whenever + * `recording-meta.json` is missing or unparseable. A failed query has no + * `data`, so treating "no data" as "still loading" leaves the editor on the + * loading skeleton indefinitely (#1812). + */ +export type EditorImportStatus = "loading" | "importing" | "ready" | "error"; + +export function deriveRawImportStatus(query: { + data: unknown; + isError: boolean; +}): EditorImportStatus { + if (query.isError) return "error"; + if (!query.data) return "loading"; + + const meta = query.data as { status?: unknown }; + if ( + "status" in meta && + meta.status && + typeof meta.status === "object" && + "status" in meta.status && + (meta.status as { status?: unknown }).status === "InProgress" + ) { + return "importing"; + } + + return "ready"; +} + +/** + * `lockedToImporting` latches once an import starts, so the UI doesn't flicker + * between screens as the metadata is re-read. An error has to outrank that + * latch: if the metadata stops being readable mid-import there is nothing left + * to wait for, and without this the editor would stay on the import screen for + * the same reason it used to stay on the skeleton. + */ +export function deriveImportStatus( + rawStatus: EditorImportStatus, + lockedToImporting: boolean, +): EditorImportStatus { + if (rawStatus === "error") return "error"; + if (lockedToImporting) return "importing"; + return rawStatus; +}