diff --git a/src/components/Stepper/index.tsx b/src/components/Stepper/index.tsx index 0147677fbf..07ee84979f 100644 --- a/src/components/Stepper/index.tsx +++ b/src/components/Stepper/index.tsx @@ -26,7 +26,7 @@ export interface StepperPageFormProps { } interface StepperPageProps { - validate?: (...args: unknown[]) => undefined | string[] | Promise> + validate?: (...args: unknown[]) => undefined | Record | Promise> component: ( ...args: unknown[] ) => (controls: React.ReactElement, formProps: StepperPageFormProps) => React.ReactElement diff --git a/src/routes/open/components/Layout.tsx b/src/routes/open/components/Layout.tsx index 78e5a43fea..45385f05e4 100644 --- a/src/routes/open/components/Layout.tsx +++ b/src/routes/open/components/Layout.tsx @@ -9,7 +9,7 @@ import Row from 'src/components/layout/Row' import { instantiateSafeContracts } from 'src/logic/contracts/safeContracts' import { Review } from 'src/routes/open/components/ReviewInformation' import SafeNameField from 'src/routes/open/components/SafeNameForm' -import { SafeOwnersPage } from 'src/routes/open/components/SafeOwnersConfirmationsForm' +import { SafeOwnersPage, validateOwnersForm } from 'src/routes/open/components/SafeOwnersConfirmationsForm' import { FIELD_CONFIRMATIONS, FIELD_CREATION_PROXY_SALT, @@ -133,7 +133,7 @@ export const Layout = (props: LayoutProps): React.ReactElement => { testId="create-safe-form" > - + diff --git a/src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx b/src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx index 76494f54c5..a14698421b 100644 --- a/src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx +++ b/src/routes/open/components/SafeOwnersConfirmationsForm/index.tsx @@ -4,7 +4,6 @@ import { makeStyles } from '@material-ui/core/styles' import CheckCircle from '@material-ui/icons/CheckCircle' import * as React from 'react' import { styles } from './style' -import { getAddressValidator } from './validators' import QRIcon from 'src/assets/icons/qrcode.svg' import trash from 'src/assets/icons/trash.svg' @@ -21,6 +20,7 @@ import { noErrorsOn, required, minMaxLength, + ADDRESS_REPEATED_ERROR, } from 'src/components/forms/validator' import Block from 'src/components/layout/Block' import Button from 'src/components/layout/Button' @@ -44,30 +44,70 @@ const { useState } = React export const ADD_OWNER_BUTTON = '+ Add another owner' -export const calculateValuesAfterRemoving = (index, notRemovedOwners, values) => { - const initialValues = { ...values } +/** + * Validates the whole OwnersForm, specially checks for non-repeated addresses + * + * If finds a repeated address, marks it as invalid + * @param {Object} values + * @return Object + */ +export const validateOwnersForm = (values: Record): Record => { + const { errors } = Object.keys(values).reduce( + (result, key) => { + if (/owner\d+Address/.test(key)) { + const address = values[key].toLowerCase() - const numOwnersAfterRemoving = notRemovedOwners - 1 + if (result.addresses.includes(address)) { + result.errors[key] = ADDRESS_REPEATED_ERROR + } - for (let i = index; i < numOwnersAfterRemoving; i += 1) { - initialValues[getOwnerNameBy(i)] = values[getOwnerNameBy(i + 1)] - initialValues[getOwnerAddressBy(i)] = values[getOwnerAddressBy(i + 1)] - } + result.addresses.push(address) + } + return result + }, + { addresses: [] as string[], errors: {} }, + ) + return errors +} - if (+values[FIELD_CONFIRMATIONS] === notRemovedOwners) { - initialValues[FIELD_CONFIRMATIONS] = numOwnersAfterRemoving.toString() - } +export const calculateValuesAfterRemoving = (index: number, values: Record): Record => + Object.keys(values) + .sort() + .reduce((newValues, key) => { + const ownerRelatedField = /owner(\d+)(Name|Address)/ - delete initialValues[getOwnerNameBy(index)] - delete initialValues[getOwnerAddressBy(index)] + if (!ownerRelatedField.test(key)) { + // no owner-related field + newValues[key] = values[key] + return newValues + } - return initialValues -} + const ownerToRemove = new RegExp(`owner${index}(Name|Address)`) + + if (ownerToRemove.test(key)) { + // skip, doing anything with the removed field + return newValues + } + + // we only have the owner-related fields to work with + // we must reduce the index value for those owners that come after the deleted owner row + const [, ownerOrder, ownerField] = key.match(ownerRelatedField) as RegExpMatchArray + + if (Number(ownerOrder) > index) { + // reduce by one the order of the owner + newValues[`owner${Number(ownerOrder) - 1}${ownerField}`] = values[key] + } else { + // previous owners to the deleted row + newValues[key] = values[key] + } + + return newValues + }, {} as Record) const useStyles = makeStyles(styles) const SafeOwnersForm = (props): React.ReactElement => { - const { errors, form, otherAccounts, values } = props + const { errors, form, values } = props const classes = useStyles() const validOwners = getNumOwnersFrom(values) @@ -87,7 +127,7 @@ const SafeOwnersForm = (props): React.ReactElement => { } const onRemoveRow = (index) => () => { - const initialValues = calculateValuesAfterRemoving(index, numOwners, values) + const initialValues = calculateValuesAfterRemoving(index, values) form.reset(initialValues) setNumOwners(numOwners - 1) @@ -171,7 +211,6 @@ const SafeOwnersForm = (props): React.ReactElement => { name={addressName} placeholder="Owner Address*" text="Owner Address" - validators={[getAddressValidator(otherAccounts, index)]} testId={`create-safe-address-field-${index}`} /> diff --git a/src/routes/open/components/SafeOwnersConfirmationsForm/tests/calculateValuesAfterRemoving.test.ts b/src/routes/open/components/SafeOwnersConfirmationsForm/tests/calculateValuesAfterRemoving.test.ts new file mode 100644 index 0000000000..a305388c03 --- /dev/null +++ b/src/routes/open/components/SafeOwnersConfirmationsForm/tests/calculateValuesAfterRemoving.test.ts @@ -0,0 +1,49 @@ +import { calculateValuesAfterRemoving } from 'src/routes/open/components/SafeOwnersConfirmationsForm' + +describe('calculateValuesAfterRemoving', () => { + it(`should properly remove the last owner row`, () => { + // Given + const formContent = { + name: 'My Safe', + owner0Name: 'Owner 0', + owner0Address: '0x90F8bf6A479f320ead074411a4B0e7944Ea8c9C1', + owner1Name: 'Owner 1', + owner1Address: '0xFFcf8FDEE72ac11b5c542428B35EEF5769C409f0', + } + + // When + const newFormContent = calculateValuesAfterRemoving(1, formContent) + + // Then + expect(newFormContent).toStrictEqual({ + name: 'My Safe', + owner0Name: 'Owner 0', + owner0Address: '0x90F8bf6A479f320ead074411a4B0e7944Ea8c9C1', + }) + }) + + it(`should properly remove an owner and recalculate fields indices`, () => { + // Given + const formContent = { + name: 'My Safe', + owner0Name: 'Owner 0', + owner0Address: '0x90F8bf6A479f320ead074411a4B0e7944Ea8c9C1', + owner1Name: 'Owner 1', + owner1Address: '0xFFcf8FDEE72ac11b5c542428B35EEF5769C409f0', + owner2Name: 'Owner 2', + owner2Address: '0x22d491Bde2303f2f43325b2108D26f1eAbA1e32b', + } + + // When + const newFormContent = calculateValuesAfterRemoving(1, formContent) + + // Then + expect(newFormContent).toStrictEqual({ + name: 'My Safe', + owner0Name: 'Owner 0', + owner0Address: '0x90F8bf6A479f320ead074411a4B0e7944Ea8c9C1', + owner1Name: 'Owner 2', + owner1Address: '0x22d491Bde2303f2f43325b2108D26f1eAbA1e32b', + }) + }) +})