Bugfix - Safes added via URL not stored when changing name (#2310)

* Fix Types

* Fix adding a safe to the list after it was accessed via URL

Co-authored-by: Daniel Sanchez <daniel.sanchez@gnosis.pm>

src/logic/safe/store/middleware/safeStorage.ts

@@ -1,3 +1,4 @@
+import { Store } from 'redux'
 import { saveDefaultSafe, saveSafes } from 'src/logic/safe/utils'
 import { ADD_SAFE_OWNER } from 'src/logic/safe/store/actions/addSafeOwner'
 import { EDIT_SAFE_OWNER } from 'src/logic/safe/store/actions/editSafeOwner'
@@ -12,6 +13,7 @@ import { makeAddressBookEntry } from 'src/logic/addressBook/model/addressBook'
 import { checksumAddress } from 'src/utils/checksumAddress'
 import { isValidAddressBookName } from 'src/logic/addressBook/utils'
 import { addOrUpdateAddressBookEntry } from 'src/logic/addressBook/store/actions/addOrUpdateAddressBookEntry'
+import { SafeRecord } from '../models/safe'
 
 const watchedActions = [
   UPDATE_SAFE,
@@ -24,7 +26,16 @@ const watchedActions = [
   SET_DEFAULT_SAFE,
 ]
 
-export const safeStorageMiddleware = (store) => (next) => async (action) => {
+type SafeProps = {
+  safe: SafeRecord
+}
+
+export const safeStorageMiddleware = (store: Store) => (
+  next: (arg0: { type: string; payload: string | SafeProps | { address: string; name: string } }) => any,
+) => async (action: {
+  type: string
+  payload: string | SafeProps | { name: string; address: string }
+}): Promise<any> => {
   const handledAction = next(action)
 
   if (watchedActions.includes(action.type)) {
@@ -35,8 +46,8 @@ export const safeStorageMiddleware = (store) => (next) => async (action) => {
 
     switch (action.type) {
       case ADD_OR_UPDATE_SAFE: {
-        const { safe } = action.payload
+        const { safe } = action.payload as SafeProps
-        safe.owners.forEach((owner) => {
+        safe.owners.forEach((owner: { address: string; name: any }) => {
           const checksumEntry = makeAddressBookEntry({ address: checksumAddress(owner.address), name: owner.name })
           if (isValidAddressBookName(checksumEntry.name)) {
             dispatch(addOrUpdateAddressBookEntry(checksumEntry))
@@ -51,7 +62,7 @@ export const safeStorageMiddleware = (store) => (next) => async (action) => {
         break
       }
       case UPDATE_SAFE: {
-        const { name, address } = action.payload
+        const { name, address } = action.payload as { name: string; address: string }
         if (name) {
           dispatch(addOrUpdateAddressBookEntry(makeAddressBookEntry({ name, address })))
         }
@@ -59,7 +70,7 @@ export const safeStorageMiddleware = (store) => (next) => async (action) => {
       }
       case SET_DEFAULT_SAFE: {
         if (action.payload) {
-          saveDefaultSafe(action.payload)
+          saveDefaultSafe(action.payload as string)
         }
         break
       }

src/logic/safe/store/reducer/safe.ts

@@ -80,16 +80,16 @@ export default handleActions<AppReduxState['safes'], Payloads>(
       const safeAddress = safe.address
 
       const shouldUpdate = shouldSafeStoreBeUpdated(safe, state.getIn(['safes', safeAddress]))
-      let loadedViaUrl = safe.loadedViaUrl
-
-      if (!state.hasIn(['safes', safeAddress])) {
-        loadedViaUrl = !safe?.name || safe?.name === LOADED_SAFE_KEY
-      }
 
       return shouldUpdate
         ? state.updateIn(
             ['safes', safeAddress],
-            makeSafe({ name: safe?.name || LOADED_SAFE_KEY, address: safeAddress, loadedViaUrl }),
+            // This intermediate value is used as prevSafe if no previous state. Else is not used
+            makeSafe({
+              name: safe?.name || LOADED_SAFE_KEY,
+              address: safeAddress,
+              loadedViaUrl: !safe?.name || safe?.name === LOADED_SAFE_KEY,
+            }),
             (prevSafe) => updateSafeProps(prevSafe, safe),
           )
         : state
@@ -106,7 +106,12 @@ export default handleActions<AppReduxState['safes'], Payloads>(
       return shouldUpdate
         ? state.updateIn(
             ['safes', safeAddress],
-            makeSafe({ name: safe?.name || LOADED_SAFE_KEY, address: safeAddress, loadedViaUrl: !!safe?.name }),
+            // This intermediate value is used as prevSafe if no previous state. Else is not used
+            makeSafe({
+              name: safe?.name || LOADED_SAFE_KEY,
+              address: safeAddress,
+              loadedViaUrl: !safe?.name || safe?.name === LOADED_SAFE_KEY,
+            }),
             (prevSafe) => updateSafeProps(prevSafe, safe),
           )
         : state

src/routes/load/components/ReviewInformation/index.tsx

@@ -106,7 +106,7 @@ const ReviewComponent = ({ userAddress, values }: Props): React.ReactElement =>
             <Hairline />
             {owners.map((address, index) => (
               <>
-                <Row className={classes.owner}>
+                <Row className={classes.owner} testId={'load-safe-review-owner-name-' + index}>
                   <Col align="center" xs={12}>
                     <EthHashInfo
                       hash={address}

src/routes/load/container/Load.tsx

@@ -1,3 +1,4 @@
+import { List } from 'immutable'
 import * as React from 'react'
 import { useDispatch, useSelector } from 'react-redux'
 import { ETHEREUM_NETWORK } from 'src/config/networks/network.d'
@@ -13,7 +14,6 @@ import { SAFELIST_ADDRESS } from 'src/routes/routes'
 import { buildSafe } from 'src/logic/safe/store/actions/fetchSafe'
 import { history } from 'src/store'
 import { SafeOwner, SafeRecordProps } from 'src/logic/safe/store/models/safe'
-import { List } from 'immutable'
 import { checksumAddress } from 'src/utils/checksumAddress'
 import { networkSelector, providerNameSelector, userAccountSelector } from 'src/logic/wallets/store/selectors'
 import { addOrUpdateSafe } from 'src/logic/safe/store/actions/addOrUpdateSafe'
@@ -26,6 +26,9 @@ export const loadSafe = async (
 ): Promise<void> => {
   const safeProps = await buildSafe(safeAddress, safeName)
   safeProps.owners = owners
+  // We are manually adding the safe. We enforce this state in case the safe was previously
+  // accessed by URL
+  safeProps.loadedViaUrl = false
 
   const storedSafes = (await loadStoredSafes()) || {}
 

src/routes/safe/components/Settings/SafeDetails/index.tsx

@@ -1,6 +1,8 @@
+import { Icon, Link, Text } from '@gnosis.pm/safe-react-components'
 import { makeStyles } from '@material-ui/core/styles'
 import React, { useEffect, useState } from 'react'
 import { useDispatch, useSelector } from 'react-redux'
+import styled from 'styled-components'
 
 import { styles } from './style'
 
@@ -22,8 +24,6 @@ import { TX_NOTIFICATION_TYPES } from 'src/logic/safe/transactions'
 import { UpdateSafeModal } from 'src/routes/safe/components/Settings/UpdateSafeModal'
 import { grantedSelector } from 'src/routes/safe/container/selector'
 import { updateSafe } from 'src/logic/safe/store/actions/updateSafe'
-import { Icon, Link, Text } from '@gnosis.pm/safe-react-components'
-import styled from 'styled-components'
 
 import {
   latestMasterContractVersionSelector,
@@ -35,6 +35,7 @@ import {
 import { useAnalytics, SAFE_NAVIGATION_EVENT } from 'src/utils/googleAnalytics'
 import { fetchMasterCopies, MasterCopy, MasterCopyDeployer } from 'src/logic/contracts/api/masterCopies'
 import { getMasterCopyAddressFromProxyAddress } from 'src/logic/contracts/safeContracts'
+import { LOADED_SAFE_KEY } from 'src/utils/constants'
 
 export const SAFE_NAME_INPUT_TEST_ID = 'safe-name-input'
 export const SAFE_NAME_SUBMIT_BTN_TEST_ID = 'change-safe-name-btn'
@@ -70,7 +71,10 @@ const SafeDetails = (): React.ReactElement => {
   }
 
   const handleSubmit = (values) => {
-    dispatch(updateSafe({ address: safeAddress, name: values.safeName }))
+    // In case they set a name we assume the safe want to be stored even if it was opened via URL
+    dispatch(
+      updateSafe({ address: safeAddress, name: values.safeName, loadedViaUrl: values.safeName === LOADED_SAFE_KEY }),
+    )
 
     const notification = getNotificationsFromTxType(TX_NOTIFICATION_TYPES.SAFE_NAME_CHANGE_TX)
     dispatch(enqueueSnackbar(enhanceSnackbarForAction(notification.afterExecution.noMoreConfirmationsNeeded)))

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

@@ -26,7 +26,7 @@ import Row from 'src/components/layout/Row'
 import Span from 'src/components/layout/Span'
 import { addressBookSelector } from 'src/logic/addressBook/store/selectors'
 import { grantedSelector } from 'src/routes/safe/container/selector'
-import { safeNeedsUpdateSelector, safeOwnersSelector } from 'src/logic/safe/store/selectors'
+import { safeLoadedViaUrlSelector, safeNeedsUpdateSelector, safeOwnersSelector } from 'src/logic/safe/store/selectors'
 
 export const OWNERS_SETTINGS_TAB_TEST_ID = 'owner-settings-tab'
 
@@ -41,6 +41,7 @@ const Settings: React.FC = () => {
   const classes = useStyles()
   const [state, setState] = useState(INITIAL_STATE)
   const owners = useSelector(safeOwnersSelector)
+  const isSafeLoadedViaUrl = useSelector(safeLoadedViaUrlSelector)
   const needsUpdate = useSelector(safeNeedsUpdateSelector)
   const granted = useSelector(grantedSelector)
   const addressBook = useSelector(addressBookSelector)
@@ -66,10 +67,12 @@ const Settings: React.FC = () => {
   ) : (
     <>
       <Row className={classes.message}>
-        <ButtonLink className={classes.removeSafeBtn} color="error" onClick={onShow('RemoveSafe')} size="lg">
+        {!isSafeLoadedViaUrl && (
-          <Span className={classes.links}>Remove Safe</Span>
+          <ButtonLink className={classes.removeSafeBtn} color="error" onClick={onShow('RemoveSafe')} size="lg">
-          <Img alt="Trash Icon" className={classes.removeSafeIcon} src={RemoveSafeIcon} />
+            <Span className={classes.links}>Remove Safe</Span>
-        </ButtonLink>
+            <Img alt="Trash Icon" className={classes.removeSafeIcon} src={RemoveSafeIcon} />
+          </ButtonLink>
+        )}
         <RemoveSafeModal isOpen={showRemoveSafe} onClose={onHide('RemoveSafe')} />
       </Row>
       <Block className={classes.root}>