From 95475509ded52e3c669cd13b8f4e1b68789977a0 Mon Sep 17 00:00:00 2001 From: Peter Bagrij Date: Wed, 13 Sep 2023 13:35:13 +0200 Subject: [PATCH] FINERACT-1971 - validation added for the case when advanced repayment strategy is used with empty or missing allocation rules. --- .../AdvancedPaymentAllocationsJsonParser.java | 8 +++--- .../AdvancedPaymentAllocationsValidator.java | 7 ++--- ...ancedPaymentAllocationsJsonParserTest.java | 16 +++++++++++- ...vancedPaymentAllocationsValidatorTest.java | 16 ++++++++++++ ...ncedPaymentAllocationIntegrationTests.java | 26 ++++++++++++++++--- 5 files changed, 61 insertions(+), 12 deletions(-) diff --git a/fineract-loan/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsJsonParser.java b/fineract-loan/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsJsonParser.java index 643679528..b5aff2293 100644 --- a/fineract-loan/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsJsonParser.java +++ b/fineract-loan/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsJsonParser.java @@ -39,8 +39,9 @@ public class AdvancedPaymentAllocationsJsonParser { public List assembleLoanProductPaymentAllocationRules(final JsonCommand command, String loanTransactionProcessingStrategyCode) { JsonArray paymentAllocations = command.arrayOfParameterNamed("paymentAllocation"); + List productPaymentAllocationRules = null; if (paymentAllocations != null) { - List productPaymentAllocationRules = paymentAllocations.asList().stream().map(json -> { + productPaymentAllocationRules = paymentAllocations.asList().stream().map(json -> { Map 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 map, diff --git a/fineract-loan/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsValidator.java b/fineract-loan/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsValidator.java index 81361cff3..858eac5fb 100644 --- a/fineract-loan/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsValidator.java +++ b/fineract-loan/src/main/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsValidator.java @@ -73,7 +73,8 @@ public class AdvancedPaymentAllocationsValidator { } private boolean hasDuplicateTransactionTypes(List 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 rules) { - return rules.stream() // + return rules != null && rules.stream() // .filter(r -> PaymentAllocationTransactionType.DEFAULT.equals(r.getTransactionType())) // .toList() // .size() > 0; } private boolean hasLoanProductPaymentAllocationRule(List rules) { - return rules.size() > 0; + return rules != null && rules.size() > 0; } private void raiseValidationError(String globalisationMessageCode, String msg) { diff --git a/fineract-loan/src/test/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsJsonParserTest.java b/fineract-loan/src/test/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsJsonParserTest.java index 113a98565..85b30ee95 100644 --- a/fineract-loan/src/test/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsJsonParserTest.java +++ b/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 map = new HashMap<>(); + JsonCommand command = createJsonCommand(map); + + // when + List 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/fineract-loan/src/test/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsValidatorTest.java b/fineract-loan/src/test/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsValidatorTest.java index afba9f6d7..b3a19d559 100644 --- a/fineract-loan/src/test/java/org/apache/fineract/portfolio/loanproduct/domain/AdvancedPaymentAllocationsValidatorTest.java +++ b/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(); diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanProductWithAdvancedPaymentAllocationIntegrationTests.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanProductWithAdvancedPaymentAllocationIntegrationTests.java index c6fbbcc4b..a899456c7 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanProductWithAdvancedPaymentAllocationIntegrationTests.java +++ b/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 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> 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") - .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) {