Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from "react";
import { Provider } from "react-redux";
import { useLocation } from "react-router-dom";
import { renderHook } from "@testing-library/react";
import { renderHook, act } from "@testing-library/react";
import { StrKey } from "stellar-sdk";
import {
makeDummyStore,
Expand All @@ -22,6 +22,7 @@ import {
saveFederationAddress,
} from "popup/ducks/transactionSubmission";
import { initialState as transactionSubmissionInitialState } from "popup/ducks/transactionSubmission";
import { saveCollections } from "popup/ducks/cache";
import * as StellarHelpers from "@shared/helpers/stellar";
import * as SorobanHelpers from "@shared/api/helpers/soroban";

Expand Down Expand Up @@ -615,4 +616,59 @@ describe("useSendQueryParams", () => {
);
});
});

describe("In-flow asset selection (issue #2871)", () => {
// Repro: open a token's detail page -> Send (URL carries ?asset=<that token>
// for the whole flow) -> switch to a different token -> submit. The success
// ("Sent!") screen reads transactionData.asset, so the user's switched asset
// must survive any effect re-run that happens after the switch (e.g. the
// account/collections refresh triggered by a successful submit).
const switchedAsset =
"AQUA:GBNZILSTVQZ4R7IKQDGHYGY2QXL5QOFJYQMXPKWRRM5PAV7Y4M67AQUA";

it("does not revert the user's switched asset when the effect re-runs without a URL change", () => {
mockUseLocation.mockReturnValue({
pathname: "/send",
search: `?asset=${validAsset}`,
state: null,
});

const store = makeDummyStore(defaultState);
const Wrapper = ({ children }: { children: React.ReactNode }) => (
<Provider store={store}>{children}</Provider>
);
renderHook(() => useSendQueryParams(), { wrapper: Wrapper });

// Mount pre-populates the asset from the URL param.
expect(store.getState().transactionSubmission.transactionData.asset).toBe(
validAsset,
);

// The user switches the source asset mid-flow (SendDestinationAsset).
act(() => {
store.dispatch(saveAsset(switchedAsset));
});
expect(store.getState().transactionSubmission.transactionData.asset).toBe(
switchedAsset,
);

// An unrelated store change re-triggers the hook's effect without any URL
// change (mirrors the account/collections refresh after a successful send,
// which changes the collections dependency reference).
act(() => {
store.dispatch(
saveCollections({
networkDetails: MAINNET_NETWORK_DETAILS,
publicKey: TEST_PUBLIC_KEY,
collections: [],
}),
);
});

// The switched asset must survive — the URL param must not clobber it.
expect(store.getState().transactionSubmission.transactionData.asset).toBe(
switchedAsset,
);
});
});
});
22 changes: 19 additions & 3 deletions extension/src/popup/views/Send/hooks/useSendQueryParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,18 @@ export function useSendQueryParams() {
const networkDetails = useSelector(settingsNetworkDetailsSelector);
const { transactionData } = useSelector(transactionSubmissionSelector);

// Read transactionData.asset via a ref so the hook reacts only to URL
// changes — not to subsequent in-flow asset picks (which would otherwise
// re-dispatch the URL param and revert the user's selection).
// currentAssetRef lets the param handlers below read the latest selected
// asset without putting transactionData.asset in the effect deps (which would
// re-run the effect on every asset change).
const currentAssetRef = useRef(transactionData.asset);
currentAssetRef.current = transactionData.asset;

// Tracks the last location.search the effect actually pre-populated from, so
// re-runs triggered by other dependencies (e.g. the collections cache
// refreshing after a successful send) don't re-apply the URL params and
// revert an asset/destination the user changed mid-flow. (Fixes #2871.)
const lastAppliedSearchRef = useRef<string | null>(null);

useEffect(() => {
const params = new URLSearchParams(location.search);
const destinationParam = params.get("destination");
Expand Down Expand Up @@ -88,6 +94,16 @@ export function useSendQueryParams() {
}
}

// Only pre-populate destination/asset from the URL when location.search
// itself changes (initial mount or a new deep link). Re-runs caused by
// other dependencies must not re-apply the params and clobber what the user
// picked mid-flow — the collectible block above still re-runs because it
// depends on collections loading asynchronously. (Fixes #2871.)
if (lastAppliedSearchRef.current === location.search) {
return;
}
lastAppliedSearchRef.current = location.search;

// Pre-populate destination if provided and valid
if (destinationParam) {
const isValidDestination =
Expand Down
Loading