adamsaghy commented on PR #6281: URL: https://github.com/apache/fineract/pull/6281#issuecomment-5452187451
> **Fixed (real bugs):** > > 1. **Lift-via-reprocessing never reversed the final amortization** — `fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanTransactionReprocessingServiceImpl.java#L366-380` now calls `discountFeeAmortizationService.undoDiscountFeeAmortizationOnChargeOff` before lifting, mirroring the explicit undo-charge-off path. > 2. **`undoDiscountFeeAdjustment` never reprocessed on a charged-off loan** — added the missing `transactionReprocessingService.reprocessTransactions(loan)` call in `fineract-working-capital-loan/src/main/java/org/apache/fineract/portfolio/workingcapitalloan/service/WorkingCapitalLoanWritePlatformServiceImpl.java#L714-719`, mirroring `makeDiscountFeeAdjustment`. > 3. **One-way door once reversed** — the relation lookup now finds the link regardless of reversed state, and `restateFinalDiscountFeeAmortization` can revive a previously-reversed amortization transaction (un-reverse, recompute amount, restate journal entries) when the pool comes back up. Added the repository method and revival branch in the same file. > 4. **UC49's reversed-CHARGE_OFF table didn't assert the reversal** — fixed the duplicated rows to actually flip debit/credit, matching UC48's correct pattern. I did _not_ touch the shared `checkJournalEntryData` step-def matching semantics (containment-based, non-consuming) that let this slip through silently — that's shared infra used across many unrelated feature files repo-wide, out of scope for this PR. Worth its own follow-up. > > **Also addressed:** > > * Point 2's example scenario (UC51) was asserting the stale, bugged state as correct — rewritten to show the fix (in-place restatement at undo time, no more stray catch-up transaction on close). > * Added UC52, a new e2e scenario with a non-zero discount pool exercising the lift path, per the reviewer's suggestion that no existing lift scenario covers this. (requires a testrail id) > * Point 5 (missing business events) — added `WorkingCapitalLoanDiscountFeeAmortizationTransactionBusinessEvent` on creation and `WorkingCapitalLoanTransactionReversedBusinessEvent` on reversal in `WorkingCapitalLoanDiscountFeeAmortizationServiceImpl`, matching what the periodic-amortization path already does. The in-place amount restatement itself stays event-silent, consistent with how the sibling charge-off amount restatement already behaves. We should not have reversal / restate where no business event is triggered. -- 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]
