FINERACT-2421: Reprocess Interest Refund txn amount if no txn was changed #5218

alberto-art3ch · 2025-12-11T03:12:17Z

Description

Let's allow to reprocess Interest Refund txn amount even if no transaction was changed (reverse-replayed) before

FINERACT-2421

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

ruzeynalov · 2025-12-11T14:55:54Z

...gration-tests/src/test/java/org/apache/fineract/integrationtests/LoanInterestRefundTest.java

+
+            loanTransactionHelper.reverseLoanTransaction(loanId, repaymentResponse.getResourceId(),
+                    new PostLoansLoanIdTransactionsTransactionIdRequest().dateFormat(DATETIME_PATTERN)
+                    .transactionDate("25 January 2021").transactionAmount(0.0).locale("en"));


@alberto-art3ch I think transaction date is incorrect here - I assume it should be 20 January 2025

Resolved, thanks!

Aman-Mittal · 2025-12-14T11:36:54Z

...oanaccount/domain/transactionprocessor/impl/AdvancedPaymentScheduleTransactionProcessor.java

-                    final Money newAmount = interestBeforeRefund.minus(progCtx.getSumOfInterestRefundAmount()).minus(interestAfterRefund);
-                    loanTransaction.updateAmount(newAmount.getAmount());
-                }
+                final Money interestAfterRefund = interestRefundService.totalInterestByTransactions(this, loan.getId(), targetDate,


I am seeing you have removed the condition check for !modifiedTransactions.isEmpty(), is there is any specific usecase for it?

@Aman-Mittal This check was preventing any recalculation in case no transactions were changed prior interest refund. While in many situations it can be considered as "optimization", but there are edge cases where it would be crucial to recalculate interest refund amount anyway, so it is safer to remove this optimization and allow the system to recalculate.

adamsaghy · 2025-12-18T08:53:57Z

Please change PR and commit to use: https://issues.apache.org/jira/browse/FINERACT-2421

alberto-art3ch · 2025-12-19T04:15:47Z

Please change PR and commit to use: https://issues.apache.org/jira/browse/FINERACT-2421

Done!

adamsaghy · 2025-12-19T08:59:56Z

> Task :fineract-rates:spotbugsMain
/home/runner/work/fineract/fineract/fineract-progressive-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/transactionprocessor/impl/AdvancedPaymentScheduleTransactionProcessor.java:40: error: [WildcardImport] Wildcard imports, static or otherwise, should not be used
import java.util.*;
^
    (see https://google.github.io/styleguide/javaguide.html?cl=head#s3.3.1-wildcard-imports)

> Task :fineract-progressive-loan:compileJava
1 error
10 warnings

alberto-art3ch · 2025-12-19T13:44:57Z

import java.util.*;

Done! Thanks

adamsaghy · 2025-12-23T11:36:05Z

...loan/src/main/java/org/apache/fineract/portfolio/loanaccount/data/TransactionChangeData.java

+        final MonetaryCurrency currency = loanTransaction.getLoan().getCurrency();
+        return (oldTransaction.getId() != null && oldTransaction.getTypeOf().equals(loanTransaction.getTypeOf()) //
+                && oldTransaction.getTransactionDate().equals(loanTransaction.getTransactionDate()) //
+                && (oldTransaction.getAmount(currency).compareTo(loanTransaction.getAmount(currency)) == 0));


A simple getId() is enought. I see no reason for the rest of the checks...

The reason to have the other checks is to ensure that we are validating the same IR Transaction for the same date and amount, If we use only the getId() method we can get a true for other new transaction that is not a IR transaction

adamsaghy · 2025-12-23T11:36:39Z

...gration-tests/src/test/java/org/apache/fineract/integrationtests/LoanInterestRefundTest.java

-                    transaction(2.88, "Interest Refund", "22 January 2021"), //
-                    transaction(4.66, "Accrual", "22 January 2021") //
-            );
+            /*


why to comment out?

Restored, sorry!

adamsaghy · 2025-12-23T11:36:49Z

fineract-e2e-tests-runner/src/test/resources/features/EMICalculation.feature

      | 4  | 30   | 01 May 2024      | 01 February 2024 | 319.66          | 169.66        | 1.83     | 0.0  | 0.0       | 171.49 | 171.49 | 171.49     | 0.0  | 0.0         |
      | 5  | 31   | 01 June 2024     | 01 February 2024 | 148.17          | 171.49        | 0.0      | 0.0  | 0.0       | 171.49 | 171.49 | 171.49     | 0.0  | 0.0         |
-      | 6  | 30   | 01 July 2024     |                  | 0.0             | 148.17        | 2.77     | 0.0  | 0.0       | 150.94 | 80.6   | 80.6       | 0.0  | 70.34       |
+      | 6  | 30   | 01 July 2024     |                  | 0.0             | 148.17        | 2.77     | 0.0  | 0.0       | 150.94 | 80.59  | 80.59      | 0.0  | 70.35       |


why is the change?

adamsaghy · 2025-12-23T11:38:51Z

...gration-tests/src/test/java/org/apache/fineract/integrationtests/LoanInterestRefundTest.java

                    transaction(1.44, "Interest Refund", "22 January 2021"), //
                    transaction(400.0, "Payout Refund", "26 January 2021"), //
-                    transaction(2.58, "Interest Refund", "26 January 2021") //
+                    transaction(2.57, "Interest Refund", "26 January 2021") //


why the change?

But why would it be changed? Rounding does not explain... i am interested in what caused this change

adamsaghy · 2026-01-02T22:16:42Z

@alberto-art3ch Kindly check my raised questions.

…nged

… txn amount if no txn was changed

adamsaghy · 2026-01-08T11:19:54Z

...oanaccount/domain/transactionprocessor/impl/AdvancedPaymentScheduleTransactionProcessor.java


+    private boolean validateInterestRefundTransactionRelation(final LoanTransaction interestRefundTransaction) {


Can you please refresh my memory why we need this?

ruzeynalov suggested changes Dec 11, 2025

View reviewed changes

Aman-Mittal reviewed Dec 14, 2025

View reviewed changes

alberto-art3ch force-pushed the FINERACT-2389/allow-to-reprocess-interest-refund-txn-even-if-no-transaction-was-changed branch 2 times, most recently from 64c4403 to 5a4160f Compare December 18, 2025 03:07

ruzeynalov approved these changes Dec 18, 2025

View reviewed changes

ruzeynalov deleted the FINERACT-2389/allow-to-reprocess-interest-refund-txn-even-if-no-transaction-was-changed branch December 18, 2025 16:07

ruzeynalov force-pushed the FINERACT-2389/allow-to-reprocess-interest-refund-txn-even-if-no-transaction-was-changed branch from 556694f to e616b69 Compare December 18, 2025 16:41

alberto-art3ch force-pushed the FINERACT-2389/allow-to-reprocess-interest-refund-txn-even-if-no-transaction-was-changed branch 2 times, most recently from 8b917f0 to 1a25b9d Compare December 19, 2025 04:14

alberto-art3ch changed the title ~~FINERACT-2389: Reprocess Interest Refund txn amount if no txn was changed~~ FINERACT-2421: Reprocess Interest Refund txn amount if no txn was changed Dec 19, 2025

alberto-art3ch force-pushed the FINERACT-2389/allow-to-reprocess-interest-refund-txn-even-if-no-transaction-was-changed branch 2 times, most recently from 221a2db to f001eda Compare December 19, 2025 13:44

alberto-art3ch force-pushed the FINERACT-2389/allow-to-reprocess-interest-refund-txn-even-if-no-transaction-was-changed branch from f001eda to b9d2adb Compare December 19, 2025 20:00

adamsaghy reviewed Dec 23, 2025

View reviewed changes

alberto-art3ch force-pushed the FINERACT-2389/allow-to-reprocess-interest-refund-txn-even-if-no-transaction-was-changed branch 3 times, most recently from 0398990 to e2cd523 Compare January 8, 2026 04:14

Jose Alberto Hernandez and others added 2 commits January 7, 2026 23:16

FINERACT-2389: Reprocess Interest Refund txn amount if no txn was cha…

9312056

…nged

FINERACT-2421: added e2e test for verifying reprocess Interest Refund…

e919183

… txn amount if no txn was changed

alberto-art3ch force-pushed the FINERACT-2389/allow-to-reprocess-interest-refund-txn-even-if-no-transaction-was-changed branch from e2cd523 to e919183 Compare January 8, 2026 04:17

adamsaghy reviewed Jan 8, 2026

View reviewed changes


		private boolean validateInterestRefundTransactionRelation(final LoanTransaction interestRefundTransaction) {

FINERACT-2421: Reprocess Interest Refund txn amount if no txn was changed #5218

Are you sure you want to change the base?

FINERACT-2421: Reprocess Interest Refund txn amount if no txn was changed #5218

Conversation

alberto-art3ch commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

ruzeynalov Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamsaghy commented Dec 18, 2025

Uh oh!

alberto-art3ch commented Dec 19, 2025

Uh oh!

adamsaghy commented Dec 19, 2025

Uh oh!

alberto-art3ch commented Dec 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamsaghy Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamsaghy commented Jan 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alberto-art3ch commented Dec 11, 2025 •

edited

Loading

ruzeynalov Dec 11, 2025 •

edited

Loading

adamsaghy Jan 7, 2026 •

edited

Loading