marcel/familienarchiv
1
0
Fork 0
Code Issues 119 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

feat(lesereisen): backend item CRUD API — add, update, remove, reorder JourneyItems #751

New Issue
Open
opened 2026-06-06 16:07:11 +02:00 by marcel · 0 comments
marcel commented 2026-06-06 16:07:11 +02:00
Owner
Copy Link

Goal

Expose the JourneyItem model via REST endpoints so the frontend curator editor can build, reorder, and annotate a Reading Journey.

Background

Builds on the data model from the preceding issue (GeschichteType + JourneyItem entity). Design spec: docs/superpowers/specs/2026-06-06-lesereisen-design.md.

API surface

All endpoints require BLOG_WRITE permission.

POST   /api/geschichten/{id}/items              — append item
PATCH  /api/geschichten/{id}/items/{itemId}     — update note
DELETE /api/geschichten/{id}/items/{itemId}     — remove item
PUT    /api/geschichten/{id}/items/reorder      — body: ordered list of item IDs

POST body

{ "documentId": "uuid (optional)", "note": "string (optional)" }

At least one of documentId or note (non-blank after trim) must be present; the backend returns 400 if both are absent or the note trims to empty with no document.

New item is appended with position = max(existing positions) + 10. If no items exist yet, position starts at 10.

POST to a non-JOURNEY-type Geschichte returns 400. POST with a documentId that does not resolve to a Document returns 404 DOCUMENT_NOT_FOUND (see Review Insights — falls out of documentService.getDocumentById(), semantically honest).

POST response

Returns the created item as a JourneyItemView record (same shape as the GET item sub-object — see Review Insights "Read-model shape"):

{ "id": "...", "position": 10, "document": { "id": "...", "title": "...", "documentDate": "...", "documentDateEnd": "...", "datePrecision": "YEAR", "senderName": "...", "receiverName": "...", "receiverCount": 1 }, "note": null }

PATCH /items/{itemId}

Only the note field is mutable after creation. documentId and position cannot be changed via PATCH (position changes go through /reorder). Attaching a document to an existing note-only interlude is a non-goal (see Review Insights) — curator deletes and re-adds. Note is plain text — no HTML. note: null is accepted and clears the note field (item must still have a document if note is cleared — returns 400 if item has no document). A note that trims to empty is stored as null.

PUT /reorder body

{ "itemIds": ["uuid1", "uuid2", "uuid3"] }

Backend reassigns positions 10, 20, 30, … in the given order. Returns 400 if the provided IDs don't match the journey's current items (set equality — both extra IDs and missing IDs trigger 400). Empty [] on an empty journey returns 200 (valid no-op); empty [] on a non-empty journey returns 400. itemIds.size() is capped at max(MAX_ITEMS, currentItemCount) (so grandfathered over-cap journeys stay reorderable — see Review Insights) before the set-equality check (defense-in-depth).

PUT /reorder response: returns the updated items array (same shape as GET /api/geschichten/{id} items), enabling optimistic-update reconciliation on the frontend without a follow-up GET.

Updated GET /api/geschichten/{id} response

The documents array is replaced by items for JOURNEY type only. Non-JOURNEY Geschichten retain the existing documents field unchanged — no breaking change to existing Geschichte display. items is only present when type == JOURNEY — discriminated union by type:

{
  "type": "JOURNEY",
  "items": [
    { "id": "...", "position": 10, "document": { "id": "...", "title": "...", "documentDate": "...", "documentDateEnd": null, "datePrecision": "YEAR", "senderName": "Franz", "receiverName": "Emma", "receiverCount": 1 }, "note": null },
    { "id": "...", "position": 20, "document": null, "note": "Drei Jahre vergingen ohne Nachricht." }
  ]
}

Items are returned ordered by position ASC — this is the guaranteed display order for the reader view.

This is a breaking change — run npm run generate:api in frontend/ after this lands.

