avikganguly01 commented on a change in pull request #1536: URL: https://github.com/apache/fineract/pull/1536#discussion_r563425459
########## File path: integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanDeclineOnLoanOverPaymentTest.java ########## @@ -0,0 +1,169 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.integrationtests; + +import io.restassured.builder.RequestSpecBuilder; +import io.restassured.builder.ResponseSpecBuilder; +import io.restassured.http.ContentType; +import io.restassured.specification.RequestSpecification; +import io.restassured.specification.ResponseSpecification; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import org.apache.fineract.integrationtests.common.ClientHelper; +import org.apache.fineract.integrationtests.common.CommonConstants; +import org.apache.fineract.integrationtests.common.GlobalConfigurationHelper; +import org.apache.fineract.integrationtests.common.Utils; +import org.apache.fineract.integrationtests.common.loans.LoanApplicationTestBuilder; +import org.apache.fineract.integrationtests.common.loans.LoanProductTestBuilder; +import org.apache.fineract.integrationtests.common.loans.LoanStatusChecker; +import org.apache.fineract.integrationtests.common.loans.LoanTransactionHelper; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class LoanDeclineOnLoanOverPaymentTest { + + private static final Logger LOG = LoggerFactory.getLogger(LoanDeclineOnLoanOverPaymentTest.class); + private ResponseSpecification responseSpec; + private RequestSpecification requestSpec; + private LoanTransactionHelper loanTransactionHelper; + private LoanApplicationApprovalTest loanApplicationApprovalTest; + private GlobalConfigurationHelper globalConfigurationHelper; + private ResponseSpecification httpStatusForidden; + + @BeforeEach + public void setup() { + Utils.initializeRESTAssured(); + this.requestSpec = new RequestSpecBuilder().setContentType(ContentType.JSON).build(); + this.requestSpec.header("Authorization", "Basic " + Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey()); + this.responseSpec = new ResponseSpecBuilder().expectStatusCode(200).build(); + this.loanTransactionHelper = new LoanTransactionHelper(this.requestSpec, this.responseSpec); + this.loanApplicationApprovalTest = new LoanApplicationApprovalTest(); + this.httpStatusForidden = new ResponseSpecBuilder().expectStatusCode(400).build(); + } + + @Test + public void loanApplicationOverPayment() { + + final String proposedAmount = "10000"; + final String approvalAmount = "10000"; + final String disburseAmount = "10000"; + final String amountToBePaid = "12000.00"; + Float RepaymentAmount = Float.valueOf(amountToBePaid); + + final String approveDate = "01 March 2015"; + final String expectedDisbursementDate = "01 March 2015"; + final String writeOffDate = "01 March 2015"; + final String disbursementDate = "01 March 2015"; + final String adjustRepaymentDate = "16 March 2015"; + List<HashMap> approveTranches = null; + + final Integer clientID = ClientHelper.createClient(this.requestSpec, this.responseSpec, "01 January 2012"); + final Integer loanProductID = this.loanTransactionHelper.getLoanProductId(new LoanProductTestBuilder().build(null)); + Integer loanID = applyForLoanApplication(clientID, loanProductID, proposedAmount); + + HashMap loanStatusHashMap = LoanStatusChecker.getStatusOfLoan(this.requestSpec, this.responseSpec, loanID); + LoanStatusChecker.verifyLoanIsPending(loanStatusHashMap); + + LOG.info("-----------------------------------PENDING LOAN-----------------------------------------------------------"); + + loanStatusHashMap = this.loanTransactionHelper.approveLoanWithApproveAmount(approveDate, expectedDisbursementDate, approvalAmount, + loanID, approveTranches); + LOG.info("-----------------------------------APPROVE LOAN-----------------------------------------------------------"); + LoanStatusChecker.verifyLoanIsWaitingForDisbursal(loanStatusHashMap); + + loanStatusHashMap = this.loanTransactionHelper.disburseLoan(disbursementDate, loanID, disburseAmount); + // loanStatusHashMap = LoanStatusChecker.getStatusOfLoan(this.requestSpec, this.responseSpec, loanID); + LOG.info("-----------------------------------DISBURSE LOAN-----------------------------------------------------------"); + LoanStatusChecker.verifyLoanIsActive(loanStatusHashMap); + + // Retrieving All Global Configuration details + final ArrayList<HashMap> globalConfig = GlobalConfigurationHelper.getAllGlobalConfigurations(requestSpec, responseSpec); + Assertions.assertNotNull(globalConfig); + + // Updating Value for reschedule-repayments-on-holidays Global + // Configuration + Integer configId = (Integer) globalConfig.get(30).get("id"); Review comment: @francisguchie : Is 30 the PK ID of the config? Auto increment config might not match 30. Please fetch by config name. ########## File path: fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java ########## @@ -168,6 +180,40 @@ public LoanTransaction makeRepayment(final Loan loan, final CommandProcessingRes final List<Long> existingReversedTransactionIds = new ArrayList<>(); final Money repaymentAmount = Money.of(loan.getCurrency(), transactionAmount); + + Money outstandingBalance = Money.of(loan.getCurrency(), loan.getSummary().getTotalOutstanding()); + + if (outstandingBalance.isZero()) { // In writtenOff loans, the outstandingBalance is transferred to writtenOff + final Money writtenOffBalance = Money.of(loan.getCurrency(), loan.getSummary().getTotalWrittenOff()); + + if (writtenOffBalance.isGreaterThanZero()) { // When outstanding balance is 0 & writtenbalance is greater + // than 0, + // it confirms that the loan has been writtenOff. + final Money totalRecoveryPaid = Money.of(loan.getCurrency(), loan.getSummary().getTotalRecoveryPaid()); + + if (writtenOffBalance.isGreaterThanOrEqualTo(repaymentAmount.plus(totalRecoveryPaid)) && isAvoidLoanOverpaymentEnabled) { + outstandingBalance = writtenOffBalance; // transferring the writtenOff Balance to Outstanding for Review comment: @francisguchie @rrpawar96 : "So that's why (when the block-overpayment flag is enabled.) we will be repaying the money until it gets equal to the written-off amounts(or outstanding-balance)" - Block overpayment flag is more than welcome as overpayment creates some issues. But this doesn't look good to me. - A written off status loan should have an outstanding balance. Leads to state inconsistencies compared to ledger. - If you are changing the loan status, you have to reverse any writeoff journal entries, post accruals till date, etc - not sure if this is the route you want to go to. - If you want to continue doing recovery payments without going into overpayment, is this the right place in code to handle recovery repayments? - In either case, you have to enhance the test to handle the impact of any change in this code snippet like explicitly state writeoff balance, verify if it's recovery repayment or writeoff reversal. - Mayble also include the logic in test for the dry run you are doing. Ex:- Writeoff balance 1000, Total Recovery Repaid - 800, Repayment - 100. This IF condition is satisfied. But why should outstandingBalance become 1000 and not 100? Please correct me if I am wrong regarding assuming recovery repayments don't reduce writeoff balance. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
