Improve validation and reject inconsistent order lines

folio-org · Jan 31, 2024 · cc02c81 · yuntianhu · Jan 31, 2024 · yuntianhu
1 parent b85aa14
commit cc02c81
Show file tree

Hide file tree

Showing 10 changed files with 507 additions and 35 deletions.
diff --git a/src/main/java/org/folio/rest/core/exceptions/ErrorCodes.java b/src/main/java/org/folio/rest/core/exceptions/ErrorCodes.java
@@ -114,7 +114,11 @@ public enum ErrorCodes {
   FUND_LOCATION_RESTRICTION_VIOLATION("fundLocationRestrictionViolation", "One of the funds is restricted to be used for one of the locations."),
   ENCUMBRANCES_FOR_RE_ENCUMBER_NOT_FOUND("encumbrancesForReEncumberNotFound", "The encumbrances were correctly created during the rollover or have already been updated."),
   CLAIMING_CONFIG_INVALID("claimingConfigInvalid", "Claiming interval should be set and greater than 0 if claiming is active"),
-  TEMPLATE_NAME_ALREADY_EXISTS("templateNameNotUnique", "Template name already exists");
+  TEMPLATE_NAME_ALREADY_EXISTS("templateNameNotUnique", "Template name already exists"),
+  INVALID_PHYSICAL_POL("physicalPOLShouldContainOnlyPhysicalElement", "Physical POL should contain only \"physical\" element"),
+  INVALID_ELECTRONIC_POL("electronicPOLShouldContainOnlyElectronicElement", "Electronic POL should contain only \"electronic\" element"),
+  INVALID_PEMIX_POL("peMixPOLShouldContainPhysicalAndElectronicElement", "P\\E mix POL should contain \"physical\" and \"electronic\" element"),
+  INVALID_OTHER_POL("otherPOLShouldContainPhysicalElement", " Other should contain\"physical\" element");
 
 
   private final String code;

diff --git a/src/main/java/org/folio/service/orders/CompositePoLineValidationService.java b/src/main/java/org/folio/service/orders/CompositePoLineValidationService.java
@@ -34,7 +34,6 @@
 
 import io.vertx.core.Future;
 
-
 public class CompositePoLineValidationService extends BaseValidationService {
 
   private final ExpenseClassValidationService expenseClassValidationService;
@@ -55,6 +54,7 @@ public Future<List<Error>> validatePoLine(CompositePoLine compPOL, RequestContex
     return expenseClassValidationService.validateExpenseClasses(List.of(compPOL), false, requestContext)
       .onSuccess(v -> errors.addAll(validatePoLineFormats(compPOL)))
       .onSuccess(v -> errors.addAll(validateLocations(compPOL)))
+      .onSuccess(v -> errors.addAll(validatePoLineMaterial(compPOL)))
       .map(v -> {
         errors.addAll(validateCostPrices(compPOL));
         return errors;
@@ -76,6 +76,66 @@ private List<Error> validatePoLineFormats(CompositePoLine compPOL) {
     return Collections.emptyList();
   }
 
+  public List<Error> validatePoLineMaterial(CompositePoLine compPOL) {
+
+    CompositePoLine.OrderFormat orderFormat = compPOL.getOrderFormat();
+
+    if (orderFormat == P_E_MIX) {
+      return P_E_MixCheck(compPOL);
+    } else if (orderFormat == ELECTRONIC_RESOURCE) {
+      return electronicResourceCheck(compPOL);
+    } else if (orderFormat == CompositePoLine.OrderFormat.PHYSICAL_RESOURCE) {
+      return physicalResourceCheck(compPOL);
+    } else if (orderFormat == CompositePoLine.OrderFormat.OTHER) {
+      return otherResourceCheck(compPOL);
+    }
+
+    return Collections.emptyList();
+  }
+
+  //to check if the P_E_Mix missing either physical or electronic resources
+  private List<Error> P_E_MixCheck (CompositePoLine compPOL ){
+    List<ErrorCodes> errors = new ArrayList<>();
+    if(compPOL.getPhysical()==null || compPOL.getEresource()==null){
+     errors.add(ErrorCodes.INVALID_PEMIX_POL);
+    }else if (getElectronicCostQuantity(compPOL)==0 || getPhysicalCostQuantity(compPOL)==0){
+      errors.add(ErrorCodes.INVALID_PEMIX_POL);
+    }
+    return convertErrorCodesToErrors(compPOL, errors);
+  }
+
+  // to check if electronic order format contains physical resources
+  private List<Error>  electronicResourceCheck (CompositePoLine compPOL ){
+    List<ErrorCodes> errors = new ArrayList<>();
+    if((compPOL.getPhysical()!=null)) {
+      if (getPhysicalCostQuantity(compPOL) != 0) {
+        errors.add(ErrorCodes.INVALID_ELECTRONIC_POL);
+      }
+    }
+    return convertErrorCodesToErrors(compPOL, errors);
+  }
+
+  // to check if physical order format contains electronic resources
+  private List<Error> physicalResourceCheck (CompositePoLine compPOL ){
+    List<ErrorCodes> errors = new ArrayList<>();
+    if(compPOL.getEresource()!=null) {
+      if (getElectronicCostQuantity(compPOL) != 0) {
+        errors.add(ErrorCodes.INVALID_PHYSICAL_POL);
+      }
+    }
+    return convertErrorCodesToErrors(compPOL, errors);
+  }
+
+  // to check if other order format contains electronic resources
+  private List<Error> otherResourceCheck (CompositePoLine compPOL ){
+    List<ErrorCodes> errors = new ArrayList<>();
+    if(compPOL.getEresource()!=null) {
+      if (getElectronicCostQuantity(compPOL) != 0) {
+        errors.add(ErrorCodes.INVALID_OTHER_POL);
+      }
+    }
+    return convertErrorCodesToErrors(compPOL, errors);
+  }
   private List<Error> validatePoLineWithMixedFormat(CompositePoLine compPOL) {
 
     List<ErrorCodes> errors = new ArrayList<>();

diff --git a/src/main/resources/postgres-conf.json b/src/main/resources/postgres-conf.json
@@ -0,0 +1,7 @@
+{
+  "host" : "localhost",
+  "port" : 5432,
+  "database" : "okapi_modules",
+  "username" : "folio_admin",
+  "password" : "folio_admin"
+}
diff --git a/src/main/resources/templates/db_scripts/schema.json b/src/main/resources/templates/db_scripts/schema.json
diff --git a/src/test/java/org/folio/rest/impl/PurchaseOrderLinesApiTest.java b/src/test/java/org/folio/rest/impl/PurchaseOrderLinesApiTest.java
@@ -59,6 +59,7 @@
 import static org.folio.rest.core.exceptions.ErrorCodes.ELECTRONIC_COST_LOC_QTY_MISMATCH;
 import static org.folio.rest.core.exceptions.ErrorCodes.INACTIVE_EXPENSE_CLASS;
 import static org.folio.rest.core.exceptions.ErrorCodes.INSTANCE_ID_NOT_ALLOWED_FOR_PACKAGE_POLINE;
+import static org.folio.rest.core.exceptions.ErrorCodes.INVALID_ELECTRONIC_POL;
 import static org.folio.rest.core.exceptions.ErrorCodes.ISBN_NOT_VALID;
 import static org.folio.rest.core.exceptions.ErrorCodes.LOCATION_CAN_NOT_BE_MODIFIER_AFTER_OPEN;
 import static org.folio.rest.core.exceptions.ErrorCodes.NON_ZERO_COST_ELECTRONIC_QTY;
@@ -449,7 +450,7 @@ void testPutOrderLineElectronicFormatIncorrectQuantityAndPrice() {
     final Errors response = verifyPut(String.format(LINE_BY_ID_PATH, reqData.getId()), JsonObject.mapFrom(reqData),
       APPLICATION_JSON, 422).as(Errors.class);
 
-    assertThat(response.getErrors(), hasSize(8));
+    assertThat(response.getErrors(), hasSize(9));
     List<String> errorCodes = response.getErrors()
       .stream()
       .map(Error::getCode)
@@ -462,7 +463,8 @@ void testPutOrderLineElectronicFormatIncorrectQuantityAndPrice() {
       COST_ADDITIONAL_COST_INVALID.getCode(),
       COST_DISCOUNT_INVALID.getCode(),
       ELECTRONIC_COST_LOC_QTY_MISMATCH.getCode(),
-      PHYSICAL_COST_LOC_QTY_MISMATCH.getCode()));
+      PHYSICAL_COST_LOC_QTY_MISMATCH.getCode(),
+      INVALID_ELECTRONIC_POL.getCode()));
 
 
     // Check that no any calls made by the business logic to other services
@@ -1681,7 +1683,7 @@ public void testFundDistributionValidationWhenZeroPriceAndDifferentDistributionT
   void testPutPhysicalOrderLineByIdWhenSpecificElementIsPresentAndProtectedFieldsChanged(CompositePoLine.OrderFormat orderFormat) {
     logger.info("=== Test PUT Order Line By Id - Protected fields changed ===");
 
-    String lineId = "0009662b-8b80-4001-b704-ca10971f222d";
+    String lineId = "0009662b-8b80-4001-b704-ca10971f222e";
     JsonObject body = getMockAsJson(PO_LINES_MOCK_DATA_PATH, lineId);
     Object[] expected = new Object[]{ POLineFieldNames.ACQUISITION_METHOD.getFieldName()};
     if (CompositePoLine.OrderFormat.ELECTRONIC_RESOURCE == orderFormat) {
@@ -1749,7 +1751,7 @@ void testPutElecOrderLineByIdWhenSpecificElementIsPresentAndProtectedFieldsChang
   void testPutMixedOrderLineByIdWhenSpecificElementIsPresentAndProtectedFieldsChanged(CompositePoLine.OrderFormat orderFormat) {
     logger.info("=== Test PUT Order Line By Id - Protected fields changed ===");
 
-    String lineId = "0009662b-8b80-4001-b704-ca10971f222d";
+    String lineId = "0009662b-8b80-4001-b704-ca10971f222f";
     JsonObject body = getMockAsJson(PO_LINES_MOCK_DATA_PATH, lineId);
     Object[] expected = new Object[]{ POLineFieldNames.ACQUISITION_METHOD.getFieldName()};
 

diff --git a/src/test/java/org/folio/rest/impl/PurchaseOrdersApiTest.java b/src/test/java/org/folio/rest/impl/PurchaseOrdersApiTest.java
@@ -55,6 +55,9 @@
 import static org.folio.rest.core.exceptions.ErrorCodes.INACTIVE_EXPENSE_CLASS;
 import static org.folio.rest.core.exceptions.ErrorCodes.INCORRECT_FUND_DISTRIBUTION_TOTAL;
 import static org.folio.rest.core.exceptions.ErrorCodes.INSTANCE_ID_NOT_ALLOWED_FOR_PACKAGE_POLINE;
+import static org.folio.rest.core.exceptions.ErrorCodes.INVALID_OTHER_POL;
+import static org.folio.rest.core.exceptions.ErrorCodes.INVALID_PEMIX_POL;
+import static org.folio.rest.core.exceptions.ErrorCodes.INVALID_PHYSICAL_POL;
 import static org.folio.rest.core.exceptions.ErrorCodes.ISBN_NOT_VALID;
 import static org.folio.rest.core.exceptions.ErrorCodes.MISMATCH_BETWEEN_ID_IN_PATH_AND_BODY;
 import static org.folio.rest.core.exceptions.ErrorCodes.MISSING_MATERIAL_TYPE;
@@ -603,7 +606,7 @@ void testPostOrderWithIncorrectCost() throws Exception {
 
     final Errors response = verifyPostResponse(COMPOSITE_ORDERS_PATH, JsonObject.mapFrom(reqData).encode(),
       prepareHeaders(EXIST_CONFIG_X_OKAPI_TENANT_LIMIT_10), APPLICATION_JSON, 422).as(Errors.class);
-    assertThat(response.getErrors(), hasSize(10));
+    assertThat(response.getErrors(), hasSize(12));
     Set<String> errorCodes = response.getErrors()
                                      .stream()
                                      .map(Error::getCode)
@@ -615,7 +618,9 @@ void testPostOrderWithIncorrectCost() throws Exception {
                                               PHYSICAL_COST_LOC_QTY_MISMATCH.getCode(),
                                               ELECTRONIC_COST_LOC_QTY_MISMATCH.getCode(),
                                               COST_UNIT_PRICE_ELECTRONIC_INVALID.getCode(),
-                                              COST_UNIT_PRICE_INVALID.getCode()));
+                                              COST_UNIT_PRICE_INVALID.getCode(),
+                                              INVALID_OTHER_POL.getCode(),
+                                              INVALID_PEMIX_POL.getCode()));
   }
 
   @Test
@@ -729,7 +734,7 @@ void testPutOrderWithIncorrectQuantities() throws Exception {
 
       APPLICATION_JSON, 422).as(Errors.class);
 
-    assertThat(response.getErrors(), hasSize(5));
+    assertThat(response.getErrors(), hasSize(6));
     Set<String> errorCodes = response.getErrors()
                                      .stream()
                                      .map(Error::getCode)
@@ -739,7 +744,8 @@ void testPutOrderWithIncorrectQuantities() throws Exception {
                                               ZERO_COST_PHYSICAL_QTY.getCode(),
                                               ELECTRONIC_COST_LOC_QTY_MISMATCH.getCode(),
                                               PHYSICAL_COST_LOC_QTY_MISMATCH.getCode(),
-                                              ZERO_LOCATION_QTY.getCode()));
+                                              ZERO_LOCATION_QTY.getCode(),
+                                              INVALID_PEMIX_POL.getCode()));
   }
 
   @Test
