alberto-art3ch commented on PR #6604: URL: https://github.com/apache/fineract/pull/6604#issuecomment-6159013120
> @alberto-art3ch Please review mine and @mariiaKraievska concerns: > > ### 1. The PR description, docs and feature file name different E2E scenarios > * **PR description:** "E2E scenarios UC17–UC19". > * **Docs:** `working-capital-delinquency-management.adoc` says "E2E scenarios UC17–UC20". > * **Feature file:** `WorkingCapitalDelinquencyReset.feature` actually adds UC17–UC26 and UC28. There is no UC27. > > The description and the docs sentence should both list the scenarios that were actually added. > > ### 2. A backdated pause still breaks the reset cut (the bug was already there before this PR) > `resetPeriods` (`WorkingCapitalLoanDelinquencyRangeScheduleServiceImpl.java:414-428`) always cuts the **last** period. It assumes the last period is the one that contains the reset date. A backdated pause can break that assumption, because the pause validator doesn't check the start date against the business date. > > **Example:** > > 1. Period 1 is 1–31 Jan. COB on 1 Feb generates period 2, 1 Feb–2 Mar. > 2. On 10 Feb a pause is recorded for 25 Jan–20 Feb (27 days). `extendPeriodsForPause` moves period 1 to 1 Jan–27 Feb and period 2 to 28 Feb–29 Mar. > 3. A reset with `startNewPeriod = true` is recorded on 10 Feb, while that pause is active. > > **Result:** > > * Period 2 is the one that gets cut. Its end becomes 9 Feb, which is before its start of 28 Feb, so it's an invalid period. It is flagged reset. > * Period 1 still runs to 27 Feb. Since that isn't before the reset date, it is **not** flagged reset. > * The new period 3 starts on 10 Feb, which is correct thanks to this PR. But it overlaps period 1 from 10 to 27 Feb, so the reprocess replay counts repayments in that window twice. > > The new period does start on the reset date, as the PR claims. But a reset inside an active pause still produces a broken schedule when the pause began in an earlier, already generated period. @adamsaghy 1) Renumbered UC28 → UC27, docs and description now list UC17–UC29. 2) Fixed here since it is the same flow: resetPeriods now cuts the period that contains the reset date instead of the last one. The later periods start after the business date, so they are discarded and replaced by the restarted period. Your example is covered by a unit test and E2E UC29. <img width="1329" height="316" alt="Screenshot 2026-10-13 at 8 39 26 PM" src="https://github.com/user-attachments/assets/a7ad20d5-59eb-4457-86ff-bb24189dd840" /> -- 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]
