diff --git a/CLAUDE.md b/CLAUDE.md index b3a5b189..e902c469 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -192,7 +192,8 @@ frontend/src/routes/ ├── persons/ │ ├── [id]/ Person detail │ ├── [id]/edit/ Person edit form -│ └── new/ Create person form +│ ├── new/ Create person form +│ └── review/ Triage view — confirm/rename/merge/delete provisional persons ├── briefwechsel/ Bilateral conversation timeline (Briefwechsel) ├── aktivitaeten/ Unified activity feed (Chronik) ├── geschichten/ Stories — list, [id], [id]/edit, new 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..dad52b5a 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,37 @@ 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, wrapped in the + // same envelope so /api/persons always returns one shape. It is explicitly NON-paged — + // the top-N query returns the complete result, so PersonSearchResult.topN reports an + // honest totalElements (= returned count) instead of pretending to be a page slice. + 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.topN(top)); } - 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 +135,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/PersonFilter.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonFilter.java new file mode 100644 index 00000000..bc41214a --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonFilter.java @@ -0,0 +1,36 @@ +package org.raddatz.familienarchiv.person; + +import lombok.Builder; + +/** + * The reader/triage filter set for the persons directory, threaded as one value through + * {@code PersonController -> PersonService -> PersonRepository}. Each field is nullable: + * null means "do not constrain on this dimension". + * + *
    + *
  • {@code type} — restrict to a single {@link PersonType}.
    • + *
  • {@code familyOnly} — when true, only {@code familyMember} persons.
    • + *
  • {@code hasDocuments} — when true, only persons with documentCount > 0.
    • + *
  • {@code provisional} — match the {@code Person.provisional} flag exactly.
    • + *
  • {@code readerDefault} — when true, restrict to {@code familyMember OR documentCount > 0} + * (the clean reader view). The explicit filters above AND with this restriction.
    • + *
