[MODORDERS-1143] Paying an invoice reopens the order (#1025)

* [MODORDERS-1143] Refactor existing code * [MODORDERS-1174] Minor refactoring * [MODORDERS-1174] Prevent incorrect order reopen * [MODORDERS-1143] Fix failing tests * [MODORDERS-1174] Simplify status change logic * [MODORDERS-1174] Cleanup * [MODORDERS-1143] Address comments * [MODORDERS-1143] Remove old method with side effects * [MODORDERS-1143] Fix future recovery issue * [MODORDERS-1143] Fix sonar issues * [MODORDERS-1143] Revert back to modifiable lists
folio-org · Sep 23, 2024 · 08a60f6 · 08a60f6
1 parent 3eefce4
commit 08a60f6
Show file tree

Hide file tree

Showing 14 changed files with 349 additions and 405 deletions.
diff --git a/src/main/java/org/folio/helper/CheckinReceivePiecesHelper.java b/src/main/java/org/folio/helper/CheckinReceivePiecesHelper.java
@@ -399,7 +399,7 @@ private PoLine updateRelatedPoLineDetails(PoLine poLine,
     }
 
     short updatedLocations = 0;
-    CompositePoLine compositePoLine = PoLineCommonUtil.convertToCompositePoLine(poLine);
+    var compositePoLine = PoLineCommonUtil.convertToCompositePoLine(poLine);
     for (Piece pieceToUpdate : successfullyProcessed) {
       Optional<Piece> relatedStoragePiece = piecesFromStorage.stream()
         .filter(piece -> piece.getId().equals(pieceToUpdate.getId()))
@@ -415,7 +415,7 @@ private PoLine updateRelatedPoLineDetails(PoLine poLine,
       updatedLocations++;
     }
 
-    return updatedLocations > 0 ? HelperUtils.convertToPoLine(compositePoLine) : poLine;
+    return updatedLocations > 0 ? PoLineCommonUtil.convertToPoLine(compositePoLine) : poLine;
   }
 
   /**

diff --git a/src/main/java/org/folio/helper/PurchaseOrderHelper.java b/src/main/java/org/folio/helper/PurchaseOrderHelper.java
@@ -6,7 +6,6 @@
 import static org.folio.orders.utils.HelperUtils.ORDER_CONFIG_MODULE_NAME;
 import static org.folio.orders.utils.HelperUtils.REASON_CANCELLED;
 import static org.folio.orders.utils.HelperUtils.WORKFLOW_STATUS;
-import static org.folio.orders.utils.HelperUtils.changeOrderStatus;
 import static org.folio.orders.utils.HelperUtils.collectResultsOnSuccess;
 import static org.folio.orders.utils.HelperUtils.convertToCompositePurchaseOrder;
 import static org.folio.orders.utils.OrderStatusTransitionUtil.isOrderClosing;
@@ -24,6 +23,7 @@
 import static org.folio.rest.jaxrs.model.CompositePurchaseOrder.WorkflowStatus.OPEN;
 import static org.folio.rest.jaxrs.model.CompositePurchaseOrder.WorkflowStatus.PENDING;
 import static org.folio.service.UserService.getCurrentUserId;
+import static org.folio.service.orders.utils.StatusUtils.changeOrderStatus;
 
 import java.util.Arrays;
 import java.util.List;
@@ -282,7 +282,7 @@ public Future<Void> handleFinalOrderStatus(CompositePurchaseOrder compPO, String
     } else {
       Future.succeededFuture()
         .map(v -> {
-          List<PoLine> poLines = HelperUtils.convertToPoLines(compPO.getCompositePoLines());
+          List<PoLine> poLines = PoLineCommonUtil.convertToPoLines(compPO.getCompositePoLines());
           if (initialOrdersStatus.equals(compPO.getWorkflowStatus().value()))
             changeOrderStatus(purchaseOrder, poLines);
           promise.complete(poLines);

diff --git a/src/main/java/org/folio/helper/PurchaseOrderLineHelper.java b/src/main/java/org/folio/helper/PurchaseOrderLineHelper.java
diff --git a/src/main/java/org/folio/models/PoLineInvoiceLineHolder.java b/src/main/java/org/folio/models/PoLineInvoiceLineHolder.java
@@ -7,18 +7,17 @@
 import org.folio.rest.jaxrs.model.CompositePoLine;
 import org.folio.rest.jaxrs.model.PoLine;
 
-import io.vertx.core.json.JsonObject;
-
 
 public class PoLineInvoiceLineHolder {
-  private CompositePoLine poLineFromRequest;
-  private PoLine poLineFromStorage;
+
+  private final CompositePoLine poLineFromRequest;
+  private final PoLine poLineFromStorage;
   private List<InvoiceLine> invoiceLines;
   private List<InvoiceLine> openOrReviewedInvoiceLines;
 
-  public PoLineInvoiceLineHolder(CompositePoLine poLineFromRequest, JsonObject poLineFromStorage) {
+  public PoLineInvoiceLineHolder(CompositePoLine poLineFromRequest, PoLine poLineFromStorage) {
     this.poLineFromRequest = poLineFromRequest;
-    this.poLineFromStorage = poLineFromStorage.mapTo(PoLine.class);
+    this.poLineFromStorage = poLineFromStorage;
     this.invoiceLines = new ArrayList<>();
     this.openOrReviewedInvoiceLines = new ArrayList<>();
   }

diff --git a/src/main/java/org/folio/orders/events/handlers/AbstractOrderStatusHandler.java b/src/main/java/org/folio/orders/events/handlers/AbstractOrderStatusHandler.java
@@ -1,7 +1,7 @@
 package org.folio.orders.events.handlers;
 
-import static org.folio.orders.utils.HelperUtils.changeOrderStatus;
 import static org.folio.orders.utils.HelperUtils.getOkapiHeaders;
+import static org.folio.service.orders.utils.StatusUtils.changeOrderStatus;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -10,8 +10,8 @@
 import org.folio.completablefuture.AsyncUtil;
 import org.folio.helper.BaseHelper;
 import org.folio.helper.PurchaseOrderHelper;
+import org.folio.orders.utils.PoLineCommonUtil;
 import org.folio.rest.core.models.RequestContext;
-import org.folio.rest.jaxrs.model.CompositePoLine;
 import org.folio.rest.jaxrs.model.CompositePurchaseOrder;
 import org.folio.rest.jaxrs.model.PoLine;
 import org.folio.rest.jaxrs.model.PurchaseOrder;
@@ -108,7 +108,7 @@ protected JsonArray messageAsJsonArray(String rootElement, Message<JsonObject> m
   }
 
   protected CompositePurchaseOrder convert(PurchaseOrder po, List<PoLine> poLines) {
-    var lines = poLines.stream().map(line -> JsonObject.mapFrom(line).mapTo(CompositePoLine.class)).toList();
+    var lines = poLines.stream().map(PoLineCommonUtil::convertToCompositePoLine).toList();
     return JsonObject.mapFrom(po).mapTo(CompositePurchaseOrder.class).withCompositePoLines(lines);
   }
 

diff --git a/src/main/java/org/folio/orders/utils/HelperUtils.java b/src/main/java/org/folio/orders/utils/HelperUtils.java
@@ -20,18 +20,13 @@
 import org.folio.rest.core.exceptions.HttpException;
 import org.folio.rest.core.exceptions.NoInventoryRecordException;
 import org.folio.rest.core.models.RequestContext;
-import org.folio.rest.jaxrs.model.Alert;
-import org.folio.rest.jaxrs.model.CloseReason;
 import org.folio.rest.jaxrs.model.CompositePoLine;
 import org.folio.rest.jaxrs.model.CompositePurchaseOrder;
 import org.folio.rest.jaxrs.model.Cost;
 import org.folio.rest.jaxrs.model.Location;
 import org.folio.rest.jaxrs.model.Piece;
 import org.folio.rest.jaxrs.model.PoLine;
-import org.folio.rest.jaxrs.model.PoLine.PaymentStatus;
-import org.folio.rest.jaxrs.model.PoLine.ReceiptStatus;
 import org.folio.rest.jaxrs.model.PurchaseOrder;
-import org.folio.rest.jaxrs.model.ReportingCode;
 import org.folio.rest.jaxrs.model.Title;
 import org.folio.service.exchange.ExchangeRateProviderResolver;
 import org.javamoney.moneta.Money;
@@ -58,19 +53,12 @@
 import java.util.regex.Pattern;
 
 import static io.vertx.core.Future.succeededFuture;
-import static java.util.stream.Collectors.toList;
 import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
 import static org.apache.commons.lang3.StringUtils.EMPTY;
 import static org.apache.commons.lang3.StringUtils.isEmpty;
-import static org.folio.orders.utils.ResourcePathResolver.ALERTS;
-import static org.folio.orders.utils.ResourcePathResolver.REPORTING_CODES;
 import static org.folio.rest.RestConstants.EN;
 import static org.folio.rest.core.exceptions.ErrorCodes.MULTIPLE_NONPACKAGE_TITLES;
 import static org.folio.rest.core.exceptions.ErrorCodes.TITLE_NOT_FOUND;
-import static org.folio.rest.jaxrs.model.PoLine.PaymentStatus.FULLY_PAID;
-import static org.folio.rest.jaxrs.model.PoLine.PaymentStatus.PAYMENT_NOT_REQUIRED;
-import static org.folio.rest.jaxrs.model.PoLine.ReceiptStatus.FULLY_RECEIVED;
-import static org.folio.rest.jaxrs.model.PoLine.ReceiptStatus.RECEIPT_NOT_REQUIRED;
 
 public class HelperUtils {
 
@@ -393,79 +381,11 @@ public static CompositePurchaseOrder convertToCompositePurchaseOrder(JsonObject
     return poJson.mapTo(CompositePurchaseOrder.class);
   }
 
-  public static boolean changeOrderStatus(PurchaseOrder purchaseOrder, List<PoLine> poLines) {
-    if (toBeCancelled(purchaseOrder, poLines)) {
-      purchaseOrder.setWorkflowStatus(PurchaseOrder.WorkflowStatus.CLOSED);
-      purchaseOrder.setCloseReason(new CloseReason().withReason(REASON_CANCELLED));
-      return true;
-    }
-
-    if (toBeClosed(purchaseOrder, poLines)) {
-      purchaseOrder.setWorkflowStatus(PurchaseOrder.WorkflowStatus.CLOSED);
-      purchaseOrder.setCloseReason(new CloseReason().withReason(REASON_COMPLETE));
-      return true;
-    }
-
-    if (toBeReopened(purchaseOrder, poLines)) {
-      purchaseOrder.setWorkflowStatus(PurchaseOrder.WorkflowStatus.OPEN);
-      return true;
-    }
-
-    return false;
-  }
-
   public static String convertTagListToCqlQuery(Collection<String> values, String fieldName, boolean strictMatch) {
-
     String prefix = fieldName + (strictMatch ? "==(\"" : "=(\"");
     return StreamEx.of(values).joining("\" or \"", prefix, "\")");
   }
 
-  private static boolean toBeClosed(PurchaseOrder purchaseOrder, List<PoLine> poLines) {
-    return purchaseOrder.getWorkflowStatus() == PurchaseOrder.WorkflowStatus.OPEN
-      && poLines.stream().allMatch(HelperUtils::isCompletedPoLine);
-  }
-
-  private static boolean toBeCancelled(PurchaseOrder purchaseOrder, List<PoLine> poLines) {
-    return purchaseOrder.getWorkflowStatus() == PurchaseOrder.WorkflowStatus.OPEN
-      && poLines.stream().allMatch(HelperUtils::isCancelledPoLine);
-  }
-
-  private static boolean isCancelledPoLine(PoLine line) {
-    PoLine.PaymentStatus paymentStatus = line.getPaymentStatus();
-    PoLine.ReceiptStatus receiptStatus = line.getReceiptStatus();
-    return paymentStatus == PaymentStatus.CANCELLED && receiptStatus == ReceiptStatus.CANCELLED;
-  }
-
-  private static boolean toBeReopened(PurchaseOrder purchaseOrder, List<PoLine> poLines) {
-    return purchaseOrder.getWorkflowStatus() == PurchaseOrder.WorkflowStatus.CLOSED
-      && poLines.stream().anyMatch(line -> !isCompletedPoLine(line));
-  }
-
-  private static boolean isCompletedPoLine(PoLine line) {
-    PoLine.PaymentStatus paymentStatus = line.getPaymentStatus();
-    PoLine.ReceiptStatus receiptStatus = line.getReceiptStatus();
-    return (paymentStatus == PAYMENT_NOT_REQUIRED || paymentStatus == FULLY_PAID || paymentStatus == PaymentStatus.CANCELLED)
-      && (receiptStatus == FULLY_RECEIVED || receiptStatus == RECEIPT_NOT_REQUIRED || receiptStatus == ReceiptStatus.CANCELLED);
-  }
-
-  public static PoLine convertToPoLine(CompositePoLine compPoLine) {
-    JsonObject pol = JsonObject.mapFrom(compPoLine);
-    pol.remove(ALERTS);
-    pol.remove(REPORTING_CODES);
-    PoLine poLine = pol.mapTo(PoLine.class);
-    poLine.setAlerts(compPoLine.getAlerts().stream().map(Alert::getId).collect(toList()));
-    poLine.setReportingCodes(compPoLine.getReportingCodes().stream().map(ReportingCode::getId).collect(toList()));
-    return poLine;
-  }
-
-
-  public static List<PoLine> convertToPoLines(List<CompositePoLine> compositePoLines) {
-    return compositePoLines
-      .stream()
-      .map(HelperUtils::convertToPoLine)
-      .collect(toList());
-  }
-
   public static boolean isProductIdsExist(CompositePoLine compPOL) {
     return compPOL.getDetails() != null && CollectionUtils.isNotEmpty(compPOL.getDetails().getProductIds());
   }
@@ -543,17 +463,11 @@ public static String extractCreatedDate(JsonObject json) {
   }
 
   public static CompositePurchaseOrder convertToCompositePurchaseOrder(PurchaseOrder purchaseOrder, List<PoLine> poLineList) {
-    List<CompositePoLine> compositePoLines = new ArrayList<>(poLineList.size());
-    if (CollectionUtils.isNotEmpty(poLineList)) {
-      poLineList.forEach(poLine -> {
-        poLine.setAlerts(null);
-        poLine.setReportingCodes(null);
-        CompositePoLine compositePoLine = PoLineCommonUtil.convertToCompositePoLine(poLine);
-        compositePoLines.add(compositePoLine);
-      });
-    }
-    JsonObject jsonLine = JsonObject.mapFrom(purchaseOrder);
-    return jsonLine.mapTo(CompositePurchaseOrder.class).withCompositePoLines(compositePoLines);
+    var purchaseOrderJson = JsonObject.mapFrom(purchaseOrder);
+    var compositePoLines = poLineList.stream()
+      .map(PoLineCommonUtil::convertToCompositePoLine)
+      .toList();
+    return purchaseOrderJson.mapTo(CompositePurchaseOrder.class).withCompositePoLines(compositePoLines);
   }
 
   public static void sendEvent(MessageAddress messageAddress, JsonObject data, RequestContext requestContext) {

diff --git a/src/main/java/org/folio/orders/utils/PoLineCommonUtil.java b/src/main/java/org/folio/orders/utils/PoLineCommonUtil.java
@@ -1,7 +1,10 @@
 package org.folio.orders.utils;
 
+import static java.util.stream.Collectors.toList;
 import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
 import static org.folio.orders.utils.HelperUtils.calculateTotalLocationQuantity;
+import static org.folio.orders.utils.ResourcePathResolver.ALERTS;
+import static org.folio.orders.utils.ResourcePathResolver.REPORTING_CODES;
 import static org.folio.rest.core.exceptions.ErrorCodes.PROHIBITED_FIELD_CHANGING;
 import static org.folio.rest.core.exceptions.ErrorCodes.WRONG_ONGOING_NOT_SUBSCRIPTION_FIELDS_CHANGED;
 import static org.folio.rest.core.exceptions.ErrorCodes.WRONG_ONGOING_SUBSCRIPTION_FIELDS_CHANGED;
@@ -27,6 +30,7 @@
 import org.apache.commons.lang3.ObjectUtils;
 import org.folio.rest.core.exceptions.HttpException;
 import org.folio.rest.core.exceptions.ErrorCodes;
+import org.folio.rest.jaxrs.model.Alert;
 import org.folio.rest.jaxrs.model.CompositePoLine;
 import org.folio.rest.jaxrs.model.CompositePurchaseOrder;
 import org.folio.rest.jaxrs.model.Eresource;
@@ -35,6 +39,7 @@
 import org.folio.rest.jaxrs.model.Parameter;
 import org.folio.rest.jaxrs.model.Physical;
 import org.folio.rest.jaxrs.model.PoLine;
+import org.folio.rest.jaxrs.model.ReportingCode;
 import org.folio.rest.tools.parser.JsonPathParser;
 
 import io.vertx.core.json.JsonArray;
@@ -250,10 +255,26 @@ public static List<String> getTenantsFromLocations(List<Location> locations) {
   }
 
   public static CompositePoLine convertToCompositePoLine(PoLine poLine) {
-    poLine.setAlerts(null);
-    poLine.setReportingCodes(null);
-    JsonObject jsonLine = JsonObject.mapFrom(poLine);
-    return jsonLine.mapTo(CompositePoLine.class);
+    var poLineJson = JsonObject.mapFrom(poLine);
+    poLineJson.remove(ALERTS);
+    poLineJson.remove(REPORTING_CODES);
+    return poLineJson.mapTo(CompositePoLine.class);
+  }
+
+  public static PoLine convertToPoLine(CompositePoLine compPoLine) {
+    var pol = JsonObject.mapFrom(compPoLine);
+    pol.remove(ALERTS);
+    pol.remove(REPORTING_CODES);
+    var poLine = pol.mapTo(PoLine.class);
+    poLine.setAlerts(compPoLine.getAlerts().stream().map(Alert::getId).collect(toList()));
+    poLine.setReportingCodes(compPoLine.getReportingCodes().stream().map(ReportingCode::getId).collect(toList()));
+    return poLine;
+  }
+
+  public static List<PoLine> convertToPoLines(List<CompositePoLine> compositePoLines) {
+    return compositePoLines.stream()
+      .map(PoLineCommonUtil::convertToPoLine)
+      .collect(toList());
   }
 
   public static void updateLocationsQuantity(List<Location> locations) {

diff --git a/src/main/java/org/folio/service/invoice/PoLineInvoiceLineHolderBuilder.java b/src/main/java/org/folio/service/invoice/PoLineInvoiceLineHolderBuilder.java
@@ -11,7 +11,6 @@
 import org.folio.rest.jaxrs.model.PoLine;
 
 import io.vertx.core.Future;
-import io.vertx.core.json.JsonObject;
 
 public class PoLineInvoiceLineHolderBuilder {
   private static final List<InvoiceLine.InvoiceLineStatus> EDITABLE_STATUSES = List.of(InvoiceLine.InvoiceLineStatus.OPEN, InvoiceLine.InvoiceLineStatus.REVIEWED);
@@ -22,7 +21,7 @@ public PoLineInvoiceLineHolderBuilder(InvoiceLineService invoiceLineService) {
   }
 
   public Future<PoLineInvoiceLineHolder> buildHolder(CompositePoLine compOrderLine, PoLine poLineFromStorage, RequestContext requestContext) {
-    PoLineInvoiceLineHolder holder = new PoLineInvoiceLineHolder(compOrderLine, JsonObject.mapFrom(poLineFromStorage));
+    var holder = new PoLineInvoiceLineHolder(compOrderLine, poLineFromStorage);
     return invoiceLineService.getInvoiceLinesByOrderLineId(compOrderLine.getId(), requestContext)
       .onSuccess(holder::withInvoiceLines)
       .compose(aResult -> CollectionUtils.isEmpty(holder.getInvoiceLines()) ? Future.succeededFuture(holder) :

diff --git a/src/main/java/org/folio/service/orders/OrderValidationService.java b/src/main/java/org/folio/service/orders/OrderValidationService.java
@@ -11,7 +11,6 @@
 import org.folio.helper.PurchaseOrderLineHelper;
 import org.folio.okapi.common.GenericCompositeFuture;
 import org.folio.orders.utils.AcqDesiredPermissions;
-import org.folio.orders.utils.HelperUtils;
 import org.folio.orders.utils.PoLineCommonUtil;
 import org.folio.orders.utils.ProtectedOperationType;
 import org.folio.rest.core.exceptions.ErrorCodes;
@@ -292,7 +291,7 @@ private Set<ProtectedOperationType> getInvolvedOperations(CompositePurchaseOrder
     if (CollectionUtils.isEmpty(compPO.getCompositePoLines())) {
       return Collections.singleton(UPDATE);
     }
-    List<PoLine> poLines = HelperUtils.convertToPoLines(poFromStorage.getCompositePoLines());
+    List<PoLine> poLines = PoLineCommonUtil.convertToPoLines(poFromStorage.getCompositePoLines());
     Set<String> newIds = compPO.getCompositePoLines().stream().map(CompositePoLine::getId).collect(Collectors.toSet());
     Set<String> storageIds = poLines.stream().map(PoLine::getId).collect(Collectors.toSet());
     Set<ProtectedOperationType> operations = new HashSet<>();

diff --git a/src/main/java/org/folio/service/orders/PurchaseOrderLineService.java b/src/main/java/org/folio/service/orders/PurchaseOrderLineService.java
@@ -126,7 +126,7 @@ public Future<Void> saveOrderLine(CompositePoLine compositePoLine, RequestContex
   }
 
   public Future<Void> saveOrderLine(CompositePoLine compositePoLine, List<Location> locations, RequestContext requestContext) {
-    PoLine poLine = HelperUtils.convertToPoLine(compositePoLine);
+    PoLine poLine = PoLineCommonUtil.convertToPoLine(compositePoLine);
     return saveOrderLine(poLine, locations, requestContext);
   }
 

diff --git a/src/main/java/org/folio/service/orders/flows/update/unopen/UnOpenCompositeOrderManager.java b/src/main/java/org/folio/service/orders/flows/update/unopen/UnOpenCompositeOrderManager.java
@@ -122,7 +122,7 @@ private void makePoLinePending(CompositePoLine poLine) {
 
   private Future<Void> updatePoLinesSummary(List<CompositePoLine> compositePoLines, RequestContext requestContext) {
     return GenericCompositeFuture.join(compositePoLines.stream()
-        .map(HelperUtils::convertToPoLine)
+        .map(PoLineCommonUtil::convertToPoLine)
         .map(line -> purchaseOrderLineService.saveOrderLine(line, requestContext))
         .collect(toList()))
       .map(ok -> null);