feat(persons): visually distinguish incomplete placeholder persons + filter by completeness on /persons #323
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?
Context
/personsrenders 221 persons. Many are placeholders inherited from the ODS import — examples pulled from the live list on 2026-04-24:? ?(13 docs)B.Zeer ?(3 docs)Carl ?(3 docs)Conrad ?(2 docs)DeNaeyer ?(1 doc)Elisabeth geb Fernow(1 doc) — only first name knownEllen de Gruyter ?(1 doc) — partial alias onlyVisually these look identical to fully resolved persons. Admins can't triage at a glance which need enrichment. Volunteers who could help fill gaps can't narrow the list.
This also undermines the crowd workflow at a different layer from the transcribe-onboarding work in #320 and #327: before transcribers draw regions, someone needs to link each letter to two real people. Today that's a needle-in-a-haystack against 221 rows.
Related but not duplicate:
titleandpersonTypefieldsNon-goals
Proposed model
"Incompleteness" definition (tune against real data)
Sample 30 placeholders from the live list; confirm the rule set catches them.
Compute
completenessStatus: INCOMPLETE | PARTIAL | COMPLETEon the fly (not stored):lastName IS NULL OR lastName = ''lastName = '?'ORfirstName = '?'firstNameANDlastNameboth missing,aliasessize ≤ 1birthYear IS NULL AND deathYear IS NULLaliases IS EMPTYExpose on the API
PersonSummaryDTOgainscompletenessStatus: CompletenessStatus(new enum)./api/persons/searchaccepts optionalstatus=incomplete|partial|complete|all.Show in the UI
PersonCardrenders a muted variant + small badge (Unvollständig/Teilweise) when notCOMPLETE./personsgets a filter chip group under the search bar:Alle (221) · Unvollständig (N) · Teilweise (M) · Vollständig (L). Selection sets?status=...in URL.COMPLETE.Implementation plan
Backend
CompletenessStatus { INCOMPLETE, PARTIAL, COMPLETE }inmodel/.PersonService.computeStatus(Person)— pure function, unit-testable.PersonSummaryDTOgetscompletenessStatus(@Schema(requiredMode = REQUIRED)).PersonController.searchacceptsstatusquery param; filter applied after core search.PersonSummaryDTOflow — keep the same paging/sort semantics.Frontend
frontend/src/lib/components/PersonCard.svelte— readcompletenessStatus, add badge + muted class variant.frontend/src/routes/persons/+page.svelte— filter chips, URL sync.frontend/src/routes/persons/+page.server.ts— passstatusto the API.frontend/src/routes/persons/[id]/+page.svelte— badge beside name.i18n
3–4 new keys:
persons_filter_all,persons_filter_incomplete,persons_filter_partial,persons_filter_complete,person_badge_incomplete,person_badge_partial.Tests
computeStatuscovers each rule branch — include real placeholder samples from current data./api/persons/search?status=incompletereturns only incomplete persons;status=allreturns everything.PersonCardrenders each status variant./persons?status=incomplete, assert only placeholder rows are visible; check the header count matches.Verification
curl -s .../api/persons/search?status=incomplete | jq '.total'. Eyeball against/personslist — should be roughly 40–80 based on the sample.? ?—Unvollständigbadge.Acceptance criteria
CompletenessStatusenum defined + exposed onPersonSummaryDTOPersonService.computeStatuscovers the rules above with tests/persons(Alle / Unvollständig / Teilweise / Vollständig), URL-shareable/personssort, search, paginationCritical files
Related
👨💻 Markus Keller — Application Architect
Observations
PersonSummaryDTOis an interface projection, not a class. The issue says to addcompletenessStatusas a field on it, but interface projections cannot carry computed values directly — they map database columns 1:1. AddingcompletenessStatusrequires either (a) a default method that computes status from the other getters, or (b) converting to a class-based DTO. Option (a) is usable but mixes data and computation in the DTO layer, which is a smell. Option (b) breaks the native query projection pattern used by bothfindAllWithDocumentCountandsearchWithDocumentCount.WHEREclause to those queries. Post-query filtering in Java works but bypasses pagination if pagination is ever added.PersonSummaryDTOlacksnameAliases, which the completeness rule needs (aliases IS EMPTY). The interface projection does not include the alias collection — only the legacyaliasvarchar column (from before theperson_name_aliasestable existed). The issue's INCOMPLETE rule referencesaliases size ≤ 1, but in the current data model, aliases live inperson_name_aliases(a separate table), not inPerson.alias. This is a data model gap that affects the rule definition.CompletenessStatusplacement. The issue places the enum inmodel/— but no such package exists. Theperson/package owns all person domain types. The enum should live inorg.raddatz.familienarchiv.person.completenessStatusis computed on the fly — no schema change, no migration. This is correctly called out in the issue ("not stored")./api/persons/searchroute mentioned in the issue does not exist. The current endpoint isGET /api/personswith optionalqquery param. Addingstatusto that endpoint is the right move — consistent with the existing pattern.PersonController.getPersonscurrently callspersonService.findAll(q). Adding the status filter meansfindAllneeds a second parameter or a new method. Given the existing clean split betweenfindAllWithDocumentCountandsearchWithDocumentCount, the cleanest approach is to keep filtering in the service layer (not the controller) and passstatusthrough.Recommendations
PersonSummaryDTOto computecompletenessStatusfrom the existing getters —getFirstName(),getLastName(),getBirthYear(),getDeathYear(), and the count of aliases fromgetNameAliasCount()(add one more native query projection column). This avoids DTO conversion while keeping the computation close to the data.nameAliasCount(integer) to the native SQL projections so the DTO can compute completeness without a second DB query. A subselect(SELECT COUNT(*) FROM person_name_aliases WHERE person_id = p.id) AS nameAliasCountalongside the existingdocumentCountsubselect is the right pattern.PersonService.findAll— accept(String q, String status)— and filter the returned list. Keep it in Java; a SQL WHERE on a computed value would require a CTE or subquery, adding query complexity for a list that will stay small.CompletenessStatusinorg.raddatz.familienarchiv.person, not a non-existentmodel/package.CLAUDE.mdpackage table anddocs/architecture/c4/l3-backend-*.pumlfor the newCompletenessStatusenum and any controller signature change. The doc obligation table is non-negotiable for PR merge.Open Decisions
aliasesto mean theperson_name_aliasestable, but the legacyPerson.aliasvarchar column also exists. Clarify whether "has at least one alias" meansnameAliases.size() >= 1(the formal alias table) oralias IS NOT NULL(the legacy column), or both. These are different data concepts in the current schema./personsever gets server-side pagination, post-Java-filtering breaks paging counts. For now it's fine, but a brief design note acknowledging this constraint is worth adding to the implementation.👨💻 Felix Brandt — Fullstack Developer
Observations
PersonSummaryDTOis an interface projection. You cannot add a non-default method to it that returnsCompletenessStatusfrom a database column — there is no such column. Adefaultmethod that derives status from the other getters works, but it puts business logic in the DTO layer. The clean alternative isPersonService.computeStatus(PersonSummaryDTO)— a pure static or instance method — and the issue correctly identifies this. Recommend keeping the computation in the service.aliases IS EMPTYrule referencesnameAliases, which the currentPersonSummaryDTOprojection does not expose. The projection only hasgetAlias()(the legacy varchar). To evaluate the alias count rule, you need either a new projection column (nameAliasCount) or a separate query — the former is clearly preferable.PersonController.getPersonssignature: currentlyfindAll(String q). Addingstatusto the controller and service is straightforward, but the controller should pass it through, not apply the filter itself. Controllers delegate; services contain logic.+page.sveltecompleteness badge inline in the card template. The current/persons/+page.sveltecard content is already 170 lines. Adding badge rendering inline will push it well past the 60-line component limit. The badge rendering should be extracted — the issue already plans aPersonCardinfrontend/src/lib/components/PersonCard.svelte, but note that the existing/persons/[id]/PersonCard.svelteis the detail card. The list card is currently inline in+page.svelte. The cleaner move is to extract the list card into a named component (e.g.,PersonListCard.svelte) in thepersons/route folder.goto()with a URL query param for search — the right pattern for SvelteKit. The same pattern should be used for thestatusfilter:?status=incomplete. Derive the active chip fromdata.status(server-loaded), not from local$state. This ensures URL-shareability and SSR correctness.$effectforqsync. The current page has$effect(() => { if (!qFocused) q = data.q || ''; }). The same effect pattern must be replicated forstatusif it's reactive. However, for chips where the user clicks rather than types, this is simpler — no debounce needed,goto()fires immediately on chip click.frontend/src/lib/components/PersonCard.svelte— this path doesn't exist. The person library components live in$lib/person/. Placing a newPersonCardthere is consistent. The detail-pagePersonCard.sveltelives atroutes/persons/[id]/PersonCard.svelte— a different visual concern. Choose names carefully to avoid confusion across these two locations.Recommendations
nameAliasCountas a projection column to bothfindAllWithDocumentCountandsearchWithDocumentCountnative queries. Use a(SELECT COUNT(*) FROM person_name_aliases WHERE person_id = p.id)subselect.PersonService.computeStatus(PersonSummaryDTO dto)— pure static method, unit-testable with a mock DTO, no Spring context required. Call it after loading the list, or havefindAllreturn enriched DTOs.PersonListCard.sveltefrom the inline card markup in+page.svelte. It receivesperson(withcompletenessStatus) and renders the full card including the new badge.goto(\/persons?${params}`)with aURLSearchParamsbuilder** that mergesqandstatus` — preserving both when either changes.{#each data.persons as person (person.id)}keyed — already correct in the current template. Do not lose the key when adding the badge wrapper.npm run generate:apiafter addingcompletenessStatustoPersonSummaryDTO. The TypeScript types will then carrycompletenessStatusas a required string field. Annotate with@Schema(requiredMode = REQUIRED)on the interface default method if using a class-based DTO, or document the enum string values explicitly in the OpenAPI annotation.👨💻 Tobias Wendt — DevOps & Platform Engineer
Observations
PersonServiceTest.computeStatusunit tests andPersonControllerTestadditions will add a few hundred milliseconds to the test run at most. Well within the <10s unit test target and <2min integration target./actuator/healthendpoint and the existing MinIO/PostgreSQLdepends_on: service_healthyconditions indocker-compose.ymlneed no changes.docker-compose.ymlhas pending changes (shown in git status as modified). If those changes introduce anything relevant to the person domain (e.g., a new service), it should be reviewed before this feature branch is created. Since I can't see the diff, flagging this as a precaution.Recommendations
docker-compose.yml(visible in git status) is committed or intentional before branching — to avoid carrying unrelated changes into the feature branch.curl -s .../api/persons?status=incomplete | jq '.total'(or equivalent) to the post-deploy verification script, as the issue itself suggests. This confirms the filter is wired end-to-end in production without needing a full E2E run.👨💻 Elicit — Requirements Engineer
Observations
firstName AND lastName both missing, aliases size ≤ 1appears to overlap with the first two INCOMPLETE conditions (lastName IS NULL or = '?'). If lastName is NULL, the first condition already fires. The third condition (both missing AND aliases ≤ 1) seems to target a specific edge case — a person with no name at all but one alias — but this could never pass thelastName NOT NULLDB constraint seen in the entity (@Column(nullable = false)). Clarify whether this condition is reachable given the schema constraint, or remove it.aliasesambiguity: the issue usesaliasesloosely. ThePersonentity has bothalias(legacy varchar, single string) andnameAliases(theperson_name_aliasestable, a proper collection). The INCOMPLETE/PARTIAL rules referencealiases IS EMPTYandaliases size ≤ 1— which collection is meant? The legacyaliasfield is always a single string (or null); thenameAliasescollection can have 0..N entries. These are semantically different.? ?with 13 documents might have no formal aliases but are clearly incomplete by other signals. The OR could cause many otherwise-clean persons to show as PARTIAL.q=Hansand clicksUnvollständig, the count shown in the chip should be the count of incomplete persons matching "Hans" — not the global count. This requires the backend to return counts broken down by status for the current search result, which the current API design does not support. The issue says "Counts in chips reflect the current search" in the AC but does not address how the backend delivers these counts. This is a gap.persons_filter_all,persons_filter_incomplete,persons_filter_partial,persons_filter_complete,person_badge_incomplete,person_badge_partial— but notperson_badge_complete. If COMPLETE persons show no badge (by design), this is fine and should be explicitly stated: "COMPLETE persons render no badge."Recommendations
aliasesterm in the completeness rule: specify exactly which field —alias(varchar),nameAliases(collection), or both — is used for each INCOMPLETE/PARTIAL condition. A one-line note in the implementation plan is enough.firstName AND lastName both missing, aliases size ≤ 1INCOMPLETE sub-rule iflastNameis constrained NOT NULL at the schema level and this case cannot occur.{ incomplete: N, partial: M, complete: L }count breakdown alongside the list (a new response field), or (b) counts are computed client-side from the full unfiltered list — which breaks whenqis active. This is a genuine design decision needing resolution before implementation.Open Decisions
aliases IS EMPTYalone (without missing birth/death) be sufficient to classify a person as PARTIAL? This will likely flag a large portion of the 221 persons.👨💻 Nora "NullX" Steiner — Security Engineer
Observations
statusquery parameter is a filter on an enum (incomplete|partial|complete|all). It drives a Java-side list filter — it does not enter a JPQL query or native SQL string. There is no injection risk.GET /api/personsendpoint already requires@RequirePermission(Permission.READ_ALL), which is correct and carries over naturally to the status-filtered response. No new endpoint means no new permission gap.PersonControllerusesResponseStatusExceptionfor validation errors in several places (e.g.,validatePersonNames) rather thanDomainException. Addingstatusparam validation should use the same lightweight approach: ifstatusis an unrecognized value, return 400. Since it's an enum, Spring can deserialize it automatically with@RequestParam(required = false) CompletenessStatus status— an invalid string will trigger a Spring-managedMethodArgumentTypeMismatchExceptionresulting in a 400 before any service code runs. This is safe by default.ErrorCoderequired unless the feature decides to surface invalidstatusvalues as structured errors. Given this is a filter param (not a domain operation), a plain 400 from Spring's type coercion is sufficient and consistent with the existing style.PersonSummaryDTOwill exposecompletenessStatusin the API response. This is metadata about data quality, not sensitive personal data. No privacy concern.completenessStatusis in the OpenAPI spec, it must not be typed asanyon the frontend. The@Schemaannotation (withrequiredMode = REQUIREDif it is always present) drives the generated type. Verify afternpm run generate:apithat the field is non-optional in the generated TypeScript.Recommendations
@RequestParam(required = false) CompletenessStatus statusin the controller — let Spring's enum deserialization handle validation. Invalid values produce a 400 automatically, no manual validation code needed.@RequirePermissionannotation needed — the existingREAD_ALLcheck onGET /api/personsis the correct gate. Confirm it remains on the updated endpoint signature.PersonControllerfor missing@RequirePermissionongetCorrespondentsandgetPersonDocuments: these endpoints currently have no permission annotation (visible in the current code). They are accessible to any authenticated user. This is pre-existing, but worth flagging — especiallygetPersonDocuments, which returns full document objects. Consider adding@RequirePermission(Permission.READ_ALL)to close this gap as part of this PR.👨💻 Sara Holt — QA Engineer & Test Strategist
Observations
computeStatus, controller slice tests for thestatusparam, component tests forPersonCardbadge rendering, and an E2E test for the filter — this covers all four pyramid layers correctly.PersonServiceTestuses@ExtendWith(MockitoExtension.class)with@Mock+@InjectMocks— the correct pattern. The newcomputeStatusunit tests should follow the same class and use the same setup. No Spring context overhead.computeStatusrule set has multiple branches (INCOMPLETE conditions are OR-chained, PARTIAL has an OR). Each branch needs an isolated test with a factory method producing exactly the right person state. The issue calls this out ("include real placeholder samples from current data") — this is the right instinct. I'd formalize it as: one test per INCOMPLETE condition, one per PARTIAL condition, one COMPLETE test, and at least two real-world placeholder names from the live data (e.g.,? ?,Carl ?).PersonControllerTestwill need amockPersonSummaryWithStatushelper that includescompletenessStatus. The existingmockPersonSummaryanonymous implementation does not return acompletenessStatusgetter — it will need to be extended./persons?status=incomplete, assert only placeholder rows visible") is appropriate for the critical journey layer. However, this test will depend on seed data. Make sure the E2E seed includes at least one known INCOMPLETE person and one known COMPLETE person so the assertion is deterministic.PersonCardbadge rendering should test three states — INCOMPLETE (badge visible, muted class), PARTIAL (badge visible), and COMPLETE (no badge). Userender()fromvitest-browser-svelteandgetByRole('status')or adata-testidif the badge has no semantic role./personswithoutstatusparam: confirm existing sort, search, and pagination still work unchanged. The issue calls this out, but it should be an explicit test case, not just a manual check.Recommendations
computeStatustest cases as a@ParameterizedTestwith a stream ofArgumentscovering each rule branch — one per INCOMPLETE sub-condition, PARTIAL with/without birth year, PARTIAL with/without aliases, and COMPLETE. Parameterized tests are more readable for rule tables than individual@Testmethods.status=allexplicitly in the controller test: verify it returns the same count as no-status, and that the service is called withnullstatus (or an explicitALLenum value) in that case.status=unknown_valuein the controller test: verify it returns 400 (Spring enum deserialization rejection).{ counts: { incomplete: N, partial: M, complete: L } }in the response, or (if counts are client-computed) a frontend unit test on the count derivation logic.data-testid="completeness-badge"on the badge so component and E2E tests have a stable selector that doesn't break on label text changes (e.g., when i18n translations change).👨💻 Leonie Voss — UI/UX Design Lead
Observations
bg-surface border border-line shadow-smfor the card body. A badge that readsUnvollständigintext-[11px]on a muted background may fail the WCAG 4.5:1 contrast requirement. The brand-mint color on white already fails at ~2.8:1 — do not use mint for badge text. Usetext-ink-2onbg-muted(which has sufficient contrast based on the current palette).Alle (221) · Unvollständig (N) · Teilweise (M) · Vollständig (L)needs sufficient padding —py-2 px-4minimum for each chip.border-l-4 border-amber-400for PARTIAL,border-l-4 border-red-400for INCOMPLETE) rather than a text-color mute — which preserves readability while adding a clear visual signal.PersonCard.sveltedetail view is straightforward. The badge should be inline after the<h1>name — usingflex items-baseline gap-2to keep the name and badge on the same line without vertical misalignment.text-amber-600(light) /text-amber-400(dark) is a common safe pairing. Adddark:variants to any color class used on badges.focus-visible:ring-2 focus-visible:ring-brand-navy— the existing project focus pattern. Chips without visible focus indicators fail WCAG 2.4.7.Recommendations
border-l-4) for incomplete card variants rather than a full muted overlay — preserves name readability for senior users while adding a clear visual cue.role="group" aria-label="Nach Vollständigkeit filtern"on the container.min-h-[44px]withpx-4on each chip. Usearia-pressed="true/false"(oraria-current="true") on the active chip.data-themedark mode test in the Playwright E2E for the badge — the axe-core a11y check should run in both light and dark mode as per the existing QA standard.Open Decisions
Decision Queue
Consolidated open decisions raised by the review personas. These require a human call before implementation starts.
Theme 1 — "Aliases" field ambiguity (Markus, Elicit)
The completeness rules reference
aliases IS EMPTYandaliases size ≤ 1, but thePersonentity has two different alias concepts:alias— a single legacy varchar column (from ODS import, always a string or null)nameAliases— theperson_name_aliasestable collection (0..N formal alias entries)Decision needed: For each INCOMPLETE/PARTIAL rule condition, specify which field is checked — the legacy
aliasvarchar, theperson_name_aliasescount, or both.Theme 2 — Chip count data source (Elicit, Sara)
The acceptance criterion says "Counts in chips reflect the current search (not fixed totals)." Two implementation options exist:
{ counts: { incomplete: N, partial: M, complete: L } }breakdown alongside the person list — computed for the activeqsearch. Requires a new response structure.statusis already applied, the chip counts for other statuses are wrong).Decision needed: Choose Option A or B. If A, the API response shape and endpoint design need to be specified before backend implementation.
Theme 3 — PARTIAL rule: OR vs. AND (Elicit, Leonie)
The PARTIAL condition is currently:
birthYear IS NULL AND deathYear IS NULLORaliases IS EMPTY.Using OR means a person with fully known birth/death years but no aliases is still PARTIAL. This will likely classify a large fraction of the 221 persons as PARTIAL (most historical persons have no formal aliases).
Decision needed: Should
aliases IS EMPTYalone be sufficient for PARTIAL, or must it be combined with missing life dates? Consider running a quick count against real data before deciding.Theme 4 — INCOMPLETE card visual treatment (Leonie)
Three options for visually differentiating INCOMPLETE/PARTIAL cards in the
/personslist:border-l-4 border-amber-400/border-red-400) — additive, preserves text contrastbg-amber-50 dark:bg-amber-950/20) — visible but conservativeDecision needed: Choose a visual treatment before the
PersonListCard.sveltecomponent is built. The treatment must work in both light and dark mode.