marcel/familienarchiv
1
0
Fork 0
Code Issues 108 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

refactor(person): unify fill-blank under preferHuman and clarify rowId trap

Browse Source 
Unify birthYear/deathYear fill-blank logic under an Integer preferHuman overload so
every canonical field uses one self-documenting precedence idiom, and add a guard
test pinning year fill-blank vs human-edit preservation. Add a comment in
PersonTreeImporter.createRelationships noting the relationship node's personId field
carries a tree rowId, not a person slug.

Refs #669

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-05-27 11:03:56 +02:00
parent 5f53c3670f
commit e9ddaed76a
3 changed files with 31 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
backend/src/main/java/org/raddatz/familienarchiv/importing/PersonTreeImporter.java
View File

@@ -88,6 +88,9 @@ public class PersonTreeImporter {
private int createRelationships(JsonNode relationships, Map<String, UUID> idByRowId) { private int createRelationships(JsonNode relationships, Map<String, UUID> idByRowId) {
int created = 0; int created = 0;
for (JsonNode node : relationships) { 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 person = idByRowId.get(text(node, "personId"));
UUID related = idByRowId.get(text(node, "relatedPersonId")); UUID related = idByRowId.get(text(node, "relatedPersonId"));
if (person == null || related == null) { if (person == null || related == null) {

10
backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java
View File

@@ -163,8 +163,8 @@ public class PersonService {
existing.setFirstName(preferHuman(existing.getFirstName(), cmd.firstName())); existing.setFirstName(preferHuman(existing.getFirstName(), cmd.firstName()));
existing.setLastName(preferHuman(existing.getLastName(), cmd.lastName())); existing.setLastName(preferHuman(existing.getLastName(), cmd.lastName()));
existing.setNotes(preferHuman(existing.getNotes(), cmd.notes())); existing.setNotes(preferHuman(existing.getNotes(), cmd.notes()));
if (existing.getBirthYear() == null) existing.setBirthYear(cmd.birthYear()); existing.setBirthYear(preferHuman(existing.getBirthYear(), cmd.birthYear()));
if (existing.getDeathYear() == null) existing.setDeathYear(cmd.deathYear()); existing.setDeathYear(preferHuman(existing.getDeathYear(), cmd.deathYear()));
if (cmd.personType() != null && existing.getPersonType() == PersonType.PERSON) { if (cmd.personType() != null && existing.getPersonType() == PersonType.PERSON) {
existing.setPersonType(cmd.personType()); existing.setPersonType(cmd.personType());
} }
@@ -180,10 +180,16 @@ public class PersonService {
return existing; 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) { private static String preferHuman(String existing, String canonical) {
return (existing == null || existing.isBlank()) ? blankToNull(canonical) : existing; 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) { private static String blankToNull(String s) {
return (s == null || s.isBlank()) ? null : s.trim(); return (s == null || s.isBlank()) ? null : s.trim();
} }

20
backend/src/test/java/org/raddatz/familienarchiv/person/PersonImportUpsertTest.java
View File

@@ -97,6 +97,26 @@ class PersonImportUpsertTest {
assertThat(result.getNotes()).isEqualTo("Nichte von Herbert"); 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 @Test
void upsertBySourceRef_neverFlipsProvisionalBackToTrue_onceHumanConfirmed() { void upsertBySourceRef_neverFlipsProvisionalBackToTrue_onceHumanConfirmed() {
// A human confirmed this provisional importer-created person (provisional -> false). // A human confirmed this provisional importer-created person (provisional -> false).