From 949421a0760649d0da59f15166e5efc5f736f279 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 9 Jun 2026 17:18:28 +0200 Subject: [PATCH] fix(geschichten): restore documentId filter via journey_items EXISTS subquery The PR removed the documentId filter from list() along with the old Geschichte.documents ManyToMany, but the document-detail page and its frontend server still query GET /api/geschichten?documentId= to show related stories. Without the filter the endpoint silently returned every published story. Restores the filter through a JPQL EXISTS check on journey_items so only journeys that include the given document are returned. Co-Authored-By: Claude Sonnet 4.6 --- .../geschichte/GeschichteController.java | 2 + .../geschichte/GeschichteRepository.java | 6 ++- .../geschichte/GeschichteService.java | 4 +- .../geschichte/GeschichteControllerTest.java | 10 ++--- .../GeschichteListProjectionTest.java | 16 ++++---- .../GeschichteServiceIntegrationTest.java | 18 ++++---- .../geschichte/GeschichteServiceTest.java | 41 ++++++++++++------- 7 files changed, 58 insertions(+), 39 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java index 4eed4e16..73055218 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java @@ -37,10 +37,12 @@ public class GeschichteController { public List list( @RequestParam(required = false) GeschichteStatus status, @RequestParam(name = "personId", required = false) List personIds, + @RequestParam(required = false) UUID documentId, @RequestParam(required = false, defaultValue = "50") int limit) { return geschichteService.list( status, personIds == null ? List.of() : personIds, + documentId, limit); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteRepository.java index f0947218..98c4bf13 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteRepository.java @@ -33,11 +33,15 @@ public interface GeschichteRepository extends JpaRepository, J (SELECT COUNT(DISTINCT p.id) FROM Geschichte g2 JOIN g2.persons p WHERE g2.id = g.id AND p.id IN :personIds) = :personCount) + AND (:documentId IS NULL OR + EXISTS (SELECT 1 FROM JourneyItem ji + WHERE ji.geschichte = g AND ji.document.id = :documentId)) ORDER BY COALESCE(g.publishedAt, g.updatedAt) DESC """) List findSummaries( @Param("effectiveStatus") GeschichteStatus effectiveStatus, @Param("authorId") UUID authorId, @Param("personIds") Collection personIds, - @Param("personCount") long personCount); + @Param("personCount") long personCount, + @Param("documentId") UUID documentId); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java index 2a0d8819..571564e6 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java @@ -105,7 +105,7 @@ public class GeschichteService { *

Returns a {@link GeschichteSummary} projection — never carries items, preventing * LazyInitializationException on the non-transactional list path. */ - public List list(GeschichteStatus status, List personIds, int limit) { + public List list(GeschichteStatus status, List personIds, UUID documentId, int limit) { GeschichteStatus effective = currentUserHasBlogWrite() ? status : GeschichteStatus.PUBLISHED; int safeLimit = limit <= 0 ? DEFAULT_LIMIT : Math.min(limit, MAX_LIMIT); @@ -119,7 +119,7 @@ public class GeschichteService { long personCount = (personIds == null) ? 0 : personIds.size(); return geschichteRepository - .findSummaries(effective, authorId, safePersonIds, personCount) + .findSummaries(effective, authorId, safePersonIds, personCount, documentId) .stream() .limit(safeLimit) .toList(); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java index 9c7bf25c..f6a3b498 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java @@ -63,7 +63,7 @@ class GeschichteControllerTest { @Test @WithMockUser(authorities = "READ_ALL") void list_returns200_forReader() throws Exception { - when(geschichteService.list(any(), any(), anyInt())) + when(geschichteService.list(any(), any(), any(), anyInt())) .thenReturn(List.of(summaryStub("Story A"))); mockMvc.perform(get("/api/geschichten")) @@ -75,13 +75,13 @@ class GeschichteControllerTest { @WithMockUser(authorities = "READ_ALL") void list_passesSinglePersonIdFilterToServiceAsListOfOne() throws Exception { UUID personId = UUID.randomUUID(); - when(geschichteService.list(any(), eq(List.of(personId)), anyInt())) + when(geschichteService.list(any(), eq(List.of(personId)), any(), anyInt())) .thenReturn(List.of()); mockMvc.perform(get("/api/geschichten").param("personId", personId.toString())) .andExpect(status().isOk()); - verify(geschichteService).list(any(), eq(List.of(personId)), anyInt()); + verify(geschichteService).list(any(), eq(List.of(personId)), any(), anyInt()); } @Test @@ -89,7 +89,7 @@ class GeschichteControllerTest { void list_passesRepeatedPersonIdParamsAsListForAndFilter() throws Exception { UUID a = UUID.randomUUID(); UUID b = UUID.randomUUID(); - when(geschichteService.list(any(), eq(List.of(a, b)), anyInt())) + when(geschichteService.list(any(), eq(List.of(a, b)), any(), anyInt())) .thenReturn(List.of()); mockMvc.perform(get("/api/geschichten") @@ -97,7 +97,7 @@ class GeschichteControllerTest { .param("personId", b.toString())) .andExpect(status().isOk()); - verify(geschichteService).list(any(), eq(List.of(a, b)), anyInt()); + verify(geschichteService).list(any(), eq(List.of(a, b)), any(), anyInt()); } // ─── GET /api/geschichten/{id} ─────────────────────────────────────────── diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteListProjectionTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteListProjectionTest.java index ec0d79a5..3c1cefcf 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteListProjectionTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteListProjectionTest.java @@ -48,7 +48,7 @@ class GeschichteListProjectionTest { geschichteRepository.save(draft("Entwurf", author)); List result = geschichteRepository.findSummaries( - GeschichteStatus.PUBLISHED, null, sentinel(), 0); + GeschichteStatus.PUBLISHED, null, sentinel(), 0, null); assertThat(result).hasSize(1); assertThat(result.get(0).getTitle()).isEqualTo("Veröffentlicht"); @@ -59,7 +59,7 @@ class GeschichteListProjectionTest { geschichteRepository.save(draft("Nur Entwurf", author)); List result = geschichteRepository.findSummaries( - GeschichteStatus.PUBLISHED, null, sentinel(), 0); + GeschichteStatus.PUBLISHED, null, sentinel(), 0, null); assertThat(result).isEmpty(); } @@ -73,7 +73,7 @@ class GeschichteListProjectionTest { geschichteRepository.save(published("Briefe aus der Front", richAuthor)); List result = geschichteRepository.findSummaries( - GeschichteStatus.PUBLISHED, null, sentinel(), 0); + GeschichteStatus.PUBLISHED, null, sentinel(), 0, null); assertThat(result).hasSize(1); GeschichteSummary.AuthorSummary a = result.get(0).getAuthor(); @@ -94,7 +94,7 @@ class GeschichteListProjectionTest { geschichteRepository.save(journey); List result = geschichteRepository.findSummaries( - GeschichteStatus.PUBLISHED, null, sentinel(), 0); + GeschichteStatus.PUBLISHED, null, sentinel(), 0, null); assertThat(result).hasSize(1); assertThat(result.get(0).getType()).isEqualTo(GeschichteType.JOURNEY); @@ -108,7 +108,7 @@ class GeschichteListProjectionTest { geschichteRepository.save(draft("Fremder Entwurf", otherAuthor)); List result = geschichteRepository.findSummaries( - GeschichteStatus.DRAFT, author.getId(), sentinel(), 0); + GeschichteStatus.DRAFT, author.getId(), sentinel(), 0, null); assertThat(result).hasSize(1); assertThat(result.get(0).getTitle()).isEqualTo("Mein Entwurf"); @@ -122,7 +122,7 @@ class GeschichteListProjectionTest { geschichteRepository.save(published("B", author)); List result = geschichteRepository.findSummaries( - GeschichteStatus.PUBLISHED, null, sentinel(), 0); + GeschichteStatus.PUBLISHED, null, sentinel(), 0, null); assertThat(result).hasSize(2); } @@ -143,7 +143,7 @@ class GeschichteListProjectionTest { geschichteRepository.save(withAnna); List result = geschichteRepository.findSummaries( - GeschichteStatus.PUBLISHED, null, List.of(franz.getId()), 1); + GeschichteStatus.PUBLISHED, null, List.of(franz.getId()), 1, null); assertThat(result).hasSize(1); assertThat(result.get(0).getTitle()).isEqualTo("Franz story"); @@ -164,7 +164,7 @@ class GeschichteListProjectionTest { geschichteRepository.save(onlyFranz); List result = geschichteRepository.findSummaries( - GeschichteStatus.PUBLISHED, null, List.of(franz.getId(), anna.getId()), 2); + GeschichteStatus.PUBLISHED, null, List.of(franz.getId(), anna.getId()), 2, null); assertThat(result).hasSize(1); assertThat(result.get(0).getTitle()).isEqualTo("Both"); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java index 11db3c1a..f882e4f9 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java @@ -90,7 +90,7 @@ class GeschichteServiceIntegrationTest { // Reader cannot see DRAFT in list authenticateAs(reader, Permission.READ_ALL); - assertThat(geschichteService.list(null, List.of(), 50)).isEmpty(); + assertThat(geschichteService.list(null, List.of(), null, 50)).isEmpty(); // Reader cannot fetch DRAFT by id (404 via GESCHICHTE_NOT_FOUND) UUID draftId = created.getId(); @@ -106,8 +106,8 @@ class GeschichteServiceIntegrationTest { // Reader can now see and fetch it authenticateAs(reader, Permission.READ_ALL); - assertThat(geschichteService.list(null, List.of(), 50)).hasSize(1); - assertThat(geschichteService.list(null, List.of(franz.getId()), 50)).hasSize(1); + assertThat(geschichteService.list(null, List.of(), null, 50)).hasSize(1); + assertThat(geschichteService.list(null, List.of(franz.getId()), null, 50)).hasSize(1); Geschichte fetched = geschichteService.getById(draftId); GeschichteView fetchedView = geschichteService.toView(fetched, journeyItemService.getItems(draftId)); assertThat(fetchedView.title()).isEqualTo("Erinnerung an Opa Franz"); @@ -141,26 +141,26 @@ class GeschichteServiceIntegrationTest { authenticateAs(reader, Permission.READ_ALL); // No filter → all three - assertThat(geschichteService.list(null, List.of(), 50)) + assertThat(geschichteService.list(null, List.of(), null, 50)) .extracting(GeschichteSummary::getId) .containsExactlyInAnyOrder(storyAB, storyAC, storyA); // Single filter (Anna) → all three - assertThat(geschichteService.list(null, List.of(a.getId()), 50)) + assertThat(geschichteService.list(null, List.of(a.getId()), null, 50)) .extracting(GeschichteSummary::getId) .containsExactlyInAnyOrder(storyAB, storyAC, storyA); // AND: Anna AND Bertha → only the AB story (NOT story_A, NOT story_AC) - assertThat(geschichteService.list(null, List.of(a.getId(), b.getId()), 50)) + assertThat(geschichteService.list(null, List.of(a.getId(), b.getId()), null, 50)) .extracting(GeschichteSummary::getId) .containsExactly(storyAB); // AND: Bertha AND Carl → none (no story has both) - assertThat(geschichteService.list(null, List.of(b.getId(), c.getId()), 50)) + assertThat(geschichteService.list(null, List.of(b.getId(), c.getId()), null, 50)) .isEmpty(); // AND: Anna AND Bertha AND Carl → none - assertThat(geschichteService.list(null, List.of(a.getId(), b.getId(), c.getId()), 50)) + assertThat(geschichteService.list(null, List.of(a.getId(), b.getId(), c.getId()), null, 50)) .isEmpty(); } @@ -179,7 +179,7 @@ class GeschichteServiceIntegrationTest { geschichteService.create(dto); authenticateAs(writer2, Permission.BLOG_WRITE); - List result = geschichteService.list(GeschichteStatus.DRAFT, List.of(), 50); + List result = geschichteService.list(GeschichteStatus.DRAFT, List.of(), null, 50); assertThat(result).isEmpty(); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java index d2d5bfed..06b00490 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java @@ -34,6 +34,7 @@ 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.anyLong; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -222,12 +223,12 @@ class GeschichteServiceTest { @Test void list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE() { authenticateAs(reader, Permission.READ_ALL); - when(geschichteRepository.findSummaries(any(), any(), any(), anyLong())) + when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any())) .thenReturn(List.of()); - geschichteService.list(null, List.of(), 50); + geschichteService.list(null, List.of(), null, 50); - verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong()); + verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong(), any()); } @Test @@ -235,25 +236,25 @@ class GeschichteServiceTest { authenticateAs(writer, Permission.BLOG_WRITE); GeschichteSummary s1 = mock(GeschichteSummary.class); GeschichteSummary s2 = mock(GeschichteSummary.class); - when(geschichteRepository.findSummaries(any(), any(), any(), anyLong())) + when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any())) .thenReturn(List.of(s1, s2)); - List out = geschichteService.list(null, List.of(), 50); + List out = geschichteService.list(null, List.of(), null, 50); assertThat(out).hasSize(2); - verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong()); + verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong(), any()); } @Test void list_invokes_repository_findSummaries_when_filtering_by_single_personId() { authenticateAs(reader, Permission.READ_ALL); UUID personId = UUID.randomUUID(); - when(geschichteRepository.findSummaries(any(), any(), any(), anyLong())) + when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any())) .thenReturn(List.of()); - geschichteService.list(null, List.of(personId), 50); + geschichteService.list(null, List.of(personId), null, 50); - verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong()); + verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong(), any()); } @Test @@ -261,21 +262,33 @@ class GeschichteServiceTest { authenticateAs(reader, Permission.READ_ALL); UUID a = UUID.randomUUID(); UUID b = UUID.randomUUID(); - when(geschichteRepository.findSummaries(any(), any(), any(), anyLong())) + when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any())) .thenReturn(List.of()); - geschichteService.list(null, List.of(a, b), 50); + geschichteService.list(null, List.of(a, b), null, 50); - verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong()); + verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong(), any()); + } + + @Test + void list_passes_documentId_to_repository_as_journey_item_filter() { + authenticateAs(reader, Permission.READ_ALL); + UUID documentId = UUID.randomUUID(); + when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any())) + .thenReturn(List.of()); + + geschichteService.list(null, List.of(), documentId, 50); + + verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong(), eq(documentId)); } @Test void list_caps_limit_at_max_when_caller_passes_huge_value() { authenticateAs(reader, Permission.READ_ALL); - when(geschichteRepository.findSummaries(any(), any(), any(), anyLong())) + when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any())) .thenReturn(List.of(mock(GeschichteSummary.class))); - List out = geschichteService.list(null, List.of(), 9999); + List out = geschichteService.list(null, List.of(), null, 9999); assertThat(out).hasSizeLessThanOrEqualTo(200); }