You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fineract.apache.org by "Cocoa-Puffs (via GitHub)" <gi...@apache.org> on 2026/08/18 11:47:17 UTC
[PR] FINERACT-2455: Add final discount fee amortization on charge-off and reversal [fineract]
Cocoa-Puffs opened a new pull request, #6281:
URL: https://github.com/apache/fineract/pull/6281
## 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](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests)
- [ ] 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](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made.
- [ ] Follow our [coding conventions](https://cwiki.apache.org/confluence/display/FINERACT/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"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute).
- [ ] 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](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide).
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Add final discount fee amortization on charge-off and reversal [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Add final discount fee amortization on charge-off and reversal [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #6281:
URL: https://github.com/apache/fineract/pull/6281#issuecomment-5454721321
@Cocoa-Puffs Please rebase
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Add final discount fee amortization on charge-off and reversal [fineract]
Posted by "Cocoa-Puffs (via GitHub)" <gi...@apache.org>.
Cocoa-Puffs commented on PR #6281:
URL: https://github.com/apache/fineract/pull/6281#issuecomment-5395420366
**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.
- 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.
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2455: Add final discount fee amortization on charge-off and reversal [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #6281:
URL: https://github.com/apache/fineract/pull/6281
--
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: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org