Cocoa-Puffs commented on PR #6588: URL: https://github.com/apache/fineract/pull/6588#issuecomment-6167857297
I have reviewed / resolved the concerns. **1. Disbursement on the 1st unrolled.** I believe the logic should stay With `FIRST_DAY_OF_NEXT_MONTH` and anchor day 31, February’s due date is 1 Mar. That makes 1 Mar a regular point on the loan’s monthly grid, so 1 Mar → 31 Mar is one whole period. It doesn’t matter whether a due date was rolled there or the loan was disbursed there. The rest of the code treats it the same way: for a first due date of 31 Mar, idealDisbursementDateForMonthlyRepayments also gives 1 Mar. It’s also the whole-period rule the ticket confirmed for 30/360. 5.97 periods would be correct under `LAST_DAY_OF_MONTH`, where February’s due date is 28 Feb and 1 Mar → 31 Mar is a day short of a full period. The comment in `calculatePeriodsBetweenDates` suggested that only rolled due dates are affected, so I reworded it. **2. Rolled dates inferred from the 1st.** It’s chained by design: reschedules restart the chain, and the clamp keeps the original anchor after a reschedule to the 28th or later. Computing `seed + n × repayEvery` would put AC 1 (existing schedules unchanged) and AC 10 (variations as today) at risk. The holiday example can’t happen: with “reschedule to next repayment date”, `ApplyHolidaysToLoansTasklet` always starts from an existing due date of the loan, and in a roll-forward loan a 1st after a short month is a rolled date unless a reschedule put it there, which is already handled. Verified with a new API test, `testHolidayRescheduledToNextRepaymentDateKeepsRolledDates`. A holiday on 31 Mar gives 1 Mar, 1 May, 31 May, 1 Jul against 28 Feb, 30 Apr, 31 May, 30 Jun when clamped: every installment moves one slot forward on its own grid (AC 9). **3. Fixed-length max date on calendar loans.** Fixed. `calculateMaxDateForFixedLength` ignores the strategy when the loan has a meeting calendar (AC 8 and AC 11). While verifying this I found the strategy also leaked into progressive interest on calendar loans (30/360 whole-period counting), both at schedule generation and when transactions reprocess the loan. Both routes now ignore it for calendar-linked loans: - generation: `LoanApplicationTerms.toLoanConfigurationDetails`. - reprocessing: `LoanConfigurationDetailsMapper` with a new `LoanMeetingCalendarService`, which only queries when the strategy rolls forward. - the processor’s direct reads of the strategy go through the same rule. `testMeetingCalendarDecidesDueDatesOfCalendarLoans` now covers cumulative, progressive and progressive fixed-length calendar loans. It checks dates and interest, before and after a repayment. **4. Calendar loans store an ignored strategy.** Keeping both the stored value and the debug log, because the ticket asks for them: AC 8: “log at debug when the attribute is set but superseded by a calendar”; AC 15: the loan “inherits the product value at submission and reads it back”. Rejecting the combination was the alternative the ticket considered and decided against. With the fixes in point 3 the value now has no effect anywhere. The Swagger descriptions say it doesn’t apply to loans following a meeting calendar. **5. Product change keeps the old strategy.** Fixed. When a modify changes `productId` and doesn’t send `monthEndDueDateStrategy`, the loan takes the new product’s value; an explicit value still wins. Covered by `testChangingTheProductOfAnApplicationTakesItsStrategy`. **6. Variation scan before the strategy check.** Fixed. `unrollChainedDueDate` returns early unless the date is the 1st and the strategy rolls forward. **7. Duplicated plusPeriods.** Fixed. One `MonthlyDueDateResolver.plusPeriods` is used by the processor and the EMI calculator. It isn’t routed through `ScheduledDateGenerator`, because `LoanScheduleGeneratorTest` builds the EMI calculator with a mocked generator. **8. Duplicated validator.** Fixed. `LoanApplicationValidator` uses `LoanProductDataValidator.validateMonthEndDueDateStrategy`. Also found while verifying. Before disbursement, the first due date of a roll-forward loan was derived with plain `plusMonths` in `LoanScheduleAssembler.deriveFirstRepaymentDate`. Submitted loans and the `calculateLoanSchedule` preview showed 28 Feb instead of 1 Mar, which breaks AC 2/3. Fixed and covered by `testScheduleOfSubmittedLoanRollsForward`. -- 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]
