From 19f1cf16a1e27847f014e64c719bf5e0f76a34a2 Mon Sep 17 00:00:00 2001 From: Sebastian Frick Date: Wed, 25 Feb 2026 22:59:07 +0100 Subject: [PATCH] fix(production): Review-Findings aus US-P17 beheben MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RescheduleNotAllowed Error-Typ statt InvalidStatusTransition(status, status), Null-Guard für newDate, Controller-Parameter als Enum statt String (verhindert 500 bei ungültigem Status), partielle Datumsangaben als 400 ablehnen, ORDER BY in findAll() konsistent mit gefilterten Queries. --- .../domain/production/ProductionOrder.java | 7 +++++-- .../production/ProductionOrderError.java | 5 +++++ .../JdbcProductionOrderRepository.java | 2 +- .../controller/ProductionOrderController.java | 20 ++++++++++++------- .../ProductionErrorHttpStatusMapper.java | 1 + .../RescheduleProductionOrderTest.java | 2 +- .../production/ProductionOrderTest.java | 17 +++++++++++++--- ...ductionOrderControllerIntegrationTest.java | 15 ++++++-------- 8 files changed, 46 insertions(+), 23 deletions(-) diff --git a/backend/src/main/java/de/effigenix/domain/production/ProductionOrder.java b/backend/src/main/java/de/effigenix/domain/production/ProductionOrder.java index 1129aa2..b8099b7 100644 --- a/backend/src/main/java/de/effigenix/domain/production/ProductionOrder.java +++ b/backend/src/main/java/de/effigenix/domain/production/ProductionOrder.java @@ -25,7 +25,7 @@ import java.time.ZoneOffset; * 10. Only IN_PROGRESS → COMPLETED transition allowed via complete() (Batch must be COMPLETED – enforced by Use Case) * 11. Only PLANNED or RELEASED → CANCELLED transition allowed via cancel(reason) * 12. CancelledReason is set exactly once during cancel() and must not be blank - * 13. Reschedule only in PLANNED or RELEASED, new date must not be in the past + * 13. Reschedule only in PLANNED or RELEASED, new date must not be null or in the past */ public class ProductionOrder { @@ -196,7 +196,10 @@ public class ProductionOrder { public Result reschedule(LocalDate newDate) { if (status != ProductionOrderStatus.PLANNED && status != ProductionOrderStatus.RELEASED) { - return Result.failure(new ProductionOrderError.InvalidStatusTransition(status, status)); + return Result.failure(new ProductionOrderError.RescheduleNotAllowed(status)); + } + if (newDate == null) { + return Result.failure(new ProductionOrderError.ValidationFailure("newDate must not be null")); } if (newDate.isBefore(LocalDate.now())) { return Result.failure(new ProductionOrderError.PlannedDateInPast(newDate)); diff --git a/backend/src/main/java/de/effigenix/domain/production/ProductionOrderError.java b/backend/src/main/java/de/effigenix/domain/production/ProductionOrderError.java index 30a429b..bb8e19a 100644 --- a/backend/src/main/java/de/effigenix/domain/production/ProductionOrderError.java +++ b/backend/src/main/java/de/effigenix/domain/production/ProductionOrderError.java @@ -50,6 +50,11 @@ public sealed interface ProductionOrderError { @Override public String message() { return "Batch '" + batchId.value() + "' is not in COMPLETED status"; } } + record RescheduleNotAllowed(ProductionOrderStatus current) implements ProductionOrderError { + @Override public String code() { return "PRODUCTION_ORDER_RESCHEDULE_NOT_ALLOWED"; } + @Override public String message() { return "Cannot reschedule production order in status " + current; } + } + record ValidationFailure(String message) implements ProductionOrderError { @Override public String code() { return "PRODUCTION_ORDER_VALIDATION_ERROR"; } } diff --git a/backend/src/main/java/de/effigenix/infrastructure/production/persistence/JdbcProductionOrderRepository.java b/backend/src/main/java/de/effigenix/infrastructure/production/persistence/JdbcProductionOrderRepository.java index 8c0523b..8a3092d 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/production/persistence/JdbcProductionOrderRepository.java +++ b/backend/src/main/java/de/effigenix/infrastructure/production/persistence/JdbcProductionOrderRepository.java @@ -47,7 +47,7 @@ public class JdbcProductionOrderRepository implements ProductionOrderRepository @Override public Result> findAll() { try { - var orders = jdbc.sql("SELECT * FROM production_orders ORDER BY created_at DESC") + var orders = jdbc.sql("SELECT * FROM production_orders ORDER BY planned_date, created_at DESC") .query(this::mapRow) .list(); return Result.success(orders); diff --git a/backend/src/main/java/de/effigenix/infrastructure/production/web/controller/ProductionOrderController.java b/backend/src/main/java/de/effigenix/infrastructure/production/web/controller/ProductionOrderController.java index 76cc17e..09e831a 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/production/web/controller/ProductionOrderController.java +++ b/backend/src/main/java/de/effigenix/infrastructure/production/web/controller/ProductionOrderController.java @@ -73,20 +73,26 @@ public class ProductionOrderController { public ResponseEntity> listProductionOrders( @RequestParam(required = false) @DateTimeFormat(iso = DateTimeFormat.ISO.DATE) LocalDate dateFrom, @RequestParam(required = false) @DateTimeFormat(iso = DateTimeFormat.ISO.DATE) LocalDate dateTo, - @RequestParam(required = false) String status, + @RequestParam(required = false) ProductionOrderStatus status, Authentication authentication ) { logger.info("Listing production orders by actor: {}", authentication.getName()); var actor = ActorId.of(authentication.getName()); - ProductionOrderStatus statusEnum = status != null ? ProductionOrderStatus.valueOf(status) : null; - var result = (dateFrom != null && dateTo != null && statusEnum != null) - ? listProductionOrders.executeByDateRangeAndStatus(dateFrom, dateTo, statusEnum, actor) - : (dateFrom != null && dateTo != null) + boolean hasDateRange = dateFrom != null && dateTo != null; + boolean hasPartialDate = (dateFrom != null) != (dateTo != null); + + if (hasPartialDate) { + return ResponseEntity.badRequest().build(); + } + + var result = hasDateRange && status != null + ? listProductionOrders.executeByDateRangeAndStatus(dateFrom, dateTo, status, actor) + : hasDateRange ? listProductionOrders.executeByDateRange(dateFrom, dateTo, actor) - : (statusEnum != null) - ? listProductionOrders.executeByStatus(statusEnum, actor) + : status != null + ? listProductionOrders.executeByStatus(status, actor) : listProductionOrders.execute(actor); if (result.isFailure()) { diff --git a/backend/src/main/java/de/effigenix/infrastructure/production/web/exception/ProductionErrorHttpStatusMapper.java b/backend/src/main/java/de/effigenix/infrastructure/production/web/exception/ProductionErrorHttpStatusMapper.java index 31176ee..a81bfd3 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/production/web/exception/ProductionErrorHttpStatusMapper.java +++ b/backend/src/main/java/de/effigenix/infrastructure/production/web/exception/ProductionErrorHttpStatusMapper.java @@ -58,6 +58,7 @@ public final class ProductionErrorHttpStatusMapper { case ProductionOrderError.RecipeNotFound e -> 404; case ProductionOrderError.RecipeNotActive e -> 409; case ProductionOrderError.InvalidStatusTransition e -> 409; + case ProductionOrderError.RescheduleNotAllowed e -> 409; case ProductionOrderError.BatchAlreadyAssigned e -> 409; case ProductionOrderError.BatchNotCompleted e -> 409; case ProductionOrderError.ValidationFailure e -> 400; diff --git a/backend/src/test/java/de/effigenix/application/production/RescheduleProductionOrderTest.java b/backend/src/test/java/de/effigenix/application/production/RescheduleProductionOrderTest.java index e2efc72..9fc3513 100644 --- a/backend/src/test/java/de/effigenix/application/production/RescheduleProductionOrderTest.java +++ b/backend/src/test/java/de/effigenix/application/production/RescheduleProductionOrderTest.java @@ -109,7 +109,7 @@ class RescheduleProductionOrderTest { var result = rescheduleProductionOrder.execute(validCommand(), performedBy); assertThat(result.isFailure()).isTrue(); - assertThat(result.unsafeGetError()).isInstanceOf(ProductionOrderError.InvalidStatusTransition.class); + assertThat(result.unsafeGetError()).isInstanceOf(ProductionOrderError.RescheduleNotAllowed.class); verify(productionOrderRepository, never()).save(any()); } diff --git a/backend/src/test/java/de/effigenix/domain/production/ProductionOrderTest.java b/backend/src/test/java/de/effigenix/domain/production/ProductionOrderTest.java index dd41bf3..eb12cb3 100644 --- a/backend/src/test/java/de/effigenix/domain/production/ProductionOrderTest.java +++ b/backend/src/test/java/de/effigenix/domain/production/ProductionOrderTest.java @@ -778,7 +778,7 @@ class ProductionOrderTest { var result = order.reschedule(LocalDate.now().plusDays(14)); assertThat(result.isFailure()).isTrue(); - var err = (ProductionOrderError.InvalidStatusTransition) result.unsafeGetError(); + var err = (ProductionOrderError.RescheduleNotAllowed) result.unsafeGetError(); assertThat(err.current()).isEqualTo(ProductionOrderStatus.IN_PROGRESS); } @@ -790,7 +790,7 @@ class ProductionOrderTest { var result = order.reschedule(LocalDate.now().plusDays(14)); assertThat(result.isFailure()).isTrue(); - var err = (ProductionOrderError.InvalidStatusTransition) result.unsafeGetError(); + var err = (ProductionOrderError.RescheduleNotAllowed) result.unsafeGetError(); assertThat(err.current()).isEqualTo(ProductionOrderStatus.COMPLETED); } @@ -802,10 +802,21 @@ class ProductionOrderTest { var result = order.reschedule(LocalDate.now().plusDays(14)); assertThat(result.isFailure()).isTrue(); - var err = (ProductionOrderError.InvalidStatusTransition) result.unsafeGetError(); + var err = (ProductionOrderError.RescheduleNotAllowed) result.unsafeGetError(); assertThat(err.current()).isEqualTo(ProductionOrderStatus.CANCELLED); } + @Test + @DisplayName("should fail when newDate is null") + void should_Fail_When_NewDateNull() { + var order = orderWithStatus(ProductionOrderStatus.PLANNED); + + var result = order.reschedule(null); + + assertThat(result.isFailure()).isTrue(); + assertThat(result.unsafeGetError()).isInstanceOf(ProductionOrderError.ValidationFailure.class); + } + @Test @DisplayName("should fail when new date is in the past") void should_Fail_When_DateInPast() { diff --git a/backend/src/test/java/de/effigenix/infrastructure/production/web/ProductionOrderControllerIntegrationTest.java b/backend/src/test/java/de/effigenix/infrastructure/production/web/ProductionOrderControllerIntegrationTest.java index d00f3ac..72f94ff 100644 --- a/backend/src/test/java/de/effigenix/infrastructure/production/web/ProductionOrderControllerIntegrationTest.java +++ b/backend/src/test/java/de/effigenix/infrastructure/production/web/ProductionOrderControllerIntegrationTest.java @@ -887,7 +887,7 @@ class ProductionOrderControllerIntegrationTest extends AbstractIntegrationTest { .contentType(MediaType.APPLICATION_JSON) .content(json)) .andExpect(status().isConflict()) - .andExpect(jsonPath("$.code").value("PRODUCTION_ORDER_INVALID_STATUS_TRANSITION")); + .andExpect(jsonPath("$.code").value("PRODUCTION_ORDER_RESCHEDULE_NOT_ALLOWED")); } @Test @@ -928,7 +928,7 @@ class ProductionOrderControllerIntegrationTest extends AbstractIntegrationTest { .contentType(MediaType.APPLICATION_JSON) .content(json)) .andExpect(status().isConflict()) - .andExpect(jsonPath("$.code").value("PRODUCTION_ORDER_INVALID_STATUS_TRANSITION")); + .andExpect(jsonPath("$.code").value("PRODUCTION_ORDER_RESCHEDULE_NOT_ALLOWED")); } @Test @@ -955,7 +955,7 @@ class ProductionOrderControllerIntegrationTest extends AbstractIntegrationTest { .contentType(MediaType.APPLICATION_JSON) .content(json)) .andExpect(status().isConflict()) - .andExpect(jsonPath("$.code").value("PRODUCTION_ORDER_INVALID_STATUS_TRANSITION")); + .andExpect(jsonPath("$.code").value("PRODUCTION_ORDER_RESCHEDULE_NOT_ALLOWED")); } @Test @@ -1078,15 +1078,12 @@ class ProductionOrderControllerIntegrationTest extends AbstractIntegrationTest { } @Test - @DisplayName("Nur dateFrom ohne dateTo → alle auflisten (kein Range-Filter)") - void listWithOnlyDateFrom_returnsAll() throws Exception { - createPlannedOrder(); - + @DisplayName("Nur dateFrom ohne dateTo → 400 Bad Request") + void listWithOnlyDateFrom_returns400() throws Exception { mockMvc.perform(get("/api/production/production-orders") .param("dateFrom", LocalDate.now().toString()) .header("Authorization", "Bearer " + adminToken)) - .andExpect(status().isOk()) - .andExpect(jsonPath("$.length()").value(org.hamcrest.Matchers.greaterThanOrEqualTo(1))); + .andExpect(status().isBadRequest()); } @Test