From d078ad8224da02680c26869bae1287d990c7f0a1 Mon Sep 17 00:00:00 2001 From: Marcel Date: Thu, 26 Mar 2026 11:31:31 +0100 Subject: [PATCH] feat(upload): warn on duplicate filename with link to existing document - storeDocument now returns StoreResult(document, isNew) to distinguish new uploads from updates to existing documents - QuickUploadResult gains an `updated` list alongside `created` - Frontend shows an amber warning with a "View document" link for duplicates instead of silently re-uploading and leaving the user confused Co-Authored-By: Claude Sonnet 4.6 --- .../controller/DocumentController.java | 14 +++++--- .../service/DocumentService.java | 11 ++++--- .../controller/DocumentControllerTest.java | 23 +++++++++++-- .../service/DocumentServiceTest.java | 33 ++++++++++++++++++- frontend/messages/de.json | 2 ++ frontend/messages/en.json | 2 ++ frontend/messages/es.json | 2 ++ frontend/src/routes/+page.svelte | 23 +++++++++++-- 8 files changed, 96 insertions(+), 14 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java index b3b6118c..8cafba7a 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java @@ -121,17 +121,18 @@ public class DocumentController { "application/pdf", "image/jpeg", "image/png", "image/tiff"); public record UploadError(String filename, String code) {} - public record QuickUploadResult(List created, List errors) {} + public record QuickUploadResult(List created, List updated, List errors) {} @PostMapping(value = "/quick-upload", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) @RequirePermission(Permission.WRITE_ALL) public QuickUploadResult quickUpload( @RequestPart(value = "files", required = false) List files) { List created = new ArrayList<>(); + List updated = new ArrayList<>(); List errors = new ArrayList<>(); if (files == null || files.isEmpty()) { - return new QuickUploadResult(created, errors); + return new QuickUploadResult(created, updated, errors); } for (MultipartFile file : files) { @@ -140,14 +141,19 @@ public class DocumentController { continue; } try { - created.add(documentService.storeDocument(file)); + DocumentService.StoreResult result = documentService.storeDocument(file); + if (result.isNew()) { + created.add(result.document()); + } else { + updated.add(result.document()); + } } catch (Exception e) { errors.add(new UploadError(file.getOriginalFilename(), "FILE_UPLOAD_FAILED")); log.warn("Quick upload failed for file {}: {}", file.getOriginalFilename(), e.getMessage()); } } - return new QuickUploadResult(created, errors); + return new QuickUploadResult(created, updated, errors); } @GetMapping("/search") diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java index 28213d73..cd878814 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java @@ -42,18 +42,21 @@ public class DocumentService { private final DocumentVersionService documentVersionService; private final AnnotationService annotationService; + public record StoreResult(Document document, boolean isNew) {} + /** * Lädt eine Datei hoch. * - Prüft, ob ein Eintrag (aus Excel) schon existiert. - * - Wenn JA: Aktualisiert Status und verknüpft Datei. - * - Wenn NEIN: Erstellt neuen Eintrag (wartet auf Metadaten). + * - Wenn JA: Aktualisiert Status und verknüpft Datei — isNew = false. + * - Wenn NEIN: Erstellt neuen Eintrag — isNew = true. */ @Transactional - public Document storeDocument(MultipartFile file) throws IOException { + public StoreResult storeDocument(MultipartFile file) throws IOException { String originalFilename = file.getOriginalFilename(); // 1. Check for existing record (findFirst to survive duplicate filenames in the DB) Optional existingDoc = documentRepository.findFirstByOriginalFilename(originalFilename); + boolean isNew = existingDoc.isEmpty(); Document document; if (existingDoc.isPresent()) { @@ -77,7 +80,7 @@ public class DocumentService { document.setStatus(DocumentStatus.UPLOADED); } - return documentRepository.save(document); + return new StoreResult(documentRepository.save(document), isNew); } @Transactional diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java index 23a7e1b7..f4f6c439 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java @@ -167,7 +167,8 @@ class DocumentControllerTest { void quickUpload_returns200_withValidPdfFile() throws Exception { Document doc = Document.builder() .id(UUID.randomUUID()).title("scan001").originalFilename("scan001.pdf").build(); - when(documentService.storeDocument(any())).thenReturn(doc); + when(documentService.storeDocument(any())) + .thenReturn(new DocumentService.StoreResult(doc, true)); org.springframework.mock.web.MockMultipartFile file = new org.springframework.mock.web.MockMultipartFile("files", "scan001.pdf", "application/pdf", new byte[]{1}); @@ -175,7 +176,25 @@ class DocumentControllerTest { mockMvc.perform(multipart("/api/documents/quick-upload").file(file)) .andExpect(status().isOk()) .andExpect(jsonPath("$.created[0].title").value("scan001")) - .andExpect(jsonPath("$.errors").isArray()) + .andExpect(jsonPath("$.updated").isEmpty()) + .andExpect(jsonPath("$.errors").isEmpty()); + } + + @Test + @WithMockUser(authorities = "WRITE_ALL") + void quickUpload_placesDocumentInUpdated_whenFilenameAlreadyExists() throws Exception { + Document existing = Document.builder() + .id(UUID.randomUUID()).title("Alter Brief").originalFilename("scan001.pdf").build(); + when(documentService.storeDocument(any())) + .thenReturn(new DocumentService.StoreResult(existing, false)); + + org.springframework.mock.web.MockMultipartFile file = + new org.springframework.mock.web.MockMultipartFile("files", "scan001.pdf", "application/pdf", new byte[]{1}); + + mockMvc.perform(multipart("/api/documents/quick-upload").file(file)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.created").isEmpty()) + .andExpect(jsonPath("$.updated[0].title").value("Alter Brief")) .andExpect(jsonPath("$.errors").isEmpty()); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java index 367a86a1..984be862 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java @@ -262,7 +262,7 @@ class DocumentServiceTest { FileService.UploadResult uploadResult = new FileService.UploadResult("documents/uuid_scan001.pdf", "abc123"); Document placeholder = Document.builder() .id(UUID.randomUUID()).title("Brief an Oma").originalFilename("scan001.pdf") - .status(org.raddatz.familienarchiv.model.DocumentStatus.PLACEHOLDER).build(); + .status(DocumentStatus.PLACEHOLDER).build(); when(documentRepository.findFirstByOriginalFilename("scan001.pdf")).thenReturn(Optional.of(placeholder)); when(documentRepository.save(any())).thenReturn(placeholder); @@ -273,6 +273,37 @@ class DocumentServiceTest { assertThat(placeholder.getTitle()).isEqualTo("Brief an Oma"); } + @Test + void storeDocument_marksResultAsNew_whenNoExistingDocument() throws Exception { + org.springframework.mock.web.MockMultipartFile file = + new org.springframework.mock.web.MockMultipartFile("file", "new.pdf", "application/pdf", new byte[]{1}); + Document saved = Document.builder().id(UUID.randomUUID()).originalFilename("new.pdf").build(); + + when(documentRepository.findFirstByOriginalFilename("new.pdf")).thenReturn(Optional.empty()); + when(documentRepository.save(any())).thenReturn(saved); + when(fileService.uploadFile(any(), any())).thenReturn(new FileService.UploadResult("documents/new.pdf", "hash")); + + DocumentService.StoreResult result = documentService.storeDocument(file); + + assertThat(result.isNew()).isTrue(); + } + + @Test + void storeDocument_marksResultAsNotNew_whenDocumentWithSameFilenameExists() throws Exception { + org.springframework.mock.web.MockMultipartFile file = + new org.springframework.mock.web.MockMultipartFile("file", "existing.pdf", "application/pdf", new byte[]{1}); + Document existing = Document.builder().id(UUID.randomUUID()).originalFilename("existing.pdf") + .status(DocumentStatus.UPLOADED).build(); + + when(documentRepository.findFirstByOriginalFilename("existing.pdf")).thenReturn(Optional.of(existing)); + when(documentRepository.save(any())).thenReturn(existing); + when(fileService.uploadFile(any(), any())).thenReturn(new FileService.UploadResult("documents/existing.pdf", "hash")); + + DocumentService.StoreResult result = documentService.storeDocument(file); + + assertThat(result.isNew()).isFalse(); + } + // ─── backfillFileHashes ─────────────────────────────────────────────────── @Test diff --git a/frontend/messages/de.json b/frontend/messages/de.json index d30fccde..c033a147 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -271,6 +271,8 @@ "upload_drop_hint": "Dateien ablegen oder auswählen", "upload_accepted_types": "PDF, JPEG, PNG, TIFF", "upload_success": "{count} Dokument(e) erstellt", + "upload_duplicate": "{filename} existiert bereits —", + "upload_duplicate_link": "Zum Dokument", "upload_invalid_type": "{filename}: Dateiformat nicht unterstützt", "upload_error": "Fehler beim Hochladen von {filename}" } diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 3f826d89..f9d53545 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -271,6 +271,8 @@ "upload_drop_hint": "Drop files or click to select", "upload_accepted_types": "PDF, JPEG, PNG, TIFF", "upload_success": "{count} document(s) created", + "upload_duplicate": "{filename} already exists —", + "upload_duplicate_link": "View document", "upload_invalid_type": "{filename}: unsupported file format", "upload_error": "Error uploading {filename}" } diff --git a/frontend/messages/es.json b/frontend/messages/es.json index 0a93f594..958fb02e 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -271,6 +271,8 @@ "upload_drop_hint": "Soltar archivos o hacer clic para seleccionar", "upload_accepted_types": "PDF, JPEG, PNG, TIFF", "upload_success": "{count} documento(s) creado(s)", + "upload_duplicate": "{filename} ya existe —", + "upload_duplicate_link": "Ver documento", "upload_invalid_type": "{filename}: formato de archivo no admitido", "upload_error": "Error al subir {filename}" } diff --git a/frontend/src/routes/+page.svelte b/frontend/src/routes/+page.svelte index 68e02669..777abb48 100644 --- a/frontend/src/routes/+page.svelte +++ b/frontend/src/routes/+page.svelte @@ -25,7 +25,7 @@ let isDragging = $state(false); let windowDragging = $state(false); let dragCounter = 0; let isUploading = $state(false); -let uploadMessages = $state<{ text: string; isError: boolean }[]>([]); +let uploadMessages = $state<{ text: string; isError: boolean; link?: string }[]>([]); let fileInput: HTMLInputElement; let searchTimer: ReturnType; @@ -67,7 +67,7 @@ async function handleFileSelect(e: Event) { async function uploadFiles(files: File[]) { if (files.length === 0) return; - const messages: { text: string; isError: boolean }[] = []; + const messages: { text: string; isError: boolean; link?: string }[] = []; // Client-side type validation const valid: File[] = []; @@ -101,6 +101,13 @@ async function uploadFiles(files: File[]) { if (result.created?.length > 0) { messages.push({ text: m.upload_success({ count: result.created.length }), isError: false }); } + for (const doc of result.updated ?? []) { + messages.push({ + text: m.upload_duplicate({ filename: doc.originalFilename }), + isError: false, + link: `/documents/${doc.id}` + }); + } for (const err of result.errors ?? []) { messages.push({ text: `${err.filename}: ${getErrorMessage(err.code)}`, @@ -374,8 +381,18 @@ $effect(() => { {#if uploadMessages.length > 0}
{#each uploadMessages as msg, i (i)} -

+

{msg.text} + {#if msg.link} + {m.upload_duplicate_link()} + {/if}

{/each}