adamsaghy commented on PR #6635: URL: https://github.com/apache/fineract/pull/6635#issuecomment-6192825739
@mariiaKraievska Please review below concerns: The new seed date is still not always the one the loan's schedule was generated from, though. I traced the date logic by hand and did not run anything. ### 1. Seed ignores `repaymentsStartingFromDate`: new bug for the 28th, 29th–31st still not fixed `ProgressiveEMICalculator.java:224` `getStartDate()` returns the first period's `fromDate`, which is the submitted-on or disbursement date. When `repaymentsStartingFromDate` is set, `LoanApplicationTerms` seeds the loan's own schedule from that date instead (`LoanApplicationTerms.java:840-845`). - **Newly broken:** submitted 31 Jan 2026 with first repayment date 28 Feb 2026. The real seed day is 28, so due dates fall on 28 Mar, 28 Apr, and so on. A tranche on 10 Apr falls in the period 28 Mar → 28 Apr. The new seed is 31 Jan, so `adjustDate` moves 28 Apr to 30 Apr. The (fromDate, dueDate) match fails and a full duplicate schedule is appended. Before this PR the seed was 28 Mar, which gives 28 Apr and merges correctly. - **Still broken:** submitted on the 15th with first repayment date on the 31st. The seed day is 15, so no month-end adjustment happens. The tranche gets 30 Oct instead of 31 Oct and the schedule is duplicated again. ### 2. Seed ignores the re-aged anchor date `ProgressiveEMICalculator.java:224` `addRepaymentPeriods` (line 363) uses the re-age `startDate` as its seed for this reason, but the full-term tranche path has no such handling. Example: the loan was originally anchored on the 31st and re-aged so installments fall on the 28th. A tranche in a re-aged period now seeds from the 31st, so 28 Apr becomes 30 Apr and the schedule is duplicated. Before this PR the seed was the period's `fromDate` (the 28th), which merged correctly. The reverse case, a 15th anchor re-aged to the 31st, never gets the month-end adjustment. ### 3. Underlying cause: the merge appends a whole schedule on any date mismatch `ProgressiveEMICalculator.java:280` `mergeNewScheduleModelWithExistingOne` matches periods only by exact `(fromDate, dueDate)` and silently appends every period that doesn't match. Any difference in date generation therefore produces 2×N installments, with `fromDate > dueDate` on the first appended one. Causes include the seed issues above, loan term variations, and rescheduled due dates. **Suggestion:** reuse the existing periods' due dates for the overlapping range, and generate only the extra installments at the end with `resolveRepaymentPeriodLengthGeneratorFunction` / `generateAdditionalRepaymentPeriodDueDates`, as `addRepaymentPeriods` already does. Matching by position from `firstDisbursedPeriod` would also work. Either way, the result stops depending on regenerating identical dates from a seed. ### 4. No unit test coverage `ProgressiveEMICalculator.java:233` `ProgressiveEMICalculatorTest` already has full-term tranche tests but gets no month-end case. Please add unit tests for: - the 31st anchor, and the 29th/30th anchors after Feb 28 - the `repaymentsStartingFromDate` and re-age variants above -- 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]
