fix: import accounts requiring 2 attempts for account to persist (#6819)

* fix: account import requiring 2 attempts to import an account * chore: simplify account toggling logic * fix: handle no accounts connected in manage accounts modal * fix: accounts toggling twice on boot due to use-strict * fix: tests * fix: regression in existing add account modal
shapeshift · May 7, 2024 · 4e51a2b · 4e51a2b
1 parent dae17c4
commit 4e51a2b
Show file tree

Hide file tree

Showing 13 changed files with 169 additions and 111 deletions.
diff --git a/src/assets/translations/en/main.json b/src/assets/translations/en/main.json
@@ -1794,7 +1794,9 @@
     "manageAccounts": {
       "title": "Manage Accounts",
       "description": "These are the chains you have connected and the number of accounts you have imported.",
-      "addChain": "Add another chain"
+      "emptyList": "You have no accounts connected yet. Add a chain to get started.",
+      "addChain": "Add a chain",
+      "addAnotherChain": "Add another chain"
     },
     "selectChain": {
       "title": "Select Chain",

diff --git a/src/components/ManageAccountsDrawer/components/ImportAccounts.tsx b/src/components/ManageAccountsDrawer/components/ImportAccounts.tsx
@@ -29,6 +29,7 @@ import { accountIdToLabel } from 'state/slices/portfolioSlice/utils'
 import {
   selectFeeAssetByChainId,
   selectHighestAccountNumberForChainId,
+  selectIsAccountIdEnabled,
   selectIsAnyAccountIdEnabled,
 } from 'state/slices/selectors'
 import { store, useAppDispatch, useAppSelector } from 'state/store'
@@ -161,12 +162,12 @@ export const ImportAccounts = ({ chainId, onClose }: ImportAccountsProps) => {
   )
   const chainNamespaceDisplayName = asset?.networkName ?? ''
   const [autoFetching, setAutoFetching] = useState(true)
-  const [toggledActiveAccountIds, setToggledActiveAccountIds] = useState<Set<AccountId>>(new Set())
+  const [toggledAccountIds, setToggledAccountIds] = useState<Set<AccountId>>(new Set())
 
   // reset component state when chainId changes
   useEffect(() => {
     setAutoFetching(true)
-    setToggledActiveAccountIds(new Set())
+    setToggledAccountIds(new Set())
   }, [chainId])
 
   // initial fetch to detect the number of accounts based on the "first empty account" heuristic
@@ -214,7 +215,7 @@ export const ImportAccounts = ({ chainId, onClose }: ImportAccountsProps) => {
   }, [autoFetching, isLoading, fetchNextPage])
 
   const handleToggleAccountIds = useCallback((accountIds: AccountId[]) => {
-    setToggledActiveAccountIds(previousState => {
+    setToggledAccountIds(previousState => {
       const updatedState = new Set(previousState)
       for (const accountId of accountIds) {
         if (updatedState.has(accountId)) {
@@ -231,17 +232,25 @@ export const ImportAccounts = ({ chainId, onClose }: ImportAccountsProps) => {
   // TODO: Loading state
   const handleDone = useCallback(async () => {
     // for every new account that is active, fetch the account and upsert it into the redux state
-    for (const accountId of toggledActiveAccountIds) {
-      const isEnabled = selectIsAnyAccountIdEnabled(store.getState(), [accountId])
-      if (isEnabled) continue
+    for (const accountId of toggledAccountIds) {
+      const isEnabled = selectIsAccountIdEnabled(store.getState(), { accountId })
+      if (isEnabled) {
+        continue
+      }
       await dispatch(portfolioApi.endpoints.getAccount.initiate({ accountId, upsertOnFetch: true }))
     }
 
-    if (!accounts) return
+    if (!accounts) {
+      return
+    }
 
     const accountMetadataByAccountId = accounts.pages.reduce((accumulator, accounts) => {
       const obj = accounts.accountIdWithActivityAndMetadata.reduce(
         (innerAccumulator, { accountId, accountMetadata }) => {
+          // Don't include accounts that are not toggled - they are either only
+          // displayed and not toggled on, or are already in the store
+          if (!toggledAccountIds.has(accountId)) return innerAccumulator
+
           return { ...innerAccumulator, [accountId]: accountMetadata }
         },
         {},
@@ -256,12 +265,12 @@ export const ImportAccounts = ({ chainId, onClose }: ImportAccountsProps) => {
       }),
     )
 
-    for (const accountId of toggledActiveAccountIds) {
-      dispatch(portfolio.actions.toggleAccountIdHidden(accountId))
+    for (const accountId of toggledAccountIds) {
+      dispatch(portfolio.actions.toggleAccountIdEnabled(accountId))
     }
 
     onClose()
-  }, [toggledActiveAccountIds, accounts, dispatch, onClose, walletDeviceId])
+  }, [toggledAccountIds, accounts, dispatch, onClose, walletDeviceId])
 
   const accountRows = useMemo(() => {
     if (!asset || !accounts) return null

diff --git a/src/components/Modals/ManageAccounts/ManageAccountsModal.tsx b/src/components/Modals/ManageAccounts/ManageAccountsModal.tsx
@@ -23,7 +23,7 @@ import { ManageAccountsDrawer } from 'components/ManageAccountsDrawer/ManageAcco
 import { RawText } from 'components/Text'
 import { useModal } from 'hooks/useModal/useModal'
 import { assertGetChainAdapter, chainIdToFeeAssetId } from 'lib/utils'
-import { selectWalletSupportedChainIds } from 'state/slices/common-selectors'
+import { selectWalletChainIds } from 'state/slices/common-selectors'
 import { selectAccountIdsByChainId, selectAssetById } from 'state/slices/selectors'
 import { useAppSelector } from 'state/store'
 
@@ -82,8 +82,7 @@ export const ManageAccountsModal = ({
     console.log('info clicked')
   }, [])
 
-  // TODO: redux will have to be updated to use the selected chains from this flow
-  const walletSupportedChainIds = useAppSelector(selectWalletSupportedChainIds)
+  const walletConnectedChainIds = useAppSelector(selectWalletChainIds)
 
   const handleClickChain = useCallback(
     (chainId: ChainId) => {
@@ -99,10 +98,10 @@ export const ManageAccountsModal = ({
   }, [handleDrawerOpen])
 
   const connectedChains = useMemo(() => {
-    return walletSupportedChainIds.map(chainId => {
+    return walletConnectedChainIds.map(chainId => {
       return <ConnectedChain key={chainId} chainId={chainId} onClick={handleClickChain} />
     })
-  }, [handleClickChain, walletSupportedChainIds])
+  }, [handleClickChain, walletConnectedChainIds])
 
   const disableAddChain = false // FIXME: walletSupportedChainIds.length === connectedChains.length - blocked on Redux PR
 
@@ -121,7 +120,9 @@ export const ManageAccountsModal = ({
               {translate(title)}
             </RawText>
             <RawText color='text.subtle' fontSize='md' fontWeight='normal'>
-              {translate('accountManagement.manageAccounts.description')}
+              {walletConnectedChainIds.length === 0
+                ? translate('accountManagement.manageAccounts.emptyList')
+                : translate('accountManagement.manageAccounts.description')}
             </RawText>
           </ModalHeader>
           <IconButton
@@ -135,11 +136,13 @@ export const ManageAccountsModal = ({
             onClick={handleInfoClick}
           />
           <ModalCloseButton position='absolute' top={3} right={3} />
-          <ModalBody maxH='400px' overflow='scroll'>
-            <VStack spacing={2} width='full'>
-              {connectedChains}
-            </VStack>
-          </ModalBody>
+          {walletConnectedChainIds.length > 0 && (
+            <ModalBody maxH='400px' overflowY='auto'>
+              <VStack spacing={2} width='full'>
+                {connectedChains}
+              </VStack>
+            </ModalBody>
+          )}
           <ModalFooter justifyContent='center' pb={6}>
             <VStack spacing={2} width='full'>
               <Button
@@ -150,7 +153,9 @@ export const ManageAccountsModal = ({
                 isDisabled={disableAddChain}
                 _disabled={disabledProp}
               >
-                {translate('accountManagement.manageAccounts.addChain')}
+                {walletConnectedChainIds.length === 0
+                  ? translate('accountManagement.manageAccounts.addChain')
+                  : translate('accountManagement.manageAccounts.addAnotherChain')}
               </Button>
               <Button size='lg' colorScheme='gray' onClick={close} width='full'>
                 {translate('common.done')}

diff --git a/src/context/AppProvider/AppContext.tsx b/src/context/AppProvider/AppContext.tsx
@@ -154,6 +154,10 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
           walletId: await wallet.getDeviceID(),
         }),
       )
+
+      for (const accountId of Object.keys(accountMetadataByAccountId)) {
+        dispatch(portfolio.actions.enableAccountId(accountId))
+      }
     })()
   }, [dispatch, wallet, supportedChains, isSnapInstalled])
 

diff --git a/src/pages/Accounts/AddAccountModal.tsx b/src/pages/Accounts/AddAccountModal.tsx
@@ -89,9 +89,10 @@ export const AddAccountModal = () => {
         }),
       )
       const accountIds = Object.keys(accountMetadataByAccountId)
-      accountIds.forEach(accountId =>
-        dispatch(getAccount.initiate({ accountId, upsertOnFetch: true }, opts)),
-      )
+      accountIds.forEach(accountId => {
+        dispatch(getAccount.initiate({ accountId, upsertOnFetch: true }, opts))
+        dispatch(portfolio.actions.enableAccountId(accountId))
+      })
       const assetId = getChainAdapterManager().get(selectedChainId)!.getFeeAssetId()
       const { name } = assets[assetId] ?? {}
       toast({

diff --git a/src/state/migrations/index.ts b/src/state/migrations/index.ts
@@ -59,4 +59,5 @@ export const migrations = {
   49: clearAssets,
   50: clearPortfolio,
   51: clearAssets,
+  52: clearPortfolio,
 }
diff --git a/src/state/slices/common-selectors.ts b/src/state/slices/common-selectors.ts
@@ -26,10 +26,10 @@ export const selectWalletSupportedChainIds = (state: ReduxState) =>
 export const selectWalletAccountIds = createDeepEqualOutputSelector(
   selectWalletId,
   (state: ReduxState) => state.portfolio.wallet.byId,
-  (state: ReduxState) => state.portfolio.hiddenAccountIds,
-  (walletId, walletById, hiddenAccountIds): AccountId[] => {
+  (state: ReduxState) => state.portfolio.enabledAccountIds,
+  (walletId, walletById, enabledAccountIds): AccountId[] => {
     const walletAccountIds = (walletId && walletById[walletId]) ?? []
-    return walletAccountIds.filter(accountId => !hiddenAccountIds.includes(accountId))
+    return walletAccountIds.filter(accountId => (enabledAccountIds ?? []).includes(accountId))
   },
 )
 

diff --git a/src/state/slices/opportunitiesSlice/selectors/selectors.test.ts b/src/state/slices/opportunitiesSlice/selectors/selectors.test.ts
@@ -47,6 +47,7 @@ describe('opportunitiesSlice selectors', () => {
         supportedChainIds: [],
       },
       wallet,
+      enabledAccountIds: [gomesAccountId, fauxmesAccountId, catpuccinoAccountId],
     },
   }
   describe('selects ID/s', () => {
@@ -65,6 +66,7 @@ describe('opportunitiesSlice selectors', () => {
       },
       ids: [gomesAccountId, fauxmesAccountId],
     }
+    const enabledAccountIds = [gomesAccountId, fauxmesAccountId]
     const lp = {
       ...initialState.lp,
       byAccountId: {
@@ -116,6 +118,7 @@ describe('opportunitiesSlice selectors', () => {
         ...mockBaseState.portfolio,
         accountBalances,
         accountMetadata,
+        enabledAccountIds,
       },
       opportunities: {
         ...initialState,

diff --git a/src/state/slices/portfolioSlice/__snapshots__/portfolioSlice.test.ts.snap b/src/state/slices/portfolioSlice/__snapshots__/portfolioSlice.test.ts.snap
@@ -29,7 +29,9 @@ exports[`portfolioSlice > reducers > upsertPortfolio > Bitcoin > should update s
       "bip122:000000000019d6689c085ae165831e93:zpub6qk8s2NQsYG6X2Mm6iU2ii3yTAqDb2XqnMu9vo2WjvqwjSvjjiYQQveYXbPxrnRT5Yb5p0x934be745172066EDF795ffc5EA9F28f19b440c637BaBw1wowPwbS8fj7uCfj3UhqhD2LLbvY6Ni1w",
     ],
   },
-  "hiddenAccountIds": [],
+  "enabledAccountIds": [
+    "bip122:000000000019d6689c085ae165831e93:zpub6qk8s2NQsYG6X2Mm6iU2ii3yTAqDb2XqnMu9vo2WjvqwjSvjjiYQQveYXbPxrnRT5Yb5p0x934be745172066EDF795ffc5EA9F28f19b440c637BaBw1wowPwbS8fj7uCfj3UhqhD2LLbvY6Ni1w",
+  ],
   "wallet": {
     "byId": {},
     "ids": [],
@@ -77,7 +79,10 @@ exports[`portfolioSlice > reducers > upsertPortfolio > Bitcoin > should update s
       "bip122:000000000019d6689c085ae165831e93:xpub6qk8s2NQsYG6X2Mm6iU2ii3yTAqDb2XqnMu9vo2WjvqwjSvjjiYQQveYXbPxrnRT5Yb5p0x934be745172066EDF795ffc5EA9F28f19b440c637BaBw1wowPwbS8fj7uCfj3UhqhD2LLbvY6Ni1w",
     ],
   },
-  "hiddenAccountIds": [],
+  "enabledAccountIds": [
+    "bip122:000000000019d6689c085ae165831e93:zpub6qk8s2NQsYG6X2Mm6iU2ii3yTAqDb2XqnMu9vo2WjvqwjSvjjiYQQveYXbPxrnRT5Yb5p0x934be745172066EDF795ffc5EA9F28f19b440c637BaBw1wowPwbS8fj7uCfj3UhqhD2LLbvY6Ni1w",
+    "bip122:000000000019d6689c085ae165831e93:xpub6qk8s2NQsYG6X2Mm6iU2ii3yTAqDb2XqnMu9vo2WjvqwjSvjjiYQQveYXbPxrnRT5Yb5p0x934be745172066EDF795ffc5EA9F28f19b440c637BaBw1wowPwbS8fj7uCfj3UhqhD2LLbvY6Ni1w",
+  ],
   "wallet": {
     "byId": {},
     "ids": [],
@@ -148,7 +153,11 @@ exports[`portfolioSlice > reducers > upsertPortfolio > Ethereum and bitcoin > sh
       "bip122:000000000019d6689c085ae165831e93:zpub6qk8s2NQsYG6X2Mm6iU2ii3yTAqDb2XqnMu9vo2WjvqwjSvjjiYQQveYXbPxrnRT5Yb5p0x934be745172066EDF795ffc5EA9F28f19b440c637BaBw1wowPwbS8fj7uCfj3UhqhD2LLbvY6Ni1w",
     ],
   },
-  "hiddenAccountIds": [],
+  "enabledAccountIds": [
+    "eip155:1:0x9a2d593725045d1727d525dd07a396f9ff079bb1",
+    "eip155:1:0xea674fdde714fd979de3edf0f56aa9716b898ec8",
+    "bip122:000000000019d6689c085ae165831e93:zpub6qk8s2NQsYG6X2Mm6iU2ii3yTAqDb2XqnMu9vo2WjvqwjSvjjiYQQveYXbPxrnRT5Yb5p0x934be745172066EDF795ffc5EA9F28f19b440c637BaBw1wowPwbS8fj7uCfj3UhqhD2LLbvY6Ni1w",
+  ],
   "wallet": {
     "byId": {},
     "ids": [],
@@ -204,7 +213,10 @@ exports[`portfolioSlice > reducers > upsertPortfolio > Ethereum and bitcoin > sh
       "bip122:000000000019d6689c085ae165831e93:zpub6qk8s2NQsYG6X2Mm6iU2ii3yTAqDb2XqnMu9vo2WjvqwjSvjjiYQQveYXbPxrnRT5Yb5p0x934be745172066EDF795ffc5EA9F28f19b440c637BaBw1wowPwbS8fj7uCfj3UhqhD2LLbvY6Ni1w",
     ],
   },
-  "hiddenAccountIds": [],
+  "enabledAccountIds": [
+    "eip155:1:0x6c8a778ef52e121b7dff1154c553662306a970e9",
+    "bip122:000000000019d6689c085ae165831e93:zpub6qk8s2NQsYG6X2Mm6iU2ii3yTAqDb2XqnMu9vo2WjvqwjSvjjiYQQveYXbPxrnRT5Yb5p0x934be745172066EDF795ffc5EA9F28f19b440c637BaBw1wowPwbS8fj7uCfj3UhqhD2LLbvY6Ni1w",
+  ],
   "wallet": {
     "byId": {},
     "ids": [],
@@ -243,7 +255,9 @@ exports[`portfolioSlice > reducers > upsertPortfolio > ethereum > should update
       "eip155:1:0x9a2d593725045d1727d525dd07a396f9ff079bb1",
     ],
   },
-  "hiddenAccountIds": [],
+  "enabledAccountIds": [
+    "eip155:1:0x9a2d593725045d1727d525dd07a396f9ff079bb1",
+  ],
   "wallet": {
     "byId": {},
     "ids": [],
@@ -295,7 +309,10 @@ exports[`portfolioSlice > reducers > upsertPortfolio > ethereum > should update
       "eip155:1:0xea674fdde714fd979de3edf0f56aa9716b898ec8",
     ],
   },
-  "hiddenAccountIds": [],
+  "enabledAccountIds": [
+    "eip155:1:0x9a2d593725045d1727d525dd07a396f9ff079bb1",
+    "eip155:1:0xea674fdde714fd979de3edf0f56aa9716b898ec8",
+  ],
   "wallet": {
     "byId": {},
     "ids": [],