feat(lesereisen): data model + Flyway migration — GeschichteType, JourneyItem, migrate geschichten_documents #750

Open
opened 2026-06-06 16:06:57 +02:00 by marcel · 1 comment
Owner

Goal

Extend the Geschichte entity to support Reading Journeys by adding a type discriminator and replacing the unordered Set<Document> with an ordered JourneyItem sequence. This is the foundation all other Lesereisen issues build on.

Background

A Lesereise (Reading Journey) is a Geschichte with type = JOURNEY. Its primary content is an ordered sequence of letters with optional curator notes. Design specs: docs/specs/lesereisen-reader-spec.html and docs/specs/lesereisen-editor-spec.html.

Tasks

New enum: GeschichteType

public enum GeschichteType { STORY, JOURNEY }

Add to Geschichte as a non-null column type, default STORY.

New entity: JourneyItem

journey_items
  id            UUID   PK
  geschichte_id UUID   FK → geschichten ON DELETE CASCADE (NOT NULL)
  position      INT    NOT NULL
  document_id   UUID   FK → documents ON DELETE SET NULL (nullable)
  note          TEXT   (nullable, plain text — NOT HTML)
  CONSTRAINT chk_journey_item_not_empty CHECK (document_id IS NOT NULL OR note IS NOT NULL)
  • document_id = null → pure interlude note (context between letters, no document); note must be non-null (enforced by CHECK constraint)
  • Both set → letter with curator annotation
  • Only document_id → plain letter entry, no note
  • position is the sort key; gaps are fine. Duplicate positions within one journey are allowed with documented undefined-order semantics (see Review Insights — round 4)
  • note is plain text (not HTML) — render with {note} in Svelte, no sanitization required
  • document field: annotate with @JsonIgnore; expose documentId (UUID) only in the API response — see serialization decision below
  • Per-field @Schema(requiredMode = REQUIRED): id, position are always populated → REQUIRED. documentId and note are nullable → NOT required (omit requiredMode so the generated TS types them as optional). This drives correct TypeScript optionality for the follow-on frontend.

Place JourneyItem in geschichte/journeyitem/ sub-package, consistent with document/transcription/ and document/annotation/ patterns.

JPA mapping on Geschichte.items — LAZY with explicit init (decision refined in round 4, see Review Insights)

@OneToMany(mappedBy = "geschichte", cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.LAZY)
@OrderBy("position ASC")
@Builder.Default
// LAZY per ADR-022. open-in-view is FALSE (application.yaml:15), so this collection is DEAD at
// Jackson serialization time unless explicitly initialized inside the service transaction.
// getById() is @Transactional(readOnly=true) AND calls getItems().size() to force-init before return.
// list() must NOT serialize items at all — it returns a GeschichteSummary projection (see below).
// This is the first List ("bag") collection on Geschichte — adding a second EAGER/fetch-joined List
// here will throw MultipleBagFetchException at boot.
private List<JourneyItem> items = new ArrayList<>();
  • cascade = ALL, orphanRemoval = true is mandatory — deleting a Geschichte must cascade to its JourneyItem rows.
  • FetchType.LAZY + @Transactional(readOnly = true) on GeschichteService.getById() plus an explicit getItems().size() (or Hibernate.initialize) inside the transaction@Transactional alone does NOT hydrate a LAZY collection (it inits on access), and open-in-view: false means there is no session at serialization time. Without the explicit touch, GET /api/geschichten/{id} throws LazyInitializationException → HTTP 500. This was the round-4 blocker.
  • @OrderBy("position ASC") — not @OrderColumn — because position is a domain-meaningful column.
  • @Builder.Default with new ArrayList<>() (not HashSet), preserving ordered list semantics.

list() returns a GeschichteSummary projection (added round 4; field set + query mechanism locked round 5)

