fix(26748): increment index label until get a label not duplicated #26959

Akaryatrh · 2024-09-06T08:00:35Z

Description

In the context of importing hardware wallet accounts, this PR prevents account name to be labelled the same as an already created account and will increment its index until getting a label that is not duplicated.

Also, I removed preferencesController.setAccountLabel call as it was also calling accountController.setAccountName under the hood (through messagingSystem). This was causing an uncaught error and it was duplicating the call to setAccountName anyway.

Related issues

Fixes: #26748

Manual testing steps

remove all existing ledger accounts
add new MM accounts, named "Ledger 1" and "Ledger 2"
add new hardware account, select Ledger and and select first account
See that imported account is named "Ledger 3"

Screenshots/Recordings

Before

Enregistrement.de.l.ecran.2024-08-30.a.16.32.49.mp4

After

Enregistrement.de.l.ecran.2024-09-09.a.10.53.01.mp4

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). 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-actions · 2024-09-06T08:00:50Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

metamaskbot · 2024-09-06T09:00:40Z

Builds ready [de58189]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1897 ± 93 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1572	2412	1906	207	99
		domContentLoaded	1560	2193	1872	179	86
		load	1568	2316	1897	193	93
		domInteractive	16	88	42	21	10

Bundle size diffs [🚀 Bundle size reduced!]

background: -5 Bytes (-0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

codecov · 2024-09-06T09:00:57Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.17%. Comparing base (416d024) to head (4b90b44).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #26959   +/-   ##
========================================
  Coverage    70.17%   70.17%           
========================================
  Files         1425     1425           
  Lines        49659    49663    +4     
  Branches     13891    13891           
========================================
+ Hits         34846    34850    +4     
  Misses       14813    14813

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sonarcloud · 2024-09-09T08:51:20Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

metamaskbot · 2024-09-09T09:19:29Z

Builds ready [4b90b44]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1719 ± 87 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	232	2371	1578	482	231
		domContentLoaded	1532	2361	1704	178	85
		load	1542	2373	1719	180	87
		domInteractive	14	74	34	18	9

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 61 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

metamaskbot · 2024-10-08T09:12:04Z

Builds ready [6ae013c]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1823 ± 72 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	309	2199	1665	471	226
		domContentLoaded	1504	2097	1798	138	66
		load	1513	2204	1823	150	72
		domInteractive	17	85	48	23	11

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 61 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

sonarcloud · 2024-10-10T08:37:31Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
90.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

metamaskbot · 2024-10-10T09:00:04Z

Builds ready [10ed013]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2452 ± 99 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	322	2886	1927	896	430
		domContentLoaded	2040	2860	2425	212	102
		load	2099	2866	2452	206	99
		domInteractive	20	132	60	25	12

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 61 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

Akaryatrh marked this pull request as ready for review September 6, 2024 08:30

Akaryatrh requested a review from a team as a code owner September 6, 2024 08:30

Akaryatrh changed the title ~~fix(26748): remove duplicated call to set account name and remove acc…~~ fix(26748): remove duplicated call to set account name and remove account on error Sep 6, 2024

Akaryatrh added the team-hardware-wallets label Sep 6, 2024

Akaryatrh marked this pull request as draft September 9, 2024 07:59

Akaryatrh force-pushed the fix/26748 branch 2 times, most recently from 5832e7b to 1b1f8b0 Compare September 9, 2024 08:44

Akaryatrh changed the title ~~fix(26748): remove duplicated call to set account name and remove account on error~~ fix(26748): increment index label until get a label not duplicated Sep 9, 2024

fix(26748): increment index label until get a label not duplicated

4b90b44

Akaryatrh force-pushed the fix/26748 branch from 1b1f8b0 to 4b90b44 Compare September 9, 2024 08:49

Akaryatrh marked this pull request as ready for review September 9, 2024 09:04

Akaryatrh and others added 3 commits October 7, 2024 10:33

Merge branch 'develop' into fix/26748

36a1e09

Merge branch 'develop' into fix/26748

9d1e7bc

Merge branch 'develop' into fix/26748

6ae013c

Akaryatrh added 2 commits October 9, 2024 10:50

Merge branch 'develop' into fix/26748

08d654e

Merge branch 'develop' into fix/26748

10ed013

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(26748): increment index label until get a label not duplicated #26959

fix(26748): increment index label until get a label not duplicated #26959

Akaryatrh commented Sep 6, 2024 •

edited

Loading

github-actions bot commented Sep 6, 2024

metamaskbot commented Sep 6, 2024

codecov bot commented Sep 6, 2024 •

edited

Loading

sonarcloud bot commented Sep 9, 2024

metamaskbot commented Sep 9, 2024

metamaskbot commented Oct 8, 2024

sonarcloud bot commented Oct 10, 2024

metamaskbot commented Oct 10, 2024

fix(26748): increment index label until get a label not duplicated #26959

Are you sure you want to change the base?

fix(26748): increment index label until get a label not duplicated #26959

Conversation

Akaryatrh commented Sep 6, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Sep 6, 2024

metamaskbot commented Sep 6, 2024

codecov bot commented Sep 6, 2024 • edited Loading

Codecov Report

sonarcloud bot commented Sep 9, 2024

Quality Gate passed

metamaskbot commented Sep 9, 2024

metamaskbot commented Oct 8, 2024

sonarcloud bot commented Oct 10, 2024

Quality Gate passed

metamaskbot commented Oct 10, 2024

Akaryatrh commented Sep 6, 2024 •

edited

Loading

codecov bot commented Sep 6, 2024 •

edited

Loading