fix: contact names should not allow duplication

ui/components/app/contact-list/contact-list.component.js

@@ -2,10 +2,12 @@ import React, { PureComponent } from 'react';
 import PropTypes from 'prop-types';
 import { sortBy } from 'lodash';
 import Button from '../../ui/button';
+import { BannerAlert, BannerAlertSeverity } from '../../component-library';
 import RecipientGroup from './recipient-group/recipient-group.component';
 
 export default class ContactList extends PureComponent {
   static propTypes = {
+    addressBook: PropTypes.array,
     searchForContacts: PropTypes.func,
     searchForRecents: PropTypes.func,
     searchForMyAccounts: PropTypes.func,
@@ -22,6 +24,19 @@ export default class ContactList extends PureComponent {
     isShowingAllRecent: false,
   };
 
+  renderDuplicateContactWarning() {
+    const { t } = this.context;
+
+    return (
+      <div className="send__select-recipient-wrapper__list__duplicate-contact-banner">
+        <BannerAlert
+          severity={BannerAlertSeverity.Warning}
+          description={t('duplicateContactWarning')}
+        />
+      </div>
+    );
+  }
+
   renderRecents() {
     const { t } = this.context;
     const { isShowingAllRecent } = this.state;
@@ -89,6 +104,24 @@ export default class ContactList extends PureComponent {
     );
   }
 
+  hasDuplicateContacts() {
+    const { addressBook } = this.props;
+
+    if (!addressBook) {
+      return false;
+    }
+
+    const seen = new Set();
+
+    const uniqueNamesOnly = addressBook.filter((contact) => {
+      const isDuplicate = seen.has(contact.name);
+      seen.add(contact.name);
+      return !isDuplicate;
+    });
+
+    return uniqueNamesOnly.length !== addressBook.length;
+  }
+
   render() {
     const {
       children,
@@ -97,9 +130,12 @@ export default class ContactList extends PureComponent {
       searchForMyAccounts,
     } = this.props;
 
+    const hasDuplicateContacts = this.hasDuplicateContacts();
+
     return (
       <div className="send__select-recipient-wrapper__list">
         {children || null}
+        {hasDuplicateContacts ? this.renderDuplicateContactWarning() : null}
         {searchForRecents ? this.renderRecents() : null}
         {searchForContacts ? this.renderAddressBook() : null}
         {searchForMyAccounts ? this.renderMyAccounts() : null}

ui/components/app/contact-list/contact-list.test.js

@@ -1,13 +1,27 @@
 import React from 'react';
 import configureMockStore from 'redux-mock-store';
 import { renderWithProvider } from '../../../../test/jest/rendering';
+import { MOCK_ADDRESS_BOOK } from '../../../../test/data/mock-data';
 import ContactList from '.';
 
 describe('Contact List', () => {
   const store = configureMockStore([])({
     metamask: {},
   });
 
+  const mockAddressBook = [...MOCK_ADDRESS_BOOK, MOCK_ADDRESS_BOOK[0]]; // Adding duplicate contact
+
+  it('displays the warning banner when multiple contacts have the same name', () => {
+    const { getByText } = renderWithProvider(
+      <ContactList addressBook={mockAddressBook} />,
+      store,
+    );
+
+    const duplicateContactBanner = getByText('You have duplicate contacts');
+
+    expect(duplicateContactBanner).toBeVisible();
+  });
+
   describe('given searchForContacts', () => {
     const selectRecipient = () => null;
     const selectedAddress = null;

ui/components/multichain/pages/send/components/domain-input-resolution-cell.tsx

@@ -97,6 +97,7 @@ export const DomainInputResolutionCell = ({
       alignItems={AlignItems.center}
       paddingBottom={2}
       style={{ cursor: 'pointer' }}
+      data-testid="multichain-send-page__recipient__item"
     >
       <Tooltip title={t('suggestedBySnap', [resolvingSnap])}>
         <BadgeWrapper

ui/pages/confirmations/send/send.scss

@@ -57,6 +57,10 @@
     height: 0;
 
     &__list {
+      &__duplicate-contact-banner {
+        padding: 8px 16px 0 16px;
+      }
+
       &__link {
         @include design-system.Paragraph;
 

ui/pages/settings/contact-list-tab/add-contact/add-contact.component.js

@@ -19,6 +19,8 @@ export default class AddContact extends PureComponent {
   };
 
   static propTypes = {
+    addressBook: PropTypes.array,
+    internalAccounts: PropTypes.array,
     addToAddressBook: PropTypes.func,
     history: PropTypes.object,
     scanQrCode: PropTypes.func,
@@ -33,7 +35,8 @@ export default class AddContact extends PureComponent {
   state = {
     newName: '',
     selectedAddress: '',
-    error: '',
+    addressInputError: '',
+    nameInputError: '',
     input: '',
   };
 
@@ -69,9 +72,9 @@ export default class AddContact extends PureComponent {
     const validEnsAddress = isValidDomainName(input);
 
     if (!validEnsAddress && !valid) {
-      this.setState({ error: INVALID_RECIPIENT_ADDRESS_ERROR });
+      this.setState({ addressInputError: INVALID_RECIPIENT_ADDRESS_ERROR });
     } else {
-      this.setState({ error: null });
+      this.setState({ addressInputError: null });
     }
   };
 
@@ -100,12 +103,37 @@ export default class AddContact extends PureComponent {
     );
   }
 
+  validateName = (newName) => {
+    const { addressBook, internalAccounts } = this.props;
+
+    const nameExistsInAddressBook = addressBook.some(
+      ({ name }) => name.toLowerCase().trim() === newName.toLowerCase().trim(),
+    );
+
+    const nameExistsInAccountList = internalAccounts.some(
+      ({ metadata }) =>
+        metadata.name.toLowerCase().trim() === newName.toLowerCase().trim(),
+    );
+
+    return !nameExistsInAddressBook && !nameExistsInAccountList;
+  };
+
+  handleNameChange = (newName) => {
+    const isValidName = this.validateName(newName);
+
+    this.setState({
+      nameInputError: isValidName ? null : this.context.t('nameAlreadyInUse'),
+    });
+
+    this.setState({ newName });
+  };
+
   render() {
     const { t } = this.context;
     const { history, addToAddressBook, domainError, domainResolutions } =
       this.props;
 
-    const errorToRender = domainError || this.state.error;
+    const addressError = domainError || this.state.addressInputError;
     const newAddress = this.state.selectedAddress || this.state.input;
     const validAddress =
       !isBurnAddress(newAddress) &&
@@ -121,10 +149,12 @@ export default class AddContact extends PureComponent {
             <TextField
               type="text"
               id="nickname"
+              placeholder={this.context.t('addAlias')}
               value={this.state.newName}
-              onChange={(e) => this.setState({ newName: e.target.value })}
+              onChange={(e) => this.handleNameChange(e.target.value)}
               fullWidth
               margin="dense"
+              error={this.state.nameInputError}
             />
           </div>
 
@@ -152,9 +182,9 @@ export default class AddContact extends PureComponent {
                     address={resolvedAddress}
                     domainName={addressBookEntryName ?? domainName}
                     onClick={() => {
+                      this.handleNameChange(domainName);
                       this.setState({
                         input: resolvedAddress,
-                        newName: this.state.newName || domainName,
                       });
                       this.props.resetDomainResolution();
                     }}
@@ -164,17 +194,20 @@ export default class AddContact extends PureComponent {
                 );
               })}
             </div>
-            {errorToRender && (
+            {addressError && (
               <div className="address-book__add-contact__error">
-                {t(errorToRender)}
+                {t(addressError)}
               </div>
             )}
           </div>
         </div>
         <PageContainerFooter
           cancelText={this.context.t('cancel')}
           disabled={Boolean(
-            this.state.error || !validAddress || !this.state.newName.trim(),
+            this.state.addressInputError ||
+              this.state.nameInputError ||
+              !validAddress ||
+              !this.state.newName.trim(),
           )}
           onSubmit={async () => {
             await addToAddressBook(newAddress, this.state.newName);

ui/pages/settings/contact-list-tab/add-contact/add-contact.container.js

@@ -12,10 +12,13 @@ import {
   getDomainResolutions,
   resetDomainResolution,
 } from '../../../../ducks/domains';
+import { getAddressBook, getInternalAccounts } from '../../../../selectors';
 import AddContact from './add-contact.component';
 
 const mapStateToProps = (state) => {
   return {
+    addressBook: getAddressBook(state),
+    internalAccounts: getInternalAccounts(state),
     qrCodeData: getQrCodeData(state),
     domainError: getDomainError(state),
     domainResolutions: getDomainResolutions(state),

ui/pages/settings/contact-list-tab/add-contact/add-contact.test.js

@@ -6,6 +6,12 @@ import { renderWithProvider } from '../../../../../test/lib/render-helpers';
 import '@testing-library/jest-dom/extend-expect';
 import { mockNetworkState } from '../../../../../test/stub/networks';
 import { CHAIN_IDS } from '../../../../../shared/constants/network';
+import { createMockInternalAccount } from '../../../../../test/jest/mocks';
+import {
+  MOCK_ADDRESS_BOOK,
+  MOCK_DOMAIN_RESOLUTION,
+} from '../../../../../test/data/mock-data';
+import * as domainDucks from '../../../../ducks/domains';
 import AddContact from './add-contact.component';
 
 describe('AddContact component', () => {
@@ -16,16 +22,29 @@ describe('AddContact component', () => {
     },
   };
   const props = {
+    addressBook: MOCK_ADDRESS_BOOK,
+    internalAccounts: [createMockInternalAccount()],
     history: { push: jest.fn() },
     addToAddressBook: jest.fn(),
     scanQrCode: jest.fn(),
     qrCodeData: { type: 'address', values: { address: '0x123456789abcdef' } },
     qrCodeDetected: jest.fn(),
-    domainResolution: '',
+    domainResolutions: [MOCK_DOMAIN_RESOLUTION],
     domainError: '',
     resetDomainResolution: jest.fn(),
   };
 
+  beforeEach(() => {
+    jest.resetAllMocks();
+    jest
+      .spyOn(domainDucks, 'lookupDomainName')
+      .mockImplementation(() => jest.fn());
+  });
+
+  afterAll(() => {
+    jest.restoreAllMocks();
+  });
+
   it('should render the component with correct properties', () => {
     const store = configureMockStore(middleware)(state);
 
@@ -102,4 +121,66 @@ describe('AddContact component', () => {
     });
     expect(getByText('Save')).toBeDisabled();
   });
+
+  it('should disable the submit button when the name is already in use', () => {
+    const duplicateName = 'Account 1';
+
+    const store = configureMockStore(middleware)(state);
+    const { getByText, getByTestId } = renderWithProvider(
+      <AddContact {...props} />,
+      store,
+    );
+
+    const nameInput = document.getElementById('nickname');
+    fireEvent.change(nameInput, { target: { value: duplicateName } });
+
+    const addressInput = getByTestId('ens-input');
+
+    fireEvent.change(addressInput, {
+      target: { value: '0x43c9159B6251f3E205B9113A023C8256cDD40D91' },
+    });
+
+    const saveButton = getByText('Save');
+    expect(saveButton).toBeDisabled();
+  });
+
+  it('should display error message when name entered is already in use', () => {
+    const duplicateName = 'Account 1';
+
+    const store = configureMockStore(middleware)(state);
+
+    const { getByText } = renderWithProvider(<AddContact {...props} />, store);
+
+    const nameInput = document.getElementById('nickname');
+
+    fireEvent.change(nameInput, { target: { value: duplicateName } });
+
+    const saveButton = getByText('Save');
+
+    expect(getByText('Name is already in use')).toBeDefined();
+    expect(saveButton).toBeDisabled();
+  });
+
+  it('should display error when ENS inserts a name that is already in use', () => {
+    const store = configureMockStore(middleware)(state);
+
+    const { getByTestId, getByText } = renderWithProvider(
+      <AddContact {...props} />,
+      store,
+    );
+
+    const ensInput = getByTestId('ens-input');
+    fireEvent.change(ensInput, { target: { value: 'example.eth' } });
+
+    const domainResolutionCell = getByTestId(
+      'multichain-send-page__recipient__item',
+    );
+
+    fireEvent.click(domainResolutionCell);
+
+    const saveButton = getByText('Save');
+
+    expect(getByText('Name is already in use')).toBeDefined();
+    expect(saveButton).toBeDisabled();
+  });
 });

ui/pages/settings/contact-list-tab/contact-list-tab.component.js

@@ -66,6 +66,7 @@ export default class ContactListTab extends Component {
       return (
         <div>
           <ContactList
+            addressBook={addressBook}
             searchForContacts={() => contacts}
             searchForRecents={() => nonContacts}
             selectRecipient={(address) => {