adamsaghy commented on PR #6082: URL: https://github.com/apache/fineract/pull/6082#issuecomment-4892353967
> > In `WorkingCapitalLoanWritePlatformServiceImpl.java` / `WorkingCapitalLoanTransactionReprocessingServiceImpl.java`: > > ```java > > - transactionReprocessingService.reprocessTransactions(loan, transactionsToReprocess); > > - } > > + } > > + > > + transactionReprocessingService.reprocessAfterUndo(loan); > > ``` > > > > > > > > > > > > > > > > > > > > > > > > `reprocessAfterUndo` -> `reprocessScheduleFromTransactions` resets every delinquency period on the loan (paid/outstanding/criteria/delinquent amount all wiped) and replays every non-reversed repayment-like transaction from scratch, and it now runs unconditionally on every single undo, not just when the loan has actually been rescheduled. Was the old incremental `applyRepaymentUndo` provably wrong once multiple reschedules are possible, or is this just the simpler path? If it's the former a one-line comment saying so would help the next reader; if it's just simplicity, might be worth gating the full replay to loans with reschedule history. > > Also: `WorkingCapitalLoanDelinquencyRangeScheduleService.applyRepaymentUndo(WorkingCapitalLoan, LocalDate, BigDecimal)` has no callers left anywhere in the codebase after this change (the only call site got replaced above). Let's get rid of it, or keep it if I'm missing a caller. > > Small thing: the docstring you touched now reads "The breach schedule are intentionally not rebuilt" - singular/plural typo. > > @galovics Thanks for the review! > > **On reprocessAfterUndo running unconditionally**: I went with full replay because AC in my task says "Any backdated repayments or reversals should triger the reprocess of Delinquency schedule and evaluation" — so, as I understand the reprocess and evaluation should happen not only after reschedule, @adamsaghy please correct me if I`m wrong. That’s why it runs on every undo. Documented on reprocessAfterUndo. > > **On applyRepaymentUndo:** Agreed — no callers left. Removed it from the service/interface, along with the private helper and the unused repository method. > > **On the typo:** Fixed. Yeah, but only from the relevant period(s). We dont need full reprocessing or anything... Delinquency reschedule should not affect or reprocess any transactions. They are not related... -- 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]
