Skip to content

FINERACT-2455: WC - Allow backdated discount fee transaction - #6496

Draft
oleksii-novikov-onix wants to merge 1 commit into
apache:developfrom
openMF:FINERACT-2455/wc-backdated-discount-fee
Draft

oleksii-novikov-onix wants to merge 1 commit into
apache:developfrom
openMF:FINERACT-2455/wc-backdated-discount-fee

Conversation

@oleksii-novikov-onix

@oleksii-novikov-onix oleksii-novikov-onix commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Description

Behaviour change: command=discountFee used to ignore transactionDate and always dated the discount fee on the disbursement date. transactionDate is now validated:

  1. a date different from the disbursement date gets 400 validation.msg.wc.loan.transaction.date.must.be.equal.disbursement.date;
  2. without relatedResourceId, a date with no active disbursement gets 400 validation.msg.wc.loan.disbursement.transaction.not.found;
  3. an unparsable date gets 400 instead of being ignored.

Requests without transactionDate work as before, and the business date no longer has to equal the disbursement date.

Unchanged limits: a discount fee is accepted only on ACTIVE loans that are not charged off, and not when the disbursement date falls in a closed accounting period.

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.
  • I followed the AI Policy.

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

@galovics galovics left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a blocking bug, and I checked the risky parts. The discount fee is always dated on the related disbursement's date (a supplied transactionDate must equal it, so it can never be dated after the business date), charged-off loans are still rejected before anything is written, and isOpen() only returns true for ACTIVE, so closed/overpaid loans are rejected too - which is why not calling reprocessing here is safe (a higher principal on an ACTIVE loan with no overpayment doesn't change how later repayments were allocated). applyDiscountFeeAdjustment rebuilds the schedule from the payments already on it, so a repayment after disbursement is kept, the backdated DISCOUNT_FEE and the catch-up amortization on the next COB have the right journal entries in UC3, and undo still only goes through undo disbursal.

