fix(deposits): stop FD Special interest postings from self-correcting after settlement
Fineract Build & Cucumber tests (without E2E tests) / verify (main, build-core) (push) Canceled after 0s
Fineract Build & Cucumber tests (without E2E tests) / verify (main, cucumber) (push) Canceled after 0s
Fineract Build & Cucumber tests (without E2E tests) / verify (progressive-loan, build-progressive-loan) (push) Canceled after 0s
Fineract Docker Builds / build (false, docker-compose-postgresql.yml, postgresql) (push) Canceled after 0s
Fineract Docker Builds / build (true, docker-compose.yml, mariadb) (push) Canceled after 0s
Fineract Documentation build / build (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 1 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 10 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 2 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 3 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 4 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 5 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 6 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 7 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 8 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 9 of 10) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MariaDB / test (test-core-2) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MariaDB / test (test-core-3) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MySQL / test (test-core-1) (push) Canceled after 0s
Fineract Messaging Smoke Tests / Smoke Test with ActiveMQ (push) Canceled after 0s
Fineract Messaging Smoke Tests / Smoke Test with Kafka (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MariaDB / test (test-core-1) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MariaDB / test (test-core-4) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MariaDB / test (test-core-5) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MySQL / test (test-core-2) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MySQL / test (test-core-3) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MySQL / test (test-core-4) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MySQL / test (test-core-5) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - PostgreSQL / test (test-core-1) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - PostgreSQL / test (test-core-2) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - PostgreSQL / test (test-core-3) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - PostgreSQL / test (test-core-4) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - PostgreSQL / test (test-core-5) (push) Canceled after 0s
Fineract Liquibase Only mode - PostgreSQL / test (push) Canceled after 0s
Fineract Build & Cucumber tests (without E2E tests) / verify (main, build-core) (push) Canceled after 0s
Fineract Build & Cucumber tests (without E2E tests) / verify (main, cucumber) (push) Canceled after 0s
Fineract Build & Cucumber tests (without E2E tests) / verify (progressive-loan, build-progressive-loan) (push) Canceled after 0s
Fineract Docker Builds / build (false, docker-compose-postgresql.yml, postgresql) (push) Canceled after 0s
Fineract Docker Builds / build (true, docker-compose.yml, mariadb) (push) Canceled after 0s
Fineract Documentation build / build (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 1 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 10 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 2 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 3 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 4 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 5 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 6 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 7 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 8 of 10) (push) Canceled after 0s
Fineract E2E Tests / E2E Tests (Shard 9 of 10) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MariaDB / test (test-core-2) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MariaDB / test (test-core-3) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MySQL / test (test-core-1) (push) Canceled after 0s
Fineract Messaging Smoke Tests / Smoke Test with ActiveMQ (push) Canceled after 0s
Fineract Messaging Smoke Tests / Smoke Test with Kafka (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MariaDB / test (test-core-1) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MariaDB / test (test-core-4) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MariaDB / test (test-core-5) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MySQL / test (test-core-2) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MySQL / test (test-core-3) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MySQL / test (test-core-4) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - MySQL / test (test-core-5) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - PostgreSQL / test (test-core-1) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - PostgreSQL / test (test-core-2) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - PostgreSQL / test (test-core-3) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - PostgreSQL / test (test-core-4) (push) Canceled after 0s
Fineract Cargo & Unit- & Integration tests - PostgreSQL / test (test-core-5) (push) Canceled after 0s
Fineract Liquibase Only mode - PostgreSQL / test (push) Canceled after 0s
FD Special (monthly interest payout) accounts post interest each month and immediately sweep it to a linked savings account. Two independent interest recalculation paths were treating that sweep withdrawal as ordinary account activity and silently reversing + reposting already-settled periods at a slightly different (rounding-drifted) amount — after the original, correct amount had already been transferred out. This left the FD short by the difference every time it happened. Root causes (two separate engines, same underlying bug pattern): 1. SavingsAccountDomainServiceJpa#handleWithdrawal re-triggers a full postInterest()/calculateInterestUsing() after every withdrawal, including the sweep withdrawal itself. At that exact moment the withdrawal's own m_account_transfer_transaction linking row does not exist yet, so the balance-exclusion logic can't recognise it as a transfer leg — it gets treated as a real balance-reducing transaction and corrupts that period's (and any later period's) interest calculation. 2. SavingsAccountInterestPostingServiceImpl (the "Post Interest For Savings" batch job) has its own, separate reverse-and-repost correction loop with the same lack of a "was this period already settled" guard. Fixes, both scoped strictly to FD Special (transfer_interest_to_linked_account = true) so plain savings and recurring deposit behaviour is provably unchanged (the new hooks all default to the prior behaviour unless overridden): - SavingsAccount: new isInterestPostingCorrectionAllowed(...) extension point guarding the postInterest() correction loop; new isPostWithdrawalInterestRecalculationRequired(...) hook checked by handleWithdrawal before it re-recalculates; new refreshBalancesAndSummaryAfterTransfer(...) — the running-balance/summary bookkeeping postInterest() always performs, without re-running the posting/correction loop, so skipping the full recalculation doesn't leave the withdrawal's own running balance and the account summary stale. - FixedDepositAccount: overrides both hooks using the existing interestAlreadyPaidOut(...) date-matched sweep-withdrawal check (already used by postMaturityInterest) so an already-swept period is never recalculated again, from either the withdrawal path or a later job run. - SavingsAccountInterestPostingServiceImpl: mirrors the same guard (isInterestAlreadySweptOut) in its own correction loop for the batch job. Unrelated correctness fixes carried in the same files, applicable to any account type (not FD-Special-specific), found while investigating the above: - SavingsAccountWritePlatformServiceJpaRepositoryImpl: interest-payable GL entries were reading a financial_account_type that never exists in acc_product_mapping, always resolving to account id 0. Now reads the correct getGlAccountIdForInterestOnSavings(). - DepositAccountInterestRateChart#getApplicableInterestRate: missing break in the slab-matching loop meant HashSet's non-deterministic iteration order could pick an arbitrary matching slab instead of the intended one. - SavingsAccountReadPlatformServiceImpl / SavingsAccountData: thread the real transfer_interest_to_linked_account flag into the batch interest engine's per-account data so it can correctly disable interest compounding for FD Special (previously hard-coded false, causing the batch job to compound interest that should reset to zero each period). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+3
@@ -151,6 +151,9 @@ public class DepositAccountInterestRateChart extends AbstractPersistableCustom<L
|
||||
if (effectiveInterestRate == null || effectiveInterestRate.compareTo(BigDecimal.ZERO) == 0) {
|
||||
effectiveInterestRate = slab.slabFields().annualInterestRate();
|
||||
}
|
||||
// Only one slab should ever match a given period/amount; stop at the first match instead of
|
||||
// letting a later slab in iteration order silently overwrite it.
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+96
-3
@@ -515,6 +515,52 @@ public class SavingsAccount extends AbstractAuditableWithUTCDateTimeCustom<Long>
|
||||
return identifiers;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extension point for the interest-posting correction loop in {@link #postInterest}: lets a subclass veto
|
||||
* reversing/reposting an already-existing posting transaction for a period. Defaults to always allowing the
|
||||
* correction, so plain savings and recurring deposit behaviour is unchanged.
|
||||
*/
|
||||
protected boolean isInterestPostingCorrectionAllowed(final SavingsAccountTransaction postingTransaction) {
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extension point checked by {@code SavingsAccountDomainServiceJpa#handleWithdrawal} before it re-triggers an
|
||||
* interest recalculation right after creating a withdrawal. Defaults to always requiring the full recalculation
|
||||
* (via {@link #postInterest} / {@link #calculateInterestUsing}), so plain savings and recurring deposit
|
||||
* behaviour is unchanged. When this returns false, {@link #refreshBalancesAndSummaryAfterTransfer} is called
|
||||
* instead.
|
||||
*/
|
||||
public boolean isPostWithdrawalInterestRecalculationRequired(final boolean isInterestTransfer) {
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Lightweight alternative to {@link #postInterest} used right after a withdrawal when the full interest
|
||||
* recalculation is skipped (see {@link #isPostWithdrawalInterestRecalculationRequired}). Refreshes running
|
||||
* balances and the account summary -- the bookkeeping {@link #postInterest} always performs at its tail,
|
||||
* regardless of whether any posting correction was needed -- without re-running the interest
|
||||
* posting/correction loop that recalculation drives.
|
||||
*/
|
||||
public void refreshBalancesAndSummaryAfterTransfer(final LocalDate interestPostingUpToDate, final boolean backdatedTxnsAllowedTill,
|
||||
final boolean postReversals) {
|
||||
Money openingAccountBalance = Money.zero(this.currency);
|
||||
if (backdatedTxnsAllowedTill) {
|
||||
if (this.summary.getLastInterestCalculationDate() == null) {
|
||||
openingAccountBalance = Money.zero(this.currency);
|
||||
} else {
|
||||
openingAccountBalance = Money.of(this.currency, this.summary.getRunningBalanceOnPivotDate());
|
||||
}
|
||||
}
|
||||
recalculateDailyBalances(openingAccountBalance, interestPostingUpToDate, backdatedTxnsAllowedTill, postReversals);
|
||||
if (!backdatedTxnsAllowedTill) {
|
||||
this.summary.updateSummary(this.currency, this.savingsAccountTransactionSummaryWrapper, this.transactions);
|
||||
} else {
|
||||
this.summary.updateSummaryWithPivotConfig(this.currency, this.savingsAccountTransactionSummaryWrapper, null,
|
||||
this.savingsAccountTransactions);
|
||||
}
|
||||
}
|
||||
|
||||
public void postInterest(final MathContext mc, final LocalDate interestPostingUpToDate, final boolean isInterestTransfer,
|
||||
final boolean isSavingsInterestPostingAtCurrentPeriodEnd, final Integer financialYearBeginningMonth,
|
||||
final LocalDate postInterestOnDate, final boolean backdatedTxnsAllowedTill, final boolean postReversals) {
|
||||
@@ -581,7 +627,7 @@ public class SavingsAccount extends AbstractAuditableWithUTCDateTimeCustom<Long>
|
||||
} else {
|
||||
correctionRequired = postingTransaction.hasNotAmount(interestEarnedToBePostedForPeriod.negated());
|
||||
}
|
||||
if (correctionRequired) {
|
||||
if (correctionRequired && isInterestPostingCorrectionAllowed(postingTransaction)) {
|
||||
boolean applyWithHoldTaxForOldTransaction = false;
|
||||
postingTransaction.reverse();
|
||||
SavingsAccountTransaction reversal = null;
|
||||
@@ -1047,14 +1093,59 @@ public class SavingsAccount extends AbstractAuditableWithUTCDateTimeCustom<Long>
|
||||
.sorted(new SavingsAccountTransactionComparator()).collect(Collectors.toList());
|
||||
}
|
||||
|
||||
/**
|
||||
* Transactions that should NOT count as part of this account's own balance history for interest
|
||||
* calculation purposes -- excludes interest-posting credits (they're the thing being calculated, not an
|
||||
* input to it) AND, for accounts that transfer their interest out to a linked account
|
||||
* (isTransferInterestToOtherAccount(), e.g. FD Special), the paired WITHDRAWAL transaction that later
|
||||
* sweeps that same interest back out (see TransferInterestToSavingsTasklet).
|
||||
* <p>
|
||||
* Without also excluding that withdrawal, only the credit half of the round-trip gets stripped out here
|
||||
* -- the debit half stays, permanently understating this account's balance (and therefore every
|
||||
* subsequent period's interest) by that amount from the first transfer onward, even though the real
|
||||
* ledger balance never actually changed. fetchPostInterestTransactionIds(getId()) returns exactly the
|
||||
* withdrawal-transaction ids that are the FROM side of a real INTEREST_TRANSFER account transfer on this
|
||||
* account, so excluding those ids here closes that gap.
|
||||
*/
|
||||
protected Collection<Long> interestTransferWithdrawalTransactionIds() {
|
||||
if (getId() == null || this.savingsHelper == null || !isTransferInterestToOtherAccount()) {
|
||||
return Collections.emptyList();
|
||||
}
|
||||
return this.savingsHelper.fetchPostInterestTransactionIds(getId());
|
||||
}
|
||||
|
||||
/**
|
||||
* The given transactions minus the interest-transfer withdrawal legs, exactly as
|
||||
* {@link #retreiveOrderedNonInterestPostingTransactions()} removes them before the interest engine ever
|
||||
* sees them.
|
||||
* <p>
|
||||
* Those two sets MUST agree. {@link #resetAccountTransactionsEndOfDayBalances} derives each transaction's
|
||||
* balance segment by walking backwards and ending the previous segment one day before the next
|
||||
* transaction -- so a transaction present there but stripped out here truncates the preceding segment by a
|
||||
* day, with nothing left in the engine's own list to account for that day. On an FD Special, whose
|
||||
* transfer-out withdrawal always carries the same date as the interest posting it pays out, that silently
|
||||
* cost the very first period one earning day.
|
||||
*/
|
||||
protected List<SavingsAccountTransaction> excludeInterestTransferWithdrawals(
|
||||
final List<SavingsAccountTransaction> transactions) {
|
||||
final Collection<Long> interestTransferWithdrawalIds = interestTransferWithdrawalTransactionIds();
|
||||
if (interestTransferWithdrawalIds.isEmpty()) {
|
||||
return transactions;
|
||||
}
|
||||
return transactions.stream().filter(transaction -> !interestTransferWithdrawalIds.contains(transaction.getId()))
|
||||
.collect(Collectors.toList());
|
||||
}
|
||||
|
||||
protected List<SavingsAccountTransaction> retreiveOrderedNonInterestPostingTransactions() {
|
||||
final List<SavingsAccountTransaction> listOfTransactionsSorted = retrieveListOfTransactions();
|
||||
|
||||
final List<SavingsAccountTransaction> orderedNonInterestPostingTransactions = new ArrayList<>();
|
||||
final Collection<Long> interestTransferWithdrawalIds = interestTransferWithdrawalTransactionIds();
|
||||
|
||||
for (final SavingsAccountTransaction transaction : listOfTransactionsSorted) {
|
||||
if (!(transaction.isInterestPostingAndNotReversed() || transaction.isOverdraftInterestAndNotReversed())
|
||||
&& transaction.isNotReversed() && !transaction.isReversalTransaction()) {
|
||||
&& transaction.isNotReversed() && !transaction.isReversalTransaction()
|
||||
&& !interestTransferWithdrawalIds.contains(transaction.getId())) {
|
||||
orderedNonInterestPostingTransactions.add(transaction);
|
||||
}
|
||||
}
|
||||
@@ -1066,10 +1157,12 @@ public class SavingsAccount extends AbstractAuditableWithUTCDateTimeCustom<Long>
|
||||
final List<SavingsAccountTransaction> listOfTransactionsSorted = retrieveSortedTransactions();
|
||||
|
||||
final List<SavingsAccountTransaction> orderedNonInterestPostingTransactions = new ArrayList<>();
|
||||
final Collection<Long> interestTransferWithdrawalIds = interestTransferWithdrawalTransactionIds();
|
||||
|
||||
for (final SavingsAccountTransaction transaction : listOfTransactionsSorted) {
|
||||
if (!(transaction.isInterestPostingAndNotReversed() || transaction.isOverdraftInterestAndNotReversed())
|
||||
&& transaction.isNotReversed() && !transaction.isReversalTransaction()) {
|
||||
&& transaction.isNotReversed() && !transaction.isReversalTransaction()
|
||||
&& !interestTransferWithdrawalIds.contains(transaction.getId())) {
|
||||
orderedNonInterestPostingTransactions.add(transaction);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user