FINERACT-2615: Support undoing of Account transfer from Loan account to Savings account

[AshharAhmadKhan]

JIRA

https://issues.apache.org/jira/browse/FINERACT-2615

Problem

AccountTransfersWritePlatformServiceImpl.undoTransfer(...) did not support Loan-to-Savings transfers and immediately threw an UnsupportedOperationException.

As a result, users could successfully perform transfers such as loan refunds from a loan account to a savings account, but attempting to undo the transfer failed. Both the loan and savings accounts remained affected because the transfer could not be reversed.

Fix

Added undo support for Loan-to-Savings transfers.

The implementation now reverses both sides of the transfer:

• Undoes the savings-side deposit through SavingsAccountWritePlatformService.undoTransaction(...).
• Reverses the loan-side refund through LoanAccountDomainService.reverseTransfer(...).
• Marks the corresponding AccountTransferTransaction as reversed, ensuring repeated undo attempts are correctly rejected with error.msg.account.transfer.already.reversed.

Why reverseTransfer(...) instead of adjustLoanTransaction(...)?

An earlier implementation (#5877) attempted to mirror the existing Savings-to-Loan undo path by calling LoanAdjustmentService.adjustLoanTransaction(...).

That approach cannot handle Loan-to-Savings transfers because adjustLoanTransaction(...) only permits repayment-like transaction types (REPAYMENT, DOWN_PAYMENT, MERCHANT_ISSUED_REFUND, etc.). The loan-side transaction created by a Loan-to-Savings transfer is a plain REFUND, so calling adjustLoanTransaction(...) results in an InvalidLoanTransactionTypeException.

LoanAccountDomainService.reverseTransfer(...) is already used elsewhere in AccountTransfersWritePlatformServiceImpl (undoTransactions(), invoked by reverseAllTransactions(...) and reverseTransfersWithFromAccountType(...)) to reverse this exact type of transaction, making it the correct and consistent implementation here.

Tests

• Verified Loan-to-Savings transfers can now be successfully undone.
• Verified both the loan and savings sides are correctly reversed.
• Verified the AccountTransferTransaction is marked as reversed after undo.
• Added a regression test confirming a second undo attempt is rejected with error.msg.account.transfer.already.reversed.

[AshharAhmadKhan]

@adamsaghy please trigger checks whenever possible, thanks a lot!

[AshharAhmadKhan]

@adamsaghy please trigger checks whenever possible, thanks!

[AshharAhmadKhan]

Hey @adamsaghy

While debugging the failing e2e tests, I found the issue wasn't actually in the new undo logic. reverseTransfer() was missing a call to loanTransaction.manuallyAdjustedOrReversed(), which the other reversal paths already make. I added that, fixed a couple of small expectation issues in the feature file, and now everything passes locally, including all AccountTransfer scenarios and the unit tests.

When you get a chance, could you trigger the checks again? Thanks!

[AshharAhmadKhan]

hey @adamsaghy
The failure looks unrelated to my changes, so I think it might be flaky. Happy to take another look if it fails again on a fresh run, but for now could you please re-trigger the check?

[AshharAhmadKhan]

Hey @adamsaghy , double-checked everything. Please review it when you get a chance. Let me know if you need any changes.

[AshharAhmadKhan]

Hi @adamsaghy! Just following up on this PR. It's ready for review whenever you have a chance. Thanks!

[adamsaghy]

@AshharAhmadKhan Please review #6136 (comment) and whether your PR has the same issue or not...

[AshharAhmadKhan]

Hey @adamsaghy , thanks for the notice.

#6136 does touch the same undo() method, but it doesn't add support for the loan to savings case. It only fixes the lookup and scoping logic for the branches that already existed. My PR is completing the last piece of the pattern from FINERACT-2604 and FINERACT-2613, so it's still needed on top of that fix.

Right now my loan to savings branch is written in the old loop style though, so once #6136 merges it'll conflict. I'll wait for that to land, rebase, and rewrite my branch to match their single transaction pattern instead.

Thanks again for pointing this out!

[adamsaghy]

Hey @adamsaghy , thanks for the notice.

#6136 does touch the same undo() method, but it doesn't add support for the loan to savings case. It only fixes the lookup and scoping logic for the branches that already existed. My PR is completing the last piece of the pattern from FINERACT-2604 and FINERACT-2613, so it's still needed on top of that fix.

Right now my loan to savings branch is written in the old loop style though, so once #6136 merges it'll conflict. I'll wait for that to land, rebase, and rewrite my branch to match their single transaction pattern instead.

Thanks again for pointing this out!

It was not just different handling, but it is using different entity to be fetched by id. Please double check whether your implementation is fetching the correct entity

[AshharAhmadKhan]

Hey Adam, thanks for pushing on this.

You're right, it was wrong. My branch fetches via accountTransferDetailRepository.findById(command.entityId()), which is the wrong entity for the id being passed in. #6136 resolves it correctly through accountTransferRepository.

I'll fix it up once #6136 merges and rebase on top.

[AshharAhmadKhan]

Hey @adamsaghy, I made some changes. Can you please review them and let me know if I need to change anything?

[adamsaghy]

@Samer-Melhem-FOO Please review.

[AshharAhmadKhan]

Hi @adamsaghy , this is ready for review. @Samer-Melhem-FOO , please feel free to review as well, and if either of you has anything to add or any further changes are needed, please let me know.

[AshharAhmadKhan]

Hi @adamsaghy, just a quick ping on this PR when you get a chance. It’s ready for review. Thanks!

[adamsaghy]

@AshharAhmadKhan I need some extra time to review this. Also #6136 is still open... :/

[AshharAhmadKhan]

@AshharAhmadKhan I need some extra time to review this. Also #6136 is still open... :/

hey adam, no problem at all haha. Please take all the time you need!

[AshharAhmadKhan]

Both failures are unrelated to my changes. Please retrigger when possible.