(Fix) Duplicated address validation (#1699)

* use createStyles/makeStyles * simplify `addressBookQueryParamsSelector` * avoid using `createSelector` as memoization in this scenario is not working as expected and list is not refreshed * refactor `uniqueAddress` curried function and strategy to validate - `selectedEntry` being `null` made the code harder to follow * fix `uniqueAddress` validator tests * use arrow function
2020-12-10 12:07:38 -03:00 · 2020-12-10 12:07:38 -03:00 · 71f1ea7ad1
parent 4079ff9abe
commit 71f1ea7ad1
7 changed files with 116 additions and 111 deletions
--- a/src/components/forms/validator.test.ts
+++ b/src/components/forms/validator.test.ts
@ -166,22 +166,16 @@ describe('Forms > Validators', () => {
  })

  describe('uniqueAddress validator', () => {
-    it('Returns undefined for an address not contained in the passed array', async () => {
+    it('Returns undefined if `addresses` does not contains the provided address', async () => {
      const addresses = ['0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe']

-      expect(uniqueAddress(addresses)()).toBeUndefined()
+      expect(uniqueAddress(addresses)('0x2D6F2B448b0F711Eb81f2929566504117d67E44F')).toBeUndefined()
    })

-    it('Returns an error message for an array with duplicated values', async () => {
-      const addresses = ['0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe', '0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe']
+    it('Returns an error message if address is in the `addresses` list already', async () => {
+      const addresses = ['0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe', '0x2D6F2B448b0F711Eb81f2929566504117d67E44F']

-      expect(uniqueAddress(addresses)()).toEqual(ADDRESS_REPEATED_ERROR)
-    })
-
-    it('Returns an error message for an array with duplicated checksum and not checksum values', async () => {
-      const addresses = ['0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe', '0xde0b295669a9fd93d5f28d9ec85e40f4cb697bae']
-
-      expect(uniqueAddress(addresses)()).toEqual(ADDRESS_REPEATED_ERROR)
+      expect(uniqueAddress(addresses)('0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe')).toEqual(ADDRESS_REPEATED_ERROR)
    })
  })

--- a/src/components/forms/validator.ts
+++ b/src/components/forms/validator.ts
@ -1,8 +1,10 @@
-import { getWeb3 } from 'src/logic/wallets/getWeb3'
+import { List } from 'immutable'
 import memoize from 'lodash.memoize'
+
+import { sameAddress } from 'src/logic/wallets/ethAddresses'
+import { getWeb3 } from 'src/logic/wallets/getWeb3'
 import { isFeatureEnabled } from 'src/config'
 import { FEATURES } from 'src/config/networks/network.d'
-import { List } from 'immutable'

 type ValidatorReturnType = string | undefined
 export type GenericValidatorType = (...args: unknown[]) => ValidatorReturnType
@ -87,17 +89,9 @@ export const minMaxLength = (minLen: number, maxLen: number) => (value: string):

 export const ADDRESS_REPEATED_ERROR = 'Address already introduced'

-export const uniqueAddress = (addresses: string[] | List<string>): GenericValidatorType => (): ValidatorReturnType => {
-  // @ts-expect-error both list and array have signatures for map but TS thinks they're not compatible
-  const lowercaseAddresses = addresses.map((address) => address.toLowerCase())
-  const uniqueAddresses = new Set(lowercaseAddresses)
-  const lengthPropName = 'size' in addresses ? 'size' : 'length'
-
-  if (uniqueAddresses.size !== addresses?.[lengthPropName]) {
-    return ADDRESS_REPEATED_ERROR
-  }
-
-  return undefined
+export const uniqueAddress = (addresses: string[] | List<string> = []) => (address?: string): string | undefined => {
+  const addressExists = addresses.some((addressFromList) => sameAddress(addressFromList, address))
+  return addressExists ? ADDRESS_REPEATED_ERROR : undefined
 }

 export const composeValidators = (...validators: Validator[]) => (value: unknown): ValidatorReturnType =>
--- a/src/logic/addressBook/store/selectors/index.ts
+++ b/src/logic/addressBook/store/selectors/index.ts
@ -8,9 +8,10 @@ import { AddressBookState } from 'src/logic/addressBook/model/addressBook'

 export const addressBookSelector = (state: AppReduxState): AddressBookState => state[ADDRESS_BOOK_REDUCER_ID]

-export const addressBookAddressesListSelector = createSelector(addressBookSelector, (addressBook): string[] => {
+export const addressBookAddressesListSelector = (state: AppReduxState): string[] => {
+  const addressBook = addressBookSelector(state)
  return addressBook.map((entry) => entry.address)
-})
+}

 export const getNameFromAddressBookSelector = createSelector(
  addressBookSelector,
--- a/src/logic/safe/store/selectors/index.ts
+++ b/src/logic/safe/store/selectors/index.ts
@ -72,14 +72,13 @@ export const safeTransactionsSelector = createSelector(
  },
 )

-export const addressBookQueryParamsSelector = (state: AppReduxState): string | null => {
+export const addressBookQueryParamsSelector = (state: AppReduxState): string | undefined => {
  const { location } = state.router
-  let entryAddressToEditOrCreateNew = null
-  if (location && location.query) {
+
+  if (location?.query) {
    const { entryAddress } = location.query
-    entryAddressToEditOrCreateNew = entryAddress
+    return entryAddress
  }
-  return entryAddressToEditOrCreateNew
 }

 export const safeCancellationTransactionsSelector = createSelector(
--- a/src/routes/safe/components/AddressBook/CreateEditEntryModal/index.tsx
+++ b/src/routes/safe/components/AddressBook/CreateEditEntryModal/index.tsx
@ -1,10 +1,9 @@
 import IconButton from '@material-ui/core/IconButton'
-import { withStyles } from '@material-ui/core/styles'
 import Close from '@material-ui/icons/Close'
-import React from 'react'
+import React, { ReactElement } from 'react'
 import { useSelector } from 'react-redux'

-import { styles } from './style'
+import { useStyles } from './style'

 import Modal from 'src/components/Modal'
 import { ScanQRWrapper } from 'src/components/ScanQRModal/ScanQRWrapper'
@ -20,55 +19,69 @@ import Hairline from 'src/components/layout/Hairline'
 import Paragraph from 'src/components/layout/Paragraph'
 import Row from 'src/components/layout/Row'
 import { addressBookAddressesListSelector } from 'src/logic/addressBook/store/selectors'
+import { AddressBookEntry } from 'src/logic/addressBook/model/addressBook'
+import { Entry } from 'src/routes/safe/components/AddressBook/index'

 export const CREATE_ENTRY_INPUT_NAME_ID = 'create-entry-input-name'
 export const CREATE_ENTRY_INPUT_ADDRESS_ID = 'create-entry-input-address'
 export const SAVE_NEW_ENTRY_BTN_ID = 'save-new-entry-btn-id'

-const CreateEditEntryModalComponent = ({
-  classes,
+const formMutators = {
+  setOwnerAddress: (args, state, utils) => {
+    utils.changeValue(state, 'address', () => args[0])
+  },
+}
+
+type CreateEditEntryModalProps = {
+  editEntryModalHandler: (entry: AddressBookEntry) => void
+  entryToEdit: Entry
+  isOpen: boolean
+  newEntryModalHandler: (entry: AddressBookEntry) => void
+  onClose: () => void
+}
+
+export const CreateEditEntryModal = ({
  editEntryModalHandler,
  entryToEdit,
  isOpen,
  newEntryModalHandler,
  onClose,
-}) => {
-  const onFormSubmitted = (values) => {
-    if (entryToEdit && !entryToEdit.entry.isNew) {
-      editEntryModalHandler(values)
-    } else {
+}: CreateEditEntryModalProps): ReactElement => {
+  const classes = useStyles()
+
+  const { isNew, ...initialValues } = entryToEdit.entry
+
+  const onFormSubmitted = (values: AddressBookEntry) => {
+    if (isNew) {
      newEntryModalHandler(values)
+    } else {
+      editEntryModalHandler(values)
    }
  }

-  const addressBookAddressesList = useSelector(addressBookAddressesListSelector)
-  const entryDoesntExist = uniqueAddress(addressBookAddressesList)
-
-  const formMutators = {
-    setOwnerAddress: (args, state, utils) => {
-      utils.changeValue(state, 'address', () => args[0])
-    },
-  }
+  const storedAddresses = useSelector(addressBookAddressesListSelector)
+  const isUniqueAddress = uniqueAddress(storedAddresses)

  return (
    <Modal
-      description={entryToEdit ? 'Edit addressBook entry' : 'Create new addressBook entry'}
+      description={isNew ? 'Create new addressBook entry' : 'Edit addressBook entry'}
      handleClose={onClose}
      open={isOpen}
      paperClassName={classes.smallerModalWindow}
-      title={entryToEdit ? 'Edit entry' : 'Create new entry'}
+      title={isNew ? 'Create new entry' : 'Edit entry'}
    >
      <Row align="center" className={classes.heading} grow>
        <Paragraph className={classes.manage} noMargin weight="bolder">
-          {entryToEdit ? 'Edit entry' : 'Create entry'}
+          {isNew ? 'Create entry' : 'Edit entry'}
        </Paragraph>
        <IconButton disableRipple onClick={onClose}>
          <Close className={classes.close} />
        </IconButton>
      </Row>
      <Hairline />
-      <GnoForm formMutators={formMutators} onSubmit={onFormSubmitted}>
+      <GnoForm formMutators={formMutators} onSubmit={onFormSubmitted} initialValues={initialValues}>
        {(...args) => {
+          const formState = args[2]
          const mutators = args[3]
          const handleScan = (value, closeQrModal) => {
            let scannedAddress = value
@ -86,13 +99,11 @@ const CreateEditEntryModalComponent = ({
                <Row margin="md">
                  <Col xs={11}>
                    <Field
-                      className={classes.addressInput}
                      component={TextField}
-                      defaultValue={entryToEdit ? entryToEdit.entry.name : undefined}
                      name="name"
-                      placeholder={entryToEdit ? 'Entry name' : 'New entry'}
+                      placeholder="Name"
                      testId={CREATE_ENTRY_INPUT_NAME_ID}
-                      text={entryToEdit ? 'Entry*' : 'New entry*'}
+                      text="Name"
                      type="text"
                      validate={composeValidators(required, minMaxLength(1, 50))}
                    />
@ -101,18 +112,16 @@ const CreateEditEntryModalComponent = ({
                <Row margin="md">
                  <Col xs={11}>
                    <AddressInput
-                      className={classes.addressInput}
-                      defaultValue={entryToEdit ? entryToEdit.entry.address : undefined}
-                      disabled={!!entryToEdit}
+                      disabled={!isNew}
                      fieldMutator={mutators.setOwnerAddress}
                      name="address"
-                      placeholder="Owner address*"
+                      placeholder="Address*"
                      testId={CREATE_ENTRY_INPUT_ADDRESS_ID}
-                      text="Owner address*"
-                      validators={entryToEdit ? undefined : [entryDoesntExist]}
+                      text="Address*"
+                      validators={[(value?: string) => (isNew ? isUniqueAddress(value) : undefined)]}
                    />
                  </Col>
-                  {!entryToEdit ? (
+                  {isNew ? (
                    <Col center="xs" className={classes} middle="xs" xs={1}>
                      <ScanQRWrapper handleScan={handleScan} />
                    </Col>
@ -131,8 +140,9 @@ const CreateEditEntryModalComponent = ({
                  testId={SAVE_NEW_ENTRY_BTN_ID}
                  type="submit"
                  variant="contained"
+                  disabled={!formState.valid}
                >
-                  {entryToEdit ? 'Save' : 'Create'}
+                  {isNew ? 'Create' : 'Save'}
                </Button>
              </Row>
            </>
@ -142,5 +152,3 @@ const CreateEditEntryModalComponent = ({
    </Modal>
  )
 }
-
-export default withStyles(styles as any)(CreateEditEntryModalComponent)
--- a/src/routes/safe/components/AddressBook/CreateEditEntryModal/style.ts
+++ b/src/routes/safe/components/AddressBook/CreateEditEntryModal/style.ts
@ -1,26 +1,30 @@
+import { createStyles, makeStyles } from '@material-ui/core/styles'
+
 import { lg, md } from 'src/theme/variables'

-export const styles = () => ({
-  heading: {
-    padding: lg,
-    justifyContent: 'space-between',
-    boxSizing: 'border-box',
-  },
-  manage: {
-    fontSize: lg,
-  },
-  container: {
-    padding: `${md} ${lg}`,
-  },
-  close: {
-    height: '35px',
-    width: '35px',
-  },
-  buttonRow: {
-    height: '84px',
-    justifyContent: 'center',
-  },
-  smallerModalWindow: {
-    height: 'auto',
-  },
-})
+export const useStyles = makeStyles(
+  createStyles({
+    heading: {
+      padding: lg,
+      justifyContent: 'space-between',
+      boxSizing: 'border-box',
+    },
+    manage: {
+      fontSize: lg,
+    },
+    container: {
+      padding: `${md} ${lg}`,
+    },
+    close: {
+      height: '35px',
+      width: '35px',
+    },
+    buttonRow: {
+      height: '84px',
+      justifyContent: 'center',
+    },
+    smallerModalWindow: {
+      height: 'auto',
+    },
+  }),
+)
--- a/src/routes/safe/components/AddressBook/index.tsx
+++ b/src/routes/safe/components/AddressBook/index.tsx
@ -3,7 +3,7 @@ import TableContainer from '@material-ui/core/TableContainer'
 import TableRow from '@material-ui/core/TableRow'
 import { makeStyles } from '@material-ui/core/styles'
 import cn from 'classnames'
-import React, { useEffect, useState } from 'react'
+import React, { ReactElement, useEffect, useState } from 'react'
 import { useDispatch, useSelector } from 'react-redux'

 import { styles } from './style'
@ -21,8 +21,8 @@ import { addAddressBookEntry } from 'src/logic/addressBook/store/actions/addAddr
 import { removeAddressBookEntry } from 'src/logic/addressBook/store/actions/removeAddressBookEntry'
 import { updateAddressBookEntry } from 'src/logic/addressBook/store/actions/updateAddressBookEntry'
 import { addressBookSelector } from 'src/logic/addressBook/store/selectors'
-import { isUserAnOwnerOfAnySafe } from 'src/logic/wallets/ethAddresses'
-import CreateEditEntryModal from 'src/routes/safe/components/AddressBook/CreateEditEntryModal'
+import { isUserAnOwnerOfAnySafe, sameAddress } from 'src/logic/wallets/ethAddresses'
+import { CreateEditEntryModal } from 'src/routes/safe/components/AddressBook/CreateEditEntryModal'
 import DeleteEntryModal from 'src/routes/safe/components/AddressBook/DeleteEntryModal'
 import {
  AB_ADDRESS_ID,
@ -47,20 +47,24 @@ interface AddressBookSelectedEntry extends AddressBookEntry {
  isNew?: boolean
 }

-const AddressBookTable = (): React.ReactElement => {
+export type Entry = {
+  entry: AddressBookSelectedEntry
+  index?: number
+  isOwnerAddress?: boolean
+}
+
+const initialEntryState: Entry = { entry: { address: '', name: '', isNew: true } }
+
+const AddressBookTable = (): ReactElement => {
  const classes = useStyles()
  const columns = generateColumns()
-  const autoColumns = columns.filter((c) => !c.custom)
+  const autoColumns = columns.filter(({ custom }) => !custom)
  const dispatch = useDispatch()
  const safesList = useSelector(safesListSelector)
  const entryAddressToEditOrCreateNew = useSelector(addressBookQueryParamsSelector)
  const addressBook = useSelector(addressBookSelector)
  const granted = useSelector(grantedSelector)
-  const [selectedEntry, setSelectedEntry] = useState<{
-    entry?: AddressBookSelectedEntry
-    index?: number
-    isOwnerAddress?: boolean
-  } | null>(null)
+  const [selectedEntry, setSelectedEntry] = useState<Entry>(initialEntryState)
  const [editCreateEntryModalOpen, setEditCreateEntryModalOpen] = useState(false)
  const [deleteEntryModalOpen, setDeleteEntryModalOpen] = useState(false)
  const [sendFundsModalOpen, setSendFundsModalOpen] = useState(false)
@ -78,8 +82,9 @@ const AddressBookTable = (): React.ReactElement => {

  useEffect(() => {
    if (entryAddressToEditOrCreateNew) {
-      const checksumEntryAdd = checksumAddress(entryAddressToEditOrCreateNew)
-      const oldEntryIndex = addressBook.findIndex((entry) => entry.address === checksumEntryAdd)
+      const address = checksumAddress(entryAddressToEditOrCreateNew)
+      const oldEntryIndex = addressBook.findIndex((entry) => sameAddress(entry.address, address))
+
      if (oldEntryIndex >= 0) {
        // Edit old entry
        setSelectedEntry({ entry: addressBook[oldEntryIndex], index: oldEntryIndex })
@ -88,7 +93,7 @@ const AddressBookTable = (): React.ReactElement => {
        setSelectedEntry({
          entry: {
            name: '',
-            address: checksumEntryAdd,
+            address,
            isNew: true,
          },
        })
@ -96,7 +101,7 @@ const AddressBookTable = (): React.ReactElement => {
    }
  }, [addressBook, entryAddressToEditOrCreateNew])

-  const newEntryModalHandler = (entry) => {
+  const newEntryModalHandler = (entry: AddressBookEntry) => {
    setEditCreateEntryModalOpen(false)
    const checksumEntries = {
      ...entry,
@ -105,8 +110,8 @@ const AddressBookTable = (): React.ReactElement => {
    dispatch(addAddressBookEntry(makeAddressBookEntry(checksumEntries)))
  }

-  const editEntryModalHandler = (entry) => {
-    setSelectedEntry(null)
+  const editEntryModalHandler = (entry: AddressBookEntry) => {
+    setSelectedEntry(initialEntryState)
    setEditCreateEntryModalOpen(false)
    const checksumEntries = {
      ...entry,
@ -116,8 +121,8 @@ const AddressBookTable = (): React.ReactElement => {
  }

  const deleteEntryModalHandler = () => {
-    const entryAddress = selectedEntry && selectedEntry.entry ? checksumAddress(selectedEntry.entry.address) : ''
-    setSelectedEntry(null)
+    const entryAddress = selectedEntry?.entry ? checksumAddress(selectedEntry.entry.address) : ''
+    setSelectedEntry(initialEntryState)
    setDeleteEntryModalOpen(false)
    dispatch(removeAddressBookEntry(entryAddress))
  }
@ -128,8 +133,8 @@ const AddressBookTable = (): React.ReactElement => {
        <Col end="sm" xs={12}>
          <ButtonLink
            onClick={() => {
-              setSelectedEntry(null)
-              setEditCreateEntryModalOpen(!editCreateEntryModalOpen)
+              setSelectedEntry(initialEntryState)
+              setEditCreateEntryModalOpen(true)
            }}
            size="lg"
            testId="manage-tokens-btn"