feat(ui): surface title & personType fields in person forms and detail card #218
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
The
titleandpersonTypefields exist in the backend model but are not yet editable or fully displayed in the UI. This issue adds:Design Spec
docs/specs/person-title-type-fields-spec.html(commit7a6b3d6)Tasks
Backend
personTypefield toPersonUpdateDTOwith validation (excludeSKIPfrom allowed values)PersonService.update()andcreate()to acceptpersonTypefrom DTOFrontend — Forms
PersonEditForm.svelte(role="radiogroup", arrow-key nav, hidden input)titleinput (narrow column) before firstName+page.server.ts(edit + new) to read/submittitle+personTypeFrontend — Detail Card
titledisplay toPersonCard.svelte(small-caps, text-ink-3, above displayName, only for PERSON type)Accessibility
role="radiogroup"/role="radio"/aria-checked/ arrow keysaria-live="polite"region announcing field visibility changesi18n
form_label_title,form_label_name,form_label_group_name,form_label_person_type,person_type_PERSON,a11y_type_fields_visible(de/en/es)🎨 Leonie Voss — UI/UX Design Lead & Accessibility Strategist
Design review of open items before implementation. Six items worked through, all resolved.
1. Title field width (120px) ✅ Resolved
The CSS grid approach in the spec is correct:
grid-cols-[120px_1fr_1fr]on desktop,grid-cols-[80px_1fr]on mobile (lastName drops to full-width below). Themax-w-[120px]on the input itself is slightly redundant given the grid column constraint, but harmless as a safety cap. No change needed.2.
text-ink-3token ✅ ResolvedFully defined in
layout.css—#6b7280(gray-500, 4.8:1 on white) in light mode,#8b97a5in dark mode. Both pass WCAG AA. Widely used across the codebase already. No new token needed.3. Focus management on type switch ✅ Resolved
When a type switch hides the currently focused field (title, firstName, birthYear, deathYear), focus must be moved explicitly to the
lastNameinput. Implementation note: checkdocument.activeElementbefore applying the{#if}toggle; if it's inside a field about to be removed, call.focus()on thelastNameinput after the state update. Thearia-live="polite"region handles the announcement independently.4. Value preservation on type switch ✅ Resolved
Field values are preserved in
$stateacross type switches. Since{#if}removes inputs from the DOM entirely, hidden values cannot be accidentally submitted. No clearing logic needed. Preserving values prevents data loss on accidental switches.5. SKIP type fallback ✅ Resolved
personType = SKIPmaps silently to UNKNOWN in the form on load. SKIP is import-only and has no user-facing meaning; mapping to UNKNOWN aligns with the spec's "hidden from UI" decision. On first save the backend will persist UNKNOWN.6. Touch targets on mobile 2×2 grid ✅ Resolved
The spec specifies
min-h-[44px]per segment button — the WCAG 2.2 minimum. Given the 60+ audience, bump this tomin-h-[48px]. Verified against the mobile layout: on a 320px viewport the 2×2 grid totals ~98px (2 × 48px + 1px border), each button ~156px wide × 48px tall. Proportionate, no overflow, form is scrollable so total height is unconstrained. The spec impl-ref table (line 670) should reflectmin-h-[48px]in the implementation.Overall: the spec is well-prepared and implementation-ready. The six items above are clarifications and one small enhancement (touch targets) — no design rework required.
👨💻 Felix Brandt — Senior Fullstack Developer
Implementation review — six decisions nailed down before coding starts.
1. Segmented control — extract
PersonTypeSelector.svelte✅ ResolvedToo much responsibility to keep inline: keyboard nav, aria attributes, active state, hidden input, responsive layout. Extracted to
PersonTypeSelector.svelte. Props:value: PersonType,onchangecallback. Hidden<input type="hidden" name="personType">lives inside it — form gets the value on submit without extra wiring in the parent.2.
PersonUpdateDTOvalidation for SKIP ✅ ResolvedBoth layers:
@Schema(allowableValues = {"PERSON", "INSTITUTION", "GROUP", "UNKNOWN"})on the DTO field for OpenAPI spec documentation, plus a service-level guard clause throwingDomainException.badRequestfor runtime enforcement. NewINVALID_PERSON_TYPEerror code needed inErrorCode.java, mirrored inerrors.ts.3. Reactive i18n label for
lastName✅ Resolved$derivedwith an intent-revealing name in the script — logic out of the template:Template reads
{lastNameLabel}. Same pattern for placeholder if spec requires it.4. Arrow-key navigation ✅ Resolved
Extract
use:radioGroupNavaction tosrc/lib/actions/radioGroupNav.ts, following theuse:clickOutsidepattern. Handles arrow left/right, wrap-around, focus, andaria-checkedupdates.PersonTypeSelector.svelteapplies it to the container. Needs its own test file alongsideclickOutside.test.ts.5. SKIP → UNKNOWN mapping location ✅ Resolved
Normalized in
+page.server.tsload function — component never seesSKIP. Consistent with the spec's "hidden from UI" decision and keeps the component ignorant of import-only backend values.6. Test strategy ✅ Resolved
Three layers covered now:
SKIP(unit test)+page.server.tsload maps SKIP → UNKNOWNradioGroupNavarrow keys and wrap-around (mirrorsclickOutsidetest coverage)Component keyboard nav (aria-checked state, focus within
PersonTypeSelector) deferred to E2E.Spec is implementation-ready. The two things that need creating before any component work:
INVALID_PERSON_TYPEerror code and theradioGroupNavaction with its test.📋 Elicit — Requirements Review
Requirements analysis of this issue against spec §4, V22 migration, and
PersonService.java. Seven open questions resolved; findings below.Corrected Conditional Field Matrix
The issue body omits the UNKNOWN column and the alias row. Full matrix from spec §4:
Additions to Backend Task List
Two items missing from the current task list:
@Size(max = 50)onPersonUpdateDTO.title— DB column isVARCHAR(50)(V22 migration line 2). The frontend input must also havemaxlength="50". Neither is currently listed.person.setPersonType(dto.getPersonType())is absent fromupdatePerson()— the method currently accepts aPersonUpdateDTObut never calls the setter. The type change is silently dropped on every save. The issue's backend task already names this fix; this confirms it is truly missing, not just undocumented.Design decision (explicit):
firstName,title,birthYear,deathYearare not cleared server-side when type changes. Values are preserved in the DB for round-trip safety — if the user switches back to PERSON, their data returns. This aligns with Felix's comment on value preservation and must be stated as deliberate, not an oversight.User Stories
US-PERSON-001
As a transcriber, I want to classify each person as Person / Institution / Group / Unknown, so that entity types are distinguishable in the archive and display names render correctly.
US-PERSON-002
As a transcriber, I want to enter a title (e.g. "Dr.", "Gräfin") separately from the first name, so that historical forms of address are preserved and searchable.
US-PERSON-003
As a reader, I want to see a person's title displayed on their detail card, so that the historical form of address is immediately visible.
Acceptance Criteria
US-PERSON-001 — Segmented Type Control
US-PERSON-002 — Title Field
US-PERSON-003 — PersonCard Title Display
EARS System Rules
Non-Goal (explicit)
Auto-detection of
personTypebased on form input is out of scope. Classification viaPersonTypeClassifierexists for mass import only. The UI is always manual.👨💻 Felix Brandt — Senior Fullstack Developer
Six decisions from the 2026-04-15 pre-review stand. This pass focuses on what codebase research turned up.
Observations
Four concrete implementation gaps confirmed in the code:
PersonUpdateDTOhas nopersonTypefield —titleis present,personTypeis not. The backend task is correct. Recommendation: declare it asPersonTypeenum (notString) so Jackson rejects invalid values at deserialization — no extra validation layer needed for garbage input.PersonService.updatePerson()andcreatePerson(PersonUpdateDTO)never callperson.setPersonType()— confirmed at lines 111–123 and 138–150. Both methods handletitlebut silently ignorepersonType.+page.server.ts(edit route) doesn't extracttitleorpersonType— confirmed at lines 29–63. TwoformData.get()calls are missing. This means both fields are lost on every save today.radioGroupNav.tsdoes not exist —clickOutside.tsis available as the pattern model. The action needs: arrow left/right, wrap-around, focus update,aria-checkedupdate. Create it with a.test.tsbeforePersonTypeSelector.svelteis written (test first).Generated type inconsistency:
api.tscurrently showspersonType?: string— a plain string. After addingpersonType: PersonTypeto the Java DTO with@Schema, regenerate types and verify the result becomes"PERSON" | "INSTITUTION" | "GROUP" | "UNKNOWN", notstring.Test coverage gap:
PersonServiceTesthas zero tests fortitlepersistence and zero forpersonTypein create/update methods.Recommendations
private PersonType personType;inPersonUpdateDTO(the enum type, not String).should_persist_personType_when_createPerson_is_called()should_persist_personType_when_updatePerson_is_called()should_throw_INVALID_PERSON_TYPE_when_SKIP_is_submitted()radioGroupNav.test.ts— arrow cycle, wrap-around, aria-checked stateshould_map_SKIP_to_UNKNOWN_in_page_server_load()— import the load function directly, mock fetch--spring.profiles.active=dev→ runnpm run generate:apibefore touching any frontend component.PersonTypeSelector.svelteprops interface:value: PersonType,onchange: (type: PersonType) => void. Hidden input lives inside the component as already decided.🏛️ Markus Keller — Application Architect
Observations
DB layer is already sound: V22 migration has
CHECK (person_type IN ('PERSON', 'INSTITUTION', 'GROUP', 'UNKNOWN'))— SKIP cannot be persisted regardless of application behavior. This is the correct design: the database enforces the invariant atomically. The service-level SKIP guard is defense-in-depth on top, not the primary protection.No module boundary violations: Pure Person-domain change. Controller → Service → Repository path unchanged. No cross-domain dependency introduced.
Import classifier vs. user override — no conflict:
findOrCreateByAlias()is find-or-create. If the person already exists it returns it without modification. A user who manually setspersonType = INSTITUTIONwill not have it overwritten by a subsequent import of the same alias. The classifier only runs during initial creation.DTO field type matters architecturally: If
personTypeis declared asString, Jackson binds any string that arrives in the request body. Service validation catches SKIP but not"","null", or"SUPERUSER". Declaring it as thePersonTypeenum means Jackson rejects non-enum values with a 400 at deserialization — no service code required for that class of invalid input.Partial update semantics:
personTypeis NOT NULL in the DB (default'PERSON'). The DTO field should be nullable so that API clients sending a partial update (e.g. only changingnotes) don't need to resendpersonType. The service should applysetPersonType()only whendto.getPersonType() != null.Recommendations
private PersonType personType;(nullable) inPersonUpdateDTO. InupdatePerson():if (dto.getPersonType() != null) person.setPersonType(dto.getPersonType());Open Decisions
personTypein DTO: preserve existing vs. require explicit value — If nullable, a partial update that omitspersonTypesilently preserves the existing type. If required (non-null), the form always sends a value and the API contract is cleaner. Given the form always sends a type (hidden input with default), either works for the UI. The nullable approach is safer for any future API client. (Raised by: Markus)🔒 Nora "NullX" Steiner — Application Security Engineer
Observations
Permissions are correct: Both
POST /api/personsandPUT /api/persons/{id}require@RequirePermission(Permission.WRITE_ALL). AddingpersonTypeas a writeable field adds no new permission surface — it's behind the same gate that already protects firstName, lastName, etc.Mass assignment risk —
personTypeas String: The prior review (Felix's comment) described adding@Schema(allowableValues = {"PERSON", "INSTITUTION", "GROUP", "UNKNOWN"})on a String field.@Schemais OpenAPI documentation only — it does not reject invalid strings at runtime. Jackson will happily bind"SKIP","ADMIN", or"'; DROP TABLE persons;--"to a String field and pass it to the service. The service guard only catches SKIP; everything else passes unchecked intoperson.setPersonType()(once that setter is added).The correct fix: declare
personTypeas thePersonTypeenum type in the DTO. Jackson's enum deserialization rejects any value not in the enum with a 400 automatically. SKIP is a valid enum value (it exists in Java), so the service guard for SKIP is still required. Two independent layers: Jackson blocks non-enum garbage; the service guard blocks the SKIP enum value.@Size(max = 50)onPersonUpdateDTO.title: The DTO currently hastitlelisted but the@Sizeannotation presence is unconfirmed. Without it, a 51-character title passes backend validation, reaches PostgreSQL, and throws a constraint violation — which the global error handler may or may not surface cleanly. Add@Size(max = 50)explicitly rather than relying on the DB constraint to be the first line.INVALID_PERSON_TYPEerror code: This is a service-layer domain error. It must be added toErrorCode.java, mirrored inerrors.ts, and translated in all three i18n files. Without this, the service throws aDomainExceptionwith an unmapped code and the frontend displays a generic error message.No new injection surfaces:
personTypeandtitleflow through parameterized JPA. No SQL injection risk introduced.Recommendations
private PersonType personType;(the Java enum) inPersonUpdateDTO. Do not useString.@Size(max = 50)is onPersonUpdateDTO.title. If missing, add it now — don't rely on the DB constraint as the first line of defense.INVALID_PERSON_TYPEtoErrorCode.java+errors.ts+{de,en,es}.jsonas part of this issue.🧪 Sara Holt — QA Engineer
Observations
Confirmed coverage gaps (codebase-verified):
PersonServiceTest: zero tests fortitlepersistence in create/update; zero forpersonTypepersistence; zero for SKIP rejectionradioGroupNav.ts: doesn't exist, so no tests+page.server.tsload function: SKIP → UNKNOWN mapping is completely untestedFull test plan for this feature:
should_persist_personType_when_createPerson_is_calledPersonServiceTestshould_persist_personType_when_updatePerson_is_calledPersonServiceTestshould_throw_INVALID_PERSON_TYPE_when_SKIP_is_submittedPersonServiceTestshould_preserve_personType_when_dto_personType_is_nullPersonServiceTestshould_persist_title_when_createPerson_is_calledPersonServiceTestaria-checkedupdateradioGroupNav.test.ts+page.server.tsunit testPersonTypeSelector.spec.tsPersonEditForm.spec.tsAxeBuilderon edit form with PERSON type activeAxeBuilderon edit form with INSTITUTION type activeTwo axe runs needed: PERSON and INSTITUTION modes have different DOM structures (different fields present/absent, different aria tree). Running axe only on PERSON state misses the INSTITUTION layout.
AC-005 (value preservation) and AC-007 (focus management) are Playwright-only — no unit test equivalent. Both are testable with
page.keyboard.press('Tab')and field value assertions after type switch.AC-013 (title maxlength): test at both layers —
@Sizeviolation test inPersonServiceTestwith a 51-character title, and a frontend attribute test assertingmaxlength="50"on the input element.Recommendations
radioGroupNav.test.tsshould mirrorclickOutside.test.tsin structure (same folder, same test runner setup).+page.server.tsload function test — it's a plain TypeScript import with a mockedfetch, takes 5 minutes to write, and is the only coverage for the SKIP → UNKNOWN mapping.🎨 Leonie Voss — UX Design Lead & Accessibility Strategist
Pre-implementation design decisions are resolved (see 2026-04-15 comment). Three items for this implementation pass.
Observations
1. Spec impl-ref table still says
min-h-[44px]— must bemin-h-[48px]My prior comment upgraded the mobile touch target to 48px. The spec implementation reference table (§2, type selector mobile row) still reads
min-h-[44px]. The developer implements from the table, not from the comment thread. If this isn't corrected in the spec, the 48px requirement will be missed. This is Critical — the 60+ audience is the primary transcriber demographic.2.
aria-livecopy needs to cover both directionsThe issue lists i18n key
a11y_type_fields_visible. This name implies fields appearing, but the live region fires on both hide and show. A key named "visible" makes no sense when fields disappear. Recommendation: replace with a single type-name template key —a11y_type_changed— used as:a11y_type_changed({ type: m[personType]() }), resolving to e.g. "Ansicht: Person" or "Ansicht: Institution". One key, always describes the new active state, works for both directions.3. Title input label association — use
<label>notaria-labelThe spec impl-ref specifies
aria-label= i18n key for "Akademischer Titel". If the input already has a visible<label for="title-input">element (which it should per the form pattern),aria-labelis redundant and conflicts — the visible label and the programmatic label would be different text. Use<label for="title-input">+<input id="title-input">pairing. This is simpler, uses one i18n key (form_label_titlewhich is already listed), and is the correct semantic pattern. No extra i18n key needed.Recommendations
min-h-[48px]before implementation starts.a11y_type_fields_visiblei18n key witha11y_type_changedaccepting atypeparameter. Add translations for all three locales.<label for>/idpairing for the title input instead ofaria-label. Remove thearia-labelreference from the impl-ref table.⚙️ Tobias Wendt — DevOps & Platform Engineer
Observations
No infrastructure changes needed: V22 migration already adds
titleandperson_typecolumns. No new Docker services, environment variables, MinIO buckets, or CI pipeline jobs are required for this feature.Type regeneration is a manual step with a silent failure mode: After adding
personTypetoPersonUpdateDTOand rebuilding the backend,npm run generate:apimust be run manually before frontend work. The currentapi.tsalready haspersonType?: stringin the DTO schema — this means the TypeScript compiler won't catch the pre/post-change difference, and a developer who forgets to regenerate will not see a build error. The drift will be invisible until runtime.CI does not run type generation automatically: There is no step in the CI workflow that regenerates the OpenAPI types on backend changes and fails if the generated file has uncommitted diffs. This is an accepted limitation for a solo project.
Recommendations
[ ] Rebuild JAR → start with --spring.profiles.active=dev → run npm run generate:api before starting frontend tasks.It's the current convention; making it explicit in the checklist prevents a silent type mismatch.npm run generate:apiand fails ifapi.tshas a diff. Cost: ~30 seconds of CI time. Benefit: catches every future DTO change that wasn't regenerated. Worth opening a separate issue for.🗳️ Decision Queue — Action Required
1 decision needs your input before implementation starts.
Backend API Design
personTypeinPersonUpdateDTO: preserve existing type vs. require explicit value — If nullable: a partial update that omits the field silently keeps the existing type (safe for future API clients, consistent with how all other optional DTO fields work). If required (non-null): the API contract is clearer and no null-guard needed in the service. In practice the form always sends a value (hidden input with default), so either works for the UI. Lean nullable unless you want to enforce that callers always declare intent. (Raised by: Markus)It's non-nullable, PersonType is always required
Implementierung abgeschlossen ✅
Alle 16 Tasks wurden umgesetzt. Branch:
feat/issue-218-person-title-type-fieldsBackend (15 neue Tests, alle grün — 1353 total)
aac8250a—INVALID_PERSON_TYPEErrorCode + i18n-Übersetzungen (de/en/es)6c117611—personType(@NotNull) +title(@Size max 50) inPersonUpdateDTO;createPersonpersistiertpersonTypefef021bf—createPersonwirftINVALID_PERSON_TYPEbei SKIP39f722fe—updatePersonpersistiertpersonType60a278ad—updatePersonwirftINVALID_PERSON_TYPEbei SKIP58b3dabe—PersonController.validatePersonNames(): Vorname nur für Typ PERSON Pflichtfeld; Institution/Gruppe/Unbekannt erlaubt keinen VornamenFrontend (23 neue Server-Tests, alle grün — 377 total)
bf313801—radioGroupNav.tsSvelte-Action: ArrowLeft/Right-Navigation, Wrap-around,aria-checked-Updates; 7 Browser-Testse7573bbe— SKIP→UNKNOWN-Normalisierung imloadder Edit-Route; 6 Unit-Tests8f755525— i18n-Keys:form_label_person_type,form_label_name,a11y_type_changed(mit{type}-Param) in de/en/esfe830ad6—PersonTypeSelector.svelte: 4-Segment-Steuerlement (PERSON/INSTITUTION/GRUPPE/UNBEKANNT),role="radiogroup",use:radioGroupNav, 2×2-Mobil-Grid,min-h-[48px]Touch-Targets,aria-live="polite"-Ankündigung17863869—PersonEditForm.svelteneu: Typ-Selektor oben, Titel-Feld (max 50 Zeichen), bedingte Felder nach §4-Matrix (Vorname/Alias/Jahresangaben nur für PERSON),lastNameLabelwechselt zu „Name" für Institution/Gruppeecf93f4b— Edit-Action:personType+titleaus FormData; Vorname-Validierung typabhängig378d35d4— Neues-Person-Formular + Server-Action analog: Typ-Selektor, Titel, bedingte Felder, Formwerte bei Fehler zurückgeben86032d5c—PersonCard.svelte: Titel in Small-Caps über dem Anzeigenamen (nur Typ PERSON, nur wenn nicht leer)Ergebnis
./mvnw test→ 1353/1353 Tests grünvitest run --project=server→ 377/377 Tests grünsvelte-check: Keine neuen Fehler in geänderten Dateien