FINERACT-1971

- validation added for the case when advanced repayment strategy is used with empty or missing allocation rules.
apache · Sep 13, 2023 · 09be618 · 09be618
1 parent 6d3357e
commit 09be618
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 12 deletions.
diff --git a/...rg/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsJsonParser.java b/...rg/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsJsonParser.java
@@ -39,19 +39,19 @@ public class AdvancedPaymentAllocationsJsonParser {
     public List<LoanProductPaymentAllocationRule> assembleLoanProductPaymentAllocationRules(final JsonCommand command,
             String loanTransactionProcessingStrategyCode) {
         JsonArray paymentAllocations = command.arrayOfParameterNamed("paymentAllocation");
+        List<LoanProductPaymentAllocationRule> productPaymentAllocationRules = null;
         if (paymentAllocations != null) {
-            List<LoanProductPaymentAllocationRule> productPaymentAllocationRules = paymentAllocations.asList().stream().map(json -> {
+            productPaymentAllocationRules = paymentAllocations.asList().stream().map(json -> {
                 Map<String, JsonElement> map = json.getAsJsonObject().asMap();
                 LoanProductPaymentAllocationRule loanProductPaymentAllocationRule = new LoanProductPaymentAllocationRule();
                 populatePaymentAllocationRules(map, loanProductPaymentAllocationRule);
                 populateFutureInstallment(map, loanProductPaymentAllocationRule);
                 populateTransactionType(map, loanProductPaymentAllocationRule);
                 return loanProductPaymentAllocationRule;
             }).toList();
-            advancedPaymentAllocationsValidator.validate(productPaymentAllocationRules, loanTransactionProcessingStrategyCode);
-            return productPaymentAllocationRules;
         }
-        return null;
+        advancedPaymentAllocationsValidator.validate(productPaymentAllocationRules, loanTransactionProcessingStrategyCode);
+        return productPaymentAllocationRules;
     }
 
     private void populatePaymentAllocationRules(Map<String, JsonElement> map,

diff --git a/...org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsValidator.java b/...org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsValidator.java
@@ -73,7 +73,8 @@ public void validatePairOfOrderAndPaymentAllocationType(List<Pair<Integer, Payme
     }
 
     private boolean hasDuplicateTransactionTypes(List<LoanProductPaymentAllocationRule> rules) {
-        return rules.stream().map(LoanProductPaymentAllocationRule::getTransactionType).distinct().toList().size() != rules.size();
+        return rules != null
+                && rules.stream().map(LoanProductPaymentAllocationRule::getTransactionType).distinct().toList().size() != rules.size();
     }
 
     private void validateAllocationRule(LoanProductPaymentAllocationRule rule) {
@@ -92,14 +93,14 @@ private boolean isAdvancedPaymentStrategy(String code) {
     }
 
     private boolean hasAtLeastOneDefaultPaymentAllocation(List<LoanProductPaymentAllocationRule> rules) {
-        return rules.stream() //
+        return rules != null && rules.stream() //
                 .filter(r -> PaymentAllocationTransactionType.DEFAULT.equals(r.getTransactionType())) //
                 .toList() //
                 .size() > 0;
     }
 
     private boolean hasLoanProductPaymentAllocationRule(List<LoanProductPaymentAllocationRule> rules) {
-        return rules.size() > 0;
+        return rules != null && rules.size() > 0;
     }
 
     private void raiseValidationError(String globalisationMessageCode, String msg) {

diff --git a/...pache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsJsonParserTest.java b/...pache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsJsonParserTest.java
@@ -68,7 +68,21 @@ public void testEmptyJson() throws JsonProcessingException {
 
         // then
         Assertions.assertNull(loanProductPaymentAllocationRules);
-        Mockito.verifyNoInteractions(advancedPaymentAllocationsValidator);
+        Mockito.verify(advancedPaymentAllocationsValidator, times(1)).validate(null, "other-strategy");
+    }
+
+    @Test
+    public void testNullAllocationRuleJson() throws JsonProcessingException {
+        Map<String, Object> map = new HashMap<>();
+        JsonCommand command = createJsonCommand(map);
+
+        // when
+        List<LoanProductPaymentAllocationRule> loanProductPaymentAllocationRules = advancedPaymentAllocationsJsonParser
+                .assembleLoanProductPaymentAllocationRules(command, "advanced-payment-allocation-strategy");
+
+        // then
+        Assertions.assertNull(loanProductPaymentAllocationRules);
+        Mockito.verify(advancedPaymentAllocationsValidator, times(1)).validate(null, "advanced-payment-allocation-strategy");
     }
 
     @Test

diff --git a/...apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsValidatorTest.java b/...apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsValidatorTest.java
@@ -80,6 +80,22 @@ public void testValidateNoError() {
         underTest.validate(List.of(lppr1, lppr2), ADVANCED_PAYMENT_ALLOCATION_STRATEGY);
     }
 
+    @Test
+    public void testValidateEmptyRuleList() {
+        assertPlatformValidationException(
+                "Advanced-payment-allocation-strategy was selected but no DEFAULT payment allocation was provided",
+                "advanced-payment-strategy-without-default-payment-allocation",
+                () -> underTest.validate(List.of(), ADVANCED_PAYMENT_ALLOCATION_STRATEGY));
+    }
+
+    @Test
+    public void testValidateMissingList() {
+        assertPlatformValidationException(
+                "Advanced-payment-allocation-strategy was selected but no DEFAULT payment allocation was provided",
+                "advanced-payment-strategy-without-default-payment-allocation",
+                () -> underTest.validate(null, ADVANCED_PAYMENT_ALLOCATION_STRATEGY));
+    }
+
     @Test
     public void testValidatePaymentAllocationThrowsErrorWhenNoDefault() {
         LoanProductPaymentAllocationRule lppr2 = createLoanProductAllocationRule2();

diff --git a/...e/fineract/integrationtests/LoanProductWithAdvancedPaymentAllocationIntegrationTests.java b/...e/fineract/integrationtests/LoanProductWithAdvancedPaymentAllocationIntegrationTests.java
@@ -133,7 +133,7 @@ public void testUpdateLoanProductOneAllocationIsRemoved() {
 
     @Test
     public void testUpdateLoanProductOneAllocationIsAdded() {
-        // given a loan with one allocations
+        // given a loan with one allocation
         AdvancedPaymentData defaultAllocation = createDefaultPaymentAllocation();
         AdvancedPaymentData repaymentPaymentAllocation = createRepaymentPaymentAllocation();
         Integer loanProductId = LOAN_TRANSACTION_HELPER.getLoanProductId(createLoanJSON(defaultAllocation));
@@ -148,6 +148,7 @@ public void testUpdateLoanProductOneAllocationIsAdded() {
 
         // then it shall be added.
         loanProduct = LOAN_TRANSACTION_HELPER.getLoanProduct(loanProductId);
+        Assertions.assertNotNull(loanProduct.getPaymentAllocation());
         Assertions.assertEquals(2, loanProduct.getPaymentAllocation().size());
         Optional<AdvancedPaymentData> first = loanProduct.getPaymentAllocation().stream()
                 .filter(advancedPaymentData -> "DEFAULT".equals(advancedPaymentData.getTransactionType())).findFirst();
@@ -200,6 +201,24 @@ public void testUpdateShouldFailWhenStrategyIsChangedBackButPaymentAllocationsAr
                 .contains("In case 'mifos-standard-strategy' payment strategy, payment_allocation must not be provided"));
     }
 
+    @Test
+    public void testCreateShouldFailWhenNoAllocationRuleIsProvided() {
+        // given
+        ResponseSpecification errorResponse = new ResponseSpecBuilder().expectStatusCode(400).build();
+        LoanTransactionHelper validationErrorHelper = new LoanTransactionHelper(REQUEST_SPEC, errorResponse);
+
+        String loanProduct = new LoanProductTestBuilder().withPrincipal("15,000.00").withNumberOfRepayments("4")
+                .withRepaymentAfterEvery("1").withRepaymentTypeAsMonth().withinterestRatePerPeriod("1")
+                .withAccountingRulePeriodicAccrual(new Account[] { ASSET_ACCOUNT, EXPENSE_ACCOUNT, INCOME_ACCOUNT, OVERPAYMENT_ACCOUNT })
+                .withInterestRateFrequencyTypeAsMonths().withAmortizationTypeAsEqualInstallments().withInterestTypeAsDecliningBalance()
+                .withFeeAndPenaltyAssetAccount(FEE_PENALTY_ACCOUNT).withRepaymentStrategy("advanced-payment-allocation-strategy").build();
+
+        // when
+        List<Map<String, String>> loanProductError = validationErrorHelper.getLoanProductError(loanProduct, "errors");
+        Assertions.assertEquals("Advanced-payment-allocation-strategy was selected but no DEFAULT payment allocation was provided",
+                loanProductError.get(0).get("defaultUserMessage"));
+    }
+
     @Test
     public void testCreateShouldFailWhenNoDefaultAllocationIsProvided() {
         // given
@@ -215,12 +234,11 @@ public void testCreateShouldFailWhenNoDefaultAllocationIsProvided() {
     }
 
     private String createLoanJSON(AdvancedPaymentData... advancedPaymentData) {
-        final String loanProductJSON = new LoanProductTestBuilder().withPrincipal("15,000.00").withNumberOfRepayments("4")
-                .withRepaymentAfterEvery("1").withRepaymentTypeAsMonth().withinterestRatePerPeriod("1")
+        return new LoanProductTestBuilder().withPrincipal("15,000.00").withNumberOfRepayments("4").withRepaymentAfterEvery("1")
+                .withRepaymentTypeAsMonth().withinterestRatePerPeriod("1")
                 .withAccountingRulePeriodicAccrual(new Account[] { ASSET_ACCOUNT, EXPENSE_ACCOUNT, INCOME_ACCOUNT, OVERPAYMENT_ACCOUNT })
                 .withInterestRateFrequencyTypeAsMonths().withAmortizationTypeAsEqualInstallments().withInterestTypeAsDecliningBalance()
                 .withFeeAndPenaltyAssetAccount(FEE_PENALTY_ACCOUNT).addAdvancedPaymentAllocation(advancedPaymentData).build();
-        return loanProductJSON;
     }
 
     private PutLoanProductsProductIdRequest updateLoanProductRequest(AdvancedPaymentData... advancedPaymentData) {