Implementation notes

  • JourneyItemService owns all JourneyItemRepository calls — do not add item methods to GeschichteService (already ~200 lines; would exceed 300). Model: AnnotationService for the annotation sub-domain. GeschichteController calls journeyItemService directly for item operations (avoids circular injection under Spring Framework 7 rules). The append method is named append (not create) to match the AC vocabulary.
  • GeschichteController delegates to JourneyItemService; GeschichteService is not the intermediary for item operations.
  • IDOR protection: use findByIdAndGeschichteId(itemId, geschichteId) in the repository for all PATCH and DELETE operations — one query, correct ownership check. Returns DomainException.notFound(ErrorCode.JOURNEY_ITEM_NOT_FOUND, ...) (not forbidden()) to avoid confirming item existence. Do not use findById + manual check — that's two queries and easy to accidentally skip under refactoring. findByIdAndGeschichteId must be the only access path for PATCH/DELETE — flag any bare findById in JourneyItemService during code review. Precedent: TranscriptionService.getBlock already uses findByIdAndDocumentId(...).orElseThrow(...notFound...) — cite it in the PR as house style.
  • ErrorCode.JOURNEY_ITEM_NOT_FOUND must be added: (1) ErrorCode.java, (2) frontend/src/lib/shared/errors.ts, (3) messages/{de,en,es}.json with key error_journey_item_not_found. ErrorCode.JOURNEY_ITEM_POSITION_CONFLICT must also be added (same four places, key error_journey_item_position_conflict) — see Review Insights for the 409 mapping.
  • Note field: plain text only — no HTML, no BODY_SANITIZER pass, no TipTap in the future editor (use <textarea>). Enforce with @Column(columnDefinition = "TEXT"). Same pattern as Geschichte.body line 34. Stored verbatim (HTML/script bytes stored as-is, escaping is the frontend's job via {note} text binding) — never run note through BODY_SANITIZER.
  • Position calculation: SELECT MAX(j.position) FROM JourneyItem j WHERE j.geschichte.id = :id inside @Transactional — race-safe within the transaction. MAX returns null on an empty table — COALESCE/null-handling must default to 0 so the first item lands at 10. Backed by UNIQUE(geschichte_id, position) at the DB layer (see Review Insights).
  • Reorder validation: compare set equality of provided IDs vs. SELECT id FROM journey_items WHERE geschichte_id = ?. A count-only check is insufficient — must use set equality to catch IDs from other journeys. Implementation: Set<UUID> existing = repository.findIdsByGeschichteId(geschichteId) → if (!existing.equals(Set.copyOf(dto.getItemIds()))) → throw 400. Cap itemIds.size() at max(MAX_ITEMS, currentItemCount) before the equality check.
  • JourneyItem is owned by the geschichte domain.
  • DTOs (flat in geschichte/ package): JourneyItemCreateDTO (documentId + note, both optional), JourneyItemUpdateDTO (note only — use JsonNullable<String> to distinguish absent vs. explicit null; test: absent→unchanged, null→cleared, "text"→set), JourneyReorderDTO (List<UUID> itemIds).
  • @Transactional on reorder method — required because it's a loop of N save() calls.
  • Embedded document summary in the item sub-object: DocumentSummary record { id, title, documentDate, documentDateEnd, datePrecision, senderName, receiverName, receiverCount } (see Review Insights for final shape and rationale). Built by JourneyItemService via documentService — never a serialized JPA @ManyToOne.
  • Flyway migration: assign version number at branch-cut time (after predecessor merges to main — latest in main today is V71, predecessor will take V72, so this issue is V73+), not at issue creation time. SQL must include: CHECK (position > 0) constraint on journey_items.position; CONSTRAINT uq_journey_items_geschichte_position UNIQUE (geschichte_id, position) (named explicitly — see Review Insights, the 409 remap matches this name) declared DEFERRABLE INITIALLY DEFERRED (see Review Insights "Reorder position writes"); REFERENCES geschichten(id) ON DELETE CASCADE on journey_items.geschichte_id; REFERENCES documents(id) ON DELETE SET NULL on journey_items.document_id (item becomes note-only when document is deleted — preserves curator work). Include data migration from geschichten_documents to journey_items with positions 10, 20, 30… via ROW_NUMBER() OVER (PARTITION BY geschichte_id ORDER BY document_id) * 10 (UUID ordering — deterministic but not semantically meaningful; add a SQL comment explaining this). Statement order is load-bearing (see Review Insights): (1) CREATE TABLE → (2) INSERT … SELECT FROM geschichten_documents → (3) DO $$ parity guard → (4) DROP TABLE geschichten_documents — the guard sits textually between INSERT and DROP. Add a SQL comment: -- MUST run in a single transaction; the parity guard's rollback of the DROP depends on it. Do not add CONCURRENTLY or set non-transactional.
  • DB diagrams: update docs/architecture/db/db-orm.puml and docs/architecture/db/db-relationships.puml as part of this PR — treat as a blocker. Remove geschichten_documents lines (db-orm.puml ~373–375, 439–440; db-relationships.puml ~69, 132–133), add journey_items with its three columns and two FK relationships.
  • @Schema(requiredMode = REQUIRED) on JourneyItem.id, JourneyItem.position — mandatory for correct TypeScript type generation.
  • @JsonInclude(JsonInclude.Include.NON_NULL) on the response items field — ensures items is absent (not null) for non-JOURNEY GET responses. Verify with a test asserting jsonPath("$.items").doesNotExist().
  • GeschichteControllerTest: add @MockitoBean JourneyItemService journeyItemService; before writing any new tests — prevents the existing controller test from breaking when GeschichteController gains the new dependency.
  • Audit: item add/remove/reorder must call auditService.logAfterCommit(...) — see Review Insights "Audit — wiring against the real AuditService" for the required new AuditKind values, the documentId == null contract, and the rollup decision.
  • Frontend note rendering: note field must be rendered as {note} (Svelte text binding, NOT {@html note}). Apply white-space: pre-line (NOT pre/pre-wrap — would break 320px wrapping) so curator line breaks are preserved. Never render note with .innerHTML.
  • CLAUDE.md: update geschichte/ package entry to list JourneyItemService and JourneyItemRepository. Also fix backend/CLAUDE.md Testing section: the stale "88% branch coverage" line must read 0.77 (matches pom.xml line 349).

Acceptance criteria

  • POST /api/geschichten/{id}/items creates a JourneyItem and returns it (with embedded document summary)

  • POST /items on a GESCHICHTE-type Geschichte returns 400

  • POST /items with a documentId that does not resolve to a Document returns 404 DOCUMENT_NOT_FOUND — test post_returns404_when_documentIdDoesNotExist

  • PATCH /api/geschichten/{id}/items/{itemId} updates the note field only (plain text)

  • DELETE /api/geschichten/{id}/items/{itemId} removes the item

  • DELETE /api/geschichten/{id}/items/{itemId} on an already-deleted item returns 404 JOURNEY_ITEM_NOT_FOUND (frontend treats as idempotent success) — test delete_returns404_when_itemAlreadyDeleted

  • PUT /api/geschichten/{id}/items/reorder reassigns positions in the given order and returns the updated items array

  • GET /api/geschichten/{id} returns items array ordered by position ASC for JOURNEY type; items field absent for non-JOURNEY type

  • GET /api/geschichten/{id} for GESCHICHTE type still returns documents field unchanged (no regression)

  • 400 returned when POST body has neither documentId nor note, OR note trims to empty with no document

  • Note that trims to whitespace-only is stored as null (POST and PATCH)

  • All endpoints require and enforce BLOG_WRITE

  • Tests cover the new endpoints and the 400 validation

  • PATCH/DELETE return 404 (not 403) when itemId belongs to a different journey (IDOR protection via findByIdAndGeschichteId)

  • PUT /reorder returns 400 when provided IDs contain extra or missing items (set equality check)

  • PUT /reorder with IDs from a different journey triggers 400 (set equality, not count-only) — test named reorder_returns400_when_itemIdBelongsToADifferentJourney, authenticated as a legitimate BLOG_WRITE user against journey A smuggling journey B's item ID (isolates IDOR from the permission check)

  • PUT /reorder with empty [] on empty journey returns 200; with empty [] on non-empty journey returns 400

  • PUT /reorder with itemIds.size() > max(MAX_ITEMS, currentItemCount) returns 400 before the set-equality check (size guard)

  • reorder_of_grandfathered_over_cap_journey_succeeds — a migrated 130-item journey reorders successfully (size cap = max(100, currentCount))

  • reorder_swap_two_adjacent_items_succeeds — 10↔20 swap succeeds without a transient UNIQUE violation (proves the deferrable constraint / atomic commit)

  • reorder_returns_items_in_new_order — asserts response body items[0].id == requestedFirstId && items[0].position == 10 (not status-only)

  • reorder_with_identical_current_order returns 200 (idempotent no-op)

  • reorder_conflict_surfaces_as_409 — a position conflict on the reorder path surfaces as 409 JOURNEY_ITEM_POSITION_CONFLICT, not 400 (shared remap helper covers both append and reorder)

  • append_after_reorder_continues_from_max_position — append after a reorder computes MAX+10 off the new positions

  • append_to_empty_journey_starts_at_10 — MAX returns null on empty table; COALESCE/null-handling pins position 10 (off-by-one guard)

  • PATCH with note: null clears the note field (accepted when document is present)

  • PATCH with note: null returns 400 when item has no document (would result in item with neither document nor note)

  • PATCH with the same note value (no-op) returns 200 idempotently

  • JourneyItemUpdateDTO.note three-way semantics tested against the real deserializer (MockMvc, real Jackson): absent→unchanged, null→cleared, "text"→set; assert via argThat/follow-up GET

  • GET /api/geschichten/{id} without items field when type is GESCHICHTE — test asserts jsonPath("$.items").doesNotExist()

  • GET a JOURNEY with items does not throw LazyInitializationException (integration test, not slice)

  • getById_returns_items_for_draft_journey_when_user_has_BLOG_WRITE — DRAFT JOURNEY read path returns items (curator's actual editor path)

  • getById_throws_NOT_FOUND_for_draft_journey_when_user_lacks_BLOG_WRITE — the existing DRAFT-hiding guard still fires for JOURNEY; the items load happens only after the guard passes

  • JOURNEY_ITEM_NOT_FOUND and JOURNEY_ITEM_POSITION_CONFLICT ErrorCodes added in backend, frontend errors.ts, and all three i18n files

  • Flyway migration includes ON DELETE CASCADE (geschichte FK), ON DELETE SET NULL (document FK), CHECK (position > 0), and named DEFERRABLE INITIALLY DEFERRED UNIQUE(geschichte_id, position)

  • Flyway migration SQL includes a comment on the ORDER BY document_id data migration ordering AND the single-transaction/no-CONCURRENTLY comment

  • Migration statement order is CREATE → INSERT → parity-guard DO $$ → DROP, in that textual order (guard between INSERT and DROP)

  • Migration includes a row-count parity guard that aborts before any DROP if journey_items count != geschichten_documents count

  • Integration test (service-level): delete a Document via documentService.delete(), verify JourneyItem still exists with document = null

  • Integration test (DB-level, raw SQL): DELETE FROM documents WHERE id=? via jdbcTemplate → re-fetch JourneyItem → document_id is null (proves the FK, isolated from JPA cascade)

  • Data-migration integration test: seed geschichten_documents with N rows across M geschichten via raw SQL/@Sql before migration → run migrations → assert journey_items has exactly N rows; per-geschichte assert the position set equals {10,20,…,10*n} and the row count — NO document→position coupling (UUID order is arbitrary; flake-proof)

  • Migration parity-guard test: a deliberately-seeded count mismatch leaves geschichten_documents intact (not dropped) — proves the RAISE EXCEPTION rolls back the DROP in the same Flyway transaction

  • position_check_rejects_nonpositive — raw insert via jdbcTemplate expecting DataIntegrityViolationException (postgres:16-alpine only)

  • unique_constraint_rejects_duplicate_position_per_geschichte — raw insert of duplicate (geschichte_id, position) expecting violation (postgres only)

  • Position-race contract: duplicate-position insert through JourneyItemService.append surfaces as 409 JOURNEY_ITEM_POSITION_CONFLICT (not 400 VALIDATION_ERROR); the remap matches the pinned constraint name uq_journey_items_geschichte_position (a renamed-constraint regression goes red); the 409-remap test is a service/slice test with real @Transactional, NOT a member of the non-@Transactional JourneyItemConstraintsTest

  • Regression test: note containing HTML/<script> is stored and returned verbatim by GET (no sanitization, no escaping at the API layer); the journey-with-note GET asserts content().contentType(MediaType.APPLICATION_JSON)

  • DocumentSummary round-trips ALL seven DatePrecision values — specifically a source document with metaDatePrecision = SEASON and one with APPROX round-trip that exact value through GET (no defaulting to UNKNOWN)

  • Multi-receiver DocumentSummary: a document with ≥2 linked receivers returns receiverName = first canonical receiver and receiverCount = total count

  • receiverCount contract: non-null for any document-bearing item; 0 when no receiver is linked (then receiverName == null); >=1 otherwise — test summary_returns_receiverCount_0_and_null_name_when_no_receiver

  • Name composition tested: linked Person → firstName lastName (trimmed); no Person → senderText/receiverText; neither → null

  • findByIdAndGeschichteId repository method tested: item ID matches but geschichte ID doesn't → empty; item ID doesn't exist → empty

  • IDOR test: PATCH /api/geschichten/{journeyAId}/items/{itemFromJourneyBId} authenticated as a valid BLOG_WRITE user → 404, named patch_returns404_when_itemBelongsToADifferentJourney

  • Controller tests cover 401 (unauthenticated) AND 403 (authenticated with READ_ALL but not BLOG_WRITE) for each write endpoint

  • @MockitoBean JourneyItemService added to GeschichteControllerTest

  • Existing GeschichteServiceTest / GeschichteServiceIntegrationTest / GeschichteControllerTest pass after the update() JOURNEY-guard is added (regression — fix any test calling update() with documentIds on a now-JOURNEY story)

  • GeschichteService.update() rejects documentIds on JOURNEY-type stories (prevents dual-write divergence); guard reads g.getType() from the entity — type is NOT added to GeschichteUpdateDTO (no type flips via PATCH)

  • Soft cap of 100 items per journey enforced on POST; the 101st returns 400 (VALIDATION_ERROR); the cap test seeds the first 100 items via @Sql/jdbcTemplate fixture, NOT 100 HTTP calls. Grandfathered journeys migrated over the cap are editable (incl. reorder) except for appending new items

  • Security-assumption regression test: a READ_ALL-only user (no BLOG_WRITE) GETs a published JOURNEY and receives the items[].document summaries — codifies the "uniform document reads" precondition; goes red the day document reads are scoped

  • Audit: add/remove/reorder each record an audit event via auditService.logAfterCommit(<new AuditKind>, actorId, null, Map.of("geschichteId", …)) — documentId arg is null; test verifies the call fires with documentId == null

  • Three new AuditKind values added (JOURNEY_ITEM_ADDED, JOURNEY_ITEM_REMOVED, JOURNEY_ITEMS_REORDERED) with Javadoc payload docs; JOURNEY_ITEMS_REORDERED added to ROLLUP_ELIGIBLE; AuditKind enum addition as its own atomic commit before the service work

  • DB diagram files updated (db-orm.puml, db-relationships.puml)

  • npm run generate:api run and TypeScript types regenerated AND committed in this PR (verify via git diff --stat on frontend/src/lib/api/...)

  • CLAUDE.md updated to list JourneyItemService and JourneyItemRepository in the geschichte/ package entry; backend/CLAUDE.md Testing section corrected from "88%" to 0.77

  • docs/GLOSSARY.md entries added for "Lesereise / Reading Journey", "JourneyItem", "Interlude" (note-only item), and the three new AuditKind values

  • Editor design spec (lesereisen-editor-spec.html) reorder body corrected to {itemIds:[...]}; note added to frontend issue #753

  • ADR added documenting the journey_items ordered join table, DocumentSummary-as-read-model, name-composition policy, multi-receiver canonical rule, ON DELETE SET NULL rationale, plain-text note, the two-write-path transition state, the EAGER-vs-LAZY collection rationale, the deferrable-unique-constraint choice, journey audit events being geschichte-scoped (documentId null), and the receiverCount cardinality-disclosure lossiness note

  • docs/DEPLOYMENT.md updated with the sequenced pre-V73 deploy runbook (pg_dump → verify → pull → up --no-deps backend frontend together), the expected brief blank-journey window during simultaneous recreate, and the explicit "no schema down-migration; rollback = restore-from-dump (data-loss window) or roll-forward V74" sentence; pre-merge restore-rehearsal performed against a real prod dump

  • Verify the migration/constraint test class naming matches the repo's executed-in-CI Testcontainers convention (*IT under Failsafe vs *Test under Surefire) so it actually runs in CI, not just locally

  • getById returns a GeschichteView wrapper for BOTH JOURNEY and GESCHICHTE types (uniform shape); author is summarized to {id, displayName} only — test getById_response_does_not_contain_author_email_or_groups asserts the response has no author.email / author.groups

  • Note length soft cap of 5000 chars enforced on POST and PATCH; the 5001st char returns 400 VALIDATION_ERROR — test post_returns400_when_note_exceeds_5000_chars and patch_returns400_when_note_exceeds_5000_chars

  • DocumentSummary is built via a lean documentService.getSummaryById(UUID) that skips tagService.resolveEffectiveColors — no wasted tag-color resolution per item

  • unique_constraint_is_deferrable_initially_deferred — raw SQL against pg_constraint (condeferrable AND condeferred both true) asserting the flags directly (flush-order-proof guard for the deferrable property)

  • Positive data-migration AC: a clean migration run on N seeded rows SUCCEEDS, drops geschichten_documents, AND leaves journey_items count == N (pairs with the negative parity-guard test to catch an early-placed guard that would abort a legitimate deploy)

  • Cap measurement: the 100-item cap is COUNT(*)-based, never MAX(position)-derived — test: a journey with 99 rows but MAX(position)=2000 still accepts the 100th append

  • delete_returns404_when_itemAlreadyDeleted also asserts NO JOURNEY_ITEM_REMOVED audit event fires on the 404 path

  • Whitespace-note test input includes embedded newlines ("\n \n"), not only spaces — trims to null

  • The 130-item grandfathered reorder test reuses the shared static PostgreSQLContainer (not a standalone @SpringBootTest)

Review Insights

Decided (round 6): GET response wrapper + author leak closed

Decision (Markus, Nora — Decision Queue Option A): getById returns a single named GeschichteView response record for BOTH JOURNEY and GESCHICHTE types. Introduce GeschichteView in the geschichte/ package, assembled by GeschichteService.getById. It carries the existing entity fields PLUS the conditional items (@JsonInclude(NON_NULL), present only for JOURNEY), and the documents/persons for both types. author is summarized to {UUID id, String displayName} — the raw AppUser graph (email, group memberships) is never serialized. GET /api/geschichten (list) keeps returning entities; only the detail endpoint changes.

Rationale: Option B (discriminated union — raw entity for GESCHICHTE, wrapper for JOURNEY) leaves two divergent serialization shapes for one endpoint (a TS union the frontend must narrow) and lets the pre-existing author AppUser info-leak persist on the GESCHICHTE path. Option A costs ~30 lines of mapping and retypes every detail-page consumer once, but closes a real IDOR-adjacent info leak and gives a single TS type. Cost is paid once; divergence would be permanent.

Breaking-change consequence (Markus): the GET response object changes from Geschichte to GeschichteView for ALL detail consumers, not just the JOURNEY path. After npm run generate:api, grep the frontend for .documents / .author accesses on the detail response (not only .items) and retype them. Add a regression AC: GET JOURNEY response contains no author.email / author.groups.

Do NOT bolt a @Transient List<JourneyItemView> items onto the entity — that reintroduces the serialization-leak / LazyInit risks round 5 eliminated. The wrapper is assembled by the service from the repository query.

Decided (round 6): Server-side note length cap (5000 chars)

Decision (Leonie — Decision Queue Option A): enforce a soft cap of 5000 chars on note at POST and PATCH (after trim), returning 400 VALIDATION_ERROR on overflow. Symmetric with the 100-item cap; bounds the reader's worst-case 320px layout once at the API instead of defending hostile/accidental walls of text in three render surfaces (reader, editor, editor-preview). One added validation branch + two tests. #753 still applies max-w-prose for in-bounds long notes.

Decided (round 6): Lean read path for DocumentSummary

Decision (Felix — Decision Queue Option A): add documentService.getSummaryById(UUID) that returns the fields DocumentSummary needs WITHOUT running tagService.resolveEffectiveColors (DocumentService:1004). getDocumentById does a tag-color resolution pass the summary never reads — ×100 items per journey GET is wasted work that would surface as a Grafana trace anomaly. The new method keeps the domain boundary clean (DocumentService owns it) and keeps JourneyItemService.toSummary honest. JourneyItemService.toSummary calls getSummaryById, not getDocumentById.

Decided (round 6): Test-strategy reinforcements

  • JsonNullable + the test's local new ObjectMapper() (GeschichteControllerTest:47) are incompatible. The hand-rolled mapper lacks JsonNullableModule. The PATCH three-way test MUST post a raw JSON string through MockMvc (which uses the Spring-context, module-registered mapper) — it must NOT pre-serialize the body with the test class's local new ObjectMapper(). Pinned for both the controller test and the QA strategy.
  • Deferrable-constraint canary needs a flush-order-proof companion. reorder_swap_two_adjacent_items_succeeds can go green even under an immediate constraint if flush order happens not to collide. Add unique_constraint_is_deferrable_initially_deferred — a raw SQL assertion against pg_constraint (condeferrable AND condeferred both true). This is the only guard the flush order cannot fake.
  • Data-migration parity guard needs BOTH directions. The negative test ("deliberate mismatch leaves geschichten_documents intact") also passes if the guard is mis-placed before the INSERT (count 0 ≠ N → RAISE → table survives) — so it can't distinguish correct from broken-too-early. Add the positive companion: clean run on N seeded rows SUCCEEDS, drops geschichten_documents, and journey_items count == N. The early-guard placement fails the positive test.
  • Cap is COUNT(*), never MAX(position)-derived. Deletions leave permanent position gaps (delete #50 in a 100-item journey → ceiling 1010 at 99 rows); a MAX(position)/10 cap check would falsely reject appends below 100 items. Test: 99 rows with MAX(position)=2000 still accepts the 100th append.
  • Concurrent-edit recovery is ONE flow, not two. #753 must treat BOTH VALIDATION_ERROR (reorder set-mismatch) AND JOURNEY_ITEM_POSITION_CONFLICT (409) as the same human cause — "this journey was changed elsewhere — reload" — a single recovery toast, not two divergent flows.
  • u. a. / receiverCount is localized prose, not a hardcoded German abbreviation — #753 maps receiverCount through Paraglide (de/en/es), never concatenates " u. a." literally.

Decided (round 6): Operational notes

  • Deferrable constraint requires transaction-level (or session-level) pooling. Prod runs PgBouncer in transaction-pooling mode (correct today). A future switch to statement-level pooling would silently break deferred constraint checking at COMMIT, turning valid reorders into commit-time failures. One sentence in the ADR's deferrable-constraint section.
  • Audit rows for operations whose afterCommit straddles the container recreate may be lost (content is safe — it committed; the audit trail is best-effort, writeLog catches and logs). One sentence in docs/DEPLOYMENT.md.
  • Flyway must run the migration in a single transaction for the parity-guard rollback to hold. PR checklist: confirm no executeInTransaction=false / no callback splits the migration; paste the Flyway log line showing it committed atomically.

Decided: Service extraction

Decision: Extract JourneyItemService. GeschichteService is already ~200 lines; adding item CRUD would push it past 300. Pattern: AnnotationService for the annotation sub-domain under document/. GeschichteController calls journeyItemService directly — GeschichteService is not the intermediary (Spring Framework 7 prohibits constructor-injection cycles). The append method is named append (not create) to match AC vocabulary.

Decided (CORRECTED round 4): DocumentSummary final shape

Final shape: record DocumentSummary(UUID id, String title, LocalDate documentDate, LocalDate documentDateEnd, DatePrecision datePrecision, String senderName, String receiverName, Integer receiverCount) {} — a Java record (pure value object, no JPA lifecycle), owned in the geschichte/ package (it is the geschichte domain's read-model view of a document).

  • DatePrecision CORRECTION (bug fix). DatePrecision.java has seven values: DAY, MONTH, SEASON, YEAR, RANGE, APPROX, UNKNOWN — there is no "separator" value. SEASON ("Frühjahr 1922") and APPROX ("um 1938") are real in a 1899–1950 letter archive (verbatim mirror of the import normalizer, ADR-025). The summary copies the enum value through unchanged; an AC pins that SEASON/APPROX round-trip without defaulting to UNKNOWN. The reader spec (#753) must format all seven, not five.
  • fileKey DROPPED. The field does not exist on Document (entity has filePath, thumbnailKey, fileHash). The frontend fetches scans via the authenticated proxy GET /api/documents/{id}/file using document.id. Embedding a storage key would leak the internal S3 object layout. The reader needs nothing beyond id.
  • documentDateEnd + datePrecision ADDED. datePrecision maps from the entity field metaDatePrecision (column meta_date_precision) — doc.getDatePrecision() does not compile. Contract: documentDate is always the range start (metaDate), documentDateEnd the end (metaDateEnd) — never swapped.
  • senderName + receiverName ADDED as two separate fields. Two fields give the 320px reader independent reflow control. Nullable; fall back to raw attribution (senderText/receiverText) when no Person is linked.
  • receiverCount (round-4 multi-receiver decision, round-5 contract tightening). Document.receivers is @ManyToMany. receiverName = first canonical receiver; receiverCount = total linked receivers. Round-5 (Leonie): receiverCount is non-null for any document-bearing item — 0 when no receiver is linked (then receiverName == null), >=1 otherwise. This removes the "nullable Integer that's sometimes absent" ambiguity: the reader's logic is unambiguous (receiverCount > 1 → " u. a."; receiverName == null → omit the "an Y" clause). The lossiness (only the first canonical name carried) and the new cardinality disclosure (receiverCount tells readers "this letter had N recipients", a new disclosure vs. the old documents array — intended for "u. a.") are recorded in the ADR.
  • Name-composition policy (round-4). Person exposes only title, firstName, lastName, nameAliases. senderName/receiverName = linked Person's firstName + lastName trimmed; else the raw senderText/receiverText; else null. "First canonical receiver" = first receiver by deterministic sort (lastName, then firstName) when ≥1 Person is linked. JourneyItemService is the single owner — a private toSummary(Document) helper (≤20 lines), unit-testable with a mocked DocumentService. If a name-display helper exists in the person domain, call it via PersonService; do NOT duplicate firstName + lastName. Do NOT put @Transient getSummary() on the entity.
  • Built by the service, never serialized from JPA. JourneyItem.document is a JPA @ManyToOne(fetch = FetchType.LAZY) @JsonIgnore (for the FK + ON DELETE SET NULL); the summary is assembled via documentService.getDocumentById(item.getDocument().getId()) (id only — the LAZY proxy is never field-initialized). See round-5 N+1 note below.

Decided (round 5): Read-model shape — JourneyItemView response record

Decision: introduce a JourneyItemView response record { UUID id, int position, DocumentSummary document, String note } in the geschichte/ package, assembled by JourneyItemService. The GET items field and the POST/reorder responses all return JourneyItemView — never the JPA JourneyItem entity (which has a geschichte back-reference that would leak / hit LazyInitializationException, and a @JsonIgnored document).

Decision (Felix's Option B — chosen for explicitness): do NOT serialize a mapped @OneToMany collection. JourneyItemService queries items via JourneyItemRepository.findByGeschichteIdOrderByPosition(id) and assembles the List<JourneyItemView>. Rationale: matches the "DocumentSummary built by service" decision, gives a fully explicit read model with zero serialization-leak risk, and avoids the LazyInit trap of a mapped collection touched outside the read transaction. Ordering lives in the repository query (OrderByPosition); cascade/ON DELETE lives in the migration FKs. If a mapped @OneToMany is added later for any reason it MUST be @JsonIgnored — but the read model never depends on it.

N+1 guard (Felix): JourneyItem.document defaults to EAGER for @ManyToOne; make it explicit @ManyToOne(fetch = FetchType.LAZY) @JsonIgnore. The service builds the summary from getDocument().getId() only, then documentService.getDocumentById(id) — no association traversal that would fire one SELECT per item.

Decided: Note rendering and HTML policy

Decision: Plain text, stored verbatim, escaped only at render. No HTML rendering, no BODY_SANITIZER, no TipTap. @Column(columnDefinition = "TEXT"). Stored exactly as received (including <script> bytes), returned verbatim by GET — escaping is the frontend's job via {note} (never {@html}). Rejecting on < would break "5 < 10"; stripping silently mutates curator input. Backend regression test pins storage AND asserts Content-Type: application/json (round-5 Nora — prevents a mis-set text/html turning verbatim storage into reflected stored XSS). The XSS burden moves entirely to the frontend {note} binding — a frontend test in #753 must assert a note containing <img src=x onerror=alert(1)> renders as visible literal text and never executes.

Round-5 (Nora): the same verbatim-text posture applies to senderName/receiverName — they fall back to unsanitized historical-import free text (senderText/receiverText), an attacker-influenced-via-import surface the curator never typed. The #753 frontend-XSS literal-text test must cover senderName/receiverName, not just note. Noted on #753.

Whitespace handling: notes are trimmed; blank-after-trim is stored as null (POST and PATCH). Consequence: an item with note == null && document == null cannot exist — the reader needs zero defensive empty-state handling for interludes.

Decided: GET response for non-JOURNEY type, and DRAFT JOURNEY read path

Decision: items absent for non-JOURNEY; documents retained for GESCHICHTE. Discriminated union by type via @JsonInclude(NON_NULL) on the response record's items field. Items are queried LAZILY by the service (see Read-model shape) — only getById loads them, not GET /geschichten. Consequently getById must be @Transactional(readOnly=true) to assemble the read model inside one transaction.

Round-5 (Markus): @Transactional(readOnly=true) must WRAP the existing DRAFT-visibility guard, not bypass it. GeschichteService.getById (lines 63–73) throws GESCHICHTE_NOT_FOUND for DRAFT stories when the caller lacks BLOG_WRITE. The transaction spans the whole method; the items load happens after the guard passes (don't load the collection for a request about to 404). Curators build journeys in DRAFT, so two realistic editor-path tests are required: getById_returns_items_for_draft_journey_when_user_has_BLOG_WRITE and getById_throws_NOT_FOUND_for_draft_journey_when_user_lacks_BLOG_WRITE.

ADR consequence (Markus): record WHY persons/documents are EAGER (list-endpoint N+1 avoidance) and items is deliberately LAZY — so a future maintainer doesn't "fix the inconsistency" by making items EAGER and reintroduce the load-everything regression.

Decided: Two-write-path guard

Decision: GeschichteService.update() rejects documentIds when type == JOURNEY. During the transition, JOURNEY documents live in journey_items while the entity retains @ManyToMany documents (used by STORY type). personIds stay shared (no guard). The guard reads g.getType() from the entity — type is NOT a GeschichteUpdateDTO field (round-5 Felix: prevents flipping a story's type via PATCH). Existing GeschichteService* tests calling update() with documentIds on a story the migration retypes to JOURNEY will fail — fix as a regression AC. Recorded in the ADR.

Decided: PATCH mutable fields; attach-document non-goal

Decision: note only via JsonNullable<String>. A plain Optional<String> cannot distinguish absent from explicit null under Spring's Jdk8Module. Add openapi-jackson-nullable and register JsonNullableModule (its own atomic commit before the PATCH DTO work). isPresent() distinguishes absent from present; .get()==null distinguishes explicit null. Three-way test against the real deserializer (MockMvc).

Round-5 (Elicit) — DECIDED: attaching a document to an existing note-only interlude is a NON-GOAL for this issue. PATCH stays note-only; documentId is immutable. A curator who wants to turn an interlude into a document item deletes and re-adds. Rationale: keeps PATCH and its test matrix small; the friction (lost note/position) is rare and acceptable for the first cut. Stated as an explicit non-goal in the PATCH contract.

Decided: PUT /reorder response, size cap, grandfathered journeys

Decision: Return the updated items array (List<JourneyItemView>), enabling optimistic drag-reorder reconciliation without a follow-up GET. Tests assert the body, not just status. Cap itemIds.size() before the set-equality check (cheap memory-DoS block).

Round-5 (Elicit) — DECIDED: the reorder size cap is max(MAX_ITEMS, currentItemCount), not a flat 100. A grandfathered 130-item journey (migrated over the cap) must stay fully reorderable — a flat-100 cap would reject the legitimate 130-id full-set reorder and contradict the round-4 "grandfathered journeys remain editable" decision. Append still rejects past 100 (a 130-item journey cannot grow). AC reorder_of_grandfathered_over_cap_journey_succeeds.

Decided (round 5): Reorder position writes — deferrable unique constraint

Round-5 (Sara) — DECIDED: make uq_journey_items_geschichte_position DEFERRABLE INITIALLY DEFERRED (Sara's Option A). With an immediate unique constraint, a valid swap (item-B → 10 while item-A still holds 10 until flush) transiently collides mid-flush and throws DataIntegrityViolationException on a perfectly valid reorder — non-deterministic, flush-order dependent. A deferrable constraint checks once at commit: the whole 10/20/30 reassignment commits atomically, swaps never transiently collide, and a genuine duplicate (concurrent append racing the reorder) still fails at commit and gets the 409 remap. This is the textbook Postgres pattern for reassigning an ordered column. Rejected: Option B (two-phase +1000 offset — ugly, 2× UPDATEs, still racy) and Option C (immediate + frontend retry — valid reorders intermittently 409, bad curator UX). New-pattern cost (no repo precedent, slight planner cost) is negligible at family-archive scale. AC reorder_swap_two_adjacent_items_succeeds goes red with an immediate constraint and green with the deferrable one.

Decided: Position uniqueness AND race HTTP contract

Decision: DEFERRABLE INITIALLY DEFERRED UNIQUE(geschichte_id, position) (named) plus CHECK (position > 0). SELECT MAX(position)+10 inside @Transactional is not fully race-free; the unique constraint catches the tie.

Decision: the race must surface as 409, not 400 — on BOTH write paths. GlobalExceptionHandler.handleDataIntegrityViolation maps every integrity violation to 400 VALIDATION_ERROR, which blames the curator for a server-side collision. Catch DataIntegrityViolationException, inspect the constraint name, rethrow as DomainException.conflict(ErrorCode.JOURNEY_ITEM_POSITION_CONFLICT, ...) → HTTP 409, message "Another change reordered this journey — please retry."

Round-5 (Sara): the 409 remap covers append AND reorder — both write position and hit the same constraint. Extract the catch-and-remap into a private JourneyItemService helper both methods call — one place to test and maintain. AC reorder_conflict_surfaces_as_409.

Round-5 (Nora): pin the constraint name as a shared constant. Name the constraint explicitly in the migration (CONSTRAINT uq_journey_items_geschichte_position UNIQUE (...)) and match the service's remap branch against a named constant (e.g. JourneyItem.UQ_POSITION_CONSTRAINT = "uq_journey_items_geschichte_position") referenced by both. If Postgres auto-names the constraint (inline UNIQUE(...) without CONSTRAINT name) or the literal drifts, the if (constraintName.equals(...)) branch silently fails open and the curator sees the misleading 400 — a silent regression no happy-path test catches. AC: the 409-remap test asserts the named path; a renamed-constraint regression goes red.

Test isolation (Sara): the 409-remap test is a service/slice test with real @Transactional and must NOT be a member of the non-@Transactional JourneyItemConstraintsTest (raw-insert jdbcTemplate tests) — a DataIntegrityViolationException inside a class-level @Transactional marks the tx rollback-only and can cascade into TransactionSystemException on teardown.

Decided: PATCH note:null on document-less item

Decision: Return 400. Consistent with the POST invariant. Auto-delete would be silent data loss; a tombstone forces reader-side null handling.

Decided: DELETE idempotency

Round-5 (Elicit) — DECIDED: DELETE /items/{itemId} on an already-deleted item returns 404 JOURNEY_ITEM_NOT_FOUND (not 204). Consistent with the IDOR contract (findByIdAndGeschichteId returns empty → notFound). For a drag-heavy editor with optimistic local state, the frontend treats "already gone" as success client-side and suppresses the toast. AC delete_returns404_when_itemAlreadyDeleted.

Decided: ErrorCodes

Decision: Add JOURNEY_ITEM_NOT_FOUND (do not reuse GESCHICHTE_NOT_FOUND) and JOURNEY_ITEM_POSITION_CONFLICT (409). Each in all four places: ErrorCode.java, errors.ts, all three messages/*.json. Keys: error_journey_item_not_found, error_journey_item_position_conflict. The reorder set-mismatch 400 reuses VALIDATION_ERROR but with a distinguishable message so #753 can show "this journey was changed elsewhere — reload" rather than a generic validation error (round-5 Elicit; concurrent-edit recovery path).

Decided: POST contracts — non-JOURNEY, bad documentId, max-items cap

Decision: POST to a non-JOURNEY Geschichte returns 400 (VALIDATION_ERROR). Verify the parent exists AND type == JOURNEY before creating.

POST with a non-existent documentId returns 404 DOCUMENT_NOT_FOUND — falls out of documentService.getDocumentById() (precedent: GeschichteService.resolveDocuments lines 179–186 already loops it), semantically honest, pinned by AC so the frontend handles 404 not 400.

Soft cap of 100 items per journey, enforced on POST (the 101st returns 400 VALIDATION_ERROR). Pre-existing journeys migrated over the cap are grandfathered — editable (incl. reorder, via the max(100, currentCount) reorder cap) except adding items. Cap test seeds 100 items via fixture, not 100 HTTP calls.

Decided (round 5): Audit — wiring against the real AuditService

Round-5 (Markus): the audit decision is unbuildable as previously written and is now corrected.

  • AuditService signatures are log(AuditKind, UUID actorId, UUID documentId, Map payload) and logAfterCommit(...) — the only entity dimension is documentId (AuditLog.java line 40, nullable). A reorder is not about a document; an interlude add has no document.
  • DECIDED: add three new AuditKind values — JOURNEY_ITEM_ADDED, JOURNEY_ITEM_REMOVED, JOURNEY_ITEMS_REORDERED — with Javadoc payload docs matching house style, as their own atomic commit before the service work (same discipline as the jackson-nullable commit). The audit AC becomes: logAfterCommit(<new kind>, actorId, null, Map.of("geschichteId", id, …)) — documentId arg is null; geschichteId lives in the jsonb payload. Jamming geschichteId into the documentId slot would corrupt every document-scoped audit query (AuditLogQueryService filters by documentId).
  • DECIDED (Markus's Option A): add JOURNEY_ITEMS_REORDERED to ROLLUP_ELIGIBLE; leave add/remove out. Drag-reordering fires many reorders per editing session (a 30-drop session → 30 rows); rollup keeps the chronik readable. Per-reorder forensic granularity is not worth flooding the family chronik for a curation tool. Add/remove are discrete, low-frequency, individually meaningful — not rolled up.
  • ADR addition: journey audit events are geschistee-scoped, not document-scoped (documentId deliberately null) — so a future maintainer doesn't "fix" the null by back-filling a document id. New AuditKind values are a domain enum addition → docs/GLOSSARY.md + the audit ADR.

Decided: JOURNEY intro body format

Decision: Out of scope here, recorded. When the editor issue lands, JOURNEY intro should be plaintext (consistent with "no TipTap for journeys"). This issue does not change body.

Decided: Migration drop strategy

Decision: Single PR, DROP gated by an in-SQL row-count parity guard. Flyway is forward-only and runs automatically on prod boot on a single VPS with no staging. One PR for schema cleanliness; the drop is self-protecting — DO $$ BEGIN IF (SELECT count(*) FROM journey_items) <> (SELECT count(*) FROM geschichten_documents) THEN RAISE EXCEPTION ...; END IF; END $$; aborts before the DROP if counts disagree. Single-transaction is load-bearing: Postgres transactional DDL + Flyway's per-migration transaction means RAISE EXCEPTION rolls back the DROP. A SQL comment forbids CONCURRENTLY/non-transactional; a test seeds a deliberate mismatch and asserts geschichten_documents survives.

Round-5 (Tobias): pin the intra-file statement order — (1) CREATE TABLE → (2) INSERT … SELECT FROM geschichten_documents → (3) DO $$ parity guard → (4) DROP TABLE. The guard sits textually between INSERT and DROP; reordering the guard above the INSERT (guards "feel like" preconditions) compares 0 vs N and aborts every deploy or passes vacuously. AC pins the order.

Decided: Document-read access model (security side channel)

Decision: Document reads are uniform across authenticated users — SecurityConfig line 77/120 is anyRequest().authenticated(); DocumentController GET /{id} (line 136) and GET /{id}/file (line 86) carry no @RequirePermission. Embedding DocumentSummary in a published journey discloses nothing a reader couldn't already fetch. But this rests on an implicit default, not an explicit policy — a security-assumption regression test (a READ_ALL-only user GETs a published JOURNEY and sees items[].document) codifies it and goes red the day document reads are scoped, forcing the drafted redaction approach ("[Dokument nicht verfügbar]" per-item, not whole-journey 403) to be implemented rather than forgotten.

Resolved contradiction: reorder body contract

This issue's { "itemIds": [...] } is authoritative. The editor design spec (lesereisen-editor-spec.html lines 523/762/783) currently says [{id, position}] — correct it to {itemIds:[...]} as part of this PR, with a note on frontend issue #753.

Corrected: coverage gate number

The enforced JaCoCo gate is <minimum>0.77</minimum> (backend/pom.xml line 349, ratcheted per #496) — not 88%. Round-5 (Markus): backend/CLAUDE.md Testing section still says "88% branch coverage" — fix it to 0.77 in this PR (the number is load-bearing for "do not panic-pad tests"). The ~14–16-branch estimate still holds and every branch needs an explicit test.

Test strategy (layer assignment + container sharing)

  • Tag each test AC by pyramid layer: unit (Mockito) — DTO three-way semantics, position-MAX+10 arithmetic, name-composition toSummary; @WebMvcTest slice — 401/403 per endpoint, jsonPath($.items).doesNotExist(), @MockitoBean wiring; Testcontainers integration — every FK/constraint/cascade/data-migration AC plus the LazyInitializationException-free GET and the DRAFT-JOURNEY read tests.
  • Consolidate FK/constraint/cascade/data-migration tests into ONE JourneyItemConstraintsTest sharing a single static PostgreSQLContainer (~6 methods) — saves ~60–80s CI per run. The 409-remap service test is NOT a member of this class (it needs real @Transactional).
  • Raw-insert constraint tests must be non-@Transactional (jdbcTemplate + explicit @Sql teardown) — a DataIntegrityViolationException inside a class-level @Transactional test marks the tx rollback-only and cascades into TransactionSystemException on teardown.
  • Data-migration assertion style: assert row count == seeded count and per-geschichte position set == {10,20,…,10*n}; never couple a specific document to a specific position (UUID order is arbitrary).
  • Round-5 (Tobias): verify the integration test class naming matches the repo's executed-in-CI Testcontainers convention (*IT under Failsafe vs *Test under Surefire) — otherwise the highest-risk test passes locally and silently never runs in CI. PR checklist: paste the CI log line showing the migration test ran and that the seed @Sql ran before Flyway.

Confirmed risks

  • Reorder self-collision under an immediate unique constraint — RESOLVED via DEFERRABLE INITIALLY DEFERRED. A swap is checked at commit, not mid-flush. reorder_swap_two_adjacent_items_succeeds is the canary.
  • 409 remap matching an unpinned constraint name fails open — pin the name as a shared constant in migration + service; AC asserts the named path.
  • Audit wiring against the real AuditService — needs three new AuditKind values and documentId == null with geschichteId in the payload; jamming geschichteId into documentId corrupts document-scoped audit queries.
  • DRAFT JOURNEY read path — @Transactional(readOnly=true) must wrap the existing DRAFT-hiding guard; items load only after the guard passes. Two editor-path tests required.
  • N+1 via EAGER @ManyToOne document — make JourneyItem.document explicitly LAZY; build the summary from the id only.
  • author AppUser graph leaked on the detail GET — closed by summarizing author to {id, displayName} in GeschichteView (round 6); a regression AC asserts no author.email/author.groups ships.
  • Serializing the JPA entity instead of JourneyItemView — would leak the geschichte back-ref / hit LazyInit; return the assembled view record everywhere.
  • Grandfathered-over-cap journey un-reorderable under a flat-100 cap — reorder cap is max(100, currentCount).
  • Irreversible migration on auto-boot: Flyway runs on prod boot, no staging, single VPS, docker compose up -d --build is one command. Mitigations: in-SQL single-transaction parity guard with pinned statement order; data-migration integration test; sequenced docs/DEPLOYMENT.md runbook (pg_dump → verify → pull → up -d --no-deps backend frontend together, with the brief blank-journey window noted and the pg_restore command recorded); the explicit "no schema down-migration; rollback = restore-from-dump (data-loss window) or roll-forward V74" sentence; pre-merge restore-rehearsal against a real prod dump (covers NULL senderText, multi-receiver docs, legacy collation synthetic data misses).
  • Data-migration coverage is the single highest-risk item — seed-before / assert-after Testcontainers test is a blocker AC; seed via raw SQL / @Sql against geschichten_documents before Flyway runs.
  • CI must actually execute the migration test against postgres:16-alpine — verify class naming matches the CI-executed convention; verify the seed @Sql ran before Flyway (not vacuously after a drop).
  • note/senderName/receiverName verbatim → frontend XSS half — the verbatim decision is only safe because the reader renders text. #753 frontend tests (<img onerror> renders as literal text, no alert) must cover note AND the two name fields. Backend GET asserts Content-Type: application/json.
  • SEASON/APPROX mis-render — a five-case date formatter in #753 would silently mis-render the two omitted precisions for exactly this archive's audience. #753 hand-off: "Frühjahr/Sommer/Herbst/Winter YYYY" (SEASON) and "um YYYY" (APPROX) are required acceptance, not optional; falling back to raw YYYY-MM-DD loses the historical hedging the archive exists to preserve.
  • senderName == null && receiverName == null is real (letter with no linked Person and no senderText/receiverText). #753 hand-off: render the document title only, no attribution line — never "von an ".
  • receiverCount cardinality disclosure — a new (intended) disclosure vs. the old documents array; record in the ADR lossiness note.
  • white-space: pre-line (not pre/pre-wrap) — pre would prevent wrapping on 320px (horizontal scroll, Critical mobile failure) and preserve accidental trailing spaces. Pinned; don't let it drift in #753.
  • Concurrent curators on one journey — A reorders while B deletes → A's itemIds set includes B's gone item → set-equality 400. Correct behavior; #753 needs an honest "this journey was changed elsewhere — reload" recovery message (distinguishable from a malformed request).
  • IDOR on item mutations — findByIdAndGeschichteId mandatory for PATCH/DELETE; PUT /reorder IDOR closed by set-equality. Both IDOR tests authenticate as a valid BLOG_WRITE user.
  • Reorder set-equality vs. count-only — a count check lets an attacker mix IDs from different journeys; compare Set<UUID> equality.
  • note three-way semantics fails with plain Optional<String> under default Jackson — JsonNullable required, test hits the real deserializer.
  • GeschichteControllerTest silent breakage — add @MockitoBean JourneyItemService before any new tests.
  • Breaking change deployment — documents → items in the JOURNEY GET response; Docker Compose restarts frontend + backend together (no rolling-restart skew). Regenerated *.d.ts committed in this PR so CI builds against the new contract.
  • Predecessor not yet merged — GeschichteType enum, JourneyItem entity, and migrations beyond V71 do not yet exist in main. The branch cannot be cut and TDD red cannot start until the predecessor PR merges and CI is green; the Flyway version (V73+) is assigned at branch-cut time.
  • No new infrastructure — pure schema + REST. No new env vars, no new Docker service, no Compose changes.
## Goal Expose the `JourneyItem` model via REST endpoints so the frontend curator editor can build, reorder, and annotate a Reading Journey. ## Background Builds on the data model from the preceding issue (GeschichteType + JourneyItem entity). Design spec: `docs/superpowers/specs/2026-06-06-lesereisen-design.md`. ## API surface All endpoints require `BLOG_WRITE` permission. ``` POST /api/geschichten/{id}/items — append item PATCH /api/geschichten/{id}/items/{itemId} — update note DELETE /api/geschichten/{id}/items/{itemId} — remove item PUT /api/geschichten/{id}/items/reorder — body: ordered list of item IDs ``` ### POST body ```json { "documentId": "uuid (optional)", "note": "string (optional)" } ``` At least one of `documentId` or `note` (non-blank after trim) must be present; the backend returns 400 if both are absent or the note trims to empty with no document. New item is appended with `position = max(existing positions) + 10`. If no items exist yet, position starts at 10. POST to a non-JOURNEY-type Geschichte returns 400. POST with a `documentId` that does not resolve to a Document returns **404 `DOCUMENT_NOT_FOUND`** (see Review Insights — falls out of `documentService.getDocumentById()`, semantically honest). ### POST response Returns the created item as a `JourneyItemView` record (same shape as the GET item sub-object — see Review Insights "Read-model shape"): ```json { "id": "...", "position": 10, "document": { "id": "...", "title": "...", "documentDate": "...", "documentDateEnd": "...", "datePrecision": "YEAR", "senderName": "...", "receiverName": "...", "receiverCount": 1 }, "note": null } ``` ### PATCH /items/{itemId} Only the `note` field is mutable after creation. `documentId` and `position` cannot be changed via PATCH (position changes go through `/reorder`). Attaching a document to an existing note-only interlude is a **non-goal** (see Review Insights) — curator deletes and re-adds. Note is plain text — no HTML. `note: null` is accepted and clears the note field (item must still have a document if note is cleared — returns 400 if item has no document). A note that trims to empty is stored as `null`. ### PUT /reorder body ```json { "itemIds": ["uuid1", "uuid2", "uuid3"] } ``` Backend reassigns positions 10, 20, 30, … in the given order. Returns 400 if the provided IDs don't match the journey's current items (set equality — both extra IDs and missing IDs trigger 400). Empty `[]` on an empty journey returns 200 (valid no-op); empty `[]` on a non-empty journey returns 400. `itemIds.size()` is capped at **`max(MAX_ITEMS, currentItemCount)`** (so grandfathered over-cap journeys stay reorderable — see Review Insights) **before** the set-equality check (defense-in-depth). **PUT /reorder response:** returns the updated items array (same shape as `GET /api/geschichten/{id}` items), enabling optimistic-update reconciliation on the frontend without a follow-up GET. ### Updated GET /api/geschichten/{id} response The `documents` array is replaced by `items` **for JOURNEY type only**. Non-JOURNEY Geschichten retain the existing `documents` field unchanged — no breaking change to existing Geschichte display. **`items` is only present when `type == JOURNEY`** — discriminated union by type: ```json { "type": "JOURNEY", "items": [ { "id": "...", "position": 10, "document": { "id": "...", "title": "...", "documentDate": "...", "documentDateEnd": null, "datePrecision": "YEAR", "senderName": "Franz", "receiverName": "Emma", "receiverCount": 1 }, "note": null }, { "id": "...", "position": 20, "document": null, "note": "Drei Jahre vergingen ohne Nachricht." } ] } ``` Items are returned **ordered by `position ASC`** — this is the guaranteed display order for the reader view. This is a **breaking change** — run `npm run generate:api` in `frontend/` after this lands. ## Implementation notes - **`JourneyItemService`** owns all `JourneyItemRepository` calls — do not add item methods to `GeschichteService` (already ~200 lines; would exceed 300). Model: `AnnotationService` for the annotation sub-domain. `GeschichteController` calls `journeyItemService` directly for item operations (avoids circular injection under Spring Framework 7 rules). The append method is named **`append`** (not `create`) to match the AC vocabulary. - `GeschichteController` delegates to `JourneyItemService`; `GeschichteService` is not the intermediary for item operations. - **IDOR protection**: use `findByIdAndGeschichteId(itemId, geschichteId)` in the repository for all PATCH and DELETE operations — one query, correct ownership check. Returns `DomainException.notFound(ErrorCode.JOURNEY_ITEM_NOT_FOUND, ...)` (not `forbidden()`) to avoid confirming item existence. Do not use `findById` + manual check — that's two queries and easy to accidentally skip under refactoring. `findByIdAndGeschichteId` must be the *only* access path for PATCH/DELETE — flag any bare `findById` in `JourneyItemService` during code review. Precedent: `TranscriptionService.getBlock` already uses `findByIdAndDocumentId(...).orElseThrow(...notFound...)` — cite it in the PR as house style. - **`ErrorCode.JOURNEY_ITEM_NOT_FOUND`** must be added: (1) `ErrorCode.java`, (2) `frontend/src/lib/shared/errors.ts`, (3) `messages/{de,en,es}.json` with key `error_journey_item_not_found`. **`ErrorCode.JOURNEY_ITEM_POSITION_CONFLICT`** must also be added (same four places, key `error_journey_item_position_conflict`) — see Review Insights for the 409 mapping. - **Note field**: plain text only — no HTML, no `BODY_SANITIZER` pass, no TipTap in the future editor (use `<textarea>`). Enforce with `@Column(columnDefinition = "TEXT")`. Same pattern as `Geschichte.body` line 34. Stored verbatim (HTML/script bytes stored as-is, escaping is the frontend's job via `{note}` text binding) — never run note through `BODY_SANITIZER`. - **Position calculation**: `SELECT MAX(j.position) FROM JourneyItem j WHERE j.geschichte.id = :id` inside `@Transactional` — race-safe within the transaction. `MAX` returns null on an empty table — `COALESCE`/null-handling must default to 0 so the first item lands at 10. Backed by `UNIQUE(geschichte_id, position)` at the DB layer (see Review Insights). - **Reorder validation**: compare set equality of provided IDs vs. `SELECT id FROM journey_items WHERE geschichte_id = ?`. A count-only check is insufficient — must use set equality to catch IDs from other journeys. Implementation: `Set<UUID> existing = repository.findIdsByGeschichteId(geschichteId)` → `if (!existing.equals(Set.copyOf(dto.getItemIds())))` → throw 400. Cap `itemIds.size()` at `max(MAX_ITEMS, currentItemCount)` before the equality check. - `JourneyItem` is owned by the `geschichte` domain. - **DTOs** (flat in `geschichte/` package): `JourneyItemCreateDTO` (documentId + note, both optional), `JourneyItemUpdateDTO` (note only — use `JsonNullable<String>` to distinguish absent vs. explicit null; test: absent→unchanged, null→cleared, "text"→set), `JourneyReorderDTO` (List\<UUID\> itemIds). - **`@Transactional` on reorder method** — required because it's a loop of N `save()` calls. - **Embedded document summary** in the item sub-object: `DocumentSummary` record `{ id, title, documentDate, documentDateEnd, datePrecision, senderName, receiverName, receiverCount }` (see Review Insights for final shape and rationale). Built by `JourneyItemService` via `documentService` — never a serialized JPA `@ManyToOne`. - **Flyway migration**: assign version number **at branch-cut time** (after predecessor merges to main — latest in `main` today is V71, predecessor will take V72, so this issue is **V73+**), not at issue creation time. SQL must include: `CHECK (position > 0)` constraint on `journey_items.position`; **`CONSTRAINT uq_journey_items_geschichte_position UNIQUE (geschichte_id, position)`** (named explicitly — see Review Insights, the 409 remap matches this name) declared **`DEFERRABLE INITIALLY DEFERRED`** (see Review Insights "Reorder position writes"); `REFERENCES geschichten(id) ON DELETE CASCADE` on `journey_items.geschichte_id`; `REFERENCES documents(id) ON DELETE SET NULL` on `journey_items.document_id` (item becomes note-only when document is deleted — preserves curator work). Include data migration from `geschichten_documents` to `journey_items` with positions 10, 20, 30… via `ROW_NUMBER() OVER (PARTITION BY geschichte_id ORDER BY document_id) * 10` (UUID ordering — deterministic but not semantically meaningful; add a SQL comment explaining this). **Statement order is load-bearing (see Review Insights): (1) CREATE TABLE → (2) INSERT … SELECT FROM geschichten_documents → (3) `DO $$` parity guard → (4) DROP TABLE geschichten_documents — the guard sits textually between INSERT and DROP.** Add a SQL comment: `-- MUST run in a single transaction; the parity guard's rollback of the DROP depends on it. Do not add CONCURRENTLY or set non-transactional.` - **DB diagrams**: update `docs/architecture/db/db-orm.puml` and `docs/architecture/db/db-relationships.puml` as part of this PR — treat as a blocker. Remove `geschichten_documents` lines (db-orm.puml ~373–375, 439–440; db-relationships.puml ~69, 132–133), add `journey_items` with its three columns and two FK relationships. - **`@Schema(requiredMode = REQUIRED)`** on `JourneyItem.id`, `JourneyItem.position` — mandatory for correct TypeScript type generation. - **`@JsonInclude(JsonInclude.Include.NON_NULL)`** on the response `items` field — ensures `items` is absent (not `null`) for non-JOURNEY GET responses. Verify with a test asserting `jsonPath("$.items").doesNotExist()`. - **`GeschichteControllerTest`**: add `@MockitoBean JourneyItemService journeyItemService;` before writing any new tests — prevents the existing controller test from breaking when `GeschichteController` gains the new dependency. - **Audit**: item add/remove/reorder must call `auditService.logAfterCommit(...)` — see Review Insights "Audit — wiring against the real AuditService" for the required new `AuditKind` values, the `documentId == null` contract, and the rollup decision. - **Frontend note rendering**: `note` field must be rendered as `{note}` (Svelte text binding, NOT `{@html note}`). Apply `white-space: pre-line` (NOT `pre`/`pre-wrap` — would break 320px wrapping) so curator line breaks are preserved. Never render note with `.innerHTML`. - **CLAUDE.md**: update `geschichte/` package entry to list `JourneyItemService` and `JourneyItemRepository`. Also fix `backend/CLAUDE.md` Testing section: the stale "88% branch coverage" line must read **0.77** (matches `pom.xml` line 349). ## Acceptance criteria - [ ] `POST /api/geschichten/{id}/items` creates a JourneyItem and returns it (with embedded document summary) - [ ] `POST /items` on a GESCHICHTE-type Geschichte returns 400 - [ ] `POST /items` with a `documentId` that does not resolve to a Document returns 404 `DOCUMENT_NOT_FOUND` — test `post_returns404_when_documentIdDoesNotExist` - [ ] `PATCH /api/geschichten/{id}/items/{itemId}` updates the note field only (plain text) - [ ] `DELETE /api/geschichten/{id}/items/{itemId}` removes the item - [ ] `DELETE /api/geschichten/{id}/items/{itemId}` on an already-deleted item returns 404 `JOURNEY_ITEM_NOT_FOUND` (frontend treats as idempotent success) — test `delete_returns404_when_itemAlreadyDeleted` - [ ] `PUT /api/geschichten/{id}/items/reorder` reassigns positions in the given order and returns the updated items array - [ ] `GET /api/geschichten/{id}` returns `items` array ordered by `position ASC` for JOURNEY type; `items` field absent for non-JOURNEY type - [ ] `GET /api/geschichten/{id}` for GESCHICHTE type still returns `documents` field unchanged (no regression) - [ ] 400 returned when POST body has neither `documentId` nor `note`, OR note trims to empty with no document - [ ] Note that trims to whitespace-only is stored as `null` (POST and PATCH) - [ ] All endpoints require and enforce `BLOG_WRITE` - [ ] Tests cover the new endpoints and the 400 validation - [ ] PATCH/DELETE return 404 (not 403) when `itemId` belongs to a different journey (IDOR protection via `findByIdAndGeschichteId`) - [ ] `PUT /reorder` returns 400 when provided IDs contain extra or missing items (set equality check) - [ ] `PUT /reorder` with IDs from a different journey triggers 400 (set equality, not count-only) — test named `reorder_returns400_when_itemIdBelongsToADifferentJourney`, authenticated as a legitimate BLOG_WRITE user against journey A smuggling journey B's item ID (isolates IDOR from the permission check) - [ ] `PUT /reorder` with empty `[]` on empty journey returns 200; with empty `[]` on non-empty journey returns 400 - [ ] `PUT /reorder` with `itemIds.size() > max(MAX_ITEMS, currentItemCount)` returns 400 before the set-equality check (size guard) - [ ] `reorder_of_grandfathered_over_cap_journey_succeeds` — a migrated 130-item journey reorders successfully (size cap = `max(100, currentCount)`) - [ ] `reorder_swap_two_adjacent_items_succeeds` — 10↔20 swap succeeds without a transient `UNIQUE` violation (proves the deferrable constraint / atomic commit) - [ ] `reorder_returns_items_in_new_order` — asserts response body `items[0].id == requestedFirstId && items[0].position == 10` (not status-only) - [ ] `reorder_with_identical_current_order` returns 200 (idempotent no-op) - [ ] `reorder_conflict_surfaces_as_409` — a position conflict on the reorder path surfaces as 409 `JOURNEY_ITEM_POSITION_CONFLICT`, not 400 (shared remap helper covers both `append` and `reorder`) - [ ] `append_after_reorder_continues_from_max_position` — append after a reorder computes `MAX+10` off the new positions - [ ] `append_to_empty_journey_starts_at_10` — `MAX` returns null on empty table; COALESCE/null-handling pins position 10 (off-by-one guard) - [ ] `PATCH` with `note: null` clears the note field (accepted when document is present) - [ ] `PATCH` with `note: null` returns 400 when item has no document (would result in item with neither document nor note) - [ ] `PATCH` with the same note value (no-op) returns 200 idempotently - [ ] `JourneyItemUpdateDTO.note` three-way semantics tested against the real deserializer (MockMvc, real Jackson): absent→unchanged, null→cleared, "text"→set; assert via `argThat`/follow-up GET - [ ] `GET /api/geschichten/{id}` without `items` field when type is `GESCHICHTE` — test asserts `jsonPath("$.items").doesNotExist()` - [ ] GET a JOURNEY with items does not throw `LazyInitializationException` (integration test, not slice) - [ ] `getById_returns_items_for_draft_journey_when_user_has_BLOG_WRITE` — DRAFT JOURNEY read path returns items (curator's actual editor path) - [ ] `getById_throws_NOT_FOUND_for_draft_journey_when_user_lacks_BLOG_WRITE` — the existing DRAFT-hiding guard still fires for JOURNEY; the `items` load happens only after the guard passes - [ ] `JOURNEY_ITEM_NOT_FOUND` and `JOURNEY_ITEM_POSITION_CONFLICT` ErrorCodes added in backend, frontend errors.ts, and all three i18n files - [ ] Flyway migration includes `ON DELETE CASCADE` (geschichte FK), `ON DELETE SET NULL` (document FK), `CHECK (position > 0)`, and named `DEFERRABLE INITIALLY DEFERRED` `UNIQUE(geschichte_id, position)` - [ ] Flyway migration SQL includes a comment on the `ORDER BY document_id` data migration ordering AND the single-transaction/no-CONCURRENTLY comment - [ ] Migration statement order is CREATE → INSERT → parity-guard `DO $$` → DROP, in that textual order (guard between INSERT and DROP) - [ ] Migration includes a row-count parity guard that aborts before any DROP if `journey_items` count != `geschichten_documents` count - [ ] Integration test (service-level): delete a Document via `documentService.delete()`, verify JourneyItem still exists with `document = null` - [ ] Integration test (DB-level, raw SQL): `DELETE FROM documents WHERE id=?` via jdbcTemplate → re-fetch JourneyItem → `document_id` is null (proves the FK, isolated from JPA cascade) - [ ] Data-migration integration test: seed `geschichten_documents` with N rows across M geschichten via raw SQL/`@Sql` before migration → run migrations → assert `journey_items` has exactly N rows; per-geschichte assert the `position` **set** equals `{10,20,…,10*n}` and the row count — NO document→position coupling (UUID order is arbitrary; flake-proof) - [ ] Migration parity-guard test: a deliberately-seeded count mismatch leaves `geschichten_documents` **intact** (not dropped) — proves the `RAISE EXCEPTION` rolls back the DROP in the same Flyway transaction - [ ] `position_check_rejects_nonpositive` — raw insert via jdbcTemplate expecting `DataIntegrityViolationException` (postgres:16-alpine only) - [ ] `unique_constraint_rejects_duplicate_position_per_geschichte` — raw insert of duplicate (geschichte_id, position) expecting violation (postgres only) - [ ] Position-race contract: duplicate-position insert through `JourneyItemService.append` surfaces as **409 `JOURNEY_ITEM_POSITION_CONFLICT`** (not 400 `VALIDATION_ERROR`); the remap matches the **pinned constraint name** `uq_journey_items_geschichte_position` (a renamed-constraint regression goes red); the 409-remap test is a service/slice test with real `@Transactional`, NOT a member of the non-`@Transactional` `JourneyItemConstraintsTest` - [ ] Regression test: note containing HTML/`<script>` is stored and returned verbatim by GET (no sanitization, no escaping at the API layer); the journey-with-note GET asserts `content().contentType(MediaType.APPLICATION_JSON)` - [ ] `DocumentSummary` round-trips ALL seven `DatePrecision` values — specifically a source document with `metaDatePrecision = SEASON` and one with `APPROX` round-trip that exact value through GET (no defaulting to `UNKNOWN`) - [ ] Multi-receiver `DocumentSummary`: a document with ≥2 linked receivers returns `receiverName` = first canonical receiver and `receiverCount` = total count - [ ] `receiverCount` contract: non-null for any document-bearing item; `0` when no receiver is linked (then `receiverName == null`); `>=1` otherwise — test `summary_returns_receiverCount_0_and_null_name_when_no_receiver` - [ ] Name composition tested: linked Person → `firstName lastName` (trimmed); no Person → `senderText`/`receiverText`; neither → null - [ ] `findByIdAndGeschichteId` repository method tested: item ID matches but geschichte ID doesn't → empty; item ID doesn't exist → empty - [ ] IDOR test: `PATCH /api/geschichten/{journeyAId}/items/{itemFromJourneyBId}` authenticated as a valid BLOG_WRITE user → 404, named `patch_returns404_when_itemBelongsToADifferentJourney` - [ ] Controller tests cover 401 (unauthenticated) AND 403 (authenticated with READ_ALL but not BLOG_WRITE) for each write endpoint - [ ] `@MockitoBean JourneyItemService` added to `GeschichteControllerTest` - [ ] Existing `GeschichteServiceTest` / `GeschichteServiceIntegrationTest` / `GeschichteControllerTest` pass after the `update()` JOURNEY-guard is added (regression — fix any test calling `update()` with `documentIds` on a now-JOURNEY story) - [ ] `GeschichteService.update()` rejects `documentIds` on JOURNEY-type stories (prevents dual-write divergence); guard reads `g.getType()` from the entity — `type` is NOT added to `GeschichteUpdateDTO` (no type flips via PATCH) - [ ] Soft cap of 100 items per journey enforced on POST; the 101st returns 400 (`VALIDATION_ERROR`); the cap test seeds the first 100 items via `@Sql`/jdbcTemplate fixture, NOT 100 HTTP calls. Grandfathered journeys migrated over the cap are editable (incl. reorder) except for appending new items - [ ] Security-assumption regression test: a `READ_ALL`-only user (no `BLOG_WRITE`) GETs a published JOURNEY and receives the `items[].document` summaries — codifies the "uniform document reads" precondition; goes red the day document reads are scoped - [ ] Audit: add/remove/reorder each record an audit event via `auditService.logAfterCommit(<new AuditKind>, actorId, null, Map.of("geschichteId", …))` — `documentId` arg is null; test verifies the call fires with `documentId == null` - [ ] Three new `AuditKind` values added (`JOURNEY_ITEM_ADDED`, `JOURNEY_ITEM_REMOVED`, `JOURNEY_ITEMS_REORDERED`) with Javadoc payload docs; `JOURNEY_ITEMS_REORDERED` added to `ROLLUP_ELIGIBLE`; `AuditKind` enum addition as its own atomic commit before the service work - [ ] DB diagram files updated (`db-orm.puml`, `db-relationships.puml`) - [ ] `npm run generate:api` run and TypeScript types regenerated AND committed in this PR (verify via `git diff --stat` on `frontend/src/lib/api/...`) - [ ] `CLAUDE.md` updated to list `JourneyItemService` and `JourneyItemRepository` in the `geschichte/` package entry; `backend/CLAUDE.md` Testing section corrected from "88%" to `0.77` - [ ] `docs/GLOSSARY.md` entries added for "Lesereise / Reading Journey", "JourneyItem", "Interlude" (note-only item), and the three new `AuditKind` values - [ ] Editor design spec (`lesereisen-editor-spec.html`) reorder body corrected to `{itemIds:[...]}`; note added to frontend issue #753 - [ ] ADR added documenting the `journey_items` ordered join table, `DocumentSummary`-as-read-model, name-composition policy, multi-receiver canonical rule, `ON DELETE SET NULL` rationale, plain-text note, the two-write-path transition state, the EAGER-vs-LAZY collection rationale, the deferrable-unique-constraint choice, journey audit events being geschichte-scoped (`documentId` null), and the `receiverCount` cardinality-disclosure lossiness note - [ ] `docs/DEPLOYMENT.md` updated with the sequenced pre-V73 deploy runbook (pg_dump → verify → pull → up `--no-deps backend frontend` together), the expected brief blank-journey window during simultaneous recreate, and the explicit "no schema down-migration; rollback = restore-from-dump (data-loss window) or roll-forward V74" sentence; pre-merge restore-rehearsal performed against a real prod dump - [ ] Verify the migration/constraint test class naming matches the repo's executed-in-CI Testcontainers convention (`*IT` under Failsafe vs `*Test` under Surefire) so it actually runs in CI, not just locally - [ ] `getById` returns a `GeschichteView` wrapper for BOTH JOURNEY and GESCHICHTE types (uniform shape); `author` is summarized to `{id, displayName}` only — test `getById_response_does_not_contain_author_email_or_groups` asserts the response has no `author.email` / `author.groups` - [ ] Note length soft cap of 5000 chars enforced on POST and PATCH; the 5001st char returns 400 `VALIDATION_ERROR` — test `post_returns400_when_note_exceeds_5000_chars` and `patch_returns400_when_note_exceeds_5000_chars` - [ ] `DocumentSummary` is built via a lean `documentService.getSummaryById(UUID)` that skips `tagService.resolveEffectiveColors` — no wasted tag-color resolution per item - [ ] `unique_constraint_is_deferrable_initially_deferred` — raw SQL against `pg_constraint` (`condeferrable` AND `condeferred` both true) asserting the flags directly (flush-order-proof guard for the deferrable property) - [ ] Positive data-migration AC: a clean migration run on N seeded rows SUCCEEDS, drops `geschichten_documents`, AND leaves `journey_items` count == N (pairs with the negative parity-guard test to catch an early-placed guard that would abort a legitimate deploy) - [ ] Cap measurement: the 100-item cap is `COUNT(*)`-based, never `MAX(position)`-derived — test: a journey with 99 rows but `MAX(position)=2000` still accepts the 100th append - [ ] `delete_returns404_when_itemAlreadyDeleted` also asserts NO `JOURNEY_ITEM_REMOVED` audit event fires on the 404 path - [ ] Whitespace-note test input includes embedded newlines (`"\n \n"`), not only spaces — trims to `null` - [ ] The 130-item grandfathered reorder test reuses the shared static `PostgreSQLContainer` (not a standalone `@SpringBootTest`) ## Review Insights ### Decided (round 6): GET response wrapper + `author` leak closed **Decision (Markus, Nora — Decision Queue Option A): `getById` returns a single named `GeschichteView` response record for BOTH JOURNEY and GESCHICHTE types.** Introduce `GeschichteView` in the `geschichte/` package, assembled by `GeschichteService.getById`. It carries the existing entity fields PLUS the conditional `items` (`@JsonInclude(NON_NULL)`, present only for JOURNEY), and the `documents`/`persons` for both types. **`author` is summarized to `{UUID id, String displayName}` — the raw `AppUser` graph (email, group memberships) is never serialized.** `GET /api/geschichten` (list) keeps returning entities; only the detail endpoint changes. Rationale: Option B (discriminated union — raw entity for GESCHICHTE, wrapper for JOURNEY) leaves two divergent serialization shapes for one endpoint (a TS union the frontend must narrow) and lets the pre-existing `author` AppUser info-leak persist on the GESCHICHTE path. Option A costs ~30 lines of mapping and retypes every detail-page consumer once, but closes a real IDOR-adjacent info leak and gives a single TS type. Cost is paid once; divergence would be permanent. **Breaking-change consequence (Markus):** the GET response object changes from `Geschichte` to `GeschichteView` for ALL detail consumers, not just the JOURNEY path. After `npm run generate:api`, grep the frontend for `.documents` / `.author` accesses on the detail response (not only `.items`) and retype them. Add a regression AC: `GET` JOURNEY response contains no `author.email` / `author.groups`. **Do NOT bolt a `@Transient List<JourneyItemView> items` onto the entity** — that reintroduces the serialization-leak / LazyInit risks round 5 eliminated. The wrapper is assembled by the service from the repository query. ### Decided (round 6): Server-side note length cap (5000 chars) **Decision (Leonie — Decision Queue Option A): enforce a soft cap of 5000 chars on `note` at POST and PATCH** (after trim), returning 400 `VALIDATION_ERROR` on overflow. Symmetric with the 100-item cap; bounds the reader's worst-case 320px layout once at the API instead of defending hostile/accidental walls of text in three render surfaces (reader, editor, editor-preview). One added validation branch + two tests. `#753` still applies `max-w-prose` for in-bounds long notes. ### Decided (round 6): Lean read path for `DocumentSummary` **Decision (Felix — Decision Queue Option A): add `documentService.getSummaryById(UUID)`** that returns the fields `DocumentSummary` needs WITHOUT running `tagService.resolveEffectiveColors` (DocumentService:1004). `getDocumentById` does a tag-color resolution pass the summary never reads — ×100 items per journey GET is wasted work that would surface as a Grafana trace anomaly. The new method keeps the domain boundary clean (DocumentService owns it) and keeps `JourneyItemService.toSummary` honest. `JourneyItemService.toSummary` calls `getSummaryById`, not `getDocumentById`. ### Decided (round 6): Test-strategy reinforcements - **`JsonNullable` + the test's local `new ObjectMapper()` (GeschichteControllerTest:47) are incompatible.** The hand-rolled mapper lacks `JsonNullableModule`. The PATCH three-way test MUST post a **raw JSON string** through MockMvc (which uses the Spring-context, module-registered mapper) — it must NOT pre-serialize the body with the test class's local `new ObjectMapper()`. Pinned for both the controller test and the QA strategy. - **Deferrable-constraint canary needs a flush-order-proof companion.** `reorder_swap_two_adjacent_items_succeeds` can go green even under an *immediate* constraint if flush order happens not to collide. Add `unique_constraint_is_deferrable_initially_deferred` — a raw SQL assertion against `pg_constraint` (`condeferrable` AND `condeferred` both true). This is the only guard the flush order cannot fake. - **Data-migration parity guard needs BOTH directions.** The negative test ("deliberate mismatch leaves `geschichten_documents` intact") also passes if the guard is mis-placed *before* the INSERT (count 0 ≠ N → RAISE → table survives) — so it can't distinguish correct from broken-too-early. Add the positive companion: clean run on N seeded rows SUCCEEDS, drops `geschichten_documents`, and `journey_items` count == N. The early-guard placement fails the positive test. - **Cap is `COUNT(*)`, never `MAX(position)`-derived.** Deletions leave permanent position gaps (delete #50 in a 100-item journey → ceiling 1010 at 99 rows); a `MAX(position)/10` cap check would falsely reject appends below 100 items. Test: 99 rows with `MAX(position)=2000` still accepts the 100th append. - **Concurrent-edit recovery is ONE flow, not two.** #753 must treat BOTH `VALIDATION_ERROR` (reorder set-mismatch) AND `JOURNEY_ITEM_POSITION_CONFLICT` (409) as the same human cause — "this journey was changed elsewhere — reload" — a single recovery toast, not two divergent flows. - **`u. a.` / `receiverCount` is localized prose, not a hardcoded German abbreviation** — #753 maps `receiverCount` through Paraglide (de/en/es), never concatenates `" u. a."` literally. ### Decided (round 6): Operational notes - **Deferrable constraint requires transaction-level (or session-level) pooling.** Prod runs PgBouncer in transaction-pooling mode (correct today). A future switch to statement-level pooling would silently break deferred constraint checking at COMMIT, turning valid reorders into commit-time failures. One sentence in the ADR's deferrable-constraint section. - **Audit rows for operations whose `afterCommit` straddles the container recreate may be lost** (content is safe — it committed; the audit trail is best-effort, `writeLog` catches and logs). One sentence in `docs/DEPLOYMENT.md`. - **Flyway must run the migration in a single transaction** for the parity-guard rollback to hold. PR checklist: confirm no `executeInTransaction=false` / no callback splits the migration; paste the Flyway log line showing it committed atomically. ### Decided: Service extraction **Decision: Extract `JourneyItemService`.** `GeschichteService` is already ~200 lines; adding item CRUD would push it past 300. Pattern: `AnnotationService` for the annotation sub-domain under `document/`. `GeschichteController` calls `journeyItemService` directly — `GeschichteService` is not the intermediary (Spring Framework 7 prohibits constructor-injection cycles). The append method is named `append` (not `create`) to match AC vocabulary. ### Decided (CORRECTED round 4): `DocumentSummary` final shape **Final shape: `record DocumentSummary(UUID id, String title, LocalDate documentDate, LocalDate documentDateEnd, DatePrecision datePrecision, String senderName, String receiverName, Integer receiverCount) {}`** — a Java record (pure value object, no JPA lifecycle), owned in the `geschichte/` package (it is the geschichte domain's read-model view of a document). - **`DatePrecision` CORRECTION (bug fix).** `DatePrecision.java` has **seven** values: **`DAY, MONTH, SEASON, YEAR, RANGE, APPROX, UNKNOWN`** — there is no "separator" value. `SEASON` ("Frühjahr 1922") and `APPROX` ("um 1938") are real in a 1899–1950 letter archive (verbatim mirror of the import normalizer, ADR-025). The summary copies the enum value through unchanged; an AC pins that `SEASON`/`APPROX` round-trip without defaulting to `UNKNOWN`. The reader spec (#753) must format all seven, not five. - **`fileKey` DROPPED.** The field does not exist on `Document` (entity has `filePath`, `thumbnailKey`, `fileHash`). The frontend fetches scans via the authenticated proxy `GET /api/documents/{id}/file` using `document.id`. Embedding a storage key would leak the internal S3 object layout. The reader needs nothing beyond `id`. - **`documentDateEnd` + `datePrecision` ADDED.** `datePrecision` maps from the **entity field `metaDatePrecision`** (column `meta_date_precision`) — `doc.getDatePrecision()` does not compile. **Contract: `documentDate` is always the range start (`metaDate`), `documentDateEnd` the end (`metaDateEnd`) — never swapped.** - **`senderName` + `receiverName` ADDED as two separate fields.** Two fields give the 320px reader independent reflow control. Nullable; fall back to raw attribution (`senderText`/`receiverText`) when no Person is linked. - **`receiverCount` (round-4 multi-receiver decision, round-5 contract tightening).** `Document.receivers` is `@ManyToMany`. **`receiverName` = first canonical receiver; `receiverCount` = total linked receivers.** Round-5 (Leonie): `receiverCount` is **non-null for any document-bearing item — `0` when no receiver is linked (then `receiverName == null`), `>=1` otherwise.** This removes the "nullable Integer that's sometimes absent" ambiguity: the reader's logic is unambiguous (`receiverCount > 1` → " u. a."; `receiverName == null` → omit the "an Y" clause). The lossiness (only the first canonical name carried) and the new cardinality disclosure (`receiverCount` tells readers "this letter had N recipients", a new disclosure vs. the old `documents` array — intended for "u. a.") are recorded in the ADR. - **Name-composition policy (round-4).** `Person` exposes only `title, firstName, lastName, nameAliases`. **`senderName`/`receiverName` = linked Person's `firstName + lastName` trimmed; else the raw `senderText`/`receiverText`; else null.** "First canonical receiver" = first receiver by deterministic sort (lastName, then firstName) when ≥1 Person is linked. **`JourneyItemService` is the single owner** — a private `toSummary(Document)` helper (≤20 lines), unit-testable with a mocked `DocumentService`. If a name-display helper exists in the person domain, call it via `PersonService`; do NOT duplicate `firstName + lastName`. Do NOT put `@Transient getSummary()` on the entity. - **Built by the service, never serialized from JPA.** `JourneyItem.document` is a JPA `@ManyToOne(fetch = FetchType.LAZY) @JsonIgnore` (for the FK + `ON DELETE SET NULL`); the summary is assembled via `documentService.getDocumentById(item.getDocument().getId())` (id only — the LAZY proxy is never field-initialized). See round-5 N+1 note below. ### Decided (round 5): Read-model shape — `JourneyItemView` response record **Decision: introduce a `JourneyItemView` response record `{ UUID id, int position, DocumentSummary document, String note }` in the `geschichte/` package**, assembled by `JourneyItemService`. The GET `items` field and the POST/reorder responses all return `JourneyItemView` — **never the JPA `JourneyItem` entity** (which has a `geschichte` back-reference that would leak / hit `LazyInitializationException`, and a `@JsonIgnore`d `document`). **Decision (Felix's Option B — chosen for explicitness): do NOT serialize a mapped `@OneToMany` collection.** `JourneyItemService` queries items via `JourneyItemRepository.findByGeschichteIdOrderByPosition(id)` and assembles the `List<JourneyItemView>`. Rationale: matches the "`DocumentSummary` built by service" decision, gives a fully explicit read model with zero serialization-leak risk, and avoids the LazyInit trap of a mapped collection touched outside the read transaction. Ordering lives in the repository query (`OrderByPosition`); cascade/`ON DELETE` lives in the migration FKs. If a mapped `@OneToMany` is added later for any reason it MUST be `@JsonIgnore`d — but the read model never depends on it. **N+1 guard (Felix):** `JourneyItem.document` defaults to EAGER for `@ManyToOne`; make it explicit `@ManyToOne(fetch = FetchType.LAZY) @JsonIgnore`. The service builds the summary from `getDocument().getId()` only, then `documentService.getDocumentById(id)` — no association traversal that would fire one SELECT per item. ### Decided: Note rendering and HTML policy **Decision: Plain text, stored verbatim, escaped only at render.** No HTML rendering, no `BODY_SANITIZER`, no TipTap. `@Column(columnDefinition = "TEXT")`. Stored exactly as received (including `<script>` bytes), returned verbatim by GET — escaping is the frontend's job via `{note}` (never `{@html}`). Rejecting on `<` would break "5 < 10"; stripping silently mutates curator input. Backend regression test pins storage AND asserts `Content-Type: application/json` (round-5 Nora — prevents a mis-set `text/html` turning verbatim storage into reflected stored XSS). **The XSS burden moves entirely to the frontend `{note}` binding** — a frontend test in #753 must assert a note containing `<img src=x onerror=alert(1)>` renders as visible literal text and never executes. **Round-5 (Nora): the same verbatim-text posture applies to `senderName`/`receiverName`** — they fall back to unsanitized historical-import free text (`senderText`/`receiverText`), an attacker-influenced-via-import surface the curator never typed. The #753 frontend-XSS literal-text test must cover `senderName`/`receiverName`, not just `note`. Noted on #753. **Whitespace handling:** notes are trimmed; blank-after-trim is stored as `null` (POST and PATCH). Consequence: an item with `note == null && document == null` cannot exist — the reader needs zero defensive empty-state handling for interludes. ### Decided: GET response for non-JOURNEY type, and DRAFT JOURNEY read path **Decision: `items` absent for non-JOURNEY; `documents` retained for GESCHICHTE.** Discriminated union by `type` via `@JsonInclude(NON_NULL)` on the response record's `items` field. Items are queried LAZILY by the service (see Read-model shape) — only `getById` loads them, not `GET /geschichten`. Consequently **`getById` must be `@Transactional(readOnly=true)`** to assemble the read model inside one transaction. **Round-5 (Markus): `@Transactional(readOnly=true)` must WRAP the existing DRAFT-visibility guard, not bypass it.** `GeschichteService.getById` (lines 63–73) throws `GESCHICHTE_NOT_FOUND` for DRAFT stories when the caller lacks `BLOG_WRITE`. The transaction spans the whole method; the `items` load happens **after** the guard passes (don't load the collection for a request about to 404). Curators build journeys in DRAFT, so two realistic editor-path tests are required: `getById_returns_items_for_draft_journey_when_user_has_BLOG_WRITE` and `getById_throws_NOT_FOUND_for_draft_journey_when_user_lacks_BLOG_WRITE`. **ADR consequence (Markus): record WHY `persons`/`documents` are EAGER (list-endpoint N+1 avoidance) and `items` is deliberately LAZY** — so a future maintainer doesn't "fix the inconsistency" by making `items` EAGER and reintroduce the load-everything regression. ### Decided: Two-write-path guard **Decision: `GeschichteService.update()` rejects `documentIds` when `type == JOURNEY`.** During the transition, JOURNEY documents live in `journey_items` while the entity retains `@ManyToMany documents` (used by STORY type). `personIds` stay shared (no guard). The guard reads `g.getType()` from the **entity** — `type` is NOT a `GeschichteUpdateDTO` field (round-5 Felix: prevents flipping a story's type via PATCH). Existing `GeschichteService*` tests calling `update()` with `documentIds` on a story the migration retypes to JOURNEY will fail — fix as a regression AC. Recorded in the ADR. ### Decided: PATCH mutable fields; attach-document non-goal **Decision: `note` only via `JsonNullable<String>`.** A plain `Optional<String>` cannot distinguish absent from explicit null under Spring's `Jdk8Module`. Add `openapi-jackson-nullable` and register `JsonNullableModule` (its own atomic commit before the PATCH DTO work). `isPresent()` distinguishes absent from present; `.get()==null` distinguishes explicit null. Three-way test against the real deserializer (MockMvc). **Round-5 (Elicit) — DECIDED: attaching a document to an existing note-only interlude is a NON-GOAL for this issue.** PATCH stays note-only; `documentId` is immutable. A curator who wants to turn an interlude into a document item deletes and re-adds. Rationale: keeps PATCH and its test matrix small; the friction (lost note/position) is rare and acceptable for the first cut. Stated as an explicit non-goal in the PATCH contract. ### Decided: PUT /reorder response, size cap, grandfathered journeys **Decision: Return the updated items array** (`List<JourneyItemView>`), enabling optimistic drag-reorder reconciliation without a follow-up GET. Tests assert the body, not just status. **Cap `itemIds.size()` before the set-equality check** (cheap memory-DoS block). **Round-5 (Elicit) — DECIDED: the reorder size cap is `max(MAX_ITEMS, currentItemCount)`, not a flat 100.** A grandfathered 130-item journey (migrated over the cap) must stay fully reorderable — a flat-100 cap would reject the legitimate 130-id full-set reorder and contradict the round-4 "grandfathered journeys remain editable" decision. Append still rejects past 100 (a 130-item journey cannot grow). AC `reorder_of_grandfathered_over_cap_journey_succeeds`. ### Decided (round 5): Reorder position writes — deferrable unique constraint **Round-5 (Sara) — DECIDED: make `uq_journey_items_geschichte_position` `DEFERRABLE INITIALLY DEFERRED` (Sara's Option A).** With an **immediate** unique constraint, a valid swap (item-B → 10 while item-A still holds 10 until flush) transiently collides mid-flush and throws `DataIntegrityViolationException` on a perfectly valid reorder — non-deterministic, flush-order dependent. A deferrable constraint checks once at commit: the whole 10/20/30 reassignment commits atomically, swaps never transiently collide, and a genuine duplicate (concurrent append racing the reorder) still fails at commit and gets the 409 remap. This is the textbook Postgres pattern for reassigning an ordered column. Rejected: Option B (two-phase +1000 offset — ugly, 2× UPDATEs, still racy) and Option C (immediate + frontend retry — valid reorders intermittently 409, bad curator UX). New-pattern cost (no repo precedent, slight planner cost) is negligible at family-archive scale. AC `reorder_swap_two_adjacent_items_succeeds` goes red with an immediate constraint and green with the deferrable one. ### Decided: Position uniqueness AND race HTTP contract **Decision: `DEFERRABLE INITIALLY DEFERRED UNIQUE(geschichte_id, position)` (named) plus `CHECK (position > 0)`.** `SELECT MAX(position)+10` inside `@Transactional` is not fully race-free; the unique constraint catches the tie. **Decision: the race must surface as 409, not 400 — on BOTH write paths.** `GlobalExceptionHandler.handleDataIntegrityViolation` maps every integrity violation to 400 `VALIDATION_ERROR`, which blames the curator for a server-side collision. **Catch `DataIntegrityViolationException`, inspect the constraint name, rethrow as `DomainException.conflict(ErrorCode.JOURNEY_ITEM_POSITION_CONFLICT, ...)` → HTTP 409**, message "Another change reordered this journey — please retry." **Round-5 (Sara): the 409 remap covers `append` AND `reorder`** — both write `position` and hit the same constraint. Extract the catch-and-remap into a **private `JourneyItemService` helper both methods call** — one place to test and maintain. AC `reorder_conflict_surfaces_as_409`. **Round-5 (Nora): pin the constraint name as a shared constant.** Name the constraint explicitly in the migration (`CONSTRAINT uq_journey_items_geschichte_position UNIQUE (...)`) and match the service's remap branch against a named constant (e.g. `JourneyItem.UQ_POSITION_CONSTRAINT = "uq_journey_items_geschichte_position"`) referenced by both. If Postgres auto-names the constraint (inline `UNIQUE(...)` without `CONSTRAINT name`) or the literal drifts, the `if (constraintName.equals(...))` branch silently fails open and the curator sees the misleading 400 — a silent regression no happy-path test catches. AC: the 409-remap test asserts the **named** path; a renamed-constraint regression goes red. **Test isolation (Sara):** the 409-remap test is a **service/slice test with real `@Transactional`** and must NOT be a member of the non-`@Transactional` `JourneyItemConstraintsTest` (raw-insert jdbcTemplate tests) — a `DataIntegrityViolationException` inside a class-level `@Transactional` marks the tx rollback-only and can cascade into `TransactionSystemException` on teardown. ### Decided: PATCH note:null on document-less item **Decision: Return 400.** Consistent with the POST invariant. Auto-delete would be silent data loss; a tombstone forces reader-side null handling. ### Decided: DELETE idempotency **Round-5 (Elicit) — DECIDED: `DELETE /items/{itemId}` on an already-deleted item returns 404 `JOURNEY_ITEM_NOT_FOUND`** (not 204). Consistent with the IDOR contract (`findByIdAndGeschichteId` returns empty → notFound). For a drag-heavy editor with optimistic local state, the frontend treats "already gone" as success client-side and suppresses the toast. AC `delete_returns404_when_itemAlreadyDeleted`. ### Decided: ErrorCodes **Decision: Add `JOURNEY_ITEM_NOT_FOUND`** (do not reuse `GESCHICHTE_NOT_FOUND`) and **`JOURNEY_ITEM_POSITION_CONFLICT`** (409). Each in all four places: `ErrorCode.java`, `errors.ts`, all three `messages/*.json`. Keys: `error_journey_item_not_found`, `error_journey_item_position_conflict`. The reorder set-mismatch 400 reuses `VALIDATION_ERROR` but with a distinguishable message so #753 can show "this journey was changed elsewhere — reload" rather than a generic validation error (round-5 Elicit; concurrent-edit recovery path). ### Decided: POST contracts — non-JOURNEY, bad documentId, max-items cap **Decision: POST to a non-JOURNEY Geschichte returns 400** (`VALIDATION_ERROR`). Verify the parent exists AND `type == JOURNEY` before creating. **POST with a non-existent `documentId` returns 404 `DOCUMENT_NOT_FOUND`** — falls out of `documentService.getDocumentById()` (precedent: `GeschichteService.resolveDocuments` lines 179–186 already loops it), semantically honest, pinned by AC so the frontend handles 404 not 400. **Soft cap of 100 items per journey**, enforced on POST (the 101st returns 400 `VALIDATION_ERROR`). Pre-existing journeys migrated over the cap are grandfathered — editable (incl. reorder, via the `max(100, currentCount)` reorder cap) except adding items. Cap test seeds 100 items via fixture, not 100 HTTP calls. ### Decided (round 5): Audit — wiring against the real AuditService **Round-5 (Markus): the audit decision is unbuildable as previously written and is now corrected.** - **`AuditService` signatures** are `log(AuditKind, UUID actorId, UUID documentId, Map payload)` and `logAfterCommit(...)` — the only entity dimension is `documentId` (`AuditLog.java` line 40, nullable). A reorder is not about a document; an interlude add has no document. - **DECIDED: add three new `AuditKind` values** — `JOURNEY_ITEM_ADDED`, `JOURNEY_ITEM_REMOVED`, `JOURNEY_ITEMS_REORDERED` — with Javadoc payload docs matching house style, as **their own atomic commit before the service work** (same discipline as the jackson-nullable commit). The audit AC becomes: `logAfterCommit(<new kind>, actorId, null, Map.of("geschichteId", id, …))` — **`documentId` arg is null; `geschichteId` lives in the jsonb payload.** Jamming `geschichteId` into the `documentId` slot would corrupt every document-scoped audit query (`AuditLogQueryService` filters by `documentId`). - **DECIDED (Markus's Option A): add `JOURNEY_ITEMS_REORDERED` to `ROLLUP_ELIGIBLE`; leave add/remove out.** Drag-reordering fires many reorders per editing session (a 30-drop session → 30 rows); rollup keeps the chronik readable. Per-reorder forensic granularity is not worth flooding the family chronik for a curation tool. Add/remove are discrete, low-frequency, individually meaningful — not rolled up. - **ADR addition: journey audit events are geschistee-scoped, not document-scoped (`documentId` deliberately null)** — so a future maintainer doesn't "fix" the null by back-filling a document id. New `AuditKind` values are a domain enum addition → `docs/GLOSSARY.md` + the audit ADR. ### Decided: JOURNEY intro `body` format **Decision: Out of scope here, recorded.** When the editor issue lands, JOURNEY intro should be plaintext (consistent with "no TipTap for journeys"). This issue does not change `body`. ### Decided: Migration drop strategy **Decision: Single PR, DROP gated by an in-SQL row-count parity guard.** Flyway is forward-only and runs automatically on prod boot on a single VPS with no staging. One PR for schema cleanliness; the drop is self-protecting — `DO $$ BEGIN IF (SELECT count(*) FROM journey_items) <> (SELECT count(*) FROM geschichten_documents) THEN RAISE EXCEPTION ...; END IF; END $$;` aborts **before** the DROP if counts disagree. **Single-transaction is load-bearing:** Postgres transactional DDL + Flyway's per-migration transaction means `RAISE EXCEPTION` rolls back the DROP. A SQL comment forbids `CONCURRENTLY`/non-transactional; a test seeds a deliberate mismatch and asserts `geschichten_documents` survives. **Round-5 (Tobias): pin the intra-file statement order** — (1) CREATE TABLE → (2) INSERT … SELECT FROM geschichten_documents → (3) `DO $$` parity guard → (4) DROP TABLE. The guard sits **textually between INSERT and DROP**; reordering the guard above the INSERT (guards "feel like" preconditions) compares 0 vs N and aborts every deploy or passes vacuously. AC pins the order. ### Decided: Document-read access model (security side channel) **Decision: Document reads are uniform across authenticated users** — `SecurityConfig` line 77/120 is `anyRequest().authenticated()`; `DocumentController` `GET /{id}` (line 136) and `GET /{id}/file` (line 86) carry no `@RequirePermission`. Embedding `DocumentSummary` in a published journey discloses nothing a reader couldn't already fetch. **But this rests on an implicit default, not an explicit policy** — a security-assumption regression test (a `READ_ALL`-only user GETs a published JOURNEY and sees `items[].document`) codifies it and goes red the day document reads are scoped, forcing the drafted redaction approach ("[Dokument nicht verfügbar]" per-item, not whole-journey 403) to be implemented rather than forgotten. ### Resolved contradiction: reorder body contract This issue's `{ "itemIds": [...] }` is authoritative. The editor design spec (`lesereisen-editor-spec.html` lines 523/762/783) currently says `[{id, position}]` — correct it to `{itemIds:[...]}` as part of this PR, with a note on frontend issue #753. ### Corrected: coverage gate number The enforced JaCoCo gate is `<minimum>0.77</minimum>` (`backend/pom.xml` line 349, ratcheted per #496) — **not 88%**. Round-5 (Markus): `backend/CLAUDE.md` Testing section still says "88% branch coverage" — fix it to `0.77` in this PR (the number is load-bearing for "do not panic-pad tests"). The ~14–16-branch estimate still holds and every branch needs an explicit test. ### Test strategy (layer assignment + container sharing) - **Tag each test AC by pyramid layer:** unit (Mockito) — DTO three-way semantics, position-MAX+10 arithmetic, name-composition `toSummary`; `@WebMvcTest` slice — 401/403 per endpoint, `jsonPath($.items).doesNotExist()`, `@MockitoBean` wiring; Testcontainers integration — every FK/constraint/cascade/data-migration AC plus the `LazyInitializationException`-free GET and the DRAFT-JOURNEY read tests. - **Consolidate FK/constraint/cascade/data-migration tests into ONE `JourneyItemConstraintsTest`** sharing a single `static PostgreSQLContainer` (~6 methods) — saves ~60–80s CI per run. **The 409-remap service test is NOT a member of this class** (it needs real `@Transactional`). - **Raw-insert constraint tests must be non-`@Transactional`** (jdbcTemplate + explicit `@Sql` teardown) — a `DataIntegrityViolationException` inside a class-level `@Transactional` test marks the tx rollback-only and cascades into `TransactionSystemException` on teardown. - **Data-migration assertion style:** assert row count == seeded count and per-geschichte `position` *set* == `{10,20,…,10*n}`; never couple a specific document to a specific position (UUID order is arbitrary). - **Round-5 (Tobias): verify the integration test class naming matches the repo's executed-in-CI Testcontainers convention** (`*IT` under Failsafe vs `*Test` under Surefire) — otherwise the highest-risk test passes locally and silently never runs in CI. PR checklist: paste the CI log line showing the migration test ran and that the seed `@Sql` ran *before* Flyway. ### Confirmed risks - **Reorder self-collision under an immediate unique constraint** — RESOLVED via `DEFERRABLE INITIALLY DEFERRED`. A swap is checked at commit, not mid-flush. `reorder_swap_two_adjacent_items_succeeds` is the canary. - **409 remap matching an unpinned constraint name fails open** — pin the name as a shared constant in migration + service; AC asserts the named path. - **Audit wiring against the real `AuditService`** — needs three new `AuditKind` values and `documentId == null` with `geschichteId` in the payload; jamming geschichteId into `documentId` corrupts document-scoped audit queries. - **DRAFT JOURNEY read path** — `@Transactional(readOnly=true)` must wrap the existing DRAFT-hiding guard; items load only after the guard passes. Two editor-path tests required. - **N+1 via EAGER `@ManyToOne document`** — make `JourneyItem.document` explicitly LAZY; build the summary from the id only. - **`author` AppUser graph leaked on the detail GET** — closed by summarizing `author` to `{id, displayName}` in `GeschichteView` (round 6); a regression AC asserts no `author.email`/`author.groups` ships. - **Serializing the JPA entity instead of `JourneyItemView`** — would leak the `geschichte` back-ref / hit LazyInit; return the assembled view record everywhere. - **Grandfathered-over-cap journey un-reorderable under a flat-100 cap** — reorder cap is `max(100, currentCount)`. - **Irreversible migration on auto-boot**: Flyway runs on prod boot, no staging, single VPS, `docker compose up -d --build` is one command. Mitigations: in-SQL single-transaction parity guard with pinned statement order; data-migration integration test; sequenced `docs/DEPLOYMENT.md` runbook (`pg_dump` → verify → `pull` → `up -d --no-deps backend frontend` together, with the brief blank-journey window noted and the `pg_restore` command recorded); the explicit "no schema down-migration; rollback = restore-from-dump (data-loss window) or roll-forward V74" sentence; pre-merge restore-rehearsal against a real prod dump (covers NULL `senderText`, multi-receiver docs, legacy collation synthetic data misses). - **Data-migration coverage is the single highest-risk item** — seed-before / assert-after Testcontainers test is a blocker AC; seed via raw SQL / `@Sql` against `geschichten_documents` before Flyway runs. - **CI must actually execute the migration test against `postgres:16-alpine`** — verify class naming matches the CI-executed convention; verify the seed `@Sql` ran *before* Flyway (not vacuously after a drop). - **`note`/`senderName`/`receiverName` verbatim → frontend XSS half** — the verbatim decision is only safe because the reader renders text. #753 frontend tests (`<img onerror>` renders as literal text, no `alert`) must cover note AND the two name fields. Backend GET asserts `Content-Type: application/json`. - **`SEASON`/`APPROX` mis-render** — a five-case date formatter in #753 would silently mis-render the two omitted precisions for exactly this archive's audience. #753 hand-off: "Frühjahr/Sommer/Herbst/Winter YYYY" (SEASON) and "um YYYY" (APPROX) are required acceptance, not optional; falling back to raw `YYYY-MM-DD` loses the historical hedging the archive exists to preserve. - **`senderName == null && receiverName == null`** is real (letter with no linked Person and no `senderText`/`receiverText`). #753 hand-off: render the document title only, no attribution line — never "von an ". - **`receiverCount` cardinality disclosure** — a new (intended) disclosure vs. the old `documents` array; record in the ADR lossiness note. - **`white-space: pre-line` (not `pre`/`pre-wrap`)** — `pre` would prevent wrapping on 320px (horizontal scroll, Critical mobile failure) and preserve accidental trailing spaces. Pinned; don't let it drift in #753. - **Concurrent curators on one journey** — A reorders while B deletes → A's `itemIds` set includes B's gone item → set-equality 400. Correct behavior; #753 needs an honest "this journey was changed elsewhere — reload" recovery message (distinguishable from a malformed request). - **IDOR on item mutations** — `findByIdAndGeschichteId` mandatory for PATCH/DELETE; `PUT /reorder` IDOR closed by set-equality. Both IDOR tests authenticate as a valid BLOG_WRITE user. - **Reorder set-equality vs. count-only** — a count check lets an attacker mix IDs from different journeys; compare `Set<UUID>` equality. - **`note` three-way semantics fails with plain `Optional<String>`** under default Jackson — `JsonNullable` required, test hits the real deserializer. - **`GeschichteControllerTest` silent breakage** — add `@MockitoBean JourneyItemService` before any new tests. - **Breaking change deployment** — `documents` → `items` in the JOURNEY GET response; Docker Compose restarts frontend + backend together (no rolling-restart skew). Regenerated `*.d.ts` committed in this PR so CI builds against the new contract. - **Predecessor not yet merged** — `GeschichteType` enum, `JourneyItem` entity, and migrations beyond V71 do not yet exist in `main`. The branch cannot be cut and TDD red cannot start until the predecessor PR merges and CI is green; the Flyway version (V73+) is assigned at branch-cut time. - **No new infrastructure** — pure schema + REST. No new env vars, no new Docker service, no Compose changes.
marcel added this to the Lesereisen (Reading Journeys) milestone 2026-06-06 16:07:11 +02:00
marcel added the P1-highfeature labels 2026-06-06 16:08:06 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
P0-critical
Blocks a core user journey, causes data loss, or is a security/accessibility barrier. Work on this before P1+.
 P1-high
Significant friction on a main user journey; workaround exists but is non-obvious. Next up after P0.
 P2-medium
Noticeable improvement; doesn't block anything; schedule opportunistically.
 P3-later
Cosmetic or parking-lot work; fine to stay open indefinitely.
 audit
Read-only audit / assessment work; produces a report rather than changing code
 bug
Something isn't working
 cleanup
Removal of dead code, vague comments, naming violations, and scratch artifacts
 collaboration
We want to extend the family archive to have more collaboration tools
 conversation
descoped
We will do that later
 devops
documentation
README, ARCHITECTURE, GLOSSARY, CONTRIBUTING, per-domain docs
 epic
Marker for epic-level issues that group multiple child issues
 feature
file-upload
legibility
Codebase Legibility Refactor — preparing the codebase for human evaluation and bus-factor reduction
 needs-discussion
Has an open decision or design question that must be resolved before implementation can start.
 notification
person
phase-1: security
Security hygiene — must be done first
 phase-2: container-images
Production-ready multi-stage Docker images
 phase-3: prod-compose
Production compose overlay + Caddy reverse proxy
 phase-4: spring-prod-profile
Spring Boot production configuration profile
 phase-5: backups
Database and object storage backup strategy
 phase-6: deployment-docs
.env.example, DEPLOYMENT.md, runbook
 phase-7: monitoring
Prometheus, Loki, Grafana, Alertmanager
 refactor
Code restructuring without behaviour change
 security
test
ui
UI/UX and accessibility issues
 user
No Label P1-high feature
Milestone
No items
No Milestone Lesereisen (Reading Journeys)
Projects
Clear projects
No project
Assignees
Clear assignees
Jens marcel
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#751
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?