+ */ +@Builder +public record PersonFilter( + PersonType type, + Boolean familyOnly, + Boolean hasDocuments, + Boolean provisional, + boolean readerDefault +) { + /** The unconstrained "show all" filter (transcriber view, no reader restriction). */ + public static PersonFilter showAll() { + return PersonFilter.builder().readerDefault(false).build(); + } + + /** The clean reader default: familyMember OR documentCount > 0, no other constraints. */ + public static PersonFilter cleanDefault() { + return PersonFilter.builder().readerDefault(true).build(); + } +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java index 940e34a2..50ff4ee9 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonRepository.java @@ -88,6 +88,61 @@ public interface PersonRepository extends JpaRepository { nativeQuery = true) List findTopByDocumentCount(@Param("limit") int limit); + // --- #667: filter-aware paged directory --- + // + // The slice query and the count query below MUST keep an IDENTICAL WHERE clause so the + // rendered page and totalElements can never drift. Every filter is nullable: a null param + // disables that predicate via the `:param IS NULL OR …` idiom. `readerDefault` (a plain + // boolean) restricts to "familyMember OR has documents"; the explicit filters AND on top. + // documentCount is recomputed inline (not via the SELECT alias) because WHERE cannot + // reference a computed alias. All params are named — no string concatenation, no injection. + String FILTER_WHERE = """ + WHERE (CAST(:type AS text) IS NULL OR p.person_type = CAST(:type AS text)) + AND (:familyOnly = FALSE OR :familyOnly IS NULL OR p.family_member = TRUE) + AND (:hasDocuments = FALSE OR :hasDocuments IS NULL OR ( + (SELECT COUNT(*) FROM documents d WHERE d.sender_id = p.id) + + (SELECT COUNT(*) FROM document_receivers dr WHERE dr.person_id = p.id)) > 0) + AND (:provisional IS NULL OR p.provisional = :provisional) + AND (:readerDefault = FALSE OR ( + p.family_member = TRUE OR ( + (SELECT COUNT(*) FROM documents d WHERE d.sender_id = p.id) + + (SELECT COUNT(*) FROM document_receivers dr WHERE dr.person_id = p.id)) > 0)) + AND (CAST(:query AS text) IS NULL OR + LOWER(CONCAT(COALESCE(p.first_name,''),' ',p.last_name)) LIKE LOWER(CONCAT('%',CAST(:query AS text),'%')) + OR LOWER(CONCAT(p.last_name,' ',COALESCE(p.first_name,''))) LIKE LOWER(CONCAT('%',CAST(:query AS text),'%')) + OR LOWER(p.alias) LIKE LOWER(CONCAT('%',CAST(:query AS text),'%'))) + """; + + @Query(value = """ + SELECT p.id, p.title, p.first_name AS firstName, p.last_name AS lastName, + p.person_type AS personType, + p.alias, p.birth_year AS birthYear, p.death_year AS deathYear, p.notes, + p.family_member AS familyMember, p.provisional AS provisional, + (SELECT COUNT(*) FROM documents d WHERE d.sender_id = p.id) + + (SELECT COUNT(*) FROM document_receivers dr WHERE dr.person_id = p.id) AS documentCount + FROM persons p + """ + FILTER_WHERE + """ + ORDER BY p.last_name ASC, p.first_name ASC + LIMIT :limit OFFSET :offset + """, + nativeQuery = true) + List findByFilter(@Param("type") String type, + @Param("familyOnly") Boolean familyOnly, + @Param("hasDocuments") Boolean hasDocuments, + @Param("provisional") Boolean provisional, + @Param("readerDefault") boolean readerDefault, + @Param("query") String query, + @Param("limit") int limit, + @Param("offset") int offset); + + @Query(value = "SELECT COUNT(*) FROM persons p " + FILTER_WHERE, nativeQuery = true) + long countByFilter(@Param("type") String type, + @Param("familyOnly") Boolean familyOnly, + @Param("hasDocuments") Boolean hasDocuments, + @Param("provisional") Boolean provisional, + @Param("readerDefault") boolean readerDefault, + @Param("query") String query); + // --- Correspondent queries --- @Query(value = """ @@ -139,6 +194,12 @@ public interface PersonRepository extends JpaRepository { @Query(value = "UPDATE documents SET sender_id = :target WHERE sender_id = :source", nativeQuery = true) void reassignSender(@Param("source") UUID source, @Param("target") UUID target); + // Used by deletePerson: detach a deleted person from documents they sent, so the hard + // delete cannot orphan a documents.sender_id FK (the column is nullable). + @Modifying + @Query(value = "UPDATE documents SET sender_id = NULL WHERE sender_id = :source", nativeQuery = true) + void reassignSenderToNull(@Param("source") UUID source); + @Modifying @Query(value = """ INSERT INTO document_receivers (document_id, person_id) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonSearchResult.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonSearchResult.java new file mode 100644 index 00000000..ff605770 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonSearchResult.java @@ -0,0 +1,50 @@ +package org.raddatz.familienarchiv.person; + +import io.swagger.v3.oas.annotations.media.Schema; + +import java.util.List; + +/** + * Paged result for the /api/persons list endpoint. + * + *

Hand-written to mirror {@code document/DocumentSearchResult} field-for-field so the + * frontend sees one paged shape across the app. Deliberately NOT Spring {@code Page} + * (unstable serialized shape across Spring versions, noisy in OpenAPI) and deliberately + * NOT a reuse of the document DTO (would couple two feature modules — duplication beats + * coupling here). + */ +public record PersonSearchResult( + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + List items, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + long totalElements, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + int pageNumber, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + int pageSize, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + int totalPages +) { + /** + * Paged factory: derives {@code totalPages} from the full match count and the page size. + * A zero count yields zero pages so the frontend hides the pagination control. + */ + public static PersonSearchResult paged(List slice, int pageNumber, int pageSize, long totalElements) { + int totalPages = pageSize == 0 ? 0 : (int) ((totalElements + pageSize - 1) / pageSize); + return new PersonSearchResult(slice, totalElements, pageNumber, pageSize, totalPages); + } + + /** + * Non-paged factory for the legacy {@code sort=documentCount} top-N dashboard path. + * That query returns the complete result in one shot — there is no further page + * to fetch — so the envelope reports reality rather than pretending to be a slice of a + * larger set: {@code totalElements} equals the number of rows actually returned, + * {@code pageSize} equals that same count, and {@code totalPages} is 1 (or 0 when empty). + * This avoids the earlier ambiguity where {@code totalElements} looked like a paged total. + */ + public static PersonSearchResult topN(List all) { + int count = all.size(); + int totalPages = count == 0 ? 0 : 1; + return new PersonSearchResult(all, count, 0, count, totalPages); + } +} 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 d2e894bf..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,20 +31,55 @@ 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); } + /** + * Filtered, paginated directory query. The slice and the total are derived from one + * shared WHERE clause (see {@link PersonRepository#FILTER_WHERE}) so totalElements can + * never drift from the rendered page. {@code type} is passed as the enum name because the + * native query compares against the string column. + */ + public PersonSearchResult search(PersonFilter filter, int page, int size, String q) { + String type = filter.type() == null ? null : filter.type().name(); + String query = (q == null || q.isBlank()) ? null : q.trim(); + int offset = page * size; + + List items = personRepository.findByFilter( + type, filter.familyOnly(), filter.hasDocuments(), filter.provisional(), + filter.readerDefault(), query, size, offset); + long total = personRepository.countByFilter( + type, filter.familyOnly(), filter.hasDocuments(), filter.provisional(), + filter.readerDefault(), query); + + return PersonSearchResult.paged(items, page, size, total); + } + + /** + * Clears the {@code provisional} flag — a deliberate state transition exposed as + * {@code PATCH /api/persons/{id}/confirm}, never as a mass-assignable DTO field (CWE-915). + */ + @Transactional + public Person confirmPerson(UUID id) { + Person person = getById(id); + person.setProvisional(false); + return personRepository.save(person); + } + + /** + * Hard-deletes a person used by triage. Detaches the person from any documents they + * sent (nulls sender_id) and from any received-document references first, so the delete + * cannot orphan an FK and fail with a 500. + */ + @Transactional + public void deletePerson(UUID id) { + getById(id); + personRepository.reassignSenderToNull(id); + personRepository.deleteReceiverReferences(id); + personRepository.deleteById(id); + } + public Person getById(UUID id) { return personRepository.findById(id) .orElseThrow(() -> DomainException.notFound(ErrorCode.PERSON_NOT_FOUND, "Person not found: " + id)); 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..d43e9a9a 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,144 @@ 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")); + .andExpect(jsonPath("$.items[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()); + void getPersons_topByDocumentCount_isNonPaged_totalElementsEqualsReturnedCount() throws Exception { + // The top-N dashboard path is deliberately NON-paged: it returns the complete result + // (no further page exists), so totalElements equals the number of rows returned and + // totalPages is 1. Pinned so nobody "fixes" it into a misleading paged total. + when(personService.findTopByDocumentCount(50)) + .thenReturn(List.of(mockPersonSummary("Käthe", "Raddatz"), + mockPersonSummary("Hans", "Müller"))); - mockMvc.perform(get("/api/persons").param("sort", "documentCount").param("size", "999")) - .andExpect(status().isOk()); + mockMvc.perform(get("/api/persons").param("sort", "documentCount")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.items.length()").value(2)) + .andExpect(jsonPath("$.totalElements").value(2)) + .andExpect(jsonPath("$.pageNumber").value(0)) + .andExpect(jsonPath("$.pageSize").value(2)) + .andExpect(jsonPath("$.totalPages").value(1)); + } - assertThat(sizeCaptor.getValue()).isEqualTo(50); + @Test + @WithMockUser(authorities = "READ_ALL") + void getPersons_topByDocumentCount_emptyResult_reportsZeroPages() throws Exception { + when(personService.findTopByDocumentCount(50)).thenReturn(Collections.emptyList()); + + mockMvc.perform(get("/api/persons").param("sort", "documentCount")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.totalElements").value(0)) + .andExpect(jsonPath("$.totalPages").value(0)); } private PersonSummaryDTO mockPersonSummary(String firstName, String lastName) { @@ -398,6 +498,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/PersonRepositoryTest.java b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonRepositoryTest.java index 2de9f69f..910e701e 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonRepositoryTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonRepositoryTest.java @@ -505,4 +505,171 @@ class PersonRepositoryTest { .filter(p -> p.getId().equals(provisional.getId())).findFirst().orElseThrow(); assertThat(summary.isProvisional()).isTrue(); } + + // ─── #667: filter-aware paged slice + paired COUNT (Postgres-only) ──────── + // The slice query (findByFilter) and the count query (countByFilter) MUST share one + // WHERE clause so totalElements can never drift from the rendered page. These tests run + // against real Postgres because the slice ORDER BY uses a computed alias that fails on H2. + + private void seedDirectoryFixture() { + // Register family member, no documents — visible by reader default (familyMember) + personRepository.save(Person.builder().firstName("Karl").lastName("Register").familyMember(true).build()); + // Person with one document — visible by reader default (documentCount > 0) + Person hasDoc = personRepository.save(Person.builder().firstName("Doku").lastName("Person").build()); + documentRepository.save(Document.builder().title("B").originalFilename("b.pdf") + .status(DocumentStatus.UPLOADED).sender(hasDoc).build()); + // Provisional, zero-document, non-family — hidden by reader default + personRepository.save(Person.builder().firstName("Unbe").lastName("Staetigt").provisional(true).build()); + // An institution with no documents, non-family, non-provisional + personRepository.save(Person.builder().lastName("Verlag GmbH").personType(PersonType.INSTITUTION).build()); + } + + @Test + void findByFilter_readerDefault_returnsOnlyFamilyOrWithDocuments() { + seedDirectoryFixture(); + + List slice = personRepository.findByFilter( + null, null, null, null, true, null, 50, 0); + + assertThat(slice).extracting(PersonSummaryDTO::getLastName) + .containsExactlyInAnyOrder("Register", "Person"); + } + + @Test + void countByFilter_readerDefault_matchesSliceSize() { + seedDirectoryFixture(); + + long count = personRepository.countByFilter(null, null, null, null, true, null); + + assertThat(count).isEqualTo(2); + } + + @Test + void findByFilter_showAll_returnsEveryone() { + seedDirectoryFixture(); + + List slice = personRepository.findByFilter( + null, null, null, null, false, null, 50, 0); + + assertThat(slice).hasSize(4); + } + + @Test + void findByFilter_typeInstitution_returnsOnlyInstitutions() { + seedDirectoryFixture(); + + List slice = personRepository.findByFilter( + "INSTITUTION", null, null, null, false, null, 50, 0); + + assertThat(slice).extracting(PersonSummaryDTO::getLastName).containsExactly("Verlag GmbH"); + } + + @Test + void findByFilter_familyOnly_returnsOnlyFamilyMembers() { + seedDirectoryFixture(); + + List slice = personRepository.findByFilter( + null, true, null, null, false, null, 50, 0); + + assertThat(slice).extracting(PersonSummaryDTO::getLastName).containsExactly("Register"); + } + + @Test + void findByFilter_hasDocuments_returnsOnlyPersonsWithDocuments() { + seedDirectoryFixture(); + + List slice = personRepository.findByFilter( + null, null, true, null, false, null, 50, 0); + + assertThat(slice).extracting(PersonSummaryDTO::getLastName).containsExactly("Person"); + } + + @Test + void findByFilter_provisionalTrue_returnsOnlyProvisional() { + seedDirectoryFixture(); + + List slice = personRepository.findByFilter( + null, null, null, true, false, null, 50, 0); + + assertThat(slice).extracting(PersonSummaryDTO::getLastName).containsExactly("Staetigt"); + } + + @Test + void findByFilter_combinedFilters_andTogether() { + seedDirectoryFixture(); + // family + has-documents → intersection is empty (Register has no docs, Doku is not family) + List slice = personRepository.findByFilter( + null, true, true, null, false, null, 50, 0); + + assertThat(slice).isEmpty(); + } + + @Test + void findByFilter_query_combinesWithFilters() { + seedDirectoryFixture(); + + List slice = personRepository.findByFilter( + null, null, null, null, false, "Verlag", 50, 0); + + assertThat(slice).extracting(PersonSummaryDTO::getLastName).containsExactly("Verlag GmbH"); + } + + @Test + void findByFilter_pageBeyondRange_returnsEmptySlice() { + seedDirectoryFixture(); + + List slice = personRepository.findByFilter( + null, null, null, null, false, null, 50, 999 * 50); + + assertThat(slice).isEmpty(); + } + + @Test + void findByFilter_respectsPageSize() { + seedDirectoryFixture(); + + List firstPage = personRepository.findByFilter( + null, null, null, null, false, null, 2, 0); + List secondPage = personRepository.findByFilter( + null, null, null, null, false, null, 2, 2); + + assertThat(firstPage).hasSize(2); + assertThat(secondPage).hasSize(2); + assertThat(firstPage).extracting(PersonSummaryDTO::getId) + .doesNotContainAnyElementsOf(secondPage.stream().map(PersonSummaryDTO::getId).toList()); + } + + @Test + void countByFilter_typeInstitution_matchesSlice() { + seedDirectoryFixture(); + + long count = personRepository.countByFilter("INSTITUTION", null, null, null, false, null); + + assertThat(count).isEqualTo(1); + } + + @Test + void countByFilter_query_matchesSliceSize() { + // The whole point of the shared FILTER_WHERE is that the slice and the count can never + // drift. Pin the query (LIKE) path explicitly: countByFilter must equal the slice size + // so a future edit to one query's LIKE clause is caught. + seedDirectoryFixture(); + + List slice = personRepository.findByFilter( + null, null, null, null, false, "Verlag", 50, 0); + long count = personRepository.countByFilter(null, null, null, null, false, "Verlag"); + + assertThat(count).isEqualTo(slice.size()); + assertThat(count).isEqualTo(1); + } + + @Test + void findByFilter_projectsDocumentCount() { + seedDirectoryFixture(); + + List slice = personRepository.findByFilter( + null, null, true, null, false, null, 50, 0); + + assertThat(slice.get(0).getDocumentCount()).isEqualTo(1); + } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceIntegrationTest.java index e8d5ed97..0578f5fb 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonServiceIntegrationTest.java @@ -2,6 +2,9 @@ package org.raddatz.familienarchiv.person; import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.PostgresContainerConfig; +import org.raddatz.familienarchiv.document.Document; +import org.raddatz.familienarchiv.document.DocumentRepository; +import org.raddatz.familienarchiv.document.DocumentStatus; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.person.PersonType; import org.raddatz.familienarchiv.person.PersonRepository; @@ -13,6 +16,11 @@ import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.transaction.annotation.Transactional; import software.amazon.awssdk.services.s3.S3Client; +import jakarta.persistence.EntityManager; +import jakarta.persistence.PersistenceContext; + +import java.util.Set; + import static org.assertj.core.api.Assertions.assertThat; @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE) @@ -24,6 +32,9 @@ class PersonServiceIntegrationTest { @MockitoBean S3Client s3Client; @Autowired PersonService personService; @Autowired PersonRepository personRepository; + @Autowired DocumentRepository documentRepository; + + @PersistenceContext EntityManager entityManager; @Test void findOrCreateByAlias_skipReturnsNull_noRecordCreated() { @@ -63,4 +74,97 @@ class PersonServiceIntegrationTest { assertThat(result.getFirstName()).isEqualTo("Clara"); assertThat(result.getLastName()).isEqualTo("Cram"); } + + // ─── #667: confirm round-trip + reader-default semantics ────────────────── + + @Test + void search_readerDefault_hidesProvisionalZeroDocumentPerson() { + personRepository.save(Person.builder() + .firstName("Unbe").lastName("Staetigt").provisional(true).build()); + + PersonSearchResult result = personService.search(PersonFilter.cleanDefault(), 0, 50, null); + + assertThat(result.items()).noneMatch(p -> p.getLastName().equals("Staetigt")); + assertThat(result.totalElements()).isEqualTo(result.items().size()); + } + + @Test + void search_showAll_includesProvisionalZeroDocumentPerson() { + personRepository.save(Person.builder() + .firstName("Unbe").lastName("Staetigt").provisional(true).build()); + + PersonSearchResult result = personService.search(PersonFilter.showAll(), 0, 50, null); + + assertThat(result.items()).anyMatch(p -> p.getLastName().equals("Staetigt")); + } + + @Test + void confirmPerson_clearsProvisional_andShowAllTreatsItAsConfirmed() { + Person provisional = personRepository.save(Person.builder() + .firstName("Bald").lastName("Bestaetigt").provisional(true).build()); + + personService.confirmPerson(provisional.getId()); + + Person reloaded = personRepository.findById(provisional.getId()).orElseThrow(); + assertThat(reloaded.isProvisional()).isFalse(); + + PersonSearchResult showAll = personService.search(PersonFilter.showAll(), 0, 50, null); + assertThat(showAll.items()) + .filteredOn(p -> p.getId().equals(provisional.getId())) + .allMatch(p -> !p.isProvisional()); + } + + @Test + void deletePerson_removesPerson() { + Person target = personRepository.save(Person.builder() + .firstName("Weg").lastName("Person").provisional(true).build()); + + personService.deletePerson(target.getId()); + + assertThat(personRepository.findById(target.getId())).isEmpty(); + } + + @Test + void deletePerson_detachesSentAndReceivedReferences_beforeDelete_noOrphan() { + // A person referenced as BOTH a document sender and a document receiver must delete + // cleanly: deletePerson nulls the sender_id FK and removes the receiver join row first + // (reassignSenderToNull → deleteReceiverReferences → deleteById), so no FK orphan and + // the documents themselves survive. + Person target = personRepository.save(Person.builder() + .firstName("Weg").lastName("Person").provisional(true).build()); + Person bystander = personRepository.save(Person.builder() + .firstName("Bleibt").lastName("Hier").build()); + + Document sent = documentRepository.save(Document.builder() + .title("Sent letter").originalFilename("sent.pdf") + .status(DocumentStatus.UPLOADED).sender(target).build()); + Document received = documentRepository.save(Document.builder() + .title("Received letter").originalFilename("received.pdf") + .status(DocumentStatus.UPLOADED).sender(bystander) + .receivers(new java.util.HashSet<>(Set.of(target))).build()); + + // Persist the fixture and detach everything so the native @Modifying deletes operate on + // the database directly without the persistence context holding stale references that + // would re-flush a now-deleted person as a transient association. + entityManager.flush(); + entityManager.clear(); + + personService.deletePerson(target.getId()); + + // Native @Modifying queries bypass the persistence context — clear it so the asserting + // reads observe the post-delete database state, not stale managed entities. + entityManager.flush(); + entityManager.clear(); + + assertThat(personRepository.findById(target.getId())).isEmpty(); + + Document reloadedSent = documentRepository.findById(sent.getId()).orElseThrow(); + assertThat(reloadedSent.getSender()).isNull(); + + Document reloadedReceived = documentRepository.findById(received.getId()).orElseThrow(); + assertThat(reloadedReceived.getReceivers()) + .noneMatch(p -> p.getId().equals(target.getId())); + // The other person and the documents themselves survive the delete. + assertThat(personRepository.findById(bystander.getId())).isPresent(); + } } 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 1ad9ce27..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,33 +58,109 @@ class PersonServiceTest { assertThat(personService.getById(id)).isEqualTo(person); } - // ─── findAll ───────────────────────────────────────────────────────────── + // ─── #667: search (filter + pagination) ────────────────────────────────── @Test - void findAll_returnsAll_whenQueryIsNull() { - List expected = List.of(); - when(personRepository.findAllWithDocumentCount()).thenReturn(expected); + void search_returnsPagedResult_withTotalsFromCountQuery() { + PersonFilter filter = PersonFilter.cleanDefault(); + when(personRepository.countByFilter(null, null, null, null, true, null)).thenReturn(120L); + when(personRepository.findByFilter(null, null, null, null, true, null, 50, 0)) + .thenReturn(List.of()); - assertThat(personService.findAll(null)).isEqualTo(expected); - verify(personRepository).findAllWithDocumentCount(); - verify(personRepository, never()).searchWithDocumentCount(any()); + PersonSearchResult result = personService.search(filter, 0, 50, null); + + assertThat(result.totalElements()).isEqualTo(120L); + assertThat(result.pageNumber()).isEqualTo(0); + assertThat(result.pageSize()).isEqualTo(50); + assertThat(result.totalPages()).isEqualTo(3); // ceil(120 / 50) } @Test - void findAll_returnsEmpty_whenQueryIsWhitespaceOnly() { - assertThat(personService.findAll(" ")).isEmpty(); - verify(personRepository, never()).findAllWithDocumentCount(); - verify(personRepository, never()).searchWithDocumentCount(any()); + void search_passesTypeAsEnumName_toRepository() { + PersonFilter filter = PersonFilter.builder().type(PersonType.INSTITUTION).build(); + when(personRepository.countByFilter("INSTITUTION", null, null, null, false, null)).thenReturn(0L); + when(personRepository.findByFilter("INSTITUTION", null, null, null, false, null, 50, 0)) + .thenReturn(List.of()); + + personService.search(filter, 0, 50, null); + + verify(personRepository).findByFilter("INSTITUTION", null, null, null, false, null, 50, 0); } @Test - void findAll_searchesByName_whenQueryIsNonBlank() { - List expected = List.of(); - when(personRepository.searchWithDocumentCount("Anna")).thenReturn(expected); + void search_computesOffset_fromPageAndSize() { + PersonFilter filter = PersonFilter.showAll(); + when(personRepository.countByFilter(null, null, null, null, false, null)).thenReturn(0L); + when(personRepository.findByFilter(null, null, null, null, false, null, 20, 40)) + .thenReturn(List.of()); - assertThat(personService.findAll("Anna")).isEqualTo(expected); - verify(personRepository).searchWithDocumentCount("Anna"); - verify(personRepository, never()).findAllWithDocumentCount(); + personService.search(filter, 2, 20, null); // offset = page * size = 40 + + verify(personRepository).findByFilter(null, null, null, null, false, null, 20, 40); + } + + @Test + void search_trimsBlankQueryToNull() { + PersonFilter filter = PersonFilter.showAll(); + when(personRepository.countByFilter(null, null, null, null, false, null)).thenReturn(0L); + when(personRepository.findByFilter(null, null, null, null, false, null, 50, 0)) + .thenReturn(List.of()); + + personService.search(filter, 0, 50, " "); + + verify(personRepository).findByFilter(null, null, null, null, false, null, 50, 0); + } + + // ─── #667: confirmPerson ────────────────────────────────────────────────── + + @Test + void confirmPerson_clearsProvisionalFlag() { + UUID id = UUID.randomUUID(); + Person provisional = Person.builder().id(id).firstName("Inferred").lastName("Person").provisional(true).build(); + when(personRepository.findById(id)).thenReturn(Optional.of(provisional)); + when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + Person result = personService.confirmPerson(id); + + assertThat(result.isProvisional()).isFalse(); + verify(personRepository).save(argThat(p -> !p.isProvisional())); + } + + @Test + void confirmPerson_throwsNotFound_whenMissing() { + UUID id = UUID.randomUUID(); + when(personRepository.findById(id)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> personService.confirmPerson(id)) + .isInstanceOf(DomainException.class) + .extracting(e -> ((DomainException) e).getStatus().value()) + .isEqualTo(404); + } + + // ─── #667: deletePerson ─────────────────────────────────────────────────── + + @Test + void deletePerson_deletes_whenPersonExists() { + UUID id = UUID.randomUUID(); + Person person = Person.builder().id(id).firstName("Weg").lastName("Person").build(); + when(personRepository.findById(id)).thenReturn(Optional.of(person)); + + personService.deletePerson(id); + + verify(personRepository).reassignSenderToNull(id); + verify(personRepository).deleteReceiverReferences(id); + verify(personRepository).deleteById(id); + } + + @Test + void deletePerson_throwsNotFound_whenMissing() { + UUID id = UUID.randomUUID(); + when(personRepository.findById(id)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> personService.deletePerson(id)) + .isInstanceOf(DomainException.class) + .extracting(e -> ((DomainException) e).getStatus().value()) + .isEqualTo(404); } // ─── createPerson ───────────────────────────────────────────────────────── diff --git a/docs/architecture/c4/l3-backend-3e-persons.puml b/docs/architecture/c4/l3-backend-3e-persons.puml index 47c884aa..f424168d 100644 --- a/docs/architecture/c4/l3-backend-3e-persons.puml +++ b/docs/architecture/c4/l3-backend-3e-persons.puml @@ -7,12 +7,12 @@ Container(frontend, "Web Frontend", "SvelteKit") ContainerDb(db, "PostgreSQL", "PostgreSQL 16") System_Boundary(backend, "API Backend (Spring Boot)") { - Component(personCtrl, "PersonController", "Spring MVC — /api/persons", "Lists and searches family members. Returns documents sent by or received by a person, correspondent suggestions, and person summary with document counts.") + Component(personCtrl, "PersonController", "Spring MVC — /api/persons", "Filtered, paginated directory (type/familyOnly/hasDocuments/provisional + page/size -> PersonSearchResult). Returns documents sent/received, correspondent suggestions, person summaries with counts. PATCH /{id}/confirm clears provisional; DELETE /{id} removes a person (both WRITE_ALL).") Component(relCtrl, "RelationshipController", "Spring MVC — /api/network, /api/persons/{id}/relationships", "CRUD for explicit person relationships and the full family network graph (nodes + edges) used by the Stammbaum view.") - Component(personSvc, "PersonService", "Spring Service", "Person CRUD, alias management, and merge operations (reassigns all document sender/receiver references before deleting duplicate persons).") + Component(personSvc, "PersonService", "Spring Service", "Person CRUD, alias management, filtered paged search (PersonFilter -> paired slice/count), confirm (clears provisional), delete (detaches document refs first), and merge operations (reassigns all document sender/receiver references before deleting duplicate persons).") Component(relSvc, "RelationshipService", "Spring Service", "Manages explicit directional family relationships (PARENT_OF, SPOUSE_OF, SIBLING_OF, etc.) with optional date ranges and notes.") Component(relInference, "RelationshipInferenceService", "Spring Service", "Computes transitive family relationships from explicit edges to infer grandparent/grandchild, aunt/uncle, and other extended-family links for the network graph.") - Component(personRepo, "PersonRepository", "Spring Data JPA", "Queries persons with name search (including aliases), correspondent discovery, person summaries with document counts, and merge/reassignment helpers.") + Component(personRepo, "PersonRepository", "Spring Data JPA", "Queries persons with name search (including aliases), correspondent discovery, person summaries with document counts, paired filter-aware slice + COUNT queries (one shared WHERE clause), and merge/reassignment helpers.") Component(relRepo, "PersonRelationshipRepository", "Spring Data JPA", "Reads and writes PersonRelationship records. Supports lookup by person ID, by relation type, and existence checks for deduplication.") } diff --git a/docs/architecture/c4/l3-frontend-3c-people-stories.puml b/docs/architecture/c4/l3-frontend-3c-people-stories.puml index 49526211..2ba3e454 100644 --- a/docs/architecture/c4/l3-frontend-3c-people-stories.puml +++ b/docs/architecture/c4/l3-frontend-3c-people-stories.puml @@ -7,8 +7,9 @@ Person(user, "User") Container(backend, "API Backend", "Spring Boot") System_Boundary(frontend, "Web Frontend (SvelteKit / SSR)") { - Component(personsPage, "/persons and /persons/[id]", "SvelteKit Routes", "Person directory and detail. Detail: metadata, document list sent/received, correspondents, explicit and inferred family relationships.") + Component(personsPage, "/persons and /persons/[id]", "SvelteKit Routes", "Person directory (server-side filtered + paginated) and detail. Directory: type/family/has-documents chips, reader default (familyMember OR documentCount > 0), writer-only show-all toggle. Detail: metadata, document list sent/received, correspondents, family relationships.") Component(personEdit, "/persons/[id]/edit and /persons/new", "SvelteKit Routes", "Create and edit person forms. Edit: metadata, aliases, explicit relationships. Actions: PUT/POST /api/persons.") + Component(personReview, "/persons/review", "SvelteKit Route", "Transcriber triage view (WRITE-gated link). Lists provisional persons; per-row Merge / Umbenennen / Bestätigen / Löschen. Actions: POST /merge, PUT /{id}, PATCH /{id}/confirm, DELETE /{id}.") Component(briefwechsel, "/briefwechsel", "SvelteKit Route", "Bilateral conversation timeline. Selects two persons via PersonTypeahead, fetches GET /api/documents/conversation, displays chronological exchange.") Component(aktivitaeten, "/aktivitaeten", "SvelteKit Route", "Unified activity feed (Chronik). Loader: GET /api/dashboard/activity and GET /api/notifications?read=false.") Component(geschichten, "/geschichten and /geschichten/[id]", "SvelteKit Routes", "Story list and detail pages. Loader: GET /api/geschichten?status=PUBLISHED.") @@ -19,8 +20,9 @@ System_Boundary(frontend, "Web Frontend (SvelteKit / SSR)") { } Rel(user, personsPage, "Browses family members", "HTTPS / Browser") -Rel(personsPage, backend, "GET /api/persons, GET /api/persons/{id}", "HTTP / JSON") +Rel(personsPage, backend, "GET /api/persons (filter + page params -> PersonSearchResult), GET /api/persons/{id}", "HTTP / JSON") Rel(personEdit, backend, "GET /api/persons/{id}, PUT /api/persons/{id}, POST /api/persons", "HTTP / JSON") +Rel(personReview, backend, "GET /api/persons?provisional=true, PATCH /api/persons/{id}/confirm, DELETE /api/persons/{id}, POST /api/persons/{id}/merge", "HTTP / JSON") Rel(briefwechsel, backend, "GET /api/documents/conversation", "HTTP / JSON") Rel(aktivitaeten, backend, "GET /api/dashboard/activity, GET /api/notifications", "HTTP / JSON") Rel(geschichten, backend, "GET /api/geschichten", "HTTP / JSON") diff --git a/frontend/CLAUDE.md b/frontend/CLAUDE.md index 3699350f..92ae7396 100644 --- a/frontend/CLAUDE.md +++ b/frontend/CLAUDE.md @@ -28,7 +28,7 @@ src/ │ ├── +layout.server.ts # Loads current user, injects auth cookie │ ├── +page.svelte # Home / document search dashboard │ ├── documents/ # Document CRUD, detail, edit, upload -│ ├── persons/ # Person directory, detail, edit, merge +│ ├── persons/ # Person directory (filtered, paginated), detail, edit, merge, review (triage) │ ├── briefwechsel/ # Bilateral conversation timeline │ ├── aktivitaeten/ # Unified activity feed (Chronik) │ ├── admin/ # User, group, tag, OCR, system management diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 0ac0a807..7358637a 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -130,6 +130,31 @@ "persons_search_placeholder": "Namen suchen...", "persons_empty_heading": "Keine Personen gefunden.", "persons_empty_text": "Versuchen Sie einen anderen Suchbegriff.", + "persons_empty_filtered": "Keine Personen für diese Filter.", + "persons_filter_group_label": "Filter", + "persons_filter_type_person": "Person", + "persons_filter_type_group": "Gruppe", + "persons_filter_type_institution": "Institution", + "persons_filter_family_only": "Nur Familie", + "persons_filter_has_documents": "Mit Dokumenten", + "persons_toggle_show_all": "Alle anzeigen", + "persons_toggle_needs_review": "Zu prüfen ({count})", + "person_badge_unconfirmed": "unbestätigt", + "persons_review_heading": "Personen prüfen", + "persons_review_intro": "Vom Import erzeugte, noch nicht bestätigte Personen. Zusammenführen, umbenennen, bestätigen oder löschen.", + "persons_review_action_merge": "Zusammenführen", + "persons_review_action_rename": "Umbenennen", + "persons_review_action_confirm": "Bestätigen", + "persons_review_action_delete": "Löschen", + "persons_review_action_cancel": "Abbrechen", + "persons_review_action_save": "Speichern", + "persons_review_empty": "Keine Personen zu prüfen.", + "persons_review_delete_confirm_title": "Person löschen", + "persons_review_delete_confirm_text": "Diese Person wird endgültig gelöscht. Dokumentverweise bleiben erhalten, verlieren aber diese Person.", + "persons_review_delete_confirm_button": "Person löschen", + "persons_review_merge_label": "Mit welcher Person zusammenführen?", + "persons_field_first_name": "Vorname", + "persons_field_last_name": "Nachname", "persons_new_heading": "Neue Person", "persons_section_details": "Angaben zur Person", "person_edit_heading": "Person bearbeiten", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 269e95d3..77097540 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -130,6 +130,31 @@ "persons_search_placeholder": "Search names...", "persons_empty_heading": "No persons found.", "persons_empty_text": "Try a different search term.", + "persons_empty_filtered": "No persons match these filters.", + "persons_filter_group_label": "Filter", + "persons_filter_type_person": "Person", + "persons_filter_type_group": "Group", + "persons_filter_type_institution": "Institution", + "persons_filter_family_only": "Family only", + "persons_filter_has_documents": "With documents", + "persons_toggle_show_all": "Show all", + "persons_toggle_needs_review": "Needs review ({count})", + "person_badge_unconfirmed": "unconfirmed", + "persons_review_heading": "Review persons", + "persons_review_intro": "Import-generated persons not yet confirmed. Merge, rename, confirm or delete.", + "persons_review_action_merge": "Merge", + "persons_review_action_rename": "Rename", + "persons_review_action_confirm": "Confirm", + "persons_review_action_delete": "Delete", + "persons_review_action_cancel": "Cancel", + "persons_review_action_save": "Save", + "persons_review_empty": "No persons to review.", + "persons_review_delete_confirm_title": "Delete person", + "persons_review_delete_confirm_text": "This person will be permanently deleted. Document references are kept but lose this person.", + "persons_review_delete_confirm_button": "Delete person", + "persons_review_merge_label": "Merge into which person?", + "persons_field_first_name": "First name", + "persons_field_last_name": "Last name", "persons_new_heading": "New person", "persons_section_details": "Person details", "person_edit_heading": "Edit person", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index 1cbd3eda..47906c0c 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -130,6 +130,31 @@ "persons_search_placeholder": "Buscar nombres...", "persons_empty_heading": "No se encontraron personas.", "persons_empty_text": "Pruebe con otro término de búsqueda.", + "persons_empty_filtered": "Ninguna persona coincide con estos filtros.", + "persons_filter_group_label": "Filtro", + "persons_filter_type_person": "Persona", + "persons_filter_type_group": "Grupo", + "persons_filter_type_institution": "Institución", + "persons_filter_family_only": "Solo familia", + "persons_filter_has_documents": "Con documentos", + "persons_toggle_show_all": "Mostrar todo", + "persons_toggle_needs_review": "Por revisar ({count})", + "person_badge_unconfirmed": "sin confirmar", + "persons_review_heading": "Revisar personas", + "persons_review_intro": "Personas generadas por la importación aún sin confirmar. Fusionar, renombrar, confirmar o eliminar.", + "persons_review_action_merge": "Fusionar", + "persons_review_action_rename": "Renombrar", + "persons_review_action_confirm": "Confirmar", + "persons_review_action_delete": "Eliminar", + "persons_review_action_cancel": "Cancelar", + "persons_review_action_save": "Guardar", + "persons_review_empty": "No hay personas por revisar.", + "persons_review_delete_confirm_title": "Eliminar persona", + "persons_review_delete_confirm_text": "Esta persona se eliminará de forma permanente. Las referencias de documentos se conservan pero pierden a esta persona.", + "persons_review_delete_confirm_button": "Eliminar persona", + "persons_review_merge_label": "¿Fusionar con qué persona?", + "persons_field_first_name": "Nombre", + "persons_field_last_name": "Apellido", "persons_new_heading": "Nueva persona", "persons_section_details": "Datos de la persona", "person_edit_heading": "Editar persona", diff --git a/frontend/src/lib/generated/api.ts b/frontend/src/lib/generated/api.ts index 5ef387b4..48b4b83d 100644 --- a/frontend/src/lib/generated/api.ts +++ b/frontend/src/lib/generated/api.ts @@ -78,12 +78,28 @@ export interface paths { get: operations["getPerson"]; put: operations["updatePerson"]; post?: never; - delete?: never; + delete: operations["deletePerson"]; options?: never; head?: never; patch?: never; trace?: never; }; + "/api/persons/{id}/confirm": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch: operations["confirmPerson"]; + trace?: never; + }; "/api/documents/{id}": { parameters: { query?: never; @@ -2244,6 +2260,17 @@ export interface components { familyMember?: boolean; provisional?: boolean; }; + PersonSearchResult: { + items: components["schemas"]["PersonSummaryDTO"][]; + /** Format: int64 */ + totalElements: number; + /** Format: int32 */ + pageNumber: number; + /** Format: int32 */ + pageSize: number; + /** Format: int32 */ + totalPages: number; + }; InferredRelationshipWithPersonDTO: { person: components["schemas"]["PersonNodeDTO"]; label: string; @@ -3129,8 +3156,14 @@ export interface operations { parameters: { query?: { q?: string; - size?: number; + type?: "PERSON" | "INSTITUTION" | "GROUP" | "UNKNOWN" | "SKIP"; + familyOnly?: boolean; + hasDocuments?: boolean; + provisional?: boolean; + review?: boolean; sort?: string; + page?: number; + size?: number; }; header?: never; path?: never; @@ -3144,11 +3177,53 @@ export interface operations { [name: string]: unknown; }; content: { - "*/*": components["schemas"]["PersonSummaryDTO"][]; + "*/*": components["schemas"]["PersonSearchResult"]; }; }; }; }; + confirmPerson: { + parameters: { + query?: never; + header?: never; + path: { + id: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["Person"]; + }; + }; + }; + }; + deletePerson: { + parameters: { + query?: never; + header?: never; + path: { + id: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description No Content */ + 204: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; createPerson: { parameters: { query?: never; diff --git a/frontend/src/lib/person/PersonCard.svelte b/frontend/src/lib/person/PersonCard.svelte new file mode 100644 index 00000000..f1c38093 --- /dev/null +++ b/frontend/src/lib/person/PersonCard.svelte @@ -0,0 +1,146 @@ + + + +

+ +
+ {#if showGlyph} + {#if person.personType === 'INSTITUTION'} + + + + {:else if person.personType === 'GROUP'} + + + + {:else} + + + {/if} + {:else} + {initials} + {/if} +
+ + +

+ {person.displayName} +

+ + {#if isUnconfirmed} + + + + {m.person_badge_unconfirmed()} + + {:else if person.personType && person.personType !== 'PERSON'} + + {/if} + + + {#if person.alias} +

„{person.alias}"

+ {/if} + + + {#if person.birthYear || person.deathYear} +

+ {formatLifeDateRange(person.birthYear, person.deathYear)} +

+ {/if} + + + {#if documentCount > 0} + + {documentCount === 1 + ? m.person_card_doc_count_one() + : m.person_card_doc_count_many({ count: documentCount })} + + {/if} +
+ diff --git a/frontend/src/lib/person/PersonCard.svelte.test.ts b/frontend/src/lib/person/PersonCard.svelte.test.ts new file mode 100644 index 00000000..a7cc7345 --- /dev/null +++ b/frontend/src/lib/person/PersonCard.svelte.test.ts @@ -0,0 +1,87 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import { cleanup, render } from 'vitest-browser-svelte'; +import { page } from 'vitest/browser'; +import PersonCard from './PersonCard.svelte'; +import type { components } from '$lib/generated/api'; + +type Person = components['schemas']['PersonSummaryDTO']; + +const makePerson = (overrides: Partial = {}): Person => ({ + id: 'p-1', + firstName: 'Anna', + lastName: 'Schmidt', + displayName: 'Anna Schmidt', + personType: 'PERSON', + familyMember: false, + provisional: false, + documentCount: 0, + ...overrides +}); + +afterEach(cleanup); + +describe('PersonCard — confirmed person', () => { + it('renders the display name', async () => { + render(PersonCard, { props: { person: makePerson() } }); + await expect.element(page.getByText('Anna Schmidt')).toBeVisible(); + }); + + it('does not show an unconfirmed badge for a confirmed person', async () => { + render(PersonCard, { props: { person: makePerson() } }); + await expect.element(page.getByText('unbestätigt')).not.toBeInTheDocument(); + }); +}); + +describe('PersonCard — unconfirmed badge keys off provisional only (badge ⇔ count ⇔ triage parity)', () => { + it('renders without throwing when lastName is null', async () => { + // Before the fix, `lastName[0]` threw at render for a null lastName. Empty-name + // crash-safety is a SEPARATE concern from the badge: the placeholder glyph renders + // regardless, but the "unbestätigt" badge only fires when provisional is true. + const person = makePerson({ + lastName: null as unknown as string, + displayName: '?', + provisional: true + }); + render(PersonCard, { props: { person } }); + // No throw + provisional → the badge is shown. + await expect.element(page.getByText('unbestätigt')).toBeVisible(); + }); + + it('shows an unbestätigt badge for a provisional person', async () => { + render(PersonCard, { props: { person: makePerson({ provisional: true }) } }); + await expect.element(page.getByText('unbestätigt')).toBeVisible(); + }); + + it('does NOT show the badge for a "?" name when not provisional', async () => { + // Empty/"?" name alone is no longer treated as unconfirmed — only `provisional` is. + // This keeps the badge in lockstep with needsReviewCount and the /persons/review list. + render(PersonCard, { + props: { + person: makePerson({ firstName: undefined, lastName: '?', displayName: '?' }) + } + }); + await expect.element(page.getByText('unbestätigt')).not.toBeInTheDocument(); + }); + + it('does NOT show the badge for an UNKNOWN type when not provisional', async () => { + render(PersonCard, { + props: { person: makePerson({ personType: 'UNKNOWN', displayName: 'Unklar' }) } + }); + await expect.element(page.getByText('unbestätigt')).not.toBeInTheDocument(); + }); + + it('renders the placeholder glyph (never a "?" initial) for an empty name even without provisional', async () => { + // Crash-safety branch: a null/empty lastName must not throw and must not show "?". + render(PersonCard, { + props: { + person: makePerson({ + firstName: undefined, + lastName: null as unknown as string, + displayName: 'Unbekannt' + }) + } + }); + await expect.element(page.getByText('Unbekannt')).toBeVisible(); + await expect.element(page.getByText('unbestätigt')).not.toBeInTheDocument(); + }); +}); diff --git a/frontend/src/lib/person/PersonFilterBar.svelte b/frontend/src/lib/person/PersonFilterBar.svelte new file mode 100644 index 00000000..ae2bf9c2 --- /dev/null +++ b/frontend/src/lib/person/PersonFilterBar.svelte @@ -0,0 +1,162 @@ + + +
+ +
+ {#each typeChips as chip (chip.value)} + {@const active = type === chip.value} + + {/each} + + + + +
+ + + {#if canWrite} + + + {/if} +
diff --git a/frontend/src/lib/person/PersonFilterBar.svelte.test.ts b/frontend/src/lib/person/PersonFilterBar.svelte.test.ts new file mode 100644 index 00000000..f1069f90 --- /dev/null +++ b/frontend/src/lib/person/PersonFilterBar.svelte.test.ts @@ -0,0 +1,90 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { cleanup, render } from 'vitest-browser-svelte'; +import { page as browserPage } from 'vitest/browser'; +import { goto } from '$app/navigation'; +import { page } from '$app/state'; +import PersonFilterBar from './PersonFilterBar.svelte'; + +vi.mock('$app/navigation', () => ({ goto: vi.fn() })); +vi.mock('$app/state', () => ({ page: { url: new URL('http://localhost/persons') } })); + +const gotoMock = vi.mocked(goto); + +function setUrl(search: string) { + (page as unknown as { url: URL }).url = new URL(`http://localhost/persons${search}`); +} + +beforeEach(() => { + gotoMock.mockClear(); + setUrl(''); +}); +afterEach(cleanup); + +const baseProps = { + type: undefined, + familyOnly: false, + hasDocuments: false, + review: false, + needsReviewCount: 12, + canWrite: true +}; + +describe('PersonFilterBar — chip activation', () => { + it('activating a type chip sets type and resets page', async () => { + setUrl('?page=3&q=Anna'); + render(PersonFilterBar, { props: baseProps }); + + await browserPage.getByRole('button', { name: 'Institution' }).click(); + + expect(gotoMock).toHaveBeenCalled(); + const target = gotoMock.mock.calls.at(-1)![0] as string; + const url = new URL(target, 'http://localhost'); + expect(url.searchParams.get('type')).toBe('INSTITUTION'); + expect(url.searchParams.get('q')).toBe('Anna'); // preserved + expect(url.searchParams.get('page')).toBeNull(); // reset + }); + + it('activating "Nur Familie" sets familyOnly=true', async () => { + render(PersonFilterBar, { props: baseProps }); + + await browserPage.getByRole('button', { name: 'Nur Familie' }).click(); + + const target = gotoMock.mock.calls.at(-1)![0] as string; + const url = new URL(target, 'http://localhost'); + expect(url.searchParams.get('familyOnly')).toBe('true'); + }); + + it('deactivating an already-active chip removes the param', async () => { + render(PersonFilterBar, { props: { ...baseProps, hasDocuments: true } }); + + await browserPage.getByRole('button', { name: 'Mit Dokumenten' }).click(); + + const target = gotoMock.mock.calls.at(-1)![0] as string; + const url = new URL(target, 'http://localhost'); + expect(url.searchParams.get('hasDocuments')).toBeNull(); + }); +}); + +describe('PersonFilterBar — review toggle', () => { + it('renders a switch with the needs-review count in its accessible name', async () => { + render(PersonFilterBar, { props: baseProps }); + await expect + .element(browserPage.getByRole('switch', { name: /Zu prüfen \(12\)/ })) + .toBeVisible(); + }); + + it('is hidden for users without write permission', async () => { + render(PersonFilterBar, { props: { ...baseProps, canWrite: false } }); + await expect.element(browserPage.getByRole('switch')).not.toBeInTheDocument(); + }); + + it('toggling on sets review=true', async () => { + render(PersonFilterBar, { props: baseProps }); + + await browserPage.getByRole('switch').click(); + + const target = gotoMock.mock.calls.at(-1)![0] as string; + const url = new URL(target, 'http://localhost'); + expect(url.searchParams.get('review')).toBe('true'); + }); +}); diff --git a/frontend/src/lib/person/PersonMultiSelect.svelte b/frontend/src/lib/person/PersonMultiSelect.svelte index 80a4e97b..41b392a0 100644 --- a/frontend/src/lib/person/PersonMultiSelect.svelte +++ b/frontend/src/lib/person/PersonMultiSelect.svelte @@ -34,9 +34,10 @@ function handleInput() { } loading = true; try { - const res = await fetch(`/api/persons?q=${encodeURIComponent(searchTerm)}`); + const res = await fetch(`/api/persons?review=true&q=${encodeURIComponent(searchTerm)}`); if (res.ok) { - const all: Person[] = await res.json(); + const body = await res.json(); + const all: Person[] = body.items ?? []; results = all.filter((p) => !selectedPersons.some((s) => s.id === p.id)); } } catch { diff --git a/frontend/src/lib/person/PersonMultiSelect.svelte.spec.ts b/frontend/src/lib/person/PersonMultiSelect.svelte.spec.ts index f188ae74..86c26274 100644 --- a/frontend/src/lib/person/PersonMultiSelect.svelte.spec.ts +++ b/frontend/src/lib/person/PersonMultiSelect.svelte.spec.ts @@ -34,11 +34,12 @@ const PERSONS = [ ]; function mockFetch(persons = PERSONS) { + // /api/persons now returns a paged { items } envelope. vi.stubGlobal( 'fetch', vi.fn().mockResolvedValue({ ok: true, - json: vi.fn().mockResolvedValue(persons) + json: vi.fn().mockResolvedValue({ items: persons }) }) ); } diff --git a/frontend/src/lib/person/PersonReviewRow.svelte b/frontend/src/lib/person/PersonReviewRow.svelte new file mode 100644 index 00000000..044ff966 --- /dev/null +++ b/frontend/src/lib/person/PersonReviewRow.svelte @@ -0,0 +1,158 @@ + + +
  • +
    + +
    + +
    + +
    +

    {person.displayName}

    +

    + {documentCount === 1 + ? m.person_card_doc_count_one() + : m.person_card_doc_count_many({ count: documentCount })} +

    +
    + +
    + + +
    + + +
    +
    { + const ok = await confirm({ + title: m.persons_review_delete_confirm_title(), + body: m.persons_review_delete_confirm_text(), + confirmLabel: m.persons_review_delete_confirm_button(), + destructive: true + }); + if (!ok) cancel(); + }} + > + + +
    +
    +
    + + {#if mode === 'rename'} +
    { + return () => { + mode = 'idle'; + }; + }} + class="flex flex-wrap items-end gap-2" + > + + + + + + +
    + {/if} + + {#if mode === 'merge'} +
    { + return () => { + mode = 'idle'; + }; + }} + class="flex flex-wrap items-end gap-2" + > + + +
    + (mergeTargetId = value)} + /> +
    + + +
    + {/if} +
    • diff --git a/frontend/src/lib/person/PersonTypeahead.svelte b/frontend/src/lib/person/PersonTypeahead.svelte index 7622fb84..1a650757 100644 --- a/frontend/src/lib/person/PersonTypeahead.svelte +++ b/frontend/src/lib/person/PersonTypeahead.svelte @@ -79,8 +79,12 @@ const typeahead = createTypeahead({ return res.ok ? filter(await res.json()) : []; } if (term.length < 1) return []; - const res = await fetch(`/api/persons?q=${encodeURIComponent(term)}`); - return res.ok ? filter(await res.json()) : []; + // review=true so the typeahead searches the whole directory (incl. provisional / + // zero-document persons), not just the clean reader subset. + const res = await fetch(`/api/persons?review=true&q=${encodeURIComponent(term)}`); + if (!res.ok) return []; + const body = await res.json(); + return filter(body.items ?? []); }, debounceMs: 300 }); diff --git a/frontend/src/lib/person/PersonTypeahead.svelte.spec.ts b/frontend/src/lib/person/PersonTypeahead.svelte.spec.ts index 81761078..7ae14ff2 100644 --- a/frontend/src/lib/person/PersonTypeahead.svelte.spec.ts +++ b/frontend/src/lib/person/PersonTypeahead.svelte.spec.ts @@ -24,12 +24,18 @@ const PERSONS = [ ]; function mockFetchWithPersons(persons = PERSONS) { + // The directory endpoint (/api/persons?…) now returns a paged { items } envelope; the + // correspondents endpoint still returns a bare array. Branch the mock on the URL. vi.stubGlobal( 'fetch', - vi.fn().mockResolvedValue({ - ok: true, - json: vi.fn().mockResolvedValue(persons) - }) + vi.fn().mockImplementation((url: string) => + Promise.resolve({ + ok: true, + json: vi + .fn() + .mockResolvedValue(url.includes('/correspondents') ? persons : { items: persons }) + }) + ) ); } @@ -266,7 +272,9 @@ describe('PersonTypeahead – correspondent mode', () => { await waitForDebounce(); const fetchMock = globalThis.fetch as ReturnType; - expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('/api/persons?q=Anna')); + expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining('/api/persons?review=true&q=Anna') + ); }); }); diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte index 466c00ed..39261606 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte @@ -197,16 +197,16 @@ onMount(() => { // Defensive client-side cap — server-side enforcement is tracked // separately. Markus on PR #629. const res = await fetch( - `/api/persons?q=${encodeURIComponent(query)}&limit=${SEARCH_RESULT_LIMIT}` + `/api/persons?review=true&q=${encodeURIComponent(query)}&size=${SEARCH_RESULT_LIMIT}` ); if (id !== requestId) return; if (!res.ok) { dropdownState.items = []; return; } - const data = (await res.json()) as Person[]; + const body = (await res.json()) as { items?: Person[] }; if (id !== requestId) return; - dropdownState.items = data.slice(0, SEARCH_RESULT_LIMIT); + dropdownState.items = (body.items ?? []).slice(0, SEARCH_RESULT_LIMIT); } catch { if (id !== requestId) return; dropdownState.items = []; diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts index 9ce23358..1dfdd053 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts @@ -53,14 +53,14 @@ const ANNA: Person = { function mockFetchWithPersons(persons: Person[] = [AUGUSTE, ANNA]) { vi.stubGlobal( 'fetch', - vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue(persons) }) + vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: persons }) }) ); } function mockFetchEmpty() { vi.stubGlobal( 'fetch', - vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([]) }) + vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: [] }) }) ); } @@ -132,28 +132,30 @@ describe('PersonMentionEditor — typeahead', () => { it('hits /api/persons?q= with the typed query', async () => { const fetchMock = vi .fn() - .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) }); + .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: [AUGUSTE] }) }); vi.stubGlobal('fetch', fetchMock); renderHost(); await userEvent.type(page.getByRole('textbox'), '@Aug'); await vi.waitFor(() => { - expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('/api/persons?q=Aug')); + expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining('/api/persons?review=true&q=Aug') + ); }); }); it('appends &limit=5 to the fetch URL (defensive client-side cap, Markus on PR #629)', async () => { const fetchMock = vi .fn() - .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) }); + .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: [AUGUSTE] }) }); vi.stubGlobal('fetch', fetchMock); renderHost(); await userEvent.type(page.getByRole('textbox'), '@Aug'); await vi.waitFor(() => { - expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('limit=5')); + expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('size=5')); }); }); @@ -206,7 +208,7 @@ describe('PersonMentionEditor — AC-2/3: search input drives fetch', () => { it('editing the search input fires a debounced fetch with the new query', async () => { const fetchMock = vi .fn() - .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) }); + .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: [AUGUSTE] }) }); vi.stubGlobal('fetch', fetchMock); renderHost(); @@ -243,7 +245,7 @@ describe('PersonMentionEditor — AC-2/3: search input drives fetch', () => { // Sara on PR #629 round 3. const fetchMock = vi .fn() - .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) }); + .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: [AUGUSTE] }) }); vi.stubGlobal('fetch', fetchMock); renderHost(); @@ -270,7 +272,7 @@ describe('PersonMentionEditor — AC-2/3: search input drives fetch', () => { it('clearing the search input clears the list without firing a fetch', async () => { const fetchMock = vi .fn() - .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) }); + .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: [AUGUSTE] }) }); vi.stubGlobal('fetch', fetchMock); renderHost(); @@ -323,10 +325,12 @@ describe('PersonMentionEditor — whitespace-only query', () => { describe('PersonMentionEditor — stale-response race', () => { it('discards a stale response that resolves after the search has been cleared', async () => { - let resolveFetch!: (v: { ok: boolean; json: () => Promise }) => void; - const pendingResponse = new Promise<{ ok: boolean; json: () => Promise }>((r) => { - resolveFetch = r; - }); + let resolveFetch!: (v: { ok: boolean; json: () => Promise<{ items: Person[] }> }) => void; + const pendingResponse = new Promise<{ ok: boolean; json: () => Promise<{ items: Person[] }> }>( + (r) => { + resolveFetch = r; + } + ); const fetchMock = vi.fn().mockReturnValue(pendingResponse); vi.stubGlobal('fetch', fetchMock); renderHost(); @@ -334,7 +338,9 @@ describe('PersonMentionEditor — stale-response race', () => { // Open the dropdown and let the debounce fire so a fetch is in flight. await userEvent.type(page.getByRole('textbox'), '@Aug'); await vi.waitFor(() => { - expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('/api/persons?q=Aug')); + expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining('/api/persons?review=true&q=Aug') + ); }); // Clear the search input *before* the fetch resolves. @@ -342,7 +348,7 @@ describe('PersonMentionEditor — stale-response race', () => { await expect.element(page.getByRole('searchbox')).toHaveValue(''); // The stale fetch now resolves with persons. The dropdown must stay empty. - resolveFetch({ ok: true, json: () => Promise.resolve([AUGUSTE]) }); + resolveFetch({ ok: true, json: () => Promise.resolve({ items: [AUGUSTE] }) }); // Flush pending Svelte reactivity so any (non-)update from the stale // fetch resolution has landed before we assert. expect.element already // polls, so no fixed-timeout fallback is needed. Sara on PR #629 round 4. diff --git a/frontend/src/lib/shared/server/permissions.spec.ts b/frontend/src/lib/shared/server/permissions.spec.ts new file mode 100644 index 00000000..4e93b11f --- /dev/null +++ b/frontend/src/lib/shared/server/permissions.spec.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest'; +import { hasWriteAll } from './permissions'; + +type Locals = { user?: { groups?: { permissions: string[] }[] } }; + +const localsWith = (permissions: string[][]): Locals => ({ + user: { groups: permissions.map((p) => ({ permissions: p })) } +}); + +describe('hasWriteAll', () => { + it('returns true when a group grants WRITE_ALL', () => { + expect(hasWriteAll(localsWith([['READ_ALL', 'WRITE_ALL']]))).toBe(true); + }); + + it('returns true when WRITE_ALL is in any of several groups', () => { + expect(hasWriteAll(localsWith([['READ_ALL'], ['WRITE_ALL']]))).toBe(true); + }); + + it('returns false when no group grants WRITE_ALL', () => { + expect(hasWriteAll(localsWith([['READ_ALL'], ['ANNOTATE_ALL']]))).toBe(false); + }); + + it('returns false for an anonymous user (no locals.user)', () => { + expect(hasWriteAll({})).toBe(false); + }); + + it('returns false when the user has no groups', () => { + expect(hasWriteAll({ user: {} })).toBe(false); + }); +}); diff --git a/frontend/src/lib/shared/server/permissions.ts b/frontend/src/lib/shared/server/permissions.ts new file mode 100644 index 00000000..11053859 --- /dev/null +++ b/frontend/src/lib/shared/server/permissions.ts @@ -0,0 +1,14 @@ +/** + * Server-side permission predicates derived from the authenticated user in `locals`. + * + * The user shape is intentionally narrowed to the only field these checks read + * (`groups[].permissions`) so the helper works against `App.Locals` without importing it. + */ +type PermissionLocals = { + user?: { groups?: { permissions: string[] }[] } | null; +}; + +/** True when any of the user's groups grants WRITE_ALL. False for anonymous users. */ +export function hasWriteAll(locals: PermissionLocals): boolean { + return locals.user?.groups?.some((group) => group.permissions.includes('WRITE_ALL')) ?? false; +} diff --git a/frontend/src/routes/+page.server.ts b/frontend/src/routes/+page.server.ts index b217452f..7f7daa6e 100644 --- a/frontend/src/routes/+page.server.ts +++ b/frontend/src/routes/+page.server.ts @@ -52,7 +52,7 @@ export async function load({ fetch, parent }) { await Promise.allSettled(readerFetches); const readerStats = settled(statsRes); - const topPersons = settled(topPersonsRes) ?? []; + const topPersons = settled<{ items: PersonSummaryDTO[] }>(topPersonsRes)?.items ?? []; const searchData = settled<{ items: { document: Document }[] }>(recentDocsRes); const recentDocs = searchData?.items.map((i) => i.document) ?? []; const recentStories = settled(recentStoriesRes) ?? []; diff --git a/frontend/src/routes/documents/[id]/edit/+page.server.ts b/frontend/src/routes/documents/[id]/edit/+page.server.ts index 4cd24a81..c012e3b9 100644 --- a/frontend/src/routes/documents/[id]/edit/+page.server.ts +++ b/frontend/src/routes/documents/[id]/edit/+page.server.ts @@ -2,6 +2,7 @@ import { error, fail, redirect } from '@sveltejs/kit'; import { env } from '$env/dynamic/private'; import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; import { parseBackendError, getErrorMessage } from '$lib/shared/errors'; +import { hasWriteAll } from '$lib/shared/server/permissions'; export async function load({ params, @@ -15,30 +16,21 @@ export async function load({ depends: (dep: string) => void; }) { depends('app:document'); - const canWrite = - locals.user?.groups?.some((g: { permissions: string[] }) => - g.permissions.includes('WRITE_ALL') - ) ?? false; - if (!canWrite) throw error(403, 'Forbidden'); + if (!hasWriteAll(locals)) throw error(403, 'Forbidden'); const { id } = params; const api = createApiClient(fetch); - const [docResult, personsResult] = await Promise.all([ - api.GET('/api/documents/{id}', { params: { path: { id } } }), - api.GET('/api/persons') - ]); + const docResult = await api.GET('/api/documents/{id}', { params: { path: { id } } }); if (!docResult.response.ok) { throw error(docResult.response.status, getErrorMessage(extractErrorCode(docResult.error))); } - if (!personsResult.response.ok) { - throw error(personsResult.response.status, getErrorMessage('INTERNAL_ERROR')); - } return { document: docResult.data!, - persons: personsResult.data + // Sender/receiver editing uses PersonTypeahead (self-fetching); no full list is consumed. + persons: [] as never[] }; } diff --git a/frontend/src/routes/documents/new/+page.server.ts b/frontend/src/routes/documents/new/+page.server.ts index b54496c3..2d6c2fd8 100644 --- a/frontend/src/routes/documents/new/+page.server.ts +++ b/frontend/src/routes/documents/new/+page.server.ts @@ -2,6 +2,7 @@ import { fail, redirect } from '@sveltejs/kit'; import { env } from '$env/dynamic/private'; import { createApiClient } from '$lib/shared/api.server'; import { parseBackendError, getErrorMessage } from '$lib/shared/errors'; +import { hasWriteAll } from '$lib/shared/server/permissions'; export async function load({ fetch, @@ -12,11 +13,7 @@ export async function load({ locals: App.Locals; url: URL; }) { - const canWrite = - locals.user?.groups?.some((g: { permissions: string[] }) => - g.permissions.includes('WRITE_ALL') - ) ?? false; - if (!canWrite) throw redirect(303, '/'); + if (!hasWriteAll(locals)) throw redirect(303, '/'); const senderId = url.searchParams.get('senderId') || ''; const receiverId = url.searchParams.get('receiverId') || ''; @@ -57,10 +54,12 @@ export async function load({ ); } - const [personsResult] = await Promise.all([api.GET('/api/persons'), ...requests]); + await Promise.all(requests); return { - persons: personsResult.response.ok ? personsResult.data : [], + // Sender/receiver selection uses PersonTypeahead, which fetches its own results on + // demand — the page never consumes a pre-loaded full person list, so none is fetched. + persons: [] as never[], initialSenderId: senderId, initialSenderName, initialReceivers diff --git a/frontend/src/routes/persons/+page.server.ts b/frontend/src/routes/persons/+page.server.ts index 62a1f8cf..3f04ff7c 100644 --- a/frontend/src/routes/persons/+page.server.ts +++ b/frontend/src/routes/persons/+page.server.ts @@ -1,25 +1,55 @@ import { error } from '@sveltejs/kit'; import { createApiClient } from '$lib/shared/api.server'; import { getErrorMessage } from '$lib/shared/errors'; +import { hasWriteAll } from '$lib/shared/server/permissions'; + +const PAGE_SIZE = 50; + +type PersonType = 'PERSON' | 'INSTITUTION' | 'GROUP'; + +function parseType(raw: string | null): PersonType | undefined { + return raw === 'PERSON' || raw === 'INSTITUTION' || raw === 'GROUP' ? raw : undefined; +} export async function load({ url, fetch, locals }) { const q = url.searchParams.get('q') || ''; + const page = Math.max(0, Number.parseInt(url.searchParams.get('page') ?? '0', 10) || 0); + const review = + url.searchParams.get('review') === '1' || url.searchParams.get('review') === 'true'; + const type = parseType(url.searchParams.get('type')); + const familyOnly = url.searchParams.get('familyOnly') === 'true'; + const hasDocuments = url.searchParams.get('hasDocuments') === 'true'; + const api = createApiClient(fetch); - const canWrite = - (locals.user as { groups?: { permissions: string[] }[] } | undefined)?.groups?.some((g) => - g.permissions.includes('WRITE_ALL') - ) ?? false; + const canWrite = hasWriteAll(locals); - const [personsResult, statsResult] = await Promise.all([ - api.GET('/api/persons', { params: { query: { q: q || undefined } } }), - api.GET('/api/stats', {}) + const filters = { + q: q || undefined, + type, + familyOnly: familyOnly || undefined, + hasDocuments: hasDocuments || undefined, + review: review || undefined, + page, + size: PAGE_SIZE + }; + + // The "Zu prüfen (N)" link count is the totalElements of a provisional-only query. A size=1 + // page keeps the extra request cheap — we only need the count, not the rows. + const [personsResult, statsResult, reviewCountResult] = await Promise.all([ + api.GET('/api/persons', { params: { query: filters } }), + api.GET('/api/stats', {}), + canWrite + ? api.GET('/api/persons', { params: { query: { provisional: true, review: true, size: 1 } } }) + : Promise.resolve(null) ]); if (!personsResult.response.ok) { throw error(personsResult.response.status, getErrorMessage(undefined)); } + const result = personsResult.data!; + const stats = statsResult.response.ok ? { totalPersons: statsResult.data!.totalPersons ?? 0, @@ -27,5 +57,21 @@ export async function load({ url, fetch, locals }) { } : { totalPersons: 0, totalDocuments: 0 }; - return { persons: personsResult.data!, stats, q, canWrite }; + const needsReviewCount = + reviewCountResult && reviewCountResult.response.ok + ? (reviewCountResult.data!.totalElements ?? 0) + : 0; + + return { + persons: result.items, + totalElements: result.totalElements, + totalPages: result.totalPages, + pageNumber: result.pageNumber, + pageSize: result.pageSize, + filters: { type, familyOnly, hasDocuments, review }, + needsReviewCount, + stats, + q, + canWrite + }; } diff --git a/frontend/src/routes/persons/+page.svelte b/frontend/src/routes/persons/+page.svelte index 5522d3ad..06080c83 100644 --- a/frontend/src/routes/persons/+page.svelte +++ b/frontend/src/routes/persons/+page.svelte @@ -1,9 +1,12 @@ @@ -32,7 +54,7 @@ function handleSearch() {
    -
    +

    {m.page_title_persons()}

    @@ -46,7 +68,7 @@ function handleSearch() {
    - +
    + + {#if data.canWrite} + + {m.persons_toggle_needs_review({ count: data.needsReviewCount })} + + {/if} + {#if data.canWrite}
    - {#if data.persons.length === 0} - + +
    + +
    + + {#if !hasResults} + {#if noFiltersActive} + + {:else} +
    +

    {m.persons_empty_filtered()}

    +
    + {/if} {:else}
    {#each data.persons as person (person.id)} - -
    - -
    - {#if person.personType && person.personType !== 'PERSON'} - - {#if person.personType === 'INSTITUTION'} - - {:else if person.personType === 'GROUP'} - - {:else} - - {/if} - - {:else} - {person.firstName ? person.firstName[0] : person.lastName[0]}{person.lastName[0]} - {/if} -
    - - -

    - {person.displayName} -

    - - {#if person.personType && person.personType !== 'PERSON'} - - {/if} - - - {#if person.alias} -

    „{person.alias}"

    - {/if} - - - {#if person.birthYear || person.deathYear} -

    - {formatLifeDateRange(person.birthYear, person.deathYear)} -

    - {/if} - - - {#if (person.documentCount ?? 0) > 0} - - {person.documentCount === 1 - ? m.person_card_doc_count_one() - : m.person_card_doc_count_many({ count: person.documentCount ?? 0 })} - - {/if} -
    -     + {/each}
    + + {/if}
    diff --git a/frontend/src/routes/persons/page.server.spec.ts b/frontend/src/routes/persons/page.server.spec.ts new file mode 100644 index 00000000..814fe2c8 --- /dev/null +++ b/frontend/src/routes/persons/page.server.spec.ts @@ -0,0 +1,137 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest'; + +vi.mock('$lib/shared/api.server', () => ({ + createApiClient: vi.fn(), + extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code +})); + +import { load } from './+page.server'; +import { createApiClient } from '$lib/shared/api.server'; + +beforeEach(() => vi.clearAllMocks()); + +function makeUrl(params: Record = {}) { + const url = new URL('http://localhost/persons'); + for (const [key, value] of Object.entries(params)) url.searchParams.set(key, value); + return url; +} + +/** Invokes the loader with a minimal event; the partial event is cast to satisfy the type. */ +function runLoad(url: URL, user: unknown) { + return load({ + url, + fetch: vi.fn() as unknown as typeof fetch, + request: new Request('http://localhost/persons'), + locals: { user } as App.Locals + } as unknown as Parameters[0]); +} + +/** Mock the typed client. /api/persons returns a paged envelope; /api/stats returns counts. */ +function mockApi() { + const personsResult = { + response: { ok: true, status: 200 }, + data: { items: [], totalElements: 0, pageNumber: 0, pageSize: 50, totalPages: 0 } + }; + // Loose `...args` signature (matching the documents loader spec) so call tuples aren't + // narrowed to length 1 — the test inspects calls[i][1].params.query. + const get = vi.fn((...args: unknown[]) => { + if (args[0] === '/api/stats') { + return Promise.resolve({ + response: { ok: true, status: 200 }, + data: { totalPersons: 7, totalDocuments: 3 } + }); + } + return Promise.resolve(personsResult); + }); + vi.mocked(createApiClient).mockReturnValue({ GET: get } as unknown as ReturnType< + typeof createApiClient + >); + return get; +} + +const writer = { groups: [{ permissions: ['READ_ALL', 'WRITE_ALL'] }] }; +const reader = { groups: [{ permissions: ['READ_ALL'] }] }; + +type GetCall = [string, { params: { query: Record } }]; + +/** Find the GET call to a path, optionally narrowing by a query predicate. */ +function findCall( + get: ReturnType, + path: string, + matchQuery?: (q: Record) => boolean +): GetCall | undefined { + return (get.mock.calls as unknown as GetCall[]).find( + (c) => c[0] === path && (!matchQuery || matchQuery(c[1].params.query)) + ); +} + +describe('persons page load — reader default', () => { + it('does NOT pass review when no review param is present (clean reader default)', async () => { + const get = mockApi(); + + await runLoad(makeUrl(), reader); + + const personsCall = findCall(get, '/api/persons'); + expect(personsCall?.[1].params.query.review).toBeUndefined(); + }); + + it('passes review=true when review=1 is in the URL', async () => { + const get = mockApi(); + + await runLoad(makeUrl({ review: '1' }), reader); + + const personsCall = findCall(get, '/api/persons'); + expect(personsCall?.[1].params.query.review).toBe(true); + }); +}); + +describe('persons page load — filter forwarding', () => { + it('forwards type, familyOnly, hasDocuments and page to the API', async () => { + const get = mockApi(); + + await runLoad( + makeUrl({ type: 'INSTITUTION', familyOnly: 'true', hasDocuments: 'true', page: '2' }), + reader + ); + + const personsCall = findCall(get, '/api/persons'); + expect(personsCall?.[1].params.query).toMatchObject({ + type: 'INSTITUTION', + familyOnly: true, + hasDocuments: true, + page: 2, + size: 50 + }); + }); + + it('clamps a negative page to 0', async () => { + const get = mockApi(); + + await runLoad(makeUrl({ page: '-5' }), reader); + + const personsCall = findCall(get, '/api/persons'); + expect(personsCall?.[1].params.query.page).toBe(0); + }); +}); + +describe('persons page load — needsReviewCount', () => { + it('fires a provisional count request for writers', async () => { + const get = mockApi(); + + await runLoad(makeUrl(), writer); + + const provisionalCall = findCall(get, '/api/persons', (query) => query.provisional === true); + expect(provisionalCall).toBeDefined(); + }); + + it('does not fire a provisional count request for read-only users', async () => { + const get = mockApi(); + + const result = await runLoad(makeUrl(), reader); + + const provisionalCall = findCall(get, '/api/persons', (query) => query.provisional === true); + expect(provisionalCall).toBeUndefined(); + expect(result.needsReviewCount).toBe(0); + expect(result.canWrite).toBe(false); + }); +}); diff --git a/frontend/src/routes/persons/page.svelte.spec.ts b/frontend/src/routes/persons/page.svelte.spec.ts index 19697f80..f20c22fe 100644 --- a/frontend/src/routes/persons/page.svelte.spec.ts +++ b/frontend/src/routes/persons/page.svelte.spec.ts @@ -6,6 +6,7 @@ import Page from './+page.svelte'; const tick = () => new Promise((r) => setTimeout(r, 0)); vi.mock('$app/navigation', () => ({ goto: vi.fn() })); +vi.mock('$app/state', () => ({ page: { url: new URL('http://localhost/persons') } })); const makePerson = (overrides = {}) => ({ id: '1', @@ -13,6 +14,8 @@ const makePerson = (overrides = {}) => ({ lastName: 'Mustermann', displayName: 'Max Mustermann', documentCount: 0, + provisional: false, + personType: 'PERSON', ...overrides }); @@ -24,7 +27,13 @@ const emptyData = { canBlogWrite: false, q: '', persons: [], - stats: defaultStats + stats: defaultStats, + totalElements: 0, + totalPages: 0, + pageNumber: 0, + pageSize: 50, + filters: { type: undefined, familyOnly: false, hasDocuments: false, review: false }, + needsReviewCount: 0 }; const dataWithPersons = { ...emptyData, diff --git a/frontend/src/routes/persons/page.svelte.test.ts b/frontend/src/routes/persons/page.svelte.test.ts index 71d90e6f..8a1cea20 100644 --- a/frontend/src/routes/persons/page.svelte.test.ts +++ b/frontend/src/routes/persons/page.svelte.test.ts @@ -16,6 +16,8 @@ vi.mock('$app/navigation', () => ({ onNavigate: () => () => {} })); +vi.mock('$app/state', () => ({ page: { url: new URL('http://localhost/persons') } })); + const { default: PersonsListPage } = await import('./+page.svelte'); afterEach(cleanup); @@ -31,8 +33,15 @@ const baseData = (overrides: Record = {}) => ({ birthYear?: number; deathYear?: number; documentCount?: number; + provisional?: boolean; }>, stats: { totalPersons: 0, totalDocuments: 0 }, + totalElements: 0, + totalPages: 0, + pageNumber: 0, + pageSize: 50, + filters: { type: undefined, familyOnly: false, hasDocuments: false, review: false }, + needsReviewCount: 0, canWrite: false, q: '', ...overrides diff --git a/frontend/src/routes/persons/review/+page.server.ts b/frontend/src/routes/persons/review/+page.server.ts new file mode 100644 index 00000000..c52121f2 --- /dev/null +++ b/frontend/src/routes/persons/review/+page.server.ts @@ -0,0 +1,107 @@ +import { error, fail } from '@sveltejs/kit'; +import { createApiClient, extractErrorCode } from '$lib/shared/api.server'; +import { getErrorMessage } from '$lib/shared/errors'; +import { hasWriteAll } from '$lib/shared/server/permissions'; + +const PAGE_SIZE = 50; + +export async function load({ url, fetch, locals }) { + const canWrite = hasWriteAll(locals); + + const page = Math.max(0, Number.parseInt(url.searchParams.get('page') ?? '0', 10) || 0); + const api = createApiClient(fetch); + + const result = await api.GET('/api/persons', { + params: { query: { provisional: true, review: true, page, size: PAGE_SIZE } } + }); + + if (!result.response.ok) { + throw error(result.response.status, getErrorMessage(undefined)); + } + + const data = result.data!; + + return { + persons: data.items, + totalElements: data.totalElements, + totalPages: data.totalPages, + pageNumber: data.pageNumber, + canWrite + }; +} + +export const actions = { + confirm: async ({ request, fetch }) => { + const id = (await request.formData()).get('id') as string; + const api = createApiClient(fetch); + const result = await api.PATCH('/api/persons/{id}/confirm', { + params: { path: { id } } + }); + if (!result.response.ok) { + return fail(result.response.status, { + error: getErrorMessage(extractErrorCode(result.error)) + }); + } + return { success: true }; + }, + + delete: async ({ request, fetch }) => { + const id = (await request.formData()).get('id') as string; + const api = createApiClient(fetch); + const result = await api.DELETE('/api/persons/{id}', { + params: { path: { id } } + }); + if (!result.response.ok) { + return fail(result.response.status, { + error: getErrorMessage(extractErrorCode(result.error)) + }); + } + return { success: true }; + }, + + merge: async ({ request, fetch }) => { + const formData = await request.formData(); + const id = formData.get('id') as string; + const targetPersonId = formData.get('targetPersonId') as string; + if (!targetPersonId) { + return fail(400, { error: getErrorMessage('INVALID_INPUT') }); + } + const api = createApiClient(fetch); + const result = await api.POST('/api/persons/{id}/merge', { + params: { path: { id } }, + body: { targetPersonId } + }); + if (!result.response.ok) { + return fail(result.response.status, { + error: getErrorMessage(extractErrorCode(result.error)) + }); + } + return { success: true }; + }, + + rename: async ({ request, fetch }) => { + const formData = await request.formData(); + const id = formData.get('id') as string; + const firstName = (formData.get('firstName') as string)?.trim() || undefined; + const lastName = (formData.get('lastName') as string)?.trim(); + const personType = (formData.get('personType') as string) || 'PERSON'; + if (!lastName) { + return fail(400, { error: getErrorMessage('INVALID_INPUT') }); + } + const api = createApiClient(fetch); + const result = await api.PUT('/api/persons/{id}', { + params: { path: { id } }, + body: { + firstName, + lastName, + personType: personType as 'PERSON' | 'INSTITUTION' | 'GROUP' | 'UNKNOWN' + } + }); + if (!result.response.ok) { + return fail(result.response.status, { + error: getErrorMessage(extractErrorCode(result.error)) + }); + } + return { success: true }; + } +}; diff --git a/frontend/src/routes/persons/review/+page.svelte b/frontend/src/routes/persons/review/+page.svelte new file mode 100644 index 00000000..0fc0cc3e --- /dev/null +++ b/frontend/src/routes/persons/review/+page.svelte @@ -0,0 +1,56 @@ + + + + {m.persons_review_heading()} + + +
    + + +
    +

    {m.persons_review_heading()}

    +

    {m.persons_review_intro()}

    +
    + + {#if form?.error} + + {/if} + + {#if !hasResults} +
    +

    {m.persons_review_empty()}

    +
    + {:else} +
      + {#each data.persons as person (person.id)} + + {/each} +
    + + + {/if} +