diff --git a/backend/src/main/java/org/raddatz/familienarchiv/importing/PersonTreeImporter.java b/backend/src/main/java/org/raddatz/familienarchiv/importing/PersonTreeImporter.java index 4cf1f55f..26ae0dcd 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/importing/PersonTreeImporter.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/importing/PersonTreeImporter.java @@ -88,6 +88,9 @@ public class PersonTreeImporter { private int createRelationships(JsonNode relationships, Map idByRowId) { int created = 0; for (JsonNode node : relationships) { + // Trap: a relationship node's personId / relatedPersonId fields carry the tree's + // local rowId (e.g. "row_a"), NOT a person slug. They are resolved through + // idByRowId to the upserted person's UUID. UUID person = idByRowId.get(text(node, "personId")); UUID related = idByRowId.get(text(node, "relatedPersonId")); if (person == null || related == null) { diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java index 82de40b5..d2e894bf 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java @@ -163,8 +163,8 @@ public class PersonService { existing.setFirstName(preferHuman(existing.getFirstName(), cmd.firstName())); existing.setLastName(preferHuman(existing.getLastName(), cmd.lastName())); existing.setNotes(preferHuman(existing.getNotes(), cmd.notes())); - if (existing.getBirthYear() == null) existing.setBirthYear(cmd.birthYear()); - if (existing.getDeathYear() == null) existing.setDeathYear(cmd.deathYear()); + existing.setBirthYear(preferHuman(existing.getBirthYear(), cmd.birthYear())); + existing.setDeathYear(preferHuman(existing.getDeathYear(), cmd.deathYear())); if (cmd.personType() != null && existing.getPersonType() == PersonType.PERSON) { existing.setPersonType(cmd.personType()); } @@ -180,10 +180,16 @@ public class PersonService { return existing; } + // preferHuman keeps an existing human-entered value and only falls back to the canonical + // value when the existing one is absent — the single idiom for every fill-blank field. private static String preferHuman(String existing, String canonical) { return (existing == null || existing.isBlank()) ? blankToNull(canonical) : existing; } + private static Integer preferHuman(Integer existing, Integer canonical) { + return existing != null ? existing : canonical; + } + private static String blankToNull(String s) { return (s == null || s.isBlank()) ? null : s.trim(); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonImportUpsertTest.java b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonImportUpsertTest.java index 75a6381a..c8b81b2b 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonImportUpsertTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonImportUpsertTest.java @@ -97,6 +97,26 @@ class PersonImportUpsertTest { assertThat(result.getNotes()).isEqualTo("Nichte von Herbert"); } + @Test + void upsertBySourceRef_fillsBlankYears_butPreservesHumanEditedYears_onReimport() { + // Existing has a human-set birthYear and a blank deathYear. + Person existing = Person.builder() + .id(UUID.randomUUID()).sourceRef("clara-cram") + .lastName("Cram").birthYear(1890).deathYear(null).build(); + when(personRepository.findBySourceRef("clara-cram")).thenReturn(Optional.of(existing)); + when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + PersonUpsertCommand cmd = PersonUpsertCommand.builder() + .sourceRef("clara-cram").lastName("Cram") + .birthYear(1888).deathYear(1965) + .personType(PersonType.PERSON).provisional(false).build(); + + Person result = personService.upsertBySourceRef(cmd); + + assertThat(result.getBirthYear()).isEqualTo(1890); // human value kept + assertThat(result.getDeathYear()).isEqualTo(1965); // blank filled from canonical + } + @Test void upsertBySourceRef_neverFlipsProvisionalBackToTrue_onceHumanConfirmed() { // A human confirmed this provisional importer-created person (provisional -> false).