Merge pull request #4042 from alberto-art3ch/enhancement/loan_account_mandatory_parameters

FINERACT-2081: Validation error in mandatory loan account parameters

fineract-core/src/main/java/org/apache/fineract/infrastructure/core/data/DataValidatorBuilder.java

@@ -1056,4 +1056,12 @@ public class DataValidatorBuilder {
             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);
+    }
+
 }

fineract-e2e-tests-core/src/test/java/org/apache/fineract/test/stepdef/loan/LoanStepDef.java

@@ -513,8 +513,8 @@ public class LoanStepDef extends AbstractStepDef {
 
     @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.";
+        String errorMessageExpected = "Loan transaction processing strategy cannot be Advanced Payment Allocation Strategy if it's not configured on loan product";
 
         List<List<String>> data = table.asLists();
         List<String> loanData = data.get(1);

fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/serialization/LoanApplicationValidator.java

@@ -248,6 +248,9 @@ public final class LoanApplicationValidator {
         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 @@ public final class LoanApplicationValidator {
         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 @@ public final class LoanApplicationValidator {
             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 @@ public final class LoanApplicationValidator {
                 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 @@ public final class LoanApplicationValidator {
                         .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 @@ public final class LoanApplicationValidator {
 
             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 @@ public final class LoanApplicationValidator {
 
             final String transactionProcessingStrategy = this.fromApiJsonHelper
                     .extractStringNamed(LoanApiConstants.transactionProcessingStrategyCodeParameterName, element);
-
+            baseDataValidator.reset().parameter(LoanApiConstants.transactionProcessingStrategyCodeParameterName)
-            validateTransactionProcessingStrategy(transactionProcessingStrategy, loanProduct, baseDataValidator);
+                    .value(transactionProcessingStrategy).notNull();
 
             validateLinkedSavingsAccount(element, baseDataValidator);
 
@@ -727,9 +725,6 @@ public final class LoanApplicationValidator {
 
             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 @@ public final class LoanApplicationValidator {
             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 @@ public final class LoanApplicationValidator {
                         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 final class LoanApplicationValidator {
                 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 final class LoanApplicationValidator {
 
             // 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 final class LoanApplicationValidator {
     }
 
     private void validateClientOrGroup(Client client, Group group, Long productId) {
-        if (client != null) {
+        validateOrThrow("loan", baseDataValidator -> {
-            officeSpecificLoanProductValidation(productId, client.getOffice().getId());
+            if (client == null && group == null) {
-            if (client.isNotActive()) {
+                baseDataValidator.reset().parameter(LoanApiConstants.clientIdParameterName).value(client).notNull();
-                throw new ClientNotActiveException(client.getId());
+            } else {
-            }
+                if (client != null) {
-        }
+                    officeSpecificLoanProductValidation(productId, client.getOffice().getId());
-        if (group != null) {
+                    if (client.isNotActive()) {
-            officeSpecificLoanProductValidation(productId, group.getOffice().getId());
+                        throw new ClientNotActiveException(client.getId());
-            if (group.isNotActive()) {
+                    }
-                throw new GroupNotActiveException(group.getId());
+                }
-            }
+                if (group != null) {
-        }
+                    officeSpecificLoanProductValidation(productId, group.getOffice().getId());
+                    if (group.isNotActive()) {
+                        throw new GroupNotActiveException(group.getId());
+                    }
+                }
 
-        if (client != null && group != null) {
+                if (client != null && group != null) {
-            if (!group.hasClientAsMember(client)) {
+                    if (!group.hasClientAsMember(client)) {
-                throw new ClientNotInGroupException(client.getId(), group.getId());
+                        throw new ClientNotInGroupException(client.getId(), group.getId());
+                    }
+                }
             }
-        }
+        });
     }
 
     private void validateDisbursementDetails(LoanProduct loanProduct, JsonElement element) {
@@ -1747,18 +1773,13 @@ public final class LoanApplicationValidator {
         }
     }
 
-    private void validateTransactionProcessingStrategy(final String transactionProcessingStrategy, final LoanProduct loanProduct,
+    private void validateTransactionProcessingStrategy(final String transactionProcessingStrategy, final LoanProduct loanProduct) {
-            final DataValidatorBuilder baseDataValidator) {
-
-        baseDataValidator.reset().parameter(LoanApiConstants.transactionProcessingStrategyCodeParameterName)
-                .value(transactionProcessingStrategy).notNull();
 
         // 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(
+            throw new GeneralPlatformDomainRuleException("strategy.cannot.be.advanced.payment.allocation.if.not.configured",
-                    "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 final class LoanApplicationValidator {
                     dataValidationErrors);
         }
     }
+
+    public Long resolveOfficeId(Client client, Group group) {
+        if (client != null) {
+            return client.getOffice().getId();
+        }
+        if (group != null) {
+            return group.getOffice().getId();
+        }
+        return null;
+    }
+
+    private void throwMandatoryParameterError(final String parameterName) {
+        final List<ApiParameterError> 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);
+    }
 }

fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/serialization/LoanProductDataValidator.java

@@ -574,7 +574,6 @@ public final class LoanProductDataValidator {
         }
 
         // Fixed Length validation
-
         fixedLengthValidations(transactionProcessingStrategyCode, isInterestBearing, numberOfRepayments, repaymentEvery, element,
                 baseDataValidator);
 

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

@@ -160,7 +160,7 @@ public class LoanApplicationApprovalTest {
         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 class LoanApplicationApprovalTest {
                 .withRepaymentStrategy(AdvancedPaymentScheduleTransactionProcessor.ADVANCED_PAYMENT_ALLOCATION_STRATEGY)
                 .build(clientId.toString(), loanProductId.toString(), null);
         List<HashMap> error = (List<HashMap>) loanTransactionHelper.createLoanAccount(loanApplicationJSON, CommonConstants.RESPONSE_ERROR);
-        assertEquals(
+        assertEquals("strategy.cannot.be.advanced.payment.allocation.if.not.configured",
-                "validation.msg.loan.transactionProcessingStrategyCode.strategy.cannot.be.advanced.payment.allocation.if.not.configured",
                 error.get(0).get(CommonConstants.RESPONSE_ERROR_MESSAGE_CODE));
 
     }

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 class LoanOriginationValidationTest extends BaseLoanIntegrationTest {
         });
     }
 
+    // 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) {