From 3c0a6559ad06a28b9ce88971e6b81f70ecc0fb22 Mon Sep 17 00:00:00 2001 From: Samer Melhem Date: Wed, 15 Jul 2026 15:36:51 +0300 Subject: [PATCH] FINERACT-2690: Fix undo account transfer to resolve and scope by transaction id undo() resolved the path id against m_account_transfer_details, but the transfer read/list APIs expose m_account_transfer_transaction.id instead - two independent id sequences that drift apart, so undo could 404 on a transfer a caller could clearly see. It also reversed every transaction tied to the resolved details record instead of only the one requested, over-reversing recurring transfers (e.g. standing instructions) that share one details row across many executions. Fixed for both the savings-to-loan (FINERACT-2604) and savings-to-savings (FINERACT-2613) paths. Also adds the missing UNDO_ACCOUNTTRANSFER permission (entityName= ACCOUNTTRANSFER, actionName=UNDO) referenced by the command wiring, which had no matching m_permission row and so failed permission validation. --- ...ountTransfersWritePlatformServiceImpl.java | 52 ++++++++++--------- .../db/changelog/tenant/changelog-tenant.xml | 1 + ...42_add_undo_accounttransfer_permission.xml | 40 ++++++++++++++ 3 files changed, 68 insertions(+), 25 deletions(-) create mode 100644 fineract-provider/src/main/resources/db/changelog/tenant/parts/0242_add_undo_accounttransfer_permission.xml diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java index 8fce2c4ecec..8c5e24c17ca 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java @@ -500,15 +500,21 @@ public AccountTransferDetails repayLoanWithTopup(AccountTransferDTO accountTrans @Override public CommandProcessingResult undo(JsonCommand command) { - AccountTransferDetails accountTransferDetails = accountTransferDetailRepository.findById(command.entityId()) - .orElseThrow(() -> new AccountTransferNotFoundException(command.entityId())); - - if (accountTransferDetails.getAccountTransferTransactions().stream().anyMatch(AccountTransferTransaction::isReversed)) { + final Long accountTransferId = command.entityId(); + // accountTransferId is the id exposed by the transfer read/list APIs, i.e. m_account_transfer_transaction.id. + // Resolving against that table (rather than m_account_transfer_details) means the lookup always matches + // what a caller can actually see, and reversal is scoped to this single transaction only - not every + // transaction sharing the same details record (e.g. a recurring standing instruction). + final AccountTransferTransaction transaction = accountTransferRepository.findById(accountTransferId) + .orElseThrow(() -> new AccountTransferNotFoundException(accountTransferId)); + + if (transaction.isReversed()) { throw new GeneralPlatformDomainRuleException("error.msg.account.transfer.already.reversed", - "Account transfer is already reverted", command.entityId()); + "Account transfer is already reverted", accountTransferId); } final PaymentDetail paymentDetail = null; + final AccountTransferDetails accountTransferDetails = transaction.getAccountTransferDetails(); PortfolioAccountType fromAccountType = accountTransferDetails.fromLoanAccount() != null ? PortfolioAccountType.LOAN : accountTransferDetails.fromSavingsAccount() != null ? PortfolioAccountType.SAVINGS : throwUnsupported(); @@ -517,32 +523,28 @@ public CommandProcessingResult undo(JsonCommand command) { : accountTransferDetails.toSavingsAccount() != null ? PortfolioAccountType.SAVINGS : throwUnsupported(); if (isSavingsToSavingsAccountTransfer(fromAccountType, toAccountType)) { - accountTransferDetails.getAccountTransferTransactions().forEach(transaction -> { - this.savingsAccountWritePlatformService.undoTransaction(transaction.getFromSavingsTransaction().getSavingsAccount().getId(), - transaction.getFromSavingsTransaction().getId(), true); - this.savingsAccountWritePlatformService.undoTransaction(transaction.getToSavingsTransaction().getSavingsAccount().getId(), - transaction.getToSavingsTransaction().getId(), true); - transaction.reverse(); - }); + this.savingsAccountWritePlatformService.undoTransaction(transaction.getFromSavingsTransaction().getSavingsAccount().getId(), + transaction.getFromSavingsTransaction().getId(), true); + this.savingsAccountWritePlatformService.undoTransaction(transaction.getToSavingsTransaction().getSavingsAccount().getId(), + transaction.getToSavingsTransaction().getId(), true); + transaction.reverse(); } else if (isSavingsToLoanAccountTransfer(fromAccountType, toAccountType)) { - accountTransferDetails.getAccountTransferTransactions().forEach(transaction -> { - this.savingsAccountWritePlatformService.undoTransaction(transaction.getFromSavingsTransaction().getSavingsAccount().getId(), - transaction.getFromSavingsTransaction().getId(), true); - final ExternalId reversalTxnExternalId = externalIdFactory.create(); - LoanAdjustmentParameter parameter = LoanAdjustmentParameter.builder().transactionAmount(BigDecimal.ZERO) - .paymentDetail(paymentDetail).transactionDate(transaction.getToLoanTransaction().getTransactionDate()) - .txnExternalId(transaction.getToLoanTransaction().getExternalId()).reversalTxnExternalId(reversalTxnExternalId) - .noteText(null).build(); - this.loanAdjustmentService.adjustLoanTransaction(transaction.getToLoanTransaction().getLoan(), - transaction.getToLoanTransaction(), parameter, null, new HashMap<>()); - transaction.reverse(); - }); + this.savingsAccountWritePlatformService.undoTransaction(transaction.getFromSavingsTransaction().getSavingsAccount().getId(), + transaction.getFromSavingsTransaction().getId(), true); + final ExternalId reversalTxnExternalId = externalIdFactory.create(); + LoanAdjustmentParameter parameter = LoanAdjustmentParameter.builder().transactionAmount(BigDecimal.ZERO) + .paymentDetail(paymentDetail).transactionDate(transaction.getToLoanTransaction().getTransactionDate()) + .txnExternalId(transaction.getToLoanTransaction().getExternalId()).reversalTxnExternalId(reversalTxnExternalId) + .noteText(null).build(); + this.loanAdjustmentService.adjustLoanTransaction(transaction.getToLoanTransaction().getLoan(), + transaction.getToLoanTransaction(), parameter, null, new HashMap<>()); + transaction.reverse(); } else if (isLoanToSavingsAccountTransfer(fromAccountType, toAccountType)) { throw new UnsupportedOperationException("Undo Loan to Savings Account Transfer is not implemented"); } final CommandProcessingResultBuilder builder = new CommandProcessingResultBuilder() // - .withEntityId(accountTransferDetails.getId()); + .withEntityId(transaction.getId()); return builder.build(); } diff --git a/fineract-provider/src/main/resources/db/changelog/tenant/changelog-tenant.xml b/fineract-provider/src/main/resources/db/changelog/tenant/changelog-tenant.xml index c603611fb2e..e158709f8a3 100644 --- a/fineract-provider/src/main/resources/db/changelog/tenant/changelog-tenant.xml +++ b/fineract-provider/src/main/resources/db/changelog/tenant/changelog-tenant.xml @@ -259,4 +259,5 @@ + diff --git a/fineract-provider/src/main/resources/db/changelog/tenant/parts/0242_add_undo_accounttransfer_permission.xml b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0242_add_undo_accounttransfer_permission.xml new file mode 100644 index 00000000000..9c32592e9ec --- /dev/null +++ b/fineract-provider/src/main/resources/db/changelog/tenant/parts/0242_add_undo_accounttransfer_permission.xml @@ -0,0 +1,40 @@ + + + + + + + SELECT COUNT(1) FROM m_permission WHERE code = 'UNDO_ACCOUNTTRANSFER' + + + + + + + + + + + +