From 0b177247859f17c3420b07cd7392d8ed2ce88912 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 16:26:26 +0200 Subject: [PATCH 01/41] feat(config): add jackson-databind-nullable for JsonNullable PATCH DTO support Registers JsonNullableModule globally so JsonNullable in JourneyItemUpdateDTO can distinguish absent (unchanged) from explicit null (clear field) on PATCH operations. Co-Authored-By: Claude Sonnet 4.6 --- backend/pom.xml | 7 +++++++ .../familienarchiv/config/JacksonConfig.java | 16 ++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java diff --git a/backend/pom.xml b/backend/pom.xml index b01f2362..30fa0719 100644 --- a/backend/pom.xml +++ b/backend/pom.xml @@ -253,6 +253,13 @@ 1.18.1 + + + org.openapitools + jackson-databind-nullable + 0.2.6 + + io.micrometer diff --git a/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java b/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java new file mode 100644 index 00000000..ca027548 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java @@ -0,0 +1,16 @@ +package org.raddatz.familienarchiv.config; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.openapitools.jackson.nullable.JsonNullableModule; +import org.springframework.boot.autoconfigure.jackson.Jackson2ObjectMapperBuilderCustomizer; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class JacksonConfig { + + @Bean + public Jackson2ObjectMapperBuilderCustomizer jsonNullableModule() { + return builder -> builder.modulesToInstall(new JsonNullableModule()); + } +} -- 2.49.1 From 408ae3345c39691fa46f76b9c8f3e656c60f2c37 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 16:27:15 +0200 Subject: [PATCH 02/41] feat(audit,error): add JourneyItem AuditKind values and ErrorCodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds JOURNEY_ITEM_ADDED, JOURNEY_ITEM_REMOVED, JOURNEY_ITEMS_REORDERED (last is ROLLUP_ELIGIBLE — drag-heavy editing produces many events). Adds JOURNEY_ITEM_NOT_FOUND (404) and JOURNEY_ITEM_POSITION_CONFLICT (409) to ErrorCode for IDOR protection and concurrent-edit feedback. Co-Authored-By: Claude Sonnet 4.6 --- .../raddatz/familienarchiv/audit/AuditKind.java | 16 ++++++++++++++-- .../familienarchiv/exception/ErrorCode.java | 4 ++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java index 62f04874..659de3c3 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java @@ -50,10 +50,22 @@ public enum AuditKind { ADMIN_FORCE_LOGOUT, /** Payload: {@code {"ip": "1.2.3.4", "email": "addr"}} — password NEVER included */ - LOGIN_RATE_LIMITED; + LOGIN_RATE_LIMITED, + + // --- Reading Journeys (Lesereisen) --- + + /** Payload: {@code {"geschichteId": "uuid", "itemId": "uuid"}} — documentId is null (journey-scoped, not document-scoped) */ + JOURNEY_ITEM_ADDED, + + /** Payload: {@code {"geschichteId": "uuid", "itemId": "uuid"}} — documentId is null */ + JOURNEY_ITEM_REMOVED, + + /** Payload: {@code {"geschichteId": "uuid", "itemCount": 3}} — documentId is null; rolled up in chronik */ + JOURNEY_ITEMS_REORDERED; public static final Set ROLLUP_ELIGIBLE = Set.of( TEXT_SAVED, FILE_UPLOADED, ANNOTATION_CREATED, - BLOCK_REVIEWED, COMMENT_ADDED, MENTION_CREATED + BLOCK_REVIEWED, COMMENT_ADDED, MENTION_CREATED, + JOURNEY_ITEMS_REORDERED ); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java index 3eb5287d..a6acde3a 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java @@ -122,6 +122,10 @@ public enum ErrorCode { // --- Geschichten (Stories) --- /** A Geschichte (story) with the given ID does not exist, or is a DRAFT and the caller lacks BLOG_WRITE. 404 */ GESCHICHTE_NOT_FOUND, + /** A JourneyItem with the given ID does not exist, or belongs to a different journey (IDOR). 404 */ + JOURNEY_ITEM_NOT_FOUND, + /** A position uniqueness conflict occurred on the journey_items table — concurrent append or reorder. 409 */ + JOURNEY_ITEM_POSITION_CONFLICT, // --- Tags --- /** A tag with the given ID does not exist. 404 */ -- 2.49.1 From 7b06c3adec4311babdec2d9aa8251344d617f7dc Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 16:30:07 +0200 Subject: [PATCH 03/41] feat(migration): V73 adds UNIQUE DEFERRABLE and CHECK position > 0 on journey_items DEFERRABLE INITIALLY DEFERRED allows mid-transaction position swaps during reorder (checked at COMMIT, not per-row). CHECK (position > 0) guards against off-by-one in the append path. Both verified by JourneyItemConstraintsTest via raw pg_constraint query + jdbcTemplate inserts against a real postgres:16-alpine container. Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/config/JacksonConfig.java | 7 +- ...add_journey_items_position_constraints.sql | 19 ++++ .../JourneyItemConstraintsTest.java | 99 +++++++++++++++++++ 3 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 backend/src/main/resources/db/migration/V73__add_journey_items_position_constraints.sql create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemConstraintsTest.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java b/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java index ca027548..5c5e0945 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java @@ -1,8 +1,7 @@ package org.raddatz.familienarchiv.config; -import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.Module; import org.openapitools.jackson.nullable.JsonNullableModule; -import org.springframework.boot.autoconfigure.jackson.Jackson2ObjectMapperBuilderCustomizer; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -10,7 +9,7 @@ import org.springframework.context.annotation.Configuration; public class JacksonConfig { @Bean - public Jackson2ObjectMapperBuilderCustomizer jsonNullableModule() { - return builder -> builder.modulesToInstall(new JsonNullableModule()); + public Module jsonNullableModule() { + return new JsonNullableModule(); } } diff --git a/backend/src/main/resources/db/migration/V73__add_journey_items_position_constraints.sql b/backend/src/main/resources/db/migration/V73__add_journey_items_position_constraints.sql new file mode 100644 index 00000000..76a8af2f --- /dev/null +++ b/backend/src/main/resources/db/migration/V73__add_journey_items_position_constraints.sql @@ -0,0 +1,19 @@ +-- Adds the two constraints that V72 deferred: +-- 1. UNIQUE(geschichte_id, position) DEFERRABLE INITIALLY DEFERRED +-- Allows mid-transaction position swaps during reorder (checked at COMMIT, not per-row). +-- Requires transaction-level or session-level connection pooling (prod uses PgBouncer +-- in transaction mode — correct today; a future switch to statement-level would silently +-- break deferred checking at COMMIT). +-- 2. CHECK (position > 0) — defense against off-by-one in the append path. +-- +-- MUST run in a single transaction; Flyway's default per-migration transaction satisfies this. +-- Do NOT add executeInTransaction=false or any callback that splits this migration. + +ALTER TABLE journey_items + ADD CONSTRAINT uq_journey_items_geschichte_position + UNIQUE (geschichte_id, position) + DEFERRABLE INITIALLY DEFERRED; + +ALTER TABLE journey_items + ADD CONSTRAINT chk_journey_item_position + CHECK (position > 0); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemConstraintsTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemConstraintsTest.java new file mode 100644 index 00000000..a2615003 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemConstraintsTest.java @@ -0,0 +1,99 @@ +package org.raddatz.familienarchiv.geschichte.journeyitem; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.raddatz.familienarchiv.PostgresContainerConfig; +import org.raddatz.familienarchiv.document.Document; +import org.raddatz.familienarchiv.document.DocumentRepository; +import org.raddatz.familienarchiv.document.DocumentStatus; +import org.raddatz.familienarchiv.geschichte.Geschichte; +import org.raddatz.familienarchiv.geschichte.GeschichteRepository; +import org.raddatz.familienarchiv.geschichte.GeschichteStatus; +import org.raddatz.familienarchiv.geschichte.GeschichteType; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.annotation.Import; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import software.amazon.awssdk.services.s3.S3Client; + +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Raw-SQL constraint tests for journey_items — deliberately NOT @Transactional at class level. + * A DataIntegrityViolationException inside a class-level @Transactional marks the tx + * rollback-only and cascades into TransactionSystemException on teardown. + * Each test inserts via jdbcTemplate and uses explicit SQL teardown. + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE) +@ActiveProfiles("test") +@Import(PostgresContainerConfig.class) +class JourneyItemConstraintsTest { + + @MockitoBean + S3Client s3Client; + + @Autowired JdbcTemplate jdbcTemplate; + @Autowired GeschichteRepository geschichteRepository; + @Autowired DocumentRepository documentRepository; + + private UUID geschichteId; + private UUID documentId; + + @BeforeEach + void seed() { + jdbcTemplate.execute("DELETE FROM journey_items"); + Document doc = documentRepository.save(Document.builder() + .title("Constraints-Test-Doc") + .originalFilename("ct.pdf") + .status(DocumentStatus.UPLOADED) + .build()); + documentId = doc.getId(); + Geschichte g = geschichteRepository.save(Geschichte.builder() + .title("Constraints-Test-Journey") + .status(GeschichteStatus.DRAFT) + .type(GeschichteType.JOURNEY) + .build()); + geschichteId = g.getId(); + } + + @Test + void unique_constraint_is_deferrable_initially_deferred() { + Boolean condeferrable = jdbcTemplate.queryForObject( + "SELECT condeferrable FROM pg_constraint WHERE conname = 'uq_journey_items_geschichte_position'", + Boolean.class); + Boolean condeferred = jdbcTemplate.queryForObject( + "SELECT condeferred FROM pg_constraint WHERE conname = 'uq_journey_items_geschichte_position'", + Boolean.class); + assertThat(condeferrable).as("constraint must be deferrable").isTrue(); + assertThat(condeferred).as("constraint must be initially deferred").isTrue(); + } + + @Test + void position_check_rejects_nonpositive() { + UUID itemId = UUID.randomUUID(); + assertThatThrownBy(() -> + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, note) VALUES (?, ?, ?, ?)", + itemId, geschichteId, 0, "test")) + .isInstanceOf(DataIntegrityViolationException.class); + } + + @Test + void unique_constraint_rejects_duplicate_position_per_geschichte() { + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, document_id) VALUES (?, ?, ?, ?)", + UUID.randomUUID(), geschichteId, 10, documentId); + + assertThatThrownBy(() -> + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, document_id) VALUES (?, ?, ?, ?)", + UUID.randomUUID(), geschichteId, 10, documentId)) + .isInstanceOf(DataIntegrityViolationException.class); + } +} -- 2.49.1 From 160ca1c3e99d9f74c60668437c886a290e76755a Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 16:31:40 +0200 Subject: [PATCH 04/41] feat(geschichte): add DocumentSummary, JourneyItemView, GeschichteView read models MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DocumentSummary: lean document projection for journey item embedding — skips tag-color resolution (getSummaryById), includes receiverCount (0 when no receivers, non-null). JourneyItemView: response record for item CRUD and GET. GeschichteView: detail response with summarised author {id, displayName} to prevent AppUser email/group leak. Co-Authored-By: Claude Sonnet 4.6 --- .../document/DocumentService.java | 10 ++++++ .../geschichte/DocumentSummary.java | 23 ++++++++++++ .../geschichte/GeschichteView.java | 36 +++++++++++++++++++ .../journeyitem/JourneyItemView.java | 17 +++++++++ 4 files changed, 86 insertions(+) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/geschichte/DocumentSummary.java create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteView.java create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemView.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java index 4f69922c..624a2a37 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -1006,6 +1006,16 @@ public class DocumentService { return doc; } + /** + * Lean document lookup for embedding in JourneyItemView. Skips + * {@code tagService.resolveEffectiveColors} — ×N items per journey GET is wasted + * work that summary consumers never read. Called within a caller-provided transaction. + */ + public Document getSummaryById(UUID id) { + return documentRepository.findById(id) + .orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id)); + } + /** * Loads a document for the detail view, additionally flagging whether it has any * transcription to read. Kept separate from {@link #getDocumentById} so the cheap diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/DocumentSummary.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/DocumentSummary.java new file mode 100644 index 00000000..151017f7 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/DocumentSummary.java @@ -0,0 +1,23 @@ +package org.raddatz.familienarchiv.geschichte; + +import io.swagger.v3.oas.annotations.media.Schema; +import org.raddatz.familienarchiv.document.DatePrecision; + +import java.time.LocalDate; +import java.util.UUID; + +/** + * Lean read-model view of a Document for embedding in JourneyItemView. + * Built by JourneyItemService.toSummary(Document) — never serialised from + * a JPA entity to avoid LazyInitializationException and tag-color overhead. + */ +public record DocumentSummary( + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) UUID id, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) String title, + LocalDate documentDate, + LocalDate documentDateEnd, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) DatePrecision datePrecision, + String senderName, + String receiverName, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) Integer receiverCount +) {} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteView.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteView.java new file mode 100644 index 00000000..2fa5a920 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteView.java @@ -0,0 +1,36 @@ +package org.raddatz.familienarchiv.geschichte; + +import com.fasterxml.jackson.annotation.JsonInclude; +import io.swagger.v3.oas.annotations.media.Schema; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemView; +import org.raddatz.familienarchiv.person.Person; + +import java.time.LocalDateTime; +import java.util.List; +import java.util.Set; +import java.util.UUID; + +/** + * Detail-view response for GET /api/geschichten/{id}. Assembled by + * GeschichteService — never the raw entity (author AppUser graph must not leak). + * items is always present (both STORY and JOURNEY); empty list for stories with no items. + */ +public record GeschichteView( + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) UUID id, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) String title, + String body, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) GeschichteStatus status, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) GeschichteType type, + AuthorView author, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) Set persons, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) List items, + LocalDateTime publishedAt, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) LocalDateTime createdAt, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) LocalDateTime updatedAt +) { + /** Summarised author — exposes only id and displayName, never email or group memberships. */ + public record AuthorView( + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) UUID id, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) String displayName + ) {} +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemView.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemView.java new file mode 100644 index 00000000..b15dc6b1 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemView.java @@ -0,0 +1,17 @@ +package org.raddatz.familienarchiv.geschichte.journeyitem; + +import io.swagger.v3.oas.annotations.media.Schema; +import org.raddatz.familienarchiv.geschichte.DocumentSummary; + +import java.util.UUID; + +/** + * Read-model response for a JourneyItem. Never the JPA entity (which has a + * Geschichte back-reference that would leak / hit LazyInitializationException). + */ +public record JourneyItemView( + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) UUID id, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) int position, + DocumentSummary document, + String note +) {} -- 2.49.1 From 2ad5c36e3cdb82ff7ea693b62fd7b523897057eb Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 16:32:50 +0200 Subject: [PATCH 05/41] feat(geschichte): extend JourneyItemRepository and add item DTOs Repository: findByIdAndGeschichteId (IDOR-safe lookup), findByGeschichteIdOrderByPosition, findIdsByGeschichteId (Set for set-equality reorder check), findMaxPositionByGeschichteId, countByGeschichteId. DTOs: JourneyItemCreateDTO (documentId+note), JourneyItemUpdateDTO (JsonNullable note for 3-way PATCH), JourneyReorderDTO (List). Co-Authored-By: Claude Sonnet 4.6 --- .../journeyitem/JourneyItemCreateDTO.java | 12 +++++++++++ .../journeyitem/JourneyItemRepository.java | 21 +++++++++++++++++++ .../journeyitem/JourneyItemUpdateDTO.java | 16 ++++++++++++++ .../journeyitem/JourneyReorderDTO.java | 12 +++++++++++ 4 files changed, 61 insertions(+) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemCreateDTO.java create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemUpdateDTO.java create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyReorderDTO.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemCreateDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemCreateDTO.java new file mode 100644 index 00000000..9a7c420d --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemCreateDTO.java @@ -0,0 +1,12 @@ +package org.raddatz.familienarchiv.geschichte.journeyitem; + +import lombok.Data; + +import java.util.UUID; + +/** Input for POST /api/geschichten/{id}/items. Both fields optional; at least one must be present. */ +@Data +public class JourneyItemCreateDTO { + private UUID documentId; + private String note; +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java index 5534195b..4a56ee07 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java @@ -1,13 +1,34 @@ package org.raddatz.familienarchiv.geschichte.journeyitem; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; import java.util.List; +import java.util.Optional; +import java.util.Set; import java.util.UUID; @Repository public interface JourneyItemRepository extends JpaRepository { List findAllByGeschichteId(UUID geschichteId); + + /** Returns items ordered by position ASC for the read-model assembly path. */ + List findByGeschichteIdOrderByPosition(UUID geschichteId); + + /** IDOR-safe lookup: returns empty when itemId exists but belongs to a different journey. */ + Optional findByIdAndGeschichteId(UUID id, UUID geschichteId); + + /** Returns only the IDs — used for set-equality check in reorder. */ + @Query("SELECT i.id FROM JourneyItem i WHERE i.geschichte.id = :geschichteId") + Set findIdsByGeschichteId(@Param("geschichteId") UUID geschichteId); + + /** MAX position for computing the next append position; returns empty when journey has no items. */ + @Query("SELECT MAX(i.position) FROM JourneyItem i WHERE i.geschichte.id = :geschichteId") + Optional findMaxPositionByGeschichteId(@Param("geschichteId") UUID geschichteId); + + /** COUNT for the 100-item cap check — COUNT(*)-based, never MAX(position)-derived. */ + long countByGeschichteId(UUID geschichteId); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemUpdateDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemUpdateDTO.java new file mode 100644 index 00000000..db8c4e6e --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemUpdateDTO.java @@ -0,0 +1,16 @@ +package org.raddatz.familienarchiv.geschichte.journeyitem; + +import lombok.Data; +import org.openapitools.jackson.nullable.JsonNullable; + +/** + * Input for PATCH /api/geschichten/{id}/items/{itemId}. + * JsonNullable enables three-way semantics: + * absent → field not present in JSON → leave unchanged + * null → {"note": null} → clear the note field + * string → {"note": "text"} → set the note + */ +@Data +public class JourneyItemUpdateDTO { + private JsonNullable note = JsonNullable.undefined(); +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyReorderDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyReorderDTO.java new file mode 100644 index 00000000..b11f343b --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyReorderDTO.java @@ -0,0 +1,12 @@ +package org.raddatz.familienarchiv.geschichte.journeyitem; + +import lombok.Data; + +import java.util.List; +import java.util.UUID; + +/** Input for PUT /api/geschichten/{id}/items/reorder. */ +@Data +public class JourneyReorderDTO { + private List itemIds; +} -- 2.49.1 From fdc9273c860f58476a3084dc9903b01ac07740b3 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 16:47:42 +0200 Subject: [PATCH 06/41] =?UTF-8?q?feat(geschichte):=20implement=20JourneyIt?= =?UTF-8?q?emService=20=E2=80=94=20append,=20updateNote,=20delete,=20reord?= =?UTF-8?q?er=20(35=20unit=20tests)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- .../journeyitem/JourneyItemService.java | 242 +++++++ .../journeyitem/JourneyItemServiceTest.java | 629 ++++++++++++++++++ 2 files changed, 871 insertions(+) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java new file mode 100644 index 00000000..6b1923f8 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -0,0 +1,242 @@ +package org.raddatz.familienarchiv.geschichte.journeyitem; + +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.openapitools.jackson.nullable.JsonNullable; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; +import org.raddatz.familienarchiv.document.DatePrecision; +import org.raddatz.familienarchiv.document.Document; +import org.raddatz.familienarchiv.document.DocumentService; +import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.exception.ErrorCode; +import org.raddatz.familienarchiv.geschichte.Geschichte; +import org.raddatz.familienarchiv.geschichte.GeschichteRepository; +import org.raddatz.familienarchiv.geschichte.GeschichteType; +import org.raddatz.familienarchiv.geschichte.DocumentSummary; +import org.raddatz.familienarchiv.person.Person; +import org.raddatz.familienarchiv.user.AppUser; +import org.raddatz.familienarchiv.user.UserService; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import java.util.*; + +@Service +@RequiredArgsConstructor +@Slf4j +public class JourneyItemService { + + static final int MAX_ITEMS = 100; + static final int POSITION_STEP = 10; + static final int MAX_NOTE_LENGTH = 5000; + + private final JourneyItemRepository journeyItemRepository; + private final GeschichteRepository geschichteRepository; + private final DocumentService documentService; + private final AuditService auditService; + private final UserService userService; + + @Transactional + public JourneyItemView append(UUID geschichteId, JourneyItemCreateDTO dto) { + Geschichte g = geschichteRepository.findById(geschichteId) + .orElseThrow(() -> DomainException.notFound(ErrorCode.GESCHICHTE_NOT_FOUND, + "Journey not found: " + geschichteId)); + + if (g.getType() != GeschichteType.JOURNEY) { + throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, + "Geschichte is not a JOURNEY — cannot append items"); + } + + long count = journeyItemRepository.countByGeschichteId(geschichteId); + if (count >= MAX_ITEMS) { + throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, + "Journey already has the maximum of " + MAX_ITEMS + " items"); + } + + String note = normalizeNote(dto.getNote()); + + if (dto.getDocumentId() == null && note == null) { + throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, + "At least one of documentId or note must be provided"); + } + + if (note != null && note.length() > MAX_NOTE_LENGTH) { + throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, + "Note exceeds maximum length of " + MAX_NOTE_LENGTH + " characters"); + } + + Document doc = null; + if (dto.getDocumentId() != null) { + doc = documentService.getSummaryById(dto.getDocumentId()); + } + + int nextPosition = journeyItemRepository.findMaxPositionByGeschichteId(geschichteId) + .map(max -> max + POSITION_STEP) + .orElse(POSITION_STEP); + + JourneyItem item = JourneyItem.builder() + .geschichte(g) + .position(nextPosition) + .document(doc) + .note(note) + .build(); + JourneyItem saved = journeyItemRepository.save(item); + + UUID actorId = currentUser().getId(); + auditService.logAfterCommit(AuditKind.JOURNEY_ITEM_ADDED, actorId, null, + Map.of("geschichteId", geschichteId, "itemId", saved.getId())); + + return toView(saved); + } + + @Transactional + public JourneyItemView updateNote(UUID geschichteId, UUID itemId, JourneyItemUpdateDTO dto) { + JourneyItem item = journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId) + .orElseThrow(() -> DomainException.notFound(ErrorCode.JOURNEY_ITEM_NOT_FOUND, + "Journey item not found: " + itemId)); + + JsonNullable noteField = dto.getNote(); + if (!noteField.isPresent()) { + return toView(item); + } + + String note = normalizeNote(noteField.get()); + + if (note != null && note.length() > MAX_NOTE_LENGTH) { + throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, + "Note exceeds maximum length of " + MAX_NOTE_LENGTH + " characters"); + } + + if (note == null && item.getDocumentId() == null) { + throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, + "Cannot clear note on an item that has no linked document"); + } + + item.setNote(note); + JourneyItem saved = journeyItemRepository.save(item); + return toView(saved); + } + + @Transactional + public void delete(UUID geschichteId, UUID itemId) { + JourneyItem item = journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId) + .orElseThrow(() -> DomainException.notFound(ErrorCode.JOURNEY_ITEM_NOT_FOUND, + "Journey item not found: " + itemId)); + + journeyItemRepository.delete(item); + + UUID actorId = currentUser().getId(); + auditService.logAfterCommit(AuditKind.JOURNEY_ITEM_REMOVED, actorId, null, + Map.of("geschichteId", geschichteId, "itemId", itemId)); + } + + @Transactional + public List reorder(UUID geschichteId, JourneyReorderDTO dto) { + Set existingIds = journeyItemRepository.findIdsByGeschichteId(geschichteId); + List requestedIds = dto.getItemIds() != null ? dto.getItemIds() : List.of(); + + if (!existingIds.equals(new HashSet<>(requestedIds))) { + throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, + "Requested item IDs do not match the journey's existing items"); + } + + if (requestedIds.isEmpty()) { + return List.of(); + } + + List items = journeyItemRepository.findByGeschichteIdOrderByPosition(geschichteId); + Map itemMap = new HashMap<>(); + for (JourneyItem item : items) { + itemMap.put(item.getId(), item); + } + + List reordered = new ArrayList<>(requestedIds.size()); + for (int i = 0; i < requestedIds.size(); i++) { + JourneyItem item = itemMap.get(requestedIds.get(i)); + item.setPosition((i + 1) * POSITION_STEP); + reordered.add(journeyItemRepository.save(item)); + } + + UUID actorId = currentUser().getId(); + auditService.logAfterCommit(AuditKind.JOURNEY_ITEMS_REORDERED, actorId, null, + Map.of("geschichteId", geschichteId, "itemCount", reordered.size())); + + return reordered.stream().map(this::toView).toList(); + } + + public List getItems(UUID geschichteId) { + return journeyItemRepository.findByGeschichteIdOrderByPosition(geschichteId) + .stream().map(this::toView).toList(); + } + + public DocumentSummary toSummary(Document doc) { + String senderName = buildSenderName(doc); + Set receivers = doc.getReceivers(); + String receiverName = buildCanonicalReceiverName(receivers); + + return new DocumentSummary( + doc.getId(), + doc.getTitle(), + doc.getDocumentDate(), + doc.getMetaDateEnd(), + doc.getMetaDatePrecision() != null ? doc.getMetaDatePrecision() : DatePrecision.UNKNOWN, + senderName, + receiverName, + receivers != null ? receivers.size() : 0 + ); + } + + public JourneyItemView toView(JourneyItem item) { + DocumentSummary docSummary = null; + if (item.getDocumentId() != null) { + Document doc = documentService.getSummaryById(item.getDocumentId()); + docSummary = toSummary(doc); + } + return new JourneyItemView(item.getId(), item.getPosition(), docSummary, item.getNote()); + } + + private static String buildSenderName(Document doc) { + Person sender = doc.getSender(); + if (sender != null) { + String name = join(sender.getFirstName(), sender.getLastName()); + if (!name.isBlank()) return name; + } + String senderText = doc.getSenderText(); + return (senderText != null && !senderText.isBlank()) ? senderText : null; + } + + private static String buildCanonicalReceiverName(Set receivers) { + if (receivers == null || receivers.isEmpty()) return null; + return receivers.stream() + .min(Comparator.comparing(p -> sortKey(p.getLastName()) + " " + sortKey(p.getFirstName()))) + .map(p -> { + String name = join(p.getFirstName(), p.getLastName()); + return name.isBlank() ? null : name; + }) + .orElse(null); + } + + private static String normalizeNote(String raw) { + if (raw == null || raw.isBlank()) return null; + return raw.trim(); + } + + private static String join(String first, String last) { + return ((first != null ? first : "") + " " + (last != null ? last : "")).trim(); + } + + private static String sortKey(String s) { + return s != null ? s : ""; + } + + private AppUser currentUser() { + Authentication auth = SecurityContextHolder.getContext().getAuthentication(); + if (auth == null || !auth.isAuthenticated()) { + throw DomainException.unauthorized("Authentication required"); + } + return userService.findByEmail(auth.getName()); + } +} diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java new file mode 100644 index 00000000..9ffb4fc1 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java @@ -0,0 +1,629 @@ +package org.raddatz.familienarchiv.geschichte.journeyitem; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.openapitools.jackson.nullable.JsonNullable; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; +import org.raddatz.familienarchiv.document.DatePrecision; +import org.raddatz.familienarchiv.document.Document; +import org.raddatz.familienarchiv.document.DocumentService; +import org.raddatz.familienarchiv.document.DocumentStatus; +import org.raddatz.familienarchiv.exception.DomainException; +import org.raddatz.familienarchiv.exception.ErrorCode; +import org.raddatz.familienarchiv.geschichte.Geschichte; +import org.raddatz.familienarchiv.geschichte.GeschichteRepository; +import org.raddatz.familienarchiv.geschichte.GeschichteStatus; +import org.raddatz.familienarchiv.geschichte.GeschichteType; +import org.raddatz.familienarchiv.person.Person; +import org.raddatz.familienarchiv.user.AppUser; +import org.raddatz.familienarchiv.user.UserService; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContextHolder; + +import java.time.LocalDate; +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class JourneyItemServiceTest { + + @Mock JourneyItemRepository journeyItemRepository; + @Mock GeschichteRepository geschichteRepository; + @Mock DocumentService documentService; + @Mock AuditService auditService; + @Mock UserService userService; + + @InjectMocks JourneyItemService journeyItemService; + + UUID geschichteId = UUID.randomUUID(); + UUID itemId = UUID.randomUUID(); + UUID docId = UUID.randomUUID(); + UUID actorId = UUID.randomUUID(); + + @BeforeEach + void setupAuth() { + AppUser actor = AppUser.builder().id(actorId).email("test@test.de").build(); + lenient().when(userService.findByEmail("test@test.de")).thenReturn(actor); + SecurityContextHolder.getContext().setAuthentication( + new UsernamePasswordAuthenticationToken("test@test.de", null, + List.of(new SimpleGrantedAuthority("BLOG_WRITE")))); + } + + // ─── toSummary — name composition ──────────────────────────────────────── + + @Test + void toSummary_uses_linked_person_firstName_lastName() { + Person sender = Person.builder().firstName("Franz").lastName("Raddatz").build(); + Document doc = makeDoc(docId, sender, List.of(), null, null); + + var summary = journeyItemService.toSummary(doc); + + assertThat(summary.senderName()).isEqualTo("Franz Raddatz"); + } + + @Test + void toSummary_falls_back_to_senderText_when_no_person() { + Document doc = makeDoc(docId, null, List.of(), "Familie Müller", null); + + var summary = journeyItemService.toSummary(doc); + + assertThat(summary.senderName()).isEqualTo("Familie Müller"); + } + + @Test + void toSummary_returns_null_senderName_when_neither_person_nor_text() { + Document doc = makeDoc(docId, null, List.of(), null, null); + + var summary = journeyItemService.toSummary(doc); + + assertThat(summary.senderName()).isNull(); + } + + @Test + void toSummary_receiverCount_0_and_null_name_when_no_receiver() { + Document doc = makeDoc(docId, null, List.of(), null, null); + + var summary = journeyItemService.toSummary(doc); + + assertThat(summary.receiverCount()).isEqualTo(0); + assertThat(summary.receiverName()).isNull(); + } + + @Test + void toSummary_multi_receiver_returns_first_canonical_name_and_total_count() { + Person emma = Person.builder().firstName("Emma").lastName("Raddatz").build(); + Person anna = Person.builder().firstName("Anna").lastName("Amann").build(); + Document doc = makeDoc(docId, null, List.of(emma, anna), null, null); + + var summary = journeyItemService.toSummary(doc); + + assertThat(summary.receiverCount()).isEqualTo(2); + assertThat(summary.receiverName()).isEqualTo("Anna Amann"); // alphabetically first by lastName + } + + @Test + void toSummary_datePrecision_SEASON_roundtrips() { + Document doc = makeDoc(docId, null, List.of(), null, null); + doc.setMetaDatePrecision(DatePrecision.SEASON); + + var summary = journeyItemService.toSummary(doc); + + assertThat(summary.datePrecision()).isEqualTo(DatePrecision.SEASON); + } + + @Test + void toSummary_datePrecision_APPROX_roundtrips() { + Document doc = makeDoc(docId, null, List.of(), null, null); + doc.setMetaDatePrecision(DatePrecision.APPROX); + + var summary = journeyItemService.toSummary(doc); + + assertThat(summary.datePrecision()).isEqualTo(DatePrecision.APPROX); + } + + // ─── append ────────────────────────────────────────────────────────────── + + @Test + void append_to_empty_journey_starts_at_10() { + Geschichte journey = journey(geschichteId); + when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); + when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.empty()); + JourneyItem saved = savedItem(itemId, journey, 10, null, "Note"); + when(journeyItemRepository.save(any())).thenReturn(saved); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setNote("Note"); + + JourneyItemView view = journeyItemService.append(geschichteId, dto); + + assertThat(view.position()).isEqualTo(10); + } + + @Test + void append_after_reorder_continues_from_max_position() { + Geschichte journey = journey(geschichteId); + when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(2L); + when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.of(40)); + JourneyItem saved = savedItem(itemId, journey, 50, null, "Note"); + when(journeyItemRepository.save(any())).thenReturn(saved); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setNote("Note"); + + JourneyItemView view = journeyItemService.append(geschichteId, dto); + + assertThat(view.position()).isEqualTo(50); + } + + @Test + void append_returns400_when_neither_documentId_nor_note() { + Geschichte journey = journey(geschichteId); + when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + + assertThatThrownBy(() -> journeyItemService.append(geschichteId, dto)) + .isInstanceOf(DomainException.class) + .hasMessageContaining("documentId or note"); + } + + @Test + void append_returns400_when_note_trims_to_empty_and_no_document() { + Geschichte journey = journey(geschichteId); + when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setNote(" \n "); + + assertThatThrownBy(() -> journeyItemService.append(geschichteId, dto)) + .isInstanceOf(DomainException.class); + } + + @Test + void append_returns400_when_note_exceeds_5000_chars() { + Geschichte journey = journey(geschichteId); + when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setNote("x".repeat(5001)); + + assertThatThrownBy(() -> journeyItemService.append(geschichteId, dto)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.VALIDATION_ERROR)); + } + + @Test + void append_returns400_on_non_JOURNEY_type() { + Geschichte story = Geschichte.builder() + .id(geschichteId) + .title("Story") + .type(GeschichteType.STORY) + .status(GeschichteStatus.DRAFT) + .build(); + when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(story)); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setNote("Note"); + + assertThatThrownBy(() -> journeyItemService.append(geschichteId, dto)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.VALIDATION_ERROR)); + } + + @Test + void append_returns404_when_documentId_does_not_exist() { + Geschichte journey = journey(geschichteId); + when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); + when(documentService.getSummaryById(docId)) + .thenThrow(DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "not found")); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setDocumentId(docId); + + assertThatThrownBy(() -> journeyItemService.append(geschichteId, dto)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.DOCUMENT_NOT_FOUND)); + } + + @Test + void append_returns400_when_100_items_exist() { + Geschichte journey = journey(geschichteId); + when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(100L); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setNote("Note"); + + assertThatThrownBy(() -> journeyItemService.append(geschichteId, dto)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.VALIDATION_ERROR)); + } + + @Test + void cap_is_COUNT_based_not_MAX_position_based() { + // 99 rows with MAX(position)=2000 should still accept the 100th append + Geschichte journey = journey(geschichteId); + when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(99L); + when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.of(2000)); + JourneyItem saved = savedItem(itemId, journey, 2010, null, "Note"); + when(journeyItemRepository.save(any())).thenReturn(saved); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setNote("Note"); + + assertThat(journeyItemService.append(geschichteId, dto).position()).isEqualTo(2010); + } + + @Test + void append_audits_JOURNEY_ITEM_ADDED() { + Geschichte journey = journey(geschichteId); + when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); + when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.empty()); + JourneyItem saved = savedItem(itemId, journey, 10, null, "Note"); + when(journeyItemRepository.save(any())).thenReturn(saved); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setNote("Note"); + journeyItemService.append(geschichteId, dto); + + verify(auditService).logAfterCommit(eq(AuditKind.JOURNEY_ITEM_ADDED), eq(actorId), isNull(), any()); + } + + // ─── updateNote ─────────────────────────────────────────────────────────── + + @Test + void updateNote_absent_leaves_note_unchanged() { + Geschichte journey = journey(geschichteId); + JourneyItem item = savedItem(itemId, journey, 10, null, "Original note"); + when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.of(item)); + + JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); + // note is JsonNullable.undefined() by default + + JourneyItemView view = journeyItemService.updateNote(geschichteId, itemId, dto); + + assertThat(view.note()).isEqualTo("Original note"); + verify(journeyItemRepository, never()).save(any()); + } + + @Test + void updateNote_null_clears_note_when_document_is_present() { + Geschichte journey = journey(geschichteId); + Document doc = makeDoc(docId, null, List.of(), null, null); + JourneyItem item = savedItemWithDoc(itemId, journey, 10, doc, "Old note"); + when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.of(item)); + when(documentService.getSummaryById(docId)).thenReturn(doc); + JourneyItem saved = savedItemWithDoc(itemId, journey, 10, doc, null); + when(journeyItemRepository.save(item)).thenReturn(saved); + + JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); + dto.setNote(JsonNullable.of(null)); + + JourneyItemView view = journeyItemService.updateNote(geschichteId, itemId, dto); + + assertThat(view.note()).isNull(); + } + + @Test + void updateNote_string_sets_note() { + Geschichte journey = journey(geschichteId); + JourneyItem item = savedItem(itemId, journey, 10, null, null); + item.setNote(null); + when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.of(item)); + JourneyItem saved = savedItem(itemId, journey, 10, null, "New note"); + when(journeyItemRepository.save(item)).thenReturn(saved); + + JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); + dto.setNote(JsonNullable.of("New note")); + + JourneyItemView view = journeyItemService.updateNote(geschichteId, itemId, dto); + + assertThat(view.note()).isEqualTo("New note"); + } + + @Test + void updateNote_null_returns400_when_item_has_no_document() { + Geschichte journey = journey(geschichteId); + JourneyItem item = savedItem(itemId, journey, 10, null, "Only note — no doc"); + when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.of(item)); + + JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); + dto.setNote(JsonNullable.of(null)); + + assertThatThrownBy(() -> journeyItemService.updateNote(geschichteId, itemId, dto)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.VALIDATION_ERROR)); + } + + @Test + void updateNote_whitespace_only_including_newlines_stored_as_null() { + Geschichte journey = journey(geschichteId); + Document doc = makeDoc(docId, null, List.of(), null, null); + JourneyItem item = savedItemWithDoc(itemId, journey, 10, doc, "Old"); + when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.of(item)); + when(documentService.getSummaryById(docId)).thenReturn(doc); + JourneyItem saved = savedItemWithDoc(itemId, journey, 10, doc, null); + when(journeyItemRepository.save(item)).thenReturn(saved); + + JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); + dto.setNote(JsonNullable.of("\n \n")); + + JourneyItemView view = journeyItemService.updateNote(geschichteId, itemId, dto); + + assertThat(view.note()).isNull(); + } + + @Test + void patch_returns400_when_note_exceeds_5000_chars() { + Geschichte journey = journey(geschichteId); + JourneyItem item = savedItem(itemId, journey, 10, null, "Old"); + when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.of(item)); + + JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); + dto.setNote(JsonNullable.of("x".repeat(5001))); + + assertThatThrownBy(() -> journeyItemService.updateNote(geschichteId, itemId, dto)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.VALIDATION_ERROR)); + } + + @Test + void patch_returns404_when_item_belongs_to_different_journey() { + when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.empty()); + + JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); + dto.setNote(JsonNullable.of("text")); + + assertThatThrownBy(() -> journeyItemService.updateNote(geschichteId, itemId, dto)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.JOURNEY_ITEM_NOT_FOUND)); + } + + // ─── delete ─────────────────────────────────────────────────────────────── + + @Test + void delete_returns404_when_item_already_deleted() { + when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> journeyItemService.delete(geschichteId, itemId)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.JOURNEY_ITEM_NOT_FOUND)); + } + + @Test + void delete_no_audit_when_item_not_found() { + when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> journeyItemService.delete(geschichteId, itemId)) + .isInstanceOf(DomainException.class); + + verify(auditService, never()).logAfterCommit(any(), any(), any(), any()); + } + + @Test + void delete_audits_JOURNEY_ITEM_REMOVED_when_item_found() { + Geschichte journey = journey(geschichteId); + JourneyItem item = savedItem(itemId, journey, 10, null, "Note"); + when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.of(item)); + + journeyItemService.delete(geschichteId, itemId); + + verify(auditService).logAfterCommit(eq(AuditKind.JOURNEY_ITEM_REMOVED), eq(actorId), isNull(), any()); + } + + // ─── reorder ───────────────────────────────────────────────────────────── + + @Test + void reorder_returns400_when_itemId_belongs_to_different_journey() { + UUID foreignId = UUID.randomUUID(); + UUID localId = UUID.randomUUID(); + when(journeyItemRepository.findIdsByGeschichteId(geschichteId)).thenReturn(Set.of(localId)); + + JourneyReorderDTO dto = new JourneyReorderDTO(); + dto.setItemIds(List.of(foreignId)); + + assertThatThrownBy(() -> journeyItemService.reorder(geschichteId, dto)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.VALIDATION_ERROR)); + } + + @Test + void reorder_returns400_when_ids_have_extra_items() { + UUID id1 = UUID.randomUUID(); + UUID id2 = UUID.randomUUID(); + when(journeyItemRepository.findIdsByGeschichteId(geschichteId)).thenReturn(Set.of(id1)); + + JourneyReorderDTO dto = new JourneyReorderDTO(); + dto.setItemIds(List.of(id1, id2)); + + assertThatThrownBy(() -> journeyItemService.reorder(geschichteId, dto)) + .isInstanceOf(DomainException.class); + } + + @Test + void reorder_returns200_when_empty_on_empty_journey() { + when(journeyItemRepository.findIdsByGeschichteId(geschichteId)).thenReturn(Set.of()); + + JourneyReorderDTO dto = new JourneyReorderDTO(); + dto.setItemIds(List.of()); + + List result = journeyItemService.reorder(geschichteId, dto); + + assertThat(result).isEmpty(); + } + + @Test + void reorder_returns400_when_empty_on_nonempty_journey() { + UUID id1 = UUID.randomUUID(); + when(journeyItemRepository.findIdsByGeschichteId(geschichteId)).thenReturn(Set.of(id1)); + + JourneyReorderDTO dto = new JourneyReorderDTO(); + dto.setItemIds(List.of()); + + assertThatThrownBy(() -> journeyItemService.reorder(geschichteId, dto)) + .isInstanceOf(DomainException.class); + } + + @Test + void reorder_returns_items_in_new_order_starting_at_10() { + Geschichte journey = journey(geschichteId); + UUID id1 = UUID.randomUUID(); + UUID id2 = UUID.randomUUID(); + JourneyItem item1 = savedItem(id1, journey, 20, null, "A"); + JourneyItem item2 = savedItem(id2, journey, 10, null, "B"); + when(journeyItemRepository.findIdsByGeschichteId(geschichteId)).thenReturn(Set.of(id1, id2)); + when(journeyItemRepository.findByGeschichteIdOrderByPosition(geschichteId)).thenReturn(List.of(item2, item1)); + when(journeyItemRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + JourneyReorderDTO dto = new JourneyReorderDTO(); + dto.setItemIds(List.of(id1, id2)); // want id1 first + + List views = journeyItemService.reorder(geschichteId, dto); + + assertThat(views).hasSize(2); + assertThat(views.get(0).id()).isEqualTo(id1); + assertThat(views.get(0).position()).isEqualTo(10); + assertThat(views.get(1).id()).isEqualTo(id2); + assertThat(views.get(1).position()).isEqualTo(20); + } + + @Test + void reorder_identical_order_returns200() { + Geschichte journey = journey(geschichteId); + UUID id1 = UUID.randomUUID(); + JourneyItem item1 = savedItem(id1, journey, 10, null, "A"); + when(journeyItemRepository.findIdsByGeschichteId(geschichteId)).thenReturn(Set.of(id1)); + when(journeyItemRepository.findByGeschichteIdOrderByPosition(geschichteId)).thenReturn(List.of(item1)); + when(journeyItemRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + JourneyReorderDTO dto = new JourneyReorderDTO(); + dto.setItemIds(List.of(id1)); + + List views = journeyItemService.reorder(geschichteId, dto); + + assertThat(views).hasSize(1); + assertThat(views.get(0).position()).isEqualTo(10); + } + + @Test + void reorder_of_grandfathered_over_cap_journey_succeeds() { + Geschichte journey = journey(geschichteId); + // 130-item journey — reorder with all 130 IDs must succeed despite > 100 cap + List ids = new java.util.ArrayList<>(); + List items = new java.util.ArrayList<>(); + for (int i = 1; i <= 130; i++) { + UUID id = UUID.randomUUID(); + ids.add(id); + items.add(savedItem(id, journey, i * 10, null, "item " + i)); + } + when(journeyItemRepository.findIdsByGeschichteId(geschichteId)).thenReturn(new HashSet<>(ids)); + when(journeyItemRepository.findByGeschichteIdOrderByPosition(geschichteId)).thenReturn(items); + when(journeyItemRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + JourneyReorderDTO dto = new JourneyReorderDTO(); + dto.setItemIds(ids); + + List views = journeyItemService.reorder(geschichteId, dto); + + assertThat(views).hasSize(130); + } + + @Test + void reorder_audits_JOURNEY_ITEMS_REORDERED() { + Geschichte journey = journey(geschichteId); + UUID id1 = UUID.randomUUID(); + JourneyItem item1 = savedItem(id1, journey, 10, null, "A"); + when(journeyItemRepository.findIdsByGeschichteId(geschichteId)).thenReturn(Set.of(id1)); + when(journeyItemRepository.findByGeschichteIdOrderByPosition(geschichteId)).thenReturn(List.of(item1)); + when(journeyItemRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + JourneyReorderDTO dto = new JourneyReorderDTO(); + dto.setItemIds(List.of(id1)); + journeyItemService.reorder(geschichteId, dto); + + verify(auditService).logAfterCommit(eq(AuditKind.JOURNEY_ITEMS_REORDERED), eq(actorId), isNull(), any()); + } + + // ─── helpers ───────────────────────────────────────────────────────────── + + private Geschichte journey(UUID id) { + return Geschichte.builder() + .id(id) + .title("Test Journey") + .type(GeschichteType.JOURNEY) + .status(GeschichteStatus.DRAFT) + .build(); + } + + private JourneyItem savedItem(UUID id, Geschichte g, int position, Document doc, String note) { + return JourneyItem.builder() + .id(id) + .geschichte(g) + .position(position) + .document(null) // no document entity to avoid LAZY issues in unit tests + .note(note) + .build(); + } + + private JourneyItem savedItemWithDoc(UUID id, Geschichte g, int position, Document doc, String note) { + JourneyItem item = JourneyItem.builder() + .id(id) + .geschichte(g) + .position(position) + .document(doc) + .note(note) + .build(); + return item; + } + + private Document makeDoc(UUID id, Person sender, List receivers, String senderText, String receiverText) { + Document doc = Document.builder() + .id(id) + .title("Test Doc") + .originalFilename("test.pdf") + .status(DocumentStatus.UPLOADED) + .senderText(senderText) + .receiverText(receiverText) + .sender(sender) + .build(); + doc.setReceivers(new HashSet<>(receivers)); + return doc; + } +} -- 2.49.1 From d29f217328e47f48fcba936b4c1d87d3208a425a Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 16:52:29 +0200 Subject: [PATCH 07/41] feat(geschichte): getById returns GeschichteView; author email never exposed Co-Authored-By: Claude Sonnet 4.6 --- .../geschichte/GeschichteController.java | 2 +- .../geschichte/GeschichteService.java | 35 ++++++-- .../geschichte/GeschichteControllerTest.java | 9 +- .../GeschichteServiceIntegrationTest.java | 7 +- .../geschichte/GeschichteServiceTest.java | 89 ++++++++++++++++--- 5 files changed, 114 insertions(+), 28 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java index 09be9493..11468c8d 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java @@ -37,7 +37,7 @@ public class GeschichteController { } @GetMapping("/{id}") - public Geschichte getById(@PathVariable UUID id) { + public GeschichteView getById(@PathVariable UUID id) { return geschichteService.getById(id); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java index b290186c..37a1b75f 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java @@ -2,11 +2,12 @@ package org.raddatz.familienarchiv.geschichte; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import org.hibernate.Hibernate; import org.owasp.html.HtmlPolicyBuilder; import org.owasp.html.PolicyFactory; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemService; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemView; import org.raddatz.familienarchiv.user.AppUser; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.security.Permission; @@ -33,10 +34,9 @@ public class GeschichteService { private final GeschichteRepository geschichteRepository; private final PersonService personService; - // Reserved for lesereisen-editor: JourneyItem document resolution must go through - // DocumentService.getDocumentById to enforce existence and scope checks. private final DocumentService documentService; private final UserService userService; + private final JourneyItemService journeyItemService; /** * Allow-list policy for Geschichte body HTML. Tiptap on the writer side @@ -57,7 +57,7 @@ public class GeschichteService { } @Transactional(readOnly = true) - public Geschichte getById(UUID id) { + public GeschichteView getById(UUID id) { Geschichte g = geschichteRepository.findById(id) .orElseThrow(() -> DomainException.notFound( ErrorCode.GESCHICHTE_NOT_FOUND, "Geschichte not found: " + id)); @@ -66,11 +66,28 @@ public class GeschichteService { throw DomainException.notFound( ErrorCode.GESCHICHTE_NOT_FOUND, "Geschichte not found: " + id); } - // Force-initialize LAZY items inside this transaction. - // open-in-view is FALSE — without this touch, Jackson serializes a closed - // Hibernate session and throws LazyInitializationException → HTTP 500. - Hibernate.initialize(g.getItems()); - return g; + // Items loaded via repository query — never through the LAZY collection on Geschichte. + // This keeps open-in-view:false safe without Hibernate.initialize. + List items = journeyItemService.getItems(id); + return toView(g, items); + } + + private GeschichteView toView(Geschichte g, List items) { + AppUser author = g.getAuthor(); + GeschichteView.AuthorView authorView = null; + if (author != null) { + String displayName = ((author.getFirstName() != null ? author.getFirstName() : "") + + " " + (author.getLastName() != null ? author.getLastName() : "")).trim(); + if (displayName.isBlank()) displayName = author.getEmail(); + authorView = new GeschichteView.AuthorView(author.getId(), displayName); + } + return new GeschichteView( + g.getId(), g.getTitle(), g.getBody(), + g.getStatus(), g.getType(), + authorView, g.getPersons(), + items, + g.getPublishedAt(), g.getCreatedAt(), g.getUpdatedAt() + ); } /** diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java index 21fced9d..54dc8c85 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java @@ -104,7 +104,7 @@ class GeschichteControllerTest { @WithMockUser(authorities = "READ_ALL") void getById_returns200_whenFound() throws Exception { UUID id = UUID.randomUUID(); - when(geschichteService.getById(id)).thenReturn(published(id, "Hello")); + when(geschichteService.getById(id)).thenReturn(viewStub(id, "Hello")); mockMvc.perform(get("/api/geschichten/{id}", id)) .andExpect(status().isOk()) @@ -233,6 +233,13 @@ class GeschichteControllerTest { .build(); } + private GeschichteView viewStub(UUID id, String title) { + return new GeschichteView(id, title, "

x

", + GeschichteStatus.PUBLISHED, GeschichteType.STORY, + null, new HashSet<>(), List.of(), + LocalDateTime.now(), LocalDateTime.now(), LocalDateTime.now()); + } + /** Concrete implementation — Mockito interface mocks are not serialized reliably by Jackson. */ private GeschichteSummary summaryStub(String title) { return new GeschichteSummary() { diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java index 25a46552..39213aec 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java @@ -8,6 +8,7 @@ import org.raddatz.familienarchiv.geschichte.GeschichteUpdateDTO; import org.raddatz.familienarchiv.user.AppUser; import org.raddatz.familienarchiv.geschichte.Geschichte; import org.raddatz.familienarchiv.geschichte.GeschichteStatus; +import org.raddatz.familienarchiv.geschichte.GeschichteView; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.user.AppUserRepository; import org.raddatz.familienarchiv.geschichte.GeschichteRepository; @@ -104,9 +105,9 @@ class GeschichteServiceIntegrationTest { authenticateAs(reader, Permission.READ_ALL); assertThat(geschichteService.list(null, List.of(), 50)).hasSize(1); assertThat(geschichteService.list(null, List.of(franz.getId()), 50)).hasSize(1); - Geschichte fetched = geschichteService.getById(draftId); - assertThat(fetched.getTitle()).isEqualTo("Erinnerung an Opa Franz"); - assertThat(fetched.getPersons()).extracting(Person::getId).containsExactly(franz.getId()); + GeschichteView fetched = geschichteService.getById(draftId); + assertThat(fetched.title()).isEqualTo("Erinnerung an Opa Franz"); + assertThat(fetched.persons()).extracting(Person::getId).containsExactly(franz.getId()); // Delete as writer; join rows go with it authenticateAs(writer, Permission.BLOG_WRITE); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java index 64920730..19c72c15 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java @@ -9,6 +9,7 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemService; import org.raddatz.familienarchiv.user.AppUser; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.security.Permission; @@ -32,6 +33,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -40,17 +42,13 @@ import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class GeschichteServiceTest { - @Mock - GeschichteRepository geschichteRepository; - @Mock - PersonService personService; - @Mock - DocumentService documentService; - @Mock - UserService userService; + @Mock GeschichteRepository geschichteRepository; + @Mock PersonService personService; + @Mock DocumentService documentService; + @Mock UserService userService; + @Mock JourneyItemService journeyItemService; - @InjectMocks - GeschichteService geschichteService; + @InjectMocks GeschichteService geschichteService; AppUser writer; AppUser reader; @@ -60,6 +58,7 @@ class GeschichteServiceTest { SecurityContextHolder.clearContext(); writer = AppUser.builder().id(UUID.randomUUID()).email("writer@test").build(); reader = AppUser.builder().id(UUID.randomUUID()).email("reader@test").build(); + lenient().when(journeyItemService.getItems(any())).thenReturn(List.of()); } @AfterEach @@ -89,9 +88,10 @@ class GeschichteServiceTest { Geschichte draft = draft(id); when(geschichteRepository.findById(id)).thenReturn(Optional.of(draft)); - Geschichte result = geschichteService.getById(id); + GeschichteView result = geschichteService.getById(id); - assertThat(result).isSameAs(draft); + assertThat(result.id()).isEqualTo(id); + assertThat(result.status()).isEqualTo(GeschichteStatus.DRAFT); } @Test @@ -101,9 +101,70 @@ class GeschichteServiceTest { Geschichte published = published(id); when(geschichteRepository.findById(id)).thenReturn(Optional.of(published)); - Geschichte result = geschichteService.getById(id); + GeschichteView result = geschichteService.getById(id); - assertThat(result).isSameAs(published); + assertThat(result.id()).isEqualTo(id); + assertThat(result.status()).isEqualTo(GeschichteStatus.PUBLISHED); + } + + @Test + void getById_author_displayName_uses_firstName_lastName() { + authenticateAs(reader, Permission.READ_ALL); + UUID id = UUID.randomUUID(); + Geschichte published = published(id); + published.setAuthor(AppUser.builder() + .id(UUID.randomUUID()).email("author@test") + .firstName("Hans").lastName("Raddatz").build()); + when(geschichteRepository.findById(id)).thenReturn(Optional.of(published)); + + GeschichteView result = geschichteService.getById(id); + + assertThat(result.author().displayName()).isEqualTo("Hans Raddatz"); + } + + @Test + void getById_author_displayName_falls_back_to_email_when_names_blank() { + authenticateAs(reader, Permission.READ_ALL); + UUID id = UUID.randomUUID(); + Geschichte published = published(id); + published.setAuthor(AppUser.builder() + .id(UUID.randomUUID()).email("anon@test").build()); + when(geschichteRepository.findById(id)).thenReturn(Optional.of(published)); + + GeschichteView result = geschichteService.getById(id); + + assertThat(result.author().displayName()).isEqualTo("anon@test"); + } + + @Test + void getById_author_email_is_not_in_author_view() { + authenticateAs(reader, Permission.READ_ALL); + UUID id = UUID.randomUUID(); + Geschichte published = published(id); + published.setAuthor(AppUser.builder() + .id(UUID.randomUUID()).email("secret@test") + .firstName("Max").lastName("M").build()); + when(geschichteRepository.findById(id)).thenReturn(Optional.of(published)); + + GeschichteView result = geschichteService.getById(id); + + // AuthorView exposes only id + displayName — no email field at all + assertThat(result.author()).isInstanceOf(GeschichteView.AuthorView.class); + assertThat(result.author().displayName()).doesNotContain("secret@test"); + } + + @Test + void getById_items_come_from_journeyItemService() { + authenticateAs(reader, Permission.READ_ALL); + UUID id = UUID.randomUUID(); + Geschichte published = published(id); + when(geschichteRepository.findById(id)).thenReturn(Optional.of(published)); + when(journeyItemService.getItems(id)).thenReturn(List.of()); + + GeschichteView result = geschichteService.getById(id); + + assertThat(result.items()).isEmpty(); + verify(journeyItemService).getItems(id); } @Test -- 2.49.1 From a0fa8f4d02245e04a7ba3c56bed25cc350eb27d8 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 17:02:12 +0200 Subject: [PATCH 08/41] feat(journeyitem): add CRUD endpoints for JourneyItems on GeschichteController Adds POST/PATCH/DELETE/PUT-reorder endpoints for journey items, backed by JourneyItemService. Replaces jackson-databind-nullable (Jackson 2.x, incompatible with Spring Boot 4 / Jackson 3.x) with Optional three-way PATCH semantics: null = absent/no-op, empty = clear, present = set. Co-Authored-By: Claude Sonnet 4.6 --- backend/pom.xml | 7 - .../familienarchiv/config/JacksonConfig.java | 11 +- .../geschichte/GeschichteController.java | 44 +++++ .../journeyitem/JourneyItemService.java | 8 +- .../journeyitem/JourneyItemUpdateDTO.java | 15 +- .../geschichte/GeschichteControllerTest.java | 172 +++++++++++++++++- .../journeyitem/JourneyItemServiceTest.java | 15 +- 7 files changed, 232 insertions(+), 40 deletions(-) diff --git a/backend/pom.xml b/backend/pom.xml index 30fa0719..b01f2362 100644 --- a/backend/pom.xml +++ b/backend/pom.xml @@ -253,13 +253,6 @@ 1.18.1 - - - org.openapitools - jackson-databind-nullable - 0.2.6 - - io.micrometer diff --git a/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java b/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java index 5c5e0945..43a597d3 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java @@ -1,15 +1,10 @@ package org.raddatz.familienarchiv.config; -import com.fasterxml.jackson.databind.Module; -import org.openapitools.jackson.nullable.JsonNullableModule; -import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +/** + * Jackson customisations. Currently a placeholder — custom modules added here as needed. + */ @Configuration public class JacksonConfig { - - @Bean - public Module jsonNullableModule() { - return new JsonNullableModule(); - } } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java index 11468c8d..baf365a4 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java @@ -1,6 +1,11 @@ package org.raddatz.familienarchiv.geschichte; import lombok.RequiredArgsConstructor; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemCreateDTO; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemService; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemUpdateDTO; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemView; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyReorderDTO; import org.raddatz.familienarchiv.security.Permission; import org.raddatz.familienarchiv.security.RequirePermission; import org.springframework.http.HttpStatus; @@ -10,6 +15,7 @@ import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PatchMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; @@ -24,6 +30,7 @@ import java.util.UUID; public class GeschichteController { private final GeschichteService geschichteService; + private final JourneyItemService journeyItemService; @GetMapping public List list( @@ -60,4 +67,41 @@ public class GeschichteController { geschichteService.delete(id); return ResponseEntity.noContent().build(); } + + // ─── JourneyItem CRUD ──────────────────────────────────────────────────── + + @PostMapping("/{id}/items") + @RequirePermission(Permission.BLOG_WRITE) + public ResponseEntity appendItem( + @PathVariable UUID id, + @RequestBody JourneyItemCreateDTO dto) { + JourneyItemView view = journeyItemService.append(id, dto); + return ResponseEntity.status(HttpStatus.CREATED).body(view); + } + + @PatchMapping("/{id}/items/{itemId}") + @RequirePermission(Permission.BLOG_WRITE) + public JourneyItemView updateItemNote( + @PathVariable UUID id, + @PathVariable UUID itemId, + @RequestBody JourneyItemUpdateDTO dto) { + return journeyItemService.updateNote(id, itemId, dto); + } + + @DeleteMapping("/{id}/items/{itemId}") + @RequirePermission(Permission.BLOG_WRITE) + public ResponseEntity deleteItem( + @PathVariable UUID id, + @PathVariable UUID itemId) { + journeyItemService.delete(id, itemId); + return ResponseEntity.noContent().build(); + } + + @PutMapping("/{id}/items/reorder") + @RequirePermission(Permission.BLOG_WRITE) + public List reorderItems( + @PathVariable UUID id, + @RequestBody JourneyReorderDTO dto) { + return journeyItemService.reorder(id, dto); + } } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java index 6b1923f8..d0835c26 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -2,7 +2,6 @@ package org.raddatz.familienarchiv.geschichte.journeyitem; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import org.openapitools.jackson.nullable.JsonNullable; import org.raddatz.familienarchiv.audit.AuditKind; import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.document.DatePrecision; @@ -98,12 +97,13 @@ public class JourneyItemService { .orElseThrow(() -> DomainException.notFound(ErrorCode.JOURNEY_ITEM_NOT_FOUND, "Journey item not found: " + itemId)); - JsonNullable noteField = dto.getNote(); - if (!noteField.isPresent()) { + // null = field absent from JSON → no-op + Optional noteField = dto.getNote(); + if (noteField == null) { return toView(item); } - String note = normalizeNote(noteField.get()); + String note = normalizeNote(noteField.orElse(null)); if (note != null && note.length() > MAX_NOTE_LENGTH) { throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemUpdateDTO.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemUpdateDTO.java index db8c4e6e..1e63ac9c 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemUpdateDTO.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemUpdateDTO.java @@ -1,16 +1,19 @@ package org.raddatz.familienarchiv.geschichte.journeyitem; import lombok.Data; -import org.openapitools.jackson.nullable.JsonNullable; + +import java.util.Optional; /** * Input for PATCH /api/geschichten/{id}/items/{itemId}. - * JsonNullable enables three-way semantics: - * absent → field not present in JSON → leave unchanged - * null → {"note": null} → clear the note field - * string → {"note": "text"} → set the note + * Three-way semantics via Optional: + * null → field absent from JSON → leave note unchanged + * Optional.empty() → {"note": null} → clear the note + * Optional.of("x") → {"note": "x"} → set the note + * + * Jackson 3.x maps JSON null to Optional.empty(); absent fields keep the Java default (null). */ @Data public class JourneyItemUpdateDTO { - private JsonNullable note = JsonNullable.undefined(); + private Optional note = null; } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java index 54dc8c85..88320dc2 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java @@ -2,9 +2,12 @@ package org.raddatz.familienarchiv.geschichte; import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.Test; -import org.raddatz.familienarchiv.security.SecurityConfig; +import org.raddatz.familienarchiv.config.JacksonConfig; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemService; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemView; +import org.raddatz.familienarchiv.security.SecurityConfig; import org.raddatz.familienarchiv.security.PermissionAspect; import org.raddatz.familienarchiv.user.CustomUserDetailsService; import org.springframework.beans.factory.annotation.Autowired; @@ -32,11 +35,12 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilder import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @WebMvcTest(GeschichteController.class) -@Import({SecurityConfig.class, PermissionAspect.class, AopAutoConfiguration.class}) +@Import({SecurityConfig.class, PermissionAspect.class, AopAutoConfiguration.class, JacksonConfig.class}) class GeschichteControllerTest { @Autowired @@ -44,11 +48,9 @@ class GeschichteControllerTest { private final ObjectMapper objectMapper = new ObjectMapper(); - @MockitoBean - GeschichteService geschichteService; - - @MockitoBean - CustomUserDetailsService customUserDetailsService; + @MockitoBean GeschichteService geschichteService; + @MockitoBean JourneyItemService journeyItemService; + @MockitoBean CustomUserDetailsService customUserDetailsService; // ─── GET /api/geschichten ──────────────────────────────────────────────── @@ -205,8 +207,164 @@ class GeschichteControllerTest { verify(geschichteService).delete(id); } + // ─── POST /api/geschichten/{id}/items ──────────────────────────────────── + + @Test + void appendItem_returns401_whenUnauthenticated() throws Exception { + mockMvc.perform(post("/api/geschichten/{id}/items", UUID.randomUUID()).with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"note\":\"x\"}")) + .andExpect(status().isUnauthorized()); + } + + @Test + @WithMockUser(authorities = "READ_ALL") + void appendItem_returns403_whenLackingBlogWrite() throws Exception { + mockMvc.perform(post("/api/geschichten/{id}/items", UUID.randomUUID()).with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"note\":\"x\"}")) + .andExpect(status().isForbidden()); + } + + @Test + @WithMockUser(authorities = "BLOG_WRITE") + void appendItem_returns201_withBlogWrite() throws Exception { + UUID id = UUID.randomUUID(); + UUID itemId = UUID.randomUUID(); + when(journeyItemService.append(eq(id), any())).thenReturn(itemViewStub(itemId, 10, "Note")); + + mockMvc.perform(post("/api/geschichten/{id}/items", id).with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"note\":\"Note\"}")) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$.id").value(itemId.toString())) + .andExpect(jsonPath("$.position").value(10)); + } + + // ─── PATCH /api/geschichten/{id}/items/{itemId} ────────────────────────── + + @Test + @WithMockUser(authorities = "READ_ALL") + void updateItemNote_returns403_whenLackingBlogWrite() throws Exception { + mockMvc.perform(patch("/api/geschichten/{id}/items/{itemId}", + UUID.randomUUID(), UUID.randomUUID()).with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"note\":\"x\"}")) + .andExpect(status().isForbidden()); + } + + @Test + @WithMockUser(authorities = "BLOG_WRITE") + void updateItemNote_returns200_withBlogWrite() throws Exception { + UUID id = UUID.randomUUID(); + UUID itemId = UUID.randomUUID(); + when(journeyItemService.updateNote(eq(id), eq(itemId), any())) + .thenReturn(itemViewStub(itemId, 10, "Updated")); + + mockMvc.perform(patch("/api/geschichten/{id}/items/{itemId}", id, itemId).with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"note\":\"Updated\"}")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.note").value("Updated")); + } + + @Test + @WithMockUser(authorities = "BLOG_WRITE") + void updateItemNote_null_note_deserializes_as_present_null() throws Exception { + UUID id = UUID.randomUUID(); + UUID itemId = UUID.randomUUID(); + when(journeyItemService.updateNote(eq(id), eq(itemId), any())) + .thenReturn(itemViewStub(itemId, 10, null)); + + // Raw JSON — local objectMapper lacks JsonNullableModule + mockMvc.perform(patch("/api/geschichten/{id}/items/{itemId}", id, itemId).with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"note\": null}")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.note").doesNotExist()); + } + + @Test + @WithMockUser(authorities = "BLOG_WRITE") + void updateItemNote_returns404_whenItemNotFound() throws Exception { + UUID id = UUID.randomUUID(); + UUID itemId = UUID.randomUUID(); + when(journeyItemService.updateNote(eq(id), eq(itemId), any())) + .thenThrow(DomainException.notFound(ErrorCode.JOURNEY_ITEM_NOT_FOUND, "not found")); + + mockMvc.perform(patch("/api/geschichten/{id}/items/{itemId}", id, itemId).with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"note\":\"x\"}")) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.code").value("JOURNEY_ITEM_NOT_FOUND")); + } + + // ─── DELETE /api/geschichten/{id}/items/{itemId} ───────────────────────── + + @Test + @WithMockUser(authorities = "READ_ALL") + void deleteItem_returns403_whenLackingBlogWrite() throws Exception { + mockMvc.perform(delete("/api/geschichten/{id}/items/{itemId}", + UUID.randomUUID(), UUID.randomUUID()).with(csrf())) + .andExpect(status().isForbidden()); + } + + @Test + @WithMockUser(authorities = "BLOG_WRITE") + void deleteItem_returns204_withBlogWrite() throws Exception { + UUID id = UUID.randomUUID(); + UUID itemId = UUID.randomUUID(); + + mockMvc.perform(delete("/api/geschichten/{id}/items/{itemId}", id, itemId).with(csrf())) + .andExpect(status().isNoContent()); + + verify(journeyItemService).delete(id, itemId); + } + + @Test + @WithMockUser(authorities = "BLOG_WRITE") + void deleteItem_returns404_whenItemNotFound() throws Exception { + UUID id = UUID.randomUUID(); + UUID itemId = UUID.randomUUID(); + org.mockito.Mockito.doThrow(DomainException.notFound(ErrorCode.JOURNEY_ITEM_NOT_FOUND, "not found")) + .when(journeyItemService).delete(id, itemId); + + mockMvc.perform(delete("/api/geschichten/{id}/items/{itemId}", id, itemId).with(csrf())) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.code").value("JOURNEY_ITEM_NOT_FOUND")); + } + + // ─── PUT /api/geschichten/{id}/items/reorder ───────────────────────────── + + @Test + @WithMockUser(authorities = "READ_ALL") + void reorderItems_returns403_whenLackingBlogWrite() throws Exception { + mockMvc.perform(put("/api/geschichten/{id}/items/reorder", UUID.randomUUID()).with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"itemIds\":[]}")) + .andExpect(status().isForbidden()); + } + + @Test + @WithMockUser(authorities = "BLOG_WRITE") + void reorderItems_returns200_withBlogWrite() throws Exception { + UUID id = UUID.randomUUID(); + UUID itemId = UUID.randomUUID(); + when(journeyItemService.reorder(eq(id), any())).thenReturn(List.of(itemViewStub(itemId, 10, null))); + + mockMvc.perform(put("/api/geschichten/{id}/items/reorder", id).with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"itemIds\":[\"" + itemId + "\"]}")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$[0].id").value(itemId.toString())); + } + // ─── helpers ───────────────────────────────────────────────────────────── + private JourneyItemView itemViewStub(UUID id, int position, String note) { + return new JourneyItemView(id, position, null, note); + } + private Geschichte published(UUID id, String title) { return Geschichte.builder() .id(id) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java index 9ffb4fc1..a6afe3b6 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java @@ -6,7 +6,6 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import org.openapitools.jackson.nullable.JsonNullable; import org.raddatz.familienarchiv.audit.AuditKind; import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.document.DatePrecision; @@ -310,7 +309,7 @@ class JourneyItemServiceTest { when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.of(item)); JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); - // note is JsonNullable.undefined() by default + // note is null by default — absent from JSON, no-op JourneyItemView view = journeyItemService.updateNote(geschichteId, itemId, dto); @@ -329,7 +328,7 @@ class JourneyItemServiceTest { when(journeyItemRepository.save(item)).thenReturn(saved); JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); - dto.setNote(JsonNullable.of(null)); + dto.setNote(Optional.empty()); JourneyItemView view = journeyItemService.updateNote(geschichteId, itemId, dto); @@ -346,7 +345,7 @@ class JourneyItemServiceTest { when(journeyItemRepository.save(item)).thenReturn(saved); JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); - dto.setNote(JsonNullable.of("New note")); + dto.setNote(Optional.of("New note")); JourneyItemView view = journeyItemService.updateNote(geschichteId, itemId, dto); @@ -360,7 +359,7 @@ class JourneyItemServiceTest { when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.of(item)); JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); - dto.setNote(JsonNullable.of(null)); + dto.setNote(Optional.empty()); assertThatThrownBy(() -> journeyItemService.updateNote(geschichteId, itemId, dto)) .isInstanceOf(DomainException.class) @@ -379,7 +378,7 @@ class JourneyItemServiceTest { when(journeyItemRepository.save(item)).thenReturn(saved); JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); - dto.setNote(JsonNullable.of("\n \n")); + dto.setNote(Optional.of("\n \n")); JourneyItemView view = journeyItemService.updateNote(geschichteId, itemId, dto); @@ -393,7 +392,7 @@ class JourneyItemServiceTest { when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.of(item)); JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); - dto.setNote(JsonNullable.of("x".repeat(5001))); + dto.setNote(Optional.of("x".repeat(5001))); assertThatThrownBy(() -> journeyItemService.updateNote(geschichteId, itemId, dto)) .isInstanceOf(DomainException.class) @@ -406,7 +405,7 @@ class JourneyItemServiceTest { when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.empty()); JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); - dto.setNote(JsonNullable.of("text")); + dto.setNote(Optional.of("text")); assertThatThrownBy(() -> journeyItemService.updateNote(geschichteId, itemId, dto)) .isInstanceOf(DomainException.class) -- 2.49.1 From 4603e335fdebec0a8ea4f380ba07a57841751218 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 17:04:51 +0200 Subject: [PATCH 09/41] feat(frontend): add JOURNEY_ITEM error codes, i18n keys, regen API types Adds JOURNEY_ITEM_NOT_FOUND and JOURNEY_ITEM_POSITION_CONFLICT to the frontend ErrorCode union and getErrorMessage() switch. Adds de/en/es translations. Regenerates api.ts from the current OpenAPI spec (needs a second run once the backend is restarted with the new endpoints compiled in). Co-Authored-By: Claude Sonnet 4.6 --- frontend/messages/de.json | 2 + frontend/messages/en.json | 2 + frontend/messages/es.json | 2 + frontend/src/lib/generated/api.ts | 261 +++++++++++++----------------- frontend/src/lib/shared/errors.ts | 6 + 5 files changed, 126 insertions(+), 147 deletions(-) diff --git a/frontend/messages/de.json b/frontend/messages/de.json index da8b0672..b0e3e054 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -1023,6 +1023,8 @@ "nav_stammbaum": "Stammbaum", "nav_geschichten": "Geschichten", "error_geschichte_not_found": "Die Geschichte wurde nicht gefunden.", + "error_journey_item_not_found": "Der Reise-Eintrag wurde nicht gefunden.", + "error_journey_item_position_conflict": "Positionskonflikt beim Sortieren der Reise-Einträge.", "geschichten_index_title": "Geschichten", "geschichten_new_button": "Neue Geschichte", "geschichten_filter_all_pill": "Alle", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index ab47f6f9..7bca4db1 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -1023,6 +1023,8 @@ "nav_stammbaum": "Family tree", "nav_geschichten": "Stories", "error_geschichte_not_found": "The story was not found.", + "error_journey_item_not_found": "The journey item was not found.", + "error_journey_item_position_conflict": "Position conflict while reordering journey items.", "geschichten_index_title": "Stories", "geschichten_new_button": "New story", "geschichten_filter_all_pill": "All", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index abe21231..ea5002d9 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -1023,6 +1023,8 @@ "nav_stammbaum": "Árbol genealógico", "nav_geschichten": "Historias", "error_geschichte_not_found": "No se encontró la historia.", + "error_journey_item_not_found": "No se encontró el elemento del viaje.", + "error_journey_item_position_conflict": "Conflicto de posición al reordenar los elementos del viaje.", "geschichten_index_title": "Historias", "geschichten_new_button": "Nueva historia", "geschichten_filter_all_pill": "Todas", diff --git a/frontend/src/lib/generated/api.ts b/frontend/src/lib/generated/api.ts index 526e51aa..d5518fd5 100644 --- a/frontend/src/lib/generated/api.ts +++ b/frontend/src/lib/generated/api.ts @@ -692,22 +692,6 @@ export interface paths { patch?: never; trace?: never; }; - "/api/admin/backfill-titles": { - parameters: { - query?: never; - header?: never; - path?: never; - cookie?: never; - }; - get?: never; - put?: never; - post: operations["backfillTitles"]; - delete?: never; - options?: never; - head?: never; - patch?: never; - trace?: never; - }; "/api/admin/backfill-file-hashes": { parameters: { query?: never; @@ -875,7 +859,7 @@ export interface paths { path?: never; cookie?: never; }; - get: operations["search_1"]; + get: operations["search"]; put?: never; post?: never; delete?: never; @@ -1339,7 +1323,7 @@ export interface paths { path?: never; cookie?: never; }; - get: operations["search_2"]; + get: operations["search_1"]; put?: never; post?: never; delete?: never; @@ -1428,6 +1412,22 @@ export interface paths { patch?: never; trace?: never; }; + "/api/documents/conversation": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["getConversation"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/dashboard/resume": { parameters: { query?: never; @@ -1758,7 +1758,6 @@ export interface components { sender?: components["schemas"]["Person"]; tags?: components["schemas"]["Tag"][]; trainingLabels?: ("KURRENT_RECOGNITION" | "KURRENT_SEGMENTATION")[]; - hasTranscription: boolean; thumbnailUrl?: string; }; PersonMention: { @@ -1819,75 +1818,6 @@ export interface components { /** Format: uuid */ targetId: string; }; - Pageable: { - /** Format: int32 */ - page?: number; - /** Format: int32 */ - size?: number; - sort?: string[]; - }; - ActivityActorDTO: { - initials: string; - color: string; - name?: string; - }; - DocumentListItem: { - /** Format: uuid */ - id: string; - title: string; - originalFilename: string; - thumbnailUrl?: string; - /** Format: date */ - documentDate?: string; - /** @enum {string} */ - metaDatePrecision: "DAY" | "MONTH" | "SEASON" | "YEAR" | "RANGE" | "APPROX" | "UNKNOWN"; - /** Format: date */ - metaDateEnd?: string; - sender?: components["schemas"]["Person"]; - receivers: components["schemas"]["Person"][]; - tags: components["schemas"]["Tag"][]; - archiveBox?: string; - archiveFolder?: string; - location?: string; - summary?: string; - /** Format: int32 */ - completionPercentage: number; - contributors: components["schemas"]["ActivityActorDTO"][]; - matchData: components["schemas"]["SearchMatchData"]; - /** Format: date-time */ - createdAt: string; - /** Format: date-time */ - updatedAt: string; - }; - DocumentSearchResult: { - items: components["schemas"]["DocumentListItem"][]; - /** Format: int64 */ - totalElements: number; - /** Format: int32 */ - pageNumber: number; - /** Format: int32 */ - pageSize: number; - /** Format: int32 */ - totalPages: number; - /** Format: int64 */ - undatedCount: number; - }; - MatchOffset: { - /** Format: int32 */ - start: number; - /** Format: int32 */ - length: number; - }; - SearchMatchData: { - transcriptionSnippet?: string; - titleOffsets: components["schemas"]["MatchOffset"][]; - senderMatched: boolean; - matchedReceiverIds: string[]; - matchedTagIds: string[]; - snippetOffsets: components["schemas"]["MatchOffset"][]; - summarySnippet?: string; - summaryOffsets: components["schemas"]["MatchOffset"][]; - }; CreateRelationshipRequest: { /** Format: uuid */ relatedPersonId: string; @@ -2016,6 +1946,7 @@ export interface components { /** @enum {string} */ status?: "DRAFT" | "PUBLISHED"; personIds?: string[]; + documentIds?: string[]; }; Geschichte: { /** Format: uuid */ @@ -2024,11 +1955,9 @@ export interface components { body?: string; /** @enum {string} */ status: "DRAFT" | "PUBLISHED"; - /** @enum {string} */ - type: "STORY" | "JOURNEY"; author?: components["schemas"]["AppUser"]; persons?: components["schemas"]["Person"][]; - items?: components["schemas"]["JourneyItem"][]; + documents?: components["schemas"]["Document"][]; /** Format: date-time */ createdAt: string; /** Format: date-time */ @@ -2036,32 +1965,6 @@ export interface components { /** Format: date-time */ publishedAt?: string; }; - JourneyItem: { - /** Format: uuid */ - id: string; - /** Format: int32 */ - position: number; - /** Format: uuid */ - documentId?: string; - note?: string; - }; - GeschichteSummary: { - /** Format: uuid */ - id: string; - title: string; - /** @enum {string} */ - status: "DRAFT" | "PUBLISHED"; - /** @enum {string} */ - type: "STORY" | "JOURNEY"; - author?: { - firstName?: string; - lastName?: string; - email: string; - }; - body?: string; - /** Format: date-time */ - publishedAt?: string; - }; CreateTranscriptionBlockDTO: { /** Format: int32 */ pageNumber?: number; @@ -2300,6 +2203,11 @@ export interface components { /** Format: int64 */ transcriptionCount: number; }; + ActivityActorDTO: { + initials: string; + color: string; + name?: string; + }; TranscriptionQueueItemDTO: { /** Format: uuid */ id: string; @@ -2322,11 +2230,6 @@ export interface components { color?: string; /** Format: int32 */ documentCount: number; - /** - * Format: int32 - * @description Distinct documents tagged with this tag or any descendant tag (subtree rollup) - */ - subtreeDocumentCount: number; children?: components["schemas"]["TagTreeNodeDTO"][]; /** * Format: uuid @@ -2467,6 +2370,8 @@ export interface components { /** Format: int32 */ totalPages?: number; pageable?: components["schemas"]["PageableObject"]; + first?: boolean; + last?: boolean; /** Format: int32 */ size?: number; content?: components["schemas"]["NotificationDTO"][]; @@ -2475,8 +2380,6 @@ export interface components { sort?: components["schemas"]["SortObject"]; /** Format: int32 */ numberOfElements?: number; - first?: boolean; - last?: boolean; empty?: boolean; }; PageableObject: { @@ -2540,6 +2443,63 @@ export interface components { /** Format: int32 */ totalPages?: number; }; + DocumentListItem: { + /** Format: uuid */ + id: string; + title: string; + originalFilename: string; + thumbnailUrl?: string; + /** Format: date */ + documentDate?: string; + /** @enum {string} */ + metaDatePrecision: "DAY" | "MONTH" | "SEASON" | "YEAR" | "RANGE" | "APPROX" | "UNKNOWN"; + /** Format: date */ + metaDateEnd?: string; + sender?: components["schemas"]["Person"]; + receivers: components["schemas"]["Person"][]; + tags: components["schemas"]["Tag"][]; + archiveBox?: string; + archiveFolder?: string; + location?: string; + summary?: string; + /** Format: int32 */ + completionPercentage: number; + contributors: components["schemas"]["ActivityActorDTO"][]; + matchData: components["schemas"]["SearchMatchData"]; + /** Format: date-time */ + createdAt: string; + /** Format: date-time */ + updatedAt: string; + }; + DocumentSearchResult: { + items: components["schemas"]["DocumentListItem"][]; + /** Format: int64 */ + totalElements: number; + /** Format: int32 */ + pageNumber: number; + /** Format: int32 */ + pageSize: number; + /** Format: int32 */ + totalPages: number; + /** Format: int64 */ + undatedCount: number; + }; + MatchOffset: { + /** Format: int32 */ + start: number; + /** Format: int32 */ + length: number; + }; + SearchMatchData: { + transcriptionSnippet?: string; + titleOffsets: components["schemas"]["MatchOffset"][]; + senderMatched: boolean; + matchedReceiverIds: string[]; + matchedTagIds: string[]; + snippetOffsets: components["schemas"]["MatchOffset"][]; + summarySnippet?: string; + summaryOffsets: components["schemas"]["MatchOffset"][]; + }; IncompleteDocumentDTO: { /** Format: uuid */ id: string; @@ -3603,6 +3563,7 @@ export interface operations { query?: { status?: "DRAFT" | "PUBLISHED"; personId?: string[]; + documentId?: string; limit?: number; }; header?: never; @@ -3617,7 +3578,7 @@ export interface operations { [name: string]: unknown; }; content: { - "*/*": components["schemas"]["GeschichteSummary"][]; + "*/*": components["schemas"]["Geschichte"][]; }; }; }; @@ -4144,26 +4105,6 @@ export interface operations { }; }; }; - backfillTitles: { - parameters: { - query?: never; - header?: never; - path?: never; - cookie?: never; - }; - requestBody?: never; - responses: { - /** @description OK */ - 200: { - headers: { - [name: string]: unknown; - }; - content: { - "*/*": components["schemas"]["BackfillResult"]; - }; - }; - }; - }; backfillFileHashes: { parameters: { query?: never; @@ -4512,7 +4453,7 @@ export interface operations { }; }; }; - search_1: { + search: { parameters: { query?: { q?: string; @@ -5136,7 +5077,7 @@ export interface operations { }; }; }; - search_2: { + search_1: { parameters: { query?: { q?: string; @@ -5306,6 +5247,32 @@ export interface operations { }; }; }; + getConversation: { + parameters: { + query: { + senderId: string; + receiverId?: string; + from?: string; + to?: string; + dir?: string; + }; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["Document"][]; + }; + }; + }; + }; getResume: { parameters: { query?: never; diff --git a/frontend/src/lib/shared/errors.ts b/frontend/src/lib/shared/errors.ts index 6efec2a7..894d102a 100644 --- a/frontend/src/lib/shared/errors.ts +++ b/frontend/src/lib/shared/errors.ts @@ -46,6 +46,8 @@ export type ErrorCode = | 'CIRCULAR_RELATIONSHIP' | 'DUPLICATE_RELATIONSHIP' | 'GESCHICHTE_NOT_FOUND' + | 'JOURNEY_ITEM_NOT_FOUND' + | 'JOURNEY_ITEM_POSITION_CONFLICT' | 'INVALID_CREDENTIALS' | 'SESSION_EXPIRED' | 'MISSING_CREDENTIALS' @@ -164,6 +166,10 @@ export function getErrorMessage(code: ErrorCode | string | undefined): string { return m.error_duplicate_relationship(); case 'GESCHICHTE_NOT_FOUND': return m.error_geschichte_not_found(); + case 'JOURNEY_ITEM_NOT_FOUND': + return m.error_journey_item_not_found(); + case 'JOURNEY_ITEM_POSITION_CONFLICT': + return m.error_journey_item_position_conflict(); case 'INVALID_CREDENTIALS': return m.error_invalid_credentials(); case 'SESSION_EXPIRED': -- 2.49.1 From 6fc5ce6ddd6e36c231c038f9dafad4e42242eae4 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 17:06:25 +0200 Subject: [PATCH 10/41] docs: update GLOSSARY for JourneyItem view types; add ADR-035 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes GLOSSARY position-step value (1000→10), adds DEFERRABLE constraint note, and documents GeschichteView, JourneyItemView, and DocumentSummary read-model types. ADR-035 records the decision to use Optional for three-way PATCH semantics instead of jackson-databind-nullable (which targets Jackson 2.x and is incompatible with Spring Boot 4.0 / Jackson 3.x). Co-Authored-By: Claude Sonnet 4.6 --- docs/GLOSSARY.md | 8 +++- ...tional-string-three-way-patch-semantics.md | 43 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 docs/adr/035-optional-string-three-way-patch-semantics.md diff --git a/docs/GLOSSARY.md b/docs/GLOSSARY.md index 7dd3419a..e38eb070 100644 --- a/docs/GLOSSARY.md +++ b/docs/GLOSSARY.md @@ -151,7 +151,13 @@ _See also [Chronik](#chronik-internal)._ **Geschichte** (`Geschichte`) `[user-facing]` — a narrative story or curated document journey published in the archive. Two subtypes: `STORY` (free-form prose linking `Person`s) and `JOURNEY` (a *Lesereise* — an ordered sequence of `JourneyItem`s). Lifecycle: `DRAFT → PUBLISHED` (see `GeschichteStatus`). DRAFT stories are hidden from users without the `BLOG_WRITE` permission. -**JourneyItem** (`JourneyItem`, table `journey_items`) `[internal]` — a single stop in a *Lesereise* (`Geschichte` with `type=JOURNEY`). Either document-backed (`document_id IS NOT NULL`) or a note-only interlude (`note IS NOT NULL`). Ordered by `position` (gaps of 1000 leave room for drag-reorder). A CHECK constraint ensures at least one of `document_id` or `note` is present. The FK to `documents` uses `ON DELETE SET NULL`, so deleting a document preserves the item (with `document_id = null`). +**JourneyItem** (`JourneyItem`, table `journey_items`) `[internal]` — a single stop in a *Lesereise* (`Geschichte` with `type=JOURNEY`). Either document-backed (`document_id IS NOT NULL`) or a note-only interlude (`note IS NOT NULL`). Ordered by `position` (step of 10; max 100 items per journey). A DEFERRABLE UNIQUE constraint on `(geschichte_id, position)` allows atomic position swaps in the same transaction. A CHECK constraint ensures at least one of `document_id` or `note` is present. The FK to `documents` uses `ON DELETE SET NULL`, so deleting a document preserves the item (with `document_id = null`). + +**GeschichteView** (`GeschichteView`) `[internal]` — lean read-model record returned by `GeschichteService.getById()`. Contains `AuthorView` (id + displayName only — email not exposed) and a `List` loaded via a separate query rather than a lazy collection. + +**JourneyItemView** (`JourneyItemView`) `[internal]` — lean view record for a single `JourneyItem` surface, containing `id`, `position`, an optional `DocumentSummary`, and an optional `note`. + +**DocumentSummary** (`DocumentSummary`) `[internal]` — lean document read-model used inside `JourneyItemView`. Contains title, date, senderName, receiverName, receiverCount, datePrecision — no tags or file storage info. **Lesereise** `[user-facing]` — a curated reading journey through a sequence of family documents, optionally annotated with editorial notes. Implemented as a `Geschichte` with `type=JOURNEY`. The reader UI (follow-on issue) renders items as a sequential reading experience. diff --git a/docs/adr/035-optional-string-three-way-patch-semantics.md b/docs/adr/035-optional-string-three-way-patch-semantics.md new file mode 100644 index 00000000..29979bff --- /dev/null +++ b/docs/adr/035-optional-string-three-way-patch-semantics.md @@ -0,0 +1,43 @@ +# ADR-035 — `Optional` for three-way PATCH semantics + +**Status:** Accepted +**Date:** 2026-06-08 +**Issue:** #751 (JourneyItem CRUD API) + +## Context + +The `PATCH /api/geschichten/{id}/items/{itemId}` endpoint must distinguish three cases for the `note` field: + +| JSON body | Intended meaning | +|-------------------|-----------------------| +| `{"note": "text"}`| Set note to "text" | +| `{"note": null}` | Clear the note | +| `{}` (absent) | Leave note unchanged | + +The standard library for this on Jackson 2.x is `jackson-databind-nullable` (`JsonNullable` from `org.openapitools`). However, that library targets `com.fasterxml.jackson.*` (Jackson 2.x) and is incompatible with Spring Boot 4.0 / Spring Framework 7, which uses `tools.jackson.*` (Jackson 3.x). The module fails to register and throws at startup. + +## Decision + +Use `Optional` with Java's default field initializer (`= null`) to encode the three states: + +```java +@Data +public class JourneyItemUpdateDTO { + private Optional note = null; // Java default — absent = no-op +} +``` + +| Java value | JSON wire | Semantics | +|--------------------|-------------------|---------------| +| `null` (default) | field absent | no-op | +| `Optional.empty()` | `{"note": null}` | clear | +| `Optional.of("x")` | `{"note": "x"}` | set | + +Jackson 3.x natively maps a JSON `null` to `Optional.empty()` and leaves absent fields at their Java default. No custom module is needed. + +## Consequences + +- No external dependency for PATCH semantics — simpler pom.xml. +- The DTO field type is `Optional`, not `String` — service code must null-check the field first (`if (noteField == null) return;`) and then call `.orElse(null)` to unwrap. +- This pattern applies to any future PATCH DTO that needs three-way semantics on a nullable field. +- `jackson-databind-nullable` is removed from `pom.xml`; `JacksonConfig.java` is kept as a placeholder for future custom modules. -- 2.49.1 From 4eb6abd9201b2912b59a8223ca7de037c65ba76f Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 17:16:32 +0200 Subject: [PATCH 11/41] fix(review): address PR #788 review blockers - GlobalExceptionHandler maps uq_journey_items_geschichte_position constraint violation to HTTP 409 JOURNEY_ITEM_POSITION_CONFLICT - JourneyItemService.reorder() rejects duplicate IDs before set-equality check to prevent silent position overwrite - JourneyItemRepository removes orphaned findAllByGeschichteId method - GeschichteView removes stale com.fasterxml.jackson import - Tests: add appendItem_returns409_on_position_conflict (controller), reorder_returns400_when_itemIds_contain_duplicates (service) - Fix JourneyItemIntegrationTest compilation after repository cleanup - db-orm.puml annotates journey_items position with CHECK + UNIQUE DEFERRABLE Co-Authored-By: Claude Sonnet 4.6 --- .../exception/GlobalExceptionHandler.java | 9 ++++++++- .../geschichte/GeschichteView.java | 1 - .../journeyitem/JourneyItemRepository.java | 2 -- .../journeyitem/JourneyItemService.java | 5 +++++ .../geschichte/GeschichteControllerTest.java | 16 ++++++++++++++++ .../journeyitem/JourneyItemIntegrationTest.java | 2 +- .../journeyitem/JourneyItemServiceTest.java | 14 ++++++++++++++ docs/architecture/db/db-orm.puml | 4 +++- 8 files changed, 47 insertions(+), 6 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandler.java b/backend/src/main/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandler.java index 686ef457..c56cc576 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandler.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/GlobalExceptionHandler.java @@ -78,7 +78,14 @@ public class GlobalExceptionHandler { // Log the constraint NAME only — schema metadata, safe for Loki, and enough to tell which // constraint fired at 2am. Never pass `ex` / `ex.getMessage()`: those embed the SQL + the // offending values (CWE-209). No Sentry: an integrity violation is a 400, not a system fault. - log.warn("Rejected a request that violated a database integrity constraint: {}", constraintNameOf(ex)); + String constraint = constraintNameOf(ex); + log.warn("Rejected a request that violated a database integrity constraint: {}", constraint); + if ("uq_journey_items_geschichte_position".equals(constraint)) { + // DEFERRABLE INITIALLY DEFERRED — fires at commit when concurrent appends/reorders collide + return ResponseEntity.status(409) + .body(new ErrorResponse(ErrorCode.JOURNEY_ITEM_POSITION_CONFLICT, + "A position conflict was detected — another request modified this journey simultaneously")); + } return ResponseEntity.badRequest() .body(new ErrorResponse(ErrorCode.VALIDATION_ERROR, "The submitted data violated a database constraint")); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteView.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteView.java index 2fa5a920..e7d0e3f6 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteView.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteView.java @@ -1,6 +1,5 @@ package org.raddatz.familienarchiv.geschichte; -import com.fasterxml.jackson.annotation.JsonInclude; import io.swagger.v3.oas.annotations.media.Schema; import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemView; import org.raddatz.familienarchiv.person.Person; diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java index 4a56ee07..666681e6 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java @@ -13,8 +13,6 @@ import java.util.UUID; @Repository public interface JourneyItemRepository extends JpaRepository { - List findAllByGeschichteId(UUID geschichteId); - /** Returns items ordered by position ASC for the read-model assembly path. */ List findByGeschichteIdOrderByPosition(UUID geschichteId); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java index d0835c26..e898b026 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -138,6 +138,11 @@ public class JourneyItemService { Set existingIds = journeyItemRepository.findIdsByGeschichteId(geschichteId); List requestedIds = dto.getItemIds() != null ? dto.getItemIds() : List.of(); + if (requestedIds.size() != new HashSet<>(requestedIds).size()) { + throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, + "Duplicate item IDs in reorder request"); + } + if (!existingIds.equals(new HashSet<>(requestedIds))) { throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, "Requested item IDs do not match the journey's existing items"); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java index 88320dc2..0305358f 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java @@ -359,6 +359,22 @@ class GeschichteControllerTest { .andExpect(jsonPath("$[0].id").value(itemId.toString())); } + // ─── error mapping ─────────────────────────────────────────────────────── + + @Test + @WithMockUser(authorities = "BLOG_WRITE") + void appendItem_returns409_on_position_conflict() throws Exception { + UUID id = UUID.randomUUID(); + when(journeyItemService.append(eq(id), any())) + .thenThrow(DomainException.conflict(ErrorCode.JOURNEY_ITEM_POSITION_CONFLICT, "conflict")); + + mockMvc.perform(post("/api/geschichten/{id}/items", id).with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"note\":\"x\"}")) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.code").value("JOURNEY_ITEM_POSITION_CONFLICT")); + } + // ─── helpers ───────────────────────────────────────────────────────────── private JourneyItemView itemViewStub(UUID id, int position, String note) { diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java index 8de9c41e..00c2591c 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java @@ -96,7 +96,7 @@ class JourneyItemIntegrationTest { geschichteRepository.deleteById(geschichteId); em.flush(); - assertThat(journeyItemRepository.findAllByGeschichteId(geschichteId)).isEmpty(); + assertThat(journeyItemRepository.findByGeschichteIdOrderByPosition(geschichteId)).isEmpty(); } @Test diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java index a6afe3b6..2f3161e2 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java @@ -448,6 +448,20 @@ class JourneyItemServiceTest { // ─── reorder ───────────────────────────────────────────────────────────── + @Test + void reorder_returns400_when_itemIds_contain_duplicates() { + UUID id1 = UUID.randomUUID(); + when(journeyItemRepository.findIdsByGeschichteId(geschichteId)).thenReturn(Set.of(id1)); + + JourneyReorderDTO dto = new JourneyReorderDTO(); + dto.setItemIds(List.of(id1, id1)); // duplicate + + assertThatThrownBy(() -> journeyItemService.reorder(geschichteId, dto)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.VALIDATION_ERROR)); + } + @Test void reorder_returns400_when_itemId_belongs_to_different_journey() { UUID foreignId = UUID.randomUUID(); diff --git a/docs/architecture/db/db-orm.puml b/docs/architecture/db/db-orm.puml index 4f71c08a..a1ebd7d2 100644 --- a/docs/architecture/db/db-orm.puml +++ b/docs/architecture/db/db-orm.puml @@ -376,8 +376,10 @@ package "Supporting" { -- geschichte_id : UUID <> document_id : UUID <> - position : INTEGER NOT NULL + position : INTEGER NOT NULL CHECK (position > 0) note : TEXT + == + UNIQUE (geschichte_id, position) DEFERRABLE INITIALLY DEFERRED } } -- 2.49.1 From 2f471155b8a1dc078c0fda2ff57a83a942ca837a Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 18:09:40 +0200 Subject: [PATCH 12/41] =?UTF-8?q?fix(review):=20replace=20email=20fallback?= =?UTF-8?q?=20with=20[Unbekannt]=20in=20AuthorView=20=E2=80=94=20prevents?= =?UTF-8?q?=20CWE-359=20leak?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- .../raddatz/familienarchiv/geschichte/GeschichteService.java | 2 +- .../familienarchiv/geschichte/GeschichteServiceTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java index 37a1b75f..038346c9 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java @@ -78,7 +78,7 @@ public class GeschichteService { if (author != null) { String displayName = ((author.getFirstName() != null ? author.getFirstName() : "") + " " + (author.getLastName() != null ? author.getLastName() : "")).trim(); - if (displayName.isBlank()) displayName = author.getEmail(); + if (displayName.isBlank()) displayName = "[Unbekannt]"; authorView = new GeschichteView.AuthorView(author.getId(), displayName); } return new GeschichteView( diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java index 19c72c15..f72c7040 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java @@ -123,7 +123,7 @@ class GeschichteServiceTest { } @Test - void getById_author_displayName_falls_back_to_email_when_names_blank() { + void getById_author_displayName_falls_back_to_Unbekannt_when_names_blank() { authenticateAs(reader, Permission.READ_ALL); UUID id = UUID.randomUUID(); Geschichte published = published(id); @@ -133,7 +133,7 @@ class GeschichteServiceTest { GeschichteView result = geschichteService.getById(id); - assertThat(result.author().displayName()).isEqualTo("anon@test"); + assertThat(result.author().displayName()).isEqualTo("[Unbekannt]"); } @Test -- 2.49.1 From 7ed003266148a1f648cfbad9758bcd794b20cef5 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 18:12:13 +0200 Subject: [PATCH 13/41] =?UTF-8?q?fix(review):=20replace=20Set=20wi?= =?UTF-8?q?th=20Set=20in=20GeschichteView=20=E2=80=94=20preven?= =?UTF-8?q?ts=20leaking=20admin=20fields?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- .../geschichte/GeschichteService.java | 6 +++++- .../geschichte/GeschichteView.java | 10 ++++++++-- .../GeschichteServiceIntegrationTest.java | 2 +- .../geschichte/GeschichteServiceTest.java | 20 +++++++++++++++++++ 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java index 038346c9..82206f26 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java @@ -81,10 +81,14 @@ public class GeschichteService { if (displayName.isBlank()) displayName = "[Unbekannt]"; authorView = new GeschichteView.AuthorView(author.getId(), displayName); } + Set personViews = new HashSet<>(); + for (Person p : g.getPersons()) { + personViews.add(new GeschichteView.PersonView(p.getId(), p.getFirstName(), p.getLastName())); + } return new GeschichteView( g.getId(), g.getTitle(), g.getBody(), g.getStatus(), g.getType(), - authorView, g.getPersons(), + authorView, personViews, items, g.getPublishedAt(), g.getCreatedAt(), g.getUpdatedAt() ); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteView.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteView.java index e7d0e3f6..814e4038 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteView.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteView.java @@ -2,7 +2,6 @@ package org.raddatz.familienarchiv.geschichte; import io.swagger.v3.oas.annotations.media.Schema; import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemView; -import org.raddatz.familienarchiv.person.Person; import java.time.LocalDateTime; import java.util.List; @@ -21,7 +20,7 @@ public record GeschichteView( @Schema(requiredMode = Schema.RequiredMode.REQUIRED) GeschichteStatus status, @Schema(requiredMode = Schema.RequiredMode.REQUIRED) GeschichteType type, AuthorView author, - @Schema(requiredMode = Schema.RequiredMode.REQUIRED) Set persons, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) Set persons, @Schema(requiredMode = Schema.RequiredMode.REQUIRED) List items, LocalDateTime publishedAt, @Schema(requiredMode = Schema.RequiredMode.REQUIRED) LocalDateTime createdAt, @@ -32,4 +31,11 @@ public record GeschichteView( @Schema(requiredMode = Schema.RequiredMode.REQUIRED) UUID id, @Schema(requiredMode = Schema.RequiredMode.REQUIRED) String displayName ) {} + + /** Summarised person — exposes only id, firstName, and lastName. No admin-only fields. */ + public record PersonView( + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) UUID id, + String firstName, + String lastName + ) {} } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java index 39213aec..35f23a00 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java @@ -107,7 +107,7 @@ class GeschichteServiceIntegrationTest { assertThat(geschichteService.list(null, List.of(franz.getId()), 50)).hasSize(1); GeschichteView fetched = geschichteService.getById(draftId); assertThat(fetched.title()).isEqualTo("Erinnerung an Opa Franz"); - assertThat(fetched.persons()).extracting(Person::getId).containsExactly(franz.getId()); + assertThat(fetched.persons()).extracting(GeschichteView.PersonView::id).containsExactly(franz.getId()); // Delete as writer; join rows go with it authenticateAs(writer, Permission.BLOG_WRITE); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java index f72c7040..bf9f1432 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java @@ -153,6 +153,26 @@ class GeschichteServiceTest { assertThat(result.author().displayName()).doesNotContain("secret@test"); } + @Test + void getById_persons_are_mapped_to_PersonView() { + authenticateAs(reader, Permission.READ_ALL); + UUID id = UUID.randomUUID(); + UUID personId = UUID.randomUUID(); + Geschichte published = published(id); + published.setPersons(new HashSet<>(List.of( + Person.builder().id(personId).firstName("Franz").lastName("Raddatz").build() + ))); + when(geschichteRepository.findById(id)).thenReturn(Optional.of(published)); + + GeschichteView result = geschichteService.getById(id); + + assertThat(result.persons()).hasSize(1); + GeschichteView.PersonView pv = result.persons().iterator().next(); + assertThat(pv.id()).isEqualTo(personId); + assertThat(pv.firstName()).isEqualTo("Franz"); + assertThat(pv.lastName()).isEqualTo("Raddatz"); + } + @Test void getById_items_come_from_journeyItemService() { authenticateAs(reader, Permission.READ_ALL); -- 2.49.1 From 5539158e8ff13ca6fa5e6a80c30ae605e726ab16 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 18:12:50 +0200 Subject: [PATCH 14/41] fix(review): add JourneyItemService to C4 L3 supporting-domains diagram Co-Authored-By: Claude Sonnet 4.6 --- docs/architecture/c4/l3-backend-3g-supporting.puml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/architecture/c4/l3-backend-3g-supporting.puml b/docs/architecture/c4/l3-backend-3g-supporting.puml index ea08f206..11f45b1f 100644 --- a/docs/architecture/c4/l3-backend-3g-supporting.puml +++ b/docs/architecture/c4/l3-backend-3g-supporting.puml @@ -18,6 +18,7 @@ System_Boundary(backend, "API Backend (Spring Boot)") { Component(sseRegistry, "SseEmitterRegistry", "Spring Component", "In-memory ConcurrentHashMap of Spring SseEmitter instances per user. Handles registration, deregistration, and JSON event broadcasts.") Component(geschCtrl, "GeschichteController", "Spring MVC — /api/geschichten", "CRUD for publishable stories (STORY) and reading journeys (JOURNEY). Returns GeschichteSummary projections for list; full Geschichte with JourneyItems for detail. Requires BLOG_WRITE permission for write operations.") Component(geschSvc, "GeschichteService", "Spring Service", "Manages story lifecycle (DRAFT → PUBLISHED with timestamp). Supports two subtypes: STORY (prose) and JOURNEY (ordered JourneyItem sequence). Sanitizes HTML body with an allowlist policy.") + Component(journeyItemSvc, "JourneyItemService", "Spring Service", "Manages journey item lifecycle: append (100-item cap), updateNote (three-way PATCH), delete, and reorder (DEFERRABLE position swap). Enforces JOURNEY-type guard on append.") Component(exHandler, "GlobalExceptionHandler", "Spring @RestControllerAdvice", "Converts DomainException, validation errors, and generic exceptions to ErrorResponse JSON with machine-readable ErrorCode and HTTP status.") } @@ -38,6 +39,9 @@ Rel(notifCtrl, notifSvc, "Delegates to") Rel(notifCtrl, sseRegistry, "Registers client SSE connection") Rel(notifSvc, sseRegistry, "Broadcasts events to connected clients") Rel(geschCtrl, geschSvc, "Delegates to") +Rel(geschCtrl, journeyItemSvc, "Delegates journey item CRUD") +Rel(geschSvc, journeyItemSvc, "Delegates getItems()") +Rel(journeyItemSvc, db, "Reads / writes journey_items", "JDBC") Rel(auditSvc, db, "Writes audit_log", "JDBC") Rel(auditQuery, db, "Reads audit_log", "JDBC") Rel(notifSvc, db, "Reads / writes notifications", "JDBC") -- 2.49.1 From 97f22e1ce8dba86c31caa0fa109252e1ef5e0216 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 18:13:39 +0200 Subject: [PATCH 15/41] fix(review): friendlier i18n message for journey position conflict error Co-Authored-By: Claude Sonnet 4.6 --- frontend/messages/de.json | 2 +- frontend/messages/en.json | 2 +- frontend/messages/es.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend/messages/de.json b/frontend/messages/de.json index b0e3e054..f415a2ca 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -1024,7 +1024,7 @@ "nav_geschichten": "Geschichten", "error_geschichte_not_found": "Die Geschichte wurde nicht gefunden.", "error_journey_item_not_found": "Der Reise-Eintrag wurde nicht gefunden.", - "error_journey_item_position_conflict": "Positionskonflikt beim Sortieren der Reise-Einträge.", + "error_journey_item_position_conflict": "Die Reihenfolge wurde gerade von jemand anderem geändert – bitte laden Sie die Seite neu.", "geschichten_index_title": "Geschichten", "geschichten_new_button": "Neue Geschichte", "geschichten_filter_all_pill": "Alle", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 7bca4db1..e6776f27 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -1024,7 +1024,7 @@ "nav_geschichten": "Stories", "error_geschichte_not_found": "The story was not found.", "error_journey_item_not_found": "The journey item was not found.", - "error_journey_item_position_conflict": "Position conflict while reordering journey items.", + "error_journey_item_position_conflict": "The order was just changed by someone else — please reload the page.", "geschichten_index_title": "Stories", "geschichten_new_button": "New story", "geschichten_filter_all_pill": "All", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index ea5002d9..c7f8d1c1 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -1024,7 +1024,7 @@ "nav_geschichten": "Historias", "error_geschichte_not_found": "No se encontró la historia.", "error_journey_item_not_found": "No se encontró el elemento del viaje.", - "error_journey_item_position_conflict": "Conflicto de posición al reordenar los elementos del viaje.", + "error_journey_item_position_conflict": "El orden fue cambiado por otra persona — por favor recargue la página.", "geschichten_index_title": "Historias", "geschichten_new_button": "Nueva historia", "geschichten_filter_all_pill": "Todas", -- 2.49.1 From e157d90b53b730eb45eec63747df11bdbd75c964 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 18:58:20 +0200 Subject: [PATCH 16/41] docs(backend): add JourneyItemService and GeschichteQueryService to CLAUDE.md package table Co-Authored-By: Claude Sonnet 4.6 --- backend/CLAUDE.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/CLAUDE.md b/backend/CLAUDE.md index b96d242a..38d5b08b 100644 --- a/backend/CLAUDE.md +++ b/backend/CLAUDE.md @@ -33,7 +33,8 @@ src/main/java/org/raddatz/familienarchiv/ │ └── transcription/ # TranscriptionBlock, TranscriptionService, TranscriptionBlockQueryService ├── exception/ # DomainException, ErrorCode, GlobalExceptionHandler ├── filestorage/ # FileService (S3/MinIO) -├── geschichte/ # Geschichte (story) domain +├── geschichte/ # Geschichte (story) domain — GeschichteService, GeschichteQueryService +│ └── journeyitem/ # JourneyItem sub-domain — JourneyItemService, JourneyItemController ├── importing/ # CanonicalImportOrchestrator + 4 loaders + CanonicalSheetReader ├── notification/ # Notification domain + SseEmitterRegistry ├── ocr/ # OCR domain — OcrService, OcrBatchService, training -- 2.49.1 From 69db1983190c91f775714408d96ebfb55c723faf Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 19:00:32 +0200 Subject: [PATCH 17/41] refactor(geschichte): introduce GeschichteQueryService with existsById() Co-Authored-By: Claude Sonnet 4.6 --- .../geschichte/GeschichteQueryService.java | 23 +++++++++++ .../GeschichteQueryServiceTest.java | 38 +++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteQueryService.java create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteQueryServiceTest.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteQueryService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteQueryService.java new file mode 100644 index 00000000..488221db --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteQueryService.java @@ -0,0 +1,23 @@ +package org.raddatz.familienarchiv.geschichte; + +import lombok.RequiredArgsConstructor; +import org.springframework.stereotype.Service; + +import java.util.UUID; + +/** + * Thin read-only service owning {@link GeschichteRepository}. + * Exists so that {@code JourneyItemService} can check Geschichte existence + * without holding a direct reference to the Geschichte repository + * (cross-domain repository access is not allowed per layering rules). + */ +@Service +@RequiredArgsConstructor +public class GeschichteQueryService { + + private final GeschichteRepository geschichteRepository; + + public boolean existsById(UUID id) { + return geschichteRepository.existsById(id); + } +} diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteQueryServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteQueryServiceTest.java new file mode 100644 index 00000000..8cdc84db --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteQueryServiceTest.java @@ -0,0 +1,38 @@ +package org.raddatz.familienarchiv.geschichte; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class GeschichteQueryServiceTest { + + @Mock + GeschichteRepository geschichteRepository; + + @InjectMocks + GeschichteQueryService geschichteQueryService; + + @Test + void existsById_returns_true_when_geschichte_exists() { + UUID id = UUID.randomUUID(); + when(geschichteRepository.existsById(id)).thenReturn(true); + + assertThat(geschichteQueryService.existsById(id)).isTrue(); + } + + @Test + void existsById_returns_false_when_geschichte_does_not_exist() { + UUID id = UUID.randomUUID(); + when(geschichteRepository.existsById(id)).thenReturn(false); + + assertThat(geschichteQueryService.existsById(id)).isFalse(); + } +} -- 2.49.1 From 2ae1bb3a309f8be7412efc1495ff19c5f21bfb7c Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 19:02:03 +0200 Subject: [PATCH 18/41] fix(journey): reorder() throws 404 when Geschichte does not exist Co-Authored-By: Claude Sonnet 4.6 --- .../journeyitem/JourneyItemService.java | 6 ++++++ .../journeyitem/JourneyItemServiceTest.java | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java index e898b026..95bc32e3 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -10,6 +10,7 @@ import org.raddatz.familienarchiv.document.DocumentService; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.geschichte.Geschichte; +import org.raddatz.familienarchiv.geschichte.GeschichteQueryService; import org.raddatz.familienarchiv.geschichte.GeschichteRepository; import org.raddatz.familienarchiv.geschichte.GeschichteType; import org.raddatz.familienarchiv.geschichte.DocumentSummary; @@ -34,6 +35,7 @@ public class JourneyItemService { private final JourneyItemRepository journeyItemRepository; private final GeschichteRepository geschichteRepository; + private final GeschichteQueryService geschichteQueryService; private final DocumentService documentService; private final AuditService auditService; private final UserService userService; @@ -135,6 +137,10 @@ public class JourneyItemService { @Transactional public List reorder(UUID geschichteId, JourneyReorderDTO dto) { + if (!geschichteQueryService.existsById(geschichteId)) { + throw DomainException.notFound(ErrorCode.GESCHICHTE_NOT_FOUND, + "Journey not found: " + geschichteId); + } Set existingIds = journeyItemRepository.findIdsByGeschichteId(geschichteId); List requestedIds = dto.getItemIds() != null ? dto.getItemIds() : List.of(); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java index 2f3161e2..bb986049 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java @@ -15,6 +15,7 @@ import org.raddatz.familienarchiv.document.DocumentStatus; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.geschichte.Geschichte; +import org.raddatz.familienarchiv.geschichte.GeschichteQueryService; import org.raddatz.familienarchiv.geschichte.GeschichteRepository; import org.raddatz.familienarchiv.geschichte.GeschichteStatus; import org.raddatz.familienarchiv.geschichte.GeschichteType; @@ -48,6 +49,7 @@ class JourneyItemServiceTest { @Mock JourneyItemRepository journeyItemRepository; @Mock GeschichteRepository geschichteRepository; + @Mock GeschichteQueryService geschichteQueryService; @Mock DocumentService documentService; @Mock AuditService auditService; @Mock UserService userService; @@ -63,6 +65,7 @@ class JourneyItemServiceTest { void setupAuth() { AppUser actor = AppUser.builder().id(actorId).email("test@test.de").build(); lenient().when(userService.findByEmail("test@test.de")).thenReturn(actor); + lenient().when(geschichteQueryService.existsById(geschichteId)).thenReturn(true); SecurityContextHolder.getContext().setAuthentication( new UsernamePasswordAuthenticationToken("test@test.de", null, List.of(new SimpleGrantedAuthority("BLOG_WRITE")))); @@ -448,6 +451,20 @@ class JourneyItemServiceTest { // ─── reorder ───────────────────────────────────────────────────────────── + @Test + void reorder_unknownGeschichteId_throws404() { + UUID unknownId = UUID.randomUUID(); + // geschichteQueryService is not lenient-stubbed for unknownId → returns false + + JourneyReorderDTO dto = new JourneyReorderDTO(); + dto.setItemIds(List.of()); + + assertThatThrownBy(() -> journeyItemService.reorder(unknownId, dto)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.GESCHICHTE_NOT_FOUND)); + } + @Test void reorder_returns400_when_itemIds_contain_duplicates() { UUID id1 = UUID.randomUUID(); -- 2.49.1 From 7c06609816cf33e7ff894f11909dd5945d9e8d38 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 19:03:24 +0200 Subject: [PATCH 19/41] refactor(journey): make toView() and toSummary() package-private Co-Authored-By: Claude Sonnet 4.6 --- .../geschichte/journeyitem/JourneyItemService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java index 95bc32e3..6875fbf8 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -183,7 +183,7 @@ public class JourneyItemService { .stream().map(this::toView).toList(); } - public DocumentSummary toSummary(Document doc) { + DocumentSummary toSummary(Document doc) { String senderName = buildSenderName(doc); Set receivers = doc.getReceivers(); String receiverName = buildCanonicalReceiverName(receivers); @@ -200,7 +200,7 @@ public class JourneyItemService { ); } - public JourneyItemView toView(JourneyItem item) { + JourneyItemView toView(JourneyItem item) { DocumentSummary docSummary = null; if (item.getDocumentId() != null) { Document doc = documentService.getSummaryById(item.getDocumentId()); -- 2.49.1 From 147aa56386d47c477af0a0b2072d870013cc5a7a Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 19:04:43 +0200 Subject: [PATCH 20/41] feat(audit): add JOURNEY_ITEM_NOTE_UPDATED audit kind and wire into updateNote() Co-Authored-By: Claude Sonnet 4.6 --- .../raddatz/familienarchiv/audit/AuditKind.java | 3 +++ .../journeyitem/JourneyItemService.java | 5 +++++ .../journeyitem/JourneyItemServiceTest.java | 15 +++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java index 659de3c3..7b63ec27 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java @@ -60,6 +60,9 @@ public enum AuditKind { /** Payload: {@code {"geschichteId": "uuid", "itemId": "uuid"}} — documentId is null */ JOURNEY_ITEM_REMOVED, + /** Payload: {@code {"geschichteId": "uuid", "itemId": "uuid"}} — documentId is null */ + JOURNEY_ITEM_NOTE_UPDATED, + /** Payload: {@code {"geschichteId": "uuid", "itemCount": 3}} — documentId is null; rolled up in chronik */ JOURNEY_ITEMS_REORDERED; diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java index 6875fbf8..e05382ec 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -119,6 +119,11 @@ public class JourneyItemService { item.setNote(note); JourneyItem saved = journeyItemRepository.save(item); + + UUID actorId = currentUser().getId(); + auditService.logAfterCommit(AuditKind.JOURNEY_ITEM_NOTE_UPDATED, actorId, null, + Map.of("geschichteId", geschichteId, "itemId", itemId)); + return toView(saved); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java index bb986049..f3d9a97f 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java @@ -403,6 +403,21 @@ class JourneyItemServiceTest { .isEqualTo(ErrorCode.VALIDATION_ERROR)); } + @Test + void updateNote_auditsNoteUpdate() { + Geschichte journey = journey(geschichteId); + JourneyItem item = savedItem(itemId, journey, 10, null, null); + when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.of(item)); + JourneyItem saved = savedItem(itemId, journey, 10, null, "New note"); + when(journeyItemRepository.save(item)).thenReturn(saved); + + JourneyItemUpdateDTO dto = new JourneyItemUpdateDTO(); + dto.setNote(Optional.of("New note")); + journeyItemService.updateNote(geschichteId, itemId, dto); + + verify(auditService).logAfterCommit(eq(AuditKind.JOURNEY_ITEM_NOTE_UPDATED), eq(actorId), isNull(), any()); + } + @Test void patch_returns404_when_item_belongs_to_different_journey() { when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.empty()); -- 2.49.1 From 1fb0c41216f314f3b324ab41416d9d99d7cf5067 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 19:13:47 +0200 Subject: [PATCH 21/41] feat(error): add JOURNEY_AT_CAPACITY error code with i18n (de/en/es) Co-Authored-By: Claude Sonnet 4.6 --- .../java/org/raddatz/familienarchiv/exception/ErrorCode.java | 2 ++ frontend/messages/de.json | 1 + frontend/messages/en.json | 1 + frontend/messages/es.json | 1 + frontend/src/lib/shared/errors.ts | 3 +++ 5 files changed, 8 insertions(+) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java index a6acde3a..f8dd9565 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java @@ -126,6 +126,8 @@ public enum ErrorCode { JOURNEY_ITEM_NOT_FOUND, /** A position uniqueness conflict occurred on the journey_items table — concurrent append or reorder. 409 */ JOURNEY_ITEM_POSITION_CONFLICT, + /** The journey already has the maximum allowed number of items (100). 400 */ + JOURNEY_AT_CAPACITY, // --- Tags --- /** A tag with the given ID does not exist. 404 */ diff --git a/frontend/messages/de.json b/frontend/messages/de.json index f415a2ca..27628b4d 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -1025,6 +1025,7 @@ "error_geschichte_not_found": "Die Geschichte wurde nicht gefunden.", "error_journey_item_not_found": "Der Reise-Eintrag wurde nicht gefunden.", "error_journey_item_position_conflict": "Die Reihenfolge wurde gerade von jemand anderem geändert – bitte laden Sie die Seite neu.", + "error_journey_at_capacity": "Die Lesereise hat bereits die maximale Anzahl von Einträgen (100) erreicht.", "geschichten_index_title": "Geschichten", "geschichten_new_button": "Neue Geschichte", "geschichten_filter_all_pill": "Alle", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index e6776f27..690d475f 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -1025,6 +1025,7 @@ "error_geschichte_not_found": "The story was not found.", "error_journey_item_not_found": "The journey item was not found.", "error_journey_item_position_conflict": "The order was just changed by someone else — please reload the page.", + "error_journey_at_capacity": "The reading journey has already reached the maximum of 100 items.", "geschichten_index_title": "Stories", "geschichten_new_button": "New story", "geschichten_filter_all_pill": "All", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index c7f8d1c1..2b2885d4 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -1025,6 +1025,7 @@ "error_geschichte_not_found": "No se encontró la historia.", "error_journey_item_not_found": "No se encontró el elemento del viaje.", "error_journey_item_position_conflict": "El orden fue cambiado por otra persona — por favor recargue la página.", + "error_journey_at_capacity": "El viaje de lectura ya ha alcanzado el máximo de 100 entradas.", "geschichten_index_title": "Historias", "geschichten_new_button": "Nueva historia", "geschichten_filter_all_pill": "Todas", diff --git a/frontend/src/lib/shared/errors.ts b/frontend/src/lib/shared/errors.ts index 894d102a..424ae569 100644 --- a/frontend/src/lib/shared/errors.ts +++ b/frontend/src/lib/shared/errors.ts @@ -48,6 +48,7 @@ export type ErrorCode = | 'GESCHICHTE_NOT_FOUND' | 'JOURNEY_ITEM_NOT_FOUND' | 'JOURNEY_ITEM_POSITION_CONFLICT' + | 'JOURNEY_AT_CAPACITY' | 'INVALID_CREDENTIALS' | 'SESSION_EXPIRED' | 'MISSING_CREDENTIALS' @@ -170,6 +171,8 @@ export function getErrorMessage(code: ErrorCode | string | undefined): string { return m.error_journey_item_not_found(); case 'JOURNEY_ITEM_POSITION_CONFLICT': return m.error_journey_item_position_conflict(); + case 'JOURNEY_AT_CAPACITY': + return m.error_journey_at_capacity(); case 'INVALID_CREDENTIALS': return m.error_invalid_credentials(); case 'SESSION_EXPIRED': -- 2.49.1 From e400b1d77edcf2ac28358e3ed65a414abbf74d9f Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 19:15:45 +0200 Subject: [PATCH 22/41] feat(error): add GESCHICHTE_TYPE_MISMATCH error code with i18n (de/en/es) Co-Authored-By: Claude Sonnet 4.6 --- .../java/org/raddatz/familienarchiv/exception/ErrorCode.java | 2 ++ frontend/messages/de.json | 1 + frontend/messages/en.json | 1 + frontend/messages/es.json | 1 + frontend/src/lib/shared/errors.ts | 3 +++ 5 files changed, 8 insertions(+) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java index f8dd9565..1eaf8a2b 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java @@ -128,6 +128,8 @@ public enum ErrorCode { JOURNEY_ITEM_POSITION_CONFLICT, /** The journey already has the maximum allowed number of items (100). 400 */ JOURNEY_AT_CAPACITY, + /** The Geschichte is not of type JOURNEY — journey-item operations are not allowed on it. 400 */ + GESCHICHTE_TYPE_MISMATCH, // --- Tags --- /** A tag with the given ID does not exist. 404 */ diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 27628b4d..7b4b12f7 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -1026,6 +1026,7 @@ "error_journey_item_not_found": "Der Reise-Eintrag wurde nicht gefunden.", "error_journey_item_position_conflict": "Die Reihenfolge wurde gerade von jemand anderem geändert – bitte laden Sie die Seite neu.", "error_journey_at_capacity": "Die Lesereise hat bereits die maximale Anzahl von Einträgen (100) erreicht.", + "error_geschichte_type_mismatch": "Diese Geschichte ist keine Lesereise – Reise-Einträge sind hier nicht erlaubt.", "geschichten_index_title": "Geschichten", "geschichten_new_button": "Neue Geschichte", "geschichten_filter_all_pill": "Alle", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 690d475f..090b1157 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -1026,6 +1026,7 @@ "error_journey_item_not_found": "The journey item was not found.", "error_journey_item_position_conflict": "The order was just changed by someone else — please reload the page.", "error_journey_at_capacity": "The reading journey has already reached the maximum of 100 items.", + "error_geschichte_type_mismatch": "This story is not a reading journey — journey items are not allowed here.", "geschichten_index_title": "Stories", "geschichten_new_button": "New story", "geschichten_filter_all_pill": "All", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index 2b2885d4..4af6b658 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -1026,6 +1026,7 @@ "error_journey_item_not_found": "No se encontró el elemento del viaje.", "error_journey_item_position_conflict": "El orden fue cambiado por otra persona — por favor recargue la página.", "error_journey_at_capacity": "El viaje de lectura ya ha alcanzado el máximo de 100 entradas.", + "error_geschichte_type_mismatch": "Esta historia no es un viaje de lectura — los elementos de viaje no están permitidos aquí.", "geschichten_index_title": "Historias", "geschichten_new_button": "Nueva historia", "geschichten_filter_all_pill": "Todas", diff --git a/frontend/src/lib/shared/errors.ts b/frontend/src/lib/shared/errors.ts index 424ae569..59a0a846 100644 --- a/frontend/src/lib/shared/errors.ts +++ b/frontend/src/lib/shared/errors.ts @@ -49,6 +49,7 @@ export type ErrorCode = | 'JOURNEY_ITEM_NOT_FOUND' | 'JOURNEY_ITEM_POSITION_CONFLICT' | 'JOURNEY_AT_CAPACITY' + | 'GESCHICHTE_TYPE_MISMATCH' | 'INVALID_CREDENTIALS' | 'SESSION_EXPIRED' | 'MISSING_CREDENTIALS' @@ -173,6 +174,8 @@ export function getErrorMessage(code: ErrorCode | string | undefined): string { return m.error_journey_item_position_conflict(); case 'JOURNEY_AT_CAPACITY': return m.error_journey_at_capacity(); + case 'GESCHICHTE_TYPE_MISMATCH': + return m.error_geschichte_type_mismatch(); case 'INVALID_CREDENTIALS': return m.error_invalid_credentials(); case 'SESSION_EXPIRED': -- 2.49.1 From c5611250ec0801e725c1366c8e43411e852ef64c Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 19:17:25 +0200 Subject: [PATCH 23/41] test(journey): rename updateItemNote test to clarify Optional deserialization semantics Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/geschichte/GeschichteControllerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java index 0305358f..059223d6 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java @@ -270,7 +270,7 @@ class GeschichteControllerTest { @Test @WithMockUser(authorities = "BLOG_WRITE") - void updateItemNote_null_note_deserializes_as_present_null() throws Exception { + void updateItemNote_json_null_note_is_deserialized_as_empty_Optional() throws Exception { UUID id = UUID.randomUUID(); UUID itemId = UUID.randomUUID(); when(journeyItemService.updateNote(eq(id), eq(itemId), any())) -- 2.49.1 From 598ad622e795b4b5d68c1e6c52f5415be6df3dc3 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 19:31:30 +0200 Subject: [PATCH 24/41] =?UTF-8?q?fix(journeyitem):=20use=20specific=20erro?= =?UTF-8?q?r=20codes=20in=20append()=20=E2=80=94=20JOURNEY=5FAT=5FCAPACITY?= =?UTF-8?q?=20and=20GESCHICHTE=5FTYPE=5FMISMATCH?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - JourneyItemService.append(): replace VALIDATION_ERROR with GESCHICHTE_TYPE_MISMATCH (409 conflict) for non-JOURNEY type guard and JOURNEY_AT_CAPACITY (409 conflict) for 100-item cap - JourneyItemServiceTest: update assertions to expect the new specific error codes - CLAUDE.md: expand geschichte/ package table entry with GeschichteQueryService and journeyitem/ sub-domain Co-Authored-By: Claude Sonnet 4.6 --- CLAUDE.md | 3 ++- .../geschichte/journeyitem/JourneyItemService.java | 8 ++++---- .../geschichte/journeyitem/JourneyItemServiceTest.java | 8 ++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 639dd2ba..bba08400 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -86,7 +86,8 @@ backend/src/main/java/org/raddatz/familienarchiv/ │ └── transcription/ TranscriptionBlock, TranscriptionService, TranscriptionBlockQueryService ├── exception/ DomainException, ErrorCode, GlobalExceptionHandler ├── filestorage/ FileService (S3/MinIO) -├── geschichte/ Geschichte (story) domain +├── geschichte/ Geschichte (story) domain — GeschichteService, GeschichteQueryService +│ └── journeyitem/ JourneyItem sub-domain — JourneyItemService, JourneyItemController ├── importing/ CanonicalImportOrchestrator + four loaders (TagTree/PersonRegister/PersonTree/Document) + CanonicalSheetReader ├── notification/ Notification domain + SseEmitterRegistry ├── ocr/ OCR domain — OcrService, OcrBatchService, training diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java index e05382ec..5b83af6a 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -47,14 +47,14 @@ public class JourneyItemService { "Journey not found: " + geschichteId)); if (g.getType() != GeschichteType.JOURNEY) { - throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, - "Geschichte is not a JOURNEY — cannot append items"); + throw DomainException.conflict(ErrorCode.GESCHICHTE_TYPE_MISMATCH, + "Journey items can only be added to a JOURNEY-type Geschichte"); } long count = journeyItemRepository.countByGeschichteId(geschichteId); if (count >= MAX_ITEMS) { - throw DomainException.badRequest(ErrorCode.VALIDATION_ERROR, - "Journey already has the maximum of " + MAX_ITEMS + " items"); + throw DomainException.conflict(ErrorCode.JOURNEY_AT_CAPACITY, + "Journey has reached the maximum of 100 items"); } String note = normalizeNote(dto.getNote()); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java index f3d9a97f..14e299d9 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java @@ -221,7 +221,7 @@ class JourneyItemServiceTest { } @Test - void append_returns400_on_non_JOURNEY_type() { + void append_returns409_on_non_JOURNEY_type() { Geschichte story = Geschichte.builder() .id(geschichteId) .title("Story") @@ -236,7 +236,7 @@ class JourneyItemServiceTest { assertThatThrownBy(() -> journeyItemService.append(geschichteId, dto)) .isInstanceOf(DomainException.class) .satisfies(e -> assertThat(((DomainException) e).getCode()) - .isEqualTo(ErrorCode.VALIDATION_ERROR)); + .isEqualTo(ErrorCode.GESCHICHTE_TYPE_MISMATCH)); } @Test @@ -257,7 +257,7 @@ class JourneyItemServiceTest { } @Test - void append_returns400_when_100_items_exist() { + void append_returns409_when_100_items_exist() { Geschichte journey = journey(geschichteId); when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(100L); @@ -268,7 +268,7 @@ class JourneyItemServiceTest { assertThatThrownBy(() -> journeyItemService.append(geschichteId, dto)) .isInstanceOf(DomainException.class) .satisfies(e -> assertThat(((DomainException) e).getCode()) - .isEqualTo(ErrorCode.VALIDATION_ERROR)); + .isEqualTo(ErrorCode.JOURNEY_AT_CAPACITY)); } @Test -- 2.49.1 From 7ba6342a84a9cffc25f8e8477a7925c732af9e79 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 20:02:51 +0200 Subject: [PATCH 25/41] chore(config): remove empty JacksonConfig placeholder Co-Authored-By: Claude Sonnet 4.6 --- .../raddatz/familienarchiv/config/JacksonConfig.java | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java b/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java deleted file mode 100644 index 43a597d3..00000000 --- a/backend/src/main/java/org/raddatz/familienarchiv/config/JacksonConfig.java +++ /dev/null @@ -1,10 +0,0 @@ -package org.raddatz.familienarchiv.config; - -import org.springframework.context.annotation.Configuration; - -/** - * Jackson customisations. Currently a placeholder — custom modules added here as needed. - */ -@Configuration -public class JacksonConfig { -} -- 2.49.1 From 4a0fed617af02fcd04812242f39ae619fe3c91a9 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 20:04:21 +0200 Subject: [PATCH 26/41] refactor(geschichte): route all reads through GeschichteQueryService JourneyItemService no longer injects GeschichteRepository directly. GeschichteQueryService gains findById() so JourneyItemService.append() can load the Geschichte entity via the service layer, satisfying the cross-domain layering rule. Co-Authored-By: Claude Sonnet 4.6 --- .../geschichte/GeschichteQueryService.java | 10 ++++++-- .../journeyitem/JourneyItemService.java | 4 +--- .../journeyitem/JourneyItemServiceTest.java | 24 ++++++++----------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteQueryService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteQueryService.java index 488221db..cde834bd 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteQueryService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteQueryService.java @@ -3,13 +3,15 @@ package org.raddatz.familienarchiv.geschichte; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Service; +import java.util.Optional; import java.util.UUID; /** * Thin read-only service owning {@link GeschichteRepository}. * Exists so that {@code JourneyItemService} can check Geschichte existence - * without holding a direct reference to the Geschichte repository - * (cross-domain repository access is not allowed per layering rules). + * and load Geschichte instances without holding a direct reference to the + * Geschichte repository (cross-domain repository access is not allowed per + * layering rules). */ @Service @RequiredArgsConstructor @@ -20,4 +22,8 @@ public class GeschichteQueryService { public boolean existsById(UUID id) { return geschichteRepository.existsById(id); } + + public Optional findById(UUID id) { + return geschichteRepository.findById(id); + } } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java index 5b83af6a..d6fc0daf 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -11,7 +11,6 @@ import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.geschichte.Geschichte; import org.raddatz.familienarchiv.geschichte.GeschichteQueryService; -import org.raddatz.familienarchiv.geschichte.GeschichteRepository; import org.raddatz.familienarchiv.geschichte.GeschichteType; import org.raddatz.familienarchiv.geschichte.DocumentSummary; import org.raddatz.familienarchiv.person.Person; @@ -34,7 +33,6 @@ public class JourneyItemService { static final int MAX_NOTE_LENGTH = 5000; private final JourneyItemRepository journeyItemRepository; - private final GeschichteRepository geschichteRepository; private final GeschichteQueryService geschichteQueryService; private final DocumentService documentService; private final AuditService auditService; @@ -42,7 +40,7 @@ public class JourneyItemService { @Transactional public JourneyItemView append(UUID geschichteId, JourneyItemCreateDTO dto) { - Geschichte g = geschichteRepository.findById(geschichteId) + Geschichte g = geschichteQueryService.findById(geschichteId) .orElseThrow(() -> DomainException.notFound(ErrorCode.GESCHICHTE_NOT_FOUND, "Journey not found: " + geschichteId)); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java index 14e299d9..8576141e 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java @@ -16,7 +16,6 @@ import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.geschichte.Geschichte; import org.raddatz.familienarchiv.geschichte.GeschichteQueryService; -import org.raddatz.familienarchiv.geschichte.GeschichteRepository; import org.raddatz.familienarchiv.geschichte.GeschichteStatus; import org.raddatz.familienarchiv.geschichte.GeschichteType; import org.raddatz.familienarchiv.person.Person; @@ -26,7 +25,6 @@ import org.springframework.security.authentication.UsernamePasswordAuthenticatio import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.context.SecurityContextHolder; -import java.time.LocalDate; import java.util.HashSet; import java.util.List; import java.util.Optional; @@ -36,7 +34,6 @@ import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.lenient; @@ -48,7 +45,6 @@ import static org.mockito.Mockito.when; class JourneyItemServiceTest { @Mock JourneyItemRepository journeyItemRepository; - @Mock GeschichteRepository geschichteRepository; @Mock GeschichteQueryService geschichteQueryService; @Mock DocumentService documentService; @Mock AuditService auditService; @@ -148,7 +144,7 @@ class JourneyItemServiceTest { @Test void append_to_empty_journey_starts_at_10() { Geschichte journey = journey(geschichteId); - when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(journey)); when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.empty()); JourneyItem saved = savedItem(itemId, journey, 10, null, "Note"); @@ -165,7 +161,7 @@ class JourneyItemServiceTest { @Test void append_after_reorder_continues_from_max_position() { Geschichte journey = journey(geschichteId); - when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(journey)); when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(2L); when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.of(40)); JourneyItem saved = savedItem(itemId, journey, 50, null, "Note"); @@ -182,7 +178,7 @@ class JourneyItemServiceTest { @Test void append_returns400_when_neither_documentId_nor_note() { Geschichte journey = journey(geschichteId); - when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(journey)); when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); @@ -195,7 +191,7 @@ class JourneyItemServiceTest { @Test void append_returns400_when_note_trims_to_empty_and_no_document() { Geschichte journey = journey(geschichteId); - when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(journey)); when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); @@ -208,7 +204,7 @@ class JourneyItemServiceTest { @Test void append_returns400_when_note_exceeds_5000_chars() { Geschichte journey = journey(geschichteId); - when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(journey)); when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); @@ -228,7 +224,7 @@ class JourneyItemServiceTest { .type(GeschichteType.STORY) .status(GeschichteStatus.DRAFT) .build(); - when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(story)); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(story)); JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); dto.setNote("Note"); @@ -242,7 +238,7 @@ class JourneyItemServiceTest { @Test void append_returns404_when_documentId_does_not_exist() { Geschichte journey = journey(geschichteId); - when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(journey)); when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); when(documentService.getSummaryById(docId)) .thenThrow(DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "not found")); @@ -259,7 +255,7 @@ class JourneyItemServiceTest { @Test void append_returns409_when_100_items_exist() { Geschichte journey = journey(geschichteId); - when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(journey)); when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(100L); JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); @@ -275,7 +271,7 @@ class JourneyItemServiceTest { void cap_is_COUNT_based_not_MAX_position_based() { // 99 rows with MAX(position)=2000 should still accept the 100th append Geschichte journey = journey(geschichteId); - when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(journey)); when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(99L); when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.of(2000)); JourneyItem saved = savedItem(itemId, journey, 2010, null, "Note"); @@ -290,7 +286,7 @@ class JourneyItemServiceTest { @Test void append_audits_JOURNEY_ITEM_ADDED() { Geschichte journey = journey(geschichteId); - when(geschichteRepository.findById(geschichteId)).thenReturn(Optional.of(journey)); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(journey)); when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.empty()); JourneyItem saved = savedItem(itemId, journey, 10, null, "Note"); -- 2.49.1 From 3d80bc656cd7caff1363bb1a724a20a472e60574 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 20:05:28 +0200 Subject: [PATCH 27/41] refactor(journeyitem): use saveAll in reorder for efficiency Replace the per-item save() loop in reorder() with a single saveAll() call, reducing database round-trips for large journeys. Co-Authored-By: Claude Sonnet 4.6 --- .../geschichte/journeyitem/JourneyItemService.java | 5 +++-- .../geschichte/journeyitem/JourneyItemServiceTest.java | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java index d6fc0daf..4e0a78d8 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -167,12 +167,13 @@ public class JourneyItemService { itemMap.put(item.getId(), item); } - List reordered = new ArrayList<>(requestedIds.size()); + List toSave = new ArrayList<>(requestedIds.size()); for (int i = 0; i < requestedIds.size(); i++) { JourneyItem item = itemMap.get(requestedIds.get(i)); item.setPosition((i + 1) * POSITION_STEP); - reordered.add(journeyItemRepository.save(item)); + toSave.add(item); } + List reordered = journeyItemRepository.saveAll(toSave); UUID actorId = currentUser().getId(); auditService.logAfterCommit(AuditKind.JOURNEY_ITEMS_REORDERED, actorId, null, diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java index 8576141e..15ebcd87 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java @@ -551,7 +551,7 @@ class JourneyItemServiceTest { JourneyItem item2 = savedItem(id2, journey, 10, null, "B"); when(journeyItemRepository.findIdsByGeschichteId(geschichteId)).thenReturn(Set.of(id1, id2)); when(journeyItemRepository.findByGeschichteIdOrderByPosition(geschichteId)).thenReturn(List.of(item2, item1)); - when(journeyItemRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(journeyItemRepository.saveAll(any())).thenAnswer(inv -> inv.getArgument(0)); JourneyReorderDTO dto = new JourneyReorderDTO(); dto.setItemIds(List.of(id1, id2)); // want id1 first @@ -572,7 +572,7 @@ class JourneyItemServiceTest { JourneyItem item1 = savedItem(id1, journey, 10, null, "A"); when(journeyItemRepository.findIdsByGeschichteId(geschichteId)).thenReturn(Set.of(id1)); when(journeyItemRepository.findByGeschichteIdOrderByPosition(geschichteId)).thenReturn(List.of(item1)); - when(journeyItemRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(journeyItemRepository.saveAll(any())).thenAnswer(inv -> inv.getArgument(0)); JourneyReorderDTO dto = new JourneyReorderDTO(); dto.setItemIds(List.of(id1)); @@ -596,7 +596,7 @@ class JourneyItemServiceTest { } when(journeyItemRepository.findIdsByGeschichteId(geschichteId)).thenReturn(new HashSet<>(ids)); when(journeyItemRepository.findByGeschichteIdOrderByPosition(geschichteId)).thenReturn(items); - when(journeyItemRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(journeyItemRepository.saveAll(any())).thenAnswer(inv -> inv.getArgument(0)); JourneyReorderDTO dto = new JourneyReorderDTO(); dto.setItemIds(ids); @@ -613,7 +613,7 @@ class JourneyItemServiceTest { JourneyItem item1 = savedItem(id1, journey, 10, null, "A"); when(journeyItemRepository.findIdsByGeschichteId(geschichteId)).thenReturn(Set.of(id1)); when(journeyItemRepository.findByGeschichteIdOrderByPosition(geschichteId)).thenReturn(List.of(item1)); - when(journeyItemRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(journeyItemRepository.saveAll(any())).thenAnswer(inv -> inv.getArgument(0)); JourneyReorderDTO dto = new JourneyReorderDTO(); dto.setItemIds(List.of(id1)); -- 2.49.1 From 5b2ee312925118cba35aa76e149b88379e7e1ebf Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 20:06:09 +0200 Subject: [PATCH 28/41] feat(i18n): add journey_item_document_deleted placeholder key Adds de/en/es translations for the case where a JourneyItem's linked document has been deleted (document field is null), so the UI PR can display a meaningful fallback string. Co-Authored-By: Claude Sonnet 4.6 --- frontend/messages/de.json | 1 + frontend/messages/en.json | 1 + frontend/messages/es.json | 1 + 3 files changed, 3 insertions(+) diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 7b4b12f7..928460c3 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -1027,6 +1027,7 @@ "error_journey_item_position_conflict": "Die Reihenfolge wurde gerade von jemand anderem geändert – bitte laden Sie die Seite neu.", "error_journey_at_capacity": "Die Lesereise hat bereits die maximale Anzahl von Einträgen (100) erreicht.", "error_geschichte_type_mismatch": "Diese Geschichte ist keine Lesereise – Reise-Einträge sind hier nicht erlaubt.", + "journey_item_document_deleted": "[Dokument gelöscht]", "geschichten_index_title": "Geschichten", "geschichten_new_button": "Neue Geschichte", "geschichten_filter_all_pill": "Alle", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 090b1157..e10ef624 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -1027,6 +1027,7 @@ "error_journey_item_position_conflict": "The order was just changed by someone else — please reload the page.", "error_journey_at_capacity": "The reading journey has already reached the maximum of 100 items.", "error_geschichte_type_mismatch": "This story is not a reading journey — journey items are not allowed here.", + "journey_item_document_deleted": "[Document deleted]", "geschichten_index_title": "Stories", "geschichten_new_button": "New story", "geschichten_filter_all_pill": "All", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index 4af6b658..6a8d9eb3 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -1027,6 +1027,7 @@ "error_journey_item_position_conflict": "El orden fue cambiado por otra persona — por favor recargue la página.", "error_journey_at_capacity": "El viaje de lectura ya ha alcanzado el máximo de 100 entradas.", "error_geschichte_type_mismatch": "Esta historia no es un viaje de lectura — los elementos de viaje no están permitidos aquí.", + "journey_item_document_deleted": "[Documento eliminado]", "geschichten_index_title": "Historias", "geschichten_new_button": "Nueva historia", "geschichten_filter_all_pill": "Todas", -- 2.49.1 From 3f36d2a7f18c2ea5954bd0d08afe069a680cb1b7 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 20:07:03 +0200 Subject: [PATCH 29/41] chore(test): remove JacksonConfig from GeschichteControllerTest @Import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JacksonConfig was deleted (empty placeholder) — remove the now-stale import and @Import reference from the controller slice test. Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/geschichte/GeschichteControllerTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java index 059223d6..865bc73c 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java @@ -2,7 +2,6 @@ package org.raddatz.familienarchiv.geschichte; import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.Test; -import org.raddatz.familienarchiv.config.JacksonConfig; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemService; @@ -40,7 +39,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @WebMvcTest(GeschichteController.class) -@Import({SecurityConfig.class, PermissionAspect.class, AopAutoConfiguration.class, JacksonConfig.class}) +@Import({SecurityConfig.class, PermissionAspect.class, AopAutoConfiguration.class}) class GeschichteControllerTest { @Autowired -- 2.49.1 From 164178ecf1bd1f24b83996f9ebb6a34becc36011 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 20:45:09 +0200 Subject: [PATCH 30/41] =?UTF-8?q?refactor(geschichte):=20assemble=20Geschi?= =?UTF-8?q?chteView=20in=20controller=20=E2=80=94=20break=20GeschichteServ?= =?UTF-8?q?ice=E2=86=94JourneyItemService=20cycle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GeschichteService.getById() now returns the Geschichte entity (with the DRAFT visibility guard intact). The controller calls journeyItemService.getItems() and geschichteService.toView() to assemble the GeschichteView, removing the need for GeschichteService to hold a reference to JourneyItemService. Tests updated accordingly: GeschichteServiceTest tests toView() directly; GeschichteControllerTest stubs both service calls; integration test uses the two-step pattern. Co-Authored-By: Claude Sonnet 4.6 --- .../geschichte/GeschichteController.java | 5 +- .../geschichte/GeschichteService.java | 11 +- .../geschichte/GeschichteControllerTest.java | 5 +- .../GeschichteServiceIntegrationTest.java | 10 +- .../geschichte/GeschichteServiceTest.java | 163 ++++++++---------- 5 files changed, 92 insertions(+), 102 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java index baf365a4..8113238c 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java @@ -8,6 +8,7 @@ import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemView; import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyReorderDTO; import org.raddatz.familienarchiv.security.Permission; import org.raddatz.familienarchiv.security.RequirePermission; +import io.swagger.v3.oas.annotations.Operation; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; @@ -45,7 +46,9 @@ public class GeschichteController { @GetMapping("/{id}") public GeschichteView getById(@PathVariable UUID id) { - return geschichteService.getById(id); + Geschichte g = geschichteService.getById(id); + List items = journeyItemService.getItems(g.getId()); + return geschichteService.toView(g, items); } @PostMapping diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java index 82206f26..857ce314 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java @@ -6,7 +6,6 @@ import org.owasp.html.HtmlPolicyBuilder; import org.owasp.html.PolicyFactory; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; -import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemService; import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemView; import org.raddatz.familienarchiv.user.AppUser; import org.raddatz.familienarchiv.person.Person; @@ -36,7 +35,6 @@ public class GeschichteService { private final PersonService personService; private final DocumentService documentService; private final UserService userService; - private final JourneyItemService journeyItemService; /** * Allow-list policy for Geschichte body HTML. Tiptap on the writer side @@ -57,7 +55,7 @@ public class GeschichteService { } @Transactional(readOnly = true) - public GeschichteView getById(UUID id) { + public Geschichte getById(UUID id) { Geschichte g = geschichteRepository.findById(id) .orElseThrow(() -> DomainException.notFound( ErrorCode.GESCHICHTE_NOT_FOUND, "Geschichte not found: " + id)); @@ -66,13 +64,10 @@ public class GeschichteService { throw DomainException.notFound( ErrorCode.GESCHICHTE_NOT_FOUND, "Geschichte not found: " + id); } - // Items loaded via repository query — never through the LAZY collection on Geschichte. - // This keeps open-in-view:false safe without Hibernate.initialize. - List items = journeyItemService.getItems(id); - return toView(g, items); + return g; } - private GeschichteView toView(Geschichte g, List items) { + GeschichteView toView(Geschichte g, List items) { AppUser author = g.getAuthor(); GeschichteView.AuthorView authorView = null; if (author != null) { diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java index 865bc73c..df0d07d8 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java @@ -105,7 +105,10 @@ class GeschichteControllerTest { @WithMockUser(authorities = "READ_ALL") void getById_returns200_whenFound() throws Exception { UUID id = UUID.randomUUID(); - when(geschichteService.getById(id)).thenReturn(viewStub(id, "Hello")); + Geschichte g = published(id, "Hello"); + when(geschichteService.getById(id)).thenReturn(g); + when(journeyItemService.getItems(id)).thenReturn(List.of()); + when(geschichteService.toView(g, List.of())).thenReturn(viewStub(id, "Hello")); mockMvc.perform(get("/api/geschichten/{id}", id)) .andExpect(status().isOk()) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java index 35f23a00..11db3c1a 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceIntegrationTest.java @@ -8,10 +8,12 @@ import org.raddatz.familienarchiv.geschichte.GeschichteUpdateDTO; import org.raddatz.familienarchiv.user.AppUser; import org.raddatz.familienarchiv.geschichte.Geschichte; import org.raddatz.familienarchiv.geschichte.GeschichteStatus; +import org.raddatz.familienarchiv.geschichte.GeschichteType; import org.raddatz.familienarchiv.geschichte.GeschichteView; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.user.AppUserRepository; import org.raddatz.familienarchiv.geschichte.GeschichteRepository; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemService; import org.raddatz.familienarchiv.person.PersonRepository; import org.raddatz.familienarchiv.security.Permission; import org.springframework.beans.factory.annotation.Autowired; @@ -40,6 +42,7 @@ class GeschichteServiceIntegrationTest { S3Client s3Client; @Autowired GeschichteService geschichteService; + @Autowired JourneyItemService journeyItemService; @Autowired GeschichteRepository geschichteRepository; @Autowired PersonRepository personRepository; @Autowired AppUserRepository appUserRepository; @@ -105,9 +108,10 @@ class GeschichteServiceIntegrationTest { authenticateAs(reader, Permission.READ_ALL); assertThat(geschichteService.list(null, List.of(), 50)).hasSize(1); assertThat(geschichteService.list(null, List.of(franz.getId()), 50)).hasSize(1); - GeschichteView fetched = geschichteService.getById(draftId); - assertThat(fetched.title()).isEqualTo("Erinnerung an Opa Franz"); - assertThat(fetched.persons()).extracting(GeschichteView.PersonView::id).containsExactly(franz.getId()); + Geschichte fetched = geschichteService.getById(draftId); + GeschichteView fetchedView = geschichteService.toView(fetched, journeyItemService.getItems(draftId)); + assertThat(fetchedView.title()).isEqualTo("Erinnerung an Opa Franz"); + assertThat(fetchedView.persons()).extracting(GeschichteView.PersonView::id).containsExactly(franz.getId()); // Delete as writer; join rows go with it authenticateAs(writer, Permission.BLOG_WRITE); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java index bf9f1432..3f154cdc 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java @@ -9,7 +9,6 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; -import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemService; import org.raddatz.familienarchiv.user.AppUser; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.security.Permission; @@ -46,7 +45,6 @@ class GeschichteServiceTest { @Mock PersonService personService; @Mock DocumentService documentService; @Mock UserService userService; - @Mock JourneyItemService journeyItemService; @InjectMocks GeschichteService geschichteService; @@ -58,7 +56,6 @@ class GeschichteServiceTest { SecurityContextHolder.clearContext(); writer = AppUser.builder().id(UUID.randomUUID()).email("writer@test").build(); reader = AppUser.builder().id(UUID.randomUUID()).email("reader@test").build(); - lenient().when(journeyItemService.getItems(any())).thenReturn(List.of()); } @AfterEach @@ -88,10 +85,10 @@ class GeschichteServiceTest { Geschichte draft = draft(id); when(geschichteRepository.findById(id)).thenReturn(Optional.of(draft)); - GeschichteView result = geschichteService.getById(id); + Geschichte result = geschichteService.getById(id); - assertThat(result.id()).isEqualTo(id); - assertThat(result.status()).isEqualTo(GeschichteStatus.DRAFT); + assertThat(result.getId()).isEqualTo(id); + assertThat(result.getStatus()).isEqualTo(GeschichteStatus.DRAFT); } @Test @@ -101,90 +98,10 @@ class GeschichteServiceTest { Geschichte published = published(id); when(geschichteRepository.findById(id)).thenReturn(Optional.of(published)); - GeschichteView result = geschichteService.getById(id); + Geschichte result = geschichteService.getById(id); - assertThat(result.id()).isEqualTo(id); - assertThat(result.status()).isEqualTo(GeschichteStatus.PUBLISHED); - } - - @Test - void getById_author_displayName_uses_firstName_lastName() { - authenticateAs(reader, Permission.READ_ALL); - UUID id = UUID.randomUUID(); - Geschichte published = published(id); - published.setAuthor(AppUser.builder() - .id(UUID.randomUUID()).email("author@test") - .firstName("Hans").lastName("Raddatz").build()); - when(geschichteRepository.findById(id)).thenReturn(Optional.of(published)); - - GeschichteView result = geschichteService.getById(id); - - assertThat(result.author().displayName()).isEqualTo("Hans Raddatz"); - } - - @Test - void getById_author_displayName_falls_back_to_Unbekannt_when_names_blank() { - authenticateAs(reader, Permission.READ_ALL); - UUID id = UUID.randomUUID(); - Geschichte published = published(id); - published.setAuthor(AppUser.builder() - .id(UUID.randomUUID()).email("anon@test").build()); - when(geschichteRepository.findById(id)).thenReturn(Optional.of(published)); - - GeschichteView result = geschichteService.getById(id); - - assertThat(result.author().displayName()).isEqualTo("[Unbekannt]"); - } - - @Test - void getById_author_email_is_not_in_author_view() { - authenticateAs(reader, Permission.READ_ALL); - UUID id = UUID.randomUUID(); - Geschichte published = published(id); - published.setAuthor(AppUser.builder() - .id(UUID.randomUUID()).email("secret@test") - .firstName("Max").lastName("M").build()); - when(geschichteRepository.findById(id)).thenReturn(Optional.of(published)); - - GeschichteView result = geschichteService.getById(id); - - // AuthorView exposes only id + displayName — no email field at all - assertThat(result.author()).isInstanceOf(GeschichteView.AuthorView.class); - assertThat(result.author().displayName()).doesNotContain("secret@test"); - } - - @Test - void getById_persons_are_mapped_to_PersonView() { - authenticateAs(reader, Permission.READ_ALL); - UUID id = UUID.randomUUID(); - UUID personId = UUID.randomUUID(); - Geschichte published = published(id); - published.setPersons(new HashSet<>(List.of( - Person.builder().id(personId).firstName("Franz").lastName("Raddatz").build() - ))); - when(geschichteRepository.findById(id)).thenReturn(Optional.of(published)); - - GeschichteView result = geschichteService.getById(id); - - assertThat(result.persons()).hasSize(1); - GeschichteView.PersonView pv = result.persons().iterator().next(); - assertThat(pv.id()).isEqualTo(personId); - assertThat(pv.firstName()).isEqualTo("Franz"); - assertThat(pv.lastName()).isEqualTo("Raddatz"); - } - - @Test - void getById_items_come_from_journeyItemService() { - authenticateAs(reader, Permission.READ_ALL); - UUID id = UUID.randomUUID(); - Geschichte published = published(id); - when(geschichteRepository.findById(id)).thenReturn(Optional.of(published)); - when(journeyItemService.getItems(id)).thenReturn(List.of()); - - GeschichteView result = geschichteService.getById(id); - - assertThat(result.items()).isEmpty(); - verify(journeyItemService).getItems(id); + assertThat(result.getId()).isEqualTo(id); + assertThat(result.getStatus()).isEqualTo(GeschichteStatus.PUBLISHED); } @Test @@ -199,6 +116,74 @@ class GeschichteServiceTest { .isEqualTo(ErrorCode.GESCHICHTE_NOT_FOUND); } + @Test + void toView_author_displayName_uses_firstName_lastName() { + UUID id = UUID.randomUUID(); + Geschichte published = published(id); + published.setAuthor(AppUser.builder() + .id(UUID.randomUUID()).email("author@test") + .firstName("Hans").lastName("Raddatz").build()); + + GeschichteView result = geschichteService.toView(published, List.of()); + + assertThat(result.author().displayName()).isEqualTo("Hans Raddatz"); + } + + @Test + void toView_author_displayName_falls_back_to_Unbekannt_when_names_blank() { + UUID id = UUID.randomUUID(); + Geschichte published = published(id); + published.setAuthor(AppUser.builder() + .id(UUID.randomUUID()).email("anon@test").build()); + + GeschichteView result = geschichteService.toView(published, List.of()); + + assertThat(result.author().displayName()).isEqualTo("[Unbekannt]"); + } + + @Test + void toView_author_email_is_not_in_author_view() { + UUID id = UUID.randomUUID(); + Geschichte published = published(id); + published.setAuthor(AppUser.builder() + .id(UUID.randomUUID()).email("secret@test") + .firstName("Max").lastName("M").build()); + + GeschichteView result = geschichteService.toView(published, List.of()); + + // AuthorView exposes only id + displayName — no email field at all + assertThat(result.author()).isInstanceOf(GeschichteView.AuthorView.class); + assertThat(result.author().displayName()).doesNotContain("secret@test"); + } + + @Test + void toView_persons_are_mapped_to_PersonView() { + UUID id = UUID.randomUUID(); + UUID personId = UUID.randomUUID(); + Geschichte published = published(id); + published.setPersons(new HashSet<>(List.of( + Person.builder().id(personId).firstName("Franz").lastName("Raddatz").build() + ))); + + GeschichteView result = geschichteService.toView(published, List.of()); + + assertThat(result.persons()).hasSize(1); + GeschichteView.PersonView pv = result.persons().iterator().next(); + assertThat(pv.id()).isEqualTo(personId); + assertThat(pv.firstName()).isEqualTo("Franz"); + assertThat(pv.lastName()).isEqualTo("Raddatz"); + } + + @Test + void toView_items_are_passed_through() { + UUID id = UUID.randomUUID(); + Geschichte published = published(id); + + GeschichteView result = geschichteService.toView(published, List.of()); + + assertThat(result.items()).isEmpty(); + } + // ─── list ───────────────────────────────────────────────────────────────── @Test -- 2.49.1 From ad90ae75bf4c286ee765764c2aa2fb9852be2d1d Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 20:46:54 +0200 Subject: [PATCH 31/41] fix(journeyitem): use JOIN FETCH to eliminate N+1 document queries Add findByGeschichteIdWithDocument() to JourneyItemRepository with a LEFT JOIN FETCH on document. getItems() now uses this query so that all documents for a journey's items are loaded in a single SQL round-trip. toView() now reads item.getDocument() directly from the already-fetched association instead of issuing a separate documentService.getSummaryById() call per item. Co-Authored-By: Claude Sonnet 4.6 --- .../geschichte/journeyitem/JourneyItemRepository.java | 9 +++++++++ .../geschichte/journeyitem/JourneyItemService.java | 6 +++--- .../geschichte/journeyitem/JourneyItemServiceTest.java | 2 -- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java index 666681e6..a1b3baee 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemRepository.java @@ -29,4 +29,13 @@ public interface JourneyItemRepository extends JpaRepository /** COUNT for the 100-item cap check — COUNT(*)-based, never MAX(position)-derived. */ long countByGeschichteId(UUID geschichteId); + + /** + * Loads journey items with their linked Document in a single JOIN FETCH query, + * eliminating the N+1 SELECT that would occur when accessing item.getDocument() + * lazily for each item. Items without a document (note-only) are included via + * LEFT JOIN. Ordered by position ASC. + */ + @Query("SELECT ji FROM JourneyItem ji LEFT JOIN FETCH ji.document WHERE ji.geschichte.id = :geschichteId ORDER BY ji.position ASC") + List findByGeschichteIdWithDocument(@Param("geschichteId") UUID geschichteId); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java index 4e0a78d8..38259047 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -183,7 +183,7 @@ public class JourneyItemService { } public List getItems(UUID geschichteId) { - return journeyItemRepository.findByGeschichteIdOrderByPosition(geschichteId) + return journeyItemRepository.findByGeschichteIdWithDocument(geschichteId) .stream().map(this::toView).toList(); } @@ -206,8 +206,8 @@ public class JourneyItemService { JourneyItemView toView(JourneyItem item) { DocumentSummary docSummary = null; - if (item.getDocumentId() != null) { - Document doc = documentService.getSummaryById(item.getDocumentId()); + Document doc = item.getDocument(); + if (doc != null) { docSummary = toSummary(doc); } return new JourneyItemView(item.getId(), item.getPosition(), docSummary, item.getNote()); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java index 15ebcd87..d7b31be7 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java @@ -322,7 +322,6 @@ class JourneyItemServiceTest { Document doc = makeDoc(docId, null, List.of(), null, null); JourneyItem item = savedItemWithDoc(itemId, journey, 10, doc, "Old note"); when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.of(item)); - when(documentService.getSummaryById(docId)).thenReturn(doc); JourneyItem saved = savedItemWithDoc(itemId, journey, 10, doc, null); when(journeyItemRepository.save(item)).thenReturn(saved); @@ -372,7 +371,6 @@ class JourneyItemServiceTest { Document doc = makeDoc(docId, null, List.of(), null, null); JourneyItem item = savedItemWithDoc(itemId, journey, 10, doc, "Old"); when(journeyItemRepository.findByIdAndGeschichteId(itemId, geschichteId)).thenReturn(Optional.of(item)); - when(documentService.getSummaryById(docId)).thenReturn(doc); JourneyItem saved = savedItemWithDoc(itemId, journey, 10, doc, null); when(journeyItemRepository.save(item)).thenReturn(saved); -- 2.49.1 From 70da532f54fc75990c95fcc4a31b88f4ed45362f Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 20:47:46 +0200 Subject: [PATCH 32/41] docs(c4): add GeschichteQueryService component; fix GeschichteService relationship Add GeschichteQueryService component to the L3 supporting-domains diagram. Remove the now-deleted Rel(geschSvc, journeyItemSvc, "Delegates getItems()") arrow and add the correct Rel(journeyItemSvc, geschQuerySvc, ...) arrow that reflects the actual dependency direction after the refactor in the prior commit. Co-Authored-By: Claude Sonnet 4.6 --- docs/architecture/c4/l3-backend-3g-supporting.puml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/architecture/c4/l3-backend-3g-supporting.puml b/docs/architecture/c4/l3-backend-3g-supporting.puml index 11f45b1f..fba5056b 100644 --- a/docs/architecture/c4/l3-backend-3g-supporting.puml +++ b/docs/architecture/c4/l3-backend-3g-supporting.puml @@ -18,6 +18,7 @@ System_Boundary(backend, "API Backend (Spring Boot)") { Component(sseRegistry, "SseEmitterRegistry", "Spring Component", "In-memory ConcurrentHashMap of Spring SseEmitter instances per user. Handles registration, deregistration, and JSON event broadcasts.") Component(geschCtrl, "GeschichteController", "Spring MVC — /api/geschichten", "CRUD for publishable stories (STORY) and reading journeys (JOURNEY). Returns GeschichteSummary projections for list; full Geschichte with JourneyItems for detail. Requires BLOG_WRITE permission for write operations.") Component(geschSvc, "GeschichteService", "Spring Service", "Manages story lifecycle (DRAFT → PUBLISHED with timestamp). Supports two subtypes: STORY (prose) and JOURNEY (ordered JourneyItem sequence). Sanitizes HTML body with an allowlist policy.") + Component(geschQuerySvc, "GeschichteQueryService", "Spring Service", "Read-only facade over GeschichteRepository. Exposes existsById() and findById() to prevent JourneyItemService from crossing domain boundaries.") Component(journeyItemSvc, "JourneyItemService", "Spring Service", "Manages journey item lifecycle: append (100-item cap), updateNote (three-way PATCH), delete, and reorder (DEFERRABLE position swap). Enforces JOURNEY-type guard on append.") Component(exHandler, "GlobalExceptionHandler", "Spring @RestControllerAdvice", "Converts DomainException, validation errors, and generic exceptions to ErrorResponse JSON with machine-readable ErrorCode and HTTP status.") } @@ -40,7 +41,8 @@ Rel(notifCtrl, sseRegistry, "Registers client SSE connection") Rel(notifSvc, sseRegistry, "Broadcasts events to connected clients") Rel(geschCtrl, geschSvc, "Delegates to") Rel(geschCtrl, journeyItemSvc, "Delegates journey item CRUD") -Rel(geschSvc, journeyItemSvc, "Delegates getItems()") +Rel(journeyItemSvc, geschQuerySvc, "Checks Geschichte existence and type") +Rel(geschQuerySvc, db, "Reads geschichten", "JDBC") Rel(journeyItemSvc, db, "Reads / writes journey_items", "JDBC") Rel(auditSvc, db, "Writes audit_log", "JDBC") Rel(auditQuery, db, "Reads audit_log", "JDBC") -- 2.49.1 From f9ae6a91ba335b86a5a6858bc0d4ef115fcbdeec Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 20:49:23 +0200 Subject: [PATCH 33/41] test(journeyitem): add integration tests for append and reorder against real PostgreSQL Add two service-level integration tests to JourneyItemIntegrationTest: - append_persists_item_at_position_10: verifies that the first append to an empty journey creates an item at position 10 in the DB. - reorder_swaps_positions_atomically: appends two items then reorders them, asserting the DB reflects the new position assignment. Both tests use the SecurityContextHolder authentication pattern from GeschichteServiceIntegrationTest and mock S3Client to avoid MinIO connections. Co-Authored-By: Claude Sonnet 4.6 --- .../JourneyItemIntegrationTest.java | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java index 00c2591c..b6c9fcbe 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java @@ -2,6 +2,7 @@ package org.raddatz.familienarchiv.geschichte.journeyitem; import jakarta.persistence.EntityManager; import jakarta.persistence.PersistenceContext; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.PostgresContainerConfig; @@ -12,9 +13,15 @@ import org.raddatz.familienarchiv.geschichte.Geschichte; import org.raddatz.familienarchiv.geschichte.GeschichteRepository; import org.raddatz.familienarchiv.geschichte.GeschichteStatus; import org.raddatz.familienarchiv.geschichte.GeschichteType; +import org.raddatz.familienarchiv.security.Permission; +import org.raddatz.familienarchiv.user.AppUser; +import org.raddatz.familienarchiv.user.AppUserRepository; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.context.annotation.Import; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.transaction.annotation.Transactional; @@ -40,13 +47,20 @@ class JourneyItemIntegrationTest { @Autowired GeschichteRepository geschichteRepository; @Autowired JourneyItemRepository journeyItemRepository; + @Autowired JourneyItemService journeyItemService; @Autowired DocumentRepository documentRepository; + @Autowired AppUserRepository appUserRepository; Geschichte journey; Document doc; + AppUser writer; @BeforeEach void seed() { + writer = appUserRepository.save(AppUser.builder() + .email("journey-writer@test") + .password("hash") + .build()); doc = documentRepository.save(Document.builder() .title("Testbrief") .originalFilename("testbrief.pdf") @@ -61,6 +75,19 @@ class JourneyItemIntegrationTest { em.clear(); } + @AfterEach + void clearSecurity() { + SecurityContextHolder.clearContext(); + } + + private void authenticateAs(AppUser user, Permission... permissions) { + var authorities = java.util.Arrays.stream(permissions) + .map(p -> new SimpleGrantedAuthority(p.name())) + .toList(); + SecurityContextHolder.getContext().setAuthentication( + new UsernamePasswordAuthenticationToken(user.getEmail(), null, authorities)); + } + // ─── @OrderBy ───────────────────────────────────────────────────────────── @Test @@ -206,4 +233,62 @@ class JourneyItemIntegrationTest { journeyItemRepository.flush(); }).isInstanceOf(Exception.class); } + + // ─── JourneyItemService.append — end-to-end persistence ────────────────── + + @Test + void append_persists_item_at_position_10() { + // Arrange: authenticate as a user with BLOG_WRITE + authenticateAs(writer, Permission.BLOG_WRITE); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setNote("First stop"); + + // Act + JourneyItemView view = journeyItemService.append(journey.getId(), dto); + em.flush(); + em.clear(); + + // Assert: item exists in DB at position 10 + assertThat(view.position()).isEqualTo(10); + assertThat(view.note()).isEqualTo("First stop"); + List persisted = journeyItemRepository.findByGeschichteIdOrderByPosition(journey.getId()); + assertThat(persisted).hasSize(1); + assertThat(persisted.get(0).getPosition()).isEqualTo(10); + assertThat(persisted.get(0).getNote()).isEqualTo("First stop"); + } + + // ─── JourneyItemService.reorder — atomicity check ──────────────────────── + + @Test + void reorder_swaps_positions_atomically() { + // Arrange: append two items (pos 10, pos 20) + authenticateAs(writer, Permission.BLOG_WRITE); + + JourneyItemCreateDTO dto1 = new JourneyItemCreateDTO(); + dto1.setNote("Item one"); + JourneyItemView item1View = journeyItemService.append(journey.getId(), dto1); + + JourneyItemCreateDTO dto2 = new JourneyItemCreateDTO(); + dto2.setNote("Item two"); + JourneyItemView item2View = journeyItemService.append(journey.getId(), dto2); + + assertThat(item1View.position()).isEqualTo(10); + assertThat(item2View.position()).isEqualTo(20); + + // Act: reorder with [item2, item1] + JourneyReorderDTO reorderDto = new JourneyReorderDTO(); + reorderDto.setItemIds(List.of(item2View.id(), item1View.id())); + List reordered = journeyItemService.reorder(journey.getId(), reorderDto); + em.flush(); + em.clear(); + + // Assert: item2 is now at position 10, item1 is at position 20 + List persisted = journeyItemRepository.findByGeschichteIdOrderByPosition(journey.getId()); + assertThat(persisted).hasSize(2); + assertThat(persisted.get(0).getId()).isEqualTo(item2View.id()); + assertThat(persisted.get(0).getPosition()).isEqualTo(10); + assertThat(persisted.get(1).getId()).isEqualTo(item1View.id()); + assertThat(persisted.get(1).getPosition()).isEqualTo(20); + } } -- 2.49.1 From c31f82a69c1a91a4b1dc5765de3d9238a5b49fbb Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 20:50:17 +0200 Subject: [PATCH 34/41] fix(test): use nullValue() matcher instead of doesNotExist() for null note field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit doesNotExist() asserts the key is absent from the JSON object, but Jackson serializes a null Optional as {"note": null} — the key is present with a null value. nullValue() correctly matches that case. Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/geschichte/GeschichteControllerTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java index df0d07d8..2aec2e4c 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java @@ -24,6 +24,7 @@ import java.util.HashSet; import java.util.List; import java.util.UUID; +import static org.hamcrest.CoreMatchers.nullValue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; @@ -283,7 +284,7 @@ class GeschichteControllerTest { .contentType(MediaType.APPLICATION_JSON) .content("{\"note\": null}")) .andExpect(status().isOk()) - .andExpect(jsonPath("$.note").doesNotExist()); + .andExpect(jsonPath("$.note").value(nullValue())); } @Test -- 2.49.1 From 73004ce49f1427605369d6ae0b4423b849bba6df Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 20:51:10 +0200 Subject: [PATCH 35/41] docs(document): document scope-check bypass on getSummaryById Clarify in the Javadoc that getSummaryById intentionally skips scope checks and tag-colour resolution. This is safe under the current single-tenant model and is explicitly used by JourneyItemService.append() to validate that a linked document exists before persisting a JourneyItem. Co-Authored-By: Claude Sonnet 4.6 --- .../raddatz/familienarchiv/document/DocumentService.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java index 624a2a37..dc4d70e4 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -1007,9 +1007,10 @@ public class DocumentService { } /** - * Lean document lookup for embedding in JourneyItemView. Skips - * {@code tagService.resolveEffectiveColors} — ×N items per journey GET is wasted - * work that summary consumers never read. Called within a caller-provided transaction. + * Lightweight summary lookup for internal use (e.g. journey item append validation). + * Intentionally skips scope checks and tag-colour resolution — safe only + * under the current single-tenant model where all authenticated users share + * the same document scope. Called within a caller-provided transaction. */ public Document getSummaryById(UUID id) { return documentRepository.findById(id) -- 2.49.1 From 9db3b41fdbd9f4e8486c7cb793a4a64ede94f91f Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 20:52:02 +0200 Subject: [PATCH 36/41] docs(api): document reorder full-list contract in OpenAPI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add @Operation annotation to reorderItems() clarifying that itemIds must contain ALL item IDs for the journey in the desired order — a partial list returns 400 Bad Request. This surfaces the contract in the generated OpenAPI spec and Swagger UI. Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/geschichte/GeschichteController.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java index 8113238c..b675a2b5 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java @@ -102,6 +102,10 @@ public class GeschichteController { @PutMapping("/{id}/items/reorder") @RequirePermission(Permission.BLOG_WRITE) + @Operation( + summary = "Reorder journey items", + description = "itemIds must contain ALL item IDs for the given journey in the desired new order. Sending a partial list returns 400 Bad Request." + ) public List reorderItems( @PathVariable UUID id, @RequestBody JourneyReorderDTO dto) { -- 2.49.1 From 1108277472fb50c68609b2d8c594aa9d03a5994a Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 20:53:49 +0200 Subject: [PATCH 37/41] refactor(geschichte): extract PersonNameFormatter to eliminate duplicated name-join logic Create PersonNameFormatter with a single static join(firstName, lastName) method. Replace the inline string concatenation in GeschichteService.toView() and the private join() method in JourneyItemService with calls to PersonNameFormatter.join(). The new helper handles null-safety and trimming consistently in one place. Co-Authored-By: Claude Sonnet 4.6 --- .../geschichte/GeschichteService.java | 3 +-- .../geschichte/PersonNameFormatter.java | 22 +++++++++++++++++++ .../journeyitem/JourneyItemService.java | 9 +++----- 3 files changed, 26 insertions(+), 8 deletions(-) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/geschichte/PersonNameFormatter.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java index 857ce314..32afda8a 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java @@ -71,8 +71,7 @@ public class GeschichteService { AppUser author = g.getAuthor(); GeschichteView.AuthorView authorView = null; if (author != null) { - String displayName = ((author.getFirstName() != null ? author.getFirstName() : "") - + " " + (author.getLastName() != null ? author.getLastName() : "")).trim(); + String displayName = PersonNameFormatter.join(author.getFirstName(), author.getLastName()); if (displayName.isBlank()) displayName = "[Unbekannt]"; authorView = new GeschichteView.AuthorView(author.getId(), displayName); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/PersonNameFormatter.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/PersonNameFormatter.java new file mode 100644 index 00000000..91c63231 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/PersonNameFormatter.java @@ -0,0 +1,22 @@ +package org.raddatz.familienarchiv.geschichte; + +/** + * Utility for joining a person's first and last name into a display string. + * Centralises the logic that was previously duplicated across GeschichteService + * and JourneyItemService. + */ +public class PersonNameFormatter { + + private PersonNameFormatter() { + // utility class — no instances + } + + public static String join(String firstName, String lastName) { + String first = firstName != null ? firstName.trim() : ""; + String last = lastName != null ? lastName.trim() : ""; + if (first.isEmpty() && last.isEmpty()) return ""; + if (first.isEmpty()) return last; + if (last.isEmpty()) return first; + return first + " " + last; + } +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java index 38259047..82d18816 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -13,6 +13,7 @@ import org.raddatz.familienarchiv.geschichte.Geschichte; import org.raddatz.familienarchiv.geschichte.GeschichteQueryService; import org.raddatz.familienarchiv.geschichte.GeschichteType; import org.raddatz.familienarchiv.geschichte.DocumentSummary; +import org.raddatz.familienarchiv.geschichte.PersonNameFormatter; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.user.AppUser; import org.raddatz.familienarchiv.user.UserService; @@ -216,7 +217,7 @@ public class JourneyItemService { private static String buildSenderName(Document doc) { Person sender = doc.getSender(); if (sender != null) { - String name = join(sender.getFirstName(), sender.getLastName()); + String name = PersonNameFormatter.join(sender.getFirstName(), sender.getLastName()); if (!name.isBlank()) return name; } String senderText = doc.getSenderText(); @@ -228,7 +229,7 @@ public class JourneyItemService { return receivers.stream() .min(Comparator.comparing(p -> sortKey(p.getLastName()) + " " + sortKey(p.getFirstName()))) .map(p -> { - String name = join(p.getFirstName(), p.getLastName()); + String name = PersonNameFormatter.join(p.getFirstName(), p.getLastName()); return name.isBlank() ? null : name; }) .orElse(null); @@ -239,10 +240,6 @@ public class JourneyItemService { return raw.trim(); } - private static String join(String first, String last) { - return ((first != null ? first : "") + " " + (last != null ? last : "")).trim(); - } - private static String sortKey(String s) { return s != null ? s : ""; } -- 2.49.1 From f09c79744ee37a73d08c6fc08747cfc1b62ea29a Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 21:12:25 +0200 Subject: [PATCH 38/41] =?UTF-8?q?fix(geschichte):=20restore=20getView()=20?= =?UTF-8?q?on=20GeschichteService=20with=20@Transactional(readOnly=3Dtrue)?= =?UTF-8?q?=20=E2=80=94=20fixes=20two-call=20transaction=20gap?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-inject JourneyItemService into GeschichteService (no cycle: JourneyItemService → GeschichteQueryService, not GeschichteService). Add getView(UUID) that loads the Geschichte and its items in a single @Transactional(readOnly=true) session. Controller now delegates to getView() instead of making two separate service calls. Tests updated to stub getView() and cover the new method. Co-Authored-By: Claude Sonnet 4.6 --- .../geschichte/GeschichteController.java | 4 +-- .../geschichte/GeschichteService.java | 9 +++++ .../geschichte/GeschichteControllerTest.java | 7 ++-- .../geschichte/GeschichteServiceTest.java | 33 +++++++++++++++++++ 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java index b675a2b5..4eed4e16 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteController.java @@ -46,9 +46,7 @@ public class GeschichteController { @GetMapping("/{id}") public GeschichteView getById(@PathVariable UUID id) { - Geschichte g = geschichteService.getById(id); - List items = journeyItemService.getItems(g.getId()); - return geschichteService.toView(g, items); + return geschichteService.getView(id); } @PostMapping diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java index 32afda8a..2a0d8819 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/GeschichteService.java @@ -6,6 +6,7 @@ import org.owasp.html.HtmlPolicyBuilder; import org.owasp.html.PolicyFactory; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemService; import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemView; import org.raddatz.familienarchiv.user.AppUser; import org.raddatz.familienarchiv.person.Person; @@ -35,6 +36,7 @@ public class GeschichteService { private final PersonService personService; private final DocumentService documentService; private final UserService userService; + private final JourneyItemService journeyItemService; /** * Allow-list policy for Geschichte body HTML. Tiptap on the writer side @@ -67,6 +69,13 @@ public class GeschichteService { return g; } + @Transactional(readOnly = true) + public GeschichteView getView(UUID id) { + Geschichte g = getById(id); + List items = journeyItemService.getItems(id); + return toView(g, items); + } + GeschichteView toView(Geschichte g, List items) { AppUser author = g.getAuthor(); GeschichteView.AuthorView authorView = null; diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java index 2aec2e4c..9c7bf25c 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteControllerTest.java @@ -106,10 +106,7 @@ class GeschichteControllerTest { @WithMockUser(authorities = "READ_ALL") void getById_returns200_whenFound() throws Exception { UUID id = UUID.randomUUID(); - Geschichte g = published(id, "Hello"); - when(geschichteService.getById(id)).thenReturn(g); - when(journeyItemService.getItems(id)).thenReturn(List.of()); - when(geschichteService.toView(g, List.of())).thenReturn(viewStub(id, "Hello")); + when(geschichteService.getView(id)).thenReturn(viewStub(id, "Hello")); mockMvc.perform(get("/api/geschichten/{id}", id)) .andExpect(status().isOk()) @@ -121,7 +118,7 @@ class GeschichteControllerTest { @WithMockUser(authorities = "READ_ALL") void getById_returns404_whenServiceThrowsNotFound() throws Exception { UUID id = UUID.randomUUID(); - when(geschichteService.getById(id)) + when(geschichteService.getView(id)) .thenThrow(DomainException.notFound(ErrorCode.GESCHICHTE_NOT_FOUND, "x")); mockMvc.perform(get("/api/geschichten/{id}", id)) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java index 3f154cdc..d2d5bfed 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/GeschichteServiceTest.java @@ -9,6 +9,8 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemService; +import org.raddatz.familienarchiv.geschichte.journeyitem.JourneyItemView; import org.raddatz.familienarchiv.user.AppUser; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.security.Permission; @@ -45,6 +47,7 @@ class GeschichteServiceTest { @Mock PersonService personService; @Mock DocumentService documentService; @Mock UserService userService; + @Mock JourneyItemService journeyItemService; @InjectMocks GeschichteService geschichteService; @@ -116,6 +119,36 @@ class GeschichteServiceTest { .isEqualTo(ErrorCode.GESCHICHTE_NOT_FOUND); } + // ─── getView ────────────────────────────────────────────────────────────── + + @Test + void getView_returns_assembled_view_and_delegates_to_journeyItemService() { + authenticateAs(reader, Permission.READ_ALL); + UUID id = UUID.randomUUID(); + Geschichte published = published(id); + JourneyItemView item = new JourneyItemView(UUID.randomUUID(), 10, null, "Note"); + when(geschichteRepository.findById(id)).thenReturn(Optional.of(published)); + when(journeyItemService.getItems(id)).thenReturn(List.of(item)); + + GeschichteView view = geschichteService.getView(id); + + assertThat(view.id()).isEqualTo(id); + assertThat(view.items()).containsExactly(item); + verify(journeyItemService).getItems(id); + } + + @Test + void getView_throws_NOT_FOUND_when_id_unknown() { + authenticateAs(reader, Permission.READ_ALL); + UUID id = UUID.randomUUID(); + when(geschichteRepository.findById(id)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> geschichteService.getView(id)) + .isInstanceOf(DomainException.class) + .extracting("code") + .isEqualTo(ErrorCode.GESCHICHTE_NOT_FOUND); + } + @Test void toView_author_displayName_uses_firstName_lastName() { UUID id = UUID.randomUUID(); -- 2.49.1 From 99111273e5f4b1e5b5f021590b8b0bf6cd71e224 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 21:13:35 +0200 Subject: [PATCH 39/41] refactor(document): rename getSummaryById to findSummaryByIdInternal to signal scope-check bypass The method intentionally skips permission checks and tag-colour resolution. Renaming it to findSummaryByIdInternal makes the internal-only contract visible at every call site, closing the latent CWE-284 risk flagged in the PR review. Co-Authored-By: Claude Sonnet 4.6 --- .../org/raddatz/familienarchiv/document/DocumentService.java | 2 +- .../geschichte/journeyitem/JourneyItemService.java | 2 +- .../geschichte/journeyitem/JourneyItemServiceTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java index dc4d70e4..66a56a22 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -1012,7 +1012,7 @@ public class DocumentService { * under the current single-tenant model where all authenticated users share * the same document scope. Called within a caller-provided transaction. */ - public Document getSummaryById(UUID id) { + public Document findSummaryByIdInternal(UUID id) { return documentRepository.findById(id) .orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id)); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java index 82d18816..f12a4593 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -70,7 +70,7 @@ public class JourneyItemService { Document doc = null; if (dto.getDocumentId() != null) { - doc = documentService.getSummaryById(dto.getDocumentId()); + doc = documentService.findSummaryByIdInternal(dto.getDocumentId()); } int nextPosition = journeyItemRepository.findMaxPositionByGeschichteId(geschichteId) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java index d7b31be7..91a51c93 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java @@ -240,7 +240,7 @@ class JourneyItemServiceTest { Geschichte journey = journey(geschichteId); when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(journey)); when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); - when(documentService.getSummaryById(docId)) + when(documentService.findSummaryByIdInternal(docId)) .thenThrow(DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "not found")); JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); -- 2.49.1 From 84b47f1836cc6e3ced999715fe38dd2e14b5c496 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 21:37:07 +0200 Subject: [PATCH 40/41] fix(geschichte): move DocumentSummary to journeyitem sub-package Co-Authored-By: Claude Sonnet 4.6 --- .../geschichte/{ => journeyitem}/DocumentSummary.java | 2 +- .../geschichte/journeyitem/JourneyItemService.java | 1 - .../familienarchiv/geschichte/journeyitem/JourneyItemView.java | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) rename backend/src/main/java/org/raddatz/familienarchiv/geschichte/{ => journeyitem}/DocumentSummary.java (93%) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/DocumentSummary.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/DocumentSummary.java similarity index 93% rename from backend/src/main/java/org/raddatz/familienarchiv/geschichte/DocumentSummary.java rename to backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/DocumentSummary.java index 151017f7..17ba1848 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/DocumentSummary.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/DocumentSummary.java @@ -1,4 +1,4 @@ -package org.raddatz.familienarchiv.geschichte; +package org.raddatz.familienarchiv.geschichte.journeyitem; import io.swagger.v3.oas.annotations.media.Schema; import org.raddatz.familienarchiv.document.DatePrecision; diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java index f12a4593..f189a9d3 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemService.java @@ -12,7 +12,6 @@ import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.geschichte.Geschichte; import org.raddatz.familienarchiv.geschichte.GeschichteQueryService; import org.raddatz.familienarchiv.geschichte.GeschichteType; -import org.raddatz.familienarchiv.geschichte.DocumentSummary; import org.raddatz.familienarchiv.geschichte.PersonNameFormatter; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.user.AppUser; diff --git a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemView.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemView.java index b15dc6b1..141860af 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemView.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemView.java @@ -1,7 +1,6 @@ package org.raddatz.familienarchiv.geschichte.journeyitem; import io.swagger.v3.oas.annotations.media.Schema; -import org.raddatz.familienarchiv.geschichte.DocumentSummary; import java.util.UUID; -- 2.49.1 From 77cbbd34a0bec87f9807074f89407d867bb1dc44 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 8 Jun 2026 21:38:25 +0200 Subject: [PATCH 41/41] test(journeyitem): verify findSummaryByIdInternal never called before JOURNEY-type guard Co-Authored-By: Claude Sonnet 4.6 --- .../document/DocumentService.java | 17 ++++++++++++--- .../journeyitem/JourneyItemServiceTest.java | 21 +++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java index 66a56a22..d4715f40 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -1008,9 +1008,20 @@ public class DocumentService { /** * Lightweight summary lookup for internal use (e.g. journey item append validation). - * Intentionally skips scope checks and tag-colour resolution — safe only - * under the current single-tenant model where all authenticated users share - * the same document scope. Called within a caller-provided transaction. + * + *

