alberto-art3ch commented on PR #6604: URL: https://github.com/apache/fineract/pull/6604#issuecomment-6082447708
> @alberto-art3ch Please review the failing tests and the below concern: > > **The gap: undoing the reset.** > > * `activeRestartResetDates` drops a reset once it's undone (`endDate != null`). On breach that's correct, because undo merges the split back together. > * Delinquency undo doesn't merge it back. The PR's own doc says so: on undo, "Period dates are not changed: a period the reset cut stays cut, and the period the reset started stays in place." > * So after an undo the boundary is still in the schedule, but the code has stopped protecting it. > > **Reproduced (resume):** pause 11–15 Jan, reset with `startNewPeriod` on 12 Jan, undo on 13 Jan, then resume on 13 Jan. A probe test added to the PR's own `WorkingCapitalLoanDelinquencyResetInsidePauseTest` setup prints: > > ``` > p2=2026-01-07..2026-01-11 p3=2026-01-10..2026-01-19 > ``` > > Period 3's start moved back from 12 to 10 Jan, so it overlaps period 2 on 10–11 Jan. The PR leaves this sequence open: undo and resume both only require the business date. > > **Traced but not run (reschedule):** with the same setup, a reschedule after the undo counts the whole pause again from 11 Jan. Under the same 6-day frequency, period 3 would end on 22 Jan instead of 21 Jan. The extra day is 11 Jan, which already belongs to period 2. > > **Suggested fix:** for delinquency, treat every `startNewPeriod` reset as a boundary, undone or not, since the cut it made stays. The unit test `activeRestartResetDates_keepsOnlyActiveResetsThatStartedANewPeriod` currently asserts the opposite and would need to change. An E2E scenario for pause → reset → undo → resume would cover it. Thanks @adamsaghy, good catch. You're right: unlike breach, the delinquency undo only clears the reset flags and never merges the cut period back, so the boundary has to keep holding after the undo. _Changes_: - restartResetDates (renamed from activeRestartResetDates) now returns every startNewPeriod reset, undone or not. Generation, resume and reschedule all go through it, so both of your cases are covered. Breach is unchanged. - Unit tests: inverted the assertion you pointed out, added your resume repro (pause 11–15, reset 12, undo 13, resume 13 → p2 07–11, p3 12–19, no overlap) and a reschedule-after-undo case (ends on 19 Jan, not 20). - E2E UC20: pause → reset with new period → undo → resume. - Docs: the delinquency chapter now states that the reset date stays a boundary after an undo. -- 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]
