You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "magyari-adam (via GitHub)" <gi...@apache.org> on 2025/01/23 13:35:16 UTC

[PR] FINERACT-1981: fix progressive accruals on early repayment [fineract]

magyari-adam opened a new pull request, #4282:
URL: https://github.com/apache/fineract/pull/4282

   ## Description
   
   Fix for progressive loan accrual handling in case of early repayments.
   
   Ignore if these details are present on the associated [Apache Fineract JIRA ticket](https://github.com/apache/fineract/pull/1284).
   
   
   ## Checklist
   
   Please make sure these boxes are checked before submitting your pull request - thanks!
   
   - [x] Write the commit message as per https://github.com/apache/fineract/#pull-requests
   
   - [x] 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.
   
   - [x] Create/update unit or integration tests for verifying the changes made.
   
   - [x] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
   
   - [x] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
   
   - [x] 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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-1981: fix progressive accruals on early repayment [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #4282:
URL: https://github.com/apache/fineract/pull/4282


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-1981: fix progressive accruals on early repayment [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4282:
URL: https://github.com/apache/fineract/pull/4282#discussion_r1927220050


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanAccrualsProcessingServiceImpl.java:
##########
@@ -499,7 +499,7 @@ private BigDecimal calcInterestAccruedAmount(@NotNull LoanRepaymentScheduleInsta
                     .map(p -> MathUtil.toBigDecimal(p.getTransactionAccrued())).reduce(BigDecimal.ZERO, MathUtil::add);
             BigDecimal accrued = MathUtil.subtractToZero(totalAccrued, prevAccrued);
             // if this is the current-last period, all the remaining accrued amount is added
-            return isInPeriod(tillDate, installment, false) ? accrued : MathUtil.min(installment.getInterestCharged(), accrued, false);

Review Comment:
   Are you sure we need this 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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-1981: fix progressive accruals on early repayment [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4282:
URL: https://github.com/apache/fineract/pull/4282#discussion_r1927219011


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanAccrualsProcessingServiceImpl.java:
##########
@@ -427,9 +427,9 @@ private void addInterestAccrual(@NotNull Loan loan, @NotNull LocalDate tillDate,
         AccrualPeriodData period = accrualPeriods.getPeriodByInstallmentNumber(installment.getInstallmentNumber());
         MonetaryCurrency currency = accrualPeriods.getCurrency();
         Money interest = null;
-        boolean isFullPeriod = isFullPeriod(tillDate, installment);
+        boolean isClosedPeriod = isAfterPeriod(tillDate, installment);

Review Comment:
   Something like "pastDue" or "pastPeriod" field name might be better...



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-1981: fix progressive accruals on early repayment [fineract]

Posted by "magyari-adam (via GitHub)" <gi...@apache.org>.
magyari-adam commented on code in PR #4282:
URL: https://github.com/apache/fineract/pull/4282#discussion_r1933487957


##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanCOBCreateAccrualsTest.java:
##########
@@ -423,4 +424,44 @@ public void chargeForObligationMetLoan() {
 
         });
     }
+
+    @Test
+    public void testEarlyRepaymentAccruals() {
+        AtomicReference<Long> loanIdRef = new AtomicReference<>();
+        setup();
+        final PostLoanProductsResponse loanProductsResponse = loanProductHelper.createLoanProduct(create4IProgressive());
+
+        runAt("20 December 2024", () -> {
+            Long loanId = applyAndApproveProgressiveLoan(client.getClientId(), loanProductsResponse.getResourceId(), "20 December 2024",
+                    430.0, 26.0, 6, null);
+
+            loanIdRef.set(loanId);
+
+            disburseLoan(loanId, BigDecimal.valueOf(430), "20 December 2024");
+
+            GetLoansLoanIdResponse loanDetails = loanTransactionHelper.getLoanDetails(loanId);
+            validateFullyUnpaidRepaymentPeriod(loanDetails, 1, "20 January 2025", 67.88, 0, 0, 9.32);
+            validateFullyUnpaidRepaymentPeriod(loanDetails, 2, "20 February 2025", 69.35, 0, 0, 7.85);
+            validateFullyUnpaidRepaymentPeriod(loanDetails, 3, "20 March 2025", 70.86, 0, 0, 6.34);
+            validateFullyUnpaidRepaymentPeriod(loanDetails, 4, "20 April 2025", 72.39, 0, 0, 4.81);
+            validateFullyUnpaidRepaymentPeriod(loanDetails, 5, "20 May 2025", 73.96, 0, 0, 3.24);
+            validateFullyUnpaidRepaymentPeriod(loanDetails, 6, "20 June 2025", 75.56, 0, 0, 1.64);
+
+            verifyTransactions(loanId, transaction(430.0d, "Disbursement", "20 December 2024"));
+            executeInlineCOB(loanId);
+        });
+        runAt("30 December 2024", () -> {
+            Long loanId = loanIdRef.get();
+            loanTransactionHelper.makeLoanRepayment("30 December 2024", 200.0F, loanId.intValue());
+        });
+        runAt("22 March 2025", () -> {
+            Long loanId = loanIdRef.get();
+            executeInlineCOB(loanId);
+
+            GetLoansLoanIdResponse loanDetails = loanTransactionHelper.getLoanDetails(loanId);
+            Assertions.assertTrue(loanDetails.getTransactions().stream().noneMatch(t -> (t.getType().getAccrual() && t.getAmount() > 0.31)

Review Comment:
   Added checks for around due date.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-1981: fix progressive accruals on early repayment [fineract]

Posted by "magyari-adam (via GitHub)" <gi...@apache.org>.
magyari-adam commented on code in PR #4282:
URL: https://github.com/apache/fineract/pull/4282#discussion_r1933487605


##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanAccrualsProcessingServiceImpl.java:
##########
@@ -427,9 +427,9 @@ private void addInterestAccrual(@NotNull Loan loan, @NotNull LocalDate tillDate,
         AccrualPeriodData period = accrualPeriods.getPeriodByInstallmentNumber(installment.getInstallmentNumber());
         MonetaryCurrency currency = accrualPeriods.getCurrency();
         Money interest = null;
-        boolean isFullPeriod = isFullPeriod(tillDate, installment);
+        boolean isClosedPeriod = isAfterPeriod(tillDate, installment);

Review Comment:
   Renamed it.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] FINERACT-1981: fix progressive accruals on early repayment [fineract]

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #4282:
URL: https://github.com/apache/fineract/pull/4282#discussion_r1927233282


##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanCOBCreateAccrualsTest.java:
##########
@@ -423,4 +424,44 @@ public void chargeForObligationMetLoan() {
 
         });
     }
+
+    @Test
+    public void testEarlyRepaymentAccruals() {
+        AtomicReference<Long> loanIdRef = new AtomicReference<>();
+        setup();
+        final PostLoanProductsResponse loanProductsResponse = loanProductHelper.createLoanProduct(create4IProgressive());
+
+        runAt("20 December 2024", () -> {
+            Long loanId = applyAndApproveProgressiveLoan(client.getClientId(), loanProductsResponse.getResourceId(), "20 December 2024",
+                    430.0, 26.0, 6, null);
+
+            loanIdRef.set(loanId);
+
+            disburseLoan(loanId, BigDecimal.valueOf(430), "20 December 2024");
+
+            GetLoansLoanIdResponse loanDetails = loanTransactionHelper.getLoanDetails(loanId);
+            validateFullyUnpaidRepaymentPeriod(loanDetails, 1, "20 January 2025", 67.88, 0, 0, 9.32);
+            validateFullyUnpaidRepaymentPeriod(loanDetails, 2, "20 February 2025", 69.35, 0, 0, 7.85);
+            validateFullyUnpaidRepaymentPeriod(loanDetails, 3, "20 March 2025", 70.86, 0, 0, 6.34);
+            validateFullyUnpaidRepaymentPeriod(loanDetails, 4, "20 April 2025", 72.39, 0, 0, 4.81);
+            validateFullyUnpaidRepaymentPeriod(loanDetails, 5, "20 May 2025", 73.96, 0, 0, 3.24);
+            validateFullyUnpaidRepaymentPeriod(loanDetails, 6, "20 June 2025", 75.56, 0, 0, 1.64);
+
+            verifyTransactions(loanId, transaction(430.0d, "Disbursement", "20 December 2024"));
+            executeInlineCOB(loanId);
+        });
+        runAt("30 December 2024", () -> {
+            Long loanId = loanIdRef.get();
+            loanTransactionHelper.makeLoanRepayment("30 December 2024", 200.0F, loanId.intValue());
+        });
+        runAt("22 March 2025", () -> {
+            Long loanId = loanIdRef.get();
+            executeInlineCOB(loanId);
+
+            GetLoansLoanIdResponse loanDetails = loanTransactionHelper.getLoanDetails(loanId);
+            Assertions.assertTrue(loanDetails.getTransactions().stream().noneMatch(t -> (t.getType().getAccrual() && t.getAmount() > 0.31)

Review Comment:
   This is a little bit weird way to check whether there was any "unwanted" accrual / accrual adjustment. Would you mind rather check specifically the original issue does not happens on the due date and due date + 1 at least?
   
   However the best would be to check each transaction!
   
   Something like this:
   ```
     verifyTransactions(loanId, //
                       transaction(430.d, "Disbursement", "20 December 2024", 430.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, false), //
                       transaction(200.0d, "Repayment", "30 December 2024", 0, 197.0, 0, 0, 0, 0, 3.0, false)),//
                       transaction(0.3d, "Accrual", "21 December 2024", 0, 0, 0, 0, 0, 0, 0.3, false)),//
                       ....
                       transaction(0.3d, "Accrual", "29 December 2024", 0, 0, 0, 0, 0, 0, 0.3, false)),//
   ```



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org