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]

Reply via email to