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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
