Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(multichain): fix showFiat option for test assets #26224

Merged
merged 27 commits into from
Aug 1, 2024
Merged

fix(multichain): fix showFiat option for test assets #26224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
1e52ba4
fix(multichain): fix showFiat option for test assets
ccharly Jul 30, 2024
9fb4733
feat(multichain): add getMultichainIsTestnet + use it for accout-list…
ccharly Jul 31, 2024
bb7f238
fix(multichain): use account in getMultichainIsMainnet
ccharly Jul 31, 2024
44b5e49
refactor(multichain): remove old comment
ccharly Jul 31, 2024
3631381
feat: add more missing TEST_NETWORK_IDS
ccharly Jul 31, 2024
693d24a
chore: remove unused logs
ccharly Jul 31, 2024
3056b77
fix(multichain): checks for getMultichainShouldShowFiat in account-li…
ccharly Jul 31, 2024
536ed9b
fix(multichain): always use account.balance for EVM accounts
ccharly Jul 31, 2024
0f41494
test(multichain): use mainnet provider config in account-list-item + …
ccharly Jul 31, 2024
33e365d
test(multichain): use mainnet provider config in connect-accounts-mod…
ccharly Jul 31, 2024
4eb1225
chore: lint
ccharly Jul 31, 2024
7aa1c11
test: enable showFiatInTestnets in mock-state.json
ccharly Jul 31, 2024
ea15201
test: no longer uses mainnet provider config for connect-accounts-mod…
ccharly Jul 31, 2024
abacc02
test: fix ui/components/multichain/pages/send state + snapshot
ccharly Jul 31, 2024
aefb465
test: fix ui/components/multichain/pages/connections snapshot
ccharly Jul 31, 2024
09720cb
test: fix ui/components/app/user-preferenced-currency-input state
ccharly Jul 31, 2024
00a120d
test: fix ui/components/app/user-preferenced-token-input state
ccharly Jul 31, 2024
8216bc2
test: fix ui/components/ui/token-input state
ccharly Jul 31, 2024
39db594
test: fix ui/pages/confirmations/components/confirm/info/contract-int…
ccharly Jul 31, 2024
da414d1
test: fix ui/pages/confirmations/confirm-send-ether state
ccharly Jul 31, 2024
a248e9e
test: fix ui/pages/confirmations/confirmation/templates/remove-snap-a…
ccharly Jul 31, 2024
ef336a0
test: add test for "Show conversion on testnets" + update snapshot fo…
ccharly Jul 31, 2024
ed47113
test: fix e2e account-token-list fiat test
ccharly Jul 31, 2024
bdbdc12
test: revert back to showFiatInTestnet=false + re-update snapshots
ccharly Jul 31, 2024
05dacfc
feat(multichain): adapt logic for mainnet/testnet for getMultichainSh…
ccharly Jul 31, 2024
a18b2b9
chore: typo in ui/selectors/multichain.ts
ccharly Aug 1, 2024
24e70a6
refactor: revert newly added TEST_NETWORK_IDS
ccharly Aug 1, 2024
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
12 changes: 11 additions & 1 deletion test/e2e/tests/settings/account-token-list.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,16 @@ describe('Settings', function () {
tag: 'div',
});
await driver.clickElement({ text: 'Fiat', tag: 'label' });
// We now need to enable "Show fiat on testnet" if we are using testnets (and since our custom
// network during test is using a testnet chain ID, it will be considered as a test network)
await driver.clickElement({
text: 'Advanced',
tag: 'div',
});
await driver.clickElement('.show-fiat-on-testnets-toggle');
// Looks like when enabling the "Show fiat on testnet" it takes some time to re-update the
// overview screen, so just wait a bit here:
await driver.delay(1000);

