adamsaghy commented on PR #6588:
URL: https://github.com/apache/fineract/pull/6588#issuecomment-6129407837
@Cocoa-Puffs Can you please review the below concerns?
### 1. Re-aged installments ignore `FIRST_DAY_OF_NEXT_MONTH` unless interest
recalculation is on
The PR passes the strategy into one re-age path,
`handleReAgeWithInterestRecalculationEnabled`
(`AdvancedPaymentScheduleTransactionProcessor` ~L4276). That path builds a
temporary schedule from `LoanApplicationTerms`, so its dates move forward as
expected.
The other re-age paths still compute dates with
`ScheduledDateGenerator.getRepaymentPeriodDate(...)`, which is a plain
`plusMonths` and doesn't know about the strategy:
- **Default re-age** (`handleReAgeWithCommonStrategy`),
`AdvancedPaymentScheduleTransactionProcessor` ~L4162:
`scheduledDateGenerator.getRepaymentPeriodDate(freqType, freqNumber * i,
startDate)`
- **Equal-amortization re-age**,
`ProgressiveEMICalculator.updateModelForReageEqualAmortization` ~L2320, same
call.
- **Due-date change on re-aged installments**,
`handleDueDateChangeOnRepaymentPeriod` (`dateShiftOnly` branch) and
`shiftReAgedInstallmentsAfterReschedule`: `newDueDate.plus(offset * repayEvery,
MONTHS)`.
**Example:** Loan with `FIRST_DAY_OF_NEXT_MONTH`, re-aged with start date 31
Jan, monthly, 3 installments.
- Interest recalculation off: **31 Jan, 28 Feb, 31 Mar**. February is pulled
back to the 28th; the setting is silently ignored.
- Interest recalculation on: **31 Jan, 1 Mar, 31 Mar**.
So the same loan setting gives different due dates depending on an unrelated
flag. The ticket asks to shift *all* due dates, so re-aged installments should
follow the setting too.
**Suggested fix:** Use `MonthlyDueDateResolver.plusMonths(startDate, n,
strategy)` for monthly stepping in these places. A strategy-aware overload of
`getRepaymentPeriodDate` would also work. Then add one re-age scenario to
`MonthEndDueDateStrategyTest`; none of the 18 IT methods or 12 E2E scenarios
cover re-aging.
- **Empty PR description.** It's the unfilled template. Please summarise the
behaviour, including that meeting-calendar loans ignore the setting, plus the
new API field and the DB migration.
- **E2E UC9 changes a shared global product** and changes it back at the end
of the scenario. If an assertion fails in between, the product keeps
`FIRST_DAY_OF_NEXT_MONTH` and later scenarios using it can fail. Safer to
create a product just for this scenario.
--
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]
