From f6686b0c8f644c139c25fc5d4ec691f1dca8deb7 Mon Sep 17 00:00:00 2001 From: Nitesh Yadav Date: Thu, 3 Sep 2026 10:41:14 +0530 Subject: [PATCH] fix(savings): stop manual interest postings from double-counting across runs SavingsAccount#postInterest silently double-credited interest whenever a manual posting ("Post Interest As On", or close() closing an account) ran on two different dates against the same account. Root cause: postInterestOnDate=D makes SavingsHelper#determineInterestPostingPeriods close a period at D-1, so the resulting transaction is dated D-1, not D. The next run's getManualPostingDates() feeds that D-1 date back in as a boundary, which closes a period at D-2 instead. The old posting's date (D-1) then matches no period end in the new run, so findInterestPostingTransactionFor() never finds it, it is never reversed or corrected, and a new posting is created alongside it covering the very same days. total_interest_posted_derived drifts above what was actually earned by exactly the overlapping amount. Reproduced live: posting "as on" 02 Sep then 03 Sep on the same account left a stray 0.35 posting dated 01 Sep plus a new 0.97 posting for 01-02 Sep -- 1.32 posted against 0.97 actually earned. Fix: before applying this run's periods, reverse any manual posting whose date matches no period-posting-date this run computed AND whose days are covered by a period this run WILL post (posting date <= interestPostingUpToDate). The second condition is deliberate -- it is what stops this from reversing a posting whose days are simply not being re-posted yet (e.g. the recalculation Fineract runs right after a withdrawal, where the remaining period matures in the future and posts nothing for those days). Automatic (non-manual) postings are untouched; they sit on natural period ends, which always recur correctly. Affects every caller of the base postInterest/calculateInterestUsing pair, including FixedDepositAccount and RecurringDepositAccount (both delegate to super.postInterest for their day-to-day interest recalculation, though not for postMaturityInterest, which is a separate, untouched method) -- so this also closes the same double-count window for FD/RD accounts, not just plain savings. --- .../savings/domain/SavingsAccount.java | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java b/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java index 3eb427dcf..039ca6d90 100644 --- a/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java +++ b/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/domain/SavingsAccount.java @@ -595,6 +595,62 @@ public class SavingsAccount extends AbstractAuditableWithUTCDateTimeCustom withholdTransactions.addAll(findWithHoldTransactions()); } + // Reverse manual interest postings that no longer line up with any period this run computed, + // BEFORE the periods below are applied. + // + // A user posting ("post interest as on D") lands its transaction on D-1, because + // SavingsHelper#determineInterestPostingPeriods closes the period at D-1 when it meets D in + // postedAsOnDates. But getManualPostingDates() then feeds that transaction's own date -- D-1 + // -- back in as a boundary on every later run, which closes a period at D-2 instead. So the + // posting's date matches no period end the second time around: findInterestPostingTransactionFor + // below misses it, it is never reversed or corrected, and it survives alongside a new posting + // covering the very same days. The member is credited twice, and total_interest_posted_derived + // drifts above what was actually earned. + // + // A posting is only treated as orphaned, and only reversed, when BOTH hold: + // - its date is not any period's posting date, so the loop below will never find and correct it; + // - some period that WILL be posted in this run (posting date on or before + // interestPostingUpToDate) covers that date, so the days it credited are being credited again. + // The second condition is what keeps this from ever destroying interest. A run whose remaining + // period only matures in the future posts nothing for those days -- the recalculation that + // follows a withdrawal is exactly that case -- and reversing there would silently take earned + // interest back off the member instead of de-duplicating it. + // Automatic postings are left alone: they sit on natural period ends, which always recur. + final Set periodPostingDates = new HashSet<>(); + final List repostedIntervals = new ArrayList<>(); + for (final PostingPeriod period : postingPeriods) { + periodPostingDates.add(period.dateOfPostingTransaction()); + if (!DateUtils.isAfter(period.dateOfPostingTransaction(), interestPostingUpToDate)) { + repostedIntervals.add(period.getPeriodInterval()); + } + } + final List postingsInScope = backdatedTxnsAllowedTill + ? getSavingsAccountTransactionsWithPivotConfig() + : getTransactions(); + for (final SavingsAccountTransaction transaction : new ArrayList<>(postingsInScope)) { + final LocalDate transactionDate = transaction.getTransactionDate(); + if (!transaction.isInterestPosting() || !transaction.isNotReversed() || transaction.isReversalTransaction() + || !transaction.isManualTransaction() || periodPostingDates.contains(transactionDate)) { + continue; + } + boolean coveredByAPostedPeriod = false; + for (final LocalDateInterval interval : repostedIntervals) { + if (interval.contains(transactionDate)) { + coveredByAPostedPeriod = true; + break; + } + } + if (!coveredByAPostedPeriod) { + continue; + } + transaction.reverse(); + final SavingsAccountTransaction strandedWithholdTransaction = findTransactionFor(transactionDate, withholdTransactions); + if (strandedWithholdTransaction != null) { + strandedWithholdTransaction.reverse(); + } + recalucateDailyBalanceDetails = true; + } + for (final PostingPeriod interestPostingPeriod : postingPeriods) { final LocalDate interestPostingTransactionDate = interestPostingPeriod.dateOfPostingTransaction(); final Money interestEarnedToBePostedForPeriod = interestPostingPeriod.getInterestEarned();