You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2020/12/04 22:07:37 UTC
[GitHub] [fineract] francisguchie opened a new pull request #1536: block-loan-overpayment (FINERACT-1052)
francisguchie opened a new pull request #1536:
URL: https://github.com/apache/fineract/pull/1536
## Description
https://github.com/apache/fineract/pull/1264
Addressing the following comments from @ptuomola
Addressing these comments below
The current behaviour should be kept as the default - i.e. without a configuration present in the database, the same behaviour should remain (overpayments allowed)
The default configuration added to this should also keep the current behaviour as-is - i.e. the default Flyway script should create the configuration in such way that it allows overpayments
The existing integration tests should be kept-as is and they should continue to be used to test the current behaviour (i.e. overpayment allowed)
but Not able to answer this
We should create a new integration test (or multiple tests) to test the scenario where overpayments are not allowed. These tests should first set the flag to disable overpayments, then carry out the tests to confirm that overpayments fail in different scenarios, and then reset the flag back to enabling overpayments
## Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Write the commit message as per https://github.com/apache/fineract/#pull-requests
- [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
- [ ] Create/update unit or integration tests for verifying the changes made.
- [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
- [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/api-docs/apiLive.htm with details of any API changes
- [ ] Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)
FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [fineract] avikganguly01 commented on a change in pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1536:
URL: https://github.com/apache/fineract/pull/1536#issuecomment-766094667
@ptuomola probably doesn't have time to review this currently (which is fine, no obligation!) so it would be good to find someone else wanting to review this. I'll post to the dev mailing list asking for volunteers. If we cannot find someone, I will just merge this PR in 1 week, based on "trust" of the parties involved here so far.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [fineract] francisguchie commented on pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
francisguchie commented on pull request #1536:
URL: https://github.com/apache/fineract/pull/1536#issuecomment-766359355
@vorburger , thank you for your commitment to support. Been away on holiday in Uganda I am back to work now
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [fineract] vorburger commented on pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1536:
URL: https://github.com/apache/fineract/pull/1536#issuecomment-743778606
@ptuomola if I understand this one correctly, this is "take II" of #1264, for FINERACT-1052. Would you like to review this?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [fineract] ptuomola commented on a change in pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
ptuomola commented on a change in pull request #1536:
URL: https://github.com/apache/fineract/pull/1536#discussion_r543710273
##########
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:
Hmm... not quite sure I understand this. Seems that this code allows repayments to be used to pay written off amounts. Whilst this may be right (though should we not deduct the amount transferred from writtenOffBalance?), why/how is this linked to the functionality to stop overpayments?
In other words, why does turning off overpayments for loans also change how we handle written off balances?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [fineract] francisguchie commented on pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
francisguchie commented on pull request #1536:
URL: https://github.com/apache/fineract/pull/1536#issuecomment-778845697
@avikganguly01 - I hope i understand your comments well please see my comments below for every bullet
• A written off status loan should have an outstanding balance. Leads to state inconsistencies compared to ledger.
**_**o When a loan is written-off, on the face value (loan summary you have no balance seen, however, the loan still maintains a balance (in the background) that is why one can still make a payment against it which payment is sent to incomes for that particular period. As-is, Over-payment is allowed, thus fineract does not consider the loan balance (written-off amount) to restrict the user. So for fineract to restrict over-payment, we need to supply the written-off amount to it. So that it can alert user when they hit that value.
o What we are doing here does not in any-way affect the Ledger balances or how the payments are treated**_**
• If you are changing the loan status, you have to reverse any write-off journal entries, post accruals till date, etc. - not sure if this is the route you want to go to.
**_o We are not in anyway changing the loan status- please make me understand this better – what I know is we are only keeping in mind what was written-off_**
• If you want to continue doing recovery payments without going into overpayment, is this the right place in code to handle recovery repayments?
**_o This is the same place loan repayment transactions are being addressed, I think putting a condition in the same area of code would be nice. Nevertheless, help me with some options (please note am a beginner)_**
• In either case, you have to enhance the test to handle the impact of any change in this code snippet like explicitly state write-off balance, verify if it's recovery repayment or write-off reversal.
**_o At the moment, we do not have an option to reverse a write-off so we shall only have to look at recovery repayment._**
• Mayble also include the logic in test for the dry run you are doing. Ex:- Write-off balance 1000, Total Recovery Repaid - 800, Repayment - 100. This IF condition is satisfied. But why should outstanding Balance become 1000 and not 100? Please correct me if I am wrong regarding assuming recovery repayments don't reduce write-off balance.
**_o Once a loan is written-off, the write-off amount (accounting standards) never changes it remains as such and during the period which this write-off is done, it is recorded as a loss. (never changes)
o So as we recover, we look at how much has been recovered as against what was written-off so in your example Write-off balance 1,000, Total Recovery Repaid - 800, Repayment – 100, we simply keep use the approach
Write-off balance 1,000
Total recovery so far 800
new recovery repayment 100
so we test 100+800 vs write-off balance (1,000)_**
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [fineract] francisguchie commented on a change in pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
francisguchie commented on a change in pull request #1536:
URL: https://github.com/apache/fineract/pull/1536#discussion_r546602134
##########
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:
@rrpawar96
thanks for sharing these details
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [fineract] francisguchie closed pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
francisguchie closed pull request #1536:
URL: https://github.com/apache/fineract/pull/1536
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [fineract] avikganguly01 commented on a change in pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [fineract] francisguchie commented on a change in pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
francisguchie commented on a change in pull request #1536:
URL: https://github.com/apache/fineract/pull/1536#discussion_r575864732
##########
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:
Thanks for this @avikganguly01, I will change this
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [fineract] vorburger edited a comment on pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
vorburger edited a comment on pull request #1536:
URL: https://github.com/apache/fineract/pull/1536#issuecomment-743778606
@ptuomola if I understand this one correctly, this is "take II" of #1264, for FINERACT-1052. Would you like to (re)review this?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [fineract] francisguchie commented on pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
francisguchie commented on pull request #1536:
URL: https://github.com/apache/fineract/pull/1536#issuecomment-794268322
> NOK, 81 commits.. somehow Git got confused. Try rebase, or start over.
I will close and open new PR for this
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [fineract] francisguchie commented on pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
francisguchie commented on pull request #1536:
URL: https://github.com/apache/fineract/pull/1536#issuecomment-794616511
Starting afresh for this take 2
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [fineract] rrpawar96 commented on a change in pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
rrpawar96 commented on a change in pull request #1536:
URL: https://github.com/apache/fineract/pull/1536#discussion_r546418957
##########
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:
@ptuomola, yes, we are considering the written-off amounts because usually when
we repay the loan, it deducts from the outstanding-balance but,
when a user written-off the loan, the outstanding-balance straight away becomes 0.
because the exact same amount is transferred to the Written-off.
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)
otherwise, if the flag is not enabled(default condition) it will not stop even after the amount gets equal and will continue to get overpaid.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [fineract] francisguchie commented on pull request #1536: block-loan-overpayment (FINERACT-1052)
Posted by GitBox <gi...@apache.org>.
francisguchie commented on pull request #1536:
URL: https://github.com/apache/fineract/pull/1536#issuecomment-741776760
@vorburger @rrpawar96 I have done a functional test and this is working fine
----------------------------------------------------------------
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:
users@infra.apache.org