From 18e675a5b29640bcd998a7d5f30f0c3e286e051f Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 19 May 2026 08:45:39 +0200 Subject: [PATCH] =?UTF-8?q?fix(import):=20address=20non-blocking=20review?= =?UTF-8?q?=20feedback=20=E2=80=94=20touch=20target,=20glossary,=20edge-ca?= =?UTF-8?q?se=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add min-h-[44px] py-2 to in ImportStatusCard for 44 px touch target - Add SkippedFile and skipped count entries to docs/GLOSSARY.md - Add MassImportServiceTest case: ALREADY_EXISTS fires before file I/O when doc is UPLOADED and file is present on disk Co-Authored-By: Claude Sonnet 4.6 --- .../importing/MassImportServiceTest.java | 27 +++++++++++++++++++ docs/GLOSSARY.md | 4 +++ .../admin/system/ImportStatusCard.svelte | 2 +- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java index 26d38679..126a6d74 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/importing/MassImportServiceTest.java @@ -160,6 +160,33 @@ class MassImportServiceTest { assertThat(result).isPresent().contains("ALREADY_EXISTS"); } + // ─── importSingleDocument — already-exists guard fires before file I/O ───── + + @Test + void importSingleDocument_skipsWithAlreadyExists_whenDocumentUploadedAndFileIsPresent(@TempDir Path tempDir) throws Exception { + // Document already exists with status UPLOADED (not PLACEHOLDER). + // A physical PDF file is also present on disk (valid magic bytes). + // Expected: ALREADY_EXISTS is returned and no S3 upload is attempted — + // the guard fires before any file I/O, so no partial processing occurs. + Document existing = Document.builder() + .id(UUID.randomUUID()) + .originalFilename("present.pdf") + .status(DocumentStatus.UPLOADED) + .build(); + when(documentService.findByOriginalFilename("present.pdf")).thenReturn(Optional.of(existing)); + + Path physicalFile = tempDir.resolve("present.pdf"); + byte[] pdfHeader = {0x25, 0x50, 0x44, 0x46, 0x2D}; // %PDF- + Files.write(physicalFile, pdfHeader); + + Optional result = service.importSingleDocument( + minimalCells("present.pdf"), Optional.of(physicalFile.toFile()), "present.pdf", "present"); + + assertThat(result).isPresent().contains("ALREADY_EXISTS"); + verify(s3Client, never()).putObject(any(PutObjectRequest.class), any(RequestBody.class)); + verify(documentService, never()).save(any()); + } + // ─── importSingleDocument — S3 failure surfaced in skippedFiles ────────── @Test diff --git a/docs/GLOSSARY.md b/docs/GLOSSARY.md index 55ffca93..21addb4c 100644 --- a/docs/GLOSSARY.md +++ b/docs/GLOSSARY.md @@ -57,6 +57,10 @@ _See also [Annotation](#annotation-documentannotation)._ **Mass import** — an asynchronous batch process (`MassImportService`) that reads an Excel or ODS file and creates `Person`s, `Tag`s, and `PLACEHOLDER` `Document`s in one shot. Only one import can run at a time (`IMPORT_ALREADY_RUNNING` error if attempted concurrently). +**SkippedFile** (`MassImportService.SkippedFile`) — a file that was presented for import but not processed, recorded with a `filename` and a `reason` code. Possible reasons: `INVALID_PDF_SIGNATURE` (magic-byte validation failed), `S3_UPLOAD_FAILED` (file upload to MinIO/S3 threw an exception), `FILE_READ_ERROR` (the file could not be opened for reading), or `ALREADY_EXISTS` (a document with the same filename already exists in the archive with a status other than `PLACEHOLDER`). + +**skipped count** — the total number of `SkippedFile` entries accumulated during a single import run (`ImportStatus.skipped()`). Shown in the amber warning section of the Import Status Card in the admin UI; a value of zero suppresses the section entirely. + **Transcription queue** — the set of `Document`s and `TranscriptionBlock`s awaiting work, computed on-the-fly from `Document`/`Block` status. Three views: segmentation queue, transcription queue, ready-to-read queue. NOT a persistent entity — no `transcription_queues` table exists. _See also [DocumentStatus lifecycle](#documentstatus-lifecycle)._ diff --git a/frontend/src/routes/admin/system/ImportStatusCard.svelte b/frontend/src/routes/admin/system/ImportStatusCard.svelte index c18fcef3..bb9bce72 100644 --- a/frontend/src/routes/admin/system/ImportStatusCard.svelte +++ b/frontend/src/routes/admin/system/ImportStatusCard.svelte @@ -59,7 +59,7 @@ function reasonLabel(code: string): string {
{#if importStatus.skipped > 0}
- +