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 7b63ec27..b666c8d8 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/audit/AuditKind.java @@ -52,6 +52,11 @@ public enum AuditKind { /** Payload: {@code {"ip": "1.2.3.4", "email": "addr"}} — password NEVER included */ LOGIN_RATE_LIMITED, + // --- Documents --- + + /** Payload: none — the deleted document's id is carried in the documentId column */ + DOCUMENT_DELETED, + // --- Reading Journeys (Lesereisen) --- /** Payload: {@code {"geschichteId": "uuid", "itemId": "uuid"}} — documentId is null (journey-scoped, not document-scoped) */ diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentController.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentController.java index beb98256..5aa58ca7 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentController.java @@ -168,8 +168,8 @@ public class DocumentController { @DeleteMapping("/{id}") @RequirePermission(Permission.WRITE_ALL) - public ResponseEntity deleteDocument(@PathVariable UUID id) { - documentService.deleteDocument(id); + public ResponseEntity deleteDocument(@PathVariable UUID id, Authentication authentication) { + documentService.deleteDocument(id, requireUserId(authentication)); return ResponseEntity.noContent().build(); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentDeletingEvent.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentDeletingEvent.java new file mode 100644 index 00000000..ed30aa91 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentDeletingEvent.java @@ -0,0 +1,11 @@ +package org.raddatz.familienarchiv.document; + +import java.util.UUID; + +/** + * Published by DocumentService.deleteDocument inside its @Transactional boundary, + * before documentRepository.deleteById fires. Listeners run synchronously in the + * publisher's thread and transaction via plain @EventListener — this is load-bearing: + * see ADR-038. + */ +public record DocumentDeletingEvent(UUID documentId) {} 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 011ab3b1..61b578de 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -28,6 +28,7 @@ import org.raddatz.familienarchiv.ocr.TrainingLabel; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.tag.Tag; import org.raddatz.familienarchiv.document.DocumentRepository; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; @@ -80,6 +81,7 @@ public class DocumentService { private final TranscriptionBlockQueryService transcriptionBlockQueryService; private final AuditLogQueryService auditLogQueryService; private final ThumbnailAsyncRunner thumbnailAsyncRunner; + private final ApplicationEventPublisher eventPublisher; public record StoreResult(Document document, boolean isNew) {} @@ -1097,11 +1099,13 @@ public class DocumentService { } @Transactional - public void deleteDocument(UUID id) { + public void deleteDocument(UUID id, UUID actorId) { if (!documentRepository.existsById(id)) { throw DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id); } + eventPublisher.publishEvent(new DocumentDeletingEvent(id)); documentRepository.deleteById(id); + auditService.logAfterCommit(AuditKind.DOCUMENT_DELETED, actorId, id, null); } @Transactional 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/JourneyItemDocumentDeleteListener.java b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteListener.java new file mode 100644 index 00000000..01a7cd4e --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteListener.java @@ -0,0 +1,30 @@ +package org.raddatz.familienarchiv.geschichte.journeyitem; + +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.raddatz.familienarchiv.document.DocumentDeletingEvent; +import org.springframework.context.event.EventListener; +import org.springframework.stereotype.Component; + +@Component +@RequiredArgsConstructor +@Slf4j +class JourneyItemDocumentDeleteListener { + + private final JourneyItemRepository journeyItemRepository; + + /** + * Plain @EventListener — runs synchronously in the publisher's thread and transaction. + * Load-bearing choice: AFTER_COMMIT would fire after the FK ON DELETE SET NULL has + * already 500'd; @Async would run outside the delete transaction (breaks AC-5 rollback). + * See ADR-038. DocumentService cannot call JourneyItemService directly because + * Spring Framework 7 prohibits the resulting constructor-injection cycle. + */ + @EventListener + void onDocumentDeleting(DocumentDeletingEvent event) { + int deleted = journeyItemRepository.deleteNoteLessByDocumentId(event.documentId()); + if (deleted > 0) { + log.warn("Cascade-deleted {} note-less journey item(s) for document {}", deleted, event.documentId()); + } + } +} 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 66b9ab27..7f7d38ab 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,6 +1,7 @@ package org.raddatz.familienarchiv.geschichte.journeyitem; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; @@ -43,6 +44,20 @@ public interface JourneyItemRepository extends JpaRepository boolean existsByGeschichteIdAndDocumentId( @Param("geschichteId") UUID geschichteId, @Param("documentId") UUID documentId); + /** + * Deletes note-less items (note IS NULL or note = '') linked to the given document. + * Used by JourneyItemDocumentDeleteListener before the document row is removed, so + * the FK ON DELETE SET NULL never fires on rows that would violate chk_journey_item_not_empty. + * Explicit JPQL — same trap as existsByGeschichteIdAndDocumentId: the transient + * getDocumentId() getter makes Spring Data unable to resolve a derived query path. + * clearAutomatically = true invalidates the L1 cache so AC-2's "note-carrying survives" + * assertion never reads a stale entity. flushAutomatically = true makes the + * flush-before-delete contract explicit rather than relying on Hibernate AUTO flush mode. + */ + @Modifying(clearAutomatically = true, flushAutomatically = true) + @Query("DELETE FROM JourneyItem i WHERE i.document.id = :documentId AND (i.note IS NULL OR i.note = '')") + int deleteNoteLessByDocumentId(@Param("documentId") UUID documentId); + /** * 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() 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/document/DocumentControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java index b655e0bf..15934986 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java @@ -402,6 +402,7 @@ class DocumentControllerTest { @WithMockUser(authorities = "WRITE_ALL") void deleteDocument_returns204_whenHasWritePermission() throws Exception { UUID id = UUID.randomUUID(); + when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build()); mockMvc.perform(org.springframework.test.web.servlet.request.MockMvcRequestBuilders .delete("/api/documents/" + id).with(csrf())) .andExpect(status().isNoContent()); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java index 65fbb21f..eadad6ac 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java @@ -30,6 +30,7 @@ import org.raddatz.familienarchiv.document.DocumentRepository; import org.raddatz.familienarchiv.filestorage.FileService; import org.raddatz.familienarchiv.tag.TagService; import org.raddatz.familienarchiv.person.PersonService; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.PageRequest; @@ -75,6 +76,7 @@ class DocumentServiceTest { @Mock AuditLogQueryService auditLogQueryService; @Mock TranscriptionBlockQueryService transcriptionBlockQueryService; @Mock ThumbnailAsyncRunner thumbnailAsyncRunner; + @Mock ApplicationEventPublisher eventPublisher; // Real factory (pure, dependency-free) so save-time title-regeneration tests exercise the // shared composition rather than a stub — the #726 single source of truth. @Spy DocumentTitleFactory documentTitleFactory = new DocumentTitleFactory(); @@ -87,7 +89,7 @@ class DocumentServiceTest { UUID id = UUID.randomUUID(); when(documentRepository.existsById(id)).thenReturn(true); - documentService.deleteDocument(id); + documentService.deleteDocument(id, UUID.randomUUID()); verify(documentRepository).deleteById(id); } @@ -97,7 +99,7 @@ class DocumentServiceTest { UUID id = UUID.randomUUID(); when(documentRepository.existsById(id)).thenReturn(false); - assertThatThrownBy(() -> documentService.deleteDocument(id)) + assertThatThrownBy(() -> documentService.deleteDocument(id, UUID.randomUUID())) .isInstanceOf(DomainException.class) .hasMessageContaining(id.toString()); verify(documentRepository, never()).deleteById(any()); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteTest.java b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteTest.java new file mode 100644 index 00000000..f1319cb2 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/geschichte/journeyitem/JourneyItemDocumentDeleteTest.java @@ -0,0 +1,261 @@ +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; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; +import org.raddatz.familienarchiv.document.Document; +import org.raddatz.familienarchiv.document.DocumentRepository; +import org.raddatz.familienarchiv.document.DocumentService; +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.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.jdbc.core.JdbcTemplate; +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.test.context.bean.override.mockito.MockitoSpyBean; +import software.amazon.awssdk.services.s3.S3Client; + +import java.util.List; +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.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verify; + +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE) +@ActiveProfiles("test") +@Import(PostgresContainerConfig.class) +class JourneyItemDocumentDeleteTest { + + @MockitoBean + S3Client s3Client; + + @MockitoBean + AuditService auditService; + + @MockitoSpyBean + DocumentRepository documentRepository; + + @PersistenceContext + EntityManager em; + + @Autowired DocumentService documentService; + @Autowired JourneyItemRepository journeyItemRepository; + @Autowired GeschichteRepository geschichteRepository; + @Autowired DocumentRepository docRepo; + @Autowired AppUserRepository appUserRepository; + @Autowired JdbcTemplate jdbcTemplate; + + Geschichte journey; + Document doc; + AppUser writer; + + @BeforeEach + void seed() { + writer = appUserRepository.save(AppUser.builder() + .email("delete-test-writer@test") + .password("hash") + .build()); + doc = docRepo.save(Document.builder() + .title("Testbrief") + .originalFilename("testbrief.pdf") + .status(DocumentStatus.UPLOADED) + .build()); + journey = geschichteRepository.save(Geschichte.builder() + .title("Eine Lesereise") + .status(GeschichteStatus.DRAFT) + .type(GeschichteType.JOURNEY) + .build()); + SecurityContextHolder.getContext().setAuthentication( + new UsernamePasswordAuthenticationToken(writer.getEmail(), null, + List.of(new SimpleGrantedAuthority("BLOG_WRITE")))); + } + + @AfterEach + void cleanup() { + SecurityContextHolder.clearContext(); + reset(documentRepository); + // Deletion order is FK-load-bearing: journey_items reference both documents + // and geschichten, so children must be removed before their parents. + journeyItemRepository.deleteAll(); + docRepo.deleteAll(); + geschichteRepository.deleteAll(); + appUserRepository.deleteAll(); + } + + // ─── AC-1: headline ─────────────────────────────────────────────────────── + + @Test + void deleting_document_linked_via_note_less_item_deletes_item_not_500() { + JourneyItem item = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).document(doc).build()); + em.clear(); + + documentService.deleteDocument(doc.getId(), writer.getId()); + + assertThat(journeyItemRepository.findById(item.getId())).isEmpty(); + assertThat(docRepo.findById(doc.getId())).isEmpty(); + } + + // ─── AC-2: note-carrying item survives as placeholder ───────────────────── + + @Test + void deleting_document_preserves_note_carrying_item_as_placeholder() { + JourneyItem item = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).document(doc).note("curator context").build()); + em.clear(); + + documentService.deleteDocument(doc.getId(), writer.getId()); + em.clear(); + + JourneyItem surviving = journeyItemRepository.findById(item.getId()).orElseThrow(); + assertThat(surviving.getDocumentId()).isNull(); + assertThat(surviving.getNote()).isEqualTo("curator context"); + } + + // ─── AC-3: note-only item untouched ─────────────────────────────────────── + + @Test + void deleting_document_does_not_affect_note_only_item() { + JourneyItem noteOnly = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).note("Einleitung").build()); + em.clear(); + + documentService.deleteDocument(doc.getId(), writer.getId()); + em.clear(); + + JourneyItem reloaded = journeyItemRepository.findById(noteOnly.getId()).orElseThrow(); + assertThat(reloaded.getDocumentId()).isNull(); + assertThat(reloaded.getNote()).isEqualTo("Einleitung"); + } + + // ─── AC-4: asymmetric multi-journey ─────────────────────────────────────── + + @Test + void deleting_document_applies_independently_per_referencing_item() { + Geschichte journey2 = geschichteRepository.save(Geschichte.builder() + .title("Zweite Reise") + .status(GeschichteStatus.DRAFT) + .type(GeschichteType.JOURNEY) + .build()); + + JourneyItem noteLess = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).document(doc).build()); + JourneyItem noteCarrying = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey2).position(10).document(doc).note("Begleittext").build()); + em.clear(); + + documentService.deleteDocument(doc.getId(), writer.getId()); + em.clear(); + + assertThat(journeyItemRepository.findById(noteLess.getId())).isEmpty(); + JourneyItem surviving = journeyItemRepository.findById(noteCarrying.getId()).orElseThrow(); + assertThat(surviving.getDocumentId()).isNull(); + assertThat(surviving.getNote()).isEqualTo("Begleittext"); + } + + // ─── AC-5: rollback guard ───────────────────────────────────────────────── + + @Test + void listener_deletes_roll_back_when_document_delete_fails() { + JourneyItem item = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).document(doc).build()); + em.clear(); + + doThrow(new RuntimeException("simulated failure")) + .when(documentRepository).deleteById(any()); + + assertThatThrownBy(() -> documentService.deleteDocument(doc.getId(), writer.getId())) + .isInstanceOf(RuntimeException.class); + + em.clear(); + assertThat(journeyItemRepository.findById(item.getId())).isPresent(); + } + + // ─── AC-6: empty-string note boundary ──────────────────────────────────── + + @Test + void empty_string_note_item_is_cascaded_whitespace_only_note_is_preserved() { + // uq_journey_items_geschichte_document prevents two items with the same + // (geschichte_id, document_id) in one journey — use two separate journeys. + Geschichte journey2 = geschichteRepository.save(Geschichte.builder() + .title("Zweite Reise für AC-6") + .status(GeschichteStatus.DRAFT) + .type(GeschichteType.JOURNEY) + .build()); + + UUID emptyNoteItemId = UUID.randomUUID(); + UUID whitespaceNoteItemId = UUID.randomUUID(); + + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, document_id, note) VALUES (?,?,?,?,?)", + emptyNoteItemId, journey.getId(), 10, doc.getId(), ""); + jdbcTemplate.update( + "INSERT INTO journey_items (id, geschichte_id, position, document_id, note) VALUES (?,?,?,?,?)", + whitespaceNoteItemId, journey2.getId(), 20, doc.getId(), " "); + em.clear(); + + documentService.deleteDocument(doc.getId(), writer.getId()); + em.clear(); + + assertThat(journeyItemRepository.findById(emptyNoteItemId)).isEmpty(); + JourneyItem whitespaceItem = journeyItemRepository.findById(whitespaceNoteItemId).orElseThrow(); + assertThat(whitespaceItem.getDocumentId()).isNull(); + assertThat(whitespaceItem.getNote()).isEqualTo(" "); + } + + // ─── Idempotency / no-collateral ────────────────────────────────────────── + + @Test + void deleting_document_in_zero_journeys_returns_no_collateral() { + Document unlinked = docRepo.save(Document.builder() + .title("Unverknüpfter Brief") + .originalFilename("other.pdf") + .status(DocumentStatus.UPLOADED) + .build()); + JourneyItem unrelated = journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).note("unrelated note").build()); + em.clear(); + + documentService.deleteDocument(unlinked.getId(), writer.getId()); + em.clear(); + + assertThat(docRepo.findById(unlinked.getId())).isEmpty(); + assertThat(journeyItemRepository.findById(unrelated.getId())).isPresent(); + assertThat(journeyItemRepository.count()).isEqualTo(1); + } + + // ─── AC-7: audit — DOCUMENT_DELETED emitted, JOURNEY_ITEM_REMOVED absent ─ + + @Test + void deleting_document_emits_document_audit_but_no_journey_item_audit() { + journeyItemRepository.save( + JourneyItem.builder().geschichte(journey).position(10).document(doc).build()); + em.clear(); + + documentService.deleteDocument(doc.getId(), writer.getId()); + + verify(auditService).logAfterCommit(eq(AuditKind.DOCUMENT_DELETED), eq(writer.getId()), eq(doc.getId()), any()); + verify(auditService, never()).logAfterCommit(eq(AuditKind.JOURNEY_ITEM_REMOVED), any(), any(), any()); + } +} 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..59a2bf83 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 @@ -6,8 +6,10 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.PostgresContainerConfig; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.document.Document; import org.raddatz.familienarchiv.document.DocumentRepository; +import org.raddatz.familienarchiv.document.DocumentService; import org.raddatz.familienarchiv.document.DocumentStatus; import org.raddatz.familienarchiv.geschichte.Geschichte; import org.raddatz.familienarchiv.geschichte.GeschichteRepository; @@ -42,12 +44,16 @@ class JourneyItemIntegrationTest { @MockitoBean S3Client s3Client; + @MockitoBean + AuditService auditService; + @PersistenceContext EntityManager em; @Autowired GeschichteRepository geschichteRepository; @Autowired JourneyItemRepository journeyItemRepository; @Autowired JourneyItemService journeyItemService; + @Autowired DocumentService documentService; @Autowired DocumentRepository documentRepository; @Autowired AppUserRepository appUserRepository; @@ -212,8 +218,9 @@ class JourneyItemIntegrationTest { UUID itemId = saved.getItems().get(0).getId(); // extract before clear em.clear(); - // Delete document — ON DELETE SET NULL fires at DB level - documentRepository.deleteById(doc.getId()); + // Route through service so the DocumentDeletingEvent fires and the listener + // removes note-less items before ON DELETE SET NULL acts on note-carrying rows. + documentService.deleteDocument(doc.getId(), writer.getId()); em.flush(); em.clear(); @@ -284,6 +291,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_through_service() { + 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(); + + // Route through service so the DocumentDeletingEvent fires (V72 cascade fix). + documentService.deleteDocument(doc.getId(), writer.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/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index c1bf1d9e..dcc7acd2 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -38,7 +38,7 @@ Both stacks are organised **package-by-domain**: each domain owns its entities, **`user`** — login accounts and permission groups. Owns `AppUser`, `UserGroup`, invite tokens. Does NOT own `Person` records. Cross-domain deps: `audit` (user management events). -**`geschichte`** — family stories and Lesereisen. Owns `Geschichte` (`DRAFT → PUBLISHED` lifecycle) and `JourneyItem` (ordered stops in a JOURNEY-type Geschichte). Two subtypes: `STORY` (prose) and `JOURNEY` (curated document sequence). Cross-domain deps: `person` (linked persons), `document` (via `JourneyItem.document_id`, ON DELETE SET NULL). +**`geschichte`** — family stories and Lesereisen. Owns `Geschichte` (`DRAFT → PUBLISHED` lifecycle) and `JourneyItem` (document attachments / editorial notes shared by both subtypes — no application-level type guard). Two subtypes: `STORY` (prose + attached documents) and `JOURNEY` (ordered curated sequence). Cross-domain deps: `person` (linked persons), `document` (via `JourneyItem.document_id`, ON DELETE SET NULL). See ADR-037. **`notification`** — in-app messages. Owns `Notification`. Delivers via `SseEmitterRegistry` (live) and persisted rows (bell dropdown). Cross-domain deps: `user` (recipient), `document` (context). diff --git a/docs/GLOSSARY.md b/docs/GLOSSARY.md index 65e902b8..8fd84f5c 100644 --- a/docs/GLOSSARY.md +++ b/docs/GLOSSARY.md @@ -149,9 +149,9 @@ _See also [Chronik](#chronik-internal)._ **Chronik** `[internal]` — the conceptual and code-level name for the unified activity feed (per ADR-003 `003-chronik-unified-activity-feed.md`). Used in code, architecture documents, and ADRs. The user-facing label for the same concept is [Aktivität](#aktivitat--aktivitaten-user-facing). -**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. +**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 attaching documents via `journey_items`) 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` (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`). +**JourneyItem** (`JourneyItem`, table `journey_items`) `[internal]` — a document attachment or editorial note belonging to a `Geschichte` of either subtype. JOURNEY-type Geschichten use items for their ordered reading sequence; STORY-type Geschichten use items to attach referenced documents (no type guard is enforced at the application layer — both subtypes share this table). 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 Geschichte). 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`). See ADR-037. **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. diff --git a/docs/adr/037-journey-items-serve-both-geschichte-subtypes.md b/docs/adr/037-journey-items-serve-both-geschichte-subtypes.md new file mode 100644 index 00000000..099211f4 --- /dev/null +++ b/docs/adr/037-journey-items-serve-both-geschichte-subtypes.md @@ -0,0 +1,78 @@ +# ADR-037 — `journey_items` serves both STORY and JOURNEY Geschichte subtypes + +**Status:** Accepted +**Date:** 2026-06-11 +**Issue:** #795 (restore document management for STORY-type Geschichten), PR #804 review + +## Context + +V72 added the `journey_items` table as the backing store for Lesereisen (JOURNEY-type +Geschichten). At the same time, the previous `geschichten_documents` join table was +dropped (#753) and the restoration of a STORY-level document attachment mechanism was +deferred to a future issue. + +`JourneyItemService.append()` contained an application-level type guard that rejected +`append()` calls on STORY-type Geschichten with `GESCHICHTE_TYPE_MISMATCH`. This guard +was the only place where the STORY restriction was encoded — the database schema never +enforced it (no CHECK constraint, no partial index on `type='JOURNEY'`). + +When #795 restored document attachment for STORY-type Geschichten, the type guard was +the only obstacle. Two implementation paths were considered: + +1. Keep an allowlist (`if type not in (JOURNEY, STORY) throw ...`) — dead code today + because `GeschichteType` is a two-constant enum; the branch can never be reached and + would fail the JaCoCo branch-coverage gate. +2. Delete the guard entirely — the schema never encoded the restriction; deleting dead + application logic rather than replacing it with more dead logic. + +Path 2 was chosen. + +## Decision + +`journey_items` is the document-attachment mechanism for **both** `STORY` and `JOURNEY` +subtypes. No application-level type guard governs which subtype may hold items. The only +behavioral difference between the two subtypes' use of items is at the UI layer: + +- JOURNEY: items form an ordered reading sequence rendered as a *Lesereise*. +- STORY: items are a set of attached reference documents rendered as a sidebar panel. + +Both subtypes share the same capacity cap (100 items), dedup index, position semantics, +and DEFERRABLE constraint — enforced at the database layer, not re-implemented per subtype. + +The `GESCHICHTE_TYPE_MISMATCH` error code was removed end-to-end (backend enum, +frontend `ErrorCode` type + `getErrorMessage()` case, all three locale files). +`GESCHICHTE_TYPE_IMMUTABLE` is unrelated and was left intact. + +## Naming asymmetry (intentional) + +The error codes `JOURNEY_AT_CAPACITY` and `JOURNEY_DOCUMENT_ALREADY_ADDED` carry +journey-flavored names. Renaming them would ripple through `ErrorCode.java`, `errors.ts`, +and three locale files for zero behavior change. `StoryDocumentPanel` remaps these two +codes to story-worded user messages at the presentation layer — the asymmetry is a +documented decision, not an accident. + +## Alternatives rejected + +- **Separate `story_documents` join table for STORY** — creates two nearly-identical + schemas for the same concept (document attachment with dedup and ordering), doubles the + migration surface, and splits the capacity/dedup logic. Rejected as unnecessary + duplication. +- **Allowlist type guard (`if type not in (JOURNEY, STORY)`)** — unreachable dead code + under a two-constant enum; fails the JaCoCo branch gate. Rejected. +- **Per-subtype application validation** — the schema never encoded the restriction; an + application-only rule with no schema backing is the weakest kind of invariant and was + removed when the product decision reversed it. + +## Consequences + +- `JourneyItemService.append()` accepts items for any `Geschichte`, regardless of subtype. + The 100-item cap and dedup constraint apply to all. +- GLOSSARY.md and ARCHITECTURE.md updated to reflect that `JourneyItem` is not + JOURNEY-specific. +- The `l3-backend-3g-supporting.puml` C4 diagram updated: type-guard language removed, + `geschQuerySvc` rel label reads "Checks Geschichte existence" (not "and type"). +- `StoryDocumentPanel.svelte` is the STORY-side consumer; `JourneyEditor.svelte` is the + JOURNEY-side consumer. Neither is aware of the other. +- Known pre-existing constraint conflict: `ON DELETE SET NULL` on `journey_items.document_id` + combined with `chk_journey_item_not_empty` causes a DB-level 500 when a document linked + via a note-less item is deleted. Pre-existing; tracked in follow-up issue. diff --git a/docs/adr/038-domain-event-driven-journey-item-cleanup.md b/docs/adr/038-domain-event-driven-journey-item-cleanup.md new file mode 100644 index 00000000..83096f6e --- /dev/null +++ b/docs/adr/038-domain-event-driven-journey-item-cleanup.md @@ -0,0 +1,118 @@ +# ADR-038 — Domain event drives note-less journey-item cleanup on document delete + +**Status:** Accepted +**Date:** 2026-06-11 +**Issue:** #805 (P1 — deleting a document linked via a note-less journey_item 500s at DB constraint) + +## Context + +Two constraints in V72 encode contradictory rules for a journey item that has a +`document_id` but no `note`: + +- **`fk_journey_items_document` → `ON DELETE SET NULL`** — when a document is deleted, + Postgres nulls out `document_id`. +- **`chk_journey_item_not_empty`** — requires at least one of `document_id` or `note` + to be non-null. + +A note-less item (`document_id` set, `note IS NULL`) satisfies the CHECK while the +document exists. Deleting the document causes Postgres to attempt `SET NULL`, which +would leave both columns null — a direct CHECK violation. Postgres aborts the +transaction with a 500 that bypasses `GlobalExceptionHandler`. + +The natural fix — delete note-less items inside `DocumentService.deleteDocument` before +`deleteById` runs — cannot call `JourneyItemService` directly: `JourneyItemService` +already injects `DocumentService`, and Spring Framework 7 (used by Spring Boot 4) +**fully prohibits constructor-injection cycles**. The application will not start if such +a cycle is introduced. + +## Decision + +`DocumentService.deleteDocument` publishes a **`DocumentDeletingEvent`** (plain record, +payload: `documentId` UUID only) via `ApplicationEventPublisher` **before** +`documentRepository.deleteById`. A dedicated `@Component` +`JourneyItemDocumentDeleteListener` in the `geschichte.journeyitem` package consumes +this event and calls `journeyItemRepository.deleteNoteLessByDocumentId(documentId)` +directly — bypassing `JourneyItemService` to avoid re-introducing the cycle and to +suppress the per-item `JOURNEY_ITEM_REMOVED` audit emission (see audit decision below). + +### Load-bearing listener-phase choice: plain `@EventListener` + +The listener is annotated with `@EventListener` (not +`@TransactionalEventListener(AFTER_COMMIT)`, not `@Async`). **This choice is +load-bearing:** + +- **`AFTER_COMMIT` would break the fix entirely.** `AFTER_COMMIT` fires *after* the + surrounding transaction has committed. By that point, `documentRepository.deleteById` + has already executed and Postgres has already tried `ON DELETE SET NULL` — the + constraint violation fires before the listener ever runs. +- **`@Async` would break rollback atomicity (AC-5).** An async listener runs on a + separate thread in its own transaction. If `deleteDocument` subsequently rolls back + (e.g. due to an unrelated failure), the listener's deletes are in a committed async + transaction and cannot be undone. +- **Plain `@EventListener` runs synchronously in the publisher's thread and + transaction.** The listener's JPQL delete and the `deleteById` are a single atomic + unit: if either fails, both roll back together. + +### Repository method + +```java +@Modifying(clearAutomatically = true) +@Query("DELETE FROM JourneyItem i WHERE i.document.id = :documentId AND (i.note IS NULL OR i.note = '')") +int deleteNoteLessByDocumentId(@Param("documentId") UUID documentId); +``` + +`i.document.id` (the real association path) is used instead of `i.documentId`: the +transient `getDocumentId()` getter on `JourneyItem` makes Spring Data unable to resolve +a derived query path (same trap documented at `JourneyItemRepository:33-44`). + +`clearAutomatically = true` invalidates the L1 cache so subsequent reads in the same +session do not return stale entities. + +The predicate `(note IS NULL OR note = '')` covers the `note = ''` edge case that the +service layer can never produce (normalizeNote converts blank strings to null), but that +may exist via raw SQL inserts or legacy data. Whitespace-only notes (`note = ' '`) +do not match and are preserved as note-carrying placeholders. + +### Audit decision + +The listener calls the repository directly rather than routing through +`JourneyItemService.delete`. This deliberately bypasses the `JOURNEY_ITEM_REMOVED` +audit emission: a document used in multiple journeys would otherwise produce N audit +rows for a single user action. The `DOCUMENT_DELETED` entry written by `deleteDocument` +is the sole audit record for the operation. + +### Boundary: documents must not depend on journey + +The event direction is `document → journey`, never the reverse. `DocumentService` +publishes events it knows nothing about the consumers of; `JourneyItemService`'s +dependency on `DocumentService` is unchanged and remains the only cross-domain +reference. This direction is the prerequisite for the cycle constraint to hold. + +## Alternatives rejected + +- **DB trigger on `journey_items`** — trigger logic is opaque to Java developers, + invisible to code review, and not covered by the JPA test harness. +- **RESTRICT instead of SET NULL** — breaks the existing note-carrying placeholder + UX: deleting a document with a note-carrying journey item would 409 instead of + preserving the item as a placeholder. +- **Relax `chk_journey_item_not_empty`** — the constraint enforces a real invariant + (every item must have at least document or note). Removing it would allow empty rows. +- **`@Lazy` on the `JourneyItemService → DocumentService` injection** — Spring Boot 4 / + Spring Framework 7 prohibits constructor-injection cycles regardless of `@Lazy`. +- **Make `DocumentService` call `JourneyItemService`** — introduces the prohibited + cycle. Rejected at design time. + +## Consequences + +- **No schema change** — no new Flyway migration, no `db-orm.puml` / + `db-relationships.puml` update. +- This is the **first custom domain event** in the codebase. No prior + `ApplicationEventPublisher` usage existed in `main/`. New cross-domain cleanup that + cannot use direct service calls should follow this pattern. +- All tests that delete documents and then assert journey-item state **must route + through `DocumentService.deleteDocument`**, not `documentRepository.deleteById`. + The existing `JourneyItemIntegrationTest` tests that covered the note-carrying + placeholder UX have been updated accordingly. +- The `DOCUMENT_DELETED` `AuditKind` was added as part of this fix to give AC-7's + audit assertion a positive check (absence-only assertions pass vacuously if all + auditing regresses). diff --git a/docs/architecture/c4/l3-backend-3g-supporting.puml b/docs/architecture/c4/l3-backend-3g-supporting.puml index fba5056b..8f550f89 100644 --- a/docs/architecture/c4/l3-backend-3g-supporting.puml +++ b/docs/architecture/c4/l3-backend-3g-supporting.puml @@ -19,7 +19,8 @@ System_Boundary(backend, "API Backend (Spring Boot)") { 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(journeyItemSvc, "JourneyItemService", "Spring Service", "Manages journey item lifecycle: append (100-item cap), updateNote (three-way PATCH), delete, and reorder (DEFERRABLE position swap). Serves both STORY and JOURNEY subtypes.") + Component(journeyListener, "JourneyItemDocumentDeleteListener", "Spring @EventListener", "Consumes DocumentDeletingEvent synchronously inside the delete transaction and removes note-less journey items before ON DELETE SET NULL fires, preventing a chk_journey_item_not_empty violation. See ADR-038.") Component(exHandler, "GlobalExceptionHandler", "Spring @RestControllerAdvice", "Converts DomainException, validation errors, and generic exceptions to ErrorResponse JSON with machine-readable ErrorCode and HTTP status.") } @@ -41,9 +42,11 @@ 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(journeyItemSvc, geschQuerySvc, "Checks Geschichte existence and type") +Rel(journeyItemSvc, geschQuerySvc, "Checks Geschichte existence") Rel(geschQuerySvc, db, "Reads geschichten", "JDBC") Rel(journeyItemSvc, db, "Reads / writes journey_items", "JDBC") +Rel(documentSvc, journeyListener, "DocumentDeletingEvent", "in-process event") +Rel(journeyListener, db, "Deletes note-less journey_items", "JDBC") Rel(auditSvc, db, "Writes audit_log", "JDBC") Rel(auditQuery, db, "Reads audit_log", "JDBC") Rel(notifSvc, db, "Reads / writes notifications", "JDBC") diff --git a/docs/architecture/c4/l3-frontend-3c-people-stories.puml b/docs/architecture/c4/l3-frontend-3c-people-stories.puml index 544e7540..cf424d63 100644 --- a/docs/architecture/c4/l3-frontend-3c-people-stories.puml +++ b/docs/architecture/c4/l3-frontend-3c-people-stories.puml @@ -12,7 +12,7 @@ System_Boundary(frontend, "Web Frontend (SvelteKit / SSR)") { Component(personReview, "/persons/review", "SvelteKit Route", "Transcriber triage view (WRITE-gated link). Lists provisional persons; per-row Merge / Umbenennen / Bestätigen / Löschen. Actions: POST /merge, PUT /{id}, PATCH /{id}/confirm, DELETE /{id}.") Component(aktivitaeten, "/aktivitaeten", "SvelteKit Route", "Unified activity feed (Chronik). Loader: GET /api/dashboard/activity and GET /api/notifications?read=false.") Component(geschichten, "/geschichten and /geschichten/[id]", "SvelteKit Routes", "Story/Journey list and detail pages. List: GeschichteListRow with REISE badge for JOURNEY type. Detail: dispatches to StoryReader (rich text + persons) or JourneyReader (intro + ordered JourneyItemCard/JourneyInterlude items + empty state) based on GeschichteType. BLOG_WRITE users see edit/delete actions. Loader: GET /api/geschichten, GET /api/geschichten/{id}.") - Component(geschichtenEdit, "/geschichten/[id]/edit and /geschichten/new", "SvelteKit Routes", "Story editor and creation flow. New: TypeSelector (STORY/JOURNEY radio group with roving tabindex) → StoryCreate (TipTap rich text, person linking, POST /api/geschichten) or JourneyCreate (title + first item). Edit: branches on GeschichteType — STORY opens GeschichteEditor (TipTap body + GeschichteSidebar); JOURNEY opens JourneyEditor (title, intro textarea, ordered JourneyItemRow list with drag-reorder + move-up/down, JourneyAddBar for document/interlude addition, GeschichteSidebar). JourneyEditor mutations: POST/DELETE /items, PUT /items/reorder, PATCH /items/{id}. Requires BLOG_WRITE permission.") + Component(geschichtenEdit, "/geschichten/[id]/edit and /geschichten/new", "SvelteKit Routes", "Story editor and creation flow. New: TypeSelector (STORY/JOURNEY radio group with roving tabindex) → StoryCreate (TipTap rich text, person linking, POST /api/geschichten) or JourneyCreate (title + first item). Edit: branches on GeschichteType — STORY opens GeschichteEditor (TipTap body + GeschichteSidebar incl. StoryDocumentPanel: document linking via POST/DELETE /items); JOURNEY opens JourneyEditor (title, intro textarea, ordered JourneyItemRow list with drag-reorder + move-up/down, JourneyAddBar for document/interlude addition, GeschichteSidebar). JourneyEditor mutations: POST/DELETE /items, PUT /items/reorder, PATCH /items/{id}. Requires BLOG_WRITE permission.") Component(stammbaum, "/stammbaum", "SvelteKit Route", "Family tree visualisation. Loader: GET /api/network (nodes + edges). Renders interactive family tree from network graph data.") Component(themen, "/themen", "SvelteKit Route", "Browsable topic index. Shows all root tags as cards with color bars and child rows. ThemenWidget also embedded in the home dashboard (reader + editor sidebar). Loader: GET /api/tags/tree.") Component(profilePage, "/profile", "SvelteKit Route", "Current user profile settings. Loader: GET /api/users/me/notification-preferences. Actions: update name/password and notification preferences.") diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 204026cc..9e008600 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", @@ -1068,8 +1067,17 @@ "geschichte_editor_unsaved_changes": "Du hast ungespeicherte Änderungen — wirklich verlassen?", "geschichte_editor_personen_heading": "Personen", "geschichte_editor_personen_hint": "Welche historischen Personen kommen in dieser Geschichte vor?", - "geschichte_editor_dokumente_heading": "Dokumente", - "geschichte_editor_dokumente_hint": "Welche Briefe oder Dokumente sind Teil dieser Geschichte?", + "geschichte_documents_heading": "Briefe & Dokumente", + "geschichte_documents_hint": "Welche Dokumente gehören zu dieser Geschichte?", + "geschichte_documents_empty": "Noch keine Dokumente verknüpft. Suche unten nach einem Brief, um ihn dieser Geschichte hinzuzufügen.", + "geschichte_documents_picker_label": "Dokument hinzufügen", + "geschichte_documents_picker_placeholder": "Brief oder Dokument suchen…", + "geschichte_documents_deleted_placeholder": "Dokument wurde gelöscht", + "geschichte_documents_remove_label": "Dokument entfernen: {title}", + "geschichte_documents_capacity": "Diese Geschichte hat bereits die maximale Anzahl von Dokumenten (100) erreicht.", + "geschichte_documents_duplicate": "Dieses Dokument ist bereits mit der Geschichte verknüpft.", + "geschichte_documents_added_announce": "Hinzugefügt: {title}", + "geschichte_documents_removed_announce": "Entfernt: {title}", "geschichte_editor_search_person": "Person suchen…", "geschichte_editor_search_document": "Dokument suchen…", "geschichte_editor_toolbar_bold": "Fett (Strg+B)", @@ -1215,9 +1223,6 @@ "error_geschichte_title_too_long": "Der Titel ist zu lang (maximal 255 Zeichen).", "error_geschichte_intro_too_long": "Die Einleitung ist zu lang (maximal 4000 Zeichen).", "person_unknown": "[Unbekannt]", - "error_geschichte_title_too_long": "Der Titel ist zu lang (maximal 255 Zeichen).", - "error_geschichte_intro_too_long": "Die Einleitung ist zu lang (maximal 4000 Zeichen).", - "person_unknown": "[Unbekannt]", "error_journey_document_already_added": "Dieser Brief ist bereits in der Lesereise enthalten.", "error_geschichte_type_immutable": "Der Typ einer Geschichte kann nach der Erstellung nicht mehr geändert werden." } diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 4bce75a0..c837b5cc 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", @@ -1068,8 +1067,17 @@ "geschichte_editor_unsaved_changes": "You have unsaved changes — leave anyway?", "geschichte_editor_personen_heading": "People", "geschichte_editor_personen_hint": "Which historical persons appear in this story?", - "geschichte_editor_dokumente_heading": "Documents", - "geschichte_editor_dokumente_hint": "Which letters or documents are part of this story?", + "geschichte_documents_heading": "Letters & documents", + "geschichte_documents_hint": "Which documents belong to this story?", + "geschichte_documents_empty": "No documents linked yet. Search below for a letter to add it to this story.", + "geschichte_documents_picker_label": "Add document", + "geschichte_documents_picker_placeholder": "Search for a letter or document…", + "geschichte_documents_deleted_placeholder": "Document was deleted", + "geschichte_documents_remove_label": "Remove document: {title}", + "geschichte_documents_capacity": "This story has already reached the maximum of 100 documents.", + "geschichte_documents_duplicate": "This document is already linked to the story.", + "geschichte_documents_added_announce": "Added: {title}", + "geschichte_documents_removed_announce": "Removed: {title}", "geschichte_editor_search_person": "Search person…", "geschichte_editor_search_document": "Search document…", "geschichte_editor_toolbar_bold": "Bold (Ctrl+B)", @@ -1215,9 +1223,6 @@ "error_geschichte_title_too_long": "The title is too long (maximum 255 characters).", "error_geschichte_intro_too_long": "The introduction is too long (maximum 4000 characters).", "person_unknown": "[Unknown]", - "error_geschichte_title_too_long": "The title is too long (maximum 255 characters).", - "error_geschichte_intro_too_long": "The introduction is too long (maximum 4000 characters).", - "person_unknown": "[Unknown]", "error_journey_document_already_added": "This letter is already included in the reading journey.", "error_geschichte_type_immutable": "The type of a story cannot be changed after creation." } diff --git a/frontend/messages/es.json b/frontend/messages/es.json index 8a8b5706..b133a089 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", @@ -1068,8 +1067,17 @@ "geschichte_editor_unsaved_changes": "Tienes cambios no guardados — ¿salir igualmente?", "geschichte_editor_personen_heading": "Personas", "geschichte_editor_personen_hint": "¿Qué personas históricas aparecen en esta historia?", - "geschichte_editor_dokumente_heading": "Documentos", - "geschichte_editor_dokumente_hint": "¿Qué cartas o documentos forman parte de esta historia?", + "geschichte_documents_heading": "Cartas y documentos", + "geschichte_documents_hint": "¿Qué documentos pertenecen a esta historia?", + "geschichte_documents_empty": "Aún no hay documentos vinculados. Busca abajo una carta para añadirla a esta historia.", + "geschichte_documents_picker_label": "Añadir documento", + "geschichte_documents_picker_placeholder": "Buscar una carta o documento…", + "geschichte_documents_deleted_placeholder": "El documento fue eliminado", + "geschichte_documents_remove_label": "Quitar documento: {title}", + "geschichte_documents_capacity": "Esta historia ya ha alcanzado el número máximo de documentos (100).", + "geschichte_documents_duplicate": "Este documento ya está vinculado a la historia.", + "geschichte_documents_added_announce": "Añadido: {title}", + "geschichte_documents_removed_announce": "Quitado: {title}", "geschichte_editor_search_person": "Buscar persona…", "geschichte_editor_search_document": "Buscar documento…", "geschichte_editor_toolbar_bold": "Negrita (Ctrl+B)", @@ -1215,9 +1223,6 @@ "error_geschichte_title_too_long": "El título es demasiado largo (máximo 255 caracteres).", "error_geschichte_intro_too_long": "La introducción es demasiado larga (máximo 4000 caracteres).", "person_unknown": "[Desconocido]", - "error_geschichte_title_too_long": "El título es demasiado largo (máximo 255 caracteres).", - "error_geschichte_intro_too_long": "La introducción es demasiado larga (máximo 4000 caracteres).", - "person_unknown": "[Desconocido]", "error_journey_document_already_added": "Esta carta ya está incluida en el viaje de lectura.", "error_geschichte_type_immutable": "El tipo de una historia no se puede cambiar después de su creación." } diff --git a/frontend/src/lib/document/DocumentPickerDropdown.svelte b/frontend/src/lib/document/DocumentPickerDropdown.svelte index 5a3f454b..b3fa4a2a 100644 --- a/frontend/src/lib/document/DocumentPickerDropdown.svelte +++ b/frontend/src/lib/document/DocumentPickerDropdown.svelte @@ -10,17 +10,21 @@ import { interface Props { alreadyAddedIds?: Set; placeholder?: string; + /** Set when a visible