Skip to content

Commit

Permalink
refactor: Init getTokenNetworkFilter selector (#29068)
Browse files Browse the repository at this point in the history
## **Description**

We are falling back to an empty object when tokenNetworkFilter is
undefined in several areas around the app. This PR aims to consolidate
those within a single selector to make is easier to maintain and fall
back on.

This only mitigates the issue. Root cause is that the
`tokenNetworkFilter` needs to be added via a migration script in order
to properly be present, without having to fallback.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29068?quickstart=1)

## **Related issues**

Fixes:
https://metamask.sentry.io/issues/6125799129/?environment=production&project=273505&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D%20firstRelease%3A12.9.0&referrer=issue-stream&sort=freq&statsPeriod=7d&stream_index=0

## **Manual testing steps**

1. Hardcode tokenNetworkFilter to undefined, should not break app.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: Brian Bergeron <brian.e.bergeron@gmail.com>
  • Loading branch information
gambinish and bergeron authored Dec 10, 2024
1 parent 653177a commit e7bba6d
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import React, { useEffect, useRef, useState, useContext, useMemo } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { getCurrentNetwork, getPreferences } from '../../../../../selectors';
import {
getCurrentNetwork,
getTokenNetworkFilter,
} from '../../../../../selectors';
import { getNetworkConfigurationsByChainId } from '../../../../../../shared/modules/selectors/networks';
import {
Box,
Expand Down Expand Up @@ -55,7 +58,7 @@ const AssetListControlBar = ({ showTokensLinks }: AssetListControlBarProps) => {
const currentNetwork = useSelector(getCurrentNetwork);
const allNetworks = useSelector(getNetworkConfigurationsByChainId);

const { tokenNetworkFilter } = useSelector(getPreferences);
const tokenNetworkFilter = useSelector(getTokenNetworkFilter);
const [isTokenSortPopoverOpen, setIsTokenSortPopoverOpen] = useState(false);
const [isImportTokensPopoverOpen, setIsImportTokensPopoverOpen] =
useState(false);
Expand All @@ -72,7 +75,7 @@ const AssetListControlBar = ({ showTokensLinks }: AssetListControlBarProps) => {
});

const allNetworksFilterShown =
Object.keys(tokenNetworkFilter || {}).length !==
Object.keys(tokenNetworkFilter).length !==
Object.keys(allOpts || {}).length;

useEffect(() => {
Expand All @@ -88,7 +91,7 @@ const AssetListControlBar = ({ showTokensLinks }: AssetListControlBarProps) => {
useEffect(() => {
if (
process.env.PORTFOLIO_VIEW &&
Object.keys(tokenNetworkFilter || {}).length === 0
Object.keys(tokenNetworkFilter).length === 0
) {
dispatch(setTokenNetworkFilter(allOpts));
} else {
Expand All @@ -99,7 +102,7 @@ const AssetListControlBar = ({ showTokensLinks }: AssetListControlBarProps) => {
// When a network gets added/removed we want to make sure that we switch to the filtered list of the current network
// We only want to do this if the "Current Network" filter is selected
useEffect(() => {
if (Object.keys(tokenNetworkFilter || {}).length === 1) {
if (Object.keys(tokenNetworkFilter).length === 1) {
dispatch(setTokenNetworkFilter({ [currentNetwork.chainId]: true }));
}
}, [Object.keys(allNetworks).length]);
Expand Down
6 changes: 3 additions & 3 deletions ui/components/app/assets/asset-list/asset-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import {
getAllDetectedTokensForSelectedAddress,
getDetectedTokensInCurrentNetwork,
getIstokenDetectionInactiveOnNonMainnetSupportedNetwork,
getPreferences,
getSelectedAccount,
getTokenNetworkFilter,
} from '../../../../selectors';
import { getNetworkConfigurationsByChainId } from '../../../../../shared/modules/selectors/networks';
import {
Expand Down Expand Up @@ -80,14 +80,14 @@ const AssetList = ({ onClickAsset, showTokensLinks }: AssetListProps) => {
);

const allNetworks = useSelector(getNetworkConfigurationsByChainId);
const { tokenNetworkFilter } = useSelector(getPreferences);
const tokenNetworkFilter = useSelector(getTokenNetworkFilter);
const allOpts: Record<string, boolean> = {};
Object.keys(allNetworks || {}).forEach((chainId) => {
allOpts[chainId] = true;
});

const allNetworksFilterShown =
Object.keys(tokenNetworkFilter || {}).length !==
Object.keys(tokenNetworkFilter).length !==
Object.keys(allOpts || {}).length;

const [showFundingMethodModal, setShowFundingMethodModal] = useState(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ import { useDispatch, useSelector } from 'react-redux';
import { setTokenNetworkFilter } from '../../../../../store/actions';
import {
getCurrentNetwork,
getPreferences,
getShouldHideZeroBalanceTokens,
getSelectedAccount,
getAllChainsToPoll,
getTokenNetworkFilter,
} from '../../../../../selectors';
import {
getCurrentChainId,
Expand Down Expand Up @@ -48,7 +48,7 @@ const NetworkFilter = ({ handleClose }: SortControlProps) => {
const selectedAccount = useSelector(getSelectedAccount);
const allNetworks = useSelector(getNetworkConfigurationsByChainId);
const [chainsToShow, setChainsToShow] = useState<string[]>([]);
const { tokenNetworkFilter } = useSelector(getPreferences);
const tokenNetworkFilter = useSelector(getTokenNetworkFilter);
const shouldHideZeroBalanceTokens = useSelector(
getShouldHideZeroBalanceTokens,
);
Expand Down Expand Up @@ -102,7 +102,7 @@ const NetworkFilter = ({ handleClose }: SortControlProps) => {
<>
<SelectableListItem
isSelected={
Object.keys(tokenNetworkFilter || {}).length ===
Object.keys(tokenNetworkFilter).length ===
Object.keys(allNetworks || {}).length
}
onClick={() => handleFilter(allOpts)}
Expand Down Expand Up @@ -163,8 +163,8 @@ const NetworkFilter = ({ handleClose }: SortControlProps) => {
</SelectableListItem>
<SelectableListItem
isSelected={
tokenNetworkFilter[chainId] &&
Object.keys(tokenNetworkFilter || {}).length === 1
Object.keys(tokenNetworkFilter).length === 1 &&
tokenNetworkFilter[chainId]
}
onClick={() => handleFilter({ [chainId]: true })}
testId="network-filter-current"
Expand Down
7 changes: 4 additions & 3 deletions ui/components/app/assets/token-list/token-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
getSelectedAccountTokensAcrossChains,
getShowFiatInTestnets,
getTokenExchangeRates,
getTokenNetworkFilter,
} from '../../../../selectors';
import { getConversionRate } from '../../../../ducks/metamask/metamask';
import { filterAssets } from '../util/filter';
Expand Down Expand Up @@ -86,8 +87,8 @@ export default function TokenList({
const dispatch = useDispatch();
const currentNetwork = useSelector(getCurrentNetwork);
const allNetworks = useSelector(getNetworkConfigurationIdByChainId);
const { tokenSortConfig, tokenNetworkFilter, privacyMode } =
useSelector(getPreferences);
const { tokenSortConfig, privacyMode } = useSelector(getPreferences);
const tokenNetworkFilter = useSelector(getTokenNetworkFilter);
const selectedAccount = useSelector(getSelectedAccount);
const conversionRate = useSelector(getConversionRate);
const contractExchangeRates = useSelector(
Expand Down Expand Up @@ -116,7 +117,7 @@ export default function TokenList({
const allNetworkFilters = Object.fromEntries(
Object.keys(allNetworks).map((chainId) => [chainId, true]),
);
if (Object.keys(tokenNetworkFilter || {}).length > 1) {
if (Object.keys(tokenNetworkFilter).length > 1) {
dispatch(setTokenNetworkFilter(allNetworkFilters));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
getAllDetectedTokensForSelectedAddress,
getCurrentNetwork,
getDetectedTokensInCurrentNetwork,
getPreferences,
getTokenNetworkFilter,
} from '../../../../selectors';

import Popover from '../../../ui/popover';
Expand All @@ -41,14 +41,14 @@ const DetectedTokenSelectionPopover = ({

const detectedTokens = useSelector(getDetectedTokensInCurrentNetwork);
const allNetworks = useSelector(getNetworkConfigurationsByChainId);
const { tokenNetworkFilter } = useSelector(getPreferences);
const tokenNetworkFilter = useSelector(getTokenNetworkFilter);
const allOpts = {};
Object.keys(allNetworks || {}).forEach((networkId) => {
allOpts[networkId] = true;
});

const allNetworksFilterShown =
Object.keys(tokenNetworkFilter || {}).length !==
Object.keys(tokenNetworkFilter).length !==
Object.keys(allOpts || {}).length;

const currentNetwork = useSelector(getCurrentNetwork);
Expand Down
6 changes: 3 additions & 3 deletions ui/components/app/detected-token/detected-token.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
import {
getAllDetectedTokensForSelectedAddress,
getDetectedTokensInCurrentNetwork,
getPreferences,
getTokenNetworkFilter,
} from '../../../selectors';
import { MetaMetricsContext } from '../../../contexts/metametrics';

Expand Down Expand Up @@ -63,14 +63,14 @@ const DetectedToken = ({ setShowDetectedTokens }) => {
);
const currentChainId = useSelector(getCurrentChainId);
const allNetworks = useSelector(getNetworkConfigurationsByChainId);
const { tokenNetworkFilter } = useSelector(getPreferences);
const tokenNetworkFilter = useSelector(getTokenNetworkFilter);
const allOpts = {};
Object.keys(allNetworks || {}).forEach((chainId) => {
allOpts[chainId] = true;
});

const allNetworksFilterShown =
Object.keys(tokenNetworkFilter || {}).length !==
Object.keys(tokenNetworkFilter).length !==
Object.keys(allOpts || {}).length;

const totalDetectedTokens = useMemo(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
import {
getDetectedTokensInCurrentNetwork,
getAllDetectedTokensForSelectedAddress,
getPreferences,
getTokenNetworkFilter,
} from '../../../selectors';
import { MetaMetricsContext } from '../../../contexts/metametrics';
import {
Expand All @@ -28,7 +28,7 @@ export const DetectedTokensBanner = ({
}) => {
const t = useI18nContext();
const trackEvent = useContext(MetaMetricsContext);
const { tokenNetworkFilter } = useSelector(getPreferences);
const tokenNetworkFilter = useSelector(getTokenNetworkFilter);
const allNetworks = useSelector(getNetworkConfigurationsByChainId);

const allOpts = {};
Expand All @@ -37,7 +37,7 @@ export const DetectedTokensBanner = ({
});

const allNetworksFilterShown =
Object.keys(tokenNetworkFilter || {}).length !==
Object.keys(tokenNetworkFilter).length !==
Object.keys(allOpts || {}).length;

const detectedTokens = useSelector(getDetectedTokensInCurrentNetwork);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ import {
getAllDomains,
getPermittedChainsForSelectedTab,
getPermittedAccountsForSelectedTab,
getPreferences,
getTokenNetworkFilter,
} from '../../../selectors';
import ToggleButton from '../../ui/toggle-button';
import {
Expand Down Expand Up @@ -116,7 +116,7 @@ export const NetworkListMenu = ({ onClose }: { onClose: () => void }) => {
const dispatch = useDispatch();
const trackEvent = useContext(MetaMetricsContext);

const { tokenNetworkFilter } = useSelector(getPreferences);
const tokenNetworkFilter = useSelector(getTokenNetworkFilter);
const showTestNetworks = useSelector(getShowTestNetworks);
const currentChainId = useSelector(getCurrentChainId);
const selectedTabOrigin = useSelector(getOriginOfCurrentTab);
Expand Down
8 changes: 6 additions & 2 deletions ui/selectors/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1076,8 +1076,7 @@ export function getPetnamesEnabled(state) {

export function getIsTokenNetworkFilterEqualCurrentNetwork(state) {
const chainId = getCurrentChainId(state);
const { tokenNetworkFilter: tokenNetworkFilterValue } = getPreferences(state);
const tokenNetworkFilter = tokenNetworkFilterValue || {};
const tokenNetworkFilter = getTokenNetworkFilter(state);
if (
Object.keys(tokenNetworkFilter).length === 1 &&
Object.keys(tokenNetworkFilter)[0] === chainId
Expand All @@ -1087,6 +1086,11 @@ export function getIsTokenNetworkFilterEqualCurrentNetwork(state) {
return false;
}

export function getTokenNetworkFilter(state) {
const { tokenNetworkFilter } = getPreferences(state);
return tokenNetworkFilter || {};
}

export function getUseTransactionSimulations(state) {
return Boolean(state.metamask.useTransactionSimulations);
}
Expand Down

0 comments on commit e7bba6d

Please sign in to comment.