Uh oh!
There was an error while loading. Please reload this page.
FINERACT-2455: WC - Introduce Loan OverpaidOnDate - #6346
Conversation
78b6572 to
f7a198aComparef7a198a to
922689fCompare
galovics
left a comment
There was a problem hiding this comment.
CI is red on this one (3 e2e failures in the new scenarios), and I think the failures are pointing at real gaps rather than flakes.
overpaidOnDate is only ever written at the moment of a status change, via WorkingCapitalLoanLifecycleStateMachine. Two consequences:
- UC11 (backdated repayment that tips the loan into overpayment): the date gets stamped with the transaction date passed in, not the date the loan actually became overpaid -
expected: "2026-01-06" but was: "2026-01-03". - UC12 (undoing one of two overpaying repayments, leaving the loan still OVERPAID): since
determineAndTransitiononly acts when the status actually changes, and the loan stays OVERPAID after the undo, the date never gets recomputed and the stale value survives -expected: "2026-01-05" but was: "2026-01-03".
Core Loan's own lifecycle state machine (DefaultLoanLifecycleStateMachine.determineAndTransition) has an unconditional guard - if (isEmpty(totalOverpaid)) setOverpaidOnDate(null) - that runs independent of whether a transition fires. This WC version ties clearing exclusively to a transition, which is the root of both failures. Worth mirroring that guard, and deciding explicitly whether the semantics should be "date of the triggering transaction" (current impl) or "date first became overpaid, recomputed from history" (what the tests assert) - right now the code and the tests disagree.
- UC13: the business event never carries the new field -
WorkingCapitalLoanAccountDataMapperstill has@Mapping(target = "overpaidOnDate", ignore = true), which predates this PR adding the field toWorkingCapitalLoanData. Thatignoreneeds to go now that there's something to map.expected: "2026-01-04" but was: null.
Two more things worth fixing before merge:
- No backfill for loans that are already in OVERPAID status today - they'll report
nullforever since nothing will ever re-trigger a transition for them. The module has a precedent for add-column-plus-backfill in the same part file (0070_wc_loan_principal_balance_backfill.xml) - worth following that here. - No unit test for
WorkingCapitalLoanLifecycleStateMachineat all (core has one for its equivalent). Given the date semantics are non-trivial across a dozen transitions, that would catch this class of bug faster than a 25-minute e2e shard.
Recommendation: CHANGES_REQUESTED
756001a to
78b5a7aCompare78b5a7a to
9f110afCompare
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!
Your assigned reviewer(s) will follow our guidelines for code reviews.