You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fineract.apache.org by "Cocoa-Puffs (via GitHub)" <gi...@apache.org> on 2026/09/17 14:33:35 UTC
[PR] FINERACT-2455: Fix COB amortizing discount fee before payment, fix WC loan closedOnDate wrong when backdated transaction closes [fineract]
Cocoa-Puffs opened a new pull request, #6469:
URL: https://github.com/apache/fineract/pull/6469
## 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](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#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](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made.
- [ ] Follow our [coding conventions](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
- [ ] [This PR must not be a "code dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute).
- [ ] If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.
- [ ] I followed the [AI Policy](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#ai-policy).
Your assigned reviewer(s) will follow our [guidelines for code reviews](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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Fix COB amortizing discount fee before payment, fix WC loan closedOnDate wrong when backdated transaction closes [fineract]
Posted by "Cocoa-Puffs (via GitHub)" <gi...@apache.org>.
Cocoa-Puffs commented on PR #6469:
URL: https://github.com/apache/fineract/pull/6469#issuecomment-5811818284
@galovics I've addressed the concerns.
## 1. A backdated settlement that moves earlier leaves stale dates — confirmed, fixed
Reproduced exactly as described. Negative control on the pre-fix build (9000 on 02 June, 1500 on 10 June, then 1000 backdated to 05 June):
```
timeline.actualMaturityDate expected: <2026-06-05> but was: <2026-06-10>
```
`recalculateSettlementDates(loan, statusBeforeEvent)` is now `recalculateSettlementDates(loan)`. The one-way rule, the floor and the `statusBeforeEvent` argument are gone; both dates are assigned outright from a new `settlementDateFromHistory(loan)`.
I didn't take the literal suggestion of adding `DISCOUNT_FEE_ADJUSTMENT` to the query, because `WorkingCapitalLoanTransactionAllocation.forDiscountFeeAdjustment` hardcodes `overpaymentPortion = 0`. The `COALESCE(a.overpaymentPortion, 0) < t.transactionAmount` filter would therefore pass for *every* adjustment, including one on an already-overpaid loan, which would then claim a settlement it didn't make — loan overpaid since 02 Jan, adjustment on 05 Jan, maturity wrongly moves to 05 Jan. Instead the state machine's stamp survives only while a non-reversed discount fee adjustment exists on that exact date. Same pure-function-of-history property you were after, without that regression.
## 2. The as-of target isn't matched by an as-of "already posted" total — confirmed, fixed
New `sumNetAmortizationAsOf`; `processDiscountFeeAmortization` now bounds the posted total by the same date as the target. Negative control on the pre-fix build:
```
transactionDate expected: <05 January 2026> but was: <02 January 2026>
```
That's the 1000 reversal landing on the first replayed day, before the income it reverses.
One correction to your figures: in the scenario we built, the closing amortization is dated on the *settlement* day rather than the payment day, so the corrected shape is +739.90 on 05 January and −1000 on 07 January, not 0 / −260.10. Net realized income is 739.90 either way; the defect and the fix are exactly as you described.
## 3. Closure income dated inconsistently — fixed
All three `accrueOnClosure` sites now date from the settlement, and `settlementDate()` reads `maturedOnDate` in both branches, so it can no longer disagree with `recalculateSettlementDates`.
The accruals use a separate `closureIncomeDate()` rather than `settlementDate()`, because the two answer different questions. The discount is measured against a schedule, and a schedule changes on the day it changes — so a discount fee adjustment landing after the settlement earns its correction on its own later day, and `settlementDate()` keeps its floor for that. A charge accrual instead recognizes income the *closure* leaves unrecognized. A write-off still falls back to the transaction date, where the closing transaction is the settlement.
In fairness this one is a consistency fix rather than a defect repair: `accrueOnClosure` already fires on whichever event settles the loan, so by the time a later non-settling event reaches it every active charge is accrued and the call is idempotent. The "accruals land before the settlement" case is the opposite direction, and the floor already handled it.
## 4. CBR scope — you're right, and we've reversed it
Agreed: a CBR is a real money movement, the account isn't closed until it happens, and it can absolutely be what closes the loan. `DefaultLoanLifecycleStateMachine.determineTransitionFromOverpaid` stamps both `closedOnDate` and `actualMaturityDate` from the refund's own date, and WC now does the same. The CBR path does no correction at all — `recalculateSettlementDates` is not called there, and the javadoc says why. Three assertions moved:
| Test | Field | Was | Now |
| --- | --- | --- | --- |
| `C106757` (UC17) | `closedOnDate`, `actualMaturityDate` | `2026-01-05` | `2026-01-09` |
| `C98268` (UC6) | `closedOnDate` | `2026-01-04` | `2026-01-06` |
| `C83084` (UC9) | `closedOnDate`, `actualMaturityDate` | `2026-01-03` | `2026-01-04` |
`C106757`'s scenario title was reworded too, since it asserted the old rule in prose. `C98268` goes back to the tester's original value.
## Tests added
All three you asked for:
- `ProjectedAmortizationScheduleAsOfTest` — 5 tests covering `totalActualAmortizationAsOf`: fast vs rebuild path, rate change before and after the as-of date, with `assertNotEquals` guards so a dropped rate change fails rather than passing silently.
- A Feign test for the backdated repayment into an overpaid loan with a later refund, asserting both `actualMaturityDate` and `closedOnDate`.
- UC19 — a COB after the undo, asserting the amortization adjustment. It carries a `# TODO: assign a TestRail ID` rather than an invented one.
## Minor
`asOfModel` now adds payments straight to the list and rebuilds once, the way `regenerate()` already does. Comments trimmed throughout.
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Fix COB amortizing discount fee before payment, fix WC loan closedOnDate wrong when backdated transaction closes [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #6469:
URL: https://github.com/apache/fineract/pull/6469
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Fix COB amortizing discount fee before payment, fix WC loan closedOnDate wrong when backdated transaction closes [fineract]
Posted by "Cocoa-Puffs (via GitHub)" <gi...@apache.org>.
Cocoa-Puffs commented on PR #6469:
URL: https://github.com/apache/fineract/pull/6469#issuecomment-5773694969
Both findings turned out to be correct. `recalculateClosedOnDate(loan)` became `recalculateSettlementDates(loan, statusBeforeEvent)`, covering the OVERPAID branch as well as the closed one. I've extended the repository query to return the date of the newest live repayment-like transaction whose allocation is not entirely overpayment.
The overpaid branch can now set this date directly.
The closed branch now considers whether the loan was overpaid going into the current transaction processing, if prior the loan was overpaid and now it is simply closed it means that the current transaction is takins some sort of excess away. In this case the obligationMetOnDate is assigned outright, meaning they can move "earlier" as the loan is falling back onto a settlement that already happened.
Otherwise the event is what settled the loan, and it may not have been a repayment. Each date only moves later, never earlier, so a settlement completed by a non-repayment keeps the transition's own 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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Fix COB amortizing discount fee before payment, fix WC loan closedOnDate wrong when backdated transaction closes [fineract]
Posted by "Cocoa-Puffs (via GitHub)" <gi...@apache.org>.
Cocoa-Puffs commented on PR #6469:
URL: https://github.com/apache/fineract/pull/6469#issuecomment-5793607306
Both findings are correct. During the retest after the previous round of findings a new scenario was found in which case the closing date turned out to be wrong. This was added as a deliberate test with testrail id: @TestRailId:C106757.
The issue was that an overpaid loan with credit balance refund set the maturity and closed on dates as the date of the CBR, which was considered wrong, as a CBR hands back money that the loan never needed, so it is not what settled it. The solution was to take the latest transactions date the paid into any outstanding amount, instead of 100% overpayment. That's what made the closed on date move in @TestRailId:C98268 as well.
Unfortunately this introduced a regression, while a Discount Fee Adjustment, while not a repayment removes part of the obligation, therefore can be the transaction that closes or settles the loan.
The fix: Two changes in `WorkingCapitalLoanTransactionProcessor.java`.
`recalculateMaturityOnOverpaid` now only moves the date later, and the already-settled branch of `recalculateClosure` is floored at the maturity the loan already carries.
A floor rather than a direction ban, because the undo case still needs closedOnDate to move earlier — from the undo day back to the settlement it falls onto. The javadoc on both now says why: obligationsMetOn names the last payment the loan needed, which is the day obligations were met only when a payment is what met them, and the stamp is the only record of a settlement completed by something else.
I have also added extra validations to @TestRailId:C83084 to pin down this behaviour.
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Fix COB amortizing discount fee before payment, fix WC loan closedOnDate wrong when backdated transaction closes [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #6469:
URL: https://github.com/apache/fineract/pull/6469#issuecomment-5792675978
@Cocoa-Puffs Can you please review the below concerns?
**Regression?**
- recalculateMaturityOnOverpaid moves the maturity earlier when a non-repayment settles the loan
obligationsMetOn answers "the last repayment the loan needed", which equals "the day obligations were met" only when a repayment is what met them. recalculateClosure guards against that — it keeps the one-directional rule precisely because settlement can be completed by "a discount fee adjustment that reduces what is owed to what has already been paid". recalculateMaturityOnOverpaid has no such guard and assigns "in either direction".
Existing scenario WorkingCapitalDiscountAmortizationAdjustment.feature UC9 hits it exactly:
- 02 Jan: repayment 9900 against 10000 owed → ACTIVE, still owes 100.
- 03 Jan: discount fee adjustment of 500 drops what's owed to 9500 → OVERPAID by 400. Reprocessing reallocates the 02 Jan repayment to 9500 principal + 400 overpayment, so overpaymentPortion (400) < transactionAmount (9900) and it qualifies as "had a due portion".
- determineAndTransition correctly stamps maturedOnDate = 03 Jan (it was null). recalculateSettlementDates(loan, ACTIVE) then takes the overpaid branch and overwrites it with 02 Jan — a day the loan was ACTIVE and owed 100.
- 04 Jan: the CBR closes the loan. statusBeforeEvent is OVERPAID, so the bidirectional branch in recalculateClosure sets both closedOnDate and maturedOnDate to obligationsMetOn = 02 Jan.
Published timeline for UC9 goes from closedOnDate 2026-01-04 / actualMaturityDate 2026-01-03 to 2026-01-02 / 2026-01-02. UC9 asserts status, balances and transactions but no timeline fields, which is why CI stays green. DISCOUNT_FEE_ADJUSTMENT (46) is not in getRepaymentLikeTransactionTypes() — I checked it isn't aliased to CHARGE_ADJUSTMENT (26) — so the query can never name the day that actually settled the loan.
Working through the reachable transitions, the bidirectional assignment in recalculateMaturityOnOverpaid never helps: the case its javadoc cites ("an extra payment onto an already closed loan") is already covered upstream by determineAndTransition's if (getMaturedOnDate() == null) guard, and the backdated-overpayment case (UC18) only needs the date moved later. The same one-directional rule recalculateClosure uses would be correct in every one of them. The already-settled branch of recalculateClosure needs the same treatment — it can't fall back below a maturedOnDate that already records when obligations were met.
Additional scope introduced?
the credit-balance-refund behaviour change
The CBR path is new in this revision and isn't covered by either title claim. It changes a published API field: WorkingCapitalLoanRepaymentOverpayment.feature UC6 had its timeline.closedOnDate expectation edited from 2026-01-06 (the refund day) to 2026-01-04 (the day obligations were met), and loan.setMaturedOnDate(transactionDate) became conditional. This also diverges from core, where DefaultLoanLifecycleStateMachine stamps closedOnDate from the CBR's own date. The reasoning in the javadoc is coherent, but closedOnDate now names a day on which the loan's status was OVERPAID rather than closed — worth explicit product sign-off rather than riding along on a COB-dating fix.
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org