adamsaghy commented on PR #6601: URL: https://github.com/apache/fineract/pull/6601#issuecomment-6129393065
@budaidev Please review below concerns: **1. A replay no longer clears `nearBreach` when a period moves, so stale values survive** `WorkingCapitalLoanBreachScheduleServiceImpl.java:373` (`applyResolvedDemand`) The old code set `nearBreach` to `null` whenever a replay changed a period's dates or minimum payment. Now that is left to `rederiveSince`, which keeps the old value whenever `resolveNearBreachValue` returns empty. Examples: - The open period has `nearBreach = true`. A breach RESCHEDULE shortens it so the first near-breach checkpoint (grace days + frequency) now falls after the new `toDate`. The result is empty, the value stays `true`, and the API shows a near breach for a period that has no checkpoint. - A pause moves the open period's `fromDate` to today. At `businessDate - 1` no checkpoint has passed, so the result is empty and the value from the old period carries over. - `rederiveSince` returns early when `resolveParameters` is empty (loan not ACTIVE), so periods that moved keep their old values. Suggestion: on rebuild paths where the period's inputs actually changed (`hasMoved`), clear to `null` when the result is empty instead of keeping the old value. **2. "Nothing to judge against" keeps the value even on paths where the inputs changed** `WorkingCapitalLoanNearBreachEvaluationServiceImpl.java:131` (`rederive`) This is the same root cause as #1, seen from the shared derivation. Example: after a breach ENABLE, `recalculateMinimumPayment` sets the open period's `minPaymentAmount` to 0, so the result is empty, and a `nearBreach = true` from before the DISABLE stays on a period that has no demand at all. Keeping the value makes sense for COB and repayments. It does not make sense for replay, reprocess or ENABLE. **3. Repayments assume COB has already run for `businessDate - 1`** `WorkingCapitalLoanNearBreachRederivation.java:83` (`nearBreachEffectiveDate`) The PR description says repayments are judged as of "the date COB evaluated". For a loan that is behind on COB (`lastClosedBusinessDate < businessDate - 1`, catch-up still pending), a repayment judges checkpoints COB has not processed yet and may fire `WorkingCapitalLoanNearBreachChangeBusinessEvent` for them. Consider using the loan's `lastClosedBusinessDate` so repayments and COB always use the same date. **4. Split/restore reprocess skips the breach-disabled check** `WorkingCapitalLoanBreachScheduleServiceImpl.java:444` `splitPeriodAtResetAndReprocess` and `restoreSplitPeriodAndReprocess` call the private `reprocessBreachSchedule(loan, baseline)` directly. That skips the `isBreachEvaluationDisabled` early return the old path got from the public `reprocessBreachSchedule(loan)`. The API can't reach this today, because the reset and undo-reset validator rejects these actions while breach is disabled. Still, it is a guard that was silently dropped. Please either add the check back or note why it isn't needed. **5. Redundant `isBreachDisabled` query in `rederiveSince`** `WorkingCapitalLoanNearBreachRederivation.java:69` `resolveParameters` runs `isBreachDisabled` again after `replayPeriodsFrom` and the public `reprocessBreachSchedule` have already checked it, so every replay and reprocess does an extra query. Passing a flag, or using the `WithBreachEvaluationEnabled` variant where the check already ran, would also let the two near-identical `resolveParameters*` methods on the public interface become one. **6. The before-snapshot needs an extra query of the whole schedule** `WorkingCapitalLoanBreachScheduleServiceImpl.java:498` (`nearBreachBaseline`) `nearBreachBaseline(loan)` runs `findByLoanIdOrderByPeriodNumberAsc`, and then `reprocessBreachSchedule` runs it again. On the plain reprocess path the snapshot could be built from the list that is already loaded, before `resetAllPeriodsForReprocessing` changes it. -- 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]
