Skip to content

Commit d36a77d

Browse files
authored
feat(account-tree-controller): allow more pattern when extracting highest index from account group names (#6696)
## Explanation Now allows a bit more pattern when extracting account group name index. ## References N/A ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent 7beec79 commit d36a77d

File tree

3 files changed

+98
-3
lines changed

3 files changed

+98
-3
lines changed

packages/account-tree-controller/CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Changed
1111

12-
- Add new group naming for non-HD keyring accounts ([#6679](https://github.com/MetaMask/core/pull/6679))
12+
- Add new group naming for non-HD keyring accounts ([#6679](https://github.com/MetaMask/core/pull/6679)), ([#6696](https://github.com/MetaMask/core/pull/6696))
1313
- Hardware-wallet account groups are now named: "Ledger|Trezor|QR|Lattice|OneKey Account N".
1414
- Private key account groups are now named: "Imported Account N".
1515
- Snap account groups are now named: "Snap Account N".
16-
- Account group names now use natural indexing as a fallback ([#6677](https://github.com/MetaMask/core/pull/6677)), ([#6679](https://github.com/MetaMask/core/pull/6679))
16+
- Account group names now use natural indexing as a fallback ([#6677](https://github.com/MetaMask/core/pull/6677)), ([#6679](https://github.com/MetaMask/core/pull/6679)), ([#6696](https://github.com/MetaMask/core/pull/6696))
1717
- If a user names his accounts without any indexes, we would just use the number of accounts to compute the next available index.
1818

1919
### Fixed

packages/account-tree-controller/src/AccountTreeController.test.ts

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4110,6 +4110,100 @@ describe('AccountTreeController', () => {
41104110
expect(mockGroup2.metadata.name).toBe('Ledger Account 2');
41114111
});
41124112

4113+
it('ignores bad account group name pattern and fallback to natural indexing', () => {
4114+
const { controller, messenger } = setup({
4115+
accounts: [mockAccount1],
4116+
});
4117+
4118+
controller.init();
4119+
4120+
const mockGroup1 = getAccountGroupFromAccount(controller, mockAccount1);
4121+
expect(mockGroup1).toBeDefined();
4122+
4123+
const mockIndex = 90;
4124+
controller.setAccountGroupName(
4125+
mockGroup1.id,
4126+
`Account${mockIndex}`, // No space, so this should fallback to natural indexing
4127+
);
4128+
4129+
// The first account has a non-matching pattern, thus we should fallback to the next
4130+
// natural index.
4131+
messenger.publish('AccountsController:accountAdded', mockAccount2);
4132+
const mockGroup2 = getAccountGroupFromAccount(controller, mockAccount2);
4133+
expect(mockGroup2).toBeDefined();
4134+
expect(mockGroup2.metadata.name).toBe(`Ledger Account 2`); // Natural indexing.
4135+
});
4136+
4137+
it.each([
4138+
['Account', 'account'],
4139+
['Account', 'aCCount'],
4140+
['Account', 'accOunT'],
4141+
[' ', ' '],
4142+
[' ', '\t'],
4143+
[' ', ' \t'],
4144+
[' ', '\t '],
4145+
])(
4146+
'ignores case (case-insensitive) and spaces when extracting highest index: "$0" -> "$1"',
4147+
(toReplace, replaced) => {
4148+
const { controller, messenger } = setup({
4149+
accounts: [mockAccount1],
4150+
});
4151+
4152+
controller.init();
4153+
4154+
const mockGroup1 = getAccountGroupFromAccount(controller, mockAccount1);
4155+
expect(mockGroup1).toBeDefined();
4156+
4157+
const mockIndex = 90;
4158+
controller.setAccountGroupName(
4159+
mockGroup1.id,
4160+
mockGroup1.metadata.name
4161+
.replace(toReplace, replaced)
4162+
.replace('1', `${mockIndex}`), // Use index different than 1.
4163+
);
4164+
4165+
// Even if the account is not strictly named "Ledger Account 90", we should be able
4166+
// to compute the next index from there.
4167+
messenger.publish('AccountsController:accountAdded', mockAccount2);
4168+
const mockGroup2 = getAccountGroupFromAccount(controller, mockAccount2);
4169+
expect(mockGroup2).toBeDefined();
4170+
expect(mockGroup2.metadata.name).toBe(
4171+
`Ledger Account ${mockIndex + 1}`,
4172+
);
4173+
},
4174+
);
4175+
4176+
it.each([' ', ' ', '\t', ' \t'])(
4177+
'extract name indexes and ignore multiple spaces: "%s"',
4178+
(space) => {
4179+
const { controller, messenger } = setup({
4180+
accounts: [mockAccount1],
4181+
});
4182+
4183+
controller.init();
4184+
4185+
const mockGroup1 = getAccountGroupFromAccount(controller, mockAccount1);
4186+
expect(mockGroup1).toBeDefined();
4187+
4188+
const mockIndex = 90;
4189+
controller.setAccountGroupName(
4190+
mockGroup1.id,
4191+
mockGroup1.metadata.name
4192+
.replace(' ', space)
4193+
.replace('1', `${mockIndex}`), // Use index different than 1.
4194+
);
4195+
4196+
// Even if the account is not strictly named "Ledger Account 90", we should be able
4197+
// to compute the next index from there.
4198+
messenger.publish('AccountsController:accountAdded', mockAccount2);
4199+
const mockGroup2 = getAccountGroupFromAccount(controller, mockAccount2);
4200+
expect(mockGroup2).toBeDefined();
4201+
expect(mockGroup2.metadata.name).toBe(
4202+
`Ledger Account ${mockIndex + 1}`,
4203+
);
4204+
},
4205+
);
4206+
41134207
it('uses natural indexing for pre-existing accounts', () => {
41144208
const { controller } = setup({
41154209
accounts: [mockAccount1, mockAccount2, mockAccount3],

packages/account-tree-controller/src/AccountTreeController.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,8 @@ export class AccountTreeController extends BaseController<
452452
continue;
453453
}
454454
// Parse the existing group name to extract the numeric index
455-
const nameMatch = existingGroup.metadata.name.match(/Account (\d+)$/u);
455+
const nameMatch =
456+
existingGroup.metadata.name.match(/account\s+(\d+)$/iu);
456457
if (nameMatch) {
457458
const nameIndex = parseInt(nameMatch[1], 10);
458459
if (nameIndex > highestNameIndex) {

0 commit comments

Comments
 (0)