-
Notifications
You must be signed in to change notification settings - Fork 38
Bugfix/update custom token balance #2286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| )} ${networkDetails.network}`, | ||
| ); | ||
| } | ||
| onCancel(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one small change here - I noticed when adding a trustline, we just dismiss the slideup modal, but on custom tokens, we were nav'ing back to Account. I'm matching the trustline UX and just dismissing the modal
| await expect( | ||
| page.getByTestId("ManageAssetRowButton__ellipsis-E2E"), | ||
| ).toBeVisible(); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we assert that the new balance is visible on the account page before removing it?
Also, if this test cleans up state by removing the token and it's not actually part of the test, then maybe that code should go into an afterAll block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I can add a test to make sure it's on the balances page
For your second point, we're actually testing that remove works (and that it properly removes the token from the UI). Custom tokens don't persist, so there isn't really a need to clean up state. So, I think it makes sense to leave this part of the test here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh gotcha, then maybe we can just rename the test to reflect that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed here: c8c632a
* upgrade to yarn 2 and use resolutions to block vulnerable package versions (#2239) * upgrade to yarn 2 and use resolutions to block vulnerable package versions * rm deprecated .yarnrc * rm yarnpath * try committing yarn binary to repo * try corepack enable for gha * update run tests cmd * rm yarnpath * rm npm i yarn * update all pipelines * rm superfluous history types * ensure invoke host function tx shows contract parameters (#2243) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * Bugfix/rm auth param names (#2244) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * do not show contract parameters for authorizations * add tests for create contract v1 and invoke contract * add issuer for changeTrust op (#2246) * add issuer for changeTrust op * programmatically disable overflow:hidden when copying a value * Revert "add issuer for changeTrust op (#2246)" (#2247) This reverts commit 19c8a68. * Bugfix/add issuer for changetrust (#2249) * ensure invoke host function tx shows contract parameters * add test for fallback if contract spec retrieval fails * do not show contract parameters for authorizations * add tests for create contract v1 and invoke contract * add issuer for changeTrust op * programmatically disable overflow:hidden when copying a value * cache account balances and poll for updates (#2252) * cache account balances and poll for updates * fix CI tests * rm `force:true` which was causing action to happen too fast * do a fresh balance fetch on account/network change * pr comments * add more sentry tracking for Account and Wallets views (#2268) * add more sentry tracking for Account and Wallets views * adding more sentry reporting * gracefully degrade on errors from Blockaid (#2269) * gracefully degrade on errors from Blockaid * should not be necessary to skip dapp scanning on custom network * rm extra dep * add a test for persisting configurations in the send flow (#2271) * add a test for persisting configurations in the send flow * rm logs and update muxed acct; lower xlm payment * handle missing scan-tx result; add disabled state for Confirm Anyway (#2272) * handle missing scan-tx result; add disabled state for Confirm Anyway * assertions to show correct confirm button on Blockaid error * add cache for balances to ensure we do a fresh lookup when needed (#2275) * add cache for balances to ensure we do a fresh lookup when needed * add try...catch to token-prices polling * rm log * only dispatch saveBalancesForAccount when fresh data has been fetched * adjust test to wait for UI change * replace yarn setup with just yarn * after adding/removing asset: update status (#2285) * Bugfix/update custom token balance (#2286) * after adding/removing asset: update status * update token balances after adding/removing token * update comment * test for showing e2e token in account balances view * remove event properties from blockaid metric event properties (#2288) * Bugfix/fix avail balance (#2291) * better logic for preventing negative numbers in available balance * simplify minimum balance calc by using BE's value * update mock data
Related to #2248
After adding a custom token, it should refresh the balance for the user so the UI updates properly