adamsaghy commented on PR #6604: URL: https://github.com/apache/fineract/pull/6604#issuecomment-6076636544
@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. -- 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]
