Skip to content

FINERACT-2455: Add final discount fee amortization on charge-off and reversal - #6281

Open
Cocoa-Puffs wants to merge 2 commits into
apache:developfrom
openMF:FINERACT-2455-discount-fees-amortization-handling-on-charge-off-loan-account
Open

FINERACT-2455: Add final discount fee amortization on charge-off and reversal#6281
Cocoa-Puffs wants to merge 2 commits into
apache:developfrom
openMF:FINERACT-2455-discount-fees-amortization-handling-on-charge-off-loan-account

Conversation

@Cocoa-Puffs

Copy link
Copy Markdown
Contributor

Description

Describe the changes made and why they were made. (Ignore if these details are present on the associated Apache Fineract JIRA ticket.)

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Write the commit message as per our guidelines
  • 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 our 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
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
  • If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

@Cocoa-Puffs
Cocoa-Puffs force-pushed the FINERACT-2455-discount-fees-amortization-handling-on-charge-off-loan-account branch 2 times, most recently from d704146 to 17a622d Compare August 19, 2026 09:50
@MarianaDmytrivBinariks
MarianaDmytrivBinariks force-pushed the FINERACT-2455-discount-fees-amortization-handling-on-charge-off-loan-account branch from 17a622d to 8e91fd4 Compare August 21, 2026 14:36

@galovics galovics left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went back over this one more carefully given the size and the financial logic involved, and I think there are real gaps in the charge-off/reversal symmetry.

1. Charge-off lifted via reprocessing never reverses the final amortization

WorkingCapitalLoanTransactionReprocessingServiceImpl#replayChargeOff:

if (!MathUtil.isGreaterThanZero(chargeOffAmount)) {
    chargeOffTransaction.setReversed(true);
    ...
    loan.liftChargeOff();
    ...
    return false;   // <-- returns before restateFinalDiscountFeeAmortization runs
}
...
restateFinalDiscountFeeAmortization(loan, balance, chargeOffTransaction, accountingEnabled);

When a backdated repayment/goodwill/payout-refund zeroes the outstanding, the charge-off is auto-lifted, but the linked final discount-fee amortization survives un-reversed - its journal entries keep crediting the charge-off expense account on a loan that's no longer charged off. undoChargeOff (the explicit path) does reverse it via undoDiscountFeeAmortizationOnChargeOff; this path doesn't. Every lift scenario in the feature file uses discount | 0, so nothing currently exercises a lift with a non-zero discount pool - worth adding one, and it should catch this once fixed.

2. Apply-adjustment restates, undo-adjustment doesn't

makeDiscountFeeAdjustment reprocesses transactions when the loan is charged off; undoDiscountFeeAdjustment doesn't have the same call. So on a charged-off loan, applying a backdated adjustment shrinks the final amortization, but undoing that adjustment restores the discount pool while leaving the amortization shrunk - and since DiscountFeeAmortizationBusinessStep now skips charged-off loans, there's no COB path to reconcile the difference either. New scenario UC51 actually asserts this stale state as "correct" (discount back to 1000, amortization still 750) - it only looks fine because the scenario fully repays and closes the loan afterward, which happens to post the missing amount on close. A charged-off loan that's never repaid would strand that amount permanently.

3. One-way door once reversed

The relation lookup used by the restate path filters on reversed = false, so once a final amortization has been reversed by a pool drop to zero, a later undo of that adjustment can never link back to it and restore it. Combined with #2 this means any deferred-income residual on a charged-off loan has no path back onto the ledger.

4. UC49's reversed-charge-off table doesn't actually assert the reversal

Then Working Capital Loan Transactions tab has a reversed "CHARGE_OFF" transaction ...
  | EXPENSE | ... | Credit Loss/Bad Debt-Fraud | 10000.0 |         |
  | ASSET   | ... | Loans Receivable           |         | 10000.0 |
  | EXPENSE | ... | Credit Loss/Bad Debt-Fraud | 10000.0 |         |   <- duplicate, should flip sides
  | ASSET   | ... | Loans Receivable           |         | 10000.0 |   <- duplicate, should flip sides

The mirror entries should flip debit/credit sides (as UC48 and the pre-existing UC2/UC4 correctly do), but the step def only checks row containment + count, so a duplicated row passes silently and the actual reversal legs go unverified.

5. No business events on the new lump-sum transaction, its reversal, or its restatement - processFinalDiscountFeeAmortizationOnChargeOff, undoDiscountFeeAmortizationOnChargeOff, and restateFinalDiscountFeeAmortization all publish nothing, unlike the periodic amortization path.

The rounding/precision side is fine - no division anywhere in the new path, the final amount is always a residual (pool - realized), so it self-corrects against periodic drift. That part I have no complaints about.

Recommendation: CHANGES_REQUESTED

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants