From 2155c94161e0209792d54a2904ead7256775c421 Mon Sep 17 00:00:00 2001 From: Nitesh Yadav Date: Mon, 28 Sep 2026 11:41:18 +0530 Subject: [PATCH] fix(fd): Reinvest Principal Only renews in place, same account, instead of opening a new one Reinvest Principal Only was still on the legacy close-and-reopen path (open a new FD with just the principal, close the old one) while Reinvest Principal + Interest and Partial Reinvest had already moved to renewInPlace -- the same customer choosing a different split ended up with two different outcomes: same account number, or a brand new one. Unified all three onto renewInPlace: Principal Only is now exactly Partial Reinvest with the split fixed at the closing principal, so it renews on the same account with only the leftover interest actually leaving (to the chosen/linked Savings account), in both handleFDAccountPayment (Payment dialog / FD Dashboard Renew) and handleFDAccountMaturityClosure (the nightly job's path for an account with an already-stored instruction). Also fixes a related bug found while testing this: recordInPlaceRenewal re-archived the closing term's full rate chart on every renewal of the same term, even when it had already been archived by an earlier renewal -- archiveChartSlabs now only runs the first time a term's closing row is created. Co-Authored-By: Claude Sonnet 5 --- .../DepositAccountDomainServiceJpa.java | 193 ++++++------------ ...WritePlatformServiceJpaRepositoryImpl.java | 23 +-- 2 files changed, 72 insertions(+), 144 deletions(-) diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/DepositAccountDomainServiceJpa.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/DepositAccountDomainServiceJpa.java index a884d9f19..acdd50fdc 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/DepositAccountDomainServiceJpa.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/domain/DepositAccountDomainServiceJpa.java @@ -303,16 +303,29 @@ public class DepositAccountDomainServiceJpa implements DepositAccountDomainServi account.postMaturityInterest(isSavingsInterestPostingAtCurrentPeriodEnd, financialYearBeginningMonth); account.setClosedOnDate(closedDate); final DepositAccountOnClosureType onClosureType = DepositAccountOnClosureType.fromInt(onAccountClosureId); - if (onClosureType.isReinvestPrincipalAndInterest()) { + if (onClosureType.isReinvestPrincipalAndInterest() || onClosureType.isReinvestPrincipal()) { // Renews on the SAME account, so the customer keeps one account number however many times - // the deposit rolls over. Nothing moves: the whole matured balance simply becomes the new - // term's principal, so there is no withdrawal, no deposit and no journal entry for the - // rollover itself -- which also removes the transfer-suspense round trip the old - // close-and-reopen flow needed. + // the deposit rolls over -- Reinvest Principal Only renews just the closing principal here + // (below), with only the leftover interest actually leaving, rather than the legacy + // close-and-reopen this branch used to use, which left the customer with a second account + // number for something that renews here just as cleanly as Reinvest Principal + Interest + // does. Full reinvest itself still moves nothing: the whole matured balance simply becomes + // the new term's principal, so there is no withdrawal, no deposit and no journal entry for + // the rollover itself. final BigDecimal closingPrincipal = account.getDepositAmount(); final BigDecimal maturedBalance = account.getAccountBalance(); final BigDecimal interestEarned = maturedBalance == null || closingPrincipal == null ? null : maturedBalance.subtract(closingPrincipal); + final BigDecimal renewedAmount = onClosureType.isReinvestPrincipal() ? closingPrincipal : maturedBalance; + // Whatever doesn't renew is interest the customer earned and has to actually be paid out + // somewhere -- validated before anything below runs, so a missing target fails cleanly with + // nothing done rather than leaving a half-completed reinvestment. + final BigDecimal leftoverInterest = maturedBalance.subtract(renewedAmount); + if (onClosureType.isReinvestPrincipal() && leftoverInterest.compareTo(BigDecimal.ZERO) > 0 && toSavingsId == null) { + throw new GeneralPlatformDomainRuleException( + "error.msg.fixeddepositaccount.reinvest.principal.only.requires.savings.account", + "Reinvest Principal Only requires a Savings account to receive the leftover interest."); + } // The active CBS base interest-rate version for this FD type: the same rate card the Base // Interest Rates screen shows and the account-opening wizard prices from. Used for both the @@ -337,66 +350,20 @@ public class DepositAccountDomainServiceJpa implements DepositAccountDomainServi final Integer closingTenure = account.depositPeriod(); final Integer closingTenureFrequencyEnum = account.depositPeriodFrequency(); - account.renewInPlace(maturedBalance, closedDate, currentSeniorCitizenRate, baseChartFieldsOf(activeBaseRates), + account.renewInPlace(renewedAmount, closedDate, currentSeniorCitizenRate, baseChartFieldsOf(activeBaseRates), baseChartSlabsOf(activeBaseRates, account.getCurrency().getCode())); account.updateMaturityDateAndAmount(mc, isPreMatureClosure, isSavingsInterestPostingAtCurrentPeriodEnd, financialYearBeginningMonth); this.savingsAccountRepository.save(account); - recordInPlaceRenewal(account, closingPrincipal, interestEarned, maturedBalance, maturedBalance, closedDate, + recordInPlaceRenewal(account, closingPrincipal, interestEarned, maturedBalance, renewedAmount, closedDate, FixedDepositRenewalHistory.RENEWAL_TYPE_AUTO, user, closingChartName, closingChartSlabs, closingInterestRate, closingTenure, closingTenureFrequencyEnum); changes.put("renewedInPlace", Boolean.TRUE); changes.put("renewalCycleNo", account.renewalCycleNo()); - } else if (onClosureType.isReinvest()) { - // "Re-invest principal only" still closes and reopens, because only the principal carries - // forward and the interest has to leave the deposit -- which needs a payout destination that - // an in-place renewal has nowhere to send. Left on the original flow rather than guessing. - final BigDecimal maturedBalance = account.getAccountBalance(); - final BigDecimal reInvestAmount = account.getDepositAmount(); - // Whatever is left after the principal moves into the new deposit is interest the customer - // earned and has to actually be paid out somewhere -- it must not just sit on this account, - // which is about to be closed. Validated before anything below runs, so a missing target - // fails cleanly with nothing done rather than leaving a half-completed reinvestment. - final BigDecimal leftoverInterest = maturedBalance.subtract(reInvestAmount); - if (leftoverInterest.compareTo(BigDecimal.ZERO) > 0 && toSavingsId == null) { - throw new GeneralPlatformDomainRuleException( - "error.msg.fixeddepositaccount.reinvest.principal.only.requires.savings.account", - "Reinvest Principal Only requires a Savings account to receive the leftover interest."); - } - ExternalId externalId = this.externalIdFactory.create(); - - FixedDepositAccount reinvestedDeposit = account.reInvest(reInvestAmount, externalId); - this.depositAccountAssembler.assignSavingAccountHelpers(reinvestedDeposit); - reinvestedDeposit.updateMaturityDateAndAmountBeforeAccountActivation(mc, isPreMatureClosure, - isSavingsInterestPostingAtCurrentPeriodEnd, financialYearBeginningMonth); - - this.savingsAccountRepository.saveAndFlush(reinvestedDeposit); - autoGenerateAccountNumber(reinvestedDeposit); - // Renewal moves the balance internally into the new deposit -- no cash leaves the branch -- - // so this leg is posted as an account transfer (against the transfer-suspense GL) rather - // than as a counter withdrawal, which would wrongly credit the cash/vault account. - final boolean isInternalTransferLeg = true; - final SavingsAccountTransaction withdrawal = this.handleWithdrawal(account, fmt, closedDate, reInvestAmount, paymentDetail, - false, isRegularTransaction, isInternalTransferLeg); - savingsTransactionId = withdrawal.getId(); - // handleWithdrawal() posts this withdrawal's own journal entries, so it has to be marked as - // already accounted for -- exactly as the transfer-to-savings branch below does -- or the - // postJournalEntries() call at the end of this method posts it a second time. - updateAlreadyPostedTransactions(existingTransactionIds, account); - - changes.put("reinvestedDepositId", reinvestedDeposit.getId()); - reinvestedDeposit.approveAndActivateApplication(closedDate, user); - this.savingsAccountRepository.save(reinvestedDeposit); - - // Reached from the maturity job processing the account's standing maturity instruction. - recordRenewal(account, reinvestedDeposit, maturedBalance, reInvestAmount, closedDate, - FixedDepositRenewalHistory.RENEWAL_TYPE_AUTO, user); - - // Pay out whatever didn't move into the new deposit, then actually close this account -- - // previously this branch left the leftover interest stranded on an account that was never - // closed, silently, because nothing past this point ever referenced toSavingsId at all. + // Only Reinvest Principal Only leaves anything here -- Full Reinvest's leftoverInterest is + // zero (renewedAmount already equals maturedBalance), so this stays a no-op for it. if (leftoverInterest.compareTo(BigDecimal.ZERO) > 0) { final SavingsAccount toSavingsAccount = this.depositAccountAssembler.assembleFrom(toSavingsId, DepositAccountType.SAVINGS_DEPOSIT); @@ -409,8 +376,6 @@ public class DepositAccountDomainServiceJpa implements DepositAccountDomainServi this.accountTransfersWritePlatformService.transferFunds(leftoverTransferDTO); updateAlreadyPostedTransactions(existingTransactionIds, account); } - account.updateClosedStatus(); - account.updateOnAccountClosureStatus(onClosureType); } else if (onClosureType.isTransferToSavings()) { final SavingsAccount toSavingsAccount = this.depositAccountAssembler.assembleFrom(toSavingsId, @@ -751,13 +716,16 @@ public class DepositAccountDomainServiceJpa implements DepositAccountDomainServi account.getId(), account.getId(), null, null, newCycleNo - 1, closingPrincipal, null, maturedBalance, closingPrincipal, closingInterestRate, account.getActivationDate(), renewedOn, closingTenure, closingTenureFrequencyEnum, renewalType, account.getActivationDate(), createdById)); + // Only on this first-ever close of this term -- otherwise a term that renews more than once + // (e.g. this account renewed, then that renewal itself later undone/retried) re-archives the + // same chart against the same closingRow every time, piling up duplicate slab rows for a + // chart that only ever needs archiving once. + archiveChartSlabs(closingRow.getId(), closingChartName, closingChartSlabs, account.getCurrency().getCode()); } closingRow.recordOutcome(closingInterestEarned, maturedBalance); closingRow.markMatured(); this.fixedDepositRenewalHistoryRepository.save(closingRow); - archiveChartSlabs(closingRow.getId(), closingChartName, closingChartSlabs, account.getCurrency().getCode()); - // The new term's own interest/maturity stay unset until it closes in turn. this.fixedDepositRenewalHistoryRepository.save(FixedDepositRenewalHistory.create(fdAccountNumber, account.getId(), account.getId(), null, closingRow.getId(), newCycleNo, renewedAmount, null, null, renewedAmount, @@ -884,9 +852,13 @@ public class DepositAccountDomainServiceJpa implements DepositAccountDomainServi * A staff-triggered, always-available alternative to Premature Close (Active-only, no reinvest) and Close * (Matured-only): works on an FD in either state, and adds a Partial Reinvest option neither of those two support * -- part of the matured amount renews on this same account via {@link FixedDepositAccount#renewInPlace}, the rest - * transfers out to a savings account picked at the moment of the request. Full reinvest here also goes through - * {@code renewInPlace} rather than the legacy close-and-reopen {@link FixedDepositAccount#reInvest}, which is what - * the existing manual Close command still uses today. + * transfers out to a savings account picked at the moment of the request. Reinvest Principal + Interest and + * Reinvest Principal Only both go through the same {@code renewInPlace} mechanism too -- Principal Only is just + * Partial Reinvest with the split fixed at exactly the closing principal, so it renews on this same account with + * only the interest actually leaving, rather than the legacy close-and-reopen {@link FixedDepositAccount#reInvest} + * this branch used to use (still what the existing manual Close command uses today, and what the scheduled job's + * own closure path -- {@link #handleFDAccountMaturityClosure} -- still uses for an account with an + * already-stored instruction). *

* Deliberately independent of {@link #handleFDAccountClosure} and {@link #handleFDAccountPreMatureClosure} -- * neither of those two methods is touched, so the existing Close/Premature Close buttons keep behaving exactly as @@ -953,10 +925,10 @@ public class DepositAccountDomainServiceJpa implements DepositAccountDomainServi final Integer onAccountClosureId = command.integerValueOfParameterNamed(onAccountClosureIdParamName); final DepositAccountOnClosureType onClosureType = DepositAccountOnClosureType.fromInt(onAccountClosureId); - if (onClosureType.isReinvestPrincipalAndInterest() || onClosureType.isPartialReinvest()) { - // These two branches keep the account open, so they never reach close()/prematureClosure() and - // would otherwise accept any date at all -- including one behind already-posted interest, which - // the renewal would then turn into a pivot that reverses and re-posts it. + if (onClosureType.isReinvestPrincipalAndInterest() || onClosureType.isPartialReinvest() || onClosureType.isReinvestPrincipal()) { + // These three branches keep the account open, so they never reach close()/prematureClosure() + // and would otherwise accept any date at all -- including one behind already-posted interest, + // which the renewal would then turn into a pivot that reverses and re-posts it. account.validateRenewalDate(closedDate); final BigDecimal closingPrincipal = account.getDepositAmount(); @@ -972,6 +944,12 @@ public class DepositAccountDomainServiceJpa implements DepositAccountDomainServi throw new GeneralPlatformDomainRuleException("error.msg.fixeddeposit.partial.reinvest.amount.invalid", "The reinvest amount must be greater than zero and less than the matured balance.", renewedAmount); } + } else if (onClosureType.isReinvestPrincipal()) { + // Exactly the closing term's own principal -- the fixed point on the same split Partial + // Reinvest lets staff pick freely. Renews in place same as the other two, on just this + // account, with only the interest actually leaving (below) -- no second account, unlike the + // legacy close-and-reopen this branch used to do. + renewedAmount = closingPrincipal; } else { renewedAmount = maturedBalance; } @@ -1004,74 +982,25 @@ public class DepositAccountDomainServiceJpa implements DepositAccountDomainServi changes.put("renewedInPlace", Boolean.TRUE); changes.put("renewalCycleNo", account.renewalCycleNo()); - if (onClosureType.isPartialReinvest()) { + if (onClosureType.isPartialReinvest() || onClosureType.isReinvestPrincipal()) { // The kept portion never left the account -- renewInPlace() just relabels it as the new - // term's principal -- so only the remainder is an actual money movement, posted the same - // way the Transfer to Savings branch below posts its full-balance transfer. + // term's principal -- so only the remainder (the interest, for Reinvest Principal Only) is + // an actual money movement, posted the same way the Transfer to Savings branch below posts + // its full-balance transfer. final BigDecimal remainder = maturedBalance.subtract(renewedAmount); - final Long toSavingsId = command.longValueOfParameterNamed(toSavingsAccountIdParamName); - final String transferDescription = command.stringValueOfParameterNamed(transferDescriptionParamName); - final SavingsAccount toSavingsAccount = this.depositAccountAssembler.assembleFrom(toSavingsId, - DepositAccountType.SAVINGS_DEPOSIT); - final boolean isExceptionForBalanceCheck = false; - final AccountTransferDTO accountTransferDTO = new AccountTransferDTO(closedDate, remainder, PortfolioAccountType.SAVINGS, - PortfolioAccountType.SAVINGS, null, null, transferDescription, locale, fmt, null, null, null, null, null, - AccountTransferType.ACCOUNT_TRANSFER.getValue(), null, null, ExternalId.empty(), null, toSavingsAccount, account, - isAccountTransfer, isExceptionForBalanceCheck); - this.accountTransfersWritePlatformService.transferFunds(accountTransferDTO); - updateAlreadyPostedTransactions(existingTransactionIds, account); - } - - } else if (onClosureType.isReinvestPrincipal()) { - // Legacy close-and-reopen, same shape as handleFDAccountClosure()'s isReinvest() branch -- - // the interest has to leave the deposit, which needs a payout destination an in-place renewal - // has nowhere to send. - final BigDecimal maturedBalance = account.getAccountBalance(); - final BigDecimal reInvestAmount = account.getDepositAmount(); - final ExternalId externalId = this.externalIdFactory.create(); - - final FixedDepositAccount reinvestedDeposit = account.reInvest(reInvestAmount, externalId); - this.depositAccountAssembler.assignSavingAccountHelpers(reinvestedDeposit); - reinvestedDeposit.updateMaturityDateAndAmountBeforeAccountActivation(mc, isPreMatureClosure, - isSavingsInterestPostingAtCurrentPeriodEnd, financialYearBeginningMonth); - - this.savingsAccountRepository.saveAndFlush(reinvestedDeposit); - autoGenerateAccountNumber(reinvestedDeposit); - final boolean isInternalTransferLeg = true; - final SavingsAccountTransaction withdrawal = this.handleWithdrawal(account, fmt, closedDate, reInvestAmount, paymentDetail, - false, isRegularTransaction, isInternalTransferLeg); - savingsTransactionId = withdrawal.getId(); - updateAlreadyPostedTransactions(existingTransactionIds, account); - - changes.put("reinvestedDepositId", reinvestedDeposit.getId()); - reinvestedDeposit.approveAndActivateApplication(closedDate, user); - this.savingsAccountRepository.save(reinvestedDeposit); - - recordRenewal(account, reinvestedDeposit, maturedBalance, reInvestAmount, closedDate, - FixedDepositRenewalHistory.RENEWAL_TYPE_MANUAL, user); - - // Only the principal moved into the new deposit -- the interest earned on this term is still - // sitting in this account's balance. Pay it out to the chosen savings account now, before - // closing, rather than stranding it here with nowhere left to go. - final BigDecimal interestRemainder = maturedBalance.subtract(reInvestAmount); - if (interestRemainder.compareTo(BigDecimal.ZERO) > 0) { - final Long toSavingsId = command.longValueOfParameterNamed(toSavingsAccountIdParamName); - final String transferDescription = command.stringValueOfParameterNamed(transferDescriptionParamName); - final SavingsAccount toSavingsAccount = this.depositAccountAssembler.assembleFrom(toSavingsId, - DepositAccountType.SAVINGS_DEPOSIT); - final boolean isExceptionForBalanceCheck = false; - final AccountTransferDTO accountTransferDTO = new AccountTransferDTO(closedDate, interestRemainder, - PortfolioAccountType.SAVINGS, PortfolioAccountType.SAVINGS, null, null, transferDescription, locale, fmt, null, - null, null, null, null, AccountTransferType.ACCOUNT_TRANSFER.getValue(), null, null, ExternalId.empty(), null, - toSavingsAccount, account, isAccountTransfer, isExceptionForBalanceCheck); - this.accountTransfersWritePlatformService.transferFunds(accountTransferDTO); - updateAlreadyPostedTransactions(existingTransactionIds, account); - } - - if (alreadyMatured) { - account.close(user, command, changes); - } else { - account.prematureClosure(user, command, changes); + if (remainder.compareTo(BigDecimal.ZERO) > 0) { + final Long toSavingsId = command.longValueOfParameterNamed(toSavingsAccountIdParamName); + final String transferDescription = command.stringValueOfParameterNamed(transferDescriptionParamName); + final SavingsAccount toSavingsAccount = this.depositAccountAssembler.assembleFrom(toSavingsId, + DepositAccountType.SAVINGS_DEPOSIT); + final boolean isExceptionForBalanceCheck = false; + final AccountTransferDTO accountTransferDTO = new AccountTransferDTO(closedDate, remainder, + PortfolioAccountType.SAVINGS, PortfolioAccountType.SAVINGS, null, null, transferDescription, locale, fmt, null, + null, null, null, null, AccountTransferType.ACCOUNT_TRANSFER.getValue(), null, null, ExternalId.empty(), null, + toSavingsAccount, account, isAccountTransfer, isExceptionForBalanceCheck); + this.accountTransfersWritePlatformService.transferFunds(accountTransferDTO); + updateAlreadyPostedTransactions(existingTransactionIds, account); + } } } else if (onClosureType.isTransferToSavings()) { diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/DepositAccountWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/DepositAccountWritePlatformServiceJpaRepositoryImpl.java index 5f4b58c09..42e258e8e 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/DepositAccountWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/DepositAccountWritePlatformServiceJpaRepositoryImpl.java @@ -1008,9 +1008,10 @@ public class DepositAccountWritePlatformServiceJpaRepositoryImpl implements Depo this.depositAccountDomainService.handleFDAccountPayment(account, paymentDetail, user, command, changes); - // Reinvest Principal Only opens a second account and the domain method withdrew the principal out - // of this one against the transfer-suspense GL. Post the matching credit leg so the funds actually - // land in the new deposit -- same sequence updateMaturityDetails() uses for the automated path. + // Reinvest Principal Only now renews in place on this same account (see + // DepositAccountDomainServiceJpa#handleFDAccountPayment) -- reinvestedDepositId is never actually + // set any more, but this guard is left as a harmless no-op rather than removed, in case a future + // closure type reintroduces a genuine close-and-reopen. if (changes.get("reinvestedDepositId") != null) { final Long reinvestedDepositId = (Long) changes.get("reinvestedDepositId"); final FixedDepositAccount reinvestAccount = (FixedDepositAccount) this.depositAccountAssembler @@ -1566,16 +1567,13 @@ public class DepositAccountWritePlatformServiceJpaRepositoryImpl implements Depo // would treat it as new and post it a second time without this. markWithdrawalsAsAlreadyPosted(account, existingTransactionIds); + // Both reinvest types now renew fdAccount itself in place -- reinvestedDepositId is never + // actually set any more, but this guard is left as a harmless no-op rather than removed, + // in case a future closure type reintroduces a genuine close-and-reopen. if (changes.get("reinvestedDepositId") != null) { Long reinvestedDepositId = (Long) changes.get("reinvestedDepositId"); final FixedDepositAccount reinvestAccount = (FixedDepositAccount) this.depositAccountAssembler .assembleFrom(reinvestedDepositId, DepositAccountType.FIXED_DEPOSIT); - // activateWithBalance() always returns zero here -- it exists to stop a manually - // opened FD from auto-crediting itself at activation (funding is supposed to be a - // separate, clerk-posted deposit). A reinvestment isn't that: the balance was just - // withdrawn from the matured account and needs to land in the new one as part of - // this same automated closure, so it has to come from what reInvest() actually - // carried over onto the new account, not from that unrelated activation guard. Money amountForDeposit = Money.of(reinvestAccount.getCurrency(), reinvestAccount.getDepositAmount()); Money activationChargeAmount = getActivationCharge(reinvestAccount); if (activationChargeAmount.isGreaterThanZero()) { @@ -1663,9 +1661,10 @@ public class DepositAccountWritePlatformServiceJpaRepositoryImpl implements Depo this.savingAccountRepositoryWrapper.saveAndFlush(fdAccount); postJournalEntries(fdAccount, existingTransactionIds, existingReversedTransactionIds); - // REINVEST_PRINCIPAL_ONLY closes the old account and opens a new one (reinvestedDepositId); - // REINVEST_PRINCIPAL_AND_INTEREST renews the same account in place -- report the account that - // actually carries the new term's maturity date/amount either way. + // Both reinvest types now renew this same account in place (see + // DepositAccountDomainServiceJpa#handleFDAccountMaturityClosure) -- reinvestedDepositId is never + // actually set any more, but this guard is left as a harmless no-op rather than removed, in case + // a future closure type reintroduces a genuine close-and-reopen. FixedDepositAccount renewedAccount = fdAccount; if (changes.get("reinvestedDepositId") != null) { renewedAccount = (FixedDepositAccount) this.depositAccountAssembler.assembleFrom((Long) changes.get("reinvestedDepositId"),