Things to settle before it leaves draft:

  • GL closure. This is the first PR that backdates a discount fee journal entry (before it could only land on today's business date). postDiscountFeeDeferralEntries does call checkForBranchClosures for accrual products, so backdating into a closed period is blocked there - but nothing in the WC module checks closures for other paths, so please confirm the check covers this entry and add an e2e case for a disbursement on or before the latest branch closure.
  • No unit tests for the new lookup. WorkingCapitalLoanWritePlatformServiceImpl ~L553-562 has four outcomes (id only, id plus matching date, id plus a different date, date only with no active disbursement on that date) and the PR only deletes the validator test. The Optional.map().or().filter() chain also hides which check failed - "no disbursement on that date" and "date mismatch" share one error code.
  • The negative e2e cases skip the likeliest client mistake. UC2 only tries 31 December 2025 (before disbursement); there's no case sending the later business date (08 January), with or without relatedResourceId.
  • transactionDate was already an allowed parameter but silently ignored; now a mismatching date is rejected with a 400. That's arguably right, but call it out in the ticket/release notes.
  • findActiveByTypeAndTransactionDate returns an Optional, so two active disbursements on one date would 500. Can't happen while a loan is disbursed once, but it's a latent bug if multi-tranche WC lands.
  • Question: makeDiscountFeeAdjustment reprocesses when there's an overpayment above zero but makeDiscountFee never does - I couldn't find a path that gives an ACTIVE loan a leftover overpayment (a reopen, maybe), just flagging it. The charge-off rule also differs (a discount fee before the charge-off is rejected, an adjustment before it is allowed) - a sentence in the doc would explain why.

Recommendation: COMMENT

@oleksii-novikov-onix
oleksii-novikov-onix force-pushed the FINERACT-2455/wc-backdated-discount-fee branch 2 times, most recently from a9504c8 to f22cbcd Compare September 25, 2026 07:28
@oleksii-novikov-onix

Copy link
Copy Markdown
Contributor Author

I couldn't find a blocking bug, and I checked the risky parts. The discount fee is always dated on the related disbursement's date (a supplied transactionDate must equal it, so it can never be dated after the business date), charged-off loans are still rejected before anything is written, and isOpen() only returns true for ACTIVE, so closed/overpaid loans are rejected too - which is why not calling reprocessing here is safe (a higher principal on an ACTIVE loan with no overpayment doesn't change how later repayments were allocated). applyDiscountFeeAdjustment rebuilds the schedule from the payments already on it, so a repayment after disbursement is kept, the backdated DISCOUNT_FEE and the catch-up amortization on the next COB have the right journal entries in UC3, and undo still only goes through undo disbursal.

Things to settle before it leaves draft:

  • GL closure. This is the first PR that backdates a discount fee journal entry (before it could only land on today's business date). postDiscountFeeDeferralEntries does call checkForBranchClosures for accrual products, so backdating into a closed period is blocked there - but nothing in the WC module checks closures for other paths, so please confirm the check covers this entry and add an e2e case for a disbursement on or before the latest branch closure.
  • No unit tests for the new lookup. WorkingCapitalLoanWritePlatformServiceImpl ~L553-562 has four outcomes (id only, id plus matching date, id plus a different date, date only with no active disbursement on that date) and the PR only deletes the validator test. The Optional.map().or().filter() chain also hides which check failed - "no disbursement on that date" and "date mismatch" share one error code.
  • The negative e2e cases skip the likeliest client mistake. UC2 only tries 31 December 2025 (before disbursement); there's no case sending the later business date (08 January), with or without relatedResourceId.
  • transactionDate was already an allowed parameter but silently ignored; now a mismatching date is rejected with a 400. That's arguably right, but call it out in the ticket/release notes.
  • findActiveByTypeAndTransactionDate returns an Optional, so two active disbursements on one date would 500. Can't happen while a loan is disbursed once, but it's a latent bug if multi-tranche WC lands.
  • Question: makeDiscountFeeAdjustment reprocesses when there's an overpayment above zero but makeDiscountFee never does - I couldn't find a path that gives an ACTIVE loan a leftover overpayment (a reopen, maybe), just flagging it. The charge-off rule also differs (a discount fee before the charge-off is rejected, an adjustment before it is allowed) - a sentence in the doc would explain why.

Recommendation: COMMENT

  1. GL closure: Covered. The discount fee journal entry is checked against the branch closure on its backdated date, and without accounting nothing is posted. New e2e: branch closed on the disbursement date gives 403 and nothing is saved.

  2. Unit tests: Added for your four cases plus two more, with the lookup moved into its own method. "No disbursement on that date" and "date mismatch" now return separate errors.

  3. Negative e2e: Added to UC2: 08 January on business date 08 January, with and without relatedResourceId, each with its own error.

  4. Release notes: In the PR description. transactionDate used to be ignored; now a wrong or unparsable date gets 400. Requests without it work as before, just no longer limited to the disbursement day.

  5. Two disbursements on one date: Agreed, though it would be a generic 403 data.integrity.issue rather than a 500. Today a loan has only one active disbursement and a per-loan discount, so we'll revisit the lookup with multi-tranche rather than silently pick a tranche now.

  6. Overpayment: No such path: the status check after every money movement moves any loan with an overpayment to OVERPAID, and discountFee is ACTIVE-only. Undo of a discount adjustment skips reprocessing for the same reason. Caveat: with a not-yet-due charge, a bigger principal can change an earlier repayment's split, but develop already does the same on that undo, so it's out of scope.

  7. Charge-off: Added to the doc. The discount fee path doesn't replay the charge-off, so it would leave principal the charge-off never wrote off, and it stays rejected as before this PR. A backdated adjustment only lowers the discount, and the loan is reprocessed so the charge-off is restated.

@oleksii-novikov-onix
oleksii-novikov-onix force-pushed the FINERACT-2455/wc-backdated-discount-fee branch from f22cbcd to fb35ab1 Compare September 28, 2026 08:36

@galovics galovics left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Thanks @oleksii-novikov-onix, all of my points are covered. The branch closure e2e, the unit tests for the lookup with separate error codes, the 08 January negative cases, the release note in the description and the charge-off explanation in the doc all look good. I'm fine with leaving the multi-tranche lookup for later.

LGTM once it's out of draft.

Recommendation: APPROVE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants