From 040b85356447a4c1b8eed108f0aa98fa35cc54a4 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 | 2 +- .../LoanApplicationValidator.java | 49 ++++++---- ...ntAllocationLoanRepaymentScheduleTest.java | 3 +- .../LoanApplicationApprovalTest.java | 5 +- .../LoanOriginationValidationTest.java | 90 +++++++++++++++++++ 6 files changed, 136 insertions(+), 21 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..0c4a1db7b77 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,7 +513,7 @@ 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; + int errorCodeExpected = 403; String errorMessageExpected = "Failed data validation due to: strategy.cannot.be.advanced.payment.allocation.if.not.configured."; List> data = table.asLists(); 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..1f7752ae20a 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(); @@ -511,7 +513,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 +524,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); @@ -744,12 +745,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 +759,12 @@ 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); } private void validateBorrowerCycle(JsonElement element, LoanProduct loanProduct, Long clientId, Long groupId, @@ -928,7 +934,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 +1416,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(); @@ -1747,18 +1753,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 +2128,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/integration-tests/src/test/java/org/apache/fineract/integrationtests/AdvancedPaymentAllocationLoanRepaymentScheduleTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/AdvancedPaymentAllocationLoanRepaymentScheduleTest.java index 8dac9a6fe6c..86183140fc7 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/AdvancedPaymentAllocationLoanRepaymentScheduleTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/AdvancedPaymentAllocationLoanRepaymentScheduleTest.java @@ -3784,8 +3784,7 @@ public void uc130() { .loanScheduleProcessingType(LoanScheduleProcessingType.HORIZONTAL.name()))); assertEquals(403, exception.getResponse().code()); - assertTrue(exception.getMessage() - .contains("error.msg.loan.repayment.strategy.can.not.be.different.than.advanced.payment.allocation")); + assertTrue(exception.getMessage().contains("error.msg.fixed.length.only.supported.for.advanced.payment.allocation")); }); } 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..5544a0157ae 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(client.getClientId(), loanProductResponse.getResourceId(), + operationDate, 100.0, 5).numberOfRepayments(6)// + .clientId(null) // + .loanTermFrequency(6)// + .loanTermFrequencyType(2)// + .transactionProcessingStrategyCode(LoanProductTestBuilder.ADVANCED_PAYMENT_ALLOCATION_STRATEGY)// + .repaymentEvery(1)// + .repaymentFrequencyType(2)// + ;// + callFailedRuntimeException = Assertions.assertThrows(CallFailedRuntimeException.class, + () -> loanTransactionHelper.applyLoan(applicationRequest03)); + 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) {