From cc3a492d5788282ac6739209b86418e150ab40c2 Mon Sep 17 00:00:00 2001 From: Jose Alberto Hernandez Date: Fri, 30 Aug 2024 14:16:57 -0500 Subject: [PATCH] FINERACT-2081: Validation error in mandatory loan account parameters --- .../core/data/DataValidatorBuilder.java | 8 ++ .../test/stepdef/loan/LoanStepDef.java | 4 +- .../LoanApplicationValidator.java | 113 ++++++++++++------ .../LoanProductDataValidator.java | 1 - .../LoanApplicationApprovalTest.java | 5 +- .../LoanOriginationValidationTest.java | 90 ++++++++++++++ 6 files changed, 178 insertions(+), 43 deletions(-) diff --git a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/data/DataValidatorBuilder.java b/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/data/DataValidatorBuilder.java index 6a718e16e62..4a6bc70505f 100644 --- a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/data/DataValidatorBuilder.java +++ b/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/data/DataValidatorBuilder.java @@ -1056,4 +1056,12 @@ public void throwValidationErrors() throws PlatformApiDataValidationException { throw new PlatformApiDataValidationException(dataValidationErrors); } } + + public static ApiParameterError buildValidationParameterApiError(final String resource, final String parameterName, + final String errorCode, final String errorMessage, final Object... defaultUserMessageArgs) { + final String validationErrorCode = "validation.msg." + resource + "." + parameterName + errorCode; + String defaultEnglishMessage = "The parameter `" + parameterName + "` " + errorMessage; + return ApiParameterError.parameterError(validationErrorCode, defaultEnglishMessage, parameterName, defaultUserMessageArgs); + } + } diff --git a/fineract-e2e-tests-core/src/test/java/org/apache/fineract/test/stepdef/loan/LoanStepDef.java b/fineract-e2e-tests-core/src/test/java/org/apache/fineract/test/stepdef/loan/LoanStepDef.java index 24cbeeb951f..9b36c382826 100644 --- a/fineract-e2e-tests-core/src/test/java/org/apache/fineract/test/stepdef/loan/LoanStepDef.java +++ b/fineract-e2e-tests-core/src/test/java/org/apache/fineract/test/stepdef/loan/LoanStepDef.java @@ -513,8 +513,8 @@ public void createFullyCustomizedLoanFixedLength(int fixedLength, DataTable tabl @When("Admin creates a fully customized loan with Advanced payment allocation and with product no Advanced payment allocation set results an error:") public void createFullyCustomizedLoanNoAdvancedPaymentError(DataTable table) throws IOException { - int errorCodeExpected = 400; - String errorMessageExpected = "Failed data validation due to: strategy.cannot.be.advanced.payment.allocation.if.not.configured."; + int errorCodeExpected = 403; + String errorMessageExpected = "Loan transaction processing strategy cannot be Advanced Payment Allocation Strategy if it's not configured on loan product"; List> data = table.asLists(); List loanData = data.get(1); diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanApplicationValidator.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanApplicationValidator.java index 688858b434f..f229571383a 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanApplicationValidator.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanApplicationValidator.java @@ -248,6 +248,9 @@ private void validateForCreate(final JsonElement element) { boolean isMeetingMandatoryForJLGLoans = configurationDomainService.isMeetingMandatoryForJLGLoans(); final Long productId = this.fromApiJsonHelper.extractLongNamed(LoanApiConstants.productIdParameterName, element); + if (productId == null) { + throwMandatoryParameterError(LoanApiConstants.productIdParameterName); + } final LoanProduct loanProduct = this.loanProductRepository.findById(productId) .orElseThrow(() -> new LoanProductNotFoundException(productId)); @@ -257,7 +260,6 @@ private void validateForCreate(final JsonElement element) { final Group group = groupId != null ? this.groupRepository.findOneWithNotFoundDetection(groupId) : null; validateClientOrGroup(client, group, productId); - validateOrThrow("loan", baseDataValidator -> { final String loanTypeStr = this.fromApiJsonHelper.extractStringNamed(LoanApiConstants.loanTypeParameterName, element); baseDataValidator.reset().parameter(LoanApiConstants.loanTypeParameterName).value(loanTypeStr).notNull(); @@ -394,7 +396,6 @@ private void validateForCreate(final JsonElement element) { baseDataValidator.reset().parameter(LoanApiConstants.interestCalculationPeriodTypeParameterName) .value(interestCalculationPeriodType).notNull().inMinMaxRange(0, 1); - boolean isInterestBearing = false; if (loanProduct.isLinkedToFloatingInterestRate()) { if (isEqualAmortization) { throw new EqualAmortizationUnsupportedFeatureException("floating.interest.rate", "floating interest rate"); @@ -430,7 +431,6 @@ private void validateForCreate(final JsonElement element) { baseDataValidator.reset().parameter(interestRateDifferentialParameterName).value(interestRateDifferential).notNull() .zeroOrPositiveAmount().inMinAndMaxAmountRange(loanProduct.getFloatingRates().getMinDifferentialLendingRate(), loanProduct.getFloatingRates().getMaxDifferentialLendingRate()); - isInterestBearing = true; } else { if (this.fromApiJsonHelper.parameterExists(LoanApiConstants.isFloatingInterestRate, element)) { @@ -448,7 +448,6 @@ private void validateForCreate(final JsonElement element) { .extractBigDecimalWithLocaleNamed(LoanApiConstants.interestRatePerPeriodParameterName, element); baseDataValidator.reset().parameter(LoanApiConstants.interestRatePerPeriodParameterName).value(interestRatePerPeriod) .notNull().zeroOrPositiveAmount(); - isInterestBearing = interestRatePerPeriod != null && interestRatePerPeriod.compareTo(BigDecimal.ZERO) > 0; } final Integer amortizationType = this.fromApiJsonHelper @@ -511,7 +510,6 @@ private void validateForCreate(final JsonElement element) { final LocalDate submittedOnDate = this.fromApiJsonHelper.extractLocalDateNamed(LoanApiConstants.submittedOnDateParameterName, element); - baseDataValidator.reset().parameter(LoanApiConstants.submittedOnDateParameterName).value(submittedOnDate).notNull(); if (this.fromApiJsonHelper.parameterExists(LoanApiConstants.submittedOnNoteParameterName, element)) { @@ -523,8 +521,8 @@ private void validateForCreate(final JsonElement element) { final String transactionProcessingStrategy = this.fromApiJsonHelper .extractStringNamed(LoanApiConstants.transactionProcessingStrategyCodeParameterName, element); - - validateTransactionProcessingStrategy(transactionProcessingStrategy, loanProduct, baseDataValidator); + baseDataValidator.reset().parameter(LoanApiConstants.transactionProcessingStrategyCodeParameterName) + .value(transactionProcessingStrategy).notNull(); validateLinkedSavingsAccount(element, baseDataValidator); @@ -727,9 +725,6 @@ private void validateForCreate(final JsonElement element) { validateBorrowerCycle(element, loanProduct, clientId, groupId, baseDataValidator); - loanProductDataValidator.fixedLengthValidations(transactionProcessingStrategy, isInterestBearing, numberOfRepayments, - repaymentEvery, element, baseDataValidator); - // Validate If the externalId is already registered final String externalIdStr = this.fromApiJsonHelper.extractStringNamed("externalId", element); ExternalId externalId = ExternalIdFactory.produce(externalIdStr); @@ -744,12 +739,11 @@ private void validateForCreate(final JsonElement element) { loanScheduleValidator.validateDownPaymentAttribute(loanProduct.getLoanProductRelatedDetail().isEnableDownPayment(), element); checkForProductMixRestrictions(element); - validateSubmittedOnDate(element, null, null, loanProduct); validateDisbursementDetails(loanProduct, element); validateCollateral(element); // validate if disbursement date is a holiday or a non-working day validateDisbursementDateIsOnNonWorkingDay(expectedDisbursementDate); - Long officeId = client != null ? client.getOffice().getId() : group.getOffice().getId(); + Long officeId = resolveOfficeId(client, group); validateDisbursementDateIsOnHoliday(expectedDisbursementDate, officeId); final Integer recurringMoratoriumOnPrincipalPeriods = this.fromApiJsonHelper .extractIntegerWithLocaleNamed("recurringMoratoriumOnPrincipalPeriods", element); @@ -759,6 +753,32 @@ private void validateForCreate(final JsonElement element) { graceOnInterestPayment, graceOnInterestCharged, recurringMoratoriumOnPrincipalPeriods, baseDataValidator); } }); + + validateSubmittedOnDate(element, null, null, loanProduct); + + final String transactionProcessingStrategy = this.fromApiJsonHelper + .extractStringNamed(LoanApiConstants.transactionProcessingStrategyCodeParameterName, element); + validateTransactionProcessingStrategy(transactionProcessingStrategy, loanProduct); + + fixedLengthValidations(element); + } + + private void fixedLengthValidations(final JsonElement element) { + validateOrThrow("loan", baseDataValidator -> { + boolean isInterestBearing = false; + final String transactionProcessingStrategy = this.fromApiJsonHelper + .extractStringNamed(LoanApiConstants.transactionProcessingStrategyCodeParameterName, element); + final Integer numberOfRepayments = this.fromApiJsonHelper + .extractIntegerWithLocaleNamed(LoanApiConstants.numberOfRepaymentsParameterName, element); + final Integer repaymentEvery = this.fromApiJsonHelper + .extractIntegerWithLocaleNamed(LoanApiConstants.repaymentEveryParameterName, element); + + final BigDecimal interestRatePerPeriod = this.fromApiJsonHelper + .extractBigDecimalWithLocaleNamed(LoanApiConstants.interestRatePerPeriodParameterName, element); + isInterestBearing = interestRatePerPeriod != null && interestRatePerPeriod.compareTo(BigDecimal.ZERO) > 0; + loanProductDataValidator.fixedLengthValidations(transactionProcessingStrategy, isInterestBearing, numberOfRepayments, + repaymentEvery, element, baseDataValidator); + }); } private void validateBorrowerCycle(JsonElement element, LoanProduct loanProduct, Long clientId, Long groupId, @@ -928,7 +948,7 @@ public void validateForModify(final JsonCommand command, final Loan loan) { baseDataValidator.reset().parameter(LoanApiConstants.transactionProcessingStrategyCodeParameterName) .value(transactionProcessingStrategy).notNull(); // Validating whether the processor is existing - validateTransactionProcessingStrategy(transactionProcessingStrategy, loanProduct, baseDataValidator); + validateTransactionProcessingStrategy(transactionProcessingStrategy, loanProduct); } if (!AdvancedPaymentScheduleTransactionProcessor.ADVANCED_PAYMENT_ALLOCATION_STRATEGY @@ -1410,7 +1430,7 @@ public void validateForModify(final JsonCommand command, final Loan loan) { // validate if disbursement date is a holiday or a non-working day validateDisbursementDateIsOnNonWorkingDay(expectedDisbursementDate); - Long officeId = client != null ? client.getOffice().getId() : group.getOffice().getId(); + final Long officeId = resolveOfficeId(client, group); validateDisbursementDateIsOnHoliday(expectedDisbursementDate, officeId); Integer recurringMoratoriumOnPrincipalPeriods = loan.getLoanProductRelatedDetail().getRecurringMoratoriumOnPrincipalPeriods(); @@ -1426,24 +1446,30 @@ public void validateForModify(final JsonCommand command, final Loan loan) { } private void validateClientOrGroup(Client client, Group group, Long productId) { - if (client != null) { - officeSpecificLoanProductValidation(productId, client.getOffice().getId()); - if (client.isNotActive()) { - throw new ClientNotActiveException(client.getId()); - } - } - if (group != null) { - officeSpecificLoanProductValidation(productId, group.getOffice().getId()); - if (group.isNotActive()) { - throw new GroupNotActiveException(group.getId()); - } - } + validateOrThrow("loan", baseDataValidator -> { + if (client == null && group == null) { + baseDataValidator.reset().parameter(LoanApiConstants.clientIdParameterName).value(client).notNull(); + } else { + if (client != null) { + officeSpecificLoanProductValidation(productId, client.getOffice().getId()); + if (client.isNotActive()) { + throw new ClientNotActiveException(client.getId()); + } + } + if (group != null) { + officeSpecificLoanProductValidation(productId, group.getOffice().getId()); + if (group.isNotActive()) { + throw new GroupNotActiveException(group.getId()); + } + } - if (client != null && group != null) { - if (!group.hasClientAsMember(client)) { - throw new ClientNotInGroupException(client.getId(), group.getId()); + if (client != null && group != null) { + if (!group.hasClientAsMember(client)) { + throw new ClientNotInGroupException(client.getId(), group.getId()); + } + } } - } + }); } private void validateDisbursementDetails(LoanProduct loanProduct, JsonElement element) { @@ -1747,18 +1773,13 @@ private void officeSpecificLoanProductValidation(final Long productId, final Lon } } - private void validateTransactionProcessingStrategy(final String transactionProcessingStrategy, final LoanProduct loanProduct, - final DataValidatorBuilder baseDataValidator) { - - baseDataValidator.reset().parameter(LoanApiConstants.transactionProcessingStrategyCodeParameterName) - .value(transactionProcessingStrategy).notNull(); + private void validateTransactionProcessingStrategy(final String transactionProcessingStrategy, final LoanProduct loanProduct) { // TODO: Review exceptions if (!AdvancedPaymentScheduleTransactionProcessor.ADVANCED_PAYMENT_ALLOCATION_STRATEGY .equals(loanProduct.getTransactionProcessingStrategyCode()) && AdvancedPaymentScheduleTransactionProcessor.ADVANCED_PAYMENT_ALLOCATION_STRATEGY.equals(transactionProcessingStrategy)) { - baseDataValidator.reset().parameter(LoanApiConstants.transactionProcessingStrategyCodeParameterName).failWithCode( - "strategy.cannot.be.advanced.payment.allocation.if.not.configured", + throw new GeneralPlatformDomainRuleException("strategy.cannot.be.advanced.payment.allocation.if.not.configured", "Loan transaction processing strategy cannot be Advanced Payment Allocation Strategy if it's not configured on loan product"); } else { // PROGRESSIVE: Repayment strategy MUST be only "advanced payment allocation" @@ -2127,4 +2148,22 @@ public static void validateOrThrow(String resource, Consumer dataValidationErrors = new ArrayList<>(); + dataValidationErrors + .add(DataValidatorBuilder.buildValidationParameterApiError("loans", parameterName, ".cannot.be.blank", "is mandatory.", 0)); + throw new PlatformApiDataValidationException("validation.msg.validation.errors.exist", "Validation errors exist.", + dataValidationErrors); + } } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/serialization/LoanProductDataValidator.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/serialization/LoanProductDataValidator.java index 06b330f6e9d..3758e4e20ab 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/serialization/LoanProductDataValidator.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/serialization/LoanProductDataValidator.java @@ -574,7 +574,6 @@ public void validateForCreate(final JsonCommand command) { } // Fixed Length validation - fixedLengthValidations(transactionProcessingStrategyCode, isInterestBearing, numberOfRepayments, repaymentEvery, element, baseDataValidator); diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanApplicationApprovalTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanApplicationApprovalTest.java index 0f24bdf647b..b8035abeb4c 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanApplicationApprovalTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanApplicationApprovalTest.java @@ -160,7 +160,7 @@ public void loanApplicationShouldFailIfTransactionProcessingStrategyIsAdvancedPa log.info("----------------------------------LOAN PRODUCT CREATED WITH ID------------------------------------------- {}", loanProductId); - loanTransactionHelper = new LoanTransactionHelper(this.requestSpec, this.responseSpecForStatusCode400); + loanTransactionHelper = new LoanTransactionHelper(this.requestSpec, this.responseSpecForStatusCode403); final String loanApplicationJSON = new LoanApplicationTestBuilder().withPrincipal("1000").withLoanTermFrequency("1") .withLoanTermFrequencyAsMonths().withNumberOfRepayments("1").withRepaymentEveryAfter("1") .withRepaymentFrequencyTypeAsMonths().withInterestRatePerPeriod("0").withInterestTypeAsFlatBalance() @@ -169,8 +169,7 @@ public void loanApplicationShouldFailIfTransactionProcessingStrategyIsAdvancedPa .withRepaymentStrategy(AdvancedPaymentScheduleTransactionProcessor.ADVANCED_PAYMENT_ALLOCATION_STRATEGY) .build(clientId.toString(), loanProductId.toString(), null); List error = (List) loanTransactionHelper.createLoanAccount(loanApplicationJSON, CommonConstants.RESPONSE_ERROR); - assertEquals( - "validation.msg.loan.transactionProcessingStrategyCode.strategy.cannot.be.advanced.payment.allocation.if.not.configured", + assertEquals("strategy.cannot.be.advanced.payment.allocation.if.not.configured", error.get(0).get(CommonConstants.RESPONSE_ERROR_MESSAGE_CODE)); } diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanOriginationValidationTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanOriginationValidationTest.java index 62e7f1bb3b4..63ac43f35cb 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanOriginationValidationTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanOriginationValidationTest.java @@ -18,6 +18,8 @@ */ package org.apache.fineract.integrationtests; +import static org.junit.jupiter.api.Assertions.assertEquals; + import io.restassured.builder.RequestSpecBuilder; import io.restassured.builder.ResponseSpecBuilder; import io.restassured.http.ContentType; @@ -172,6 +174,94 @@ public void uc2() { }); } + // uc3: Negative Test: Loan application without required parameters + // 1. Create a Loan product + // 2. Submit Loan application without required parameters + @Test + public void uc3() { + String operationDate = "15 August 2024"; + runAt(operationDate, () -> { + + LOG.info("------------------------------CREATING NEW LOAN PRODUCT ---------------------------------------"); + PostLoanProductsResponse loanProductResponse = loanProductHelper + .createLoanProduct(createOnePeriod30DaysLongNoInterestPeriodicAccrualProductWithAdvancedPaymentAllocation() + .loanScheduleType(LoanScheduleType.PROGRESSIVE.toString())); + // Product Id null + final PostLoansRequest applicationRequest01 = applyLoanRequest(client.getClientId(), null, operationDate, 100.0, 5) + .numberOfRepayments(6)// + .loanTermFrequency(6)// + .loanTermFrequencyType(2)// + .repaymentEvery(1)// + .repaymentFrequencyType(2)// + ;// + CallFailedRuntimeException callFailedRuntimeException = Assertions.assertThrows(CallFailedRuntimeException.class, + () -> loanTransactionHelper.applyLoan(applicationRequest01)); + assertEquals(400, callFailedRuntimeException.getResponse().code()); + Assertions.assertTrue(callFailedRuntimeException.getMessage().contains("The parameter `productId` is mandatory.")); + + // Transaction Processing Strategy Code null + final PostLoansRequest applicationRequest02 = applyLoanRequest(client.getClientId(), loanProductResponse.getResourceId(), + operationDate, 100.0, 5).numberOfRepayments(6)// + .loanTermFrequency(6)// + .loanTermFrequencyType(2)// + .transactionProcessingStrategyCode(null)// + .repaymentEvery(1)// + .repaymentFrequencyType(2)// + ;// + + callFailedRuntimeException = Assertions.assertThrows(CallFailedRuntimeException.class, + () -> loanTransactionHelper.applyLoan(applicationRequest02)); + assertEquals(400, callFailedRuntimeException.getResponse().code()); + Assertions.assertTrue( + callFailedRuntimeException.getMessage().contains("The parameter `transactionProcessingStrategyCode` is mandatory.")); + + final PostLoansRequest applicationRequest03 = applyLoanRequest(null, loanProductResponse.getResourceId(), operationDate, 100.0, + 5).numberOfRepayments(6)// + .loanTermFrequency(6)// + .loanTermFrequencyType(2)// + .transactionProcessingStrategyCode(LoanProductTestBuilder.ADVANCED_PAYMENT_ALLOCATION_STRATEGY)// + .repaymentEvery(1)// + .repaymentFrequencyType(2)// + ;// + callFailedRuntimeException = Assertions.assertThrows(CallFailedRuntimeException.class, + () -> loanTransactionHelper.applyLoan(applicationRequest03)); + LOG.info("DETAIL: {}", callFailedRuntimeException.getMessage()); + assertEquals(400, callFailedRuntimeException.getResponse().code()); + Assertions.assertTrue(callFailedRuntimeException.getMessage().contains("The parameter `clientId` is mandatory.")); + + // Submitted Date null + final PostLoansRequest applicationRequest04 = applyLoanRequest(client.getClientId(), loanProductResponse.getResourceId(), + operationDate, 100.0, 5).numberOfRepayments(6)// + .submittedOnDate(null) // + .loanTermFrequency(6)// + .loanTermFrequencyType(2)// + .transactionProcessingStrategyCode(LoanProductTestBuilder.ADVANCED_PAYMENT_ALLOCATION_STRATEGY)// + .repaymentEvery(1)// + .repaymentFrequencyType(2)// + ;// + callFailedRuntimeException = Assertions.assertThrows(CallFailedRuntimeException.class, + () -> loanTransactionHelper.applyLoan(applicationRequest04)); + assertEquals(400, callFailedRuntimeException.getResponse().code()); + Assertions.assertTrue(callFailedRuntimeException.getMessage().contains("The parameter `submittedOnDate` is mandatory.")); + + // Expected disbursement Date null + final PostLoansRequest applicationRequest05 = applyLoanRequest(client.getClientId(), loanProductResponse.getResourceId(), + operationDate, 100.0, 5).numberOfRepayments(6)// + .expectedDisbursementDate(null) // + .loanTermFrequency(6)// + .loanTermFrequencyType(2)// + .transactionProcessingStrategyCode(LoanProductTestBuilder.ADVANCED_PAYMENT_ALLOCATION_STRATEGY)// + .repaymentEvery(1)// + .repaymentFrequencyType(2)// + ;// + callFailedRuntimeException = Assertions.assertThrows(CallFailedRuntimeException.class, + () -> loanTransactionHelper.applyLoan(applicationRequest05)); + assertEquals(400, callFailedRuntimeException.getResponse().code()); + Assertions + .assertTrue(callFailedRuntimeException.getMessage().contains("The parameter `expectedDisbursementDate` is mandatory.")); + }); + } + private static Integer createLoanProduct(final String principal, final String repaymentAfterEvery, final String numberOfRepayments, boolean downPaymentEnabled, String downPaymentPercentage, boolean autoPayForDownPayment, LoanScheduleType loanScheduleType, LoanScheduleProcessingType loanScheduleProcessingType, final Account... accounts) {