fix(deposits): enforce savings-only maturity transfers and fix duplicate GL posting on FD/RD closure

Adds validation so Transfer-to-Savings can only target a real savings account,
with a proper user-facing message instead of the generic validation error.
Fixes three duplicate journal-entry bugs uncovered while testing FD/RD
maturity: the dormant-accounts job re-posting already-closed accounts
(escheat/setSubStatusInactive/setSubStatusDormant), and the maturity-closure
flow itself re-deriving accounting for a withdrawal that the account-transfer
service had already posted.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

fineract-provider/src/main/java/org/apache/fineract/accounting/journalentry/service/AccountingProcessorHelper.java

@@ -1263,6 +1263,11 @@ public class AccountingProcessorHelper {
                     accountMapping = paymentChannelSpecificAccountMapping;
                 }
             }
+
+            if (accountMapping == null) {
+                throw new ProductToGLAccountMappingNotFoundException(PortfolioProductType.SAVING, savingsProductId,
+                        CashAccountsForSavings.fromInt(accountMappingTypeId).toString());
+            }
             glAccount = accountMapping.getGlAccount();
         }
         return glAccount;

fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/data/DepositAccountDataValidator.java

@@ -97,6 +97,7 @@ import org.apache.fineract.portfolio.savings.SavingsInterestCalculationType;
 import org.apache.fineract.portfolio.savings.SavingsPeriodFrequencyType;
 import org.apache.fineract.portfolio.savings.SavingsPostingInterestPeriodType;
 import org.apache.fineract.portfolio.savings.domain.SavingsAccount;
+import org.apache.fineract.portfolio.savings.domain.SavingsAccountRepositoryWrapper;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
@@ -105,11 +106,14 @@ public class DepositAccountDataValidator {
 
     private final FromJsonHelper fromApiJsonHelper;
     private final DepositProductDataValidator productDataValidator;
+    private final SavingsAccountRepositoryWrapper savingsAccountRepositoryWrapper;
 
     @Autowired
-    public DepositAccountDataValidator(final FromJsonHelper fromApiJsonHelper, final DepositProductDataValidator productDataValidator) {
+    public DepositAccountDataValidator(final FromJsonHelper fromApiJsonHelper, final DepositProductDataValidator productDataValidator,
+            final SavingsAccountRepositoryWrapper savingsAccountRepositoryWrapper) {
         this.fromApiJsonHelper = fromApiJsonHelper;
         this.productDataValidator = productDataValidator;
+        this.savingsAccountRepositoryWrapper = savingsAccountRepositoryWrapper;
     }
 
     public void validateFixedDepositForSubmit(final String json) {
@@ -351,10 +355,31 @@ public class DepositAccountDataValidator {
             baseDataValidator.reset().parameter(maturityInstructionIdParamName).value(depositRolloverId).notNull()
                     .isOneOfTheseValues(DepositAccountOnClosureType.integerValues());
 
+            // Withdraw Deposit books the maturity proceeds straight out of the bank's cash/vault GL the
+            // moment the account matures -- before the customer has actually come in to collect it. For
+            // maturities processed automatically by the scheduled job (not a live counter transaction),
+            // that books cash as disbursed while it is still physically in the vault, so this option is
+            // disabled: every account must resolve to Transfer to Savings or a Reinvest option instead.
+            if (depositRolloverId.equals(DepositAccountOnClosureType.WITHDRAW_DEPOSIT.getValue())) {
+                baseDataValidator.reset().parameter(maturityInstructionIdParamName).value(depositRolloverId).failWithCodeAndMessage(
+                        "maturity.instruction.withdraw.deposit.not.allowed",
+                        "Withdraw Deposit is not a valid maturity instruction -- choose Transfer to Savings or a Reinvest option.");
+            }
+
             if (depositRolloverId.equals(DepositAccountOnClosureType.TRANSFER_TO_SAVINGS.getValue())) {
                 final Long transferToSavingsId = this.fromApiJsonHelper.extractLongNamed(transferToSavingsIdParamName, element);
                 baseDataValidator.reset().parameter(transferToSavingsIdParamName).value(transferToSavingsId).notNull()
                         .longGreaterThanZero();
+
+                // Transfer to Savings can only ever post into a genuine Savings account -- Current accounts
+                // live in a separate ledger/module that this maturity-closure transfer cannot write into.
+                if (transferToSavingsId != null && transferToSavingsId > 0
+                        && !this.savingsAccountRepositoryWrapper.existsWithDepositAccountType(transferToSavingsId,
+                                DepositAccountType.SAVINGS_DEPOSIT)) {
+                    baseDataValidator.reset().parameter(transferToSavingsIdParamName).value(transferToSavingsId)
+                            .failWithCodeAndMessage("transfer.to.savings.id.is.not.a.savings.account",
+                                    "The selected account is not a Savings account -- Transfer to Savings can only target a Savings account.");
+                }
             }
         }
     }
@@ -493,10 +518,31 @@ public class DepositAccountDataValidator {
             baseDataValidator.reset().parameter(maturityInstructionIdParamName).value(depositRolloverId).notNull()
                     .isOneOfTheseValues(DepositAccountOnClosureType.integerValues());
 
+            // Withdraw Deposit books the maturity proceeds straight out of the bank's cash/vault GL the
+            // moment the account matures -- before the customer has actually come in to collect it. For
+            // maturities processed automatically by the scheduled job (not a live counter transaction),
+            // that books cash as disbursed while it is still physically in the vault, so this option is
+            // disabled: every account must resolve to Transfer to Savings or a Reinvest option instead.
+            if (depositRolloverId.equals(DepositAccountOnClosureType.WITHDRAW_DEPOSIT.getValue())) {
+                baseDataValidator.reset().parameter(maturityInstructionIdParamName).value(depositRolloverId).failWithCodeAndMessage(
+                        "maturity.instruction.withdraw.deposit.not.allowed",
+                        "Withdraw Deposit is not a valid maturity instruction -- choose Transfer to Savings or a Reinvest option.");
+            }
+
             if (depositRolloverId.equals(DepositAccountOnClosureType.TRANSFER_TO_SAVINGS.getValue())) {
                 final Long transferToSavingsId = this.fromApiJsonHelper.extractLongNamed(transferToSavingsIdParamName, element);
                 baseDataValidator.reset().parameter(transferToSavingsIdParamName).value(transferToSavingsId).notNull()
                         .longGreaterThanZero();
+
+                // Transfer to Savings can only ever post into a genuine Savings account -- Current accounts
+                // live in a separate ledger/module that this maturity-closure transfer cannot write into.
+                if (transferToSavingsId != null && transferToSavingsId > 0
+                        && !this.savingsAccountRepositoryWrapper.existsWithDepositAccountType(transferToSavingsId,
+                                DepositAccountType.SAVINGS_DEPOSIT)) {
+                    baseDataValidator.reset().parameter(transferToSavingsIdParamName).value(transferToSavingsId)
+                            .failWithCodeAndMessage("transfer.to.savings.id.is.not.a.savings.account",
+                                    "The selected account is not a Savings account -- Transfer to Savings can only target a Savings account.");
+                }
             }
         }
 

fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/DepositAccountDomainServiceJpa.java

@@ -555,14 +555,19 @@ public class DepositAccountDomainServiceJpa implements DepositAccountDomainServi
     }
 
     private void updateAlreadyPostedTransactions(final Set<Long> existingTransactionIds, final SavingsAccount savingsAccount) {
-        List<SavingsAccountTransaction> transactions = savingsAccount.getTransactions();
-        int size = transactions.size();
-        for (int i = size - 1;; i--) {
-            SavingsAccountTransaction transaction = transactions.get(i);
-            if (transaction.isWithdrawal() || transaction.isWithdrawalFee()) {
+        // accountTransfersWritePlatformService.transferFunds() already posts accounting for the
+        // withdrawal it creates on this account (via SavingsAccountDomainServiceJpa#handleWithdrawal),
+        // so that withdrawal must be added to existingTransactionIds here or the caller's own
+        // postJournalEntries() re-derives and re-posts it a second time. The previous implementation
+        // found it by walking the transaction list backwards from the end while the type stayed
+        // withdrawal/withdrawal-fee -- but @OneToMany collections only guarantee @OrderBy ordering on
+        // a fresh fetch, not after an in-memory append, so a same-day interest posting or any other
+        // ordering quirk could put the new withdrawal anywhere in the list and break the walk
+        // silently. Scanning every transaction and checking against the original snapshot is correct
+        // regardless of collection order.
+        for (final SavingsAccountTransaction transaction : savingsAccount.getTransactions()) {
+            if ((transaction.isWithdrawal() || transaction.isWithdrawalFee()) && !existingTransactionIds.contains(transaction.getId())) {
                 existingTransactionIds.add(transaction.getId());
-            } else {
-                break;
             }
         }
     }

fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/DepositAccountReadPlatformServiceImpl.java

@@ -554,7 +554,7 @@ public class DepositAccountReadPlatformServiceImpl implements DepositAccountRead
                         fieldOfficerOptions, interestCompoundingPeriodTypeOptions, interestPostingPeriodTypeOptions,
                         interestCalculationTypeOptions, interestCalculationDaysInYearTypeOptions, lockinPeriodFrequencyTypeOptions,
                         withdrawalFeeTypeOptions, transactions, charges, chargeOptions, preClosurePenalInterestOnTypeOptions,
-                        periodFrequencyTypeOptions);
+                        periodFrequencyTypeOptions, savingsAccountDatas, maturityInstructionOptions);
                 template = RecurringDepositAccountData.withInterestChartAndRecurringDetails((RecurringDepositAccountData) template,
                         accountChart, null, null);
                 // Re-apply CBS product fields lost during factory method object creation
@@ -626,7 +626,7 @@ public class DepositAccountReadPlatformServiceImpl implements DepositAccountRead
                         fieldOfficerOptions, interestCompoundingPeriodTypeOptions, interestPostingPeriodTypeOptions,
                         interestCalculationTypeOptions, interestCalculationDaysInYearTypeOptions, lockinPeriodFrequencyTypeOptions,
                         withdrawalFeeTypeOptions, transactions, charges, chargeOptions, preClosurePenalInterestOnTypeOptions,
-                        periodFrequencyTypeOptions);
+                        periodFrequencyTypeOptions, savingsAccountDatas, maturityInstructionOptions);
             }
         }
 

fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/DepositAccountWritePlatformServiceJpaRepositoryImpl.java

@@ -1393,6 +1393,15 @@ public class DepositAccountWritePlatformServiceJpaRepositoryImpl implements Depo
                 this.depositAccountDomainService.handleFDAccountMaturityClosure(fdAccount, null, user, fmt, fdAccount.maturityDate(),
                         fdAccount.getOnAccountClosureId(), toSavingsId, "Apply maturity instructions", changes);
 
+                // Every maturity-instruction branch takes the balance out of this account through
+                // SavingsAccountDomainServiceJpa#handleWithdrawal, which posts that withdrawal's journal
+                // entries itself. existingTransactionIds was snapshotted at the top of this method --
+                // before that withdrawal existed -- so the postJournalEntries() call at the end would
+                // treat it as new and post it a second time. Interest postings created by
+                // updateMaturityStatus() above are still genuinely unposted at this point and must stay
+                // out of the snapshot so that final call can post them exactly once.
+                markWithdrawalsAsAlreadyPosted(account, existingTransactionIds);
+
                 if (changes.get("reinvestedDepositId") != null) {
                     Long reinvestedDepositId = (Long) changes.get("reinvestedDepositId");
                     Money amountForDeposit = account.activateWithBalance();
@@ -1421,6 +1430,19 @@ public class DepositAccountWritePlatformServiceJpaRepositoryImpl implements Depo
         existingReversedTransactionIds.addAll(account.findExistingReversedTransactionIds());
     }
 
+    /**
+     * Marks the account's withdrawal (and withdrawal-fee) transactions as already accounted for, so a
+     * subsequent postJournalEntries() built on an older snapshot does not post them again. Used after a
+     * maturity closure, whose withdrawal leg posts its own journal entries.
+     */
+    private void markWithdrawalsAsAlreadyPosted(final SavingsAccount account, final Set<Long> existingTransactionIds) {
+        for (final SavingsAccountTransaction transaction : account.getTransactions()) {
+            if (transaction.getId() != null && (transaction.isWithdrawal() || transaction.isWithdrawalFee())) {
+                existingTransactionIds.add(transaction.getId());
+            }
+        }
+    }
+
     private void postJournalEntries(final SavingsAccount savingsAccount, final Set<Long> existingTransactionIds,
             final Set<Long> existingReversedTransactionIds) {
 

fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountWritePlatformServiceJpaRepositoryImpl.java

@@ -1676,6 +1676,15 @@ public class SavingsAccountWritePlatformServiceJpaRepositoryImpl implements Savi
     @Override
     public void setSubStatusInactive(Long savingsId) {
         final SavingsAccount account = this.savingAccountAssembler.assembleFrom(savingsId, false);
+        // Same snapshot-vs-execution gap as escheat() below: the dormant-accounts batch selects
+        // pending-inactive ids from a query that requires status = ACTIVE, but the account can close
+        // through another path (e.g. deposit maturity) before this call runs. Re-deriving accounting
+        // off the full transaction list at that point re-posts entries that closure already posted.
+        if (!account.isActive()) {
+            log.warn("Skipping setSubStatusInactive for savings account {} -- no longer active (already closed by another process)",
+                    savingsId);
+            return;
+        }
         final Set<Long> existingTransactionIds = new HashSet<>();
         final Set<Long> existingReversedTransactionIds = new HashSet<>();
         updateExistingTransactionsDetails(account, existingTransactionIds, existingReversedTransactionIds);
@@ -1687,6 +1696,11 @@ public class SavingsAccountWritePlatformServiceJpaRepositoryImpl implements Savi
     @Override
     public void setSubStatusDormant(Long savingsId) {
         final SavingsAccount account = this.savingAccountAssembler.assembleFrom(savingsId, false);
+        if (!account.isActive()) {
+            log.warn("Skipping setSubStatusDormant for savings account {} -- no longer active (already closed by another process)",
+                    savingsId);
+            return;
+        }
         account.setSubStatusDormant();
         this.savingAccountRepositoryWrapper.saveAndFlush(account);
     }
@@ -1694,6 +1708,16 @@ public class SavingsAccountWritePlatformServiceJpaRepositoryImpl implements Savi
     @Override
     public void escheat(Long savingsId) {
         final SavingsAccount account = this.savingAccountAssembler.assembleFrom(savingsId, false);
+        // The dormant-accounts batch selects escheat-eligible ids from a snapshot query, so an
+        // account can legitimately close through another path (e.g. deposit maturity) in the gap
+        // between that selection and this call. escheat() unconditionally forces status/sub-status
+        // to CLOSED/ESCHEAT and re-derives accounting off the account's full transaction list, so
+        // running it against an already-closed account both mislabels the closure reason and
+        // re-posts journal entries for transactions that were already posted by the real closure.
+        if (!account.isActive()) {
+            log.warn("Skipping escheat for savings account {} -- no longer active (already closed by another process)", savingsId);
+            return;
+        }
         final Set<Long> existingTransactionIds = new HashSet<>();
         final Set<Long> existingReversedTransactionIds = new HashSet<>();
         updateExistingTransactionsDetails(account, existingTransactionIds, existingReversedTransactionIds);