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.
This commit is contained in:
+56
@@ -595,6 +595,62 @@ public class SavingsAccount extends AbstractAuditableWithUTCDateTimeCustom<Long>
|
||||
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<LocalDate> periodPostingDates = new HashSet<>();
|
||||
final List<LocalDateInterval> repostedIntervals = new ArrayList<>();
|
||||
for (final PostingPeriod period : postingPeriods) {
|
||||
periodPostingDates.add(period.dateOfPostingTransaction());
|
||||
if (!DateUtils.isAfter(period.dateOfPostingTransaction(), interestPostingUpToDate)) {
|
||||
repostedIntervals.add(period.getPeriodInterval());
|
||||
}
|
||||
}
|
||||
final List<SavingsAccountTransaction> 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();
|
||||
|
||||
Reference in New Issue
Block a user