diff --git a/backend/docs/tickets/003-review-batch-block-reason-persistence.md b/backend/docs/tickets/003-review-batch-block-reason-persistence.md new file mode 100644 index 0000000..4fb3e2e --- /dev/null +++ b/backend/docs/tickets/003-review-batch-block-reason-persistence.md @@ -0,0 +1,41 @@ +# Ticket 003 – Prüfung: Sperr-Grund (reason) im Domain Model persistieren? + +**Datum:** 2026-02-20 +**Commit:** e7c3258 (feat(inventory): Charge sperren/entsperren) +**Status:** Offen + +--- + +## Kontext + +Beim Code-Review von `blockBatch`/`unblockBatch` wurde festgestellt, dass der `reason`-Parameter +(Sperr-Grund) zwar über die API entgegengenommen und validiert wird (`@NotBlank`), aber aktuell +nur im Audit-Log als `details` gespeichert wird – nicht im Domain Model selbst. + +Das DDD-Modell (`docs/mvp/ddd/07-inventory-bc.md`, Zeile 63) sieht die Signatur +`blockBatch(StockBatchId batchId, String reason)` vor, d.h. der `reason` war ursprünglich +als Teil der Domain-Logik geplant. + +## Zu klären + +1. **Soll `reason` in `StockBatch` persistiert werden?** + - Pro: Direkt abfragbar ohne Audit-Log-Suche, fachlich sichtbar im Bestandsüberblick + - Contra: Audit-Log reicht für Nachvollziehbarkeit (HACCP/GoBD), zusätzliches Feld = Schema-Migration + +2. **Falls ja: als optionales Feld `blockReason` in `StockBatch`?** + - Nur gesetzt wenn `status == BLOCKED` + - Wird bei `unblockBatch` auf `null` zurückgesetzt + - Erfordert: Liquibase-Migration, JPA-Entity-Anpassung, Mapper-Update + +3. **Domain-Event `StockBatchBlocked(StockId, StockBatchId, String reason)`?** + - Das DDD-Modell (Zeile 302) sieht dieses Event vor + - Aktuell keine Domain-Event-Infrastruktur implementiert → separates Thema + +## Aktuelle Lösung + +`reason` wird im Audit-Log via `AuditLogger.log(STOCK_BATCH_BLOCKED, batchId, "Reason: " + reason, actorId)` festgehalten. Damit ist die HACCP-Compliance gewährleistet. + +## Empfehlung + +Kurzfristig reicht der Audit-Log-Ansatz. Mittelfristig (wenn Chargen-Sperrgrund in der UI angezeigt +werden soll) sollte `blockReason` als optionales Feld in `StockBatch` aufgenommen werden. diff --git a/backend/src/main/java/de/effigenix/application/inventory/BlockStockBatch.java b/backend/src/main/java/de/effigenix/application/inventory/BlockStockBatch.java index 5e6126b..048b4e8 100644 --- a/backend/src/main/java/de/effigenix/application/inventory/BlockStockBatch.java +++ b/backend/src/main/java/de/effigenix/application/inventory/BlockStockBatch.java @@ -1,20 +1,25 @@ package de.effigenix.application.inventory; import de.effigenix.application.inventory.command.BlockStockBatchCommand; +import de.effigenix.application.usermanagement.AuditEvent; +import de.effigenix.application.usermanagement.AuditLogger; import de.effigenix.domain.inventory.*; import de.effigenix.shared.common.Result; +import de.effigenix.shared.security.ActorId; import org.springframework.transaction.annotation.Transactional; @Transactional public class BlockStockBatch { private final StockRepository stockRepository; + private final AuditLogger auditLogger; - public BlockStockBatch(StockRepository stockRepository) { + public BlockStockBatch(StockRepository stockRepository, AuditLogger auditLogger) { this.stockRepository = stockRepository; + this.auditLogger = auditLogger; } - public Result execute(BlockStockBatchCommand cmd) { + public Result execute(BlockStockBatchCommand cmd, ActorId performedBy) { // 1. Stock laden Stock stock; switch (stockRepository.findById(StockId.of(cmd.stockId()))) { @@ -41,6 +46,7 @@ public class BlockStockBatch { case Result.Success(var ignored) -> { } } + auditLogger.log(AuditEvent.STOCK_BATCH_BLOCKED, cmd.batchId(), "Reason: " + cmd.reason(), performedBy); return Result.success(null); } } diff --git a/backend/src/main/java/de/effigenix/application/inventory/UnblockStockBatch.java b/backend/src/main/java/de/effigenix/application/inventory/UnblockStockBatch.java index 7d26770..ca21978 100644 --- a/backend/src/main/java/de/effigenix/application/inventory/UnblockStockBatch.java +++ b/backend/src/main/java/de/effigenix/application/inventory/UnblockStockBatch.java @@ -1,20 +1,27 @@ package de.effigenix.application.inventory; import de.effigenix.application.inventory.command.UnblockStockBatchCommand; +import de.effigenix.application.usermanagement.AuditEvent; +import de.effigenix.application.usermanagement.AuditLogger; import de.effigenix.domain.inventory.*; import de.effigenix.shared.common.Result; +import de.effigenix.shared.security.ActorId; import org.springframework.transaction.annotation.Transactional; +import java.time.LocalDate; + @Transactional public class UnblockStockBatch { private final StockRepository stockRepository; + private final AuditLogger auditLogger; - public UnblockStockBatch(StockRepository stockRepository) { + public UnblockStockBatch(StockRepository stockRepository, AuditLogger auditLogger) { this.stockRepository = stockRepository; + this.auditLogger = auditLogger; } - public Result execute(UnblockStockBatchCommand cmd) { + public Result execute(UnblockStockBatchCommand cmd, ActorId performedBy) { // 1. Stock laden Stock stock; switch (stockRepository.findById(StockId.of(cmd.stockId()))) { @@ -29,7 +36,7 @@ public class UnblockStockBatch { } // 2. Batch entsperren (Domain validiert) - switch (stock.unblockBatch(StockBatchId.of(cmd.batchId()))) { + switch (stock.unblockBatch(StockBatchId.of(cmd.batchId()), LocalDate.now())) { case Result.Failure(var err) -> { return Result.failure(err); } case Result.Success(var ignored) -> { } } @@ -41,6 +48,7 @@ public class UnblockStockBatch { case Result.Success(var ignored) -> { } } + auditLogger.log(AuditEvent.STOCK_BATCH_UNBLOCKED, cmd.batchId(), performedBy); return Result.success(null); } } diff --git a/backend/src/main/java/de/effigenix/application/usermanagement/AuditEvent.java b/backend/src/main/java/de/effigenix/application/usermanagement/AuditEvent.java index a3b07b1..eb3a3e2 100644 --- a/backend/src/main/java/de/effigenix/application/usermanagement/AuditEvent.java +++ b/backend/src/main/java/de/effigenix/application/usermanagement/AuditEvent.java @@ -46,6 +46,8 @@ public enum AuditEvent { STOCK_ADJUSTED, STOCK_MOVEMENT_RECORDED, INVENTORY_COUNT_PERFORMED, + STOCK_BATCH_BLOCKED, + STOCK_BATCH_UNBLOCKED, // ==================== Procurement BC ==================== PURCHASE_ORDER_CREATED, diff --git a/backend/src/main/java/de/effigenix/domain/inventory/Stock.java b/backend/src/main/java/de/effigenix/domain/inventory/Stock.java index d2c83ad..68c38ae 100644 --- a/backend/src/main/java/de/effigenix/domain/inventory/Stock.java +++ b/backend/src/main/java/de/effigenix/domain/inventory/Stock.java @@ -173,7 +173,7 @@ public class Stock { return Result.success(null); } - public Result unblockBatch(StockBatchId batchId) { + public Result unblockBatch(StockBatchId batchId, LocalDate referenceDate) { StockBatch batch = batches.stream() .filter(b -> b.id().equals(batchId)) .findFirst() @@ -186,8 +186,8 @@ public class Stock { } StockBatchStatus newStatus = StockBatchStatus.AVAILABLE; - if (minimumShelfLife != null) { - LocalDate threshold = LocalDate.now().plusDays(minimumShelfLife.days()); + if (minimumShelfLife != null && batch.expiryDate() != null) { + LocalDate threshold = referenceDate.plusDays(minimumShelfLife.days()); if (batch.expiryDate().isBefore(threshold)) { newStatus = StockBatchStatus.EXPIRING_SOON; } diff --git a/backend/src/main/java/de/effigenix/infrastructure/config/InventoryUseCaseConfiguration.java b/backend/src/main/java/de/effigenix/infrastructure/config/InventoryUseCaseConfiguration.java index 09bf4d6..bb726a5 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/config/InventoryUseCaseConfiguration.java +++ b/backend/src/main/java/de/effigenix/infrastructure/config/InventoryUseCaseConfiguration.java @@ -10,6 +10,7 @@ import de.effigenix.application.inventory.CreateStorageLocation; import de.effigenix.application.inventory.DeactivateStorageLocation; import de.effigenix.application.inventory.ListStorageLocations; import de.effigenix.application.inventory.UpdateStorageLocation; +import de.effigenix.application.usermanagement.AuditLogger; import de.effigenix.domain.inventory.StockRepository; import de.effigenix.domain.inventory.StorageLocationRepository; import org.springframework.context.annotation.Bean; @@ -63,12 +64,12 @@ public class InventoryUseCaseConfiguration { } @Bean - public BlockStockBatch blockStockBatch(StockRepository stockRepository) { - return new BlockStockBatch(stockRepository); + public BlockStockBatch blockStockBatch(StockRepository stockRepository, AuditLogger auditLogger) { + return new BlockStockBatch(stockRepository, auditLogger); } @Bean - public UnblockStockBatch unblockStockBatch(StockRepository stockRepository) { - return new UnblockStockBatch(stockRepository); + public UnblockStockBatch unblockStockBatch(StockRepository stockRepository, AuditLogger auditLogger) { + return new UnblockStockBatch(stockRepository, auditLogger); } } diff --git a/backend/src/main/java/de/effigenix/infrastructure/inventory/web/controller/StockController.java b/backend/src/main/java/de/effigenix/infrastructure/inventory/web/controller/StockController.java index 6b8404b..fc19447 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/inventory/web/controller/StockController.java +++ b/backend/src/main/java/de/effigenix/infrastructure/inventory/web/controller/StockController.java @@ -11,6 +11,7 @@ import de.effigenix.application.inventory.command.CreateStockCommand; import de.effigenix.application.inventory.command.RemoveStockBatchCommand; import de.effigenix.application.inventory.command.UnblockStockBatchCommand; import de.effigenix.domain.inventory.StockError; +import de.effigenix.shared.security.ActorId; import de.effigenix.infrastructure.inventory.web.dto.AddStockBatchRequest; import de.effigenix.infrastructure.inventory.web.dto.BlockStockBatchRequest; import de.effigenix.infrastructure.inventory.web.dto.CreateStockRequest; @@ -136,7 +137,7 @@ public class StockController { logger.info("Blocking batch {} of stock {} by actor: {}", batchId, stockId, authentication.getName()); var cmd = new BlockStockBatchCommand(stockId, batchId, request.reason()); - var result = blockStockBatch.execute(cmd); + var result = blockStockBatch.execute(cmd, ActorId.of(authentication.getName())); if (result.isFailure()) { throw new StockDomainErrorException(result.unsafeGetError()); @@ -156,7 +157,7 @@ public class StockController { logger.info("Unblocking batch {} of stock {} by actor: {}", batchId, stockId, authentication.getName()); var cmd = new UnblockStockBatchCommand(stockId, batchId); - var result = unblockStockBatch.execute(cmd); + var result = unblockStockBatch.execute(cmd, ActorId.of(authentication.getName())); if (result.isFailure()) { throw new StockDomainErrorException(result.unsafeGetError()); diff --git a/backend/src/test/java/de/effigenix/application/inventory/BlockStockBatchTest.java b/backend/src/test/java/de/effigenix/application/inventory/BlockStockBatchTest.java index 064a80f..bcecfde 100644 --- a/backend/src/test/java/de/effigenix/application/inventory/BlockStockBatchTest.java +++ b/backend/src/test/java/de/effigenix/application/inventory/BlockStockBatchTest.java @@ -1,11 +1,13 @@ package de.effigenix.application.inventory; import de.effigenix.application.inventory.command.BlockStockBatchCommand; +import de.effigenix.application.usermanagement.AuditLogger; import de.effigenix.domain.inventory.*; import de.effigenix.shared.common.Quantity; import de.effigenix.shared.common.RepositoryError; import de.effigenix.shared.common.Result; import de.effigenix.shared.common.UnitOfMeasure; +import de.effigenix.shared.security.ActorId; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -29,15 +31,17 @@ import static org.mockito.Mockito.*; class BlockStockBatchTest { @Mock private StockRepository stockRepository; + @Mock private AuditLogger auditLogger; private BlockStockBatch blockStockBatch; private StockBatchId batchId; private Stock existingStock; private BlockStockBatchCommand validCommand; + private final ActorId actor = ActorId.of("test-user"); @BeforeEach void setUp() { - blockStockBatch = new BlockStockBatch(stockRepository); + blockStockBatch = new BlockStockBatch(stockRepository, auditLogger); batchId = StockBatchId.of("batch-1"); var batch = StockBatch.reconstitute( @@ -67,11 +71,12 @@ class BlockStockBatchTest { .thenReturn(Result.success(Optional.of(existingStock))); when(stockRepository.save(any())).thenReturn(Result.success(null)); - var result = blockStockBatch.execute(validCommand); + var result = blockStockBatch.execute(validCommand, actor); assertThat(result.isSuccess()).isTrue(); verify(stockRepository).save(existingStock); assertThat(existingStock.batches().getFirst().status()).isEqualTo(StockBatchStatus.BLOCKED); + verify(auditLogger).log(any(), eq("batch-1"), eq("Reason: Quality issue"), eq(actor)); } @Test @@ -80,11 +85,12 @@ class BlockStockBatchTest { when(stockRepository.findById(StockId.of("stock-1"))) .thenReturn(Result.success(Optional.empty())); - var result = blockStockBatch.execute(validCommand); + var result = blockStockBatch.execute(validCommand, actor); assertThat(result.isFailure()).isTrue(); assertThat(result.unsafeGetError()).isInstanceOf(StockError.StockNotFound.class); verify(stockRepository, never()).save(any()); + verifyNoInteractions(auditLogger); } @Test @@ -93,11 +99,12 @@ class BlockStockBatchTest { when(stockRepository.findById(StockId.of("stock-1"))) .thenReturn(Result.failure(new RepositoryError.DatabaseError("connection lost"))); - var result = blockStockBatch.execute(validCommand); + var result = blockStockBatch.execute(validCommand, actor); assertThat(result.isFailure()).isTrue(); assertThat(result.unsafeGetError()).isInstanceOf(StockError.RepositoryFailure.class); verify(stockRepository, never()).save(any()); + verifyNoInteractions(auditLogger); } @Test @@ -108,10 +115,11 @@ class BlockStockBatchTest { when(stockRepository.save(any())) .thenReturn(Result.failure(new RepositoryError.DatabaseError("connection lost"))); - var result = blockStockBatch.execute(validCommand); + var result = blockStockBatch.execute(validCommand, actor); assertThat(result.isFailure()).isTrue(); assertThat(result.unsafeGetError()).isInstanceOf(StockError.RepositoryFailure.class); + verifyNoInteractions(auditLogger); } @Test @@ -121,11 +129,12 @@ class BlockStockBatchTest { .thenReturn(Result.success(Optional.of(existingStock))); var cmd = new BlockStockBatchCommand("stock-1", "nonexistent", "Quality issue"); - var result = blockStockBatch.execute(cmd); + var result = blockStockBatch.execute(cmd, actor); assertThat(result.isFailure()).isTrue(); assertThat(result.unsafeGetError()).isInstanceOf(StockError.BatchNotFound.class); verify(stockRepository, never()).save(any()); + verifyNoInteractions(auditLogger); } @Test @@ -149,10 +158,11 @@ class BlockStockBatchTest { when(stockRepository.findById(StockId.of("stock-1"))) .thenReturn(Result.success(Optional.of(stock))); - var result = blockStockBatch.execute(validCommand); + var result = blockStockBatch.execute(validCommand, actor); assertThat(result.isFailure()).isTrue(); assertThat(result.unsafeGetError()).isInstanceOf(StockError.BatchAlreadyBlocked.class); verify(stockRepository, never()).save(any()); + verifyNoInteractions(auditLogger); } } diff --git a/backend/src/test/java/de/effigenix/application/inventory/UnblockStockBatchTest.java b/backend/src/test/java/de/effigenix/application/inventory/UnblockStockBatchTest.java index ac242e7..dd63ead 100644 --- a/backend/src/test/java/de/effigenix/application/inventory/UnblockStockBatchTest.java +++ b/backend/src/test/java/de/effigenix/application/inventory/UnblockStockBatchTest.java @@ -1,11 +1,13 @@ package de.effigenix.application.inventory; import de.effigenix.application.inventory.command.UnblockStockBatchCommand; +import de.effigenix.application.usermanagement.AuditLogger; import de.effigenix.domain.inventory.*; import de.effigenix.shared.common.Quantity; import de.effigenix.shared.common.RepositoryError; import de.effigenix.shared.common.Result; import de.effigenix.shared.common.UnitOfMeasure; +import de.effigenix.shared.security.ActorId; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -29,15 +31,17 @@ import static org.mockito.Mockito.*; class UnblockStockBatchTest { @Mock private StockRepository stockRepository; + @Mock private AuditLogger auditLogger; private UnblockStockBatch unblockStockBatch; private StockBatchId batchId; private Stock existingStock; private UnblockStockBatchCommand validCommand; + private final ActorId actor = ActorId.of("test-user"); @BeforeEach void setUp() { - unblockStockBatch = new UnblockStockBatch(stockRepository); + unblockStockBatch = new UnblockStockBatch(stockRepository, auditLogger); batchId = StockBatchId.of("batch-1"); var batch = StockBatch.reconstitute( @@ -67,11 +71,12 @@ class UnblockStockBatchTest { .thenReturn(Result.success(Optional.of(existingStock))); when(stockRepository.save(any())).thenReturn(Result.success(null)); - var result = unblockStockBatch.execute(validCommand); + var result = unblockStockBatch.execute(validCommand, actor); assertThat(result.isSuccess()).isTrue(); verify(stockRepository).save(existingStock); assertThat(existingStock.batches().getFirst().status()).isEqualTo(StockBatchStatus.AVAILABLE); + verify(auditLogger).log(any(), eq("batch-1"), eq(actor)); } @Test @@ -80,11 +85,12 @@ class UnblockStockBatchTest { when(stockRepository.findById(StockId.of("stock-1"))) .thenReturn(Result.success(Optional.empty())); - var result = unblockStockBatch.execute(validCommand); + var result = unblockStockBatch.execute(validCommand, actor); assertThat(result.isFailure()).isTrue(); assertThat(result.unsafeGetError()).isInstanceOf(StockError.StockNotFound.class); verify(stockRepository, never()).save(any()); + verifyNoInteractions(auditLogger); } @Test @@ -93,11 +99,12 @@ class UnblockStockBatchTest { when(stockRepository.findById(StockId.of("stock-1"))) .thenReturn(Result.failure(new RepositoryError.DatabaseError("connection lost"))); - var result = unblockStockBatch.execute(validCommand); + var result = unblockStockBatch.execute(validCommand, actor); assertThat(result.isFailure()).isTrue(); assertThat(result.unsafeGetError()).isInstanceOf(StockError.RepositoryFailure.class); verify(stockRepository, never()).save(any()); + verifyNoInteractions(auditLogger); } @Test @@ -108,10 +115,11 @@ class UnblockStockBatchTest { when(stockRepository.save(any())) .thenReturn(Result.failure(new RepositoryError.DatabaseError("connection lost"))); - var result = unblockStockBatch.execute(validCommand); + var result = unblockStockBatch.execute(validCommand, actor); assertThat(result.isFailure()).isTrue(); assertThat(result.unsafeGetError()).isInstanceOf(StockError.RepositoryFailure.class); + verifyNoInteractions(auditLogger); } @Test @@ -121,11 +129,12 @@ class UnblockStockBatchTest { .thenReturn(Result.success(Optional.of(existingStock))); var cmd = new UnblockStockBatchCommand("stock-1", "nonexistent"); - var result = unblockStockBatch.execute(cmd); + var result = unblockStockBatch.execute(cmd, actor); assertThat(result.isFailure()).isTrue(); assertThat(result.unsafeGetError()).isInstanceOf(StockError.BatchNotFound.class); verify(stockRepository, never()).save(any()); + verifyNoInteractions(auditLogger); } @Test @@ -149,10 +158,11 @@ class UnblockStockBatchTest { when(stockRepository.findById(StockId.of("stock-1"))) .thenReturn(Result.success(Optional.of(stock))); - var result = unblockStockBatch.execute(validCommand); + var result = unblockStockBatch.execute(validCommand, actor); assertThat(result.isFailure()).isTrue(); assertThat(result.unsafeGetError()).isInstanceOf(StockError.BatchNotBlocked.class); verify(stockRepository, never()).save(any()); + verifyNoInteractions(auditLogger); } } diff --git a/backend/src/test/java/de/effigenix/domain/inventory/StockTest.java b/backend/src/test/java/de/effigenix/domain/inventory/StockTest.java index eb7c052..8a7c474 100644 --- a/backend/src/test/java/de/effigenix/domain/inventory/StockTest.java +++ b/backend/src/test/java/de/effigenix/domain/inventory/StockTest.java @@ -513,7 +513,7 @@ class StockTest { var stock = createStockWithBatch("10", UnitOfMeasure.KILOGRAM, StockBatchStatus.BLOCKED); var batchId = stock.batches().getFirst().id(); - var result = stock.unblockBatch(batchId); + var result = stock.unblockBatch(batchId, LocalDate.now()); assertThat(result.isSuccess()).isTrue(); assertThat(stock.batches().getFirst().status()).isEqualTo(StockBatchStatus.AVAILABLE); @@ -539,7 +539,7 @@ class StockTest { ); var batchId = stock.batches().getFirst().id(); - var result = stock.unblockBatch(batchId); + var result = stock.unblockBatch(batchId, LocalDate.now()); assertThat(result.isSuccess()).isTrue(); assertThat(stock.batches().getFirst().status()).isEqualTo(StockBatchStatus.EXPIRING_SOON); @@ -565,7 +565,7 @@ class StockTest { ); var batchId = stock.batches().getFirst().id(); - var result = stock.unblockBatch(batchId); + var result = stock.unblockBatch(batchId, LocalDate.now()); assertThat(result.isSuccess()).isTrue(); assertThat(stock.batches().getFirst().status()).isEqualTo(StockBatchStatus.AVAILABLE); @@ -577,7 +577,7 @@ class StockTest { var stock = createStockWithBatch("10", UnitOfMeasure.KILOGRAM, StockBatchStatus.AVAILABLE); var batchId = stock.batches().getFirst().id(); - var result = stock.unblockBatch(batchId); + var result = stock.unblockBatch(batchId, LocalDate.now()); assertThat(result.isFailure()).isTrue(); assertThat(result.unsafeGetError()).isInstanceOf(StockError.BatchNotBlocked.class); @@ -588,7 +588,7 @@ class StockTest { void shouldFailWhenBatchNotFound() { var stock = createValidStock(); - var result = stock.unblockBatch(StockBatchId.of("nonexistent")); + var result = stock.unblockBatch(StockBatchId.of("nonexistent"), LocalDate.now()); assertThat(result.isFailure()).isTrue(); assertThat(result.unsafeGetError()).isInstanceOf(StockError.BatchNotFound.class);