feat(person): route generation through service write paths (#689)
- fromCanonical writes the imported generation into a new Person row. - mergeCanonical routes existing/canonical generation through the existing preferHuman(Integer, Integer) overload so a human-edited value is never overwritten on re-import (ADR-025). - updatePerson writes generation verbatim from the form DTO so a human can clear it back to null — same shape as birthYear/deathYear. - createPerson(PersonUpdateDTO) writes generation so /persons/new flow doesn't silently drop a selected G value on create. Pinned with five tests covering the four write paths plus the documenting test that captures preferHuman's known limitation (explicit human null is overwritten by a non-null canonical value — same as birthYear/deathYear, deferred to a future helper rework if it ever produces a user-visible bug). Refs #689 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -177,6 +177,7 @@ public class PersonService {
|
|||||||
.notes(blankToNull(cmd.notes()))
|
.notes(blankToNull(cmd.notes()))
|
||||||
.birthYear(cmd.birthYear())
|
.birthYear(cmd.birthYear())
|
||||||
.deathYear(cmd.deathYear())
|
.deathYear(cmd.deathYear())
|
||||||
|
.generation(cmd.generation())
|
||||||
.familyMember(cmd.familyMember())
|
.familyMember(cmd.familyMember())
|
||||||
.personType(cmd.personType() == null ? PersonType.PERSON : cmd.personType())
|
.personType(cmd.personType() == null ? PersonType.PERSON : cmd.personType())
|
||||||
.provisional(cmd.provisional())
|
.provisional(cmd.provisional())
|
||||||
@@ -200,6 +201,7 @@ public class PersonService {
|
|||||||
existing.setNotes(preferHuman(existing.getNotes(), cmd.notes()));
|
existing.setNotes(preferHuman(existing.getNotes(), cmd.notes()));
|
||||||
existing.setBirthYear(preferHuman(existing.getBirthYear(), cmd.birthYear()));
|
existing.setBirthYear(preferHuman(existing.getBirthYear(), cmd.birthYear()));
|
||||||
existing.setDeathYear(preferHuman(existing.getDeathYear(), cmd.deathYear()));
|
existing.setDeathYear(preferHuman(existing.getDeathYear(), cmd.deathYear()));
|
||||||
|
existing.setGeneration(preferHuman(existing.getGeneration(), cmd.generation()));
|
||||||
if (cmd.personType() != null && existing.getPersonType() == PersonType.PERSON) {
|
if (cmd.personType() != null && existing.getPersonType() == PersonType.PERSON) {
|
||||||
existing.setPersonType(cmd.personType());
|
existing.setPersonType(cmd.personType());
|
||||||
}
|
}
|
||||||
@@ -254,6 +256,7 @@ public class PersonService {
|
|||||||
.notes(dto.getNotes() == null || dto.getNotes().isBlank() ? null : dto.getNotes().trim())
|
.notes(dto.getNotes() == null || dto.getNotes().isBlank() ? null : dto.getNotes().trim())
|
||||||
.birthYear(dto.getBirthYear())
|
.birthYear(dto.getBirthYear())
|
||||||
.deathYear(dto.getDeathYear())
|
.deathYear(dto.getDeathYear())
|
||||||
|
.generation(dto.getGeneration())
|
||||||
.build();
|
.build();
|
||||||
return personRepository.save(person);
|
return personRepository.save(person);
|
||||||
}
|
}
|
||||||
@@ -286,6 +289,9 @@ public class PersonService {
|
|||||||
person.setNotes(dto.getNotes() == null || dto.getNotes().isBlank() ? null : dto.getNotes().trim());
|
person.setNotes(dto.getNotes() == null || dto.getNotes().isBlank() ? null : dto.getNotes().trim());
|
||||||
person.setBirthYear(dto.getBirthYear());
|
person.setBirthYear(dto.getBirthYear());
|
||||||
person.setDeathYear(dto.getDeathYear());
|
person.setDeathYear(dto.getDeathYear());
|
||||||
|
// Form path: a human can clear generation back to null. Unlike the importer
|
||||||
|
// which routes through preferHuman, we write the DTO value verbatim.
|
||||||
|
person.setGeneration(dto.getGeneration());
|
||||||
return personRepository.save(person);
|
return personRepository.save(person);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -148,4 +148,55 @@ class PersonImportUpsertTest {
|
|||||||
|
|
||||||
assertThat(result.isProvisional()).isTrue();
|
assertThat(result.isProvisional()).isTrue();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ─── generation (#689) ─────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void upsertBySourceRef_writesGeneration_onFirstImport() {
|
||||||
|
when(personRepository.findBySourceRef("herbert-cram")).thenReturn(Optional.empty());
|
||||||
|
when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||||
|
|
||||||
|
PersonUpsertCommand cmd = PersonUpsertCommand.builder()
|
||||||
|
.sourceRef("herbert-cram").firstName("Herbert").lastName("Cram")
|
||||||
|
.generation(3).personType(PersonType.PERSON).provisional(false).build();
|
||||||
|
|
||||||
|
Person result = personService.upsertBySourceRef(cmd);
|
||||||
|
|
||||||
|
assertThat(result.getGeneration()).isEqualTo(3);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void upsertBySourceRef_preservesHumanEditedGeneration_onReimport() {
|
||||||
|
Person humanEdited = Person.builder()
|
||||||
|
.id(UUID.randomUUID()).sourceRef("herbert-cram")
|
||||||
|
.firstName("Herbert").lastName("Cram").generation(4).build();
|
||||||
|
when(personRepository.findBySourceRef("herbert-cram")).thenReturn(Optional.of(humanEdited));
|
||||||
|
when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||||
|
|
||||||
|
PersonUpsertCommand cmd = PersonUpsertCommand.builder()
|
||||||
|
.sourceRef("herbert-cram").firstName("Herbert").lastName("Cram")
|
||||||
|
.generation(2).personType(PersonType.PERSON).provisional(false).build();
|
||||||
|
|
||||||
|
Person result = personService.upsertBySourceRef(cmd);
|
||||||
|
|
||||||
|
assertThat(result.getGeneration()).isEqualTo(4);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void mergeCanonical_overwrites_human_null_with_canonical_value_documenting_known_limitation() {
|
||||||
|
// If preferHuman gains explicit-null-vs-unset semantics, delete this test (see issue #689).
|
||||||
|
Person existing = Person.builder()
|
||||||
|
.id(UUID.randomUUID()).sourceRef("herbert-cram")
|
||||||
|
.firstName("Herbert").lastName("Cram").generation(null).build();
|
||||||
|
when(personRepository.findBySourceRef("herbert-cram")).thenReturn(Optional.of(existing));
|
||||||
|
when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||||
|
|
||||||
|
PersonUpsertCommand cmd = PersonUpsertCommand.builder()
|
||||||
|
.sourceRef("herbert-cram").firstName("Herbert").lastName("Cram")
|
||||||
|
.generation(3).personType(PersonType.PERSON).provisional(false).build();
|
||||||
|
|
||||||
|
Person result = personService.upsertBySourceRef(cmd);
|
||||||
|
|
||||||
|
assertThat(result.getGeneration()).isEqualTo(3);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -261,6 +261,54 @@ class PersonServiceTest {
|
|||||||
.isEqualTo(400);
|
.isEqualTo(400);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void createPerson_dto_persistsGeneration() {
|
||||||
|
when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||||
|
|
||||||
|
PersonUpdateDTO dto = new PersonUpdateDTO();
|
||||||
|
dto.setFirstName("Hans"); dto.setLastName("Raddatz");
|
||||||
|
dto.setPersonType(PersonType.PERSON); dto.setGeneration(3);
|
||||||
|
|
||||||
|
Person result = personService.createPerson(dto);
|
||||||
|
|
||||||
|
assertThat(result.getGeneration()).isEqualTo(3);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void updatePerson_writesGeneration_includingExplicitNullClear() {
|
||||||
|
// The form path is the only place a human can clear generation back to null.
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
Person existing = Person.builder().id(id).firstName("Hans").lastName("Raddatz")
|
||||||
|
.personType(PersonType.PERSON).generation(3).build();
|
||||||
|
when(personRepository.findById(id)).thenReturn(Optional.of(existing));
|
||||||
|
when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||||
|
|
||||||
|
PersonUpdateDTO dto = new PersonUpdateDTO();
|
||||||
|
dto.setFirstName("Hans"); dto.setLastName("Raddatz");
|
||||||
|
dto.setPersonType(PersonType.PERSON); dto.setGeneration(null);
|
||||||
|
|
||||||
|
Person result = personService.updatePerson(id, dto);
|
||||||
|
|
||||||
|
assertThat(result.getGeneration()).isNull();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void updatePerson_writesGeneration_whenSet() {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
Person existing = Person.builder().id(id).firstName("Hans").lastName("Raddatz")
|
||||||
|
.personType(PersonType.PERSON).build();
|
||||||
|
when(personRepository.findById(id)).thenReturn(Optional.of(existing));
|
||||||
|
when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||||
|
|
||||||
|
PersonUpdateDTO dto = new PersonUpdateDTO();
|
||||||
|
dto.setFirstName("Hans"); dto.setLastName("Raddatz");
|
||||||
|
dto.setPersonType(PersonType.PERSON); dto.setGeneration(2);
|
||||||
|
|
||||||
|
Person result = personService.updatePerson(id, dto);
|
||||||
|
|
||||||
|
assertThat(result.getGeneration()).isEqualTo(2);
|
||||||
|
}
|
||||||
|
|
||||||
// ─── updatePerson (personType) ───────────────────────────────────────────
|
// ─── updatePerson (personType) ───────────────────────────────────────────
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
Reference in New Issue
Block a user