From 07300aeff764bca04ea700e00f7656097aa18146 Mon Sep 17 00:00:00 2001 From: Marcel Date: Thu, 28 May 2026 09:15:37 +0200 Subject: [PATCH] fix(person): flip family_member on both endpoints when a family-graph relationship is added MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The canonical importer creates persons via PersonRegisterImporter first (no family_member set) and then upserts them via PersonTreeImporter, but mergeCanonical never propagates family_member to existing persons — so persons with imported relationships ended up flagged family_member=false and never appeared in /api/persons family filters or the family-network view. RelationshipService is documented as the owner of the family_member flag, so the fix lives there: addRelationship now sets family_member=true on both endpoints whenever the relation type is PARENT_OF / SPOUSE_OF / SIBLING_OF (the same set getFamilyNetwork filters by). Non-family types (FRIEND/COLLEAGUE/EMPLOYER/DOCTOR/NEIGHBOR/OTHER) leave the flag alone — a family doctor isn't a family member. Extracted the type list as a FAMILY_RELATION_TYPES constant and reused it in getFamilyNetwork for a single source of truth. Co-Authored-By: Claude Opus 4.7 --- .../relationship/RelationshipService.java | 18 +++++++- .../relationship/RelationshipServiceTest.java | 46 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/relationship/RelationshipService.java b/backend/src/main/java/org/raddatz/familienarchiv/person/relationship/RelationshipService.java index 032c1263..d813b8e8 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/relationship/RelationshipService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/relationship/RelationshipService.java @@ -31,6 +31,12 @@ import java.util.UUID; @RequiredArgsConstructor public class RelationshipService { + // Single source of truth for which relationship types are part of the family graph. + // Consulted by addRelationship (to set family_member on both endpoints) and by + // getFamilyNetwork (to filter the edges returned). FRIEND/COLLEAGUE/etc. are excluded. + private static final List FAMILY_RELATION_TYPES = + List.of(RelationType.PARENT_OF, RelationType.SPOUSE_OF, RelationType.SIBLING_OF); + private final PersonRelationshipRepository relationshipRepository; private final PersonService personService; private final RelationshipInferenceService inferenceService; @@ -64,7 +70,7 @@ public class RelationshipService { } List familyEdges = relationshipRepository.findAllByRelationTypeIn( - List.of(RelationType.PARENT_OF, RelationType.SPOUSE_OF, RelationType.SIBLING_OF)); + FAMILY_RELATION_TYPES); List edges = new ArrayList<>(); for (PersonRelationship r : familyEdges) { @@ -105,15 +111,23 @@ public class RelationshipService { .notes(blankToNull(dto.notes())) .build(); + PersonRelationship saved; try { // saveAndFlush so the unique_rel constraint violates synchronously and is // caught here, not at commit time outside the @Transactional boundary. - return toDTO(relationshipRepository.saveAndFlush(rel)); + saved = relationshipRepository.saveAndFlush(rel); } catch (DataIntegrityViolationException e) { throw DomainException.conflict( ErrorCode.DUPLICATE_RELATIONSHIP, "Relationship already exists for (" + personId + ", " + relatedPerson.getId() + ", " + dto.relationType() + ")"); } + // Family-graph edges imply both endpoints are family members. Idempotent: the + // setter is a no-op when the person is already flagged, so re-imports stay clean. + if (FAMILY_RELATION_TYPES.contains(dto.relationType())) { + personService.setFamilyMember(person.getId(), true); + personService.setFamilyMember(relatedPerson.getId(), true); + } + return toDTO(saved); } @Transactional diff --git a/backend/src/test/java/org/raddatz/familienarchiv/person/relationship/RelationshipServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/person/relationship/RelationshipServiceTest.java index 0a0b963c..c8d1faf6 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/person/relationship/RelationshipServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/person/relationship/RelationshipServiceTest.java @@ -23,6 +23,8 @@ import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -148,6 +150,50 @@ class RelationshipServiceTest { assertThat(result.notes()).isEqualTo("first born"); } + @Test + void addRelationship_marks_both_endpoints_as_family_member_when_type_is_family() { + // Creating a family-graph edge (PARENT_OF / SPOUSE_OF / SIBLING_OF) must mark both + // endpoints as family members so they appear in findAllFamilyMembers and the network. + // This is what makes the canonical importer's relationships actually show up in the UI. + when(personService.getById(alice.getId())).thenReturn(alice); + when(personService.getById(bob.getId())).thenReturn(bob); + when(relationshipRepository.existsByPersonIdAndRelatedPersonIdAndRelationType( + bob.getId(), alice.getId(), RelationType.PARENT_OF)).thenReturn(false); + when(relationshipRepository.saveAndFlush(any())).thenAnswer(inv -> { + PersonRelationship r = inv.getArgument(0); + r.setId(UUID.randomUUID()); + r.setCreatedAt(Instant.now()); + return r; + }); + + var dto = new CreateRelationshipRequest(bob.getId(), RelationType.PARENT_OF, null, null, null); + service.addRelationship(alice.getId(), dto); + + verify(personService).setFamilyMember(alice.getId(), true); + verify(personService).setFamilyMember(bob.getId(), true); + } + + @Test + void addRelationship_does_not_flip_family_member_for_non_family_type() { + // FRIEND / COLLEAGUE / EMPLOYER / DOCTOR / NEIGHBOR / OTHER are NOT family-graph + // edges (see getFamilyNetwork's filter), so addRelationship must leave family_member + // alone — a doctor of the family is not a family member. + when(personService.getById(alice.getId())).thenReturn(alice); + when(personService.getById(bob.getId())).thenReturn(bob); + when(relationshipRepository.saveAndFlush(any())).thenAnswer(inv -> { + PersonRelationship r = inv.getArgument(0); + r.setId(UUID.randomUUID()); + r.setCreatedAt(Instant.now()); + return r; + }); + + var dto = new CreateRelationshipRequest(bob.getId(), RelationType.FRIEND, null, null, null); + service.addRelationship(alice.getId(), dto); + + verify(personService, never()).setFamilyMember(eq(alice.getId()), anyBoolean()); + verify(personService, never()).setFamilyMember(eq(bob.getId()), anyBoolean()); + } + @Test void deleteRelationship_succeeds_when_viewpoint_is_object() { UUID relId = UUID.randomUUID();