Fix wrong asset on "Sent!" screen after switching token mid-send - #2873
Merged
Conversation
…id-send (#2871) When Send is opened from a token's detail page, the URL carries ?asset=<that token> for the entire (linearized) flow. useSendQueryParams' effect re-dispatched saveAsset(<URL asset>) on every run, and its deps include the collections cache — which refreshes on a successful submit. So switching to a different token mid-flow was silently reverted to the URL token exactly at the "Sending..." -> "Sent!" transition, and the success screen (which reads transactionData.asset) showed the wrong token. 100% reproducible. The existing currentAssetRef guard only covered the no-param / invalid-param branches, not the valid-param branch that does the reverting. Gate the destination/asset URL pre-population on an actual location.search change via a lastAppliedSearchRef, so re-runs from other dependencies no longer clobber an asset/destination the user changed mid-flow. The collectible block still re-runs (it depends on collections loading asynchronously). Adds a regression test reproducing the revert. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CassioMG
changed the title
Contributor
|
PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-1dfd858e8564195ee12f (SDF collaborators only — install instructions in the release description) |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an extension Send-flow regression where the ?asset= query param could be re-applied after a successful submit (due to effect re-runs from cache refreshes), causing the “Sent!” success screen to display the wrong asset when the user switched tokens mid-flow.
Changes:
- Add a
lastAppliedSearchRefguard inuseSendQueryParamsso URL destination/asset pre-population only runs whenlocation.searchactually changes (while keeping collectible handling reactive to async collection loading). - Add a regression unit test that simulates an in-flow asset switch followed by an unrelated cache update, asserting the user-selected asset is not clobbered.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| extension/src/popup/views/Send/hooks/useSendQueryParams.ts | Prevents URL query param re-application on non-URL-driven effect re-runs by tracking the last applied location.search. |
| extension/src/popup/views/Send/hooks/tests/useSendQueryParams.test.tsx | Adds a regression test reproducing issue #2871 to ensure switched in-flow asset selection persists across unrelated store updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2871.
Opening Send from a token's detail page keeps
?asset=<token>in the URL for the whole (linearized) flow.useSendQueryParamsre-applied that param on every effect run, and its deps include the collections cache — which refreshes on a successful submit. So switching to another token mid-flow was reverted to the URL token right at the "Sending…" → "Sent!" transition, and the success screen (which readstransactionData.asset) showed the wrong token. 100% reproducible.The
currentAssetRefguard added in #2764 only covered the no-param / invalid-param branches — not the valid-param branch that does the reverting. This gates the destination/asset URL pre-population on an actuallocation.searchchange, so re-runs from other deps no longer clobber an in-flow selection. The collectible block still re-runs (it needs asynchronously-loaded collections).Regression test added.
🤖 Generated with Claude Code
Screen.Recording.2026-06-25.at.13.09.38.mov