Tech Debt: Safe Apps Refactor (#1110)

* apps refactoring wip

* apps refactoring wip

* type fixes

* add useLegalConsent hook in apps

* useAppList hook wip

* dep nump

* useAppList hook wip

* fix selecting first app

* Remove console.log

* dep bump

* update persisting app logic

* update saveToStorage type

* fix crash on apps tab

* reuse selectedApp variable in hook

* remove initialAppSelected

src/routes/safe/components/Apps/components/LegalDisclaimer.tsx

@@ -0,0 +1,34 @@
+import React from 'react'
+import { FixedDialog, Text } from '@gnosis.pm/safe-react-components'
+
+interface OwnProps {
+  onCancel: () => void
+  onConfirm: () => void
+}
+
+const LegalDisclaimer = ({ onCancel, onConfirm }: OwnProps): JSX.Element => (
+  <FixedDialog
+    body={
+      <>
+        <Text size="md">
+          You are now accessing third-party apps, which we do not own, control, maintain or audit. We are not liable for
+          any loss you may suffer in connection with interacting with the apps, which is at your own risk. You must read
+          our Terms, which contain more detailed provisions binding on you relating to the apps.
+        </Text>
+        <br />
+        <Text size="md">
+          I have read and understood the{' '}
+          <a href="https://gnosis-safe.io/terms" rel="noopener noreferrer" target="_blank">
+            Terms
+          </a>{' '}
+          and this Disclaimer, and agree to be bound by them.
+        </Text>
+      </>
+    }
+    onCancel={onCancel}
+    onConfirm={onConfirm}
+    title="Disclaimer"
+  />
+)
+
+export default LegalDisclaimer

src/routes/safe/components/Apps/components/ManageApps.tsx

@@ -2,8 +2,8 @@ import { ButtonLink, ManageListModal } from '@gnosis.pm/safe-react-components'
 import React, { useState } from 'react'
 
 import appsIconSvg from 'src/routes/safe/components/Transactions/TxsTable/TxType/assets/appsIcon.svg'
-import AddAppForm from './AddAppForm'
+import AddAppForm from '../AddAppForm'
-import { SafeApp } from './types'
+import { SafeApp } from '../types'
 
 const FORM_ID = 'add-apps-form'
 

src/routes/safe/components/Apps/confirmTransactions.tsx

@@ -75,7 +75,7 @@ const confirmTransactions = (
   ethBalance: string,
   nameApp: string,
   iconApp: string,
-  txs: Array<SafeAppTx>,
+  txs: SafeAppTx[],
   openModal: (modalInfo: GenericModalProps) => void,
   closeModal: () => void,
   onConfirm: () => void,

src/routes/safe/components/Apps/hooks/useAppList.ts

@@ -0,0 +1,107 @@
+import { useState, useEffect, useCallback } from 'react'
+import { loadFromStorage, saveToStorage } from 'src/utils/storage'
+import { getAppInfoFromUrl, staticAppsList } from '../utils'
+import { SafeApp, StoredSafeApp } from '../types'
+
+const APPS_STORAGE_KEY = 'APPS_STORAGE_KEY'
+
+type onAppToggleHandler = (appId: string, enabled: boolean) => Promise<void>
+type onAppAddedHandler = (app: SafeApp) => void
+
+type UseAppListReturnType = {
+  appList: SafeApp[]
+  loadingAppList: boolean
+  onAppToggle: onAppToggleHandler
+  onAppAdded: onAppAddedHandler
+}
+
+const useAppList = (): UseAppListReturnType => {
+  const [appList, setAppList] = useState<SafeApp[]>([])
+  const [loadingAppList, setLoadingAppList] = useState<boolean>(true)
+
+  // Load apps list
+  useEffect(() => {
+    const loadApps = async () => {
+      // recover apps from storage:
+      // * third-party apps added by the user
+      // * disabled status for both static and third-party apps
+      const persistedAppList = (await loadFromStorage<StoredSafeApp[]>(APPS_STORAGE_KEY)) || []
+      const list = [...persistedAppList]
+
+      staticAppsList.forEach((staticApp) => {
+        if (!list.some((persistedApp) => persistedApp.url === staticApp.url)) {
+          list.push(staticApp)
+        }
+      })
+
+      const apps = []
+      // using the appURL to recover app info
+      for (let index = 0; index < list.length; index++) {
+        try {
+          const currentApp = list[index]
+
+          const appInfo: any = await getAppInfoFromUrl(currentApp.url)
+          if (appInfo.error) {
+            throw Error(`There was a problem trying to load app ${currentApp.url}`)
+          }
+
+          appInfo.disabled = currentApp.disabled === undefined ? false : currentApp.disabled
+
+          apps.push(appInfo)
+        } catch (error) {
+          console.error(error)
+        }
+      }
+
+      setAppList(apps)
+      setLoadingAppList(false)
+    }
+
+    loadApps()
+  }, [])
+
+  const onAppToggle: onAppToggleHandler = useCallback(
+    async (appId, enabled) => {
+      // update in-memory list
+      const appListCopy = [...appList]
+
+      const app = appListCopy.find((a) => a.id === appId)
+      if (!app) {
+        return
+      }
+
+      app.disabled = !enabled
+      setAppList(appListCopy)
+
+      // update storage list
+      const listToPersist: StoredSafeApp[] = appListCopy.map(({ url, disabled }) => ({ url, disabled }))
+      saveToStorage(APPS_STORAGE_KEY, listToPersist)
+    },
+    [appList],
+  )
+
+  const onAppAdded: onAppAddedHandler = useCallback(
+    (app) => {
+      const newAppList = [
+        { url: app.url, disabled: false },
+        ...appList.map((a) => ({
+          url: a.url,
+          disabled: a.disabled,
+        })),
+      ]
+      saveToStorage(APPS_STORAGE_KEY, newAppList)
+
+      setAppList([...appList, { ...app, disabled: false }])
+    },
+    [appList],
+  )
+
+  return {
+    appList,
+    loadingAppList,
+    onAppToggle,
+    onAppAdded,
+  }
+}
+
+export { useAppList }

src/routes/safe/components/Apps/hooks/useIframeMessageHandler.ts

@@ -0,0 +1,52 @@
+import { useEffect } from 'react'
+
+const useIframeMessageHandler = (): void => {
+  useEffect(() => {
+    //   const handleIframeMessage = (data) => {
+    //     if (!data || !data.messageId) {
+    //       console.error('ThirdPartyApp: A message was received without message id.')
+    //       return
+    //     }
+    //     switch (data.messageId) {
+    //       case operations.SEND_TRANSACTIONS: {
+    //         const onConfirm = async () => {
+    //           closeModal()
+    //           await sendTransactions(dispatch, safeAddress, data.data, enqueueSnackbar, closeSnackbar, selectedApp.id)
+    //         }
+    //         confirmTransactions(
+    //           safeAddress,
+    //           safeName,
+    //           ethBalance,
+    //           selectedApp.name,
+    //           selectedApp.iconUrl,
+    //           data.data,
+    //           openModal,
+    //           closeModal,
+    //           onConfirm,
+    //         )
+    //         break
+    //       }
+    //       default: {
+    //         console.error(`ThirdPartyApp: A message was received with an unknown message id ${data.messageId}.`)
+    //         break
+    //       }
+    //     }
+    //   }
+    //   const onIframeMessage = async ({ data, origin }) => {
+    //     if (origin === window.origin) {
+    //       return
+    //     }
+    //     if (!selectedApp.url.includes(origin)) {
+    //       console.error(`ThirdPartyApp: A message was received from an unknown origin ${origin}`)
+    //       return
+    //     }
+    //     handleIframeMessage(data)
+    //   }
+    //   window.addEventListener('message', onIframeMessage)
+    //   return () => {
+    //     window.removeEventListener('message', onIframeMessage)
+    //   }
+  }, [])
+}
+
+export { useIframeMessageHandler }

src/routes/safe/components/Apps/hooks/useLegalConsent.ts

@@ -0,0 +1,29 @@
+import { useState, useEffect, useCallback } from 'react'
+import { loadFromStorage, saveToStorage } from 'src/utils/storage'
+
+const APPS_LEGAL_CONSENT_RECEIVED = 'APPS_LEGAL_CONSENT_RECEIVED'
+
+const useLegalConsent = (): { consentReceived: boolean; onConsentReceipt: () => void } => {
+  const [consentReceived, setConsentReceived] = useState<boolean>(false)
+
+  useEffect(() => {
+    const checkLegalDisclaimer = async () => {
+      const storedConsentReceived = await loadFromStorage(APPS_LEGAL_CONSENT_RECEIVED)
+
+      if (storedConsentReceived) {
+        setConsentReceived(true)
+      }
+    }
+
+    checkLegalDisclaimer()
+  }, [])
+
+  const onConsentReceipt = useCallback((): void => {
+    setConsentReceived(true)
+    saveToStorage(APPS_LEGAL_CONSENT_RECEIVED, true)
+  }, [])
+
+  return { consentReceived, onConsentReceipt }
+}
+
+export { useLegalConsent }

src/routes/safe/components/Apps/index.tsx

@@ -1,14 +1,16 @@
-import { Card, FixedDialog, FixedIcon, IconText, Loader, Menu, Text, Title } from '@gnosis.pm/safe-react-components'
+import { Card, FixedIcon, IconText, Loader, Menu, Title } from '@gnosis.pm/safe-react-components'
 import { withSnackbar } from 'notistack'
-import React, { useCallback, useEffect, useState } from 'react'
+import React, { useCallback, useEffect, useState, useMemo } from 'react'
 import { useDispatch, useSelector } from 'react-redux'
 import { useHistory } from 'react-router-dom'
 import styled from 'styled-components'
 
-import ManageApps from './ManageApps'
+import ManageApps from './components/ManageApps'
 import confirmTransactions from './confirmTransactions'
 import sendTransactions from './sendTransactions'
-import { getAppInfoFromUrl, staticAppsList } from './utils'
+import LegalDisclaimer from './components/LegalDisclaimer'
+import { useLegalConsent } from './hooks/useLegalConsent'
+import { useAppList } from './hooks/useAppList'
 
 import LCL from 'src/components/ListContentLayout'
 import { networkSelector } from 'src/logic/wallets/store/selectors'
@@ -19,12 +21,7 @@ import {
   safeNameSelector,
   safeParamAddressFromStateSelector,
 } from 'src/routes/safe/store/selectors'
-import { loadFromStorage, saveToStorage } from 'src/utils/storage'
 import { isSameHref } from 'src/utils/url'
-import { SafeApp, StoredSafeApp } from './types'
-
-const APPS_STORAGE_KEY = 'APPS_STORAGE_KEY'
-const APPS_LEGAL_DISCLAIMER_STORAGE_KEY = 'APPS_LEGAL_DISCLAIMER_STORAGE_KEY'
 
 const StyledIframe = styled.iframe`
   padding: 15px;
@@ -63,11 +60,10 @@ const operations = {
 }
 
 function Apps({ closeModal, closeSnackbar, enqueueSnackbar, openModal }) {
-  const [appList, setAppList] = useState<Array<SafeApp>>([])
+  const { appList, loadingAppList, onAppToggle, onAppAdded } = useAppList()
-  const [legalDisclaimerAccepted, setLegalDisclaimerAccepted] = useState(false)
+
-  const [selectedApp, setSelectedApp] = useState<string>()
+  const [appIsLoading, setAppIsLoading] = useState<boolean>(true)
-  const [loading, setLoading] = useState(true)
+  const [selectedAppId, setSelectedAppId] = useState<string>()
-  const [appIsLoading, setAppIsLoading] = useState(true)
   const [iframeEl, setIframeEl] = useState<HTMLIFrameElement | null>(null)
 
   const history = useHistory()
@@ -77,8 +73,36 @@ function Apps({ closeModal, closeSnackbar, enqueueSnackbar, openModal }) {
   const network = useSelector(networkSelector)
   const ethBalance = useSelector(safeEthBalanceSelector)
   const dispatch = useDispatch()
+  const { consentReceived, onConsentReceipt } = useLegalConsent()
 
-  const getSelectedApp = useCallback(() => appList.find((e) => e.id === selectedApp), [appList, selectedApp])
+  const selectedApp = useMemo(() => appList.find((app) => app.id === selectedAppId), [appList, selectedAppId])
+
+  const onSelectApp = useCallback(
+    (appId) => {
+      if (selectedAppId === appId) {
+        return
+      }
+
+      setAppIsLoading(true)
+      setSelectedAppId(appId)
+    },
+    [selectedAppId],
+  )
+
+  useEffect(() => {
+    const selectFirstEnabledApp = () => {
+      const firstEnabledApp = appList.find((a) => !a.disabled)
+      if (firstEnabledApp) {
+        setSelectedAppId(firstEnabledApp.id)
+      }
+    }
+
+    const initialSelect = appList.length && !selectedAppId
+    const currentAppWasDisabled = selectedApp?.disabled
+    if (initialSelect || currentAppWasDisabled) {
+      selectFirstEnabledApp()
+    }
+  }, [appList, selectedApp, selectedAppId])
 
   const iframeRef = useCallback((node) => {
     if (node !== null) {
@@ -86,58 +110,15 @@ function Apps({ closeModal, closeSnackbar, enqueueSnackbar, openModal }) {
     }
   }, [])
 
-  const onSelectApp = useCallback(
-    (appId) => {
-      const selectedApp = getSelectedApp()
-
-      if (selectedApp && selectedApp.id === appId) {
-        return
-      }
-
-      setAppIsLoading(true)
-      setSelectedApp(appId)
-    },
-    [getSelectedApp],
-  )
-
   const redirectToBalance = () => history.push(`${SAFELIST_ADDRESS}/${safeAddress}/balances`)
 
-  const onAcceptLegalDisclaimer = () => {
-    setLegalDisclaimerAccepted(true)
-    saveToStorage(APPS_LEGAL_DISCLAIMER_STORAGE_KEY, true)
-  }
-
   const getContent = () => {
     if (!selectedApp) {
       return null
     }
 
-    if (!legalDisclaimerAccepted) {
+    if (!consentReceived) {
-      return (
+      return <LegalDisclaimer onCancel={redirectToBalance} onConfirm={onConsentReceipt} />
-        <FixedDialog
-          body={
-            <>
-              <Text size="md">
-                You are now accessing third-party apps, which we do not own, control, maintain or audit. We are not
-                liable for any loss you may suffer in connection with interacting with the apps, which is at your own
-                risk. You must read our Terms, which contain more detailed provisions binding on you relating to the
-                apps.
-              </Text>
-              <br />
-              <Text size="md">
-                I have read and understood the{' '}
-                <a href="https://gnosis-safe.io/terms" rel="noopener noreferrer" target="_blank">
-                  Terms
-                </a>{' '}
-                and this Disclaimer, and agree to be bound by them.
-              </Text>
-            </>
-          }
-          onCancel={redirectToBalance}
-          onConfirm={onAcceptLegalDisclaimer}
-          title="Disclaimer"
-        />
-      )
     }
 
     if (network === 'UNKNOWN' || !granted) {
@@ -149,8 +130,6 @@ function Apps({ closeModal, closeSnackbar, enqueueSnackbar, openModal }) {
       )
     }
 
-    const app = getSelectedApp()
-
     return (
       <IframeWrapper>
         {appIsLoading && (
@@ -158,76 +137,26 @@ function Apps({ closeModal, closeSnackbar, enqueueSnackbar, openModal }) {
             <Loader size="md" />
           </LoadingContainer>
         )}
-        <StyledIframe frameBorder="0" id={`iframe-${app.name}`} ref={iframeRef} src={app.url} title={app.name} />
+        <StyledIframe
+          frameBorder="0"
+          id={`iframe-${selectedApp.name}`}
+          ref={iframeRef}
+          src={selectedApp.url}
+          title={selectedApp.name}
+        />
       </IframeWrapper>
     )
   }
 
-  const onAppAdded = (app: SafeApp) => {
+  const enabledApps = useMemo(() => appList.filter((a) => !a.disabled), [appList])
-    const newAppList = [
-      { url: app.url, disabled: false },
-      ...appList.map((a) => ({
-        url: a.url,
-        disabled: a.disabled,
-      })),
-    ]
-    saveToStorage(APPS_STORAGE_KEY, newAppList)
-
-    setAppList([...appList, { ...app, disabled: false }])
-  }
-
-  const selectFirstApp = useCallback(
-    (apps) => {
-      const firstEnabledApp = apps.find((a) => !a.disabled)
-      if (firstEnabledApp) {
-        onSelectApp(firstEnabledApp.id)
-      }
-    },
-    [onSelectApp],
-  )
-
-  const onAppToggle = async (appId, enabled) => {
-    // update in-memory list
-    const copyAppList = [...appList]
-
-    const app = copyAppList.find((a) => a.id === appId)
-    if (!app) {
-      return
-    }
-
-    app.disabled = !enabled
-    setAppList(copyAppList)
-
-    // update storage list
-    const persistedAppList = (await loadFromStorage<StoredSafeApp[]>(APPS_STORAGE_KEY)) || []
-    let storageApp = persistedAppList.find((a) => a.url === app.url)
-
-    if (!storageApp) {
-      storageApp = { url: app.url }
-      storageApp.disabled = !enabled
-      persistedAppList.push(storageApp)
-    } else {
-      storageApp.disabled = !enabled
-    }
-
-    saveToStorage(APPS_STORAGE_KEY, persistedAppList)
-
-    // select app
-    const selectedApp = getSelectedApp()
-    if (!selectedApp || (selectedApp && selectedApp.id === appId)) {
-      setSelectedApp(undefined)
-      selectFirstApp(copyAppList)
-    }
-  }
-
-  const getEnabledApps = () => appList.filter((a) => !a.disabled)
 
   const sendMessageToIframe = useCallback(
     (messageId, data) => {
-      const app = getSelectedApp()
+      if (iframeEl && selectedApp) {
-      iframeEl?.contentWindow.postMessage({ messageId, data }, app.url)
+        iframeEl.contentWindow.postMessage({ messageId, data }, selectedApp.url)
+      }
     },
-    [getSelectedApp, iframeEl],
+    [iframeEl, selectedApp],
   )
 
   // handle messages from iframe
@@ -242,22 +171,16 @@ function Apps({ closeModal, closeSnackbar, enqueueSnackbar, openModal }) {
         case operations.SEND_TRANSACTIONS: {
           const onConfirm = async () => {
             closeModal()
-            await sendTransactions(
+
-              dispatch,
+            await sendTransactions(dispatch, safeAddress, data.data, enqueueSnackbar, closeSnackbar, selectedApp.id)
-              safeAddress,
-              data.data,
-              enqueueSnackbar,
-              closeSnackbar,
-              getSelectedApp().id,
-            )
           }
 
           confirmTransactions(
             safeAddress,
             safeName,
             ethBalance,
-            getSelectedApp().name,
+            selectedApp.name,
-            getSelectedApp().iconUrl,
+            selectedApp.iconUrl,
             data.data,
             openModal,
             closeModal,
@@ -287,8 +210,7 @@ function Apps({ closeModal, closeSnackbar, enqueueSnackbar, openModal }) {
         return
       }
 
-      const app = getSelectedApp()
+      if (!selectedApp.url.includes(origin)) {
-      if (!app.url.includes(origin)) {
         console.error(`ThirdPartyApp: A message was received from an unknown origin ${origin}`)
         return
       }
@@ -303,65 +225,11 @@ function Apps({ closeModal, closeSnackbar, enqueueSnackbar, openModal }) {
     }
   })
 
-  // load legalDisclaimer
-  useEffect(() => {
-    const checkLegalDisclaimer = async () => {
-      const legalDisclaimer = await loadFromStorage(APPS_LEGAL_DISCLAIMER_STORAGE_KEY)
-
-      if (legalDisclaimer) {
-        setLegalDisclaimerAccepted(true)
-      }
-    }
-
-    checkLegalDisclaimer()
-  }, [])
-
-  // Load apps list
-  useEffect(() => {
-    const loadApps = async () => {
-      // recover apps from storage:
-      // * third-party apps added by the user
-      // * disabled status for both static and third-party apps
-      const persistedAppList = (await loadFromStorage<StoredSafeApp[]>(APPS_STORAGE_KEY)) || []
-      const list = [...persistedAppList]
-
-      staticAppsList.forEach((staticApp) => {
-        if (!list.some((persistedApp) => persistedApp.url === staticApp.url)) {
-          list.push(staticApp)
-        }
-      })
-
-      const apps = []
-      // using the appURL to recover app info
-      for (let index = 0; index < list.length; index++) {
-        try {
-          const currentApp = list[index]
-
-          const appInfo: SafeApp = await getAppInfoFromUrl(currentApp.url)
-          if (appInfo.error) {
-            throw Error(`There was a problem trying to load app ${currentApp.url}`)
-          }
-
-          appInfo.disabled = currentApp.disabled === undefined ? false : currentApp.disabled
-
-          apps.push(appInfo)
-        } catch (error) {
-          console.error(error)
-        }
-      }
-
-      setAppList(apps)
-      setLoading(false)
-      selectFirstApp(apps)
-    }
-
-    if (!appList.length) {
-      loadApps()
-    }
-  }, [appList.length, selectFirstApp])
-
   // on iframe change
   useEffect(() => {
+    const sendMessageToIframe = (messageId, data) => {
+      iframeEl.contentWindow.postMessage({ messageId, data }, selectedApp.url)
+    }
     const onIframeLoaded = () => {
       setAppIsLoading(false)
       sendMessageToIframe(operations.ON_SAFE_INFO, {
@@ -371,8 +239,7 @@ function Apps({ closeModal, closeSnackbar, enqueueSnackbar, openModal }) {
       })
     }
 
-    const app = getSelectedApp()
+    if (!iframeEl || !selectedApp || !isSameHref(iframeEl.src, selectedApp.url)) {
-    if (!iframeEl || !selectedApp || !isSameHref(iframeEl.src, app.url)) {
       return
     }
 
@@ -381,9 +248,9 @@ function Apps({ closeModal, closeSnackbar, enqueueSnackbar, openModal }) {
     return () => {
       iframeEl.removeEventListener('load', onIframeLoaded)
     }
-  }, [ethBalance, getSelectedApp, iframeEl, network, safeAddress, selectedApp, sendMessageToIframe])
+  }, [ethBalance, iframeEl, network, safeAddress, selectedApp])
 
-  if (loading || !appList.length) {
+  if (loadingAppList || !appList.length) {
     return (
       <LoadingContainer>
         <Loader size="md" />
@@ -396,26 +263,12 @@ function Apps({ closeModal, closeSnackbar, enqueueSnackbar, openModal }) {
       <Menu>
         <ManageApps appList={appList} onAppAdded={onAppAdded} onAppToggle={onAppToggle} />
       </Menu>
-      {getEnabledApps().length ? (
+      {enabledApps.length ? (
         <LCL.Wrapper>
           <LCL.Menu>
-            <LCL.List activeItem={selectedApp} items={getEnabledApps()} onItemClick={onSelectApp} />
+            <LCL.List activeItem={selectedAppId} items={enabledApps} onItemClick={onSelectApp} />
           </LCL.Menu>
           <LCL.Content>{getContent()}</LCL.Content>
-          {/* <LCL.Footer>
-            {getSelectedApp() && getSelectedApp().providedBy && (
-              <>
-                <p>This App is provided by </p>
-                <ButtonLink
-                  onClick={() => window.open(getSelectedApp().providedBy.url, '_blank')}
-                  size="lg"
-                  testId="manage-tokens-btn"
-                >
-                  {selectedApp && getSelectedApp().providedBy.name}
-                </ButtonLink>
-              </>
-            )}
-          </LCL.Footer> */}
         </LCL.Wrapper>
       ) : (
         <Card style={{ marginBottom: '24px' }}>

src/utils/storage/index.ts

@@ -24,10 +24,7 @@ export const loadFromStorage = async <T = unknown>(key: string): Promise<T | und
   }
 }
 
-export const saveToStorage = async (
+export const saveToStorage = async <T = unknown>(key: string, value: T): Promise<void> => {
-  key: string,
-  value: Record<string, unknown> | boolean | string | number | Array<unknown>,
-): Promise<void> => {
   try {
     const stringifiedValue = JSON.stringify(value)
     await storage.set(`${PREFIX}__${key}`, stringifiedValue)