From f144b025b08d5573604d11881d2bd7d54cc1f9f2 Mon Sep 17 00:00:00 2001
From: Marcel <marcel@familienarchiv>
Date: Mon, 18 May 2026 19:04:06 +0200
Subject: [PATCH] fix(import): address round-3 review concerns

- Add comment to openFileStream() explaining package-private visibility
  is intentional (Mockito spy seam for IOException test)
- Key {#each} skippedFiles by filename instead of array index
- Add test: skipped section hidden when state is FAILED
- Add test: reasonLabel returns raw code for unknown reason strings

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 .../importing/MassImportService.java          |  1 +
 .../admin/system/ImportStatusCard.svelte      |  2 +-
 .../system/ImportStatusCard.svelte.test.ts    | 33 +++++++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java b/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java
index 1ae77541..58d6493a 100644
--- a/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java
+++ b/backend/src/main/java/org/raddatz/familienarchiv/importing/MassImportService.java
@@ -312,6 +312,7 @@ public class MassImportService {
         return new ProcessResult(processed, skippedFiles);
     }
 
+    // package-private: Mockito spy in tests can override to inject IOException
     InputStream openFileStream(File file) throws IOException {
         return new FileInputStream(file);
     }
diff --git a/frontend/src/routes/admin/system/ImportStatusCard.svelte b/frontend/src/routes/admin/system/ImportStatusCard.svelte
index 176b90f5..f6e9ab82 100644
--- a/frontend/src/routes/admin/system/ImportStatusCard.svelte
+++ b/frontend/src/routes/admin/system/ImportStatusCard.svelte
@@ -78,7 +78,7 @@ function reasonLabel(code: string): string {
 					</div>
 				</summary>
 				<ul class="mt-3 space-y-1">
-					{#each importStatus.skippedFiles as skipped, i (i)}
+					{#each importStatus.skippedFiles as skipped (skipped.filename)}
 						<li class="font-mono text-sm text-ink-2">
 							{skipped.filename} — {reasonLabel(skipped.reason)}
 						</li>
diff --git a/frontend/src/routes/admin/system/ImportStatusCard.svelte.test.ts b/frontend/src/routes/admin/system/ImportStatusCard.svelte.test.ts
index ab7acacf..d7470cda 100644
--- a/frontend/src/routes/admin/system/ImportStatusCard.svelte.test.ts
+++ b/frontend/src/routes/admin/system/ImportStatusCard.svelte.test.ts
@@ -199,4 +199,37 @@ describe('ImportStatusCard', () => {
 
 		await expect.element(getByTestId('skipped-count')).not.toBeInTheDocument();
 	});
+
+	it('does not show skipped section when FAILED even with skipped > 0', async () => {
+		const { getByTestId } = render(ImportStatusCard, {
+			props: {
+				importStatus: makeStatus({
+					state: 'FAILED',
+					statusCode: 'IMPORT_FAILED_INTERNAL',
+					skipped: 1,
+					skippedFiles: [{ filename: 'bad.pdf', reason: 'INVALID_PDF_SIGNATURE' }]
+				}),
+				ontrigger: () => {}
+			}
+		});
+
+		await expect.element(getByTestId('skipped-count')).not.toBeInTheDocument();
+	});
+
+	it('shows raw reason code for unknown skip reasons', async () => {
+		const { getByText } = render(ImportStatusCard, {
+			props: {
+				importStatus: makeStatus({
+					state: 'DONE',
+					statusCode: 'IMPORT_DONE',
+					processed: 1,
+					skipped: 1,
+					skippedFiles: [{ filename: 'odd.pdf', reason: 'SOME_FUTURE_CODE' }]
+				}),
+				ontrigger: () => {}
+			}
+		});
+
+		await expect.element(getByText('SOME_FUTURE_CODE', { exact: false })).toBeInTheDocument();
+	});
 });