await driver.clickElement(
'.settings-page__header__title-container__close-button',
Expand All @@ -68,7 +78,7 @@ describe('Settings', function () {
const tokenListAmount = await driver.findElement(
'.eth-overview__primary-container',
);
assert.equal(await tokenListAmount.getText(), '25\nETH');
assert.equal(await tokenListAmount.getText(), '$42,500.00\nUSD');
await driver.clickElement('[data-testid="account-menu-icon"]');
const accountTokenValue = await driver.waitForSelector(
'.multichain-account-list-item .multichain-account-list-item__asset',
Expand Down
6 changes: 3 additions & 3 deletions ui/components/multichain/account-list-item/__snapshots__/account-list-item.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,17 +534,17 @@ exports[`AccountListItem renders AccountListItem component and shows account nam
<div
class="mm-box currency-display-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
data-testid="first-currency-display"
title="$880.18 USD"
title="0.006 ETH"
>
<span
class="mm-box mm-text currency-display-component__text mm-text--inherit mm-text--ellipsis mm-box--color-text-default"
>
$880.18
0.006
</span>
<span
class="mm-box mm-text currency-display-component__suffix mm-text--inherit mm-box--margin-inline-start-1 mm-box--color-text-default"
>
USD
ETH
</span>
</div>
</div>
Expand Down
28 changes: 13 additions & 15 deletions ui/components/multichain/account-list-item/account-list-item.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,13 @@ import {
getSelectedInternalAccount,
} from '../../../selectors';
import {
getMultichainIsTestnet,
getMultichainNativeCurrency,
getMultichainNativeCurrencyImage,
getMultichainNetwork,
getMultichainShouldShowFiat,
} from '../../../selectors/multichain';
import { useMultichainAccountTotalFiatBalance } from '../../../hooks/useMultichainAccountTotalFiatBalance';
import { TEST_NETWORKS } from '../../../../shared/constants/network';
import { ConnectedStatus } from '../connected-status/connected-status';
///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
import { getCustodianIconForAddress } from '../../../selectors/institutional/selectors';
Expand Down Expand Up @@ -91,30 +92,29 @@ export const AccountListItem = ({
useState();

const useBlockie = useSelector(getUseBlockie);
const { network: currentNetwork, isEvmNetwork } = useMultichainSelector(
getMultichainNetwork,
account,
);
const { isEvmNetwork } = useMultichainSelector(getMultichainNetwork, account);
const setAccountListItemMenuRef = (ref) => {
setAccountListItemMenuElement(ref);
};
const isTestnet = useMultichainSelector(getMultichainIsTestnet, account);
const isMainnet = !isTestnet;
const shouldShowFiat = useMultichainSelector(
getMultichainShouldShowFiat,
account,
);
const showFiatInTestnets = useSelector(getShowFiatInTestnets);
const showFiat =
!isEvmNetwork ||
(TEST_NETWORKS.includes(currentNetwork?.nickname) && !showFiatInTestnets);
shouldShowFiat && (isMainnet || (isTestnet && showFiatInTestnets));
const accountTotalFiatBalances =
useMultichainAccountTotalFiatBalance(account);
const mappedOrderedTokenList = accountTotalFiatBalances.orderedTokenList.map(
(item) => ({
avatarValue: item.iconUrl,
}),
);
let balanceToTranslate = isEvmNetwork
? accountTotalFiatBalances.totalWeiBalance
const balanceToTranslate = isEvmNetwork
? account.balance
: accountTotalFiatBalances.totalBalance;
if (showFiat && isEvmNetwork) {
balanceToTranslate = account.balance;
}

///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
const custodianIcon = useSelector((state) =>
Expand Down Expand Up @@ -309,9 +309,7 @@ export const AccountListItem = ({
ethNumberOfDecimals={MAXIMUM_CURRENCY_DECIMALS}
value={balanceToTranslate}
type={PRIMARY}
showFiat={
!showFiat || !TEST_NETWORKS.includes(currentNetwork?.nickname)
}
showFiat={showFiat}
data-testid="first-currency-display"
/>
</Text>
Expand Down
20 changes: 16 additions & 4 deletions ui/components/multichain/account-list-item/account-list-item.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,9 @@ describe('AccountListItem', () => {
chainId: CHAIN_IDS.SEPOLIA,
nickname: SEPOLIA_DISPLAY_NAME,
},
preferences: {
showFiatInTestnets: false,
},
},
},
);
Expand Down Expand Up @@ -263,7 +266,7 @@ describe('AccountListItem', () => {
'[data-testid="avatar-group"]',
);

const expectedBalance = '$0.00';
const expectedBalance = '$3.31';

expect(firstCurrencyDisplay).toBeInTheDocument();
expect(firstCurrencyDisplay.firstChild.textContent).toContain(
Expand All @@ -275,9 +278,18 @@ describe('AccountListItem', () => {
});

it('renders fiat for non-EVM account', () => {
const { container } = render({
account: mockNonEvmAccount,
});
const { container } = render(
{
account: mockNonEvmAccount,
},
{
metamask: {
preferences: {
showFiatInTestnets: true,
},
},
},
);

const firstCurrencyDisplay = container.querySelector(
'[data-testid="first-currency-display"]',
Expand Down
6 changes: 3 additions & 3 deletions ...ents/multichain/connect-accounts-modal/__snapshots__/connect-accounts-modal.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,17 +354,17 @@ exports[`Connect More Accounts Modal should render correctly 1`] = `
<div
class="mm-box currency-display-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
data-testid="first-currency-display"
title="$0.00 USD"
title="0 ETH"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this snapshot was wrong for quite some time now.

The providerConfig on mock-state.json here is 0x5 (goerli), which is a testnet.

Test assets/accounts should not show any fiat values unless showFiatInTestnets is set to true (but it's currently false here)

You will see a bunch of snapshots changes in this PR, because of this ⬆️.

>
<span
class="mm-box mm-text currency-display-component__text mm-text--inherit mm-text--ellipsis mm-box--color-text-default"
>
$0.00
0
</span>
<span
class="mm-box mm-text currency-display-component__suffix mm-text--inherit mm-box--margin-inline-start-1 mm-box--color-text-default"
>
USD
ETH
</span>
</div>
</div>
Expand Down
6 changes: 3 additions & 3 deletions ui/components/multichain/pages/connections/__snapshots__/connections.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,17 +293,17 @@ exports[`Connections Content should render correctly 1`] = `
<div
class="mm-box currency-display-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
data-testid="first-currency-display"
title="$880.18 USD"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not track why the fiat balance was actually this low in the original snapshot.

However, based on this account's balance, the actual balance should be:

0x346ba7725f412cbfdb # Hex (WEI)

# Convert to decimal
966987986469506500000 # Decimal (WEI)

# Convert to ETH
966.9879864695065 # Decimal (ETH)

So here again, I guess the snapshot was wrong too. Also, the same comment applies here, so no more fiat.

title="966.988 ETH"
>
<span
class="mm-box mm-text currency-display-component__text mm-text--inherit mm-text--ellipsis mm-box--color-text-default"
>
$880.18
966.988
</span>
<span
class="mm-box mm-text currency-display-component__suffix mm-text--inherit mm-box--margin-inline-start-1 mm-box--color-text-default"
>
USD
ETH
</span>
</div>
</div>
Expand Down
8 changes: 5 additions & 3 deletions ui/components/multichain/pages/send/__snapshots__/send.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,11 +467,13 @@ exports[`SendPage render and initialization should render correctly even when a
<div
class="mm-box currency-display-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
data-testid="first-currency-display"
title="null USD"
title="$0.00 USD"
>
<span
class="mm-box mm-text currency-display-component__text mm-text--inherit mm-text--ellipsis mm-box--color-text-default"
/>
>
$0.00
</span>
<span
class="mm-box mm-text currency-display-component__suffix mm-text--inherit mm-box--margin-inline-start-1 mm-box--color-text-default"
>
Expand Down Expand Up @@ -500,7 +502,7 @@ exports[`SendPage render and initialization should render correctly even when a
<div
class="mm-box mm-text mm-avatar-base mm-avatar-base--size-xs mm-avatar-token mm-text--body-xs mm-text--text-transform-uppercase mm-box--display-flex mm-box--justify-content-center mm-box--align-items-center mm-box--color-text-default mm-box--background-color-background-alternative mm-box--rounded-full mm-box--border-color-border-default box--border-style-solid box--border-width-1"
>
?
E
</div>
<div
class="mm-box mm-text mm-text--body-sm mm-text--text-align-end mm-box--color-text-alternative"
Expand Down
36 changes: 18 additions & 18 deletions ui/components/multichain/pages/send/components/__snapshots__/your-accounts.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,17 +241,17 @@ exports[`SendPageYourAccounts render renders correctly 1`] = `
<div
class="mm-box currency-display-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
data-testid="first-currency-display"
title="$880.18 USD"
title="966.988 ETH"
>
<span
class="mm-box mm-text currency-display-component__text mm-text--inherit mm-text--ellipsis mm-box--color-text-default"
>
$880.18
966.988
</span>
<span
class="mm-box mm-text currency-display-component__suffix mm-text--inherit mm-box--margin-inline-start-1 mm-box--color-text-default"
>
USD
ETH
</span>
</div>
</div>
Expand Down Expand Up @@ -538,17 +538,17 @@ exports[`SendPageYourAccounts render renders correctly 1`] = `
<div
class="mm-box currency-display-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
data-testid="first-currency-display"
title="$0.00 USD"
title="0 ETH"
>
<span
class="mm-box mm-text currency-display-component__text mm-text--inherit mm-text--ellipsis mm-box--color-text-default"
>
$0.00
0
</span>
<span
class="mm-box mm-text currency-display-component__suffix mm-text--inherit mm-box--margin-inline-start-1 mm-box--color-text-default"
>
USD
ETH
</span>
</div>
</div>
Expand Down Expand Up @@ -835,17 +835,17 @@ exports[`SendPageYourAccounts render renders correctly 1`] = `
<div
class="mm-box currency-display-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
data-testid="first-currency-display"
title="$0.00 USD"
title="0 ETH"
>
<span
class="mm-box mm-text currency-display-component__text mm-text--inherit mm-text--ellipsis mm-box--color-text-default"
>
$0.00
0
</span>
<span
class="mm-box mm-text currency-display-component__suffix mm-text--inherit mm-box--margin-inline-start-1 mm-box--color-text-default"
>
USD
ETH
</span>
</div>
</div>
Expand Down Expand Up @@ -1141,17 +1141,17 @@ exports[`SendPageYourAccounts render renders correctly 1`] = `
<div
class="mm-box currency-display-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
data-testid="first-currency-display"
title="$0.00 USD"
title="0 ETH"
>
<span
class="mm-box mm-text currency-display-component__text mm-text--inherit mm-text--ellipsis mm-box--color-text-default"
>
$0.00
0
</span>
<span
class="mm-box mm-text currency-display-component__suffix mm-text--inherit mm-box--margin-inline-start-1 mm-box--color-text-default"
>
USD
ETH
</span>
</div>
</div>
Expand Down Expand Up @@ -1438,17 +1438,17 @@ exports[`SendPageYourAccounts render renders correctly 1`] = `
<div
class="mm-box currency-display-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
data-testid="first-currency-display"
title="$0.00 USD"
title="0 ETH"
>
<span
class="mm-box mm-text currency-display-component__text mm-text--inherit mm-text--ellipsis mm-box--color-text-default"
>
$0.00
0
</span>
<span
class="mm-box mm-text currency-display-component__suffix mm-text--inherit mm-box--margin-inline-start-1 mm-box--color-text-default"
>
USD
ETH
</span>
</div>
</div>
Expand Down Expand Up @@ -1748,17 +1748,17 @@ exports[`SendPageYourAccounts render renders correctly 1`] = `
<div
class="mm-box currency-display-component mm-box--display-flex mm-box--flex-wrap-wrap mm-box--align-items-center"
data-testid="first-currency-display"
title="$0.00 USD"
title="0 ETH"
>
<span
class="mm-box mm-text currency-display-component__text mm-text--inherit mm-text--ellipsis mm-box--color-text-default"
>
$0.00
0
</span>
<span
class="mm-box mm-text currency-display-component__suffix mm-text--inherit mm-box--margin-inline-start-1 mm-box--color-text-default"
>
USD
ETH
</span>
</div>
</div>
Expand Down
4 changes: 4 additions & 0 deletions ui/components/multichain/pages/send/send.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ const baseStore = {
providerConfig: {
chainId: CHAIN_IDS.GOERLI,
nickname: GOERLI_DISPLAY_NAME,
ticker: 'ETH',
},
nativeCurrency: 'ETH',
featureFlags: {
Expand Down Expand Up @@ -215,6 +216,8 @@ const baseStore = {
},
},
completedOnboarding: true,
useCurrencyRateCheck: true,
ticker: 'ETH',
},
activeTab: {
origin: 'https://uniswap.org/',
Expand Down Expand Up @@ -379,6 +382,7 @@ describe('SendPage', () => {
chainId: CHAIN_IDS.GOERLI,
nickname: GOERLI_DISPLAY_NAME,
type: NETWORK_TYPES.GOERLI,
ticker: 'ETH',
},
},
};
Expand Down
Loading