FINERACT-2455: WC - Transaction Type - Charge Waiver

[oleksii-novikov-onix]

Description

Describe the changes made and why they were made. (Ignore if these details are present on the associated Apache Fineract JIRA ticket.)

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

• Write the commit message as per our guidelines
• Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
• Create/update unit or integration tests for verifying the changes made.
• Follow our coding conventions.
• Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
• This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.
• If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately.

Your assigned reviewer(s) will follow our guidelines for code reviews.

[adamsaghy]

@oleksii-novikov-onix Please rebase

[galovics]

The feature itself is well-structured and follows the module's conventions closely - Liquibase is clean (insert-only, never deletes m_external_event_configuration, permissions mirror the existing charge-adjustment pair exactly), and the e2e coverage of waiver/undo/re-waive/charged-off combinations is genuinely thorough.

One real correctness issue I'd want fixed before this leaves draft: a waiver permanently mutates the balance (feeWaived/penaltyWaived), and WorkingCapitalLoanTransactionReprocessingServiceImpl.replayChargeOff recomputes the charge-off snapshot from that live balance state on every reprocess - but the waiver transaction itself is excluded from the replay and its own journal entries are never restated. Concretely: fee accrued -> charge-off (credits Fees Receivable) -> waiver -> a later backdated repayment triggers a reprocess -> the charge-off gets restated with the fee portion now zero, reversing its Fees Receivable credit, while the waiver's credit to INCOME_FROM_CHARGE_OFF_FEES stays untouched. Net: Fees Receivable is stranded and Fee Charge Off carries a one-sided credit. In the extreme case where the waiver zeroes total outstanding, the reprocess would silently lift the charge-off. Either the waived buckets need to participate in the chronological replay, or replayChargeOff needs to exclude waivers dated after the charge-off.

Related: the accounting routing for the waiver uses the raw loan.isChargedOff() flag rather than the date-ordering-aware transactionFinder.isAfterActiveChargeOffForAccountingRouting that every other posting site in the module uses. For a charge accrued before the charge-off this gives the right answer (and I see the deliberate comment explaining that choice), but chargeAccrualPostings doesn't gate on the charged-off flag at all - so a charge added and accrued after a charge-off still gets a live Fees Receivable debit, and waiving it would credit the charge-off income account while leaving that receivable on the books forever. Since the validator explicitly permits charges on charged-off loans, this looks reachable and worth an e2e case either way.

One more worth a look: undoChargeWaiver reverses the waiver's own journal entries and reopens the loan, but if the waiver had triggered triggerInlineAmortizationIfLoanClosed at closure time, the undo doesn't unwind that final lump-sum amortization the way undoDiscountFeeAdjustment/replayChargeOff do for their equivalent side effects.

Recommendation: CHANGES_REQUESTED

[oleksii-novikov-onix]

The feature itself is well-structured and follows the module's conventions closely - Liquibase is clean (insert-only, never deletes m_external_event_configuration, permissions mirror the existing charge-adjustment pair exactly), and the e2e coverage of waiver/undo/re-waive/charged-off combinations is genuinely thorough.

One real correctness issue I'd want fixed before this leaves draft: a waiver permanently mutates the balance (feeWaived/penaltyWaived), and WorkingCapitalLoanTransactionReprocessingServiceImpl.replayChargeOff recomputes the charge-off snapshot from that live balance state on every reprocess - but the waiver transaction itself is excluded from the replay and its own journal entries are never restated. Concretely: fee accrued -> charge-off (credits Fees Receivable) -> waiver -> a later backdated repayment triggers a reprocess -> the charge-off gets restated with the fee portion now zero, reversing its Fees Receivable credit, while the waiver's credit to INCOME_FROM_CHARGE_OFF_FEES stays untouched. Net: Fees Receivable is stranded and Fee Charge Off carries a one-sided credit. In the extreme case where the waiver zeroes total outstanding, the reprocess would silently lift the charge-off. Either the waived buckets need to participate in the chronological replay, or replayChargeOff needs to exclude waivers dated after the charge-off.

