Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Bug]: Contact names should not allow duplication #26621

Open
mirceanis opened this issue Aug 22, 2024 · 0 comments
Open

[Bug]: Contact names should not allow duplication #26621

mirceanis opened this issue Aug 22, 2024 · 0 comments
Assignees
@Matt561
Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. Sev2-normal Normal severity; minor loss of service or inconvenience. team-identity type-bug Something isn't working

Comments

@mirceanis
Copy link
Contributor

mirceanis commented Aug 22, 2024 •
edited
Loading

Describe the bug:

When adding a new contact, there is no verification is the name collides with an existing contact, nor with existing local accounts:

Expected behavior

Adding a new contact with the same name as an existing contact, or with one of the reserved account names should be disallowed, unless the address also matches.

Screenshots/Recordings

Image

Steps to reproduce

  1. go to Settings / Contacts
  2. click Add Contact
  3. use the name of an existing contact but a different address
  4. observe that there is no error when you click Save
  5. observe that 2 contacts appear in your address book with the same name

Environment

Browser: Brave (but probably all)
Build type: production
Version: 12 (but the issue is older)
@metamaskbot metamaskbot added the INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. label Aug 22, 2024
@mirceanis mirceanis changed the title [BUG] Contact names should not allow duplication [Bug]: Contact names should not allow duplication Aug 22, 2024
@mirceanis mirceanis added the type-bug Something isn't working label Aug 22, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Aug 22, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Aug 22, 2024
@gauthierpetetin gauthierpetetin added the Sev2-normal Normal severity; minor loss of service or inconvenience. label Aug 26, 2024
@Matt561 Matt561 mentioned this issue Aug 27, 2024
Open
9 tasks
@Matt561 Matt561 self-assigned this Oct 30, 2024
@Matt561 Matt561 mentioned this issue Nov 1, 2024
Merged
7 tasks
github-merge-queue bot pushed a commit that referenced this issue Nov 7, 2024
@Matt561
fix: contact names should not allow duplication (#28249)
5c47f61 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
This fix prevents users from having duplicate names in their contact
list. A warning banner is now displayed in the contact list when
duplicates are found.

#### What is considered a duplicate contact?
A duplicate is a contact with a different address but same name as
another contact or account.

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28249?quickstart=1)

## **Related issues**

Fixes: 
- #26621
- #26696

## **Manual testing steps**

#### Adding a duplicate contact
1. Go to Settings > Contacts page
2. Attempt to add a new contact using an existing contact or account
name
3. An error message will appear underneath the `username` input and the
submit button will be disabled.

#### Viewing duplicate contact warning banner
1. In your local environment, go to file
`ui/components/app/contact-list/contact-list.component.js`
2. On line `138` (`{hasDuplicateContacts ?
this.renderDuplicateContactWarning() : null}` change
`hasDuplicateContacts` to `true` to display the banner.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

https://github.com/user-attachments/assets/a7813050-e906-4d42-ac10-db05401d06d3


### **After**

<!-- [screenshots/recordings] -->

https://github.com/user-attachments/assets/70e16e99-60ea-4404-9ea7-6f776f15d00e


## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
github-merge-queue bot pushed a commit that referenced this issue Nov 20, 2024
@Matt561
fix: contact names should not allow duplication (#28249)
f3cec69 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
This fix prevents users from having duplicate names in their contact
list. A warning banner is now displayed in the contact list when
duplicates are found. All duplicate contacts will also have a warning
icon next to them to help users better identify duplicates.

#### What is considered a duplicate contact?
A duplicate is a contact with a different address but same name as
another contact or account.

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28249?quickstart=1)

## **Related issues**

Fixes: 
- #26621
- #26696

## **Manual testing steps**

#### Adding a duplicate contact
1. Go to Settings > Contacts page
2. Attempt to add a new contact using an existing contact or account
name
3. An error message will appear underneath the `username` input and the
submit button will be disabled.

#### Viewing duplicate contact warning banner
1. In your local environment, go to file
`ui/components/app/contact-list/contact-list.component.js`
2. On line `138` (`{hasDuplicateContacts ?
this.renderDuplicateContactWarning() : null}` change
`hasDuplicateContacts` to `true` to display the banner.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

https://github.com/user-attachments/assets/a7813050-e906-4d42-ac10-db05401d06d3


### **After**

<!-- [screenshots/recordings] -->

https://github.com/user-attachments/assets/70e16e99-60ea-4404-9ea7-6f776f15d00e

<img width="355" alt="Screenshot 2024-11-15 at 1 59 21 PM"
src="https://github.com/user-attachments/assets/4815dae3-220d-484c-bae2-393b9b7c0631">

<img width="355" alt="Screenshot 2024-11-15 at 1 59 28 PM"
src="https://github.com/user-attachments/assets/971cb62d-1891-472d-8506-3a9695599f63">


## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Matt561 Matt561

Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. Sev2-normal Normal severity; minor loss of service or inconvenience. team-identity type-bug Something isn't working
Projects
Bugs by severity
Status: To be fixed
Bugs by team
Status: To be fixed
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mirceanis @gauthierpetetin @metamaskbot @Matt561