Security contract — read before calling: + *

    + *
  1. This method intentionally bypasses per-document scope checks and + * tag-colour resolution. It must only be invoked after + * {@code @RequirePermission(BLOG_WRITE)} has already been enforced at + * the controller layer, guaranteeing the caller is an authenticated + * author.
    2. + *
  2. In {@code JourneyItemService.append()}, it is additionally guarded by the + * JOURNEY-type check that fires before this call — so the method is never + * reached for STORY-type Geschichten.
    3. + *
+ * Under the current single-tenant model every authenticated author shares the + * same document scope, so skipping per-document scope checks is safe. */ public Document findSummaryByIdInternal(UUID id) { return documentRepository.findById(id) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java index 91a51c93..c411a3bb 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemServiceTest.java @@ -39,6 +39,7 @@ import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -235,6 +236,26 @@ class JourneyItemServiceTest { .isEqualTo(ErrorCode.GESCHICHTE_TYPE_MISMATCH)); } + @Test + void append_never_calls_findSummaryByIdInternal_when_geschichte_type_is_STORY() { + // Arrange: mock geschichteQueryService.findById() to return a STORY-type Geschichte + UUID storyId = UUID.randomUUID(); + Geschichte story = Geschichte.builder() + .id(storyId) + .type(GeschichteType.STORY) + .build(); + when(geschichteQueryService.findById(storyId)).thenReturn(Optional.of(story)); + + // Act + Assert: calling append throws GESCHICHTE_TYPE_MISMATCH + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setDocumentId(UUID.randomUUID()); + assertThatThrownBy(() -> journeyItemService.append(storyId, dto)) + .isInstanceOf(DomainException.class); + + // Verify: document service was never touched — type guard fired first + verifyNoInteractions(documentService); + } + @Test void append_returns404_when_documentId_does_not_exist() { Geschichte journey = journey(geschichteId); -- 2.49.1