Related: the accounting routing for the waiver uses the raw loan.isChargedOff() flag rather than the date-ordering-aware transactionFinder.isAfterActiveChargeOffForAccountingRouting that every other posting site in the module uses. For a charge accrued before the charge-off this gives the right answer (and I see the deliberate comment explaining that choice), but chargeAccrualPostings doesn't gate on the charged-off flag at all - so a charge added and accrued after a charge-off still gets a live Fees Receivable debit, and waiving it would credit the charge-off income account while leaving that receivable on the books forever. Since the validator explicitly permits charges on charged-off loans, this looks reachable and worth an e2e case either way.

One more worth a look: undoChargeWaiver reverses the waiver's own journal entries and reopens the loan, but if the waiver had triggered triggerInlineAmortizationIfLoanClosed at closure time, the undo doesn't unwind that final lump-sum amortization the way undoDiscountFeeAdjustment/replayChargeOff do for their equivalent side effects.

Recommendation: CHANGES_REQUESTED

1. Charge-off snapshot vs the waived buckets

Fixed the second way you suggested. replayChargeOff adds back the waivers that sort after the charge-off, so the snapshot is the outstanding at the charge-off's own position in the replay. Ones sorting before it stay subtracted, as they already had when it was booked.

The waived buckets still survive the reset, as LoanCharge.amountWaived does on the term loan side. Resetting them would let a payment replayed before the waiver consume what was forgiven.

With date-aware routing a waiver dated to a past due date can land before an active charge-off, so booking one now passes the same reprocessing gate a backdated repayment does.

That covers the silent lift: a waiver sorting after the charge-off always puts back at least its own amount, so it cannot zero the snapshot. One sorting before it still can, but only when nothing was owed on that date, and now at booking with the charge-off-lifted event rather than inside a later reprocess.

2. Routing on isChargedOff instead of the finder

Fixed, isAfterActiveChargeOffForAccountingRouting now.

It was written the other way because the finder alone credits Fees Receivable for a waiver dated before a charge-off that had already relieved it, driving the account negative. It is safe only with the snapshot fix and the gate, so all three landed together.

The charged-off accounting scenario changed with it: the waiver there is dated on the due date, so it credits Interest/Fee Receivable, and the charge-off no longer carries the fee.

3. chargeAccrualPostings not gated on charged-off

The route as described is unreachable, createLoanCharge rejects new charges on a charged-off loan.

Reachable is a charge added before the charge-off and accrued after, and there the two net out: the charge amount enters the balance when the charge is added, so the charge-off credits the receivable whether or not it was accrued, and the accrual debits it back. New e2e asserts it.

Which is why the gate stays off: had the accrual debited the charge-off income account instead, the charge-off's credit of 100 would have nothing to match and Fees Receivable would sit at minus 100 permanently.

One variant is genuinely broken: waive before the accrual, charge off, undo. The charge-off saw a zero fee outstanding and never relieved the receivable, and the accrual after the undo debits it anyway. Fixed by reprocessing on undo, as undoDiscountFeeAdjustment does. The "Undoing a waiver on a charged-off loan" scenario covers it.

4. Inline amortization on undo

Left as is. The generic undoTransaction does not unwind it either, and both paths you compared reach their amortization through a transaction relation, while the closure one is linked to nothing. It self-corrects: processDiscountFeeAmortization targets schedule minus already posted, so the first COB after the loan reopens adjusts the excess.

The charged-off variant has nothing to correct, despite the COB step skipping charged-off loans: charge-off requires an active loan and recognizes the whole remaining discount then, so a later closure finds a zero remainder.

That leaves the window until that COB. Closing it in the waiver undo alone would still leave it for an undone repayment that closed the loan, so it belongs on the generic undo.

What changed

The new aggregate query, the snapshot in replayChargeOff, the finder for routing, and the reprocess gate on booking a backdated waiver and on undoing one. Two new e2e scenarios, one updated.

[adamsaghy]

@oleksii-novikov-onix Please rebase