You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fineract.apache.org by "Samer-Melhem-FOO (via GitHub)" <gi...@apache.org> on 2026/07/15 12:41:16 UTC
[PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Samer-Melhem-FOO opened a new pull request, #6136:
URL: https://github.com/apache/fineract/pull/6136
AccountTransfersWritePlatformServiceImpl.undo() had two related defects, discovered while testing the undo-account-transfer capability added in FINERACT-2604 and extended in FINERACT-2613:
1. Wrong lookup table. The endpoint's path id was resolved against m_account_transfer_details, but AccountTransfersReadPlatformServiceImpl.retrieveOne (which backs the transfer read/list APIs) queries by m_account_transfer_transaction.id. These are two
independent, unrelated id sequences that drift apart over time, so a caller undoing a transfer they can clearly see via the list/read endpoints could get a 404.
2. Over-reversal on recurring transfers. undo() reversed every AccountTransferTransaction tied to the resolved AccountTransferDetails record, rather than only the single transaction the caller asked to undo. This incorrectly reversed every execution of a
recurring transfer (e.g. a standing instruction) sharing one details row across many runs.
This PR fixes undo() to resolve and scope by m_account_transfer_transaction id instead, for both the savings-to-loan and savings-to-savings paths, so:
- the id passed to undo matches what the read/list APIs actually expose, and
- only the specific transaction requested is reversed, leaving sibling executions of a recurring transfer untouched.
It also adds the missing UNDO_ACCOUNTTRANSFER permission (entityName=ACCOUNTTRANSFER, actionName=UNDO) via a new Liquibase changelog part — the command handler was already wired to check this permission, but no matching m_permission row was ever shipped, so
the command failed permission validation before reaching the (buggy) logic above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "AshharAhmadKhan (via GitHub)" <gi...@apache.org>.
AshharAhmadKhan commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5215012953
hey @Samer-Melhem-FOO please resolve conflicts and rebase, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "AshharAhmadKhan (via GitHub)" <gi...@apache.org>.
AshharAhmadKhan commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5254624150
Hey @Samer-Melhem-FOO please rebase to latest upstream andd resolve changelog conflicts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "Samer-Melhem-FOO (via GitHub)" <gi...@apache.org>.
Samer-Melhem-FOO commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5176599597
Rebased onto latest develop. Also had to renumber the new changelog part from 0242 to 0244 to avoid colliding with 0242/0243 that landed on develop in the meantime.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "AshharAhmadKhan (via GitHub)" <gi...@apache.org>.
AshharAhmadKhan commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5569427497
@Samer-Melhem-FOO please check the failing logs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "AshharAhmadKhan (via GitHub)" <gi...@apache.org>.
AshharAhmadKhan commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5866847984
@Samer-Melhem-FOO resolve conflicts and rebase please.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "Samer-Melhem-FOO (via GitHub)" <gi...@apache.org>.
Samer-Melhem-FOO commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5264280947
Rebased onto latest `develop` and resolved the changelog conflict — `0245_add_undo_accounttransfer_permission.xml` was renumbered to `0248` since `0245`-`0247` were already taken upstream. Force-pushed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #6136:
URL: https://github.com/apache/fineract/pull/6136
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5408548585
@Samer-Melhem-FOO Can you please review the failing checks?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "AshharAhmadKhan (via GitHub)" <gi...@apache.org>.
AshharAhmadKhan commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5699480657
@Samer-Melhem-FOO please check the failing logs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "Samer-Melhem-FOO (via GitHub)" <gi...@apache.org>.
Samer-Melhem-FOO commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5541573583
@adamsaghy Rebased onto latest `develop` (renumbered the new changelog part from `0248` to `0254` to avoid colliding with `0248`-`0253` that landed upstream in the meantime) and fixed the actual CI failure: `AccountTransferOverdraftTest.preserveTransferLinkWhenBackdatedDepositRecalculatesOverdraft()` was calling `undo()` with the create-command's `resourceId` (`m_account_transfer_details.id`) — the old, buggy id space this PR's fix stops accepting. Updated it to use the transfer-transaction id instead, which is what `undo()` now correctly requires. Force-pushed, ready for another look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5759909619
@Samer-Melhem-FOO Please rebase and check failing checks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5526608946
@Samer-Melhem-FOO Are you planning to finish this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "Samer-Melhem-FOO (via GitHub)" <gi...@apache.org>.
Samer-Melhem-FOO commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5868678429
@AshharAhmadKhan Rebased on the latest develop and resolved the conflicts. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "Samer-Melhem-FOO (via GitHub)" <gi...@apache.org>.
Samer-Melhem-FOO commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5771698212
@adamsaghy Rebased onto the latest `develop` and force-pushed. The rebase was conflict-free this time, and `0254_add_undo_accounttransfer_permission.xml` is still free upstream, so no renumbering was needed.
I also pushed a second commit that was missing: `AccountTransferOverdraftTest.preserveTransferLinkWhenBackdatedDepositRecalculatesOverdraft()` was still calling `undo` with the create-command's `resourceId` (an `m_account_transfer_details.id`) instead of the transfer-transaction id this PR's fix requires. It now passes `withdrawal.getTransfer().getId()`, which the savings transactions mapper populates from `m_account_transfer_transaction.id` — the same key space `AccountTransfersReadPlatformServiceImpl.retrieveOne` looks up (`where att.id = ?`). It passed before only because both id sequences coincide in a fresh test database.
On the two failing checks in the previous run: both looked like CI infrastructure rather than this PR.
- `E2E Tests (Shard 9 of 20)` — the `fineract` container never became healthy (`curl … localhost:8443` returned `SSL_ERROR_SYSCALL` through all 30 retries), and the gradle invocation also hit `Failed to load eclipse groovy formatter: java.lang.IllegalArgumentException: 502 at https://download.eclipse.org/eclipse/updates/4.29/…/com.sun.jna_5.13.0.jar`.
- `regression-safety` — ended with `The runner has received a shutdown signal … The operation was canceled.` before it got to the changelog checks.
All 85 other checks were green, including every MariaDB integration shard. Locally on the rebased branch, `:fineract-provider:compileJava` and `:integration-tests:compileTestJava` both build clean and `spotlessJavaCheck` reports no violations in the touched files. Hopefully this run comes back green — happy to dig in if anything real shows up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "Samer-Melhem-FOO (via GitHub)" <gi...@apache.org>.
Samer-Melhem-FOO commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5215959872
Rebased onto latest develop and resolved the conflict in changelog-tenant.xml (renumbered the new changelog part to 0245 to avoid colliding with 0244_add_payment_detail_to_account_transfer_transaction.xml). Ready for another look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5164575684
@Samer-Melhem-FOO Please rebase this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "Samer-Melhem-FOO (via GitHub)" <gi...@apache.org>.
Samer-Melhem-FOO commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5832760311
@adamsaghy Rebased onto the latest `develop` again and force-pushed.
- The conflict was in `changelog-tenant.xml`: `0254` has since been taken upstream by `0254_transaction_summary_reports_exclude_reage_and_fix_amortization_mapping_sign.xml`, so the undo permission part is renumbered to **`0255_add_undo_accounttransfer_permission.xml`**. That is the only change; the Java code is unchanged from the previous push (`git range-diff` confirms it).
- The one failing check on the previous run (`run-e2e-tests / E2E Tests (Shard 7 of 20)`) failed only in `WorkingCapitalAmortizationScheduleMatrixPt1.feature` with `ClientCreateBusinessEvent hasn't been received within 15 seconds`, which is unrelated to account transfers. Could you please re-trigger the checks?
Note for ordering: #6144 (FINERACT-2615, Loan → Savings undo) builds on this change, so this PR should go in first; #6144 can then rebase and only carry the Loan → Savings branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "AshharAhmadKhan (via GitHub)" <gi...@apache.org>.
AshharAhmadKhan commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5917845158
hey @adamsaghy please review this whenever possible, thanks a lot!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5526598761
@Samer-Melhem-FOO Please rebase and fix the failing checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "AshharAhmadKhan (via GitHub)" <gi...@apache.org>.
AshharAhmadKhan commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5217739078
hey @adamsaghy please review this when you get a chance, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-2690: Fix undo account transfer to resolve and scope by tran… [fineract]
Posted by "Samer-Melhem-FOO (via GitHub)" <gi...@apache.org>.
Samer-Melhem-FOO commented on PR #6136:
URL: https://github.com/apache/fineract/pull/6136#issuecomment-5264298533
FYI @AshharAhmadKhan @adamsaghy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org