From d0fc8ce995b63636383da0da435063f62190aacd Mon Sep 17 00:00:00 2001 From: Marcel Date: Thu, 11 Jun 2026 12:21:33 +0200 Subject: [PATCH] feat(geschichte): allow journey items on STORY-type Geschichten (#795) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Delete the JOURNEY-only type guard in JourneyItemService.append() so the existing item endpoints serve both Geschichte types. GeschichteType has exactly two constants, so an allowlist replacement would be unreachable dead code. Fix the not-found messages that claimed "Journey", and remove the now-orphaned GESCHICHTE_TYPE_MISMATCH error code end to end (ErrorCode, errors.ts union + mapping, i18n keys in de/en/es). Tests: three STORY append unit tests written red against the guard, plus end-to-end STORY coverage (append+retrieve, V72-style position-gap rows incl. removal, dangling document-deleted item). The two STORY-rejection tests die with the guard — no third enum value exists to feed it. Co-Authored-By: Claude Fable 5 --- .../familienarchiv/exception/ErrorCode.java | 2 - .../journeyitem/JourneyItemService.java | 10 +- .../JourneyItemIntegrationTest.java | 91 ++++++++++++++++ .../journeyitem/JourneyItemServiceTest.java | 100 +++++++++++------- frontend/messages/de.json | 1 - frontend/messages/en.json | 1 - frontend/messages/es.json | 1 - frontend/src/lib/shared/errors.ts | 3 - 8 files changed, 153 insertions(+), 56 deletions(-) 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 4fb36a13..84caab27 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/exception/ErrorCode.java @@ -130,8 +130,6 @@ public enum ErrorCode { JOURNEY_AT_CAPACITY, /** The document is already present in this journey — duplicate items are not allowed. 409 */ JOURNEY_DOCUMENT_ALREADY_ADDED, - /** The Geschichte is not of type JOURNEY — journey-item operations are not allowed on it. 400 */ - GESCHICHTE_TYPE_MISMATCH, /** The type of an existing Geschichte cannot be changed via PATCH. 409 */ GESCHICHTE_TYPE_IMMUTABLE, /** A journey-item note exceeds the maximum length (2000 characters). 400 */ 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 7b08c601..893c1b12 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.GeschichteType; import org.raddatz.familienarchiv.geschichte.PersonNameFormatter; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.user.AppUser; @@ -44,12 +43,7 @@ public class JourneyItemService { public JourneyItemView append(UUID geschichteId, JourneyItemCreateDTO dto) { Geschichte g = geschichteQueryService.findById(geschichteId) .orElseThrow(() -> DomainException.notFound(ErrorCode.GESCHICHTE_NOT_FOUND, - "Journey not found: " + geschichteId)); - - if (g.getType() != GeschichteType.JOURNEY) { - throw DomainException.conflict(ErrorCode.GESCHICHTE_TYPE_MISMATCH, - "Journey items can only be added to a JOURNEY-type Geschichte"); - } + "Geschichte not found: " + geschichteId)); long count = journeyItemRepository.countByGeschichteId(geschichteId); if (count >= MAX_ITEMS) { @@ -163,7 +157,7 @@ public class JourneyItemService { public List reorder(UUID geschichteId, JourneyReorderDTO dto) { if (!geschichteQueryService.existsById(geschichteId)) { throw DomainException.notFound(ErrorCode.GESCHICHTE_NOT_FOUND, - "Journey not found: " + geschichteId); + "Geschichte 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/JourneyItemIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemIntegrationTest.java index 1be93320..887f00a1 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 @@ -284,6 +284,97 @@ class JourneyItemIntegrationTest { org.raddatz.familienarchiv.exception.ErrorCode.JOURNEY_DOCUMENT_ALREADY_ADDED); } + // ─── STORY-type Geschichten hold journey items (#795) ──────────────────── + + @Test + void story_type_can_hold_journey_items_end_to_end() { + authenticateAs(writer, Permission.BLOG_WRITE); + Geschichte story = savedStory(); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setDocumentId(doc.getId()); + JourneyItemView appended = journeyItemService.append(story.getId(), dto); + em.flush(); + em.clear(); + + List items = journeyItemService.getItems(story.getId()); + assertThat(items).hasSize(1); + assertThat(items.get(0).id()).isEqualTo(appended.id()); + assertThat(items.get(0).document().id()).isEqualTo(doc.getId()); + } + + @Test + void v72_migrated_story_items_keep_position_order_and_are_removable() { + authenticateAs(writer, Permission.BLOG_WRITE); + Geschichte story = savedStory(); + Document docB = documentRepository.save(Document.builder() + .title("Zweiter Brief").originalFilename("b.pdf").status(DocumentStatus.UPLOADED).build()); + Document docC = documentRepository.save(Document.builder() + .title("Dritter Brief").originalFilename("c.pdf").status(DocumentStatus.UPLOADED).build()); + + // V72 inserted journey_items rows directly with position gaps — mirror that + // by writing through the repository instead of the service. + JourneyItem first = journeyItemRepository.save( + JourneyItem.builder().geschichte(story).position(10).document(doc).build()); + JourneyItem second = journeyItemRepository.save( + JourneyItem.builder().geschichte(story).position(20).document(docB).build()); + JourneyItem third = journeyItemRepository.save( + JourneyItem.builder().geschichte(story).position(30).document(docC).build()); + em.flush(); + em.clear(); + + assertThat(journeyItemService.getItems(story.getId())) + .extracting(JourneyItemView::position) + .containsExactly(10, 20, 30); + + journeyItemService.delete(story.getId(), second.getId()); + em.flush(); + em.clear(); + + assertThat(journeyItemService.getItems(story.getId())) + .extracting(JourneyItemView::id) + .containsExactly(first.getId(), third.getId()); + } + + @Test + void story_item_with_deleted_document_survives_and_remains_deletable() { + authenticateAs(writer, Permission.BLOG_WRITE); + Geschichte story = savedStory(); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setDocumentId(doc.getId()); + // The note keeps chk_journey_item_not_empty satisfied once ON DELETE + // SET NULL clears document_id — a note-less item would block the + // document delete at the DB instead. + dto.setNote("Begleittext"); + JourneyItemView appended = journeyItemService.append(story.getId(), dto); + em.flush(); + em.clear(); + + // ON DELETE SET NULL fires at DB level (V72) + documentRepository.deleteById(doc.getId()); + em.flush(); + em.clear(); + + List items = journeyItemService.getItems(story.getId()); + assertThat(items).hasSize(1); + assertThat(items.get(0).document()).isNull(); + + journeyItemService.delete(story.getId(), appended.id()); + em.flush(); + em.clear(); + + assertThat(journeyItemService.getItems(story.getId())).isEmpty(); + } + + private Geschichte savedStory() { + return geschichteRepository.save(Geschichte.builder() + .title("Eine Geschichte") + .status(GeschichteStatus.DRAFT) + .type(GeschichteType.STORY) + .build()); + } + // ─── JourneyItemService.reorder — atomicity check ──────────────────────── @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 5208e6ab..2c520625 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,7 +39,6 @@ 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) @@ -233,45 +232,6 @@ class JourneyItemServiceTest { assertThat(journeyItemService.append(geschichteId, dto).note()).hasSize(2000); } - @Test - void append_returns409_on_non_JOURNEY_type() { - Geschichte story = Geschichte.builder() - .id(geschichteId) - .title("Story") - .type(GeschichteType.STORY) - .status(GeschichteStatus.DRAFT) - .build(); - when(geschichteQueryService.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.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); @@ -320,6 +280,57 @@ class JourneyItemServiceTest { .isEqualTo(ErrorCode.JOURNEY_DOCUMENT_ALREADY_ADDED)); } + @Test + void append_to_STORY_type_creates_journey_item() { + Geschichte story = story(geschichteId); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(story)); + when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(0L); + when(journeyItemRepository.existsByGeschichteIdAndDocumentId(geschichteId, docId)).thenReturn(false); + Document doc = makeDoc(docId, null, List.of(), null, null); + when(documentService.findSummaryByIdInternal(docId)).thenReturn(doc); + when(journeyItemRepository.findMaxPositionByGeschichteId(geschichteId)).thenReturn(Optional.empty()); + when(journeyItemRepository.saveAndFlush(any())).thenReturn(savedItemWithDoc(itemId, story, 10, doc, null)); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setDocumentId(docId); + + JourneyItemView view = journeyItemService.append(geschichteId, dto); + + assertThat(view.position()).isEqualTo(10); + assertThat(view.document().id()).isEqualTo(docId); + } + + @Test + void append_to_STORY_type_respects_capacity_cap() { + Geschichte story = story(geschichteId); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(story)); + when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(100L); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setDocumentId(docId); + + assertThatThrownBy(() -> journeyItemService.append(geschichteId, dto)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.JOURNEY_AT_CAPACITY)); + } + + @Test + void append_to_STORY_type_rejects_duplicate_document() { + Geschichte story = story(geschichteId); + when(geschichteQueryService.findById(geschichteId)).thenReturn(Optional.of(story)); + when(journeyItemRepository.countByGeschichteId(geschichteId)).thenReturn(1L); + when(journeyItemRepository.existsByGeschichteIdAndDocumentId(geschichteId, docId)).thenReturn(true); + + JourneyItemCreateDTO dto = new JourneyItemCreateDTO(); + dto.setDocumentId(docId); + + assertThatThrownBy(() -> journeyItemService.append(geschichteId, dto)) + .isInstanceOf(DomainException.class) + .satisfies(e -> assertThat(((DomainException) e).getCode()) + .isEqualTo(ErrorCode.JOURNEY_DOCUMENT_ALREADY_ADDED)); + } + @Test void cap_is_COUNT_based_not_MAX_position_based() { // 99 rows with MAX(position)=2000 should still accept the 100th append @@ -729,6 +740,15 @@ class JourneyItemServiceTest { .build(); } + private Geschichte story(UUID id) { + return Geschichte.builder() + .id(id) + .title("Test Story") + .type(GeschichteType.STORY) + .status(GeschichteStatus.DRAFT) + .build(); + } + private JourneyItem savedItem(UUID id, Geschichte g, int position, Document doc, String note) { return JourneyItem.builder() .id(id) diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 204026cc..a154e9f0 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -1028,7 +1028,6 @@ "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.", "journey_item_document_deleted": "[Dokument gelöscht]", "geschichten_index_title": "Geschichten", "geschichten_new_button": "Neue Geschichte", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 4bce75a0..517ee825 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -1028,7 +1028,6 @@ "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.", "journey_item_document_deleted": "[Document deleted]", "geschichten_index_title": "Stories", "geschichten_new_button": "New story", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index 8a8b5706..467c3657 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -1028,7 +1028,6 @@ "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í.", "journey_item_document_deleted": "[Documento eliminado]", "geschichten_index_title": "Historias", "geschichten_new_button": "Nueva historia", diff --git a/frontend/src/lib/shared/errors.ts b/frontend/src/lib/shared/errors.ts index 37125109..e06f37d4 100644 --- a/frontend/src/lib/shared/errors.ts +++ b/frontend/src/lib/shared/errors.ts @@ -51,7 +51,6 @@ export type ErrorCode = | 'JOURNEY_AT_CAPACITY' | 'JOURNEY_NOTE_TOO_LONG' | 'JOURNEY_DOCUMENT_ALREADY_ADDED' - | 'GESCHICHTE_TYPE_MISMATCH' | 'GESCHICHTE_TYPE_IMMUTABLE' | 'GESCHICHTE_TITLE_TOO_LONG' | 'GESCHICHTE_INTRO_TOO_LONG' @@ -183,8 +182,6 @@ export function getErrorMessage(code: ErrorCode | string | undefined): string { return m.error_journey_note_too_long(); case 'JOURNEY_DOCUMENT_ALREADY_ADDED': return m.error_journey_document_already_added(); - case 'GESCHICHTE_TYPE_MISMATCH': - return m.error_geschichte_type_mismatch(); case 'GESCHICHTE_TYPE_IMMUTABLE': return m.error_geschichte_type_immutable(); case 'GESCHICHTE_TITLE_TOO_LONG':