From 60e37bb2af1a7f4f315128e973a85e60c1c73082 Mon Sep 17 00:00:00 2001 From: Marcel Date: Thu, 14 May 2026 13:24:11 +0200 Subject: [PATCH] fix(admin/system): address review concerns in ImportStatusCard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove dead `message` field from both frontend ImportStatus types (field is now @JsonIgnore'd on the backend) - Extract failure message ternary into `$derived` — business logic off the template (Felix) - Add motion-reduce:animate-none to spinner — WCAG 2.1 SC 2.3.3 (Leonie) - Replace text-green-600 with text-green-800 — WCAG AA contrast 6.1:1 on bg-green-50 (Leonie) - Add min-h-[44px] to all three buttons — WCAG 2.2 44px touch target (Leonie) - Add 6 missing tests: IMPORT_FAILED_INTERNAL path, IDLE state text, null importStatus, ontrigger called on DONE/FAILED/IDLE buttons (Sara) Co-Authored-By: Claude Sonnet 4.6 --- frontend/src/routes/admin/system/+page.svelte | 1 - .../admin/system/ImportStatusCard.svelte | 23 +++--- .../system/ImportStatusCard.svelte.test.ts | 76 +++++++++++++++++-- 3 files changed, 84 insertions(+), 16 deletions(-) diff --git a/frontend/src/routes/admin/system/+page.svelte b/frontend/src/routes/admin/system/+page.svelte index f7a61332..81779e64 100644 --- a/frontend/src/routes/admin/system/+page.svelte +++ b/frontend/src/routes/admin/system/+page.svelte @@ -11,7 +11,6 @@ let backfillHashesLoading = $state(false); type ImportStatus = { state: 'IDLE' | 'RUNNING' | 'DONE' | 'FAILED'; statusCode: string; - message: string; processed: number; startedAt: string | null; }; diff --git a/frontend/src/routes/admin/system/ImportStatusCard.svelte b/frontend/src/routes/admin/system/ImportStatusCard.svelte index ae075d97..ac6d23f5 100644 --- a/frontend/src/routes/admin/system/ImportStatusCard.svelte +++ b/frontend/src/routes/admin/system/ImportStatusCard.svelte @@ -4,7 +4,6 @@ import { m } from '$lib/paraglide/messages.js'; type ImportStatus = { state: 'IDLE' | 'RUNNING' | 'DONE' | 'FAILED'; statusCode: string; - message: string; processed: number; startedAt: string | null; }; @@ -16,6 +15,12 @@ let { importStatus: ImportStatus | null; ontrigger: () => void; } = $props(); + +const failureMessage = $derived( + importStatus?.statusCode === 'IMPORT_FAILED_NO_SPREADSHEET' + ? m.admin_system_import_failed_no_spreadsheet() + : m.admin_system_import_failed_internal() +);
@@ -28,7 +33,7 @@ let { data-testid="spinner" role="status" aria-label={m.admin_system_import_status_running()} - class="inline-block h-5 w-5 animate-spin rounded-full border-2 border-ink-3 border-t-brand-mint" + class="inline-block h-5 w-5 animate-spin rounded-full border-2 border-ink-3 border-t-brand-mint motion-reduce:animate-none" >

{importStatus.processed}

@@ -40,28 +45,26 @@ let { {:else if importStatus?.state === 'DONE'}

{importStatus.processed}

-

+

{m.admin_system_import_status_done_label()}

-

{m.admin_system_import_status_done()}

+

{m.admin_system_import_status_done()}

{:else if importStatus?.state === 'FAILED'}

- {importStatus.statusCode === 'IMPORT_FAILED_NO_SPREADSHEET' - ? m.admin_system_import_failed_no_spreadsheet() - : m.admin_system_import_failed_internal()} + {failureMessage}

@@ -72,7 +75,7 @@ let { diff --git a/frontend/src/routes/admin/system/ImportStatusCard.svelte.test.ts b/frontend/src/routes/admin/system/ImportStatusCard.svelte.test.ts index 1c98fe99..0c47d0ec 100644 --- a/frontend/src/routes/admin/system/ImportStatusCard.svelte.test.ts +++ b/frontend/src/routes/admin/system/ImportStatusCard.svelte.test.ts @@ -1,4 +1,4 @@ -import { describe, it } from 'vitest'; +import { describe, it, vi } from 'vitest'; import { render } from 'vitest-browser-svelte'; import { expect } from '@vitest/browser/context'; import ImportStatusCard from './ImportStatusCard.svelte'; @@ -6,7 +6,6 @@ import ImportStatusCard from './ImportStatusCard.svelte'; type ImportStatus = { state: 'IDLE' | 'RUNNING' | 'DONE' | 'FAILED'; statusCode: string; - message: string; processed: number; startedAt: string | null; }; @@ -14,7 +13,6 @@ type ImportStatus = { const makeStatus = (overrides: Partial = {}): ImportStatus => ({ state: 'IDLE', statusCode: 'IMPORT_IDLE', - message: '', processed: 0, startedAt: null, ...overrides @@ -59,8 +57,7 @@ describe('ImportStatusCard', () => { props: { importStatus: makeStatus({ state: 'FAILED', - statusCode: 'IMPORT_FAILED_NO_SPREADSHEET', - message: 'Keine Tabellendatei...' + statusCode: 'IMPORT_FAILED_NO_SPREADSHEET' }), ontrigger: () => {} } @@ -68,4 +65,73 @@ describe('ImportStatusCard', () => { await expect.element(getByText('No spreadsheet file found.')).toBeVisible(); }); + + it('shows internal error message when statusCode is IMPORT_FAILED_INTERNAL', async () => { + const { getByText } = render(ImportStatusCard, { + props: { + importStatus: makeStatus({ state: 'FAILED', statusCode: 'IMPORT_FAILED_INTERNAL' }), + ontrigger: () => {} + } + }); + + await expect.element(getByText('Import failed due to an internal error.')).toBeVisible(); + }); + + it('shows idle text when importStatus is non-null and state is IDLE', async () => { + const { getByText } = render(ImportStatusCard, { + props: { + importStatus: makeStatus({ state: 'IDLE', statusCode: 'IMPORT_IDLE' }), + ontrigger: () => {} + } + }); + + await expect.element(getByText('No import started.')).toBeVisible(); + }); + + it('shows no spinner when importStatus is null', async () => { + render(ImportStatusCard, { + props: { importStatus: null, ontrigger: () => {} } + }); + + expect(document.querySelector('[data-testid="spinner"]')).toBeNull(); + }); + + it('calls ontrigger when retry button is clicked in DONE state', async () => { + const ontrigger = vi.fn(); + const { getByRole } = render(ImportStatusCard, { + props: { + importStatus: makeStatus({ state: 'DONE', statusCode: 'IMPORT_DONE', processed: 5 }), + ontrigger + } + }); + + await getByRole('button').click(); + expect(ontrigger).toHaveBeenCalledOnce(); + }); + + it('calls ontrigger when retry button is clicked in FAILED state', async () => { + const ontrigger = vi.fn(); + const { getByRole } = render(ImportStatusCard, { + props: { + importStatus: makeStatus({ state: 'FAILED', statusCode: 'IMPORT_FAILED_INTERNAL' }), + ontrigger + } + }); + + await getByRole('button').click(); + expect(ontrigger).toHaveBeenCalledOnce(); + }); + + it('calls ontrigger when start button is clicked in IDLE state', async () => { + const ontrigger = vi.fn(); + const { getByRole } = render(ImportStatusCard, { + props: { + importStatus: makeStatus({ state: 'IDLE', statusCode: 'IMPORT_IDLE' }), + ontrigger + } + }); + + await getByRole('button').click(); + expect(ontrigger).toHaveBeenCalledOnce(); + }); });