diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonController.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonController.java index 5c47cbde..81c35825 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonController.java @@ -22,12 +22,15 @@ import org.springframework.web.bind.annotation.*; import org.springframework.web.server.ResponseStatusException; import jakarta.validation.Valid; +import jakarta.validation.constraints.Max; +import jakarta.validation.constraints.Min; import lombok.RequiredArgsConstructor; @RestController @RequestMapping("/api/persons") @RequiredArgsConstructor +@Validated public class PersonController { private final PersonService personService; @@ -35,15 +38,35 @@ public class PersonController { @GetMapping @RequirePermission(Permission.READ_ALL) - public ResponseEntity> getPersons( + public ResponseEntity getPersons( @RequestParam(required = false) String q, - @RequestParam(required = false, defaultValue = "0") int size, - @RequestParam(required = false) String sort) { - if ("documentCount".equals(sort) && size > 0 && q == null) { + @RequestParam(required = false) PersonType type, + @RequestParam(required = false) Boolean familyOnly, + @RequestParam(required = false) Boolean hasDocuments, + @RequestParam(required = false) Boolean provisional, + // review=true reveals the import noise (transcriber view); absent/false keeps the + // clean reader default (familyMember OR documentCount > 0). The explicit filters AND + // within whichever base the review flag selects. + @RequestParam(required = false, defaultValue = "false") boolean review, + @RequestParam(required = false) String sort, + @RequestParam(defaultValue = "0") @Min(0) int page, + @RequestParam(defaultValue = "50") @Min(1) @Max(100) int size) { + // Legacy top-N-by-document-count path (reader dashboard): preserved, now wrapped in the + // paged contract so /api/persons always returns one shape. + if ("documentCount".equals(sort) && q == null) { int safeSize = Math.min(size, 50); - return ResponseEntity.ok(personService.findTopByDocumentCount(safeSize)); + List top = personService.findTopByDocumentCount(safeSize); + return ResponseEntity.ok(PersonSearchResult.paged(top, 0, safeSize, top.size())); } - return ResponseEntity.ok(personService.findAll(q)); + + PersonFilter filter = PersonFilter.builder() + .type(type) + .familyOnly(familyOnly) + .hasDocuments(hasDocuments) + .provisional(provisional) + .readerDefault(!review) + .build(); + return ResponseEntity.ok(personService.search(filter, page, size, q)); } @GetMapping("/{id}") @@ -110,6 +133,21 @@ public class PersonController { personService.mergePersons(id, UUID.fromString(targetIdStr)); } + // Dedicated state transition that clears the provisional flag. A separate verb (not a + // mass-assignable DTO field) so provisional can never be smuggled in via create/update. + @PatchMapping("/{id}/confirm") + @RequirePermission(Permission.WRITE_ALL) + public ResponseEntity confirmPerson(@PathVariable UUID id) { + return ResponseEntity.ok(personService.confirmPerson(id)); + } + + @DeleteMapping("/{id}") + @ResponseStatus(HttpStatus.NO_CONTENT) + @RequirePermission(Permission.WRITE_ALL) + public void deletePerson(@PathVariable UUID id) { + personService.deletePerson(id); + } + // ─── Alias endpoints ──────────────────────────────────────────────────── @GetMapping("/{id}/aliases") 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 baad6e2a..175ab529 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonService.java @@ -31,16 +31,6 @@ public class PersonService { private final PersonRepository personRepository; private final PersonNameAliasRepository aliasRepository; - public List findAll(String q) { - if (q == null) { - return personRepository.findAllWithDocumentCount(); - } - if (q.isBlank()) { - return List.of(); - } - return personRepository.searchWithDocumentCount(q.trim()); - } - public List findTopByDocumentCount(int limit) { return personRepository.findTopByDocumentCount(limit); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonControllerTest.java index 2dee3baa..f421f86e 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonControllerTest.java @@ -65,44 +65,114 @@ class PersonControllerTest { @Test @WithMockUser(authorities = "READ_ALL") - void getPersons_returns200_withEmptyList() throws Exception { - when(personService.findAll(null)).thenReturn(Collections.emptyList()); + void getPersons_returns200_withEmptyPagedResult() throws Exception { + when(personService.search(any(), eq(0), eq(50), eq(null))) + .thenReturn(PersonSearchResult.paged(Collections.emptyList(), 0, 50, 0)); mockMvc.perform(get("/api/persons")) - .andExpect(status().isOk()); + .andExpect(status().isOk()) + .andExpect(jsonPath("$.items").isArray()) + .andExpect(jsonPath("$.totalElements").value(0)); } @Test @WithMockUser(authorities = "READ_ALL") void getPersons_delegatesQueryParam_toService() throws Exception { PersonSummaryDTO dto = mockPersonSummary("Hans", "Müller"); - when(personService.findAll("Hans")).thenReturn(List.of(dto)); + when(personService.search(any(), eq(0), eq(50), eq("Hans"))) + .thenReturn(PersonSearchResult.paged(List.of(dto), 0, 50, 1)); mockMvc.perform(get("/api/persons").param("q", "Hans")) .andExpect(status().isOk()) - .andExpect(jsonPath("$[0].firstName").value("Hans")); + .andExpect(jsonPath("$.items[0].firstName").value("Hans")); } @Test @WithMockUser(authorities = "READ_ALL") - void getPersons_delegatesTopByDocumentCount_whenSortAndSizeGiven() throws Exception { + void getPersons_passesFilterParams_toService() throws Exception { + ArgumentCaptor filterCaptor = ArgumentCaptor.forClass(PersonFilter.class); + when(personService.search(filterCaptor.capture(), eq(0), eq(50), eq(null))) + .thenReturn(PersonSearchResult.paged(Collections.emptyList(), 0, 50, 0)); + + mockMvc.perform(get("/api/persons") + .param("type", "INSTITUTION") + .param("familyOnly", "true") + .param("hasDocuments", "true") + .param("provisional", "false")) + .andExpect(status().isOk()); + + PersonFilter captured = filterCaptor.getValue(); + assertThat(captured.type()).isEqualTo(PersonType.INSTITUTION); + assertThat(captured.familyOnly()).isTrue(); + assertThat(captured.hasDocuments()).isTrue(); + assertThat(captured.provisional()).isFalse(); + } + + @Test + @WithMockUser(authorities = "READ_ALL") + void getPersons_defaultsToReaderDefault_whenNoReviewFlag() throws Exception { + ArgumentCaptor filterCaptor = ArgumentCaptor.forClass(PersonFilter.class); + when(personService.search(filterCaptor.capture(), eq(0), eq(50), eq(null))) + .thenReturn(PersonSearchResult.paged(Collections.emptyList(), 0, 50, 0)); + + mockMvc.perform(get("/api/persons")).andExpect(status().isOk()); + + assertThat(filterCaptor.getValue().readerDefault()).isTrue(); + } + + @Test + @WithMockUser(authorities = "READ_ALL") + void getPersons_dropsReaderDefault_whenReviewFlagSet() throws Exception { + ArgumentCaptor filterCaptor = ArgumentCaptor.forClass(PersonFilter.class); + when(personService.search(filterCaptor.capture(), eq(0), eq(50), eq(null))) + .thenReturn(PersonSearchResult.paged(Collections.emptyList(), 0, 50, 0)); + + mockMvc.perform(get("/api/persons").param("review", "true")).andExpect(status().isOk()); + + assertThat(filterCaptor.getValue().readerDefault()).isFalse(); + } + + @Test + @WithMockUser(authorities = "READ_ALL") + void getPersons_passesPageAndSize_toService() throws Exception { + when(personService.search(any(), eq(2), eq(25), eq(null))) + .thenReturn(PersonSearchResult.paged(Collections.emptyList(), 2, 25, 0)); + + mockMvc.perform(get("/api/persons").param("page", "2").param("size", "25")) + .andExpect(status().isOk()); + + verify(personService).search(any(), eq(2), eq(25), eq(null)); + } + + @Test + @WithMockUser(authorities = "READ_ALL") + void getPersons_returns400_whenSizeIsZero() throws Exception { + mockMvc.perform(get("/api/persons").param("size", "0")) + .andExpect(status().isBadRequest()); + } + + @Test + @WithMockUser(authorities = "READ_ALL") + void getPersons_returns400_whenSizeExceeds100() throws Exception { + mockMvc.perform(get("/api/persons").param("size", "101")) + .andExpect(status().isBadRequest()); + } + + @Test + @WithMockUser(authorities = "READ_ALL") + void getPersons_returns400_whenPageIsNegative() throws Exception { + mockMvc.perform(get("/api/persons").param("page", "-1")) + .andExpect(status().isBadRequest()); + } + + @Test + @WithMockUser(authorities = "READ_ALL") + void getPersons_delegatesTopByDocumentCount_whenSortGiven() throws Exception { PersonSummaryDTO top = mockPersonSummary("Käthe", "Raddatz"); when(personService.findTopByDocumentCount(4)).thenReturn(List.of(top)); mockMvc.perform(get("/api/persons").param("sort", "documentCount").param("size", "4")) .andExpect(status().isOk()) - .andExpect(jsonPath("$[0].firstName").value("Käthe")); - } - - @Test - @WithMockUser(authorities = "READ_ALL") - void getPersons_capsTopByDocumentCount_atFifty() throws Exception { - ArgumentCaptor sizeCaptor = ArgumentCaptor.forClass(Integer.class); - when(personService.findTopByDocumentCount(sizeCaptor.capture())).thenReturn(Collections.emptyList()); - - mockMvc.perform(get("/api/persons").param("sort", "documentCount").param("size", "999")) - .andExpect(status().isOk()); - - assertThat(sizeCaptor.getValue()).isEqualTo(50); + .andExpect(jsonPath("$.items[0].firstName").value("Käthe")); } private PersonSummaryDTO mockPersonSummary(String firstName, String lastName) { @@ -398,6 +468,61 @@ class PersonControllerTest { .andExpect(status().isNoContent()); } + // ─── PATCH /api/persons/{id}/confirm ────────────────────────────────────── + + @Test + void confirmPerson_returns401_whenUnauthenticated() throws Exception { + mockMvc.perform(patch("/api/persons/{id}/confirm", UUID.randomUUID()).with(csrf())) + .andExpect(status().isUnauthorized()); + } + + @Test + @WithMockUser(authorities = "READ_ALL") + void confirmPerson_returns403_whenUserHasOnlyReadPermission() throws Exception { + mockMvc.perform(patch("/api/persons/{id}/confirm", UUID.randomUUID()).with(csrf())) + .andExpect(status().isForbidden()); + } + + @Test + @WithMockUser(authorities = "WRITE_ALL") + void confirmPerson_returns200_andClearsProvisional() throws Exception { + UUID id = UUID.randomUUID(); + Person confirmed = Person.builder().id(id).firstName("Bald").lastName("Bestaetigt").provisional(false).build(); + when(personService.confirmPerson(id)).thenReturn(confirmed); + + mockMvc.perform(patch("/api/persons/{id}/confirm", id).with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.provisional").value(false)); + + verify(personService).confirmPerson(id); + } + + // ─── DELETE /api/persons/{id} ────────────────────────────────────────────── + + @Test + void deletePerson_returns401_whenUnauthenticated() throws Exception { + mockMvc.perform(delete("/api/persons/{id}", UUID.randomUUID()).with(csrf())) + .andExpect(status().isUnauthorized()); + } + + @Test + @WithMockUser(authorities = "READ_ALL") + void deletePerson_returns403_whenUserHasOnlyReadPermission() throws Exception { + mockMvc.perform(delete("/api/persons/{id}", UUID.randomUUID()).with(csrf())) + .andExpect(status().isForbidden()); + } + + @Test + @WithMockUser(authorities = "WRITE_ALL") + void deletePerson_returns204_whenValid() throws Exception { + UUID id = UUID.randomUUID(); + + mockMvc.perform(delete("/api/persons/{id}", id).with(csrf())) + .andExpect(status().isNoContent()); + + verify(personService).deletePerson(id); + } + // ─── PUT /api/persons/{id} — lastName blank branch ──────────────────────── @Test diff --git a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceTest.java index 8f6cf03c..4c8de65c 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceTest.java @@ -58,35 +58,6 @@ class PersonServiceTest { assertThat(personService.getById(id)).isEqualTo(person); } - // ─── findAll ───────────────────────────────────────────────────────────── - - @Test - void findAll_returnsAll_whenQueryIsNull() { - List expected = List.of(); - when(personRepository.findAllWithDocumentCount()).thenReturn(expected); - - assertThat(personService.findAll(null)).isEqualTo(expected); - verify(personRepository).findAllWithDocumentCount(); - verify(personRepository, never()).searchWithDocumentCount(any()); - } - - @Test - void findAll_returnsEmpty_whenQueryIsWhitespaceOnly() { - assertThat(personService.findAll(" ")).isEmpty(); - verify(personRepository, never()).findAllWithDocumentCount(); - verify(personRepository, never()).searchWithDocumentCount(any()); - } - - @Test - void findAll_searchesByName_whenQueryIsNonBlank() { - List expected = List.of(); - when(personRepository.searchWithDocumentCount("Anna")).thenReturn(expected); - - assertThat(personService.findAll("Anna")).isEqualTo(expected); - verify(personRepository).searchWithDocumentCount("Anna"); - verify(personRepository, never()).findAllWithDocumentCount(); - } - // ─── #667: search (filter + pagination) ────────────────────────────────── @Test