From 71f1ea7ad18dbea9d52d798e25fc495d997d0016 Mon Sep 17 00:00:00 2001 From: Fernando Date: Thu, 10 Dec 2020 12:07:38 -0300 Subject: [PATCH] (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 --- src/components/forms/validator.test.ts | 16 ++-- src/components/forms/validator.ts | 20 ++--- .../addressBook/store/selectors/index.ts | 5 +- src/logic/safe/store/selectors/index.ts | 9 +-- .../CreateEditEntryModal/index.tsx | 80 ++++++++++--------- .../AddressBook/CreateEditEntryModal/style.ts | 52 ++++++------ .../safe/components/AddressBook/index.tsx | 45 ++++++----- 7 files changed, 116 insertions(+), 111 deletions(-) diff --git a/src/components/forms/validator.test.ts b/src/components/forms/validator.test.ts index 3f5d45ef..6362722a 100644 --- 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) }) }) diff --git a/src/components/forms/validator.ts b/src/components/forms/validator.ts index 1f01b4d0..1f356a53 100644 --- 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): 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 = []) => (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 => diff --git a/src/logic/addressBook/store/selectors/index.ts b/src/logic/addressBook/store/selectors/index.ts index 128b9aa7..e4afde58 100644 --- 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, diff --git a/src/logic/safe/store/selectors/index.ts b/src/logic/safe/store/selectors/index.ts index 23959e6f..a526925d 100644 --- 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( diff --git a/src/routes/safe/components/AddressBook/CreateEditEntryModal/index.tsx b/src/routes/safe/components/AddressBook/CreateEditEntryModal/index.tsx index d77d11e0..bb7aa3e8 100644 --- 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 ( - {entryToEdit ? 'Edit entry' : 'Create entry'} + {isNew ? 'Create entry' : 'Edit entry'} - + {(...args) => { + const formState = args[2] const mutators = args[3] const handleScan = (value, closeQrModal) => { let scannedAddress = value @@ -86,13 +99,11 @@ const CreateEditEntryModalComponent = ({ @@ -101,18 +112,16 @@ const CreateEditEntryModalComponent = ({ (isNew ? isUniqueAddress(value) : undefined)]} /> - {!entryToEdit ? ( + {isNew ? ( @@ -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'} @@ -142,5 +152,3 @@ const CreateEditEntryModalComponent = ({ ) } - -export default withStyles(styles as any)(CreateEditEntryModalComponent) diff --git a/src/routes/safe/components/AddressBook/CreateEditEntryModal/style.ts b/src/routes/safe/components/AddressBook/CreateEditEntryModal/style.ts index 2ffa0723..839697a1 100644 --- 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', + }, + }), +) diff --git a/src/routes/safe/components/AddressBook/index.tsx b/src/routes/safe/components/AddressBook/index.tsx index b10cc4c4..e6cbaafa 100644 --- 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(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 => { { - setSelectedEntry(null) - setEditCreateEntryModalOpen(!editCreateEntryModalOpen) + setSelectedEntry(initialEntryState) + setEditCreateEntryModalOpen(true) }} size="lg" testId="manage-tokens-btn"