[Abhishek6122008]

Description

FINERACT-2909

When the Post Interest For Savings job posts interest on an account that is overdrawn at the posting date, part of the interest pays down the overdraft. The job already works out that part (overdraft_amount_derived) but booked the whole amount to Savings Control, so Overdraft Portfolio Control was never reduced and Savings Control was overstated by the same amount.

The job now splits such a posting like the manual path does:

• the part that pays down the overdraft is credited to Overdraft Portfolio Control
• the rest is credited to Savings Control

The job's debit side doesn't change: cash-based products debit Interest on Savings, accrual products debit Interest Payable, for both parts. Overdraft interest postings are not split.

Accrual products keep debiting Interest Payable because the expense was already booked when the interest accrued (debit Interest on Savings, credit Interest Payable); posting only moves it from Interest Payable to the account. The job did this before the change, and AccrualBasedAccountingProcessorForSavings does the same for an account in credit. Its overdraft branch debited Interest on Savings instead, which booked the expense twice. Reversals rebuild the entries from the processor's rules, so its overdraft branch now debits Interest Payable too; otherwise a job posting couldn't be reversed cleanly. The decision is also on the JIRA.

Tests:

• SavingsSchedularInterestPosterJournalEntriesTest: partial and full overdraft, accrual debit, account in credit, overdraft interest not split
• AccrualBasedAccountingProcessorForSavingsTest: an interest posting that pays down an overdraft, and its reversal, come out of Interest Payable
• SavingsAccrualAccountingIntegrationTest: both debits and credits of the job's split for an account overdrawn at the posting date, and reversing such a posting with a backdated deposit nets every GL account to zero, cash-based and accrual. The helper takes the first interest posting instead of the newest, so it doesn't depend on the second small posting the job made for the overdrawn days (FINERACT-3017, fixed in FINERACT-3017: Compound both parts of the overdraft interest split together #6782).

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.

[adamsaghy]

Thanks for the rebase. Two things are still missing before this can be merged:

1. PR description: it is still the empty template. Please describe what changes and why: the Post Interest job now splits an interest posting that pays down an overdraft, crediting the overdraft part to Overdraft Portfolio Control and the rest to Savings Control.
2. The open accounting question from FINERACT-2909: the ticket asks which account to debit on accrual products, and right now that is settled only in a code comment. The job debits Interest Payable for both parts, while the manual path (AccrualBasedAccountingProcessorForSavings) debits Interest on Savings, so the two paths will keep producing different entries. Please record the decision on the JIRA (and in the PR description), so reviewers can confirm the intended behaviour.

[Abhishek6122008]

@adamsaghy thanks! I filled in the PR description and recorded the accrual decision on FINERACT-2909: the job debits Interest Payable for both parts, since the expense was already booked when it accrued. The manual path's overdraft branch debits Interest on Savings, so it books the expense twice. If you agree I'll open a separate ticket for that.

I also fixed the failing shard 10. The accrual test picked the newest interest posting, but the job also posts a small second posting for the days the account was overdrawn. That's a separate bug already on develop, so I filed FINERACT-3017 and the test now takes the first posting.

[adamsaghy]

@Abhishek6122008 Please rebase

[galovics]

LGTM

[adamsaghy]

Thanks for the description and the JIRA note. The split itself looks right, and cash-based products now match the manual path, reversals included. One problem remains on accrual products:

1. A job-posted split posting can't be reversed cleanly on accrual products. Reversals don't go through the job. A backdated transaction or an undo sends the reversed posting through SavingsAccount.deriveAccountingBridgeData to AccrualBasedAccountingProcessorForSavings with isReversal=true. The processor rebuilds the entries from its own rules and swaps debit and credit; it does not copy the original entries. For a posting with overdraft_amount_derived > 0 its rule debits Interest on Savings for both parts, while the job debited Interest Payable.

   Example: overdrawn by 10, the job posts 19.18:

   • job: CR Overdraft Portfolio 10 / DR Interest Payable 10, CR Savings Control 9.18 / DR Interest Payable 9.18
   • reversal: DR Overdraft Portfolio 10 / CR Interest on Savings 10, DR Savings Control 9.18 / CR Interest on Savings 9.18

   Afterwards Interest Payable stays debited by 19.18 and the Interest on Savings expense is understated by 19.18. So deferring the manual path to a separate ticket isn't safe: the entries this PR writes can't be reversed correctly until that ticket lands. Please align the overdraft branch of AccrualBasedAccountingProcessorForSavings in this PR, so that it debits Interest Payable as you describe.

2. Tests: please add a case that reverses a job-posted split posting (for example a backdated deposit before the posting date) and checks that every GL account nets to zero, for both cash-based and accrual products. Both new integration tests also check only the debit for the overdraft part. Please also assert the debit for the remainder that goes to Savings Control.

3. Optional, for the follow-up: the job keeps its own copy of the savings GL rules (selectAccountId plus batchUpdateJournalEntries), and that copy has already drifted from the accounting processors, which is how FINERACT-2909 happened. A shared helper that turns a transaction into its debit and credit legs, used both by the processors and by the job, would keep them from drifting again.

[Abhishek6122008]

Thanks, you're right: reversals rebuild the entries from the processor's rules, so the job's postings couldn't be reversed cleanly on accrual products.

1. AccrualBasedAccountingProcessorForSavings now debits Interest Payable for both parts of an interest posting that pays down an overdraft, so a reversal mirrors the job's entries. AccrualBasedAccountingProcessorForSavingsTest checks the accounts for the posting and for its reversal.
2. Added reversal tests for cash-based and accrual products: the job posts the split, a backdated deposit reverses it, and the test checks that every GL account nets to zero over the posting's entries. The two existing tests now also assert the debit for the remainder.
3. Agreed, I can open a follow-up ticket for a shared helper. It could also cover one more case: since reversals rebuild the entries with the current rules, a posting written under older rules (a job posting from before this PR, or a manual one from before this change) still won't net to zero when reversed. Reversing the stored entries instead of rebuilding them would fix that for good.

Could you approve the workflow runs?

[Abhishek6122008]

The one failing job (PostgreSQL shard 4) is FeignWorkingCapitalLoanAmortizationNormalizationTest, which failed on a random 4-character product short name that was already taken. It's unrelated to this change, and the savings accrual ITs pass on all three databases. Could someone re-run that job?

[adamsaghy]

Thanks, this addresses everything. The processor now debits Interest Payable for both parts. The reversal tests would have caught the old mismatch, and the remainder debits are asserted.

Please open the follow-up ticket for the shared helper and for reversing the stored entries instead of rebuilding them. Also add a short upgrade note on FINERACT-2909: manual overdraft interest postings made on accrual products before this change won't net to zero if reversed afterwards.

The remaining PostgreSQL shard 4 failure was unrelated (FeignWorkingCapitalLoanAmortizationNormalizationTest), and its re-run is in progress.

[Abhishek6122008]

Thanks adam!! Added the upgrade note on FINERACT-2909, and opened #6817 for FINERACT-3047: interest postings now come from one shared set of rules for the processors and the job, and reversals undo the stored entries instead of rebuilding them.