FINERACT-1971

- validation added for the case when advanced repayment strategy is used with empty or missing allocation rules.

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsJsonParser.java

@@ -39,8 +39,9 @@ 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);
@@ -48,10 +49,9 @@ public class AdvancedPaymentAllocationsJsonParser {
                 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,

fineract-loan/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsValidator.java

@@ -73,7 +73,8 @@ public class AdvancedPaymentAllocationsValidator {
     }
 
     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 @@ public class AdvancedPaymentAllocationsValidator {
     }
 
     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) {

fineract-loan/src/test/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsJsonParserTest.java

@@ -68,7 +68,21 @@ class AdvancedPaymentAllocationsJsonParserTest {
 
         // 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

fineract-loan/src/test/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsValidatorTest.java

@@ -80,6 +80,22 @@ class AdvancedPaymentAllocationsValidatorTest {
         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();

integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanProductWithAdvancedPaymentAllocationIntegrationTests.java

@@ -133,7 +133,7 @@ public class LoanProductWithAdvancedPaymentAllocationIntegrationTests {
 
     @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 class LoanProductWithAdvancedPaymentAllocationIntegrationTests {
 
         // 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 class LoanProductWithAdvancedPaymentAllocationIntegrationTests {
                 .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 class LoanProductWithAdvancedPaymentAllocationIntegrationTests {
     }
 
     private String createLoanJSON(AdvancedPaymentData... advancedPaymentData) {
-        final String loanProductJSON = new LoanProductTestBuilder().withPrincipal("15,000.00").withNumberOfRepayments("4")
+        return new LoanProductTestBuilder().withPrincipal("15,000.00").withNumberOfRepayments("4").withRepaymentAfterEvery("1")
-                .withRepaymentAfterEvery("1").withRepaymentTypeAsMonth().withinterestRatePerPeriod("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) {