GeschichteService.list() must NOT return full Geschichte entities — with open-in-view: false and list() being non-transactional, Jackson serializing getItems() (a LAZY collection) 500s the moment any journey exists. The fix is a list-specific projection that never carries items.

  • Exact field set (round 5 — enumerated, no hand-wave): GeschichteSummary carries exactly id, title, status, type, author, publishedAt, body. It does NOT carry items and does NOT carry persons. Rationale: the live grid card (geschichten/+page.svelte:131–146) renders title, author (byline), publishedAt, and a body excerpt — and never reads persons. The round-4 draft list (id, title, status, type, publishedAt, persons + any other) both dropped the byline/excerpt (a hidden visible regression) AND carried an unused persons association. Drop the "(and any other fields the card grid already renders)" parenthetical — it is untestable.
  • GeschichteService.list() returns List<GeschichteSummary> (or Page<GeschichteSummary> mirroring the current pagination shape).
  • Query mechanism (round 5): use a Spring Data interface-based projection (interface GeschichteSummary { UUID getId(); String getTitle(); ... }) returned from an explicit @Query with ORDER BY COALESCE(g.publishedAt, g.updatedAt) DESC. This replaces the Specification-driven findAll on the list path and sidesteps the GeschichteSpecifications.orderByDisplayDateDesc() / constructor-expression composition problem entirely (that spec's query.orderBy(...) + COUNT special-case does not compose cleanly with a projection query). Keep orderByDisplayDateDesc only on countPublished/any filter spec you retain.
  • DRAFT status-clamp MUST be carried into the projection query verbatim (round 5 — security blocker): current list() computes GeschichteStatus effective = currentUserHasBlogWrite() ? status : GeschichteStatus.PUBLISHED (line 81) and applies hasStatus(effective) + hasAuthor(authorId) (line 84). The projection query must apply the identical clamp. If the implementer writes the projection fresh and forgets this, DRAFT journeys leak into the public grid — a real authz regression.
  • hasAllPersons (person-filter) MUST survive the rewrite (round 5): person-filtering the grid is shipped behaviour. The projection query must still accept and apply the person filter (reimplement against the projection query). Only hasDocument is removed.
  • This also eliminates the EAGER persons Cartesian concern for the grid entirely — the projection selects exactly what it needs.
  • Aligns with the existing PersonSummaryDTO precedent.
  • After this change npm run generate:api produces a second TS type; the geschichten/+page.server.ts / +page.svelte consumers read GeschichteSummary[] for the grid.
  • No external-caller breakage (round 5): dashboard/StatsService.java:19 is the only external caller and uses geschichteService.countPublished()geschichteRepository.count(spec), which never touches items or the projection. Safe.

Changes to Geschichte entity

  • Remove Set<Document> documents (backed by geschichten_documents)
  • Add List<JourneyItem> items ordered by position (LAZY mapping above)
  • Add GeschichteType type
  • Keep Set<Person> persons (via geschichten_persons) — intentionally NOT removed; a JOURNEY carries a denormalized persons set alongside its items
  • Existing body field stays — for STORY it is the rich-text article; for JOURNEY it is an optional intro/preface

Service / DTO / Controller changes (compile-breaking, must ship with this issue)

  • Add @Transactional(readOnly = true) to GeschichteService.getById() and an explicit getItems().size() init inside the transaction body (required by the LAZY + open-in-view:false decision so items hydrate before serialization)
  • Change GeschichteService.list() to return a GeschichteSummary projection (never serializes items) — see section above; carry the DRAFT clamp + hasAllPersons filter
  • Remove GeschichteUpdateDTO.documentIds field
  • Remove GeschichteService.resolveDocuments() method
  • Remove calls to resolveDocuments() from GeschichteService.create() and update()
  • Remove GeschichteSpecifications.hasDocument() and the documentId parameter from GeschichteService.list() — add // TODO(lesereisen-editor): restore document filter via journey_items join comment at the removal site. Keep hasStatus/hasAuthor/hasAllPersons — reimplement them for the projection query.
  • Remove @RequestParam(required = false) UUID documentId from GeschichteController.list() (line 36) and the corresponding argument passed to geschichteService.list(...) (line 41) — leaving it would reference a removed service parameter (compile error)
  • After removal, audit GeschichteService.java imports: import ...document.Document becomes unused — remove it. Keep import ...document.DocumentService and the documentService field with a // Reserved for lesereisen-editor comment (the editor issue resolves item documents through it). LinkedHashSet stays (still used by resolvePersons).
  • GeschichteService may call DocumentService (not DocumentRepository directly) when resolving documents for JourneyItem creation in later issues. Security boundary: JourneyItem.documentId writes must always go through DocumentService.getDocumentById(id) (enforces existence/scope) — never a raw client-supplied UUID persisted directly into the FK. This preserves the boundary the removed resolveDocuments() enforced. The follow-on reader must reach DRAFT JOURNEYs only via getById() (NOT_FOUND guard) — no /journeys/{id} endpoint that skips the check.

Frontend build-integrity changes (must ship with this issue)

After npm run generate:api, the documentId query param disappears from the generated OpenAPI types, turning these into compile-time TS errors that break npm run check on this PR — so they cannot be deferred to the follow-on. All four sites named precisely (round 5):

  • frontend/src/routes/geschichten/+page.server.ts — (site 1) remove the ?documentId= query read and the documentId entry in the api.GET('/api/geschichten', { params: { query } }) call; drop the documentFilter returned from the load. Consume the new GeschichteSummary[] shape for the grid.
  • frontend/src/routes/geschichten/new/+page.server.ts — (site 2) remove the ?documentId= pre-fetch of GET /api/documents/{id}
  • frontend/src/routes/geschichten/+page.svelte — (site 3, real compile error) remove the hasFilters derived's read of data.documentFilter at line 14 (hasFilters = ... || !!data.documentFilter) — once the load stops returning documentFilter, this is a TS error. (site 4, dead-but-harmless) remove url.searchParams.delete('documentId') at line 19 inside rebuildUrl()URLSearchParams.delete() accepts any string so it is not a compile error; remove for cleanliness. The surrounding rebuildUrl() (called by clearAll/addPerson/removePerson) stays — person filtering is live.
  • frontend/src/routes/geschichten/[id]/+page.svelte — change the related-letters block from {#each g.documents as d (d.id)} to {#each g.items as item (item.id)}. The each-key identity changes from document.id to journeyItem.id. The items payload exposes ONLY documentId (the document object is @JsonIgnore'd) — there is no document title or date available. Render the link with a generic localized label (new i18n key, see below), with documentId only in the href, and only when item.documentId is set. When documentId is null but note present, render the note as plain text in a <p class="whitespace-pre-line"> (interlude items are first-class; whitespace-pre-line prevents run-on rendering of hand-authored multi-paragraph notes). One {#if}.
  • New i18n key (round 5 — visible string, ships this issue): add geschichten_journey_letter_link to messages/{de,en,es}.jsonBrief öffnen / Open letter / Abrir carta. The stub link text must be descriptive (WCAG 2.4.4 Link Purpose) — never a bare UUID or empty <a>.
  • frontend/src/routes/geschichten/[id]/edit/GeschichteEditor.sveltegeschichte?.documents becomes undefined post-regen so line 48's optional chain always falls to initialDocuments (editor silently loses persisted linkage). Acceptable as a deferred stub; confirm edit/page.svelte.test.ts does not assert document round-trip. Verify npm run check passes.
  • Frontend test files (round 4 — was a coverage gap):
    • frontend/src/routes/geschichten/[id]/page.svelte.test.ts — the documents: [...] fixture and the two tests (omits the documents section when there are no linked documents, renders the documents section when there are linked documents) reference g.documents. Rewrite both tests to g.items (cheap, keeps render coverage through the stub period; must include an interlude/note-only item asserting it renders without a broken link). Assert the document-backed link via getByRole('link', { name: ... }) (the localized label) — not just that an <a> exists — so a bare-UUID/empty-text regression fails at the test layer.
    • frontend/src/routes/geschichten/[id]/edit/page.svelte.test.ts — update the documents: [] fixture; confirm no document round-trip assertion remains.

This is the minimum to keep the build green, NOT the full reader/editor redesign (that is the follow-on frontend issue).

Accepted degraded UX during the stub period (round 5 — signed-off scope boundary)

This issue ships a deliberate, temporary, visible degradation — stated here so it is a signed-off boundary, not a review surprise:

  • Grid card keeps title, author byline, publishedAt, and the body excerpt (the projection field set guarantees this — the excerpt is load-bearing scent-of-information for the reader index, not decorative).
  • /geschichten/[id] related-letters section degrades: document-backed items render as a generic localized "Open letter" link (no per-document title/date until the follow-on restores them); interlude (note-only) items render as plain text with preserved line breaks. This affects all Geschichten, including existing published family STORIES (they render the related-letters block too) — their rich title/date links degrade to generic links until the follow-on. Data is preserved in items.
  • The "start a story pre-linked to this document" entry point dies until the editor follow-on restores it.
  • Decision: ship the stub now and link the follow-on reader/editor issue as the next-priority blocking dependency (option A). Rationale: holding this foundational migration on UI work it shouldn't depend on (option B) blocks every downstream Lesereisen issue; the regression window is bounded and the follow-on is prioritized. Link the follow-on as a tracked relates-to/blocking dependency so the degraded stub does not silently become permanent.

JourneyItem sub-package contents (this issue only)

geschichte/journeyitem/ needs exactly: JourneyItem.java, JourneyItemRepository.java. No service or controller in this issue — those come later. Do not add anything not required by this issue's AC. If the dual-collection shape (persons Set + items List on one entity) causes confusion, add a short sub-package README.md explaining it.

Flyway migration

Use migration version V72 (latest committed are V70, V71). Single file: V72__add_journey_items_migrate_geschichten_documents.sql.

  1. Add type VARCHAR(50) DEFAULT 'STORY' NOT NULL to geschichten
  2. Create journey_items table (including CHECK constraint and FK cascade rules above)
  3. Add index: CREATE INDEX idx_journey_items_geschichte_position ON journey_items (geschichte_id, position ASC)
  4. Migrate all existing geschichten_documents rows → journey_items with positions initialised at multiples of 1000 (1000, 2000, 3000…), ordered by documents.document_date ASC NULLS LAST, documents.id ASC per geschichte_id (tiebreaker on id ensures deterministic ordering when dates are equal or null). Use SELECT DISTINCT on (geschichte_id, document_id) in the source query to guard against any duplicate junction rows producing duplicate journey items.
  5. Drop geschichten_documents
  6. Add migration comment (note the explicit BEFORE timing word, plus the reverse-reconstruction runbook):
-- Production pre-requisite — run BEFORE applying this migration:
--   docker exec familienarchiv-db sh -c 'pg_dump -U "$POSTGRES_USER" "$POSTGRES_DB" \
--     --table=geschichten_documents --table=journey_items \
--     -f /tmp/pre_v72_backup_'"$(date +%Y%m%d)"'.sql'
-- Take the dump even if geschichten_documents is empty — it captures the table DEFINITION
-- for emergency reconstruction. The DROP TABLE is the only irreversible step; the
-- INSERT...SELECT is a no-op when there is no data. No DDL rollback path exists after commit.
--
-- REVERSE PROCEDURE (if V72 must be rolled back): restore the pre-V72 dump, then re-derive
-- the junction from the new table:
--   INSERT INTO geschichten_documents (geschichte_id, document_id)
--     SELECT geschichte_id, document_id FROM journey_items WHERE document_id IS NOT NULL;

Use a single migration file — the codebase has no precedent for split DDL/DML migrations and the transaction boundary is correct in Flyway's single-file model (Postgres makes DROP + CREATE + INSERT...SELECT atomic in one transaction; any CHECK violation rolls the whole thing back cleanly).

No CREATE INDEX CONCURRENTLY needed — the journey_items table is empty at migration time (newly created), so index builds instantly with zero locking impact. (CONCURRENTLY cannot run inside the Flyway transaction anyway and would break the single-file atomicity.)

Deploy checklist (round 4; env-var quoting fixed round 5 — DevOps)

$POSTGRES_USER/$POSTGRES_DB are set inside the container, not on the host shell. Wrap each command in sh -c '...' so the vars expand inside the container. Single-line commands, in order:

  1. Pre-check row count: docker exec familienarchiv-db sh -c 'psql -U "$POSTGRES_USER" -d "$POSTGRES_DB" -c "SELECT COUNT(*) FROM geschichten_documents;"'
  2. Backup: the pg_dump one-liner (also sh -c-wrapped) from the migration comment above.
  3. Post-deploy smoke: docker exec familienarchiv-db sh -c 'psql -U "$POSTGRES_USER" -d "$POSTGRES_DB" -c "SELECT COUNT(*) FROM journey_items;"' — assert it equals the pre-check count (each junction row maps 1:1 to a journey_item).
  4. Confirm the backend service in docker-compose.yml has a healthcheck and the reverse proxy gates on it (depends_on: service_healthy), so a boot-validation mismatch surfaces as a failed deploy (the new image refuses to come up before serving traffic), not 502s on a swapped-out healthy container. The PR must paste the actual healthcheck: block + the proxy's depends_on from docker-compose.yml as evidence — or note its absence as a flagged pre-existing gap (do not block this issue on it).
  5. Manual data-path verification (round 5): restore a fresh pg_dump of production into a throwaway postgres:16-alpine container, run the backend's Flyway against it, then paste the three verification queries' output into the PR. A hand-built fixture won't carry the duplicate / null-date edge cases the SELECT DISTINCT dedup and the document_date ASC NULLS LAST, id ASC tiebreaker exist for — only real production junction data exercises them.

Acceptance criteria

  • GeschichteType enum exists with STORY and JOURNEY values
  • JourneyItem entity and journey_items table exist with correct columns, FK constraints (ON DELETE CASCADE from geschichten, ON DELETE SET NULL from documents), and CHECK constraint (document_id IS NOT NULL OR note IS NOT NULL)
  • JourneyItem.note has field-level comment covering ALL render contexts AND naming the CWE so it is greppable: // CWE-79 tripwire: plain text — store verbatim, no sanitization. Any HTML/feed/PDF/email renderer MUST escape this; only Svelte {note} is auto-safe.
  • JourneyItem.position has a field-level comment: // Sort key; gaps fine. Duplicate positions within a journey yield undefined relative order — the editor is responsible for keeping them distinct.
  • JourneyItem.document is annotated @JsonIgnore; documentId (UUID) is exposed as the serialized field — confirmed no circular reference / StackOverflowError risk
  • Per-field @Schema(requiredMode = REQUIRED) on id and position; documentId and note left optional (nullable)
  • Geschichte.documents (Set) is removed; Geschichte.items (List, ordered by position, cascade=ALL, orphanRemoval=true, LAZY) is added with the bag/ADR-022/open-in-view inline comment
  • GeschichteService.getById() is annotated @Transactional(readOnly = true) AND explicitly initializes items (getItems().size() or Hibernate.initialize) inside the transactionopen-in-view:false means the collection is otherwise dead at serialization time
  • GeschichteService.list() returns a GeschichteSummary projection that never carries items (no getItems() reachable by Jackson on the grid path); aligns with PersonSummaryDTO precedent
  • GeschichteSummary carries exactly id, title, status, type, author, publishedAt, body — NOT items, NOT persons (matches the live grid card; no byline/excerpt regression, no unused association)
  • GeschichteSummary is a Spring Data interface-based projection returned from an explicit @Query with ORDER BY COALESCE(g.publishedAt, g.updatedAt) DESC (replaces the Specification/orderByDisplayDateDesc composition on the list path)
  • DRAFT status-clamp carried into the projection query: the effective status (currentUserHasBlogWrite() ? status : PUBLISHED) + authorId constraints (GeschichteService:81/84) are applied to the projection query — DRAFT journeys do NOT leak into the public grid
  • hasAllPersons (grid person-filter) reimplemented for the projection query — person filtering remains shipped behaviour; only hasDocument is removed
  • Geschichte.type column exists, defaults to STORY for all existing rows
  • GeschichteUpdateDTO.documentIds removed; GeschichteService.resolveDocuments() removed; create() and update() no longer call resolveDocuments()
  • GeschichteSpecifications.hasDocument() removed; documentId parameter removed from GeschichteService.list(); // TODO(lesereisen-editor) comment left at removal site; hasStatus/hasAuthor/hasAllPersons retained
  • @RequestParam documentId removed from GeschichteController.list() and from the geschichteService.list(...) call site
  • import ...document.Document removed from GeschichteService; documentService field retained with // Reserved for lesereisen-editor comment (no unused-field lint gate fails)
  • All four frontend documentId sites fixed: (1) geschichten/+page.server.ts query param + documentFilter return; (2) geschichten/new/+page.server.ts prefetch; (3) geschichten/+page.svelte:14 hasFilters derived reading data.documentFilter (real compile error); (4) geschichten/+page.svelte:19 dead searchParams.delete('documentId') (cleanup). Grid consumes GeschichteSummary[]; npm run check passes
  • New i18n key geschichten_journey_letter_link (Brief öffnen / Open letter / Abrir carta) added to messages/{de,en,es}.json
  • [id]/+page.svelte related-letters block switched to {#each g.items as item (item.id)}; document-backed items render the generic localized label link (descriptive text, WCAG 2.4.4 — never a bare UUID) with documentId in the href; interlude (null-documentId) items render note in <p class="whitespace-pre-line">; GeschichteEditor.svelte verified non-breaking under npm run check
  • Frontend tests pass (npm run test): geschichten/[id]/page.svelte.test.ts document-section tests rewritten to g.items (including an interlude/note-only assertion; document-link asserted via getByRole('link', { name: ... })); edit/page.svelte.test.ts documents fixture updated, no document round-trip assertion remains
  • Flyway migration runs cleanly on a database with existing geschichten_documents data; positions at multiples of 1000 with deterministic ORDER BY + SELECT DISTINCT guard; single V72 file; pg_dump comment says "BEFORE applying this migration"; reverse-reconstruction SQL present in the migration comment
  • Index idx_journey_items_geschichte_position on (geschichte_id, position ASC) exists
  • All existing backend tests pass after service/DTO adaptation:
    • GeschichteServiceTest and GeschichteControllerTest: builder helpers using .documents(new HashSet<>()) updated; dto.setDocumentIds(...) removed
    • All geschichteService.list(...) call sites in GeschichteServiceTest (lines 134, 148, 161, 174, 186, 198) drop the documentId positional argument (arity change); list_filters_by_documentId (line 180) is DELETED outright (it tests a removed spec/param and cannot compile)
    • create_resolves_documentIds_via_DocumentService test (GeschichteServiceTest:286) replaced with create_does_not_call_DocumentService_for_document_resolution(): assert verify(documentService, never()).getDocumentById(any()) and delete the now-invalid assertThat(saved.getDocuments())... assertion (line 301)
    • list() tests updated to assert against the GeschichteSummary projection (no items field)
    • Any SQL fixtures in backend/src/test/resources/ referencing geschichten_documents updated
  • New behavioral tests (integration test suite) — must force a real DB round-trip:
    • Inject @PersistenceContext EntityManager em into GeschichteServiceIntegrationTest (it is @Transactional at class level, line 35 — tests pass vacuously from the first-level cache without flush/clear)
    • @OrderBy test: insert items out of position order (e.g. 3000, 1000, 2000), em.flush() + em.clear(), then getById and assert the list is [1000, 2000, 3000]. In-order insert proves nothing.
    • Cascade delete test: delete(id), em.flush() + em.clear(), then journeyItemRepository.findAllByGeschichteId(id) returns empty (add JourneyItemRepository to @Autowired list)
    • type round-trip: STORY → getType()==STORY; newly created JOURNEY → getType()==JOURNEY
    • Note-only item (document_id=null, note populated) persists and returns without NPE — covers the null arm of getDocumentId()
    • Document-backed item (document_id + note) — covers the present arm of getDocumentId() (both arms needed for the 88% branch gate, per backend/CLAUDE.md)
    • Document-delete SET-NULL test (round 4): persist a document-backed JourneyItem, em.flush()/clear(), call documentService.deleteDocument(docId), em.flush()/clear(), reload the JourneyItem and assert getDocumentId() == null AND the row still exists. Locks the ON DELETE SET NULL "preserves curator prose" decision — this transition is otherwise untested.
    • Negative CHECK constraint test (exception-type/flush mechanism reconciled, round 5): use journeyItemRepository.saveAndFlush(bothNull) (goes through the Spring @Repository translation layer) and assertThatThrownBy(...).isInstanceOf(DataIntegrityViolationException.class). If instead flushing via a bare em.flush(), the thrown type is Hibernate org.hibernate.exception.ConstraintViolationException (wrapped in PersistenceException), NOT DataIntegrityViolationException — pick saveAndFlush to keep the stated type consistent. The constraint fires on flush, not on save(). Additionally assert the exception message/constraint name contains chk_journey_item_not_empty so a stray NOT NULL elsewhere can't make it pass for the wrong reason.
  • HTTP-layer serialization test (round 4 — the single most important new test): a dedicated @SpringBootTest(webEnvironment = RANDOM_PORT) class (NOT class-level @Transactional, so the session closes before serialization like production; add class comment // NOT @Transactional — must serialize after commit to reproduce open-in-view:false lazy-init 500.) creates a JOURNEY with items, commits, then over HTTP:
    • GET /api/geschichten/{id} asserts $.items present and ordered, $.items[0].documentId exists, $.items[0].document doesNotExist(), $.type exists
    • Interlude assertion (round 5): the same journey has one document-backed + one interlude item; assert $.items[1].documentId is null/absent AND $.items[1].note present (the frontend stub depends on the API emitting a null-documentId item)
    • GET /api/geschichten (grid) asserts 200 AND $[0].author exists AND $[0].body exists AND $[0].items doesNotExist() (round 5 — a bare 200 passes while the card goes blank if the projection drops author/body)
  • DRAFT-clamp authz regression test (round 5 — security): as a READ_ALL-only (no BLOG_WRITE) user, create a DRAFT JOURNEY; GET /api/geschichten asserts the draft is absent from the grid; GET /api/geschichten/{id} of that draft asserts 404. Locks the projection-rewrite carryover of the status clamp.
  • getById_returns_type_field_in_response() in GeschichteControllerTest: the $.items[0].document doesNotExist() assertion is the regression lock on @JsonIgnore (Lombok @Data generates getDocument(); verify it does not leak). (Note: this @WebMvcTest-style assertion is the contract lock; the RANDOM_PORT test above is the lazy-init lock — both are needed.)
  • Migration data-path AC reworded to its real verification method: "Migration DDL verified by Testcontainers boot test in CI; data path (INSERT...SELECT) verified manually against a production pg_dump restored into a throwaway postgres:16-alpine before production deploy, documented in PR." Measurable form: post-migration COUNT(journey_items) equals pre-migration COUNT(DISTINCT (geschichte_id, document_id)) from the junction; positions are strictly increasing multiples of 1000 within each geschichte_id; zero rows with both document_id and note null. Paste this query output into the PR (checklist item, not prose).
  • Testcontainers/full-context boot test runs in CI as the mapping gate — a @OrderBy/column-name mismatch between V72 and the entity fails in CI, never crash-loops production after the schema is already migrated
  • npm run generate:api run after entity changes — TypeScript types regenerated (Geschichte.documentsGeschichte.items; new GeschichteSummary type for the grid)
  • API contract note: GET /api/geschichten/{id} now returns items (with documentId UUID per item, not the full document object) instead of documents; GET /api/geschichten (list) returns GeschichteSummary objects without items; full reader/editor frontend redesign is the follow-on Lesereisen frontend issue
  • Doc updates (PR blockers — not nice-to-haves):
    • docs/architecture/db/db-orm.puml — remove geschichten_documents, add journey_items with columns and FK relationships
    • docs/architecture/db/db-relationships.puml — update relationship lines
    • docs/architecture/c4/l3-backend-*.puml for the geschichte domain — add the new journeyitem/ sub-package (per doc-currency: new backend package/module)
    • CLAUDE.md package structure table — add journeyitem/ sub-package under geschichte/
    • docs/GLOSSARY.md — add GeschichteType, JourneyItem, Lesereise with display names in de/en/es (Lesereise / Reading Journey / Lectura guiada)
    • If an ADR is written for the position/fetch strategy, number it 035 (033 and 034 are taken) and record the LAZY-vs-EAGER decision, the explicit-init-under-open-in-view caveat, the GeschichteSummary list-projection split, and the 1000-gap position strategy

Review Insights

Decided

Decision Choice Rationale
GeschichteSummary field set (BLOCKER, round 5 — hit by 4 personas) Exactly id, title, status, type, author, publishedAt, body; drop persons The round-4 draft (id, title, status, type, publishedAt, persons + any other) silently dropped the author byline and body excerpt (both rendered by the live grid card +page.svelte:131–146) — a visible regression hidden inside the "fix the 500" change — AND carried persons, which the card never reads (an unused association reintroducing query cost). Enumerating the exact set removes the untestable "(and any other fields)" hand-wave. Add persons later, in the follow-on grid-redesign issue, only with the card change that consumes it.
GeschichteSummary query mechanism (round 5) Spring Data interface-based projection + explicit @Query ... ORDER BY COALESCE(g.publishedAt, g.updatedAt) DESC GeschichteSpecifications.orderByDisplayDateDesc() injects query.orderBy(...) on the entity root with a COUNT special-case; it does not compose cleanly with a constructor-expression/projection query. An interface projection with an explicit ORDER BY sidesteps the spec entirely on the list path. The DRAFT status-clamp and hasAllPersons filter must be carried into this query.
DRAFT status-clamp survives the projection rewrite (round 5 — security blocker) Carry effective status + authorId constraints into the projection query; add a list-path authz test list() currently clamps non-BLOG_WRITE users to PUBLISHED (GeschichteService:81/84). A freshly-written projection query that forgets this leaks DRAFT journeys into the public grid — a real authz regression. The new READ_ALL-only test (draft absent from grid + 404 on getById) is the regression lock.
hasAllPersons retained (round 5) Reimplement the person-filter for the projection query; remove only hasDocument Person-filtering the grid is shipped behaviour. The issue removed hasDocument but was silent on hasAllPersons; it must survive.
Stub link text for document-backed items (round 5) Generic localized label (geschichten_journey_letter_link) + documentId in href (Option A) The items payload exposes only documentId (the document object is @JsonIgnore'd) — no title/date is available. A UUID or empty <a> fails WCAG 2.4.4 (Link Purpose) and is unusable for the 60+ audience. Fetching document titles (Option B) pulls reader-redesign work into this data-model issue (scope creep) — that belongs to the follow-on. Add the i18n key this issue.
Ship the degraded stub vs hold for the reader follow-on (round 5) Ship now; link follow-on as next-priority blocking dependency (Option A) Holding the foundational migration on UI work it shouldn't depend on (Option B) blocks every downstream Lesereisen issue. The regression window (existing STORY pages' related-letters degrade to generic links) is bounded and the follow-on is prioritized + linked so the stub does not become permanent.
Negative-CHECK test: exception type vs flush mechanism (round 5) journeyItemRepository.saveAndFlush(bothNull)DataIntegrityViolationException A bare em.flush() throws Hibernate ConstraintViolationException (wrapped in PersistenceException) — Spring's DataIntegrityViolationException translation only happens at the @Repository proxy boundary, not on a bare EntityManager. saveAndFlush keeps the AC's stated type consistent and still fires on flush. Keep the chk_journey_item_not_empty message assertion.
Grid HTTP test must assert shape, not just 200 (round 5) Assert $[0].author + $[0].body present AND $[0].items absent A bare 200 passes even if the projection drops author/body (card goes blank) — the shape assertion is the only test that catches the projection field-set regression.
Interlude item serialized over the wire (round 5) Add a null-documentId + present-note assertion to the RANDOM_PORT getById test The frontend stub depends on the API actually emitting a null-documentId interlude item; the service-layer arm-coverage tests don't prove the HTTP contract.
Deploy-checklist env-var quoting (round 5) Wrap psql/pg_dump in sh -c '...' so $POSTGRES_USER/$POSTGRES_DB expand inside the container Run from the host, the vars expand on the host (empty). A copy-paste-fails-at-3am bug in an otherwise solid runbook.
Manual data-path verification target (round 5) Restore a production pg_dump into a throwaway postgres:16-alpine, run Flyway, paste query output CI's Testcontainers schema is clean — the INSERT...SELECT data path is genuinely untested in CI. Only real production junction data exercises the document_date ASC NULLS LAST, id ASC tiebreaker and the SELECT DISTINCT dedup; a synthetic fixture lacks the duplicate/null-date edge cases.
note field comment names CWE-79 (round 5) Prefix the comment with CWE-79 tripwire: Makes the stored-XSS-latent tripwire greppable; the data is stored unescaped and safety is every future consumer's responsibility ({note} auto-escapes; feed/PDF/email do not).
items fetch strategy + serialization shape (BLOCKER, round 4) LAZY + explicit getItems().size() init in getById(); list() returns a GeschichteSummary projection Round 3 chose LAZY (correct per ADR-022) but did not account for application.yaml:15 open-in-view: false. Two production 500s result: (1) @Transactional alone does NOT hydrate a LAZY collection — Jackson 500s after the session closes on getById; (2) list() is non-transactional and serializes getItems() → 500 the moment any journey exists. Fix: keep LAZY, force-init items inside getById()'s transaction, and give list() a dedicated GeschichteSummary that never carries items. Rejected: @JsonIgnore items + separate /items endpoint (extra round-trip); force-init in both paths (reintroduces Cartesian cost).
HTTP-layer reader test (round 4) Dedicated @SpringBootTest(webEnvironment = RANDOM_PORT) class, no class-level @Transactional A service-layer or class-@Transactional test runs inside its own transaction so LAZY items always init — it passes vacuously while production 500s under open-in-view:false. Only a real-HTTP test that serializes after the transaction commits reproduces the bug. The single most important new test.
Document-delete SET NULL is tested (round 4) Integration test: delete document → JourneyItem.documentId null, row survives The ON DELETE SET NULL "preserves curator prose" contract was decided round 2 but untested. DocumentService.deleteDocument does a plain deleteById; the test proves the cascade rule fires and the item row is retained.
Duplicate position within a journey (round 4) Allow, with documented undefined-order semantics for v1 A UNIQUE (geschichte_id, position) constraint forces the editor's drag-reorder to compute distinct positions every move and complicates naive swaps. The 1000-gap seed keeps positions distinct in practice. State in AC + field comment; revisit if the editor needs the hard guarantee.
Migration order is a chronological seed, not curator intent (round 4) Document as explicit assumption AS-001 The old geschichten_documents was an unordered Set — no curator order existed. Ordering by document_date is a plausible default a Lesereise lets curators re-sequence. Prevents a future reviewer treating chronological order as a requirement.
note length bound (round 4) Pre-register as a follow-on (editor-issue) AC, not this issue No write path exists, so no live DoS. When the editor lands, add a DB CHECK length(note) <= N mirroring chk_text_length (proposed 2000 chars). The column is created here, so the threshold is registered now to avoid drift.
DRAFT JOURNEY non-disclosure (round 4) Follow-on reader MUST route through getById() getById() returns NOT_FOUND (not FORBIDDEN) for DRAFT without BLOG_WRITE, to avoid leaking existence. A DRAFT JOURNEY inherits this — the follow-on must not add a /journeys/{id} endpoint that skips the check.
journey_items.document_id ON DELETE SET NULL Preserves curator prose when a document is deleted; a note-only item is still meaningful. CASCADE would destroy editorial work; RESTRICT blocks archival workflows.
note field format Plain text (v1) Safer rendering ({note}, not {@html}), no sanitization pipeline. Field comment (now CWE-79-tagged) warns feed/email/PDF must escape. No unescaped sink exists today.
Migration position spacing Multiples of 1000 Maximises headroom for drag-to-reorder without rebalance.
Migration ORDER BY tiebreaker documents.document_date ASC NULLS LAST, documents.id ASC Deterministic position assignment when dates are equal/null.
Migration duplicate guard SELECT DISTINCT (geschichte_id, document_id) Guards against duplicate junction rows producing duplicate journey items.
JourneyItem package location geschichte/journeyitem/ sub-package Consistent with document/transcription/ and document/annotation/.
API contract change scope Remove documents, add items (getById) + GeschichteSummary (list); full frontend redesign deferred — but build-breaking documentId references AND now-failing frontend tests fixed now A build-breaking TS error / red npm run test cannot ship "known broken."
GeschichteSpecifications.hasDocument() Remove (Option A) After geschichten_documents is dropped, the spec joins a non-existent table → runtime SQL 500. Rewrite via journey_items deferred to the editor issue. hasStatus/hasAuthor/hasAllPersons are retained.
GeschichteController documentId param Remove in this issue The service list() param is removed, so the controller arg would not compile.
JourneyItem.document serialization @JsonIgnore + expose documentId UUID (Option B) No circular reference, no large nested payloads. Locked by a $.items[0].document doesNotExist() assertion (Lombok @Data generates getDocument()). Not an IDOR risk — READ_ALL is global, no per-document ACL; documentId leaks no more than GET /api/documents/{id} already does.
Per-field @Schema on JourneyItem id, position REQUIRED; documentId, note optional Drives correct TS optionality.
Migration file strategy Single V72 file No split-migration precedent; Flyway per-file transaction boundary is atomic.
Cascade-delete test isolation flush()+clear() inside the existing @Transactional class Sufficient — cascade=ALL + orphanRemoval is the mapping guarantee; DB FK cascade is belt-and-suspenders. Avoids cross-test contamination.
ADR number (if written) 035 033 (tag-name-resolution) and 034 (ollama-deployment) are taken.
Spec reference docs/specs/lesereisen-reader-spec.html + lesereisen-editor-spec.html The originally-linked docs/superpowers/specs/...md does not exist (verified).
Frontend [id] test handling Rewrite to g.items now Cheaper than re-adding coverage later; keeps a live render assertion (incl. interlude-item guard + accessible-link assertion) through the stub period.
No new ErrorCode / Permission Confirmed (round 5) resolveDocuments threw DOCUMENT_NOT_FOUND via a path still live elsewhere — enum value not orphaned. No docs/ARCHITECTURE.md permission-section update.
StatsService impact None (round 5) The only external caller uses countPublished()count(spec), never items/projection. Safe.

Risks to watch

  • GeschichteSummary field-set regression is the highest-leverage round-5 finding — four personas independently hit it. Shipping the round-4 draft list silently strips the author byline + body excerpt off every story card (a visible regression hidden in the 500-fix) and carries an unused persons association. The grid HTTP test's $[0].author/$[0].body present + $[0].items absent assertions are the only gate that catches it.
  • DRAFT status-clamp must be re-applied in the projection query — a freshly-written projection that forgets effective/authorId (GeschichteService:81/84) leaks DRAFT journeys into the public grid. Authz regression, not hypothetical. The READ_ALL-only list-path test is the lock.
  • open-in-view: false turns the round-3 LAZY design into two guaranteed production 500s unless getById() explicitly inits items AND list() uses the GeschichteSummary projection. Service-layer/@Transactional tests CANNOT see this; the dedicated RANDOM_PORT HTTP test is the only gate.
  • items is the first List bag on Geschichte — adding a second EAGER/fetch-joined List throws MultipleBagFetchException at boot. Field comment documents this. If a future maintainer reverts to EAGER, add a test asserting list() returns no duplicate Geschichte rows.
  • Integration tests are false-green without flush()+clear(): the class is @Transactional. @OrderBy must insert out-of-order; cascade delete, document-delete SET NULL, and the negative CHECK must flush to hit the DB. The CHECK fires on flush, not save().
  • Negative-CHECK test exception type depends on the flush mechanismsaveAndFlushDataIntegrityViolationException (Spring translation); bare em.flush() → Hibernate ConstraintViolationException. Pick saveAndFlush to match the AC's stated type.
  • @JsonIgnore regression risk: Lombok @Data generates getDocument(); Jackson serializes by getter, but honours field-level @JsonIgnore. Lock with $.items[0].document doesNotExist().
  • Frontend breaks AND degrades — all four sites: params.query.documentId compile error in two +page.server.ts files; +page.svelte:14 hasFilters derived reading data.documentFilter (real compile error); +page.svelte:19 dead searchParams.delete (cleanup); two [id]/page.svelte.test.ts tests fail at runtime (rewrite to g.items). list_filters_by_documentId in GeschichteServiceTest must be DELETED outright (removed param) and all list(...) call sites drop the positional arg.
  • STORY pages degrade, not only JOURNEY UI: /geschichten/[id]/+page.svelte renders related-letters for all Geschichten — existing published stories lose rich title/date links (degrade to generic "Open letter" links) until the follow-on. The "start a story pre-linked to this document" entry point also dies until the editor restores it. Accepted, signed-off, follow-on linked.
  • Stub link must have accessible text — the items payload has no title/date (only documentId); a bare-UUID or empty <a> fails WCAG 2.4.4. Use the localized label; assert via getByRole('link', { name }).
  • Interlude (note-only) items are first-class (the reader spec uses "interlude" 20×) — the stub must guard item.documentId for null AND apply whitespace-pre-line to the note <p> (one class) so hand-authored multi-paragraph notes (creatable via the editor while the stub is live) don't render run-on. Follow-on reader must use whitespace-pre-line + 16px-minimum body for the 60+ audience.
  • Deploy-checklist env-var expansion$POSTGRES_USER/$POSTGRES_DB only exist inside the container; wrap in sh -c '...' or the one-liners fail at the host shell.
  • Healthcheck/depends_on: service_healthy gate — a V72/entity column mismatch makes the new image fail Hibernate ddl-auto: validate on first boot (after the schema migrated); without a gate the proxy may route to a restart-looping container → 502s instead of a clean failed deploy. PR must paste the actual healthcheck: block + proxy depends_on as evidence, or flag its absence as a pre-existing gap (do not block this issue).
  • Migration data path untested in CI — clean Testcontainers schema has no pre-migration data. Mitigation: restore a production pg_dump into a throwaway postgres:16-alpine, run Flyway, paste row-count equality + position spacing + no-double-null query output into the PR. A synthetic fixture lacks the real duplicate/null-date edge cases.
  • Irreversible migration: DROP TABLE geschichten_documents is the only consequential DDL — INSERT...SELECT is a no-op if production has no journeys-with-documents. Take the pg_dump anyway. Reverse-reconstruction SQL is in the migration comment. Run the pre-deploy COUNT to know whether the data path is live.
  • note is a NEW unescaped serialization sink (CWE-79 latent)Geschichte.body is sanitized on write; JourneyItem.note is serialized raw. Safe for {note} (auto-escaped); feed/PDF/email are future surfaces that MUST escape. The CWE-79-tagged field comment is the only tripwire.
  • Security boundary must survive the migration: JourneyItem.documentId writes (editor issue) must resolve through DocumentService.getDocumentById, never blind-persist a client UUID. No per-document ACL today (READ_ALL global), so pattern-hygiene. Migration's INSERT...SELECT copies document_id from an existing junction row — no orphan-FK/TOCTOU risk (a junction row can't reference a non-existent document).
  • Negative CHECK + SET NULL tests must hit real Postgres (Testcontainers postgres:16-alpine) — H2 lacks both; Mockito does not exercise DB constraints.
  • persons collection stays on Geschichte — the Set<Person> ManyToMany is intentionally NOT removed; a JOURNEY carries a denormalized persons set alongside its items. With the GeschichteSummary projection the grid no longer Cartesian-joins persons × items. Document the dual-collection shape in the sub-package README if it confuses.
  • Position gap exhaustion (future): the reorder/rebalance strategy when adjacent positions fill is deferred to the Journey Editor issue — do not invent an ad-hoc strategy here.

Decided (round 6)

Decision Choice Rationale
GeschichteSummary.author grain (BLOCKER, round 6 — raised by Markus, seconded Felix + Elicit) Nested closed projection: getAuthor() returns an interface exposing exactly firstName, lastName, email The live grid card (+page.svelte:41 authorName(g)) reads g.author.firstName / .lastName / .email — a nested AppUser-shaped object. A flat String getAuthor() or UUID getAuthorId() satisfies the AC's literal "carries author" text AND passes npm run check (the card types author? loosely), yet renders an empty byline at runtime and the round-5 $[0].author exists assertion passes anyway. Return a nested closed projection — NOT a flat string and NOT the full AppUser entity (which would re-expose the password hash and re-introduce a serialization surface). Minimal shape that keeps the byline and avoids over-exposure; costs one join. The HTTP grid test MUST assert $[0].author.email (or .firstName) present — not bare $[0].author — so the nested-shape regression fails at the test layer. Copy the exact inline type the card already declares at +page.svelte:41 ({ firstName?; lastName?; email }).
Stub link duplicate accessible-name (round 6 — raised by Leonie) Add aria-label with the item's 1-based position ("Open letter 1", "Open letter 2"…) — Option B N identical "Open letter" links pointing to different documents is a WCAG 2.4.4-in-context failure: SR users hear "Open letter, Open letter, Open letter." Option B is ~2 lines, no design change, gives each link a distinct accessible name, and removes the only hard a11y failure in the stub. Cheaper than restoring titles (that is reader-redesign scope). The visible label stays geschichten_journey_letter_link; the aria-label carries the position suffix.
File the follow-on reader/editor issue BEFORE merge (round 6 — raised by Elicit, seconded Leonie) File + link it now (Option A) — promote to Definition of Ready The "bounded regression window" claim is only enforceable if the follow-on exists as a numbered, linked Gitea issue carrying a "restore per-item title + date + journey heading" AC. Merging first and filing after (Option B) leaves the "temporary" stub with no tracked owner — the exact failure mode this issue's own risk list warns against. ~15 min cost. DoR: the follow-on reader/editor issue must exist, be linked relates-to/blocks, and carry the restore-title+date+heading AC before this issue merges.
ADR-022 citation collision (round 6 — Markus) Cite the fetch-strategy ADR by FILENAME, not number, in the Geschichte.items inline comment docs/adr/ contains TWO 022 files: 022-csrf-session-revocation-rate-limiting.md AND 022-eager-to-lazy-fetch-strategy.md. The inline comment must read // LAZY per docs/adr/022-eager-to-lazy-fetch-strategy.md so a future maintainer is not sent to the CSRF doc. ADR-035 (if written) should note: extends 022-eager-to-lazy, supersedes nothing; records that items is the first List bag on Geschichte (the MultipleBagFetchException tripwire) as a consequence; and notes the journeys_items.document_id ON DELETE SET NULL follows the existing geschichten.author_id ON DELETE SET NULL (V58→V60) house pattern, so it reads as continuation not invention.
hasAllPersons AND-semantics regression lock (round 6 — Sara) Add a @DataJpaTest person-filter AND-semantics test (story{A,B} vs story{A}, filter ?personId=A&personId=B → only {A,B}) The current hasAllPersons is "one EXISTS subquery per id" Criteria-API code that does NOT survive into a JPQL projection query for free. A naive JOIN g.persons p WHERE p.id IN :ids gives OR semantics (matches any), silently widening the filter — and no existing test asserts AND, so an AND→OR regression passes CI today. The JPQL must use a counting subquery (e.g. (SELECT COUNT(DISTINCT p.id) FROM g.persons p WHERE p.id IN :personIds) = :personCount). The AND-semantics test against real Postgres is the single missing regression lock.
list() test split mechanism (round 6 — Sara) Split the AC "list() tests updated" item into (a) Mockito unit test asserting the repo projection method is invoked with the clamped effective status + authorId, and (b) a @DataJpaTest (Testcontainers Postgres) asserting clamp + COALESCE ordering + person-AND against real rows An interface projection bound to a @Query is only testable via real Postgres (@DataJpaTest/Testcontainers), not Mockito. The six surviving list(...) Mockito tests (lines 134/148/161/174/186/198) can only assert the repo method is called with the clamped status; the security-critical clamp/ordering/AND behavior moves to @DataJpaTest with real data. Also grep those six tests for .getPersons(/.getDocuments( on the list() result — GeschichteSummary has no persons, so any such assertion breaks beyond the arity change and must convert or move to @DataJpaTest.
DRAFT-clamp grid test asserts specific id, not count (round 6 — Nora) Assert the specific DRAFT id is ABSENT from the projection result and the PUBLISHED id is PRESENT — not list size The genuinely new authz surface is the projection query's WHERE clause (getById's NOT_FOUND guard at GeschichteService:67 is untouched). Asserting list size alone is weak — a draft could be absent for unrelated reasons. Create one PUBLISHED + one DRAFT, run the grid as a non-BLOG_WRITE user, assert published id present AND draft id absent.
Reader stub: delete documentDate span + formatDate import (round 6 — Felix) In [id]/+page.svelte delete the formatDate(d.documentDate) span (lines 113–117) and drop the formatDate import if now unused The current card renders {d.title} AND formatDate(d.documentDate). Post-regen item.documentDate does not exist on the projection — it is a TS compile error, not just dead code. Rewriting only the {#each} is insufficient; the date span and (if orphaned) the import must go. Keep the geschichten_documents_section <h2> heading — it is still the section heading for the stub period; do NOT delete the geschichten_documents_section i18n key (one usage, [id]/+page.svelte:103). The follow-on reader replaces the heading.
Deploy-checklist count reconciliation (round 6 — Tobias) Pre-check and smoke-check both use COUNT(DISTINCT (geschichte_id, document_id)) on the source; assertion is "COUNT(journey_items) ≤ source, any shortfall explained by the SELECT DISTINCT dedup" — NOT strict equality The migration dedups via SELECT DISTINCT (geschichte_id, document_id). If production has duplicate junction rows, COUNT(journey_items) < COUNT(*) and a strict-equality smoke check FAILS on a correct migration. Step-1 pre-check must be COUNT(DISTINCT ...) (not COUNT(*)); the runbook must say "≤, shortfall = dedup worked," not "must equal." Reconciles the runbook with the measurable AC.
Pre-migration backup table list (round 6 — Tobias) Pre-migration pg_dump dumps ONLY --table=geschichten_documents — drop --table=journey_items journey_items does not exist pre-migration (V72 creates it), so pg_dump --table=journey_items warns/errors on a missing table. The post-migration table needs no pre-backup; it is reconstructable from the dump + reverse SQL. (Update the migration-comment pg_dump one-liner accordingly.)
Throwaway-container verification dump currency (round 6 — Tobias) The production pg_dump restored into the throwaway postgres:16-alpine must be schema-current (≥V71) A historical dump makes Flyway replay V60's ALTER TABLE users RENAME TO app_users against an already-renamed schema and fail. Use a full current dump so Flyway resumes at V72.
Reverse-procedure FK semantics caveat (round 6 — Nora) Add to the reverse-procedure migration comment: the reconstructed geschichten_documents.document_id FK is ON DELETE CASCADE per the original V58 (NOT the new SET NULL of journey_items); and post-V60 the domain FKs target app_users, not users A careless reconstructor copying the new table's SET NULL rule, or hand-typing V58's verbatim REFERENCES users DDL, would silently change delete behavior or fail on the renamed table. The captured dump definition already says app_users; the comment must warn against hand-typing the old V58 text and against copying the new SET-NULL semantics into the junction.
AS-002 — existing-STORY visible degradation logged (round 6 — Elicit) Add AS-002 to the assumptions register, mirroring AS-001 "Existing published Geschichten (STORYs) render the related-letters block; this block visibly degrades to generic links (loss of per-document title AND date) for ALL current readers during the stub window. Accepted because the reader follow-on is the next-priority blocking dependency." Converts the implicit sign-off into a traceable assumption a future reviewer can challenge. The date-scent loss (not just the title) is the real senior-UX cost — the follow-on's restore AC must cover title AND date.
frontend/src/lib/geschichte/README.md doc-currency (round 6 — Markus) Grep it before merge; if it references documents (the entity-shape change documentsitems), it is a doc blocker Not in the issue's enumerated doc list, but a Geschichte frontend domain README exists and may document the card/editor contract. Same doc-currency rule that flags the backend READMEs.

Additional AC items (round 6)

  • GeschichteSummary.getAuthor() is a nested closed projection exposing exactly firstName, lastName, email (NOT a flat string, NOT the full AppUser); the grid HTTP test asserts $[0].author.email (or .firstName) present — not bare $[0].author
  • Stub document-backed link carries an aria-label with the item's 1-based position (distinct accessible names; WCAG 2.4.4), preserves the block px-4 py-3 (≥44px) touch target; interlude note paragraph uses <p class="whitespace-pre-line text-ink-2"> (explicit text token for dark-mode contrast)
  • Follow-on reader/editor issue filed + linked (relates-to/blocks) with a "restore per-item title + date + journey heading" AC BEFORE this merges (Definition of Ready)
  • Geschichte.items inline comment cites docs/adr/022-eager-to-lazy-fetch-strategy.md by FILENAME (not "ADR-022" — collides with the CSRF 022)
  • AND-semantics person-filter @DataJpaTest: story{A,B} + story{A}; ?personId=A&personId=B returns exactly {A,B} (locks hasAllPersons AND semantics through the projection rewrite; JPQL uses a counting subquery, not a naive IN join)
  • list() test split: (a) Mockito — repo projection method invoked with clamped effective status + authorId; (b) @DataJpaTest (Testcontainers Postgres) — clamp + COALESCE ordering + person-AND against real rows; six surviving Mockito list(...) tests grepped for .getPersons(/.getDocuments( on the result and converted/moved
  • DRAFT-clamp grid test asserts the specific draft id is absent + published id present (not list size)
  • [id]/+page.svelte: formatDate(d.documentDate) span deleted; formatDate import dropped if orphaned; geschichten_documents_section heading + i18n key retained for the stub period
  • Deploy checklist: pre-check + smoke use COUNT(DISTINCT (geschichte_id, document_id)); assertion is "≤, shortfall = dedup"; pre-migration pg_dump dumps only geschichten_documents; throwaway-container verification uses a schema-current (≥V71) production dump
  • Reverse-procedure migration comment warns: reconstructed junction FK is ON DELETE CASCADE (V58), domain FKs target app_users (post-V60) — do not hand-type V58's REFERENCES users nor copy journey_items' SET-NULL into the junction
  • AS-002 logged in the assumptions register (existing-STORY related-letters degrade for all current readers during the stub window)
  • frontend/src/lib/geschichte/README.md grepped; updated if it references documents (doc blocker)

Risks to watch (round 6 additions)

  • GeschichteSummary.author flat-vs-nested is a second hidden byline regression — even with the field named, a flat String/UUID author passes npm run check and the round-5 $[0].author exists assertion while the byline renders empty. Only a nested closed projection + an $[0].author.email (not bare $[0].author) assertion catches it.
  • hasAllPersons AND→OR silent widening — rewriting the EXISTS-per-id Criteria filter as a naive JOIN ... WHERE p.id IN :ids in JPQL gives OR semantics; no existing test asserts AND, so the regression passes CI. Lock with the {A,B}-vs-{A} @DataJpaTest.
  • Interface-projection @Query is @DataJpaTest-only — the clamp/ordering/AND logic cannot be Mockito-tested; the security-critical clamp must be verified against real Postgres rows, not just "repo method called."
  • Deploy runbook self-contradiction — strict COUNT(*) equality FAILS on a correct dedup migration if production has duplicate junction rows; use COUNT(DISTINCT ...) and "≤" everywhere.
  • Pre-migration pg_dump --table=journey_items errors — the table doesn't exist yet; dump only geschichten_documents pre-migration.
  • ADR-022 number collides with the CSRF ADR — cite the fetch-strategy ADR by filename in the inline comment.
  • The stub's date-scent loss (not just title) is the real senior-UX cost — for a 60+ reader scanning chronological correspondence the date was the primary wayfinding cue; N identical "Open letter" rows turn a scannable list into noise. AS-002 + the follow-on's restore-title-AND-date AC make the regression provably temporary.
## Goal Extend the `Geschichte` entity to support Reading Journeys by adding a type discriminator and replacing the unordered `Set<Document>` with an ordered `JourneyItem` sequence. This is the foundation all other Lesereisen issues build on. ## Background A *Lesereise* (Reading Journey) is a `Geschichte` with `type = JOURNEY`. Its primary content is an ordered sequence of letters with optional curator notes. Design specs: `docs/specs/lesereisen-reader-spec.html` and `docs/specs/lesereisen-editor-spec.html`. ## Tasks ### New enum: `GeschichteType` ```java public enum GeschichteType { STORY, JOURNEY } ``` Add to `Geschichte` as a non-null column `type`, default `STORY`. ### New entity: `JourneyItem` ``` journey_items id UUID PK geschichte_id UUID FK → geschichten ON DELETE CASCADE (NOT NULL) position INT NOT NULL document_id UUID FK → documents ON DELETE SET NULL (nullable) note TEXT (nullable, plain text — NOT HTML) CONSTRAINT chk_journey_item_not_empty CHECK (document_id IS NOT NULL OR note IS NOT NULL) ``` - `document_id = null` → pure interlude note (context between letters, no document); `note` must be non-null (enforced by CHECK constraint) - Both set → letter with curator annotation - Only `document_id` → plain letter entry, no note - `position` is the sort key; gaps are fine. Duplicate positions within one journey are **allowed** with documented undefined-order semantics (see Review Insights — round 4) - `note` is **plain text** (not HTML) — render with `{note}` in Svelte, no sanitization required - **`document` field**: annotate with `@JsonIgnore`; expose `documentId` (UUID) only in the API response — see serialization decision below - **Per-field `@Schema(requiredMode = REQUIRED)`**: `id`, `position` are always populated → `REQUIRED`. `documentId` and `note` are nullable → NOT required (omit `requiredMode` so the generated TS types them as optional). This drives correct TypeScript optionality for the follow-on frontend. Place `JourneyItem` in `geschichte/journeyitem/` sub-package, consistent with `document/transcription/` and `document/annotation/` patterns. ### JPA mapping on `Geschichte.items` — LAZY with explicit init (decision refined in round 4, see Review Insights) ```java @OneToMany(mappedBy = "geschichte", cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.LAZY) @OrderBy("position ASC") @Builder.Default // LAZY per ADR-022. open-in-view is FALSE (application.yaml:15), so this collection is DEAD at // Jackson serialization time unless explicitly initialized inside the service transaction. // getById() is @Transactional(readOnly=true) AND calls getItems().size() to force-init before return. // list() must NOT serialize items at all — it returns a GeschichteSummary projection (see below). // This is the first List ("bag") collection on Geschichte — adding a second EAGER/fetch-joined List // here will throw MultipleBagFetchException at boot. private List<JourneyItem> items = new ArrayList<>(); ``` - `cascade = ALL, orphanRemoval = true` is mandatory — deleting a `Geschichte` must cascade to its `JourneyItem` rows. - `FetchType.LAZY` + `@Transactional(readOnly = true)` on `GeschichteService.getById()` **plus an explicit `getItems().size()` (or `Hibernate.initialize`) inside the transaction** — `@Transactional` alone does NOT hydrate a LAZY collection (it inits on *access*), and `open-in-view: false` means there is no session at serialization time. Without the explicit touch, `GET /api/geschichten/{id}` throws `LazyInitializationException` → HTTP 500. This was the round-4 blocker. - `@OrderBy("position ASC")` — not `@OrderColumn` — because `position` is a domain-meaningful column. - `@Builder.Default` with `new ArrayList<>()` (not `HashSet`), preserving ordered list semantics. ### `list()` returns a `GeschichteSummary` projection (added round 4; field set + query mechanism locked round 5) `GeschichteService.list()` must NOT return full `Geschichte` entities — with `open-in-view: false` and `list()` being non-transactional, Jackson serializing `getItems()` (a LAZY collection) 500s the moment any journey exists. The fix is a list-specific projection that never carries `items`. - **Exact field set (round 5 — enumerated, no hand-wave):** `GeschichteSummary` carries exactly `id, title, status, type, author, publishedAt, body`. It does **NOT** carry `items` and does **NOT** carry `persons`. Rationale: the live grid card (`geschichten/+page.svelte:131–146`) renders `title`, `author` (byline), `publishedAt`, and a `body` excerpt — and never reads `persons`. The round-4 draft list (`id, title, status, type, publishedAt, persons + any other`) both dropped the byline/excerpt (a hidden visible regression) AND carried an unused `persons` association. Drop the "(and any other fields the card grid already renders)" parenthetical — it is untestable. - `GeschichteService.list()` returns `List<GeschichteSummary>` (or `Page<GeschichteSummary>` mirroring the current pagination shape). - **Query mechanism (round 5):** use a **Spring Data interface-based projection** (`interface GeschichteSummary { UUID getId(); String getTitle(); ... }`) returned from an explicit `@Query` with `ORDER BY COALESCE(g.publishedAt, g.updatedAt) DESC`. This replaces the `Specification`-driven `findAll` on the list path and sidesteps the `GeschichteSpecifications.orderByDisplayDateDesc()` / constructor-expression composition problem entirely (that spec's `query.orderBy(...)` + COUNT special-case does not compose cleanly with a projection query). Keep `orderByDisplayDateDesc` only on `countPublished`/any filter spec you retain. - **DRAFT status-clamp MUST be carried into the projection query verbatim (round 5 — security blocker):** current `list()` computes `GeschichteStatus effective = currentUserHasBlogWrite() ? status : GeschichteStatus.PUBLISHED` (line 81) and applies `hasStatus(effective)` + `hasAuthor(authorId)` (line 84). The projection query must apply the identical clamp. If the implementer writes the projection fresh and forgets this, DRAFT journeys leak into the public grid — a real authz regression. - **`hasAllPersons` (person-filter) MUST survive the rewrite (round 5):** person-filtering the grid is shipped behaviour. The projection query must still accept and apply the person filter (reimplement against the projection query). Only `hasDocument` is removed. - This also eliminates the EAGER `persons` Cartesian concern for the grid entirely — the projection selects exactly what it needs. - Aligns with the existing `PersonSummaryDTO` precedent. - After this change `npm run generate:api` produces a second TS type; the `geschichten/+page.server.ts` / `+page.svelte` consumers read `GeschichteSummary[]` for the grid. - **No external-caller breakage (round 5):** `dashboard/StatsService.java:19` is the only external caller and uses `geschichteService.countPublished()` → `geschichteRepository.count(spec)`, which never touches `items` or the projection. Safe. ### Changes to `Geschichte` entity - Remove `Set<Document> documents` (backed by `geschichten_documents`) - Add `List<JourneyItem> items` ordered by `position` (LAZY mapping above) - Add `GeschichteType type` - Keep `Set<Person> persons` (via `geschichten_persons`) — intentionally NOT removed; a JOURNEY carries a denormalized `persons` set alongside its `items` - Existing `body` field stays — for STORY it is the rich-text article; for JOURNEY it is an optional intro/preface ### Service / DTO / Controller changes (compile-breaking, must ship with this issue) - Add `@Transactional(readOnly = true)` to `GeschichteService.getById()` **and an explicit `getItems().size()` init inside the transaction body** (required by the LAZY + `open-in-view:false` decision so `items` hydrate before serialization) - Change `GeschichteService.list()` to return a `GeschichteSummary` projection (never serializes `items`) — see section above; carry the DRAFT clamp + `hasAllPersons` filter - Remove `GeschichteUpdateDTO.documentIds` field - Remove `GeschichteService.resolveDocuments()` method - Remove calls to `resolveDocuments()` from `GeschichteService.create()` and `update()` - Remove `GeschichteSpecifications.hasDocument()` and the `documentId` parameter from `GeschichteService.list()` — add `// TODO(lesereisen-editor): restore document filter via journey_items join` comment at the removal site. **Keep `hasStatus`/`hasAuthor`/`hasAllPersons` — reimplement them for the projection query.** - **Remove `@RequestParam(required = false) UUID documentId` from `GeschichteController.list()`** (line 36) and the corresponding argument passed to `geschichteService.list(...)` (line 41) — leaving it would reference a removed service parameter (compile error) - After removal, audit `GeschichteService.java` imports: `import ...document.Document` becomes unused — remove it. Keep `import ...document.DocumentService` and the `documentService` field with a `// Reserved for lesereisen-editor` comment (the editor issue resolves item documents through it). `LinkedHashSet` stays (still used by `resolvePersons`). - `GeschichteService` may call `DocumentService` (not `DocumentRepository` directly) when resolving documents for `JourneyItem` creation in later issues. **Security boundary:** `JourneyItem.documentId` writes must always go through `DocumentService.getDocumentById(id)` (enforces existence/scope) — never a raw client-supplied UUID persisted directly into the FK. This preserves the boundary the removed `resolveDocuments()` enforced. The follow-on reader must reach DRAFT JOURNEYs only via `getById()` (NOT_FOUND guard) — no `/journeys/{id}` endpoint that skips the check. ### Frontend build-integrity changes (must ship with this issue) After `npm run generate:api`, the `documentId` query param disappears from the generated OpenAPI types, turning these into **compile-time TS errors that break `npm run check` on this PR** — so they cannot be deferred to the follow-on. **All four sites named precisely (round 5):** - `frontend/src/routes/geschichten/+page.server.ts` — (site 1) remove the `?documentId=` query read and the `documentId` entry in the `api.GET('/api/geschichten', { params: { query } })` call; drop the `documentFilter` returned from the load. Consume the new `GeschichteSummary[]` shape for the grid. - `frontend/src/routes/geschichten/new/+page.server.ts` — (site 2) remove the `?documentId=` pre-fetch of `GET /api/documents/{id}` - `frontend/src/routes/geschichten/+page.svelte` — (site 3, real compile error) remove the `hasFilters` derived's read of `data.documentFilter` at **line 14** (`hasFilters = ... || !!data.documentFilter`) — once the load stops returning `documentFilter`, this is a TS error. (site 4, dead-but-harmless) remove `url.searchParams.delete('documentId')` at **line 19** inside `rebuildUrl()` — `URLSearchParams.delete()` accepts any string so it is not a compile error; remove for cleanliness. The surrounding `rebuildUrl()` (called by `clearAll`/`addPerson`/`removePerson`) stays — person filtering is live. - `frontend/src/routes/geschichten/[id]/+page.svelte` — change the related-letters block from `{#each g.documents as d (d.id)}` to `{#each g.items as item (item.id)}`. **The each-key identity changes from `document.id` to `journeyItem.id`.** The `items` payload exposes ONLY `documentId` (the `document` object is `@JsonIgnore`'d) — there is **no document title or date available**. Render the link with a **generic localized label** (new i18n key, see below), with `documentId` only in the `href`, and only when `item.documentId` is set. When `documentId` is null but `note` present, render the note as plain text in a `<p class="whitespace-pre-line">` (interlude items are first-class; `whitespace-pre-line` prevents run-on rendering of hand-authored multi-paragraph notes). One `{#if}`. - **New i18n key (round 5 — visible string, ships this issue):** add `geschichten_journey_letter_link` to `messages/{de,en,es}.json` — `Brief öffnen` / `Open letter` / `Abrir carta`. The stub link text must be descriptive (WCAG 2.4.4 Link Purpose) — never a bare UUID or empty `<a>`. - `frontend/src/routes/geschichten/[id]/edit/GeschichteEditor.svelte` — `geschichte?.documents` becomes `undefined` post-regen so line 48's optional chain always falls to `initialDocuments` (editor silently loses persisted linkage). Acceptable as a deferred stub; confirm `edit/page.svelte.test.ts` does not assert document round-trip. Verify `npm run check` passes. - **Frontend test files (round 4 — was a coverage gap):** - `frontend/src/routes/geschichten/[id]/page.svelte.test.ts` — the `documents: [...]` fixture and the two tests (`omits the documents section when there are no linked documents`, `renders the documents section when there are linked documents`) reference `g.documents`. **Rewrite both tests to `g.items`** (cheap, keeps render coverage through the stub period; must include an interlude/note-only item asserting it renders without a broken link). Assert the document-backed link via `getByRole('link', { name: ... })` (the localized label) — not just that an `<a>` exists — so a bare-UUID/empty-text regression fails at the test layer. - `frontend/src/routes/geschichten/[id]/edit/page.svelte.test.ts` — update the `documents: []` fixture; confirm no document round-trip assertion remains. This is the minimum to keep the build green, NOT the full reader/editor redesign (that is the follow-on frontend issue). ### Accepted degraded UX during the stub period (round 5 — signed-off scope boundary) This issue ships a deliberate, temporary, visible degradation — stated here so it is a signed-off boundary, not a review surprise: - **Grid card keeps** title, author byline, `publishedAt`, and the `body` excerpt (the projection field set guarantees this — the excerpt is load-bearing scent-of-information for the reader index, not decorative). - **`/geschichten/[id]` related-letters section degrades**: document-backed items render as a generic localized "Open letter" link (no per-document title/date until the follow-on restores them); interlude (note-only) items render as plain text with preserved line breaks. This affects **all** Geschichten, including existing published family STORIES (they render the related-letters block too) — their rich title/date links degrade to generic links until the follow-on. Data is preserved in `items`. - The "start a story pre-linked to this document" entry point dies until the editor follow-on restores it. - **Decision: ship the stub now and link the follow-on reader/editor issue as the next-priority blocking dependency** (option A). Rationale: holding this foundational migration on UI work it shouldn't depend on (option B) blocks every downstream Lesereisen issue; the regression window is bounded and the follow-on is prioritized. Link the follow-on as a tracked `relates-to`/blocking dependency so the degraded stub does not silently become permanent. ### `JourneyItem` sub-package contents (this issue only) `geschichte/journeyitem/` needs exactly: `JourneyItem.java`, `JourneyItemRepository.java`. No service or controller in this issue — those come later. Do not add anything not required by this issue's AC. If the dual-collection shape (`persons` Set + `items` List on one entity) causes confusion, add a short sub-package `README.md` explaining it. ### Flyway migration Use migration version **V72** (latest committed are V70, V71). Single file: `V72__add_journey_items_migrate_geschichten_documents.sql`. 1. Add `type VARCHAR(50) DEFAULT 'STORY' NOT NULL` to `geschichten` 2. Create `journey_items` table (including CHECK constraint and FK cascade rules above) 3. Add index: `CREATE INDEX idx_journey_items_geschichte_position ON journey_items (geschichte_id, position ASC)` 4. Migrate all existing `geschichten_documents` rows → `journey_items` with positions initialised at multiples of **1000** (1000, 2000, 3000…), ordered by `documents.document_date ASC NULLS LAST, documents.id ASC` per `geschichte_id` (tiebreaker on `id` ensures deterministic ordering when dates are equal or null). Use `SELECT DISTINCT` on `(geschichte_id, document_id)` in the source query to guard against any duplicate junction rows producing duplicate journey items. 5. Drop `geschichten_documents` 6. Add migration comment (note the explicit **BEFORE** timing word, plus the reverse-reconstruction runbook): ```sql -- Production pre-requisite — run BEFORE applying this migration: -- docker exec familienarchiv-db sh -c 'pg_dump -U "$POSTGRES_USER" "$POSTGRES_DB" \ -- --table=geschichten_documents --table=journey_items \ -- -f /tmp/pre_v72_backup_'"$(date +%Y%m%d)"'.sql' -- Take the dump even if geschichten_documents is empty — it captures the table DEFINITION -- for emergency reconstruction. The DROP TABLE is the only irreversible step; the -- INSERT...SELECT is a no-op when there is no data. No DDL rollback path exists after commit. -- -- REVERSE PROCEDURE (if V72 must be rolled back): restore the pre-V72 dump, then re-derive -- the junction from the new table: -- INSERT INTO geschichten_documents (geschichte_id, document_id) -- SELECT geschichte_id, document_id FROM journey_items WHERE document_id IS NOT NULL; ``` Use a **single migration file** — the codebase has no precedent for split DDL/DML migrations and the transaction boundary is correct in Flyway's single-file model (Postgres makes `DROP` + `CREATE` + `INSERT...SELECT` atomic in one transaction; any CHECK violation rolls the whole thing back cleanly). No `CREATE INDEX CONCURRENTLY` needed — the `journey_items` table is empty at migration time (newly created), so index builds instantly with zero locking impact. (CONCURRENTLY cannot run inside the Flyway transaction anyway and would break the single-file atomicity.) ### Deploy checklist (round 4; env-var quoting fixed round 5 — DevOps) `$POSTGRES_USER`/`$POSTGRES_DB` are set **inside** the container, not on the host shell. Wrap each command in `sh -c '...'` so the vars expand inside the container. Single-line commands, in order: 1. Pre-check row count: `docker exec familienarchiv-db sh -c 'psql -U "$POSTGRES_USER" -d "$POSTGRES_DB" -c "SELECT COUNT(*) FROM geschichten_documents;"'` 2. Backup: the `pg_dump` one-liner (also `sh -c`-wrapped) from the migration comment above. 3. Post-deploy smoke: `docker exec familienarchiv-db sh -c 'psql -U "$POSTGRES_USER" -d "$POSTGRES_DB" -c "SELECT COUNT(*) FROM journey_items;"'` — assert it equals the pre-check count (each junction row maps 1:1 to a journey_item). 4. Confirm the backend service in `docker-compose.yml` has a healthcheck and the reverse proxy gates on it (`depends_on: service_healthy`), so a boot-validation mismatch surfaces as a failed deploy (the new image refuses to come up before serving traffic), not 502s on a swapped-out healthy container. **The PR must paste the actual `healthcheck:` block + the proxy's `depends_on` from `docker-compose.yml` as evidence — or note its absence as a flagged pre-existing gap (do not block this issue on it).** 5. **Manual data-path verification (round 5):** restore a fresh `pg_dump` of **production** into a throwaway `postgres:16-alpine` container, run the backend's Flyway against it, then paste the three verification queries' output into the PR. A hand-built fixture won't carry the duplicate / null-date edge cases the `SELECT DISTINCT` dedup and the `document_date ASC NULLS LAST, id ASC` tiebreaker exist for — only real production junction data exercises them. ## Acceptance criteria - [ ] `GeschichteType` enum exists with `STORY` and `JOURNEY` values - [ ] `JourneyItem` entity and `journey_items` table exist with correct columns, FK constraints (`ON DELETE CASCADE` from `geschichten`, `ON DELETE SET NULL` from `documents`), and CHECK constraint (`document_id IS NOT NULL OR note IS NOT NULL`) - [ ] `JourneyItem.note` has field-level comment covering ALL render contexts AND naming the CWE so it is greppable: `// CWE-79 tripwire: plain text — store verbatim, no sanitization. Any HTML/feed/PDF/email renderer MUST escape this; only Svelte {note} is auto-safe.` - [ ] `JourneyItem.position` has a field-level comment: `// Sort key; gaps fine. Duplicate positions within a journey yield undefined relative order — the editor is responsible for keeping them distinct.` - [ ] `JourneyItem.document` is annotated `@JsonIgnore`; `documentId` (UUID) is exposed as the serialized field — confirmed no circular reference / StackOverflowError risk - [ ] Per-field `@Schema(requiredMode = REQUIRED)` on `id` and `position`; `documentId` and `note` left optional (nullable) - [ ] `Geschichte.documents` (Set) is removed; `Geschichte.items` (List, ordered by position, `cascade=ALL, orphanRemoval=true`, **LAZY**) is added with the bag/ADR-022/open-in-view inline comment - [ ] `GeschichteService.getById()` is annotated `@Transactional(readOnly = true)` **AND explicitly initializes `items` (`getItems().size()` or `Hibernate.initialize`) inside the transaction** — `open-in-view:false` means the collection is otherwise dead at serialization time - [ ] `GeschichteService.list()` returns a `GeschichteSummary` projection that never carries `items` (no `getItems()` reachable by Jackson on the grid path); aligns with `PersonSummaryDTO` precedent - [ ] **`GeschichteSummary` carries exactly `id, title, status, type, author, publishedAt, body` — NOT `items`, NOT `persons`** (matches the live grid card; no byline/excerpt regression, no unused association) - [ ] **`GeschichteSummary` is a Spring Data interface-based projection** returned from an explicit `@Query` with `ORDER BY COALESCE(g.publishedAt, g.updatedAt) DESC` (replaces the `Specification`/`orderByDisplayDateDesc` composition on the list path) - [ ] **DRAFT status-clamp carried into the projection query**: the `effective` status (`currentUserHasBlogWrite() ? status : PUBLISHED`) + `authorId` constraints (GeschichteService:81/84) are applied to the projection query — DRAFT journeys do NOT leak into the public grid - [ ] **`hasAllPersons` (grid person-filter) reimplemented for the projection query** — person filtering remains shipped behaviour; only `hasDocument` is removed - [ ] `Geschichte.type` column exists, defaults to `STORY` for all existing rows - [ ] `GeschichteUpdateDTO.documentIds` removed; `GeschichteService.resolveDocuments()` removed; `create()` and `update()` no longer call `resolveDocuments()` - [ ] `GeschichteSpecifications.hasDocument()` removed; `documentId` parameter removed from `GeschichteService.list()`; `// TODO(lesereisen-editor)` comment left at removal site; `hasStatus`/`hasAuthor`/`hasAllPersons` retained - [ ] `@RequestParam documentId` removed from `GeschichteController.list()` and from the `geschichteService.list(...)` call site - [ ] `import ...document.Document` removed from `GeschichteService`; `documentService` field retained with `// Reserved for lesereisen-editor` comment (no unused-field lint gate fails) - [ ] **All four frontend `documentId` sites fixed**: (1) `geschichten/+page.server.ts` query param + `documentFilter` return; (2) `geschichten/new/+page.server.ts` prefetch; (3) `geschichten/+page.svelte:14` `hasFilters` derived reading `data.documentFilter` (real compile error); (4) `geschichten/+page.svelte:19` dead `searchParams.delete('documentId')` (cleanup). Grid consumes `GeschichteSummary[]`; `npm run check` passes - [ ] New i18n key `geschichten_journey_letter_link` (`Brief öffnen` / `Open letter` / `Abrir carta`) added to `messages/{de,en,es}.json` - [ ] `[id]/+page.svelte` related-letters block switched to `{#each g.items as item (item.id)}`; document-backed items render the **generic localized label** link (descriptive text, WCAG 2.4.4 — never a bare UUID) with `documentId` in the `href`; interlude (null-`documentId`) items render `note` in `<p class="whitespace-pre-line">`; `GeschichteEditor.svelte` verified non-breaking under `npm run check` - [ ] **Frontend tests pass (`npm run test`)**: `geschichten/[id]/page.svelte.test.ts` document-section tests rewritten to `g.items` (including an interlude/note-only assertion; document-link asserted via `getByRole('link', { name: ... })`); `edit/page.svelte.test.ts` `documents` fixture updated, no document round-trip assertion remains - [ ] Flyway migration runs cleanly on a database with existing `geschichten_documents` data; positions at multiples of 1000 with deterministic ORDER BY + `SELECT DISTINCT` guard; single V72 file; `pg_dump` comment says "BEFORE applying this migration"; reverse-reconstruction SQL present in the migration comment - [ ] Index `idx_journey_items_geschichte_position` on `(geschichte_id, position ASC)` exists - [ ] All existing backend tests pass after service/DTO adaptation: - `GeschichteServiceTest` and `GeschichteControllerTest`: builder helpers using `.documents(new HashSet<>())` updated; `dto.setDocumentIds(...)` removed - **All `geschichteService.list(...)` call sites in `GeschichteServiceTest` (lines 134, 148, 161, 174, 186, 198) drop the `documentId` positional argument (arity change); `list_filters_by_documentId` (line 180) is DELETED outright** (it tests a removed spec/param and cannot compile) - `create_resolves_documentIds_via_DocumentService` test (`GeschichteServiceTest:286`) replaced with `create_does_not_call_DocumentService_for_document_resolution()`: assert `verify(documentService, never()).getDocumentById(any())` and delete the now-invalid `assertThat(saved.getDocuments())...` assertion (line 301) - `list()` tests updated to assert against the `GeschichteSummary` projection (no `items` field) - Any SQL fixtures in `backend/src/test/resources/` referencing `geschichten_documents` updated - [ ] New behavioral tests (integration test suite) — **must force a real DB round-trip**: - Inject `@PersistenceContext EntityManager em` into `GeschichteServiceIntegrationTest` (it is `@Transactional` at class level, line 35 — tests pass vacuously from the first-level cache without flush/clear) - `@OrderBy` test: insert items **out of position order** (e.g. 3000, 1000, 2000), `em.flush()` + `em.clear()`, then `getById` and assert the list is [1000, 2000, 3000]. In-order insert proves nothing. - Cascade delete test: `delete(id)`, `em.flush()` + `em.clear()`, then `journeyItemRepository.findAllByGeschichteId(id)` returns empty (add `JourneyItemRepository` to `@Autowired` list) - `type` round-trip: STORY → `getType()==STORY`; newly created JOURNEY → `getType()==JOURNEY` - Note-only item (`document_id=null`, `note` populated) persists and returns without NPE — covers the **null arm** of `getDocumentId()` - Document-backed item (`document_id` + `note`) — covers the **present arm** of `getDocumentId()` (both arms needed for the 88% branch gate, per `backend/CLAUDE.md`) - **Document-delete SET-NULL test (round 4)**: persist a document-backed `JourneyItem`, `em.flush()/clear()`, call `documentService.deleteDocument(docId)`, `em.flush()/clear()`, reload the `JourneyItem` and assert `getDocumentId() == null` AND the row still exists. Locks the `ON DELETE SET NULL` "preserves curator prose" decision — this transition is otherwise untested. - **Negative CHECK constraint test (exception-type/flush mechanism reconciled, round 5)**: use `journeyItemRepository.saveAndFlush(bothNull)` (goes through the Spring `@Repository` translation layer) and `assertThatThrownBy(...).isInstanceOf(DataIntegrityViolationException.class)`. If instead flushing via a bare `em.flush()`, the thrown type is Hibernate `org.hibernate.exception.ConstraintViolationException` (wrapped in `PersistenceException`), NOT `DataIntegrityViolationException` — pick `saveAndFlush` to keep the stated type consistent. The constraint fires on **flush**, not on `save()`. Additionally assert the exception message/constraint name contains `chk_journey_item_not_empty` so a stray NOT NULL elsewhere can't make it pass for the wrong reason. - [ ] **HTTP-layer serialization test (round 4 — the single most important new test)**: a dedicated `@SpringBootTest(webEnvironment = RANDOM_PORT)` class (NOT class-level `@Transactional`, so the session closes before serialization like production; add class comment `// NOT @Transactional — must serialize after commit to reproduce open-in-view:false lazy-init 500.`) creates a JOURNEY with items, commits, then over HTTP: - `GET /api/geschichten/{id}` asserts `$.items` present and ordered, `$.items[0].documentId` exists, `$.items[0].document` `doesNotExist()`, `$.type` exists - **Interlude assertion (round 5):** the same journey has one document-backed + one interlude item; assert `$.items[1].documentId` is null/absent AND `$.items[1].note` present (the frontend stub depends on the API emitting a null-documentId item) - `GET /api/geschichten` (grid) asserts **200 AND `$[0].author` exists AND `$[0].body` exists AND `$[0].items` `doesNotExist()`** (round 5 — a bare 200 passes while the card goes blank if the projection drops author/body) - [ ] **DRAFT-clamp authz regression test (round 5 — security)**: as a READ_ALL-only (no `BLOG_WRITE`) user, create a DRAFT JOURNEY; `GET /api/geschichten` asserts the draft is **absent** from the grid; `GET /api/geschichten/{id}` of that draft asserts **404**. Locks the projection-rewrite carryover of the status clamp. - [ ] `getById_returns_type_field_in_response()` in `GeschichteControllerTest`: the `$.items[0].document` `doesNotExist()` assertion is the regression lock on `@JsonIgnore` (Lombok `@Data` generates `getDocument()`; verify it does not leak). (Note: this `@WebMvcTest`-style assertion is the contract lock; the `RANDOM_PORT` test above is the lazy-init lock — both are needed.) - [ ] Migration data-path AC reworded to its real verification method: "Migration DDL verified by Testcontainers boot test in CI; data path (`INSERT...SELECT`) verified manually against a **production `pg_dump` restored into a throwaway `postgres:16-alpine`** before production deploy, documented in PR." Measurable form: **post-migration `COUNT(journey_items)` equals pre-migration `COUNT(DISTINCT (geschichte_id, document_id))` from the junction; positions are strictly increasing multiples of 1000 within each `geschichte_id`; zero rows with both `document_id` and `note` null.** Paste this query output into the PR (checklist item, not prose). - [ ] Testcontainers/full-context boot test runs in CI as the mapping gate — a `@OrderBy`/column-name mismatch between V72 and the entity fails in CI, never crash-loops production after the schema is already migrated - [ ] `npm run generate:api` run after entity changes — TypeScript types regenerated (`Geschichte.documents` → `Geschichte.items`; new `GeschichteSummary` type for the grid) - [ ] **API contract note**: `GET /api/geschichten/{id}` now returns `items` (with `documentId` UUID per item, not the full document object) instead of `documents`; `GET /api/geschichten` (list) returns `GeschichteSummary` objects without `items`; full reader/editor frontend redesign is the follow-on Lesereisen frontend issue - [ ] Doc updates (PR blockers — not nice-to-haves): - `docs/architecture/db/db-orm.puml` — remove `geschichten_documents`, add `journey_items` with columns and FK relationships - `docs/architecture/db/db-relationships.puml` — update relationship lines - `docs/architecture/c4/l3-backend-*.puml` for the geschichte domain — add the new `journeyitem/` sub-package (per doc-currency: new backend package/module) - `CLAUDE.md` package structure table — add `journeyitem/` sub-package under `geschichte/` - `docs/GLOSSARY.md` — add `GeschichteType`, `JourneyItem`, `Lesereise` with display names in de/en/es (`Lesereise` / `Reading Journey` / `Lectura guiada`) - If an ADR is written for the position/fetch strategy, number it **035** (033 and 034 are taken) and record the LAZY-vs-EAGER decision, the explicit-init-under-open-in-view caveat, the `GeschichteSummary` list-projection split, and the 1000-gap position strategy ## Review Insights ### Decided | Decision | Choice | Rationale | |---|---|---| | **`GeschichteSummary` field set (BLOCKER, round 5 — hit by 4 personas)** | **Exactly `id, title, status, type, author, publishedAt, body`; drop `persons`** | The round-4 draft (`id, title, status, type, publishedAt, persons + any other`) silently dropped the `author` byline and `body` excerpt (both rendered by the live grid card `+page.svelte:131–146`) — a visible regression hidden inside the "fix the 500" change — AND carried `persons`, which the card never reads (an unused association reintroducing query cost). Enumerating the exact set removes the untestable "(and any other fields)" hand-wave. Add `persons` later, in the follow-on grid-redesign issue, only with the card change that consumes it. | | **`GeschichteSummary` query mechanism (round 5)** | **Spring Data interface-based projection + explicit `@Query ... ORDER BY COALESCE(g.publishedAt, g.updatedAt) DESC`** | `GeschichteSpecifications.orderByDisplayDateDesc()` injects `query.orderBy(...)` on the entity root with a COUNT special-case; it does not compose cleanly with a constructor-expression/projection query. An interface projection with an explicit ORDER BY sidesteps the spec entirely on the list path. The DRAFT status-clamp and `hasAllPersons` filter must be carried into this query. | | **DRAFT status-clamp survives the projection rewrite (round 5 — security blocker)** | **Carry `effective` status + `authorId` constraints into the projection query; add a list-path authz test** | `list()` currently clamps non-`BLOG_WRITE` users to `PUBLISHED` (GeschichteService:81/84). A freshly-written projection query that forgets this leaks DRAFT journeys into the public grid — a real authz regression. The new READ_ALL-only test (draft absent from grid + 404 on getById) is the regression lock. | | **`hasAllPersons` retained (round 5)** | **Reimplement the person-filter for the projection query; remove only `hasDocument`** | Person-filtering the grid is shipped behaviour. The issue removed `hasDocument` but was silent on `hasAllPersons`; it must survive. | | **Stub link text for document-backed items (round 5)** | **Generic localized label (`geschichten_journey_letter_link`) + `documentId` in href (Option A)** | The `items` payload exposes only `documentId` (the `document` object is `@JsonIgnore`'d) — no title/date is available. A UUID or empty `<a>` fails WCAG 2.4.4 (Link Purpose) and is unusable for the 60+ audience. Fetching document titles (Option B) pulls reader-redesign work into this data-model issue (scope creep) — that belongs to the follow-on. Add the i18n key this issue. | | **Ship the degraded stub vs hold for the reader follow-on (round 5)** | **Ship now; link follow-on as next-priority blocking dependency (Option A)** | Holding the foundational migration on UI work it shouldn't depend on (Option B) blocks every downstream Lesereisen issue. The regression window (existing STORY pages' related-letters degrade to generic links) is bounded and the follow-on is prioritized + linked so the stub does not become permanent. | | **Negative-CHECK test: exception type vs flush mechanism (round 5)** | **`journeyItemRepository.saveAndFlush(bothNull)` → `DataIntegrityViolationException`** | A bare `em.flush()` throws Hibernate `ConstraintViolationException` (wrapped in `PersistenceException`) — Spring's `DataIntegrityViolationException` translation only happens at the `@Repository` proxy boundary, not on a bare `EntityManager`. `saveAndFlush` keeps the AC's stated type consistent and still fires on flush. Keep the `chk_journey_item_not_empty` message assertion. | | **Grid HTTP test must assert shape, not just 200 (round 5)** | **Assert `$[0].author` + `$[0].body` present AND `$[0].items` absent** | A bare `200` passes even if the projection drops author/body (card goes blank) — the shape assertion is the only test that catches the projection field-set regression. | | **Interlude item serialized over the wire (round 5)** | **Add a null-`documentId` + present-`note` assertion to the RANDOM_PORT getById test** | The frontend stub depends on the API actually emitting a null-documentId interlude item; the service-layer arm-coverage tests don't prove the HTTP contract. | | **Deploy-checklist env-var quoting (round 5)** | **Wrap `psql`/`pg_dump` in `sh -c '...'` so `$POSTGRES_USER`/`$POSTGRES_DB` expand inside the container** | Run from the host, the vars expand on the host (empty). A copy-paste-fails-at-3am bug in an otherwise solid runbook. | | **Manual data-path verification target (round 5)** | **Restore a production `pg_dump` into a throwaway `postgres:16-alpine`, run Flyway, paste query output** | CI's Testcontainers schema is clean — the `INSERT...SELECT` data path is genuinely untested in CI. Only real production junction data exercises the `document_date ASC NULLS LAST, id ASC` tiebreaker and the `SELECT DISTINCT` dedup; a synthetic fixture lacks the duplicate/null-date edge cases. | | **`note` field comment names CWE-79 (round 5)** | **Prefix the comment with `CWE-79 tripwire:`** | Makes the stored-XSS-latent tripwire greppable; the data is stored unescaped and safety is every future consumer's responsibility (`{note}` auto-escapes; feed/PDF/email do not). | | **`items` fetch strategy + serialization shape (BLOCKER, round 4)** | **LAZY + explicit `getItems().size()` init in `getById()`; `list()` returns a `GeschichteSummary` projection** | Round 3 chose LAZY (correct per ADR-022) but did not account for `application.yaml:15` `open-in-view: false`. Two production 500s result: (1) `@Transactional` alone does NOT hydrate a LAZY collection — Jackson 500s after the session closes on `getById`; (2) `list()` is non-transactional and serializes `getItems()` → 500 the moment any journey exists. Fix: keep LAZY, force-init `items` inside `getById()`'s transaction, and give `list()` a dedicated `GeschichteSummary` that never carries `items`. Rejected: `@JsonIgnore items` + separate `/items` endpoint (extra round-trip); force-init in both paths (reintroduces Cartesian cost). | | **HTTP-layer reader test (round 4)** | **Dedicated `@SpringBootTest(webEnvironment = RANDOM_PORT)` class, no class-level `@Transactional`** | A service-layer or class-`@Transactional` test runs inside its own transaction so LAZY `items` always init — it passes vacuously while production 500s under `open-in-view:false`. Only a real-HTTP test that serializes after the transaction commits reproduces the bug. The single most important new test. | | **Document-delete SET NULL is tested (round 4)** | **Integration test: delete document → `JourneyItem.documentId` null, row survives** | The `ON DELETE SET NULL` "preserves curator prose" contract was decided round 2 but untested. `DocumentService.deleteDocument` does a plain `deleteById`; the test proves the cascade rule fires and the item row is retained. | | **Duplicate `position` within a journey (round 4)** | **Allow, with documented undefined-order semantics for v1** | A `UNIQUE (geschichte_id, position)` constraint forces the editor's drag-reorder to compute distinct positions every move and complicates naive swaps. The 1000-gap seed keeps positions distinct in practice. State in AC + field comment; revisit if the editor needs the hard guarantee. | | **Migration order is a chronological seed, not curator intent (round 4)** | **Document as explicit assumption AS-001** | The old `geschichten_documents` was an unordered Set — no curator order existed. Ordering by `document_date` is a *plausible default* a Lesereise lets curators re-sequence. Prevents a future reviewer treating chronological order as a requirement. | | **`note` length bound (round 4)** | **Pre-register as a follow-on (editor-issue) AC, not this issue** | No write path exists, so no live DoS. When the editor lands, add a DB CHECK `length(note) <= N` mirroring `chk_text_length` (proposed 2000 chars). The column is created here, so the threshold is registered now to avoid drift. | | **DRAFT JOURNEY non-disclosure (round 4)** | **Follow-on reader MUST route through `getById()`** | `getById()` returns NOT_FOUND (not FORBIDDEN) for DRAFT without `BLOG_WRITE`, to avoid leaking existence. A DRAFT JOURNEY inherits this — the follow-on must not add a `/journeys/{id}` endpoint that skips the check. | | `journey_items.document_id` ON DELETE | `SET NULL` | Preserves curator prose when a document is deleted; a note-only item is still meaningful. `CASCADE` would destroy editorial work; `RESTRICT` blocks archival workflows. | | `note` field format | Plain text (v1) | Safer rendering (`{note}`, not `{@html}`), no sanitization pipeline. Field comment (now CWE-79-tagged) warns feed/email/PDF must escape. No unescaped sink exists today. | | Migration position spacing | Multiples of 1000 | Maximises headroom for drag-to-reorder without rebalance. | | Migration ORDER BY tiebreaker | `documents.document_date ASC NULLS LAST, documents.id ASC` | Deterministic position assignment when dates are equal/null. | | Migration duplicate guard | `SELECT DISTINCT (geschichte_id, document_id)` | Guards against duplicate junction rows producing duplicate journey items. | | `JourneyItem` package location | `geschichte/journeyitem/` sub-package | Consistent with `document/transcription/` and `document/annotation/`. | | API contract change scope | Remove `documents`, add `items` (getById) + `GeschichteSummary` (list); full frontend redesign deferred — but build-breaking `documentId` references AND now-failing frontend tests fixed now | A build-breaking TS error / red `npm run test` cannot ship "known broken." | | `GeschichteSpecifications.hasDocument()` | **Remove (Option A)** | After `geschichten_documents` is dropped, the spec joins a non-existent table → runtime SQL 500. Rewrite via `journey_items` deferred to the editor issue. `hasStatus`/`hasAuthor`/`hasAllPersons` are retained. | | `GeschichteController` `documentId` param | **Remove in this issue** | The service `list()` param is removed, so the controller arg would not compile. | | `JourneyItem.document` serialization | **`@JsonIgnore` + expose `documentId` UUID (Option B)** | No circular reference, no large nested payloads. Locked by a `$.items[0].document doesNotExist()` assertion (Lombok `@Data` generates `getDocument()`). Not an IDOR risk — READ_ALL is global, no per-document ACL; `documentId` leaks no more than `GET /api/documents/{id}` already does. | | Per-field `@Schema` on `JourneyItem` | `id`, `position` REQUIRED; `documentId`, `note` optional | Drives correct TS optionality. | | Migration file strategy | **Single V72 file** | No split-migration precedent; Flyway per-file transaction boundary is atomic. | | Cascade-delete test isolation | **`flush()`+`clear()` inside the existing `@Transactional` class** | Sufficient — `cascade=ALL` + `orphanRemoval` is the mapping guarantee; DB FK cascade is belt-and-suspenders. Avoids cross-test contamination. | | ADR number (if written) | **035** | 033 (`tag-name-resolution`) and 034 (`ollama-deployment`) are taken. | | Spec reference | **`docs/specs/lesereisen-reader-spec.html` + `lesereisen-editor-spec.html`** | The originally-linked `docs/superpowers/specs/...md` does not exist (verified). | | Frontend `[id]` test handling | **Rewrite to `g.items` now** | Cheaper than re-adding coverage later; keeps a live render assertion (incl. interlude-item guard + accessible-link assertion) through the stub period. | | No new `ErrorCode` / `Permission` | **Confirmed (round 5)** | `resolveDocuments` threw `DOCUMENT_NOT_FOUND` via a path still live elsewhere — enum value not orphaned. No `docs/ARCHITECTURE.md` permission-section update. | | `StatsService` impact | **None (round 5)** | The only external caller uses `countPublished()` → `count(spec)`, never `items`/projection. Safe. | ### Risks to watch - **`GeschichteSummary` field-set regression is the highest-leverage round-5 finding** — four personas independently hit it. Shipping the round-4 draft list silently strips the author byline + body excerpt off every story card (a visible regression hidden in the 500-fix) and carries an unused `persons` association. The grid HTTP test's `$[0].author`/`$[0].body` present + `$[0].items` absent assertions are the only gate that catches it. - **DRAFT status-clamp must be re-applied in the projection query** — a freshly-written projection that forgets `effective`/`authorId` (GeschichteService:81/84) leaks DRAFT journeys into the public grid. Authz regression, not hypothetical. The READ_ALL-only list-path test is the lock. - **`open-in-view: false`** turns the round-3 LAZY design into two guaranteed production 500s unless `getById()` explicitly inits `items` AND `list()` uses the `GeschichteSummary` projection. Service-layer/`@Transactional` tests CANNOT see this; the dedicated `RANDOM_PORT` HTTP test is the only gate. - **`items` is the first `List` bag on `Geschichte`** — adding a second EAGER/fetch-joined `List` throws `MultipleBagFetchException` at boot. Field comment documents this. If a future maintainer reverts to EAGER, add a test asserting `list()` returns no duplicate Geschichte rows. - **Integration tests are false-green without `flush()`+`clear()`**: the class is `@Transactional`. `@OrderBy` must insert out-of-order; cascade delete, document-delete SET NULL, and the negative CHECK must flush to hit the DB. The CHECK fires on **flush**, not `save()`. - **Negative-CHECK test exception type depends on the flush mechanism** — `saveAndFlush` → `DataIntegrityViolationException` (Spring translation); bare `em.flush()` → Hibernate `ConstraintViolationException`. Pick `saveAndFlush` to match the AC's stated type. - **`@JsonIgnore` regression risk**: Lombok `@Data` generates `getDocument()`; Jackson serializes by getter, but honours field-level `@JsonIgnore`. Lock with `$.items[0].document doesNotExist()`. - **Frontend breaks AND degrades** — all four sites: `params.query.documentId` compile error in two `+page.server.ts` files; `+page.svelte:14` `hasFilters` derived reading `data.documentFilter` (real compile error); `+page.svelte:19` dead `searchParams.delete` (cleanup); two `[id]/page.svelte.test.ts` tests fail at runtime (rewrite to `g.items`). `list_filters_by_documentId` in `GeschichteServiceTest` must be DELETED outright (removed param) and all `list(...)` call sites drop the positional arg. - **STORY pages degrade, not only JOURNEY UI**: `/geschichten/[id]/+page.svelte` renders related-letters for *all* Geschichten — existing published stories lose rich title/date links (degrade to generic "Open letter" links) until the follow-on. The "start a story pre-linked to this document" entry point also dies until the editor restores it. Accepted, signed-off, follow-on linked. - **Stub link must have accessible text** — the `items` payload has no title/date (only `documentId`); a bare-UUID or empty `<a>` fails WCAG 2.4.4. Use the localized label; assert via `getByRole('link', { name })`. - **Interlude (note-only) items are first-class** (the reader spec uses "interlude" 20×) — the stub must guard `item.documentId` for null AND apply `whitespace-pre-line` to the note `<p>` (one class) so hand-authored multi-paragraph notes (creatable via the editor while the stub is live) don't render run-on. Follow-on reader must use `whitespace-pre-line` + 16px-minimum body for the 60+ audience. - **Deploy-checklist env-var expansion** — `$POSTGRES_USER`/`$POSTGRES_DB` only exist inside the container; wrap in `sh -c '...'` or the one-liners fail at the host shell. - **Healthcheck/`depends_on: service_healthy` gate** — a V72/entity column mismatch makes the new image fail Hibernate `ddl-auto: validate` on first boot (after the schema migrated); without a gate the proxy may route to a restart-looping container → 502s instead of a clean failed deploy. PR must paste the actual `healthcheck:` block + proxy `depends_on` as evidence, or flag its absence as a pre-existing gap (do not block this issue). - **Migration data path untested in CI** — clean Testcontainers schema has no pre-migration data. Mitigation: restore a production `pg_dump` into a throwaway `postgres:16-alpine`, run Flyway, paste row-count equality + position spacing + no-double-null query output into the PR. A synthetic fixture lacks the real duplicate/null-date edge cases. - **Irreversible migration**: `DROP TABLE geschichten_documents` is the only consequential DDL — `INSERT...SELECT` is a no-op if production has no journeys-with-documents. Take the `pg_dump` anyway. Reverse-reconstruction SQL is in the migration comment. Run the pre-deploy `COUNT` to know whether the data path is live. - **`note` is a NEW unescaped serialization sink (CWE-79 latent)** — `Geschichte.body` is sanitized on write; `JourneyItem.note` is serialized raw. Safe for `{note}` (auto-escaped); feed/PDF/email are future surfaces that MUST escape. The CWE-79-tagged field comment is the only tripwire. - **Security boundary must survive the migration**: `JourneyItem.documentId` writes (editor issue) must resolve through `DocumentService.getDocumentById`, never blind-persist a client UUID. No per-document ACL today (READ_ALL global), so pattern-hygiene. Migration's `INSERT...SELECT` copies `document_id` from an existing junction row — no orphan-FK/TOCTOU risk (a junction row can't reference a non-existent document). - **Negative CHECK + SET NULL tests must hit real Postgres** (Testcontainers `postgres:16-alpine`) — H2 lacks both; Mockito does not exercise DB constraints. - **`persons` collection stays on `Geschichte`** — the `Set<Person>` ManyToMany is intentionally NOT removed; a JOURNEY carries a denormalized `persons` set alongside its `items`. With the `GeschichteSummary` projection the grid no longer Cartesian-joins `persons` × `items`. Document the dual-collection shape in the sub-package README if it confuses. - **Position gap exhaustion** (future): the reorder/rebalance strategy when adjacent positions fill is deferred to the Journey Editor issue — do not invent an ad-hoc strategy here. ### Decided (round 6) | Decision | Choice | Rationale | |---|---|---| | **`GeschichteSummary.author` grain (BLOCKER, round 6 — raised by Markus, seconded Felix + Elicit)** | **Nested closed projection: `getAuthor()` returns an interface exposing exactly `firstName`, `lastName`, `email`** | The live grid card (`+page.svelte:41` `authorName(g)`) reads `g.author.firstName / .lastName / .email` — a nested `AppUser`-shaped object. A flat `String getAuthor()` or `UUID getAuthorId()` satisfies the AC's literal "carries author" text AND passes `npm run check` (the card types `author?` loosely), yet renders an **empty byline at runtime** and the round-5 `$[0].author exists` assertion *passes anyway*. Return a **nested closed projection** — NOT a flat string and NOT the full `AppUser` entity (which would re-expose the password hash and re-introduce a serialization surface). Minimal shape that keeps the byline and avoids over-exposure; costs one join. **The HTTP grid test MUST assert `$[0].author.email` (or `.firstName`) present — not bare `$[0].author`** — so the nested-shape regression fails at the test layer. Copy the exact inline type the card already declares at `+page.svelte:41` (`{ firstName?; lastName?; email }`). | | **Stub link duplicate accessible-name (round 6 — raised by Leonie)** | **Add `aria-label` with the item's 1-based position ("Open letter 1", "Open letter 2"…) — Option B** | N identical "Open letter" links pointing to different documents is a WCAG 2.4.4-in-context failure: SR users hear "Open letter, Open letter, Open letter." Option B is ~2 lines, no design change, gives each link a distinct accessible name, and removes the only hard a11y failure in the stub. Cheaper than restoring titles (that is reader-redesign scope). The visible label stays `geschichten_journey_letter_link`; the `aria-label` carries the position suffix. | | **File the follow-on reader/editor issue BEFORE merge (round 6 — raised by Elicit, seconded Leonie)** | **File + link it now (Option A) — promote to Definition of Ready** | The "bounded regression window" claim is only enforceable if the follow-on exists as a numbered, linked Gitea issue carrying a "restore per-item title + date + journey heading" AC. Merging first and filing after (Option B) leaves the "temporary" stub with no tracked owner — the exact failure mode this issue's own risk list warns against. ~15 min cost. **DoR: the follow-on reader/editor issue must exist, be linked `relates-to`/blocks, and carry the restore-title+date+heading AC before this issue merges.** | | **ADR-022 citation collision (round 6 — Markus)** | **Cite the fetch-strategy ADR by FILENAME, not number, in the `Geschichte.items` inline comment** | `docs/adr/` contains TWO 022 files: `022-csrf-session-revocation-rate-limiting.md` AND `022-eager-to-lazy-fetch-strategy.md`. The inline comment must read `// LAZY per docs/adr/022-eager-to-lazy-fetch-strategy.md` so a future maintainer is not sent to the CSRF doc. ADR-035 (if written) should note: extends `022-eager-to-lazy`, supersedes nothing; records that `items` is the first `List` bag on `Geschichte` (the `MultipleBagFetchException` tripwire) as a *consequence*; and notes the `journeys_items.document_id ON DELETE SET NULL` follows the existing `geschichten.author_id ON DELETE SET NULL` (V58→V60) house pattern, so it reads as continuation not invention. | | **`hasAllPersons` AND-semantics regression lock (round 6 — Sara)** | **Add a `@DataJpaTest` person-filter AND-semantics test (story{A,B} vs story{A}, filter `?personId=A&personId=B` → only {A,B})** | The current `hasAllPersons` is "one EXISTS subquery per id" Criteria-API code that does NOT survive into a JPQL projection query for free. A naive `JOIN g.persons p WHERE p.id IN :ids` gives **OR** semantics (matches any), silently widening the filter — and no existing test asserts AND, so an AND→OR regression passes CI today. The JPQL must use a counting subquery (e.g. `(SELECT COUNT(DISTINCT p.id) FROM g.persons p WHERE p.id IN :personIds) = :personCount`). The AND-semantics test against real Postgres is the single missing regression lock. | | **`list()` test split mechanism (round 6 — Sara)** | **Split the AC "list() tests updated" item into (a) Mockito unit test asserting the repo projection method is invoked with the clamped `effective` status + `authorId`, and (b) a `@DataJpaTest` (Testcontainers Postgres) asserting clamp + COALESCE ordering + person-AND against real rows** | An interface projection bound to a `@Query` is only testable via real Postgres (`@DataJpaTest`/Testcontainers), not Mockito. The six surviving `list(...)` Mockito tests (lines 134/148/161/174/186/198) can only assert the repo method is called with the clamped status; the security-critical clamp/ordering/AND behavior moves to `@DataJpaTest` with real data. Also grep those six tests for `.getPersons(`/`.getDocuments(` on the `list()` result — `GeschichteSummary` has no `persons`, so any such assertion breaks beyond the arity change and must convert or move to `@DataJpaTest`. | | **DRAFT-clamp grid test asserts specific id, not count (round 6 — Nora)** | **Assert the specific DRAFT id is ABSENT from the projection result and the PUBLISHED id is PRESENT — not list size** | The genuinely new authz surface is the projection query's `WHERE` clause (getById's NOT_FOUND guard at `GeschichteService:67` is untouched). Asserting list size alone is weak — a draft could be absent for unrelated reasons. Create one PUBLISHED + one DRAFT, run the grid as a non-`BLOG_WRITE` user, assert published id present AND draft id absent. | | **Reader stub: delete `documentDate` span + `formatDate` import (round 6 — Felix)** | **In `[id]/+page.svelte` delete the `formatDate(d.documentDate)` span (lines 113–117) and drop the `formatDate` import if now unused** | The current card renders `{d.title}` AND `formatDate(d.documentDate)`. Post-regen `item.documentDate` does not exist on the projection — it is a TS compile error, not just dead code. Rewriting only the `{#each}` is insufficient; the date span and (if orphaned) the import must go. Keep the `geschichten_documents_section` `<h2>` heading — it is still the section heading for the stub period; do NOT delete the `geschichten_documents_section` i18n key (one usage, `[id]/+page.svelte:103`). The follow-on reader replaces the heading. | | **Deploy-checklist count reconciliation (round 6 — Tobias)** | **Pre-check and smoke-check both use `COUNT(DISTINCT (geschichte_id, document_id))` on the source; assertion is "`COUNT(journey_items)` ≤ source, any shortfall explained by the `SELECT DISTINCT` dedup" — NOT strict equality** | The migration dedups via `SELECT DISTINCT (geschichte_id, document_id)`. If production has duplicate junction rows, `COUNT(journey_items) < COUNT(*)` and a strict-equality smoke check FAILS on a correct migration. Step-1 pre-check must be `COUNT(DISTINCT ...)` (not `COUNT(*)`); the runbook must say "≤, shortfall = dedup worked," not "must equal." Reconciles the runbook with the measurable AC. | | **Pre-migration backup table list (round 6 — Tobias)** | **Pre-migration `pg_dump` dumps ONLY `--table=geschichten_documents` — drop `--table=journey_items`** | `journey_items` does not exist pre-migration (V72 creates it), so `pg_dump --table=journey_items` warns/errors on a missing table. The post-migration table needs no pre-backup; it is reconstructable from the dump + reverse SQL. (Update the migration-comment `pg_dump` one-liner accordingly.) | | **Throwaway-container verification dump currency (round 6 — Tobias)** | **The production `pg_dump` restored into the throwaway `postgres:16-alpine` must be schema-current (≥V71)** | A historical dump makes Flyway replay V60's `ALTER TABLE users RENAME TO app_users` against an already-renamed schema and fail. Use a full current dump so Flyway resumes at V72. | | **Reverse-procedure FK semantics caveat (round 6 — Nora)** | **Add to the reverse-procedure migration comment: the reconstructed `geschichten_documents.document_id` FK is `ON DELETE CASCADE` per the original V58 (NOT the new `SET NULL` of `journey_items`); and post-V60 the domain FKs target `app_users`, not `users`** | A careless reconstructor copying the new table's `SET NULL` rule, or hand-typing V58's verbatim `REFERENCES users` DDL, would silently change delete behavior or fail on the renamed table. The captured dump definition already says `app_users`; the comment must warn against hand-typing the old V58 text and against copying the new SET-NULL semantics into the junction. | | **AS-002 — existing-STORY visible degradation logged (round 6 — Elicit)** | **Add AS-002 to the assumptions register, mirroring AS-001** | "Existing published Geschichten (STORYs) render the related-letters block; this block visibly degrades to generic links (loss of per-document title AND date) for ALL current readers during the stub window. Accepted because the reader follow-on is the next-priority blocking dependency." Converts the implicit sign-off into a traceable assumption a future reviewer can challenge. The date-scent loss (not just the title) is the real senior-UX cost — the follow-on's restore AC must cover title AND date. | | **`frontend/src/lib/geschichte/README.md` doc-currency (round 6 — Markus)** | **Grep it before merge; if it references `documents` (the entity-shape change `documents`→`items`), it is a doc blocker** | Not in the issue's enumerated doc list, but a Geschichte frontend domain README exists and may document the card/editor contract. Same doc-currency rule that flags the backend READMEs. | ### Additional AC items (round 6) - [ ] **`GeschichteSummary.getAuthor()` is a nested closed projection** exposing exactly `firstName`, `lastName`, `email` (NOT a flat string, NOT the full `AppUser`); the grid HTTP test asserts `$[0].author.email` (or `.firstName`) present — not bare `$[0].author` - [ ] **Stub document-backed link carries an `aria-label`** with the item's 1-based position (distinct accessible names; WCAG 2.4.4), preserves the `block px-4 py-3` (≥44px) touch target; interlude note paragraph uses `<p class="whitespace-pre-line text-ink-2">` (explicit text token for dark-mode contrast) - [ ] **Follow-on reader/editor issue filed + linked (`relates-to`/blocks) with a "restore per-item title + date + journey heading" AC BEFORE this merges** (Definition of Ready) - [ ] **`Geschichte.items` inline comment cites `docs/adr/022-eager-to-lazy-fetch-strategy.md` by FILENAME** (not "ADR-022" — collides with the CSRF 022) - [ ] **AND-semantics person-filter `@DataJpaTest`**: story{A,B} + story{A}; `?personId=A&personId=B` returns exactly {A,B} (locks `hasAllPersons` AND semantics through the projection rewrite; JPQL uses a counting subquery, not a naive `IN` join) - [ ] **`list()` test split**: (a) Mockito — repo projection method invoked with clamped `effective` status + `authorId`; (b) `@DataJpaTest` (Testcontainers Postgres) — clamp + COALESCE ordering + person-AND against real rows; six surviving Mockito `list(...)` tests grepped for `.getPersons(`/`.getDocuments(` on the result and converted/moved - [ ] **DRAFT-clamp grid test asserts the specific draft id is absent + published id present** (not list size) - [ ] **`[id]/+page.svelte`: `formatDate(d.documentDate)` span deleted; `formatDate` import dropped if orphaned**; `geschichten_documents_section` heading + i18n key retained for the stub period - [ ] **Deploy checklist**: pre-check + smoke use `COUNT(DISTINCT (geschichte_id, document_id))`; assertion is "≤, shortfall = dedup"; pre-migration `pg_dump` dumps only `geschichten_documents`; throwaway-container verification uses a schema-current (≥V71) production dump - [ ] **Reverse-procedure migration comment** warns: reconstructed junction FK is `ON DELETE CASCADE` (V58), domain FKs target `app_users` (post-V60) — do not hand-type V58's `REFERENCES users` nor copy `journey_items`' SET-NULL into the junction - [ ] **AS-002 logged** in the assumptions register (existing-STORY related-letters degrade for all current readers during the stub window) - [ ] **`frontend/src/lib/geschichte/README.md` grepped**; updated if it references `documents` (doc blocker) ### Risks to watch (round 6 additions) - **`GeschichteSummary.author` flat-vs-nested is a second hidden byline regression** — even with the field *named*, a flat `String`/`UUID author` passes `npm run check` and the round-5 `$[0].author exists` assertion while the byline renders empty. Only a nested closed projection + an `$[0].author.email` (not bare `$[0].author`) assertion catches it. - **`hasAllPersons` AND→OR silent widening** — rewriting the EXISTS-per-id Criteria filter as a naive `JOIN ... WHERE p.id IN :ids` in JPQL gives OR semantics; no existing test asserts AND, so the regression passes CI. Lock with the {A,B}-vs-{A} `@DataJpaTest`. - **Interface-projection `@Query` is `@DataJpaTest`-only** — the clamp/ordering/AND logic cannot be Mockito-tested; the security-critical clamp must be verified against real Postgres rows, not just "repo method called." - **Deploy runbook self-contradiction** — strict `COUNT(*)` equality FAILS on a correct dedup migration if production has duplicate junction rows; use `COUNT(DISTINCT ...)` and "≤" everywhere. - **Pre-migration `pg_dump --table=journey_items` errors** — the table doesn't exist yet; dump only `geschichten_documents` pre-migration. - **ADR-022 number collides with the CSRF ADR** — cite the fetch-strategy ADR by filename in the inline comment. - **The stub's date-scent loss (not just title) is the real senior-UX cost** — for a 60+ reader scanning chronological correspondence the date was the primary wayfinding cue; N identical "Open letter" rows turn a scannable list into noise. AS-002 + the follow-on's restore-title-AND-date AC make the regression provably temporary.
marcel added this to the Lesereisen (Reading Journeys) milestone 2026-06-06 16:06:57 +02:00
marcel added the P1-highfeature labels 2026-06-06 16:08:05 +02:00
Author
Owner

Implemented

Branch: feat/issue-750-lesereisen-data-model

Commits

  • b3ce9b93feat(geschichte): add GeschichteType, JourneyItem entity, GeschichteSummary, and V72 migration
  • 439385ddrefactor(geschichte): update service, controller, repository for JourneyItem model
  • 93ef2669test(geschichte): add JourneyItemIntegrationTest, GeschichteListProjectionTest, GeschichteHttpTest; update unit tests
  • e6c890c6feat(frontend): update generated API types and Geschichte routes for JourneyItem model
  • ccdf358btest(frontend): fix Geschichte component specs for GeschichteType and JourneyItem model
  • 13fa4123docs: update ARCHITECTURE, GLOSSARY, db diagram for JourneyItem and Lesereise

What was done

Backend:

  • GeschichteType enum {STORY, JOURNEY}; default STORY on all existing Geschichten
  • JourneyItem entity (journey_items table) with @OrderBy("position ASC"), CascadeType.ALL, orphanRemoval=true; CHECK constraint; @JsonIgnore on document; getDocumentId() getter
  • GeschichteSummary interface projection for list() — avoids LazyInitializationException in open-in-view=false
  • V72 Flyway migration: adds type column, creates journey_items, migrates geschichten_documents ordered by meta_date, drops junction table
  • GeschichteService: list() returns List<GeschichteSummary> via JPQL; getById() is @Transactional(readOnly=true) with Hibernate.initialize(g.getItems())
  • GeschichteController: removed documentId param, returns GeschichteSummary[]
  • 58 backend tests passing: JourneyItemIntegrationTest (9), GeschichteListProjectionTest (8), GeschichteHttpTest (5), service/controller unit tests

Frontend:

  • api.ts: GeschichteSummary, JourneyItem, GeschichteType schemas; list response changed; Geschichte gains type + items; documentIds removed from DTO
  • GeschichteEditor: document picker removed (journey editing is follow-on)
  • geschichten/[id]/+page.svelte: document list replaced with JourneyItem stub block (document-backed items link to /documents/{documentId})
  • geschichten/+page.server.ts: documentId filter removed
  • Component specs fixed for new model

Follow-on

Filed as separate issue linked to this one (reader UI next).

## Implemented ✅ Branch: `feat/issue-750-lesereisen-data-model` ### Commits - `b3ce9b93` — `feat(geschichte): add GeschichteType, JourneyItem entity, GeschichteSummary, and V72 migration` - `439385dd` — `refactor(geschichte): update service, controller, repository for JourneyItem model` - `93ef2669` — `test(geschichte): add JourneyItemIntegrationTest, GeschichteListProjectionTest, GeschichteHttpTest; update unit tests` - `e6c890c6` — `feat(frontend): update generated API types and Geschichte routes for JourneyItem model` - `ccdf358b` — `test(frontend): fix Geschichte component specs for GeschichteType and JourneyItem model` - `13fa4123` — `docs: update ARCHITECTURE, GLOSSARY, db diagram for JourneyItem and Lesereise` ### What was done **Backend:** - `GeschichteType` enum `{STORY, JOURNEY}`; default `STORY` on all existing Geschichten - `JourneyItem` entity (`journey_items` table) with `@OrderBy("position ASC")`, `CascadeType.ALL`, `orphanRemoval=true`; CHECK constraint; `@JsonIgnore` on `document`; `getDocumentId()` getter - `GeschichteSummary` interface projection for list() — avoids LazyInitializationException in open-in-view=false - V72 Flyway migration: adds `type` column, creates `journey_items`, migrates `geschichten_documents` ordered by `meta_date`, drops junction table - `GeschichteService`: `list()` returns `List<GeschichteSummary>` via JPQL; `getById()` is `@Transactional(readOnly=true)` with `Hibernate.initialize(g.getItems())` - `GeschichteController`: removed `documentId` param, returns `GeschichteSummary[]` - 58 backend tests passing: JourneyItemIntegrationTest (9), GeschichteListProjectionTest (8), GeschichteHttpTest (5), service/controller unit tests **Frontend:** - `api.ts`: `GeschichteSummary`, `JourneyItem`, `GeschichteType` schemas; list response changed; `Geschichte` gains `type` + `items`; `documentIds` removed from DTO - `GeschichteEditor`: document picker removed (journey editing is follow-on) - `geschichten/[id]/+page.svelte`: document list replaced with JourneyItem stub block (document-backed items link to `/documents/{documentId}`) - `geschichten/+page.server.ts`: `documentId` filter removed - Component specs fixed for new model ### Follow-on Filed as separate issue linked to this one (reader UI next).
Sign in to join this conversation.
No Label P1-high feature
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#750