Skip to content

Commit

Permalink
fix: update setSelectedAccount to throw if the id is not found (#4167)
Browse files Browse the repository at this point in the history
## Explanation

This PR updates the logic of setSelectedAccount to reject unknown
account ids.

## References

## Changelog

### `@metamask/accounts-controller`

- **CHANGED**: Reject unknown account ids in setSelectedAccount

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
  • Loading branch information
montelaidev authored Apr 16, 2024
1 parent 7b5e0f8 commit bdc755f
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 53 deletions.
39 changes: 3 additions & 36 deletions packages/accounts-controller/src/AccountsController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1647,6 +1647,7 @@ describe('AccountsController', () => {
});

it('should throw an error for an unknown account ID', () => {
const accountId = 'unknown id';
const accountsController = setupAccountsController({
initialState: {
internalAccounts: {
Expand All @@ -1656,8 +1657,8 @@ describe('AccountsController', () => {
},
});

expect(() => accountsController.getAccountExpect('unknown id')).toThrow(
`Account Id unknown id not found`,
expect(() => accountsController.getAccountExpect(accountId)).toThrow(
`Account Id "${accountId}" not found`,
);
});

Expand Down Expand Up @@ -1726,26 +1727,6 @@ describe('AccountsController', () => {
accountsController.state.internalAccounts.selectedAccount,
).toStrictEqual(mockAccount2.id);
});

it("should set the selected account to '' if the account is not found", () => {
const accountsController = setupAccountsController({
initialState: {
internalAccounts: {
accounts: {
[mockAccount.id]: mockAccount,
[mockAccount2.id]: mockAccount2,
},
selectedAccount: mockAccount.id,
},
},
});

accountsController.setSelectedAccount('unknown');

expect(accountsController.state.internalAccounts.selectedAccount).toBe(
'',
);
});
});

describe('setAccountName', () => {
Expand Down Expand Up @@ -1782,20 +1763,6 @@ describe('AccountsController', () => {
accountsController.setAccountName(mockAccount.id, 'Account 2'),
).toThrow('Account name already exists');
});

it('should throw an error if the account ID is not found', () => {
const accountsController = setupAccountsController({
initialState: {
internalAccounts: {
accounts: { [mockAccount.id]: mockAccount },
selectedAccount: mockAccount.id,
},
},
});
expect(() =>
accountsController.setAccountName('unknown account', 'new name'),
).toThrow(`Account Id unknown account not found`);
});
});

describe('#getNextAccountNumber', () => {
Expand Down
34 changes: 17 additions & 17 deletions packages/accounts-controller/src/AccountsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ export class AccountsController extends BaseController<

const account = this.getAccount(accountId);
if (account === undefined) {
throw new Error(`Account Id ${accountId} not found`);
throw new Error(`Account Id "${accountId}" not found`);
}
return account;
}
Expand Down Expand Up @@ -262,25 +262,18 @@ export class AccountsController extends BaseController<
* @param accountId - The ID of the account to be selected.
*/
setSelectedAccount(accountId: string): void {
const account = this.getAccount(accountId);
const account = this.getAccountExpect(accountId);

this.update((currentState: Draft<AccountsControllerState>) => {
if (account) {
currentState.internalAccounts.accounts[
account.id
].metadata.lastSelected = Date.now();
currentState.internalAccounts.selectedAccount = account.id;
} else {
currentState.internalAccounts.selectedAccount = '';
}
currentState.internalAccounts.accounts[account.id].metadata.lastSelected =
Date.now();
currentState.internalAccounts.selectedAccount = account.id;
});

if (account) {
this.messagingSystem.publish(
'AccountsController:selectedAccountChange',
account,
);
}
this.messagingSystem.publish(
'AccountsController:selectedAccountChange',
account,
);
}

/**
Expand Down Expand Up @@ -617,7 +610,14 @@ export class AccountsController extends BaseController<

// if the accountToSelect is undefined, then there are no accounts
// it mean the keyring was reinitialized.
this.setSelectedAccount(accountToSelect?.id);
if (!accountToSelect) {
this.update((currentState: Draft<AccountsControllerState>) => {
currentState.internalAccounts.selectedAccount = '';
});
return;
}

this.setSelectedAccount(accountToSelect.id);
}
}
}
Expand Down

0 comments on commit bdc755f

Please sign in to comment.