Conversation
|
Preview is available here: |
|
Preview is available here: |
There was a problem hiding this comment.
Pull request overview
Updates the Fund Account flow and signing logic to better reflect funded-account UI behavior and to handle extension-wallet signing outputs more safely.
Changes:
- Hide the XLM funding option once an account is detected as already funded, and move trustline help text/link into a footer-style hint area.
- Adjust E2E tests to account for token list re-indexing after XLM is hidden and update the “already funded” error scenario mocking.
- In
SignTransactionXdr, use the extension wallet’s returned signed XDR directly when it’s the only signer to avoid signature invalidation when wallets mutate transactions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/e2e/fundAccountPage.test.ts | Updates token-button selection after XLM is hidden and tweaks accounts API mocking for the already-funded error case. |
| src/components/SignTransactionXdr/index.tsx | Uses extension wallet signed XDR directly for extension-only signing to avoid invalid signatures from wallet-modified transactions. |
| src/app/(sidebar)/account/fund/page.tsx | Hides XLM token for funded accounts and relocates trustline explanatory content/link below the token list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| // Mock accounts API call - return unfunded first so XLM button is visible, | ||
| // then funded after the fund attempt | ||
| let accountsCallCount = 0; | ||
|
|
||
| await page.route( | ||
| `*/**/accounts/${TEST_ACCOUNT_PUBLIC_KEY}`, | ||
| async (route) => { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: "application/hal+json; charset=utf-8", | ||
| body: JSON.stringify(MOCK_ACCOUNT_FUNDED_RESPONSE), | ||
| }); | ||
| if (accountsCallCount === 0) { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: "application/hal+json; charset=utf-8", | ||
| body: JSON.stringify(MOCK_ACCOUNT_UNFUNDED_RESPONSE), | ||
| }); | ||
| } else { | ||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: "application/hal+json; charset=utf-8", | ||
| body: JSON.stringify(MOCK_ACCOUNT_FUNDED_RESPONSE), | ||
| }); | ||
| } | ||
|
|
||
| accountsCallCount++; | ||
| }, |
There was a problem hiding this comment.
The route handler uses a shared accountsCallCount that’s incremented after awaiting route.fulfill(). If the app issues multiple /accounts/:id requests concurrently (possible since fetchAccountInfo() can be triggered by multiple effects), two handlers can observe the same count and return the same response, making this test flaky. Capture and increment the call index synchronously at the start of the handler (e.g., const callIndex = accountsCallCount++) and branch on callIndex instead.
| <div | ||
| key={t.id} | ||
| className="Account__fundTokens__item" | ||
| data-testid="fund-account-token" |
There was a problem hiding this comment.
data-testid="fund-account-token" is reused for every token row. With XLM now conditionally hidden for funded accounts, E2E selectors that rely on nth() become much more brittle (indexes change as items appear/disappear). Consider making the test id include the token id (or add a data-asset-id attribute) so tests can target USDC/EURC/XLM directly without depending on list order.
| data-testid="fund-account-token" | |
| data-testid={`fund-account-token-${t.id}`} | |
| data-asset-id={t.id} |
waiting for #1949 be merged