@@ -2344,6 +2350,7 @@ void testPostOrdersCreateInventoryPhysicalNone() throws Exception {
     reqData.getCompositePoLines().get(0).getCost().setListUnitPriceElectronic(0d);
     reqData.getCompositePoLines().get(0).getLocations().get(0).setQuantityElectronic(0);
 
+
     verifyPostResponse(COMPOSITE_ORDERS_PATH, JsonObject.mapFrom(reqData).toString(),
       prepareHeaders(EXIST_CONFIG_X_OKAPI_TENANT_LIMIT_10, X_OKAPI_USER_ID), APPLICATION_JSON, 201).as(CompositePurchaseOrder.class);
 

diff --git a/src/test/java/org/folio/service/orders/CompositePoLineValidationServiceTest.java b/src/test/java/org/folio/service/orders/CompositePoLineValidationServiceTest.java
@@ -6,9 +6,12 @@
 import java.util.List;
 import java.util.UUID;
 
+import com.github.tomakehurst.wiremock.http.ssl.TrustEverythingStrategy;
+
 import org.folio.rest.core.exceptions.ErrorCodes;
 import org.folio.rest.jaxrs.model.CompositePoLine;
 import org.folio.rest.jaxrs.model.Cost;
+import org.folio.rest.jaxrs.model.Eresource;
 import org.folio.rest.jaxrs.model.Error;
 import org.folio.rest.jaxrs.model.Location;
 import org.folio.rest.jaxrs.model.Physical;
@@ -112,4 +115,83 @@ void shouldReturnErrorIfClaimingIntervalIsNegativeWhenClaimingActive() {
     assertEquals(1, errors.size());
     assertEquals(ErrorCodes.CLAIMING_CONFIG_INVALID.getCode(), errors.get(0).getCode());
   }
+
+
+  @Test
+  @DisplayName("Should return error if physical order format contains Electronic resources.")
+  void shouldReturnErrorIfPhysicalContainsElectronicSource() {
+    Eresource eresource = new Eresource();
+    CompositePoLine.OrderFormat format = CompositePoLine.OrderFormat.PHYSICAL_RESOURCE;
+    Cost cost = new Cost().withQuantityElectronic(1);
+    CompositePoLine compositePoLine = new CompositePoLine()
+      .withOrderFormat(format)
+      .withEresource(eresource)
+      .withCost(cost);
+    List<Error> errors = compositePoLineValidationService.validatePoLineMaterial(compositePoLine);
+
+      assertEquals(1, errors.size());
+      assertEquals(ErrorCodes.INVALID_PHYSICAL_POL.getCode(), errors.get(0).getCode());
+  }
+
+  @Test
+  @DisplayName("Should return error if electronic order format contains physical resources.")
+  void shouldReturnErrorIfElectronicContainsPhysicalSource() {
+    Physical physical = new Physical();
+    CompositePoLine.OrderFormat format = CompositePoLine.OrderFormat.ELECTRONIC_RESOURCE;
+    Cost cost = new Cost().withQuantityPhysical(1);
+    CompositePoLine compositePoLine = new CompositePoLine()
+      .withOrderFormat(format)
+      .withPhysical(physical)
+      .withCost(cost);
+    List<Error> errors = compositePoLineValidationService.validatePoLineMaterial(compositePoLine);
+
+    assertEquals(1, errors.size());
+    assertEquals(ErrorCodes.INVALID_ELECTRONIC_POL.getCode(), errors.get(0).getCode());
+  }
+
+  @Test
+  @DisplayName("Should return error if p/e mix order format does not contains physical resources.")
+  void shouldReturnErrorIfMixedDoesNotContainsPhysicalSource() {
+    Eresource eresource = new Eresource();
+    CompositePoLine.OrderFormat format = CompositePoLine.OrderFormat.P_E_MIX;
+    CompositePoLine compositePoLine = new CompositePoLine()
+      .withOrderFormat(format)
+      .withEresource(eresource);
+    List<Error> errors = compositePoLineValidationService.validatePoLineMaterial(compositePoLine);
+
+    assertEquals(1, errors.size());
+    assertEquals(ErrorCodes.INVALID_PEMIX_POL.getCode(), errors.get(0).getCode());
+  }
+
+  @Test
+  @DisplayName("Should return error if p/e mix order format does not contains electronic resources.")
+  void shouldReturnErrorIfMixedDoesNotContainsESource() {
+    Physical physical = new Physical();
+    CompositePoLine.OrderFormat format = CompositePoLine.OrderFormat.P_E_MIX;
+    CompositePoLine compositePoLine = new CompositePoLine()
+      .withOrderFormat(format)
+      .withPhysical(physical);
+    List<Error> errors = compositePoLineValidationService.validatePoLineMaterial(compositePoLine);
+
+    assertEquals(1, errors.size());
+    assertEquals(ErrorCodes.INVALID_PEMIX_POL.getCode(), errors.get(0).getCode());
+  }
+
+  @Test
+  @DisplayName("Should return error if other order format contains Electronic resources.")
+  void shouldReturnErrorIfOtherContainsElectronicSource() {
+    Eresource eresource = new Eresource();
+    CompositePoLine.OrderFormat format = CompositePoLine.OrderFormat.OTHER;
+    Cost cost = new Cost().withQuantityElectronic(1);
+    CompositePoLine compositePoLine = new CompositePoLine()
+      .withOrderFormat(format)
+      .withEresource(eresource)
+      .withCost(cost);
+    List<Error> errors = compositePoLineValidationService.validatePoLineMaterial(compositePoLine);
+
+    assertEquals(1, errors.size());
+    assertEquals(ErrorCodes.INVALID_OTHER_POL.getCode(), errors.get(0).getCode());
+  }
+
+
 }