Skip to content

fix(transfer): handle decline and cancel across wallets - #601

Open
hariclerry wants to merge 1 commit into
Greenstand:masterfrom
hariclerry:fix/599-decline-cancel-multi-wallet
Open

hariclerry wants to merge 1 commit into
Greenstand:masterfrom
hariclerry:fix/599-decline-cancel-multi-wallet

Conversation

@hariclerry

Copy link
Copy Markdown

Description

Fix transfer decline and cancellation failures when a transfer belongs to a second wallet owned by the same account.

Issue(s) addressed

What kind of change(s) does this PR introduce?

  • Enhancement
  • Bug fix
  • Refactor

Please check if the PR fulfils these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Issue

What is the current behavior?

  • Declining or cancelling a transfer can return HTTP 500 when the transfer is associated with a second wallet. The service looks up the transfer using the login wallet, receives no result, and then attempts to read originating_wallet from undefined.

What is the new behavior?

  • Decline and cancel now use the existing transfer lookup helper. This preserves
    the account wallet visibility and authorization checks and returns a controlled 404 when the transfer is not found or is not related to the requesting account.

  • Added regression tests covering missing transfers for both decline and cancel,
    including transaction rollback and prevention of mutation or event logging.

Breaking change

Does this PR introduce a breaking change?

  • No breaking change.

Other useful information

  • Needs confirmation on whether the PR should target keycloak or `master

@dadiorchen

Copy link
Copy Markdown
Collaborator

@samwel141 can you help review this?

@samwel141

Copy link
Copy Markdown
Collaborator

@hariclerry you'll need to rebase onto keycloak and switch the base, it's still targeting master.

@samwel141

Copy link
Copy Markdown
Collaborator

But also its not a blocker it can be merged after release @dadiorchen, @mahdi2ba also mentioned in the issue #599

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Decline crashes with 500 when the transfer was sent to a second wallet

4 participants