Skip to content

Commit

Permalink
fix: Move foreground state properties from metamask slice to `appSt…
Browse files Browse the repository at this point in the history
…ate` slice (#28703)

## **Description**

1. Moves properties in the `metamask` Redux slice that are not part of
background (i.e. controller-derived) state into the `appState` slice.

The affected state properties are as follows:

```
  customNonceValue: string;
  isAccountMenuOpen: boolean;
  isNetworkMenuOpen: boolean;
  nextNonce: string | null;
  pendingTokens: {
    [address: string]: Token & { isCustom?: boolean; unlisted?: boolean };
  };
  welcomeScreenSeen: boolean;
  confirmationExchangeRates: ContractExchangeRates;
```

- Foreground properties that are a part of `AppStateController` state
have not been moved into the `appState` slice, and will remain in the
unflattened `metamask` slice.
- It's not immediately clear why the properties listed above were
excluded from `AppStateController` state, but since all of them appear
to neither require persistence beyond a given session, nor anonymization
of PII, I'm opting to maintain the status quo and keep them out of
controller state.
- The `isInitialized` property is not included, because it's derived
from background state before the Redux store is instantiated, and is
updated by the `PatchStore` which supplies it directly to the `metamask`
slice.

2. Groups `appState` slice selectors in `ui/selectors/selectors.js`
together.

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

## **Related issues**

- Closes #29601
- Closes MetaMask/MetaMask-planning#2895
- 0 of 7 PRs that will close
#29600
- Blocking MetaMask/MetaMask-planning#2894
  - #28723
  - #28929
  - #28776

## **Manual testing steps**

## **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/develop/.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/develop/.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.
  • Loading branch information
MajorLift authored Jan 13, 2025
1 parent 9344d0a commit 2b377b3
Show file tree
Hide file tree
Showing 19 changed files with 369 additions and 295 deletions.
12 changes: 6 additions & 6 deletions .storybook/test-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,6 @@ const state = {
connectedAccounts: ['0x64a845a5b02460acf8a3d84503b0d68d028b4bb4'],
isInitialized: true,
isUnlocked: true,
isAccountMenuOpen: false,
rpcUrl: 'https://rawtestrpc.metamask.io/',
internalAccounts: {
accounts: {
Expand Down Expand Up @@ -677,8 +676,6 @@ const state = {
],
},
],
pendingTokens: {},
customNonceValue: '',
send: {
gasLimit: '0xcb28',
gasPrice: null,
Expand All @@ -702,7 +699,6 @@ const state = {
},
useBlockie: false,
featureFlags: {},
welcomeScreenSeen: false,
slides: [],
currentLocale: 'en',
preferences: {
Expand Down Expand Up @@ -731,7 +727,6 @@ const state = {
},
},
participateInMetaMetrics: true,
nextNonce: 71,
connectedStatusPopoverHasBeenShown: true,
swapsWelcomeMessageHasBeenShown: true,
defaultHomeActiveTabName: 'Tokens',
Expand Down Expand Up @@ -1630,6 +1625,12 @@ const state = {
openSeaEnabled: true,
},
appState: {
isAccountMenuOpen: false,
welcomeScreenSeen: false,
pendingTokens: {},
confirmationExchangeRates: {},
customNonceValue: '',
nextNonce: 71,
shouldClose: false,
menuOpen: false,
modal: {
Expand Down Expand Up @@ -1665,7 +1666,6 @@ const state = {
isLoading: false,
warning: null,
buyView: {},
gasIsLoading: false,
defaultHdPaths: {
trezor: "m/44'/60'/0'/0",
ledger: "m/44'/60'/0'/0/0",
Expand Down
11 changes: 6 additions & 5 deletions app/scripts/constants/sentry-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,11 +426,7 @@ const flattenedBackgroundStateMask = Object.values(
export const SENTRY_UI_STATE = {
gas: true,
history: true,
metamask: {
...flattenedBackgroundStateMask,
// This property comes from the background but isn't in controller state
isInitialized: true,
// These properties are in the `metamask` slice but not in the background state
appState: {
customNonceValue: true,
isAccountMenuOpen: true,
isNetworkMenuOpen: true,
Expand All @@ -439,6 +435,11 @@ export const SENTRY_UI_STATE = {
welcomeScreenSeen: true,
slides: false,
confirmationExchangeRates: true,
},
metamask: {
...flattenedBackgroundStateMask,
// This property comes from the background but isn't in controller state
isInitialized: true,
useSafeChainsListValidation: true,
watchEthereumAccountEnabled: false,
bitcoinSupportEnabled: false,
Expand Down
1 change: 0 additions & 1 deletion docs/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ const generateGasEstimatorProps = (overrides) => ({

const generateAppState = (overrides) => ({
networkDropdownOpen: false,
gasIsLoading: false,
isLoading: false,
modal: {
open: false,
Expand Down
1 change: 0 additions & 1 deletion test/data/mock-send-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
"networkDropdownOpen": false,
"importNftsModal": { "open": false },
"showPermittedNetworkToastOpen": false,
"gasIsLoading": false,
"isLoading": false,
"importTokensModalOpen": false,
"modal": {
Expand Down
14 changes: 8 additions & 6 deletions test/data/mock-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,17 @@
"url": "https://metamask.github.io/test-dapp/"
},
"appState": {
"isAccountMenuOpen": false,
"confirmationExchangeRates": {},
"nextNonce": 71,
"welcomeScreenSeen": false,
"pendingTokens": {},
"customTokenAmount": "10",
"networkDropdownOpen": false,
"importNftsModal": {
"open": false
},
"showPermittedNetworkToastOpen": false,
"gasIsLoading": false,
"isLoading": false,
"modal": {
"open": false,
Expand All @@ -27,8 +32,7 @@
"name": null
}
},
"warning": null,
"customTokenAmount": "10"
"warning": null
},
"confirmAlerts": {
"alerts": [],
Expand Down Expand Up @@ -419,11 +423,9 @@
"key": "tokenFiatAmount",
"order": "dsc",
"sortCallback": "stringNumeric"
},
"tokenNetworkFilter": {}
}
},
"ensResolutionsByAddress": {},
"isAccountMenuOpen": false,
"isUnlocked": true,
"alertEnabledness": {
"unconnectedAccount": true
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,71 @@
{
"DNS": "object",
"activeTab": "object",
"appState": "object",
"appState": {
"customNonceValue": "",
"isAccountMenuOpen": false,
"isNetworkMenuOpen": false,
"nextNonce": null,
"confirmationExchangeRates": {},
"ledgerTransportStatus": "string",
"ledgerWebHidConnectedStatus": "string",
"loadingMessage": "undefined",
"menuOpen": "boolean",
"modal": "object",
"alertOpen": "boolean",
"alertMessage": null,
"networkDropdownOpen": "boolean",
"importNftsModal": "object",
"keyringRemovalSnapModal": "object",
"importTokensModalOpen": "boolean",
"deprecatedNetworkModalOpen": "boolean",
"accountDetail": "object",
"isLoading": "boolean",
"isNftStillFetchingIndication": "boolean",
"buyView": "object",
"defaultHdPaths": "object",
"networksTabSelectedRpcUrl": "string",
"requestAccountTabs": "object",
"currentWindowTab": "object",
"gasLoadingAnimationIsShowing": "boolean",
"externalServicesOnboardingToggleState": "boolean",
"newNftAddedMessage": "string",
"removeNftMessage": "string",
"newNetworkAddedName": "string",
"editedNetwork": "undefined",
"newNetworkAddedConfigurationId": "string",
"newTokensImported": "string",
"newTokensImportedError": "string",
"onboardedInThisUISession": "boolean",
"customTokenAmount": "string",
"accountDetailsAddress": "string",
"isAddingNewNetwork": "boolean",
"isMultiRpcOnboarding": "boolean",
"errorInSettings": null,
"openMetaMaskTabs": "object",
"pendingTokens": "object",
"qrCodeData": null,
"selectedNetworkConfigurationId": "string",
"scrollToBottom": "boolean",
"sendInputCurrencySwitched": "boolean",
"shouldClose": "boolean",
"showBasicFunctionalityModal": "boolean",
"showDataDeletionErrorModal": "boolean",
"showDeleteMetaMetricsDataModal": "boolean",
"showIpfsModalOpen": "boolean",
"showKeyringRemovalSnapModal": "boolean",
"showNftDetectionEnablementToast": "boolean",
"showPermittedNetworkToastOpen": "boolean",
"showTermsOfUsePopup": "boolean",
"showWhatsNewPopup": "boolean",
"singleExceptions": "object",
"smartTransactionsError": null,
"smartTransactionsErrorMessageDismissed": "boolean",
"snapsInstallPrivacyWarningShown": "boolean",
"txId": null,
"warning": "string",
"welcomeScreenSeen": false
},
"bridge": "object",
"confirmAlerts": "object",
"confirmTransaction": "object",
Expand All @@ -12,18 +76,12 @@
"metamask": {
"isInitialized": true,
"isUnlocked": true,
"isAccountMenuOpen": false,
"isNetworkMenuOpen": false,
"internalAccounts": { "accounts": "object", "selectedAccount": "string" },
"transactions": "object",
"networkConfigurations": "object",
"addressBook": "object",
"confirmationExchangeRates": {},
"pendingTokens": "object",
"customNonceValue": "",
"useBlockie": false,
"featureFlags": {},
"welcomeScreenSeen": false,
"currentLocale": "en",
"preferences": {
"hideZeroBalanceTokens": false,
Expand All @@ -48,7 +106,6 @@
"use4ByteResolution": true,
"participateInMetaMetrics": true,
"dataCollectionForMarketing": "boolean",
"nextNonce": null,
"currencyRates": {
"ETH": {
"conversionDate": "number",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useContext } from 'react';
import { useSelector } from 'react-redux';
import {
getCurrentNetwork,
getPendingTokens,
getTestNetworkBackgroundColor,
getTokenList,
} from '../../../selectors';
Expand All @@ -22,7 +23,6 @@ import {
AlignItems,
FlexDirection,
} from '../../../helpers/constants/design-system';
import { getPendingTokens } from '../../../ducks/metamask/metamask';
import TokenBalance from '../../ui/token-balance/token-balance';
import { I18nContext } from '../../../contexts/i18n';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
getCurrentNetwork,
getTestNetworkBackgroundColor,
getTokenExchangeRates,
getPendingTokens,
} from '../../../selectors';
import {
addImportedTokens,
Expand Down Expand Up @@ -90,10 +91,7 @@ import {
} from '../../../helpers/utils/util';
import { tokenInfoGetter } from '../../../helpers/utils/token-util';
import { MetaMetricsContext } from '../../../contexts/metametrics';
import {
getNativeCurrency,
getPendingTokens,
} from '../../../ducks/metamask/metamask';
import { getNativeCurrency } from '../../../ducks/metamask/metamask';
import {
MetaMetricsEventCategory,
MetaMetricsEventName,
Expand Down
69 changes: 69 additions & 0 deletions ui/ducks/app/app.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,4 +358,73 @@ describe('App State', () => {

expect(state.errorInSettings).toBeNull();
});

it('toggles account menu', () => {
const state = reduceApp(
{},
{
type: actionConstants.TOGGLE_ACCOUNT_MENU,
},
);

expect(state.isAccountMenuOpen).toStrictEqual(true);
});

it('toggles network menu', () => {
const state = reduceApp(
{},
{
type: actionConstants.TOGGLE_NETWORK_MENU,
},
);

expect(state.isNetworkMenuOpen).toStrictEqual(true);
});

it('close welcome screen', () => {
const state = reduceApp(
{},
{
type: actionConstants.CLOSE_WELCOME_SCREEN,
},
);

expect(state.welcomeScreenSeen).toStrictEqual(true);
});

it('sets pending tokens', () => {
const payload = {
address: '0x617b3f8050a0bd94b6b1da02b4384ee5b4df13f4',
decimals: 18,
symbol: 'META',
};

const pendingTokensState = reduceApp(
{},
{
type: actionConstants.SET_PENDING_TOKENS,
payload,
},
);

expect(pendingTokensState.pendingTokens).toStrictEqual(payload);
});

it('clears pending tokens', () => {
const payload = {
address: '0x617b3f8050a0bd94b6b1da02b4384ee5b4df13f4',
decimals: 18,
symbol: 'META',
};

const pendingTokensState = {
pendingTokens: payload,
};

const state = reduceApp(pendingTokensState, {
type: actionConstants.CLEAR_PENDING_TOKENS,
});

expect(state.pendingTokens).toStrictEqual({});
});
});
Loading

0 comments on commit 2b377b3

Please sign in to comment.