From 988796823629f5b7d893543f952546a4d2927c1f Mon Sep 17 00:00:00 2001 From: Marcel Date: Sun, 19 Apr 2026 13:24:22 +0200 Subject: [PATCH] feat(audit): instrument TranscriptionService for TEXT_SAVED and BLOCK_REVIEWED MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - reviewBlock: add userId param; log BLOCK_REVIEWED only on false→true - updateBlock: log TEXT_SAVED only when text actually changes; include pageNumber in payload (resolved from annotation) - Both events deferred via afterCommit() when inside a transaction - Update TranscriptionBlockController to pass user to reviewBlock() Co-Authored-By: Claude Sonnet 4.6 --- .../TranscriptionBlockController.java | 6 +- .../service/TranscriptionService.java | 38 +++++++- .../TranscriptionBlockControllerTest.java | 3 +- .../TranscriptionServiceGuidedTest.java | 5 +- .../service/TranscriptionServiceTest.java | 89 ++++++++++++++++++- 5 files changed, 131 insertions(+), 10 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/TranscriptionBlockController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/TranscriptionBlockController.java index 55f059e0..7b36cd26 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/TranscriptionBlockController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/TranscriptionBlockController.java @@ -85,8 +85,10 @@ public class TranscriptionBlockController { @RequirePermission(Permission.WRITE_ALL) public TranscriptionBlock reviewBlock( @PathVariable UUID documentId, - @PathVariable UUID blockId) { - return transcriptionService.reviewBlock(documentId, blockId); + @PathVariable UUID blockId, + Authentication authentication) { + UUID userId = requireUserId(authentication); + return transcriptionService.reviewBlock(documentId, blockId, userId); } @GetMapping("/{blockId}/history") diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java index 081f588d..4ec7cf94 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java @@ -2,6 +2,8 @@ package org.raddatz.familienarchiv.service; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.dto.CreateAnnotationDTO; import org.raddatz.familienarchiv.dto.CreateTranscriptionBlockDTO; import org.raddatz.familienarchiv.dto.ReorderTranscriptionBlocksDTO; @@ -19,8 +21,12 @@ import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.UUID; @Service @@ -37,6 +43,7 @@ public class TranscriptionService { private final AnnotationService annotationService; private final DocumentService documentService; private final SenderModelService senderModelService; + private final AuditService auditService; public List listBlocks(UUID documentId) { return blockRepository.findByDocumentIdOrderBySortOrderAsc(documentId); @@ -122,6 +129,7 @@ public class TranscriptionService { UpdateTranscriptionBlockDTO dto, UUID userId) { TranscriptionBlock block = getBlock(documentId, blockId); + String previousText = block.getText(); String text = sanitizeText(dto.getText()); block.setText(text); block.setSource(BlockSource.MANUAL); @@ -133,6 +141,12 @@ public class TranscriptionService { TranscriptionBlock saved = blockRepository.save(block); saveVersion(saved, userId); + if (!text.equals(previousText)) { + Optional annotation = annotationRepository.findById(block.getAnnotationId()); + int pageNumber = annotation.map(DocumentAnnotation::getPageNumber).orElse(0); + logAfterCommit(AuditKind.TEXT_SAVED, userId, documentId, Map.of("pageNumber", pageNumber)); + } + Document doc = documentService.getDocumentById(documentId); if (doc.getSender() != null && doc.getScriptType() == ScriptType.HANDWRITING_KURRENT) { senderModelService.checkAndTriggerTraining(doc.getSender().getId()); @@ -181,10 +195,15 @@ public class TranscriptionService { } @Transactional - public TranscriptionBlock reviewBlock(UUID documentId, UUID blockId) { + public TranscriptionBlock reviewBlock(UUID documentId, UUID blockId, UUID userId) { TranscriptionBlock block = getBlock(documentId, blockId); - block.setReviewed(!block.isReviewed()); - return blockRepository.save(block); + boolean wasReviewed = block.isReviewed(); + block.setReviewed(!wasReviewed); + TranscriptionBlock saved = blockRepository.save(block); + if (!wasReviewed && saved.isReviewed()) { + logAfterCommit(AuditKind.BLOCK_REVIEWED, userId, documentId, null); + } + return saved; } public List getBlockHistory(UUID documentId, UUID blockId) { @@ -208,4 +227,17 @@ public class TranscriptionService { } return text; } + + private void logAfterCommit(AuditKind kind, UUID actorId, UUID documentId, Map payload) { + if (TransactionSynchronizationManager.isActualTransactionActive()) { + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + auditService.log(kind, actorId, documentId, payload); + } + }); + } else { + auditService.log(kind, actorId, documentId, payload); + } + } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java index 84d58d79..2231e9fb 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/TranscriptionBlockControllerTest.java @@ -362,10 +362,11 @@ class TranscriptionBlockControllerTest { @Test @WithMockUser(authorities = "WRITE_ALL") void reviewBlock_returns200_withToggledBlock() throws Exception { + when(userService.findByEmail(any())).thenReturn(mockUser()); TranscriptionBlock reviewed = TranscriptionBlock.builder() .id(BLOCK_ID).documentId(DOC_ID).annotationId(UUID.randomUUID()) .text("text").sortOrder(0).reviewed(true).build(); - when(transcriptionService.reviewBlock(DOC_ID, BLOCK_ID)).thenReturn(reviewed); + when(transcriptionService.reviewBlock(eq(DOC_ID), eq(BLOCK_ID), any())).thenReturn(reviewed); mockMvc.perform(put("/api/documents/{documentId}/transcription-blocks/{blockId}/review", DOC_ID, BLOCK_ID)) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceGuidedTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceGuidedTest.java index 59eed034..0e786d83 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceGuidedTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceGuidedTest.java @@ -2,6 +2,7 @@ package org.raddatz.familienarchiv.service; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.model.BlockSource; import org.raddatz.familienarchiv.model.TranscriptionBlock; import org.raddatz.familienarchiv.repository.AnnotationRepository; @@ -23,6 +24,7 @@ class TranscriptionServiceGuidedTest { AnnotationService annotationService; DocumentService documentService; SenderModelService senderModelService; + AuditService auditService; TranscriptionService service; UUID docId = UUID.randomUUID(); @@ -37,9 +39,10 @@ class TranscriptionServiceGuidedTest { annotationService = mock(AnnotationService.class); documentService = mock(DocumentService.class); senderModelService = mock(SenderModelService.class); + auditService = mock(AuditService.class); service = new TranscriptionService(blockRepository, versionRepository, - annotationRepository, annotationService, documentService, senderModelService); + annotationRepository, annotationService, documentService, senderModelService, auditService); when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); when(versionRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java index 7fd2aee7..4e374c41 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java @@ -2,9 +2,12 @@ package org.raddatz.familienarchiv.service; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.raddatz.familienarchiv.audit.AuditKind; +import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.dto.CreateAnnotationDTO; import org.raddatz.familienarchiv.dto.CreateTranscriptionBlockDTO; import org.raddatz.familienarchiv.dto.ReorderTranscriptionBlocksDTO; @@ -21,6 +24,8 @@ import org.raddatz.familienarchiv.repository.AnnotationRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository; +import java.util.Map; + import java.util.List; import java.util.Optional; import java.util.UUID; @@ -42,6 +47,7 @@ class TranscriptionServiceTest { @Mock AnnotationService annotationService; @Mock DocumentService documentService; @Mock SenderModelService senderModelService; + @Mock AuditService auditService; @InjectMocks TranscriptionService transcriptionService; // ─── getBlock ──────────────────────────────────────────────────────────────── @@ -386,13 +392,14 @@ class TranscriptionServiceTest { void reviewBlock_setsReviewedTrue() { UUID docId = UUID.randomUUID(); UUID blockId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); TranscriptionBlock block = TranscriptionBlock.builder() .id(blockId).documentId(docId).annotationId(UUID.randomUUID()) .text("corrected text").sortOrder(0).reviewed(false).build(); when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.of(block)); when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - TranscriptionBlock result = transcriptionService.reviewBlock(docId, blockId); + TranscriptionBlock result = transcriptionService.reviewBlock(docId, blockId, userId); assertThat(result.isReviewed()).isTrue(); verify(blockRepository).save(block); @@ -402,13 +409,14 @@ class TranscriptionServiceTest { void reviewBlock_togglesReviewedFalse_whenAlreadyReviewed() { UUID docId = UUID.randomUUID(); UUID blockId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); TranscriptionBlock block = TranscriptionBlock.builder() .id(blockId).documentId(docId).annotationId(UUID.randomUUID()) .text("corrected text").sortOrder(0).reviewed(true).build(); when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.of(block)); when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); - TranscriptionBlock result = transcriptionService.reviewBlock(docId, blockId); + TranscriptionBlock result = transcriptionService.reviewBlock(docId, blockId, userId); assertThat(result.isReviewed()).isFalse(); } @@ -419,7 +427,82 @@ class TranscriptionServiceTest { UUID blockId = UUID.randomUUID(); when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.empty()); - assertThatThrownBy(() -> transcriptionService.reviewBlock(docId, blockId)) + assertThatThrownBy(() -> transcriptionService.reviewBlock(docId, blockId, UUID.randomUUID())) .isInstanceOf(DomainException.class); } + + @Test + void reviewBlock_logsBlockReviewed_whenFlippingFalseToTrue() { + UUID docId = UUID.randomUUID(); + UUID blockId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + TranscriptionBlock block = TranscriptionBlock.builder() + .id(blockId).documentId(docId).reviewed(false).build(); + when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.of(block)); + when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + transcriptionService.reviewBlock(docId, blockId, userId); + + verify(auditService).log(AuditKind.BLOCK_REVIEWED, userId, docId, null); + } + + @Test + void reviewBlock_doesNotLogEvent_whenFlippingTrueToFalse() { + UUID docId = UUID.randomUUID(); + UUID blockId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + TranscriptionBlock block = TranscriptionBlock.builder() + .id(blockId).documentId(docId).reviewed(true).build(); + when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.of(block)); + when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + + transcriptionService.reviewBlock(docId, blockId, userId); + + verify(auditService, never()).log(any(), any(), any(), any()); + } + + @Test + void updateBlock_logsTextSaved_whenTextChanges() { + UUID docId = UUID.randomUUID(); + UUID blockId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + UUID annotId = UUID.randomUUID(); + TranscriptionBlock block = TranscriptionBlock.builder() + .id(blockId).documentId(docId).annotationId(annotId).text("old text").build(); + DocumentAnnotation annotation = DocumentAnnotation.builder() + .id(annotId).pageNumber(3).build(); + when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.of(block)); + when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.getDocumentById(any())).thenReturn( + Document.builder().scriptType(ScriptType.TYPEWRITER).build()); + when(annotationRepository.findById(annotId)).thenReturn(Optional.of(annotation)); + + transcriptionService.updateBlock(docId, blockId, new UpdateTranscriptionBlockDTO("new text", null), userId); + + @SuppressWarnings("unchecked") + ArgumentCaptor> payloadCaptor = ArgumentCaptor.forClass(Map.class); + verify(auditService).log( + org.mockito.ArgumentMatchers.eq(AuditKind.TEXT_SAVED), + org.mockito.ArgumentMatchers.eq(userId), + org.mockito.ArgumentMatchers.eq(docId), + payloadCaptor.capture()); + assertThat(payloadCaptor.getValue()).containsEntry("pageNumber", 3); + } + + @Test + void updateBlock_doesNotLogEvent_whenTextUnchanged() { + UUID docId = UUID.randomUUID(); + UUID blockId = UUID.randomUUID(); + UUID userId = UUID.randomUUID(); + TranscriptionBlock block = TranscriptionBlock.builder() + .id(blockId).documentId(docId).annotationId(UUID.randomUUID()).text("same text").build(); + when(blockRepository.findByIdAndDocumentId(blockId, docId)).thenReturn(Optional.of(block)); + when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.getDocumentById(any())).thenReturn( + Document.builder().scriptType(ScriptType.TYPEWRITER).build()); + + transcriptionService.updateBlock(docId, blockId, new UpdateTranscriptionBlockDTO("same text", null), userId); + + verify(auditService, never()).log(any(), any(), any(), any()); + } }