From 535594378a8f22af748f5b7be3b08207337bea1f Mon Sep 17 00:00:00 2001 From: Marcel Date: Thu, 28 May 2026 10:26:28 +0200 Subject: [PATCH] fix(importing): use receiver_names for provisional person display name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveReceivers passed the slug as both `sourceRef` AND `lastName`, so an unresolved receiver "smith-john" became a provisional Person with lastName="smith-john" — a regression of the existing senderName→Person contract. Fix: zip the parallel `receiver_person_ids` and `receiver_names` columns by position (the normalizer emits them 1:1 like sender_person_id/sender_name). When the names list is shorter than the slugs list, fall back to slug-as-name for the missing entries. Co-Authored-By: Claude Opus 4.7 --- .../importing/DocumentImporter.java | 16 ++++-- .../importing/DocumentImporterTest.java | 54 +++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/importing/DocumentImporter.java b/backend/src/main/java/org/raddatz/familienarchiv/importing/DocumentImporter.java index 5e6e8a6f..528b9aa2 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/importing/DocumentImporter.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/importing/DocumentImporter.java @@ -211,7 +211,7 @@ public class DocumentImporter { // shrunk set on re-import prunes stale links rather than accumulating them. The // "preserve human edits" rule does NOT extend to these collections. private void applyAuthoritativeAssociations(Document doc, CanonicalSheetReader.Row row) { - Set receivers = resolveReceivers(row.get("receiver_person_ids")); + Set receivers = resolveReceivers(row.get("receiver_person_ids"), row.get("receiver_names")); doc.getReceivers().clear(); doc.getReceivers().addAll(receivers); attachTag(doc, row.get("tags")); @@ -256,10 +256,18 @@ public class DocumentImporter { return resolvePerson(slug, rawName); } - private Set resolveReceivers(String slugs) { + // Zips the parallel `receiver_person_ids` and `receiver_names` columns by position so an + // unresolved receiver becomes a provisional Person whose lastName is the human name from + // `receiver_names`, not the slug. If the names list is shorter than the slugs list (rare — + // canonical data zips them 1:1), missing entries fall back to slug-as-name. + private Set resolveReceivers(String slugs, String names) { + List slugList = CanonicalSheetReader.splitList(slugs); + List nameList = CanonicalSheetReader.splitList(names); Set receivers = new LinkedHashSet<>(); - for (String slug : CanonicalSheetReader.splitList(slugs)) { - receivers.add(resolvePerson(slug, slug)); + for (int i = 0; i < slugList.size(); i++) { + String slug = slugList.get(i); + String name = i < nameList.size() ? nameList.get(i) : slug; + receivers.add(resolvePerson(slug, name)); } return receivers; } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/importing/DocumentImporterTest.java b/backend/src/test/java/org/raddatz/familienarchiv/importing/DocumentImporterTest.java index f136b0e0..c4e2ac77 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/importing/DocumentImporterTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/importing/DocumentImporterTest.java @@ -424,6 +424,60 @@ class DocumentImporterTest { && "Herbert Cram|Clara".equals(d.getReceiverText()))); } + @Test + void load_provisionalReceiverUsesHumanNameFromReceiverNames_notSlug(@TempDir Path tempDir) throws Exception { + // Regression: resolveReceivers used to pass the slug as both `sourceRef` AND `lastName`, + // so an unresolved receiver "smith-john" became a provisional Person with + // lastName="smith-john". The fix consumes the parallel `receiver_names` column. + ReflectionTestUtils.setField(importer, "importDir", tempDir.toString()); + Person provisional = Person.builder().id(UUID.randomUUID()).sourceRef("smith-john") + .lastName("John Smith").provisional(true).build(); + when(documentService.findByOriginalFilename("W-0050")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(personService.findBySourceRef("smith-john")).thenReturn(Optional.empty()); + when(personService.upsertBySourceRef(any())).thenReturn(provisional); + Path xlsx = writeDocs(tempDir, docRow("W-0050", "", "", + "smith-john", "John Smith", "", "", "", "")); + + importer.load(xlsx.toFile()); + + org.mockito.ArgumentCaptor captor = + org.mockito.ArgumentCaptor.forClass(PersonUpsertCommand.class); + verify(personService).upsertBySourceRef(captor.capture()); + assertThat(captor.getValue().sourceRef()).isEqualTo("smith-john"); + assertThat(captor.getValue().lastName()).isEqualTo("John Smith"); + assertThat(captor.getValue().provisional()).isTrue(); + } + + @Test + void load_provisionalReceiverFallsBackToSlug_whenNamesListShorterThanSlugs(@TempDir Path tempDir) throws Exception { + // Parallel-list zip: if the names list is shorter than the slugs list, slugs without a + // matching name fall back to slug as the display name. This is the "missing name" case + // (rare in canonical data but the contract must define it). + ReflectionTestUtils.setField(importer, "importDir", tempDir.toString()); + Person alice = Person.builder().id(UUID.randomUUID()).sourceRef("alice-jones") + .lastName("Alice Jones").provisional(true).build(); + Person bob = Person.builder().id(UUID.randomUUID()).sourceRef("bob-roe") + .lastName("bob-roe").provisional(true).build(); + when(documentService.findByOriginalFilename("W-0051")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(personService.findBySourceRef("alice-jones")).thenReturn(Optional.empty()); + when(personService.findBySourceRef("bob-roe")).thenReturn(Optional.empty()); + when(personService.upsertBySourceRef(any())).thenReturn(alice).thenReturn(bob); + Path xlsx = writeDocs(tempDir, docRow("W-0051", "", "", + "alice-jones|bob-roe", "Alice Jones", "", "", "", "")); + + importer.load(xlsx.toFile()); + + org.mockito.ArgumentCaptor captor = + org.mockito.ArgumentCaptor.forClass(PersonUpsertCommand.class); + verify(personService, org.mockito.Mockito.times(2)).upsertBySourceRef(captor.capture()); + assertThat(captor.getAllValues()).extracting(PersonUpsertCommand::sourceRef) + .containsExactly("alice-jones", "bob-roe"); + assertThat(captor.getAllValues()).extracting(PersonUpsertCommand::lastName) + .containsExactly("Alice Jones", "bob-roe"); + } + // ─── clean date values parse without semantic logic ────────────────────────────── @Test