fix(admin/system): address review concerns in ImportStatusCard
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m35s
CI / OCR Service Tests (pull_request) Successful in 19s
CI / Backend Unit Tests (pull_request) Successful in 4m25s
CI / fail2ban Regex (pull_request) Successful in 39s
CI / Compose Bucket Idempotency (pull_request) Successful in 58s
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m35s
CI / OCR Service Tests (pull_request) Successful in 19s
CI / Backend Unit Tests (pull_request) Successful in 4m25s
CI / fail2ban Regex (pull_request) Successful in 39s
CI / Compose Bucket Idempotency (pull_request) Successful in 58s
- 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 <noreply@anthropic.com>
This commit is contained in:
@@ -11,7 +11,6 @@ let backfillHashesLoading = $state(false);
|
||||
type ImportStatus = {
|
||||
state: 'IDLE' | 'RUNNING' | 'DONE' | 'FAILED';
|
||||
statusCode: string;
|
||||
message: string;
|
||||
processed: number;
|
||||
startedAt: string | null;
|
||||
};
|
||||
|
||||
@@ -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()
|
||||
);
|
||||
</script>
|
||||
|
||||
<div class="rounded-sm border border-line bg-surface p-6 shadow-sm">
|
||||
@@ -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"
|
||||
></span>
|
||||
<div>
|
||||
<p class="text-base font-bold text-ink">{importStatus.processed}</p>
|
||||
@@ -40,28 +45,26 @@ let {
|
||||
{:else if importStatus?.state === 'DONE'}
|
||||
<div class="mb-4 rounded-sm border border-green-200 bg-green-50 p-4 text-green-700">
|
||||
<p class="text-base font-bold">{importStatus.processed}</p>
|
||||
<p class="text-xs font-bold tracking-widest text-green-600 uppercase">
|
||||
<p class="text-xs font-bold tracking-widest text-green-800 uppercase">
|
||||
{m.admin_system_import_status_done_label()}
|
||||
</p>
|
||||
<p class="mt-1 text-xs text-green-600">{m.admin_system_import_status_done()}</p>
|
||||
<p class="mt-1 text-xs text-green-800">{m.admin_system_import_status_done()}</p>
|
||||
</div>
|
||||
<button
|
||||
data-import-trigger
|
||||
onclick={ontrigger}
|
||||
class="rounded-sm bg-primary px-5 py-2 font-sans text-xs font-bold tracking-widest text-primary-fg uppercase transition-opacity hover:opacity-80"
|
||||
class="min-h-[44px] rounded-sm bg-primary px-5 py-2 font-sans text-xs font-bold tracking-widest text-primary-fg uppercase transition-opacity hover:opacity-80"
|
||||
>
|
||||
{m.admin_system_import_btn_retry()}
|
||||
</button>
|
||||
{:else if importStatus?.state === 'FAILED'}
|
||||
<p class="mb-4 rounded-sm border border-red-200 bg-red-50 p-3 text-sm text-red-700">
|
||||
{importStatus.statusCode === 'IMPORT_FAILED_NO_SPREADSHEET'
|
||||
? m.admin_system_import_failed_no_spreadsheet()
|
||||
: m.admin_system_import_failed_internal()}
|
||||
{failureMessage}
|
||||
</p>
|
||||
<button
|
||||
data-import-trigger
|
||||
onclick={ontrigger}
|
||||
class="rounded-sm bg-primary px-5 py-2 font-sans text-xs font-bold tracking-widest text-primary-fg uppercase transition-opacity hover:opacity-80"
|
||||
class="min-h-[44px] rounded-sm bg-primary px-5 py-2 font-sans text-xs font-bold tracking-widest text-primary-fg uppercase transition-opacity hover:opacity-80"
|
||||
>
|
||||
{m.admin_system_import_btn_retry()}
|
||||
</button>
|
||||
@@ -72,7 +75,7 @@ let {
|
||||
<button
|
||||
data-import-trigger
|
||||
onclick={ontrigger}
|
||||
class="rounded-sm bg-primary px-5 py-2 font-sans text-xs font-bold tracking-widest text-primary-fg uppercase transition-opacity hover:opacity-80"
|
||||
class="min-h-[44px] rounded-sm bg-primary px-5 py-2 font-sans text-xs font-bold tracking-widest text-primary-fg uppercase transition-opacity hover:opacity-80"
|
||||
>
|
||||
{m.admin_system_import_btn_start()}
|
||||
</button>
|
||||
|
||||
@@ -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> = {}): 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();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user