fix: contact names should not allow duplication

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

@@ -2,10 +2,14 @@ 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';
+import { hasDuplicateContacts, buildDuplicateContactMap } from './utils';
 
 export default class ContactList extends PureComponent {
   static propTypes = {
+    addressBook: PropTypes.array,
+    internalAccounts: PropTypes.array,
     searchForContacts: PropTypes.func,
     searchForRecents: PropTypes.func,
     searchForMyAccounts: PropTypes.func,
@@ -22,6 +26,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;
@@ -45,15 +62,40 @@ export default class ContactList extends PureComponent {
   }
 
   renderAddressBook() {
-    const unsortedContactsByLetter = this.props
-      .searchForContacts()
-      .reduce((obj, contact) => {
+    const {
+      addressBook,
+      internalAccounts,
+      searchForContacts,
+      selectRecipient,
+      selectedAddress,
+    } = this.props;
+
+    const duplicateContactMap = buildDuplicateContactMap(
+      addressBook,
+      internalAccounts,
+    );
+
+    const unsortedContactsByLetter = searchForContacts().reduce(
+      (obj, contact) => {
         const firstLetter = contact.name[0].toUpperCase();
+
+        const isDuplicate =
+          (duplicateContactMap.get(contact.name.trim().toLowerCase()) ?? [])
+            .length > 1;
+
         return {
           ...obj,
-          [firstLetter]: [...(obj[firstLetter] || []), contact],
+          [firstLetter]: [
+            ...(obj[firstLetter] || []),
+            {
+              ...contact,
+              isDuplicate,
+            },
+          ],
         };
-      }, {});
+      },
+      {},
+    );
 
     const letters = Object.keys(unsortedContactsByLetter).sort();
 
@@ -71,8 +113,8 @@ export default class ContactList extends PureComponent {
         key={`${letter}-contact-group`}
         label={letter}
         items={groupItems}
-        onSelect={this.props.selectRecipient}
-        selectedAddress={this.props.selectedAddress}
+        onSelect={selectRecipient}
+        selectedAddress={selectedAddress}
       />
     ));
   }
@@ -95,11 +137,16 @@ export default class ContactList extends PureComponent {
       searchForRecents,
       searchForContacts,
       searchForMyAccounts,
+      addressBook,
+      internalAccounts,
     } = this.props;
 
     return (
       <div className="send__select-recipient-wrapper__list">
         {children || null}
+        {hasDuplicateContacts(addressBook, internalAccounts)
+          ? 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,57 @@
 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 { createMockInternalAccount } from '../../../../test/jest/mocks';
 import ContactList from '.';
 
 describe('Contact List', () => {
   const store = configureMockStore([])({
     metamask: {},
   });
 
+  const mockInternalAccounts = [createMockInternalAccount()];
+
+  it('displays the warning banner when multiple contacts have the same name', () => {
+    const mockAddressBook = [...MOCK_ADDRESS_BOOK, MOCK_ADDRESS_BOOK[0]]; // Adding duplicate contact
+
+    const { getByText } = renderWithProvider(
+      <ContactList
+        addressBook={mockAddressBook}
+        internalAccounts={mockInternalAccounts}
+      />,
+      store,
+    );
+
+    const duplicateContactBanner = getByText('You have duplicate contacts');
+
+    expect(duplicateContactBanner).toBeVisible();
+  });
+
+  it('displays the warning banner when contact has same name as an existing account', () => {
+    const mockContactWithAccountName = {
+      address: '0x2f318C334780961FB129D2a6c30D0763d9a5C970',
+      chainId: '0x1',
+      isEns: false,
+      memo: '',
+      name: mockInternalAccounts[0].metadata.name,
+    };
+
+    const mockAddressBook = [...MOCK_ADDRESS_BOOK, mockContactWithAccountName];
+
+    const { getByText } = renderWithProvider(
+      <ContactList
+        addressBook={mockAddressBook}
+        internalAccounts={mockInternalAccounts}
+      />,
+      store,
+    );
+
+    const duplicateContactBanner = getByText('You have duplicate contacts');
+
+    expect(duplicateContactBanner).toBeVisible();
+  });
+
   describe('given searchForContacts', () => {
     const selectRecipient = () => null;
     const selectedAddress = null;
@@ -37,6 +81,8 @@ describe('Contact List', () => {
           searchForContacts={() => contacts}
           selectRecipient={selectRecipient}
           selectedAddress={selectedAddress}
+          addressBook={MOCK_ADDRESS_BOOK}
+          internalAccounts={mockInternalAccounts}
         />,
         store,
       );

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

@@ -7,12 +7,13 @@ export default function RecipientGroup({ items, onSelect }) {
     return null;
   }
 
-  return items.map(({ address, name }) => (
+  return items.map(({ address, name, isDuplicate }) => (
     <AddressListItem
       address={address}
       label={name}
       onClick={() => onSelect(address, name)}
       key={address}
+      isDuplicate={isDuplicate}
     />
   ));
 }

ui/components/app/contact-list/utils.ts

@@ -0,0 +1,61 @@
+import { AddressBookEntry } from '@metamask/address-book-controller';
+import { InternalAccount } from '@metamask/keyring-api';
+
+export const buildDuplicateContactMap = (
+  addressBook: AddressBookEntry[],
+  internalAccounts: InternalAccount[],
+) => {
+  const contactMap = new Map<string, string[]>(
+    internalAccounts.map((account) => [
+      account.metadata.name.trim().toLowerCase(),
+      [`account-id-${account.id}`],
+    ]),
+  );
+
+  addressBook.forEach((entry) => {
+    const { name, address } = entry;
+
+    const sanitizedName = name.trim().toLowerCase();
+
+    const currentArray = contactMap.get(sanitizedName) ?? [];
+    currentArray.push(address);
+
+    contactMap.set(sanitizedName, currentArray);
+  });
+
+  return contactMap;
+};
+
+export const hasDuplicateContacts = (
+  addressBook: AddressBookEntry[],
+  internalAccounts: InternalAccount[],
+) => {
+  const uniqueContactNames = Array.from(
+    new Set(addressBook.map(({ name }) => name.toLowerCase().trim())),
+  );
+
+  const hasAccountNameCollision = internalAccounts.some((account) =>
+    uniqueContactNames.includes(account.metadata.name.toLowerCase().trim()),
+  );
+
+  return (
+    uniqueContactNames.length !== addressBook.length || hasAccountNameCollision
+  );
+};
+
+export const isDuplicateContact = (
+  addressBook: AddressBookEntry[],
+  internalAccounts: InternalAccount[],
+  newName: string,
+) => {
+  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;
+};