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

Copy link
Copy Markdown
Contributor

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.
  • 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

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