budaidev commented on PR #6283: URL: https://github.com/apache/fineract/pull/6283#issuecomment-5545540361
> **Can you please verify the below use cases?** > > WorkingCapitalLoanBreachScheduleServiceImpl.java:277 : a PAUSE recorded after a restart reset makes recalculatePeriodsForPauses re-date every period from the schedule start, wiping the split while leaving reset=true on a period that no longer contains the reset date. Probe: P2 goes back to 70 days and P3 shifts to [2026-05-11..2026-07-09] still flagged, so past due is computed from the wrong row. Pre-existing code, but it makes the split non-durable and this PR is what gives the split meaning: the pause/resume handler should re-derive flags via the new applyResetFlags. WorkingCapitalLoanBreachResetServiceImpl.java:53; PLAUSIBLE (also line 76): applyResetFlags runs before reprocessBreachSchedule regenerates periods, so an active reset whose date falls outside the mid-operation schedule gets pinned to the wrong row by resolveEvaluationPeriod's fallback. Needs a backwards business-date move to trigger, which the activeResets javadoc explicitly says can happen. Apply the flags after the schedule is final. 1. PAUSE after a restart reset (`recalculatePeriodsForPauses`)** - confirmed, exactly your numbers. `recalculatePeriodsForPauses` is unchanged on this branch (identical to develop), so the erasure is pre-existing, but agreed that this PR is what gives the split meaning. Fixed: the pause recalculation now re-derives the flags (via `applyResetFlags` over the persisted actions) before it recalculates the past due, so the flag and the past-due anchor follow the reset date to its re-dated row. applyResetFlags fixed by moving the derivation into the schedule service, which is the one reshaping the schedule: `reprocessBreachSchedule` derives the flags right after it generated the periods up to the business date and before it replays the payments and recalculates the past due. The reset service no longer derives flags ahead of the split/restore; on the flag-only paths (plain reset, its undo) it asks the schedule service to derive them and recalculates the past due. One derivation, always over the persisted actions, always on the final schedule. The same re-derivation now also runs after a reschedule re-dates the periods and after `generateNextPeriodIfNeeded` created periods (COB, charge extension), so a reset placed on the last period by the fallback moves to its containing period once that period exists. > > Some minor: > > restoreSplitPeriod filters reschedules to those recorded before the reset (beforeActionId) but passes the unfiltered pause list to applyPauses. That happens to be consistent today because a pause already re-dates all periods, but the asymmetry is worth a comment. The WorkingCapitalLoanAccount.feature scenario C102391 (modify with invalid product id) and its step def are unrelated to breach reset undo. I traced the path and the expectation holds: validateForUpdate collects the longGreaterThanZero() error but productRepository.findById(...).orElseThrow(WorkingCapitalLoanProductNotFoundException) fires first, so 0 and -1 do return 404, but it's scope creep in a PR the checklist asks not to be a code dump. WorkingCapitalBreachReset.feature also lost its trailing newline (not spotless-enforced for .feature, so cosmetic only). - C102391 (invalid product id) and its step definition: dropped from this PR. QA can re-add it on its own ticket. - Trailing newline in `WorkingCapitalBreachReset.feature`: restored. -- 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]
