From 637cbfefec7bdab7cc305cf8dcd19bb0cde2a4ea Mon Sep 17 00:00:00 2001 From: Gabriel Costa Date: Mon, 2 Nov 2026 15:30:13 +0000 Subject: [PATCH 1/3] Add useImportConfig hook Signed-off-by: Gabriel Costa --- src/hooks/useImportConfig.test.ts | 275 +++++++++++++++++++++++++++++ src/hooks/useImportConfig.ts | 186 +++++++++++++++++++ src/i18n/locales/en-US/common.json | 11 +- src/i18n/locales/es-ES/common.json | 11 +- src/i18n/locales/pt-BR/common.json | 11 +- 5 files changed, 491 insertions(+), 3 deletions(-) create mode 100644 src/hooks/useImportConfig.test.ts create mode 100644 src/hooks/useImportConfig.ts diff --git a/src/hooks/useImportConfig.test.ts b/src/hooks/useImportConfig.test.ts new file mode 100644 index 00000000..b8d6d8d1 --- /dev/null +++ b/src/hooks/useImportConfig.test.ts @@ -0,0 +1,275 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { act, renderHook as rtlRenderHook } from "@testing-library/react"; +import { createElement, type ReactNode } from "react"; +import { IntlProvider } from "react-intl"; +import { toast } from "sonner"; + +import { ApiError } from "@/api/client"; +import { + importExportApi, + type ConfigExport, + type ImportPreview, + type ImportResult, +} from "@/api/importExport"; +import { useAuth } from "@/auth/useAuth"; +import enMessages from "@/i18n/locales/en-US"; + +import { useImportConfig } from "./useImportConfig"; + +vi.mock("@/auth/useAuth", () => ({ useAuth: vi.fn() })); +vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } })); +// Keep parseImportFile/ImportFileError real; stub only the network calls. +vi.mock("@/api/importExport", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, importExportApi: { importConfig: vi.fn(), previewImport: vi.fn() } }; +}); + +const mockUseAuth = vi.mocked(useAuth); +const mockImportConfig = vi.mocked(importExportApi.importConfig); +const mockPreviewImport = vi.mocked(importExportApi.previewImport); + +const EXPORT: ConfigExport = { + version: "2025-06-18", + exported_at: "2026-11-02T10:00:00Z", + exported_by: "admin@example.com", + entities: { tools: [{ name: "t1" }] }, +}; + +const RESULT: ImportResult = { + import_id: "imp-1", + status: "completed", + progress: { total: 4, processed: 4, created: 1, updated: 2, skipped: 1, failed: 0 }, + errors: [], + warnings: [], + started_at: "2026-11-02T10:00:00Z", + completed_at: "2026-11-02T10:00:01Z", +}; + +const PREVIEW: ImportPreview = { + summary: { total_items: 1, by_type: { tools: 1 } }, + items: {}, + bundles: {}, + dependencies: {}, + conflicts: {}, +}; + +// jsdom's File has no .text(); parseImportFile only needs that. +const fileOf = (content: string) => ({ text: async () => content }) as unknown as File; +const exportFile = () => fileOf(JSON.stringify(EXPORT)); + +const wrapper = ({ children }: { children: ReactNode }) => + createElement( + IntlProvider, + { locale: "en", defaultLocale: "en", messages: enMessages }, + children, + ); + +const renderHook = () => rtlRenderHook(() => useImportConfig(), { wrapper }); + +function grant(...perms: string[]) { + mockUseAuth.mockReturnValue({ + hasPermission: (perm: string) => perms.includes(perm), + } as unknown as ReturnType); +} + +async function loaded() { + const hook = renderHook(); + await act(() => hook.result.current.loadFile(exportFile())); + return hook; +} + +describe("useImportConfig", () => { + beforeEach(() => { + vi.clearAllMocks(); + grant("admin.import", "admin.system_config"); + }); + + it("derives canImport and canPreview from separate permissions", () => { + expect(renderHook().result.current).toMatchObject({ canImport: true, canPreview: true }); + + grant("admin.import"); + expect(renderHook().result.current).toMatchObject({ canImport: true, canPreview: false }); + }); + + describe("loadFile", () => { + it("holds the parsed export", async () => { + const { result } = renderHook(); + + let ok!: boolean; + await act(async () => { + ok = await result.current.loadFile(exportFile()); + }); + + expect(ok).toBe(true); + expect(result.current.data).toEqual(EXPORT); + expect(result.current.error).toBeNull(); + }); + + it.each([ + ["{not json", "The selected file is not valid JSON"], + [JSON.stringify({ version: "x" }), "The selected file is not a ContextForge export"], + ])("rejects %s with a localized error and no request", async (content, message) => { + const { result } = renderHook(); + + let ok!: boolean; + await act(async () => { + ok = await result.current.loadFile(fileOf(content)); + }); + + expect(ok).toBe(false); + expect(result.current.data).toBeNull(); + expect(result.current.error).toBe(message); + expect(mockImportConfig).not.toHaveBeenCalled(); + }); + + it("maps an unreadable file to the generic failure", async () => { + const unreadable = { text: () => Promise.reject(new Error("NotReadableError")) }; + const { result } = renderHook(); + + await act(() => result.current.loadFile(unreadable as unknown as File)); + + expect(result.current.error).toBe("Import failed"); + }); + + it("clears earlier results when a new file is loaded", async () => { + mockImportConfig.mockResolvedValueOnce(RESULT); + const { result } = await loaded(); + await act(() => result.current.runImport()); + expect(result.current.result).not.toBeNull(); + + await act(() => result.current.loadFile(exportFile())); + + expect(result.current.result).toBeNull(); + }); + }); + + it("does nothing before a file is loaded", async () => { + const { result } = renderHook(); + + await act(() => result.current.dryRun()); + await act(() => result.current.runImport()); + await act(() => result.current.preview()); + + expect(mockImportConfig).not.toHaveBeenCalled(); + expect(mockPreviewImport).not.toHaveBeenCalled(); + }); + + it("dryRun sends dryRun: true and stores the result separately", async () => { + mockImportConfig.mockResolvedValueOnce(RESULT); + const { result } = await loaded(); + + await act(() => result.current.dryRun({ conflictStrategy: "skip" })); + + expect(mockImportConfig).toHaveBeenCalledWith( + { conflictStrategy: "skip", data: EXPORT, dryRun: true }, + expect.any(AbortSignal), + ); + expect(result.current.dryRunResult).toEqual(RESULT); + expect(result.current.result).toBeNull(); + expect(toast.success).not.toHaveBeenCalled(); + }); + + it("runImport sends dryRun: false and toasts the counts", async () => { + mockImportConfig.mockResolvedValueOnce(RESULT); + const { result } = await loaded(); + + await act(() => result.current.runImport({ selectedEntities: { tools: ["t1"] } })); + + expect(mockImportConfig).toHaveBeenCalledWith( + { selectedEntities: { tools: ["t1"] }, data: EXPORT, dryRun: false }, + expect.any(AbortSignal), + ); + expect(result.current.result).toEqual(RESULT); + expect(toast.success).toHaveBeenCalledWith( + "Import complete: 1 created, 2 updated, 1 skipped, 0 failed", + ); + }); + + it.each([ + [403, "You don't have permission to import this configuration"], + [409, "Import stopped: some items already exist"], + [413, "The file is too large to import"], + [422, "The file failed import validation"], + ])("maps HTTP %i to its localized message", async (status, message) => { + mockImportConfig.mockRejectedValueOnce( + new ApiError(status, { detail: "generic backend text" }, `HTTP ${status}`), + ); + const { result } = await loaded(); + + await act(() => result.current.runImport()); + + expect(result.current.error).toBe(message); + expect(toast.error).toHaveBeenCalledWith(message); + expect(result.current.isLoading).toBe(false); + }); + + it("uses the backend detail for other errors", async () => { + mockImportConfig.mockRejectedValueOnce( + new ApiError(400, { detail: "Invalid conflict strategy" }, "HTTP 400"), + ); + const { result } = await loaded(); + + await act(() => result.current.runImport()); + + expect(result.current.error).toBe("Invalid conflict strategy"); + }); + + describe("preview", () => { + it("stores the preview when allowed", async () => { + mockPreviewImport.mockResolvedValueOnce(PREVIEW); + const { result } = await loaded(); + + await act(() => result.current.preview()); + + expect(mockPreviewImport).toHaveBeenCalledWith(EXPORT, expect.any(AbortSignal)); + expect(result.current.previewResult).toEqual(PREVIEW); + }); + + it("is skipped without admin.system_config", async () => { + grant("admin.import"); + const { result } = await loaded(); + + await act(() => result.current.preview()); + + expect(mockPreviewImport).not.toHaveBeenCalled(); + }); + + it("falls back to the preview message", async () => { + mockPreviewImport.mockRejectedValueOnce(new Error("network")); + const { result } = await loaded(); + + await act(() => result.current.preview()); + + expect(result.current.error).toBe("Could not preview the import"); + }); + }); + + it("reset aborts the request and clears all state", async () => { + mockImportConfig.mockReturnValueOnce(new Promise(() => {})); + const { result } = await loaded(); + + act(() => { + void result.current.runImport(); + }); + expect(result.current.isLoading).toBe(true); + const signal = mockImportConfig.mock.calls[0][1] as AbortSignal; + + act(() => result.current.reset()); + + expect(signal.aborted).toBe(true); + expect(result.current).toMatchObject({ data: null, isLoading: false, error: null }); + }); + + it("aborts the in-flight request on unmount", async () => { + mockImportConfig.mockReturnValueOnce(new Promise(() => {})); + const { result, unmount } = await loaded(); + + act(() => { + void result.current.runImport(); + }); + const signal = mockImportConfig.mock.calls[0][1] as AbortSignal; + unmount(); + + expect(signal.aborted).toBe(true); + }); +}); diff --git a/src/hooks/useImportConfig.ts b/src/hooks/useImportConfig.ts new file mode 100644 index 00000000..a8c3b552 --- /dev/null +++ b/src/hooks/useImportConfig.ts @@ -0,0 +1,186 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import { useIntl, type IntlShape } from "react-intl"; +import { toast } from "sonner"; + +import { ApiError } from "@/api/client"; +import { + ImportFileError, + importExportApi, + parseImportFile, + type ConfigExport, + type ImportOptions, + type ImportPreview, + type ImportResult, +} from "@/api/importExport"; +import { useAuth } from "@/auth/useAuth"; +import { parseApiError } from "@/lib/errorUtils"; + +/** Everything but the file content, which the hook holds itself. */ +export type ImportRunOptions = Omit; + +export interface ImportConfigState { + /** Reads + shape-checks a file; resolves false (with `error` set) when it's unusable. */ + loadFile: (file: File) => Promise; + /** Per-item preview with conflicts; no-op without a file or `canPreview`. */ + preview: () => Promise; + /** Validates the import server-side without writing anything. */ + dryRun: (opts?: ImportRunOptions) => Promise; + runImport: (opts?: ImportRunOptions) => Promise; + reset: () => void; + data: ConfigExport | null; + previewResult: ImportPreview | null; + dryRunResult: ImportResult | null; + result: ImportResult | null; + isLoading: boolean; + error: string | null; + canImport: boolean; + canPreview: boolean; +} + +// Backend details for these are generic English; prefer our own copy. +const STATUS_MESSAGE_IDS: Record = { + 403: "importExport.import.forbidden", + 409: "importExport.import.conflict", + 413: "importExport.import.tooLarge", + 422: "importExport.import.validationFailed", +}; + +function importErrorMessage(err: unknown, intl: IntlShape, fallbackId: string): string { + const id = err instanceof ApiError ? STATUS_MESSAGE_IDS[err.status] : undefined; + return id + ? intl.formatMessage({ id }) + : parseApiError(err, intl.formatMessage({ id: fallbackId })); +} + +export function useImportConfig(): ImportConfigState { + const intl = useIntl(); + const { hasPermission } = useAuth(); + const canImport = hasPermission("admin.import"); + const canPreview = hasPermission("admin.system_config"); + const [data, setData] = useState(null); + const [previewResult, setPreviewResult] = useState(null); + const [dryRunResult, setDryRunResult] = useState(null); + const [result, setResult] = useState(null); + const [isLoading, setLoading] = useState(false); + const [error, setError] = useState(null); + const abortRef = useRef(null); + + const abortCurrent = useCallback(() => { + abortRef.current?.abort(); + abortRef.current = null; + setLoading(false); + }, []); + + useEffect(() => () => abortRef.current?.abort(), []); + + const run = useCallback( + async ( + request: (signal: AbortSignal) => Promise, + onSuccess: (value: T) => void, + fallbackId: string, + ) => { + abortRef.current?.abort(); + const controller = new AbortController(); + abortRef.current = controller; + setLoading(true); + setError(null); + try { + const value = await request(controller.signal); + if (!controller.signal.aborted) onSuccess(value); + } catch (err) { + if (controller.signal.aborted) return; + const message = importErrorMessage(err, intl, fallbackId); + setError(message); + toast.error(message); + } finally { + // A superseding call owns the loading state from here on. + if (abortRef.current === controller) { + abortRef.current = null; + setLoading(false); + } + } + }, + [intl], + ); + + const reset = useCallback(() => { + abortCurrent(); + setData(null); + setPreviewResult(null); + setDryRunResult(null); + setResult(null); + setError(null); + }, [abortCurrent]); + + const loadFile = useCallback( + async (file: File) => { + reset(); + try { + setData(await parseImportFile(file)); + return true; + } catch (err) { + // Anything else is the browser failing to read the file. + const id = !(err instanceof ImportFileError) + ? "importExport.import.failed" + : err.code === "invalid_json" + ? "importExport.import.invalidJson" + : "importExport.import.invalidShape"; + setError(intl.formatMessage({ id })); + return false; + } + }, + [intl, reset], + ); + + const preview = useCallback(async () => { + if (!data || !canPreview) return; + await run( + (signal) => importExportApi.previewImport(data, signal), + setPreviewResult, + "importExport.preview.failed", + ); + }, [canPreview, data, run]); + + const dryRun = useCallback( + async (opts: ImportRunOptions = {}) => { + if (!data) return; + await run( + (signal) => importExportApi.importConfig({ ...opts, data, dryRun: true }, signal), + setDryRunResult, + "importExport.import.failed", + ); + }, + [data, run], + ); + + const runImport = useCallback( + async (opts: ImportRunOptions = {}) => { + if (!data) return; + await run( + (signal) => importExportApi.importConfig({ ...opts, data, dryRun: false }, signal), + (value) => { + setResult(value); + toast.success(intl.formatMessage({ id: "importExport.import.success" }, value.progress)); + }, + "importExport.import.failed", + ); + }, + [data, intl, run], + ); + + return { + loadFile, + preview, + dryRun, + runImport, + reset, + data, + previewResult, + dryRunResult, + result, + isLoading, + error, + canImport, + canPreview, + }; +} diff --git a/src/i18n/locales/en-US/common.json b/src/i18n/locales/en-US/common.json index e98c45a8..cc4b070c 100644 --- a/src/i18n/locales/en-US/common.json +++ b/src/i18n/locales/en-US/common.json @@ -68,5 +68,14 @@ "common.required": "*", "common.team.label": "Team", "common.team.placeholder": "Select a team", - "importExport.export.failed": "Export failed" + "importExport.export.failed": "Export failed", + "importExport.import.success": "Import complete: {created} created, {updated} updated, {skipped} skipped, {failed} failed", + "importExport.import.failed": "Import failed", + "importExport.import.invalidJson": "The selected file is not valid JSON", + "importExport.import.invalidShape": "The selected file is not a ContextForge export", + "importExport.import.tooLarge": "The file is too large to import", + "importExport.import.conflict": "Import stopped: some items already exist", + "importExport.import.validationFailed": "The file failed import validation", + "importExport.import.forbidden": "You don't have permission to import this configuration", + "importExport.preview.failed": "Could not preview the import" } diff --git a/src/i18n/locales/es-ES/common.json b/src/i18n/locales/es-ES/common.json index 38f8376e..7e8a27d1 100644 --- a/src/i18n/locales/es-ES/common.json +++ b/src/i18n/locales/es-ES/common.json @@ -68,5 +68,14 @@ "common.required": "*", "common.team.label": "Equipo", "common.team.placeholder": "Seleccione un equipo", - "importExport.export.failed": "Error al exportar" + "importExport.export.failed": "Error al exportar", + "importExport.import.success": "Importación completada: {created} creados, {updated} actualizados, {skipped} omitidos, {failed} con error", + "importExport.import.failed": "Error al importar", + "importExport.import.invalidJson": "El archivo seleccionado no es un JSON válido", + "importExport.import.invalidShape": "El archivo seleccionado no es una exportación de ContextForge", + "importExport.import.tooLarge": "El archivo es demasiado grande para importarlo", + "importExport.import.conflict": "Importación detenida: algunos elementos ya existen", + "importExport.import.validationFailed": "El archivo no superó la validación de importación", + "importExport.import.forbidden": "No tiene permiso para importar esta configuración", + "importExport.preview.failed": "No se pudo previsualizar la importación" } diff --git a/src/i18n/locales/pt-BR/common.json b/src/i18n/locales/pt-BR/common.json index 5475771f..d7f4083b 100644 --- a/src/i18n/locales/pt-BR/common.json +++ b/src/i18n/locales/pt-BR/common.json @@ -68,5 +68,14 @@ "common.required": "*", "common.team.label": "Equipe", "common.team.placeholder": "Selecione uma equipe", - "importExport.export.failed": "Falha ao exportar" + "importExport.export.failed": "Falha ao exportar", + "importExport.import.success": "Importação concluída: {created} criados, {updated} atualizados, {skipped} ignorados, {failed} com falha", + "importExport.import.failed": "Falha ao importar", + "importExport.import.invalidJson": "O arquivo selecionado não é um JSON válido", + "importExport.import.invalidShape": "O arquivo selecionado não é uma exportação do ContextForge", + "importExport.import.tooLarge": "O arquivo é grande demais para ser importado", + "importExport.import.conflict": "Importação interrompida: alguns itens já existem", + "importExport.import.validationFailed": "O arquivo não passou na validação de importação", + "importExport.import.forbidden": "Você não tem permissão para importar esta configuração", + "importExport.preview.failed": "Não foi possível pré-visualizar a importação" } From 919b5682a6de9ccd52e1634a697d9e896da50b81 Mon Sep 17 00:00:00 2001 From: Gabriel Costa Date: Tue, 3 Nov 2026 12:00:50 +0000 Subject: [PATCH 2/3] Remove toast and preview Signed-off-by: Gabriel Costa --- src/hooks/useImportConfig.test.ts | 135 +++++++++++++++-------------- src/hooks/useImportConfig.ts | 101 ++++++++------------- src/i18n/locales/en-US/common.json | 3 +- src/i18n/locales/es-ES/common.json | 3 +- src/i18n/locales/pt-BR/common.json | 3 +- 5 files changed, 115 insertions(+), 130 deletions(-) diff --git a/src/hooks/useImportConfig.test.ts b/src/hooks/useImportConfig.test.ts index b8d6d8d1..524c4cc2 100644 --- a/src/hooks/useImportConfig.test.ts +++ b/src/hooks/useImportConfig.test.ts @@ -2,31 +2,23 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { act, renderHook as rtlRenderHook } from "@testing-library/react"; import { createElement, type ReactNode } from "react"; import { IntlProvider } from "react-intl"; -import { toast } from "sonner"; import { ApiError } from "@/api/client"; -import { - importExportApi, - type ConfigExport, - type ImportPreview, - type ImportResult, -} from "@/api/importExport"; +import { importExportApi, type ConfigExport, type ImportResult } from "@/api/importExport"; import { useAuth } from "@/auth/useAuth"; import enMessages from "@/i18n/locales/en-US"; -import { useImportConfig } from "./useImportConfig"; +import { importOutcome, useImportConfig } from "./useImportConfig"; vi.mock("@/auth/useAuth", () => ({ useAuth: vi.fn() })); -vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } })); // Keep parseImportFile/ImportFileError real; stub only the network calls. vi.mock("@/api/importExport", async (importOriginal) => { const actual = await importOriginal(); - return { ...actual, importExportApi: { importConfig: vi.fn(), previewImport: vi.fn() } }; + return { ...actual, importExportApi: { importConfig: vi.fn() } }; }); const mockUseAuth = vi.mocked(useAuth); const mockImportConfig = vi.mocked(importExportApi.importConfig); -const mockPreviewImport = vi.mocked(importExportApi.previewImport); const EXPORT: ConfigExport = { version: "2025-06-18", @@ -45,14 +37,6 @@ const RESULT: ImportResult = { completed_at: "2026-11-02T10:00:01Z", }; -const PREVIEW: ImportPreview = { - summary: { total_items: 1, by_type: { tools: 1 } }, - items: {}, - bundles: {}, - dependencies: {}, - conflicts: {}, -}; - // jsdom's File has no .text(); parseImportFile only needs that. const fileOf = (content: string) => ({ text: async () => content }) as unknown as File; const exportFile = () => fileOf(JSON.stringify(EXPORT)); @@ -81,14 +65,14 @@ async function loaded() { describe("useImportConfig", () => { beforeEach(() => { vi.clearAllMocks(); - grant("admin.import", "admin.system_config"); + grant("admin.import"); }); - it("derives canImport and canPreview from separate permissions", () => { - expect(renderHook().result.current).toMatchObject({ canImport: true, canPreview: true }); + it("canImport follows the admin.import permission", () => { + expect(renderHook().result.current.canImport).toBe(true); - grant("admin.import"); - expect(renderHook().result.current).toMatchObject({ canImport: true, canPreview: false }); + grant(); + expect(renderHook().result.current.canImport).toBe(false); }); describe("loadFile", () => { @@ -146,43 +130,47 @@ describe("useImportConfig", () => { it("does nothing before a file is loaded", async () => { const { result } = renderHook(); - await act(() => result.current.dryRun()); - await act(() => result.current.runImport()); - await act(() => result.current.preview()); + await act(async () => { + expect(await result.current.dryRun()).toBeNull(); + expect(await result.current.runImport()).toBeNull(); + }); expect(mockImportConfig).not.toHaveBeenCalled(); - expect(mockPreviewImport).not.toHaveBeenCalled(); }); it("dryRun sends dryRun: true and stores the result separately", async () => { mockImportConfig.mockResolvedValueOnce(RESULT); const { result } = await loaded(); - await act(() => result.current.dryRun({ conflictStrategy: "skip" })); + let returned!: ImportResult | null; + await act(async () => { + returned = await result.current.dryRun({ conflictStrategy: "skip" }); + }); expect(mockImportConfig).toHaveBeenCalledWith( { conflictStrategy: "skip", data: EXPORT, dryRun: true }, expect.any(AbortSignal), ); + expect(returned).toEqual(RESULT); expect(result.current.dryRunResult).toEqual(RESULT); expect(result.current.result).toBeNull(); - expect(toast.success).not.toHaveBeenCalled(); }); - it("runImport sends dryRun: false and toasts the counts", async () => { + it("runImport sends dryRun: false and resolves with the result", async () => { mockImportConfig.mockResolvedValueOnce(RESULT); const { result } = await loaded(); - await act(() => result.current.runImport({ selectedEntities: { tools: ["t1"] } })); + let returned!: ImportResult | null; + await act(async () => { + returned = await result.current.runImport({ selectedEntities: { tools: ["t1"] } }); + }); expect(mockImportConfig).toHaveBeenCalledWith( { selectedEntities: { tools: ["t1"] }, data: EXPORT, dryRun: false }, expect.any(AbortSignal), ); + expect(returned).toEqual(RESULT); expect(result.current.result).toEqual(RESULT); - expect(toast.success).toHaveBeenCalledWith( - "Import complete: 1 created, 2 updated, 1 skipped, 0 failed", - ); }); it.each([ @@ -196,10 +184,13 @@ describe("useImportConfig", () => { ); const { result } = await loaded(); - await act(() => result.current.runImport()); + let returned: ImportResult | null | undefined; + await act(async () => { + returned = await result.current.runImport(); + }); + expect(returned).toBeNull(); expect(result.current.error).toBe(message); - expect(toast.error).toHaveBeenCalledWith(message); expect(result.current.isLoading).toBe(false); }); @@ -214,34 +205,13 @@ describe("useImportConfig", () => { expect(result.current.error).toBe("Invalid conflict strategy"); }); - describe("preview", () => { - it("stores the preview when allowed", async () => { - mockPreviewImport.mockResolvedValueOnce(PREVIEW); - const { result } = await loaded(); - - await act(() => result.current.preview()); - - expect(mockPreviewImport).toHaveBeenCalledWith(EXPORT, expect.any(AbortSignal)); - expect(result.current.previewResult).toEqual(PREVIEW); - }); - - it("is skipped without admin.system_config", async () => { - grant("admin.import"); - const { result } = await loaded(); - - await act(() => result.current.preview()); - - expect(mockPreviewImport).not.toHaveBeenCalled(); - }); - - it("falls back to the preview message", async () => { - mockPreviewImport.mockRejectedValueOnce(new Error("network")); - const { result } = await loaded(); + it("falls back to the localized message when the error has no detail", async () => { + mockImportConfig.mockRejectedValueOnce(new Error("network")); + const { result } = await loaded(); - await act(() => result.current.preview()); + await act(() => result.current.runImport()); - expect(result.current.error).toBe("Could not preview the import"); - }); + expect(result.current.error).toBe("Import failed"); }); it("reset aborts the request and clears all state", async () => { @@ -260,6 +230,27 @@ describe("useImportConfig", () => { expect(result.current).toMatchObject({ data: null, isLoading: false, error: null }); }); + it("resolves null and keeps state clear when a reset request answers late", async () => { + let resolve!: (value: ImportResult) => void; + mockImportConfig.mockReturnValueOnce(new Promise((r) => (resolve = r))); + const { result } = await loaded(); + + let pending!: Promise; + act(() => { + pending = result.current.runImport(); + }); + act(() => result.current.reset()); + + let returned: ImportResult | null | undefined; + await act(async () => { + resolve(RESULT); + returned = await pending; + }); + + expect(returned).toBeNull(); + expect(result.current.result).toBeNull(); + }); + it("aborts the in-flight request on unmount", async () => { mockImportConfig.mockReturnValueOnce(new Promise(() => {})); const { result, unmount } = await loaded(); @@ -273,3 +264,21 @@ describe("useImportConfig", () => { expect(signal.aborted).toBe(true); }); }); + +describe("importOutcome", () => { + const withProgress = ( + failed: number, + total: number, + status: ImportResult["status"] = "completed", + ) => ({ ...RESULT, status, progress: { ...RESULT.progress, failed, total } }) as ImportResult; + + it.each([ + ["no failures", withProgress(0, 4), "success"], + ["an empty import", withProgress(0, 0), "success"], + ["some failures", withProgress(1, 4), "partial"], + ["every entity failed", withProgress(4, 4), "failed"], + ["a failed status", withProgress(0, 4, "failed"), "failed"], + ])("classifies %s", (_label, result, outcome) => { + expect(importOutcome(result)).toBe(outcome); + }); +}); diff --git a/src/hooks/useImportConfig.ts b/src/hooks/useImportConfig.ts index a8c3b552..31f3f8bd 100644 --- a/src/hooks/useImportConfig.ts +++ b/src/hooks/useImportConfig.ts @@ -1,6 +1,5 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { useIntl, type IntlShape } from "react-intl"; -import { toast } from "sonner"; import { ApiError } from "@/api/client"; import { @@ -9,7 +8,6 @@ import { parseImportFile, type ConfigExport, type ImportOptions, - type ImportPreview, type ImportResult, } from "@/api/importExport"; import { useAuth } from "@/auth/useAuth"; @@ -18,23 +16,36 @@ import { parseApiError } from "@/lib/errorUtils"; /** Everything but the file content, which the hook holds itself. */ export type ImportRunOptions = Omit; +/** + * Requests + state only. Actions resolve with their result, or null on failure + * (`error` set), abort, or a skipped call; feedback (toasts etc.) is the + * caller's job. Use `importOutcome` to judge an import result. + */ export interface ImportConfigState { /** Reads + shape-checks a file; resolves false (with `error` set) when it's unusable. */ loadFile: (file: File) => Promise; - /** Per-item preview with conflicts; no-op without a file or `canPreview`. */ - preview: () => Promise; /** Validates the import server-side without writing anything. */ - dryRun: (opts?: ImportRunOptions) => Promise; - runImport: (opts?: ImportRunOptions) => Promise; + dryRun: (opts?: ImportRunOptions) => Promise; + runImport: (opts?: ImportRunOptions) => Promise; reset: () => void; data: ConfigExport | null; - previewResult: ImportPreview | null; dryRunResult: ImportResult | null; result: ImportResult | null; isLoading: boolean; error: string | null; canImport: boolean; - canPreview: boolean; +} + +export type ImportOutcome = "success" | "partial" | "failed"; + +/** + * The backend reports `completed` even when entities fail, so judge by counts. + * Works for dry-run results too. + */ +export function importOutcome(result: ImportResult): ImportOutcome { + const { failed, total } = result.progress; + if (result.status === "failed" || (total > 0 && failed >= total)) return "failed"; + return failed > 0 ? "partial" : "success"; } // Backend details for these are generic English; prefer our own copy. @@ -45,20 +56,18 @@ const STATUS_MESSAGE_IDS: Record = { 422: "importExport.import.validationFailed", }; -function importErrorMessage(err: unknown, intl: IntlShape, fallbackId: string): string { +function importErrorMessage(err: unknown, intl: IntlShape): string { const id = err instanceof ApiError ? STATUS_MESSAGE_IDS[err.status] : undefined; return id ? intl.formatMessage({ id }) - : parseApiError(err, intl.formatMessage({ id: fallbackId })); + : parseApiError(err, intl.formatMessage({ id: "importExport.import.failed" })); } export function useImportConfig(): ImportConfigState { const intl = useIntl(); const { hasPermission } = useAuth(); const canImport = hasPermission("admin.import"); - const canPreview = hasPermission("admin.system_config"); const [data, setData] = useState(null); - const [previewResult, setPreviewResult] = useState(null); const [dryRunResult, setDryRunResult] = useState(null); const [result, setResult] = useState(null); const [isLoading, setLoading] = useState(false); @@ -73,25 +82,26 @@ export function useImportConfig(): ImportConfigState { useEffect(() => () => abortRef.current?.abort(), []); - const run = useCallback( - async ( - request: (signal: AbortSignal) => Promise, - onSuccess: (value: T) => void, - fallbackId: string, - ) => { + // Shared by dryRun and runImport; only the flag and the result slot differ. + const submit = useCallback( + async (opts: ImportRunOptions, dryRun: boolean): Promise => { + if (!data) return null; abortRef.current?.abort(); const controller = new AbortController(); abortRef.current = controller; setLoading(true); setError(null); try { - const value = await request(controller.signal); - if (!controller.signal.aborted) onSuccess(value); + const value = await importExportApi.importConfig( + { ...opts, data, dryRun }, + controller.signal, + ); + if (controller.signal.aborted) return null; + (dryRun ? setDryRunResult : setResult)(value); + return value; } catch (err) { - if (controller.signal.aborted) return; - const message = importErrorMessage(err, intl, fallbackId); - setError(message); - toast.error(message); + if (!controller.signal.aborted) setError(importErrorMessage(err, intl)); + return null; } finally { // A superseding call owns the loading state from here on. if (abortRef.current === controller) { @@ -100,13 +110,12 @@ export function useImportConfig(): ImportConfigState { } } }, - [intl], + [data, intl], ); const reset = useCallback(() => { abortCurrent(); setData(null); - setPreviewResult(null); setDryRunResult(null); setResult(null); setError(null); @@ -132,55 +141,19 @@ export function useImportConfig(): ImportConfigState { [intl, reset], ); - const preview = useCallback(async () => { - if (!data || !canPreview) return; - await run( - (signal) => importExportApi.previewImport(data, signal), - setPreviewResult, - "importExport.preview.failed", - ); - }, [canPreview, data, run]); - - const dryRun = useCallback( - async (opts: ImportRunOptions = {}) => { - if (!data) return; - await run( - (signal) => importExportApi.importConfig({ ...opts, data, dryRun: true }, signal), - setDryRunResult, - "importExport.import.failed", - ); - }, - [data, run], - ); - - const runImport = useCallback( - async (opts: ImportRunOptions = {}) => { - if (!data) return; - await run( - (signal) => importExportApi.importConfig({ ...opts, data, dryRun: false }, signal), - (value) => { - setResult(value); - toast.success(intl.formatMessage({ id: "importExport.import.success" }, value.progress)); - }, - "importExport.import.failed", - ); - }, - [data, intl, run], - ); + const dryRun = useCallback((opts: ImportRunOptions = {}) => submit(opts, true), [submit]); + const runImport = useCallback((opts: ImportRunOptions = {}) => submit(opts, false), [submit]); return { loadFile, - preview, dryRun, runImport, reset, data, - previewResult, dryRunResult, result, isLoading, error, canImport, - canPreview, }; } diff --git a/src/i18n/locales/en-US/common.json b/src/i18n/locales/en-US/common.json index cc4b070c..d9ca5b4b 100644 --- a/src/i18n/locales/en-US/common.json +++ b/src/i18n/locales/en-US/common.json @@ -77,5 +77,6 @@ "importExport.import.conflict": "Import stopped: some items already exist", "importExport.import.validationFailed": "The file failed import validation", "importExport.import.forbidden": "You don't have permission to import this configuration", - "importExport.preview.failed": "Could not preview the import" + "importExport.import.partial": "Import partially applied: {created} created, {updated} updated, {skipped} skipped, {failed} failed", + "importExport.import.allFailed": "Import failed: none of the {failed} items could be applied" } diff --git a/src/i18n/locales/es-ES/common.json b/src/i18n/locales/es-ES/common.json index 7e8a27d1..5cb6eac6 100644 --- a/src/i18n/locales/es-ES/common.json +++ b/src/i18n/locales/es-ES/common.json @@ -77,5 +77,6 @@ "importExport.import.conflict": "Importación detenida: algunos elementos ya existen", "importExport.import.validationFailed": "El archivo no superó la validación de importación", "importExport.import.forbidden": "No tiene permiso para importar esta configuración", - "importExport.preview.failed": "No se pudo previsualizar la importación" + "importExport.import.partial": "Importación aplicada parcialmente: {created} creados, {updated} actualizados, {skipped} omitidos, {failed} con error", + "importExport.import.allFailed": "Error al importar: no se pudo aplicar ninguno de los {failed} elementos" } diff --git a/src/i18n/locales/pt-BR/common.json b/src/i18n/locales/pt-BR/common.json index d7f4083b..abb5e097 100644 --- a/src/i18n/locales/pt-BR/common.json +++ b/src/i18n/locales/pt-BR/common.json @@ -77,5 +77,6 @@ "importExport.import.conflict": "Importação interrompida: alguns itens já existem", "importExport.import.validationFailed": "O arquivo não passou na validação de importação", "importExport.import.forbidden": "Você não tem permissão para importar esta configuração", - "importExport.preview.failed": "Não foi possível pré-visualizar a importação" + "importExport.import.partial": "Importação aplicada parcialmente: {created} criados, {updated} atualizados, {skipped} ignorados, {failed} com falha", + "importExport.import.allFailed": "Falha ao importar: nenhum dos {failed} itens pôde ser aplicado" } From c3e78523d9e3327faad8b101f759e2f2f73d6d75 Mon Sep 17 00:00:00 2001 From: Gabriel Costa Date: Thu, 5 Nov 2026 16:16:51 +0000 Subject: [PATCH 3/3] drop file on second file upload Signed-off-by: Gabriel Costa --- src/hooks/useImportConfig.test.ts | 99 +++++++++++++++++++++++++++++++ src/hooks/useImportConfig.ts | 11 +++- 2 files changed, 108 insertions(+), 2 deletions(-) diff --git a/src/hooks/useImportConfig.test.ts b/src/hooks/useImportConfig.test.ts index 524c4cc2..093aa150 100644 --- a/src/hooks/useImportConfig.test.ts +++ b/src/hooks/useImportConfig.test.ts @@ -115,6 +115,105 @@ describe("useImportConfig", () => { expect(result.current.error).toBe("Import failed"); }); + describe("superseded reads", () => { + // A file whose read settles only when the test says so. + function slowFile() { + let resolve!: (content: string) => void; + let reject!: (err: Error) => void; + const text = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { file: { text: () => text } as unknown as File, resolve, reject }; + } + const OTHER: ConfigExport = { ...EXPORT, entities: { servers: [{ name: "s1" }] } }; + + it("keeps the newer file when an older read finishes last", async () => { + const a = slowFile(); + const b = slowFile(); + const { result } = renderHook(); + + let loadA!: Promise; + let loadB!: Promise; + act(() => { + loadA = result.current.loadFile(a.file); + }); + act(() => { + loadB = result.current.loadFile(b.file); + }); + + let okB!: boolean; + await act(async () => { + b.resolve(JSON.stringify(OTHER)); + okB = await loadB; + }); + let okA!: boolean; + await act(async () => { + a.resolve(JSON.stringify(EXPORT)); + okA = await loadA; + }); + + expect(okB).toBe(true); + expect(okA).toBe(false); + expect(result.current.data).toEqual(OTHER); + }); + + it("does not restore a file discarded by reset mid-read", async () => { + const a = slowFile(); + const { result } = renderHook(); + + let load!: Promise; + act(() => { + load = result.current.loadFile(a.file); + }); + act(() => result.current.reset()); + + let ok!: boolean; + await act(async () => { + a.resolve(JSON.stringify(EXPORT)); + ok = await load; + }); + + expect(ok).toBe(false); + expect(result.current.data).toBeNull(); + }); + + it("ignores a late failure from a superseded read", async () => { + const a = slowFile(); + const { result } = renderHook(); + + let loadA!: Promise; + act(() => { + loadA = result.current.loadFile(a.file); + }); + await act(() => result.current.loadFile(exportFile())); + await act(async () => { + a.reject(new Error("NotReadableError")); + await loadA; + }); + + expect(result.current.data).toEqual(EXPORT); + expect(result.current.error).toBeNull(); + }); + + it("ignores a late failure after reset", async () => { + const a = slowFile(); + const { result } = renderHook(); + + let load!: Promise; + act(() => { + load = result.current.loadFile(a.file); + }); + act(() => result.current.reset()); + await act(async () => { + a.resolve("{not json"); + await load; + }); + + expect(result.current.error).toBeNull(); + }); + }); + it("clears earlier results when a new file is loaded", async () => { mockImportConfig.mockResolvedValueOnce(RESULT); const { result } = await loaded(); diff --git a/src/hooks/useImportConfig.ts b/src/hooks/useImportConfig.ts index 31f3f8bd..2aa8e552 100644 --- a/src/hooks/useImportConfig.ts +++ b/src/hooks/useImportConfig.ts @@ -22,7 +22,7 @@ export type ImportRunOptions = Omit; * caller's job. Use `importOutcome` to judge an import result. */ export interface ImportConfigState { - /** Reads + shape-checks a file; resolves false (with `error` set) when it's unusable. */ + /** Reads + shape-checks a file; false when unusable (`error` set) or superseded by a newer load/reset. */ loadFile: (file: File) => Promise; /** Validates the import server-side without writing anything. */ dryRun: (opts?: ImportRunOptions) => Promise; @@ -73,6 +73,8 @@ export function useImportConfig(): ImportConfigState { const [isLoading, setLoading] = useState(false); const [error, setError] = useState(null); const abortRef = useRef(null); + // Bumped by every reset/new load; a file read that finishes under an older value is dropped. + const loadGenRef = useRef(0); const abortCurrent = useCallback(() => { abortRef.current?.abort(); @@ -114,6 +116,7 @@ export function useImportConfig(): ImportConfigState { ); const reset = useCallback(() => { + loadGenRef.current += 1; abortCurrent(); setData(null); setDryRunResult(null); @@ -124,10 +127,14 @@ export function useImportConfig(): ImportConfigState { const loadFile = useCallback( async (file: File) => { reset(); + const gen = loadGenRef.current; try { - setData(await parseImportFile(file)); + const parsed = await parseImportFile(file); + if (gen !== loadGenRef.current) return false; + setData(parsed); return true; } catch (err) { + if (gen !== loadGenRef.current) return false; // Anything else is the browser failing to read the file. const id = !(err instanceof ImportFileError) ? "importExport.import.failed"