diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/PersonController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/PersonController.java index 6210f529..329a1969 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/PersonController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/PersonController.java @@ -63,27 +63,33 @@ public class PersonController { @PostMapping @RequirePermission(Permission.WRITE_ALL) public ResponseEntity createPerson(@Valid @RequestBody PersonUpdateDTO dto) { - if (dto.getFirstName() == null || dto.getFirstName().isBlank() - || dto.getLastName() == null || dto.getLastName().isBlank()) { - throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Vor- und Nachname sind Pflichtfelder"); - } - dto.setFirstName(dto.getFirstName().trim()); + validatePersonNames(dto); + if (dto.getFirstName() != null) dto.setFirstName(dto.getFirstName().trim()); dto.setLastName(dto.getLastName().trim()); + if (dto.getTitle() != null) dto.setTitle(dto.getTitle().trim()); return ResponseEntity.ok(personService.createPerson(dto)); } @PutMapping("/{id}") @RequirePermission(Permission.WRITE_ALL) public ResponseEntity updatePerson(@PathVariable UUID id, @Valid @RequestBody PersonUpdateDTO dto) { - if (dto.getFirstName() == null || dto.getFirstName().isBlank() - || dto.getLastName() == null || dto.getLastName().isBlank()) { - throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Vor- und Nachname sind Pflichtfelder"); - } - dto.setFirstName(dto.getFirstName().trim()); + validatePersonNames(dto); + if (dto.getFirstName() != null) dto.setFirstName(dto.getFirstName().trim()); dto.setLastName(dto.getLastName().trim()); + if (dto.getTitle() != null) dto.setTitle(dto.getTitle().trim()); return ResponseEntity.ok(personService.updatePerson(id, dto)); } + private void validatePersonNames(PersonUpdateDTO dto) { + if (dto.getLastName() == null || dto.getLastName().isBlank()) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Nachname ist Pflichtfeld"); + } + if (dto.getPersonType() == org.raddatz.familienarchiv.model.PersonType.PERSON + && (dto.getFirstName() == null || dto.getFirstName().isBlank())) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Vorname ist Pflichtfeld"); + } + } + @PostMapping("/{id}/merge") @ResponseStatus(HttpStatus.NO_CONTENT) @RequirePermission(Permission.WRITE_ALL) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/dto/PersonUpdateDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/dto/PersonUpdateDTO.java index b7026c70..b236811e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/dto/PersonUpdateDTO.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/dto/PersonUpdateDTO.java @@ -1,10 +1,14 @@ package org.raddatz.familienarchiv.dto; +import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.Size; import lombok.Data; +import org.raddatz.familienarchiv.model.PersonType; @Data public class PersonUpdateDTO { + @NotNull + private PersonType personType; @Size(max = 50) private String title; @Size(max = 100) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java index 187b793d..0db1d92d 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java @@ -13,6 +13,8 @@ public enum ErrorCode { PERSON_NOT_FOUND, /** A person name alias with the given ID does not exist. 404 */ ALIAS_NOT_FOUND, + /** The submitted personType value is not allowed (e.g. SKIP is import-only). 400 */ + INVALID_PERSON_TYPE, // --- Documents --- /** A document with the given ID does not exist. 404 */ diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java index e900dd4c..93ccdd5e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java @@ -109,8 +109,12 @@ public class PersonService { @Transactional public Person createPerson(PersonUpdateDTO dto) { + if (dto.getPersonType() == PersonType.SKIP) { + throw DomainException.badRequest(ErrorCode.INVALID_PERSON_TYPE, "SKIP is not a valid person type for manual creation"); + } validateYears(dto.getBirthYear(), dto.getDeathYear()); Person person = Person.builder() + .personType(dto.getPersonType()) .title(dto.getTitle() == null || dto.getTitle().isBlank() ? null : dto.getTitle().trim()) .firstName(dto.getFirstName()) .lastName(dto.getLastName()) @@ -136,9 +140,13 @@ public class PersonService { @Transactional public Person updatePerson(UUID id, PersonUpdateDTO dto) { + if (dto.getPersonType() == PersonType.SKIP) { + throw DomainException.badRequest(ErrorCode.INVALID_PERSON_TYPE, "SKIP is not a valid person type for manual editing"); + } validateYears(dto.getBirthYear(), dto.getDeathYear()); Person person = personRepository.findById(id) .orElseThrow(() -> DomainException.notFound(ErrorCode.PERSON_NOT_FOUND, "Person not found: " + id)); + person.setPersonType(dto.getPersonType()); person.setTitle(dto.getTitle() == null || dto.getTitle().isBlank() ? null : dto.getTitle().trim()); person.setFirstName(dto.getFirstName()); person.setLastName(dto.getLastName()); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/PersonControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/PersonControllerTest.java index 02973927..8539da8b 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/PersonControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/PersonControllerTest.java @@ -1,6 +1,9 @@ package org.raddatz.familienarchiv.controller; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.Person; import org.raddatz.familienarchiv.model.PersonNameAlias; @@ -25,6 +28,7 @@ import java.util.UUID; import org.raddatz.familienarchiv.dto.PersonSummaryDTO; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; @@ -183,19 +187,19 @@ class PersonControllerTest { @Test @WithMockUser(authorities = "WRITE_ALL") - void createPerson_returns400_whenFirstNameIsMissing() throws Exception { + void createPerson_returns400_whenPersonTypeIsPerson_andFirstNameIsMissing() throws Exception { mockMvc.perform(post("/api/persons") .contentType(MediaType.APPLICATION_JSON) - .content("{\"lastName\":\"Müller\"}")) + .content("{\"lastName\":\"Müller\",\"personType\":\"PERSON\"}")) .andExpect(status().isBadRequest()); } @Test @WithMockUser(authorities = "WRITE_ALL") - void createPerson_returns400_whenFirstNameIsBlank() throws Exception { + void createPerson_returns400_whenPersonTypeIsPerson_andFirstNameIsBlank() throws Exception { mockMvc.perform(post("/api/persons") .contentType(MediaType.APPLICATION_JSON) - .content("{\"firstName\":\" \",\"lastName\":\"Müller\"}")) + .content("{\"firstName\":\" \",\"lastName\":\"Müller\",\"personType\":\"PERSON\"}")) .andExpect(status().isBadRequest()); } @@ -204,7 +208,7 @@ class PersonControllerTest { void createPerson_returns400_whenLastNameIsMissing() throws Exception { mockMvc.perform(post("/api/persons") .contentType(MediaType.APPLICATION_JSON) - .content("{\"firstName\":\"Hans\"}")) + .content("{\"firstName\":\"Hans\",\"personType\":\"PERSON\"}")) .andExpect(status().isBadRequest()); } @@ -213,7 +217,7 @@ class PersonControllerTest { void createPerson_returns400_whenLastNameIsBlank() throws Exception { mockMvc.perform(post("/api/persons") .contentType(MediaType.APPLICATION_JSON) - .content("{\"firstName\":\"Hans\",\"lastName\":\" \"}")) + .content("{\"firstName\":\"Hans\",\"lastName\":\" \",\"personType\":\"PERSON\"}")) .andExpect(status().isBadRequest()); } @@ -225,11 +229,53 @@ class PersonControllerTest { mockMvc.perform(post("/api/persons") .contentType(MediaType.APPLICATION_JSON) - .content("{\"firstName\":\"Hans\",\"lastName\":\"Müller\"}")) + .content("{\"firstName\":\"Hans\",\"lastName\":\"Müller\",\"personType\":\"PERSON\"}")) .andExpect(status().isOk()) .andExpect(jsonPath("$.firstName").value("Hans")); } + @Test + @WithMockUser(authorities = "WRITE_ALL") + void createPerson_returns200_forInstitution_withoutFirstName() throws Exception { + Person saved = Person.builder().id(UUID.randomUUID()).lastName("Verlag GmbH").build(); + when(personService.createPerson(any(org.raddatz.familienarchiv.dto.PersonUpdateDTO.class))).thenReturn(saved); + + mockMvc.perform(post("/api/persons") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"lastName\":\"Verlag GmbH\",\"personType\":\"INSTITUTION\"}")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.lastName").value("Verlag GmbH")); + } + + @Test + @WithMockUser(authorities = "WRITE_ALL") + void createPerson_trimsTitle_beforePersisting() throws Exception { + ArgumentCaptor captor = + ArgumentCaptor.forClass(org.raddatz.familienarchiv.dto.PersonUpdateDTO.class); + Person saved = Person.builder().id(UUID.randomUUID()).firstName("Hans").lastName("Müller").build(); + when(personService.createPerson(captor.capture())).thenReturn(saved); + + mockMvc.perform(post("/api/persons") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"firstName\":\"Hans\",\"lastName\":\"Müller\",\"title\":\" Prof. \",\"personType\":\"PERSON\"}")) + .andExpect(status().isOk()); + + assertThat(captor.getValue().getTitle()).isEqualTo("Prof."); + } + + @Test + @WithMockUser(authorities = "WRITE_ALL") + void createPerson_returns400_whenPersonTypeIsSkip() throws Exception { + when(personService.createPerson(any())).thenThrow( + DomainException.badRequest(ErrorCode.INVALID_PERSON_TYPE, "SKIP is not a valid person type")); + + mockMvc.perform(post("/api/persons") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"lastName\":\"Müller\",\"personType\":\"SKIP\"}")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value("INVALID_PERSON_TYPE")); + } + // ─── PUT /api/persons/{id} ──────────────────────────────────────────────── @Test @@ -242,10 +288,10 @@ class PersonControllerTest { @Test @WithMockUser(authorities = "WRITE_ALL") - void updatePerson_returns400_whenFirstNameIsBlank() throws Exception { + void updatePerson_returns400_whenPersonTypeIsPerson_andFirstNameIsBlank() throws Exception { mockMvc.perform(put("/api/persons/{id}", UUID.randomUUID()) .contentType(MediaType.APPLICATION_JSON) - .content("{\"firstName\":\"\",\"lastName\":\"Müller\"}")) + .content("{\"firstName\":\"\",\"lastName\":\"Müller\",\"personType\":\"PERSON\"}")) .andExpect(status().isBadRequest()); } @@ -254,7 +300,7 @@ class PersonControllerTest { void updatePerson_returns400_whenLastNameIsNull() throws Exception { mockMvc.perform(put("/api/persons/{id}", UUID.randomUUID()) .contentType(MediaType.APPLICATION_JSON) - .content("{\"firstName\":\"Hans\"}")) + .content("{\"firstName\":\"Hans\",\"personType\":\"PERSON\"}")) .andExpect(status().isBadRequest()); } @@ -267,7 +313,7 @@ class PersonControllerTest { mockMvc.perform(put("/api/persons/{id}", id) .contentType(MediaType.APPLICATION_JSON) - .content("{\"firstName\":\"Hans\",\"lastName\":\"Müller\"}")) + .content("{\"firstName\":\"Hans\",\"lastName\":\"Müller\",\"personType\":\"PERSON\"}")) .andExpect(status().isOk()) .andExpect(jsonPath("$.lastName").value("Müller")); } @@ -317,11 +363,10 @@ class PersonControllerTest { @Test @WithMockUser(authorities = "WRITE_ALL") void updatePerson_returns400_whenLastNameIsBlank() throws Exception { - // firstName valid, lastName blank → second || operand = true → 400 UUID id = UUID.randomUUID(); mockMvc.perform(put("/api/persons/{id}", id) .contentType(MediaType.APPLICATION_JSON) - .content("{\"firstName\":\"Hans\",\"lastName\":\" \"}")) + .content("{\"firstName\":\"Hans\",\"lastName\":\" \",\"personType\":\"PERSON\"}")) .andExpect(status().isBadRequest()); } @@ -339,7 +384,7 @@ class PersonControllerTest { .contentType(MediaType.APPLICATION_JSON) .content("{\"firstName\":\"Maria\",\"lastName\":\"Raddatz\"," + "\"alias\":\"Oma Maria\",\"birthYear\":1901,\"deathYear\":1975," + - "\"notes\":\"Some notes\"}")) + "\"notes\":\"Some notes\",\"personType\":\"PERSON\"}")) .andExpect(status().isOk()) .andExpect(jsonPath("$.firstName").value("Maria")) .andExpect(jsonPath("$.alias").value("Oma Maria")) @@ -355,7 +400,7 @@ class PersonControllerTest { UUID id = UUID.randomUUID(); mockMvc.perform(put("/api/persons/{id}", id) .contentType(MediaType.APPLICATION_JSON) - .content("{\"firstName\":\"Hans\",\"lastName\":\"Müller\",\"notes\":\"" + oversizedNotes + "\"}")) + .content("{\"firstName\":\"Hans\",\"lastName\":\"Müller\",\"notes\":\"" + oversizedNotes + "\",\"personType\":\"PERSON\"}")) .andExpect(status().isBadRequest()); } @@ -366,7 +411,7 @@ class PersonControllerTest { UUID id = UUID.randomUUID(); mockMvc.perform(put("/api/persons/{id}", id) .contentType(MediaType.APPLICATION_JSON) - .content("{\"firstName\":\"" + oversizedFirstName + "\",\"lastName\":\"Müller\"}")) + .content("{\"firstName\":\"" + oversizedFirstName + "\",\"lastName\":\"Müller\",\"personType\":\"PERSON\"}")) .andExpect(status().isBadRequest()); } @@ -377,7 +422,7 @@ class PersonControllerTest { void createPerson_returns403_whenUserHasOnlyReadPermission() throws Exception { mockMvc.perform(post("/api/persons") .contentType(MediaType.APPLICATION_JSON) - .content("{\"firstName\":\"Hans\",\"lastName\":\"Müller\"}")) + .content("{\"firstName\":\"Hans\",\"lastName\":\"Müller\",\"personType\":\"PERSON\"}")) .andExpect(status().isForbidden()); } @@ -386,7 +431,7 @@ class PersonControllerTest { void updatePerson_returns403_whenUserHasOnlyReadPermission() throws Exception { mockMvc.perform(put("/api/persons/{id}", UUID.randomUUID()) .contentType(MediaType.APPLICATION_JSON) - .content("{\"firstName\":\"Hans\",\"lastName\":\"Müller\"}")) + .content("{\"firstName\":\"Hans\",\"lastName\":\"Müller\",\"personType\":\"PERSON\"}")) .andExpect(status().isForbidden()); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/PersonServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/PersonServiceTest.java index 1095e93f..da2fdde4 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/PersonServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/PersonServiceTest.java @@ -114,6 +114,43 @@ class PersonServiceTest { assertThat(result.getAlias()).isEqualTo("Hans Müller"); } + // ─── personType + title in createPerson(PersonUpdateDTO) ───────────────── + + @Test + void createPerson_dto_persistsPersonType() { + when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + PersonUpdateDTO dto = new PersonUpdateDTO(); + dto.setFirstName("Walter"); dto.setLastName("de Gruyter"); dto.setPersonType(PersonType.INSTITUTION); + + Person result = personService.createPerson(dto); + + assertThat(result.getPersonType()).isEqualTo(PersonType.INSTITUTION); + } + + @Test + void createPerson_dto_throwsInvalidPersonType_whenSkip() { + PersonUpdateDTO dto = new PersonUpdateDTO(); + dto.setFirstName("Anna"); dto.setLastName("Test"); dto.setPersonType(PersonType.SKIP); + + assertThatThrownBy(() -> personService.createPerson(dto)) + .isInstanceOf(DomainException.class) + .extracting(e -> ((DomainException) e).getStatus().value()) + .isEqualTo(400); + } + + @Test + void createPerson_dto_persistsTitle() { + when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + PersonUpdateDTO dto = new PersonUpdateDTO(); + dto.setFirstName("Dr."); dto.setLastName("Müller"); dto.setTitle("Prof."); dto.setPersonType(PersonType.PERSON); + + Person result = personService.createPerson(dto); + + assertThat(result.getTitle()).isEqualTo("Prof."); + } + // ─── Phase 2.1: createPerson(PersonUpdateDTO) ───────────────────────────── @Test @@ -145,6 +182,36 @@ class PersonServiceTest { .isEqualTo(400); } + // ─── updatePerson (personType) ─────────────────────────────────────────── + + @Test + void updatePerson_throwsInvalidPersonType_whenSkip() { + UUID id = UUID.randomUUID(); + + PersonUpdateDTO dto = new PersonUpdateDTO(); + dto.setFirstName("Anna"); dto.setLastName("Alt"); dto.setPersonType(PersonType.SKIP); + + assertThatThrownBy(() -> personService.updatePerson(id, dto)) + .isInstanceOf(DomainException.class) + .extracting(e -> ((DomainException) e).getStatus().value()) + .isEqualTo(400); + } + + @Test + void updatePerson_persistsPersonType() { + UUID id = UUID.randomUUID(); + Person person = Person.builder().id(id).firstName("Anna").lastName("Alt").personType(PersonType.PERSON).build(); + when(personRepository.findById(id)).thenReturn(Optional.of(person)); + when(personRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + PersonUpdateDTO dto = new PersonUpdateDTO(); + dto.setFirstName("Anna"); dto.setLastName("Alt"); dto.setPersonType(PersonType.INSTITUTION); + + Person result = personService.updatePerson(id, dto); + + assertThat(result.getPersonType()).isEqualTo(PersonType.INSTITUTION); + } + // ─── updatePerson (alias) ───────────────────────────────────────────────── @Test diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 0384e776..8522580c 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -33,6 +33,8 @@ "btn_back_to_overview": "Zurück zur Übersicht", "btn_back": "Zurück", "btn_back_to_document": "Zurück zum Dokument", + "form_label_person_type": "Typ", + "form_label_name": "Name", "form_label_first_name": "Vorname", "form_label_last_name": "Nachname", "form_label_alias": "Rufname / Alias", @@ -527,6 +529,7 @@ "person_type_INSTITUTION": "Institution", "person_type_GROUP": "Gruppe", "person_type_UNKNOWN": "Unbekannt", + "a11y_type_changed": "Typ geändert zu {type}", "person_alias_add_heading": "Name hinzufuegen", "person_alias_label_type": "Art", "person_alias_label_last_name": "Nachname", @@ -536,6 +539,9 @@ "person_alias_delete_body": "Dieser Name wird aus der Suche entfernt.", "person_alias_btn_delete": "Entfernen", "error_alias_not_found": "Der Namensalias wurde nicht gefunden.", + "error_invalid_person_type": "Der angegebene Personentyp ist ungültig.", + "validation_last_name_required": "Nachname ist Pflichtfeld.", + "validation_first_name_required": "Vorname ist Pflichtfeld.", "error_ocr_service_unavailable": "Der OCR-Dienst ist nicht verfügbar.", "error_ocr_job_not_found": "Der OCR-Auftrag wurde nicht gefunden.", "error_ocr_document_not_uploaded": "Das Dokument hat keine Datei — OCR ist nicht möglich.", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 5e75ef71..cddafac1 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -33,6 +33,8 @@ "btn_back_to_overview": "Back to overview", "btn_back": "Back", "btn_back_to_document": "Back to document", + "form_label_person_type": "Type", + "form_label_name": "Name", "form_label_first_name": "First name", "form_label_last_name": "Last name", "form_label_alias": "Nickname / Alias", @@ -527,6 +529,7 @@ "person_type_INSTITUTION": "Institution", "person_type_GROUP": "Group", "person_type_UNKNOWN": "Unknown", + "a11y_type_changed": "Type changed to {type}", "person_alias_add_heading": "Add name", "person_alias_label_type": "Type", "person_alias_label_last_name": "Last name", @@ -536,6 +539,9 @@ "person_alias_delete_body": "This name will be removed from search results.", "person_alias_btn_delete": "Remove", "error_alias_not_found": "The name alias was not found.", + "error_invalid_person_type": "The specified person type is not valid.", + "validation_last_name_required": "Last name is required.", + "validation_first_name_required": "First name is required.", "error_ocr_service_unavailable": "The OCR service is not available.", "error_ocr_job_not_found": "The OCR job was not found.", "error_ocr_document_not_uploaded": "The document has no file — OCR is not possible.", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index 12c2c5c9..f5125884 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -33,6 +33,8 @@ "btn_back_to_overview": "Volver al resumen", "btn_back": "Volver", "btn_back_to_document": "Volver al documento", + "form_label_person_type": "Tipo", + "form_label_name": "Nombre", "form_label_first_name": "Nombre", "form_label_last_name": "Apellido", "form_label_alias": "Apodo / Alias", @@ -527,6 +529,7 @@ "person_type_INSTITUTION": "Institución", "person_type_GROUP": "Grupo", "person_type_UNKNOWN": "Desconocido", + "a11y_type_changed": "Tipo cambiado a {type}", "person_alias_add_heading": "Agregar nombre", "person_alias_label_type": "Tipo", "person_alias_label_last_name": "Apellido", @@ -536,6 +539,9 @@ "person_alias_delete_body": "Este nombre se eliminara de los resultados de busqueda.", "person_alias_btn_delete": "Eliminar", "error_alias_not_found": "No se encontro el alias de nombre.", + "error_invalid_person_type": "El tipo de persona especificado no es válido.", + "validation_last_name_required": "El apellido es obligatorio.", + "validation_first_name_required": "El nombre es obligatorio.", "error_ocr_service_unavailable": "El servicio OCR no está disponible.", "error_ocr_job_not_found": "No se encontró el trabajo OCR.", "error_ocr_document_not_uploaded": "El documento no tiene archivo — OCR no es posible.", diff --git a/frontend/src/lib/actions/radioGroupNav.svelte.spec.ts b/frontend/src/lib/actions/radioGroupNav.svelte.spec.ts new file mode 100644 index 00000000..f624b38f --- /dev/null +++ b/frontend/src/lib/actions/radioGroupNav.svelte.spec.ts @@ -0,0 +1,87 @@ +import { describe, it, expect, afterEach } from 'vitest'; + +const { radioGroupNav } = await import('./radioGroupNav'); + +describe('radioGroupNav action', () => { + const nodes: HTMLElement[] = []; + + function makeGroup(count: number): { container: HTMLElement; buttons: HTMLElement[] } { + const container = document.createElement('div'); + container.setAttribute('role', 'radiogroup'); + const buttons: HTMLElement[] = []; + for (let i = 0; i < count; i++) { + const btn = document.createElement('button'); + btn.setAttribute('role', 'radio'); + btn.setAttribute('aria-checked', i === 0 ? 'true' : 'false'); + btn.setAttribute('tabindex', i === 0 ? '0' : '-1'); + container.appendChild(btn); + buttons.push(btn); + } + document.body.appendChild(container); + nodes.push(container); + return { container, buttons }; + } + + afterEach(() => { + nodes.forEach((n) => n.remove()); + nodes.length = 0; + }); + + it('ArrowRight moves focus to next button', () => { + const { container, buttons } = makeGroup(4); + radioGroupNav(container); + buttons[0].focus(); + buttons[0].dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true })); + expect(document.activeElement).toBe(buttons[1]); + }); + + it('ArrowRight wraps from last to first', () => { + const { container, buttons } = makeGroup(4); + radioGroupNav(container); + buttons[3].focus(); + buttons[3].dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true })); + expect(document.activeElement).toBe(buttons[0]); + }); + + it('ArrowLeft moves focus to previous button', () => { + const { container, buttons } = makeGroup(4); + radioGroupNav(container); + buttons[2].focus(); + buttons[2].dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowLeft', bubbles: true })); + expect(document.activeElement).toBe(buttons[1]); + }); + + it('ArrowLeft wraps from first to last', () => { + const { container, buttons } = makeGroup(4); + radioGroupNav(container); + buttons[0].focus(); + buttons[0].dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowLeft', bubbles: true })); + expect(document.activeElement).toBe(buttons[3]); + }); + + it('ArrowRight updates aria-checked on new button and removes it from old', () => { + const { container, buttons } = makeGroup(4); + radioGroupNav(container); + buttons[0].focus(); + buttons[0].dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true })); + expect(buttons[1].getAttribute('aria-checked')).toBe('true'); + expect(buttons[0].getAttribute('aria-checked')).toBe('false'); + }); + + it('destroy removes keydown listener', () => { + const { container, buttons } = makeGroup(4); + const { destroy } = radioGroupNav(container); + destroy(); + buttons[0].focus(); + buttons[0].dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true })); + expect(document.activeElement).toBe(buttons[0]); + }); + + it('ignores non-arrow keys', () => { + const { container, buttons } = makeGroup(4); + radioGroupNav(container); + buttons[0].focus(); + buttons[0].dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + expect(document.activeElement).toBe(buttons[0]); + }); +}); diff --git a/frontend/src/lib/actions/radioGroupNav.ts b/frontend/src/lib/actions/radioGroupNav.ts new file mode 100644 index 00000000..65721e3a --- /dev/null +++ b/frontend/src/lib/actions/radioGroupNav.ts @@ -0,0 +1,37 @@ +export function radioGroupNav( + node: HTMLElement, + onChange?: (value: string) => void +): { destroy: () => void; update: (onChange?: (value: string) => void) => void } { + let onChangeFn = onChange; + + function getRadios(): HTMLElement[] { + return Array.from(node.querySelectorAll('[role="radio"]')); + } + + function handleKeydown(event: KeyboardEvent) { + if (event.key !== 'ArrowRight' && event.key !== 'ArrowLeft') return; + + const radios = getRadios(); + const current = radios.indexOf(document.activeElement as HTMLElement); + if (current === -1) return; + + const delta = event.key === 'ArrowRight' ? 1 : -1; + const next = (current + delta + radios.length) % radios.length; + + radios[current].setAttribute('aria-checked', 'false'); + radios[next].setAttribute('aria-checked', 'true'); + radios[next].focus(); + onChangeFn?.(radios[next].getAttribute('value') ?? ''); + } + + node.addEventListener('keydown', handleKeydown); + + return { + update(newOnChange) { + onChangeFn = newOnChange; + }, + destroy() { + node.removeEventListener('keydown', handleKeydown); + } + }; +} diff --git a/frontend/src/lib/components/PersonTypeSelector.svelte b/frontend/src/lib/components/PersonTypeSelector.svelte new file mode 100644 index 00000000..1d4de750 --- /dev/null +++ b/frontend/src/lib/components/PersonTypeSelector.svelte @@ -0,0 +1,58 @@ + + +
{ if (TYPES.includes(v as PersonType)) select(v as PersonType); }} +> + {#each TYPES as type (type)} + + {/each} +
+ + + +
{announcement}
diff --git a/frontend/src/lib/components/PersonTypeSelector.svelte.spec.ts b/frontend/src/lib/components/PersonTypeSelector.svelte.spec.ts new file mode 100644 index 00000000..29eb21d0 --- /dev/null +++ b/frontend/src/lib/components/PersonTypeSelector.svelte.spec.ts @@ -0,0 +1,71 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import { cleanup, render } from 'vitest-browser-svelte'; +import { userEvent } from 'vitest/browser'; + +import PersonTypeSelector from './PersonTypeSelector.svelte'; + +afterEach(() => cleanup()); + +describe('PersonTypeSelector', () => { + it('radiogroup has an accessible name via aria-label', () => { + const { container } = render(PersonTypeSelector, { value: 'PERSON' }); + const radiogroup = container.querySelector('[role="radiogroup"]'); + expect(radiogroup).not.toBeNull(); + expect(radiogroup!.getAttribute('aria-label')).toBeTruthy(); + }); + + it('hidden input value updates when user navigates with ArrowRight', async () => { + const { container } = render(PersonTypeSelector, { value: 'PERSON' }); + const hiddenInput = container.querySelector('input[type="hidden"]') as HTMLInputElement; + expect(hiddenInput.value).toBe('PERSON'); + + const personButton = container.querySelector('[aria-checked="true"]') as HTMLElement; + personButton.focus(); + await userEvent.keyboard('{ArrowRight}'); + + expect(hiddenInput.value).toBe('INSTITUTION'); + }); + + it('hidden input value updates when user navigates with ArrowLeft (wraps around)', async () => { + const { container } = render(PersonTypeSelector, { value: 'PERSON' }); + const hiddenInput = container.querySelector('input[type="hidden"]') as HTMLInputElement; + expect(hiddenInput.value).toBe('PERSON'); + + const personButton = container.querySelector('[aria-checked="true"]') as HTMLElement; + personButton.focus(); + await userEvent.keyboard('{ArrowLeft}'); + + expect(hiddenInput.value).toBe('UNKNOWN'); + }); + it('exactly one button is aria-checked=true for the initial value', () => { + const { container } = render(PersonTypeSelector, { value: 'INSTITUTION' }); + const buttons = Array.from(container.querySelectorAll('[role="radio"]')); + const checked = buttons.filter((b) => b.getAttribute('aria-checked') === 'true'); + const unchecked = buttons.filter((b) => b.getAttribute('aria-checked') === 'false'); + expect(checked).toHaveLength(1); + expect(unchecked).toHaveLength(3); + }); + + it('aria-checked=true moves to clicked button on click', async () => { + const { container } = render(PersonTypeSelector, { value: 'PERSON' }); + const buttons = Array.from(container.querySelectorAll('[role="radio"]')); + const groupButton = buttons.find((b) => b.getAttribute('value') === 'GROUP') as HTMLElement; + await userEvent.click(groupButton); + expect(groupButton.getAttribute('aria-checked')).toBe('true'); + const others = buttons.filter((b) => b !== groupButton); + for (const btn of others) { + expect(btn.getAttribute('aria-checked')).toBe('false'); + } + }); + + it('selected button has tabindex=0, unselected buttons have tabindex=-1', () => { + const { container } = render(PersonTypeSelector, { value: 'PERSON' }); + const buttons = Array.from(container.querySelectorAll('[role="radio"]')); + const selected = buttons.find((b) => b.getAttribute('aria-checked') === 'true'); + const unselected = buttons.filter((b) => b.getAttribute('aria-checked') !== 'true'); + expect(selected!.getAttribute('tabindex')).toBe('0'); + for (const btn of unselected) { + expect(btn.getAttribute('tabindex')).toBe('-1'); + } + }); +}); diff --git a/frontend/src/lib/errors.ts b/frontend/src/lib/errors.ts index eec7a3e1..6acdefbe 100644 --- a/frontend/src/lib/errors.ts +++ b/frontend/src/lib/errors.ts @@ -7,6 +7,7 @@ import * as m from '$lib/paraglide/messages.js'; export type ErrorCode = | 'PERSON_NOT_FOUND' | 'ALIAS_NOT_FOUND' + | 'INVALID_PERSON_TYPE' | 'DOCUMENT_NOT_FOUND' | 'DOCUMENT_NO_FILE' | 'FILE_NOT_FOUND' @@ -73,6 +74,8 @@ export function getErrorMessage(code: ErrorCode | string | undefined): string { return m.error_person_not_found(); case 'ALIAS_NOT_FOUND': return m.error_alias_not_found(); + case 'INVALID_PERSON_TYPE': + return m.error_invalid_person_type(); case 'DOCUMENT_NOT_FOUND': return m.error_document_not_found(); case 'DOCUMENT_NO_FILE': diff --git a/frontend/src/lib/person-validation.test.ts b/frontend/src/lib/person-validation.test.ts new file mode 100644 index 00000000..d9f6ed58 --- /dev/null +++ b/frontend/src/lib/person-validation.test.ts @@ -0,0 +1,40 @@ +import { describe, it, expect } from 'vitest'; +import { validatePersonFields } from './person-validation'; + +describe('validatePersonFields', () => { + it('returns null when all required fields are present for PERSON', () => { + expect(validatePersonFields('PERSON', 'Hans', 'Müller')).toBeNull(); + }); + + it('returns lastName error key when lastName is missing', () => { + expect(validatePersonFields('PERSON', 'Hans', '')).toBe('validation_last_name_required'); + }); + + it('returns lastName error key when lastName is undefined', () => { + expect(validatePersonFields('INSTITUTION', undefined, undefined)).toBe( + 'validation_last_name_required' + ); + }); + + it('returns firstName error key when type is PERSON and firstName is missing', () => { + expect(validatePersonFields('PERSON', '', 'Müller')).toBe('validation_first_name_required'); + }); + + it('returns firstName error key when type is PERSON and firstName is undefined', () => { + expect(validatePersonFields('PERSON', undefined, 'Müller')).toBe( + 'validation_first_name_required' + ); + }); + + it('returns null for INSTITUTION without firstName', () => { + expect(validatePersonFields('INSTITUTION', undefined, 'Verlag GmbH')).toBeNull(); + }); + + it('returns null for GROUP without firstName', () => { + expect(validatePersonFields('GROUP', undefined, 'Familie Raddatz')).toBeNull(); + }); + + it('returns null for UNKNOWN without firstName', () => { + expect(validatePersonFields('UNKNOWN', undefined, 'Unbekannt')).toBeNull(); + }); +}); diff --git a/frontend/src/lib/person-validation.ts b/frontend/src/lib/person-validation.ts new file mode 100644 index 00000000..80f7e2c6 --- /dev/null +++ b/frontend/src/lib/person-validation.ts @@ -0,0 +1,39 @@ +import { m } from '$lib/paraglide/messages.js'; + +export const PERSON_TYPES = ['PERSON', 'INSTITUTION', 'GROUP', 'UNKNOWN'] as const; +export type PersonType = (typeof PERSON_TYPES)[number]; + +export type PersonFormData = { + personType?: string | null; + title?: string | null; + firstName?: string | null; + lastName: string; + alias?: string | null; + birthYear?: number | null; + deathYear?: number | null; + notes?: string | null; +}; + +export function normalizePersonType(raw: string | undefined | null): PersonType { + return raw === 'SKIP' ? 'UNKNOWN' : ((raw ?? 'PERSON') as PersonType); +} + +export type PersonValidationKey = + | 'validation_last_name_required' + | 'validation_first_name_required'; + +export function resolveValidationMessage(key: PersonValidationKey): string { + return key === 'validation_last_name_required' + ? m.validation_last_name_required() + : m.validation_first_name_required(); +} + +export function validatePersonFields( + personType: string, + firstName: string | undefined | null, + lastName: string | undefined | null +): PersonValidationKey | null { + if (!lastName) return 'validation_last_name_required'; + if (personType === 'PERSON' && !firstName) return 'validation_first_name_required'; + return null; +} diff --git a/frontend/src/routes/persons/[id]/PersonCard.svelte b/frontend/src/routes/persons/[id]/PersonCard.svelte index 81665515..7d30ebb4 100644 --- a/frontend/src/routes/persons/[id]/PersonCard.svelte +++ b/frontend/src/routes/persons/[id]/PersonCard.svelte @@ -13,6 +13,7 @@ let { lastName: string; displayName: string; personType?: string | null; + title?: string | null; alias?: string | null; birthYear?: number | null; deathYear?: number | null; @@ -66,6 +67,14 @@ let { + {#if person.personType === 'PERSON' && person.title} +

+ {person.title} +

+ {/if} +

{person.displayName} diff --git a/frontend/src/routes/persons/[id]/edit/+page.server.ts b/frontend/src/routes/persons/[id]/edit/+page.server.ts index e814737a..c3901f8f 100644 --- a/frontend/src/routes/persons/[id]/edit/+page.server.ts +++ b/frontend/src/routes/persons/[id]/edit/+page.server.ts @@ -1,6 +1,11 @@ import { error, fail, redirect } from '@sveltejs/kit'; import { createApiClient } from '$lib/api.server'; import { getErrorMessage } from '$lib/errors'; +import { + normalizePersonType, + validatePersonFields, + resolveValidationMessage +} from '$lib/person-validation'; export async function load({ params, fetch, locals }) { const canWrite = @@ -22,12 +27,16 @@ export async function load({ params, fetch, locals }) { throw error(result.response.status, getErrorMessage(code)); } - return { person: result.data!, aliases: aliasesResult.data ?? [] }; + const person = result.data!; + const personType = normalizePersonType(person.personType); + return { person: { ...person, personType }, aliases: aliasesResult.data ?? [] }; } export const actions = { update: async ({ request, params, fetch }) => { const formData = await request.formData(); + const personType = normalizePersonType(formData.get('personType')?.toString()); + const title = formData.get('title')?.toString().trim() || undefined; const firstName = formData.get('firstName')?.toString().trim(); const lastName = formData.get('lastName')?.toString().trim(); const alias = formData.get('alias')?.toString().trim() || undefined; @@ -37,15 +46,18 @@ export const actions = { const birthYear = birthYearStr ? parseInt(birthYearStr, 10) : undefined; const deathYear = deathYearStr ? parseInt(deathYearStr, 10) : undefined; - if (!firstName || !lastName) { - return fail(400, { updateError: 'Vor- und Nachname sind Pflichtfelder.' }); + const validationKey = validatePersonFields(personType, firstName, lastName); + if (validationKey) { + return fail(400, { updateError: resolveValidationMessage(validationKey) }); } const api = createApiClient(fetch); const result = await api.PUT('/api/persons/{id}', { params: { path: { id: params.id } }, body: { - firstName, + personType, + ...(title ? { title } : {}), + ...(firstName ? { firstName } : {}), lastName, ...(alias ? { alias } : {}), ...(notes ? { notes } : {}), diff --git a/frontend/src/routes/persons/[id]/edit/PersonEditForm.svelte b/frontend/src/routes/persons/[id]/edit/PersonEditForm.svelte index 9d88464d..b53599ea 100644 --- a/frontend/src/routes/persons/[id]/edit/PersonEditForm.svelte +++ b/frontend/src/routes/persons/[id]/edit/PersonEditForm.svelte @@ -1,93 +1,117 @@
-
- - +

+ {m.form_label_person_type()} +

+ (selectedType = type)} />
-
- + + {#if isPerson} +
+ + +
+
+ + +
+ {/if} + +
+
+ + {#if isPerson} +
+ + +
+
+ + +
+
+ + +
+ {/if} +
- - -
-
- - -
-
- - -
-
- +