diff --git a/packages/account-tree-controller/CHANGELOG.md b/packages/account-tree-controller/CHANGELOG.md index f78a2f04c5..7890f7886b 100644 --- a/packages/account-tree-controller/CHANGELOG.md +++ b/packages/account-tree-controller/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/accounts-controller` from `^39.0.7` to `^39.1.0` ([#9807](https://github.com/MetaMask/core/pull/9807)) +### Removed + +- **BREAKING:** Remove `AccountTreeControllerConfig.accountOrderCallbacks` and stop migrating pinned/hidden state from account-order callbacks + ## [7.6.1] ### Changed diff --git a/packages/account-tree-controller/src/AccountTreeController.test.ts b/packages/account-tree-controller/src/AccountTreeController.test.ts index 8ec81ea7d9..fd9a29eb08 100644 --- a/packages/account-tree-controller/src/AccountTreeController.test.ts +++ b/packages/account-tree-controller/src/AccountTreeController.test.ts @@ -289,9 +289,6 @@ const MOCK_PREPOPULATED_STATE: Partial = { * @param options.config.backupAndSync.onBackupAndSyncEvent - Event handler for backup and sync events. * @param options.config.backupAndSync.isAccountSyncingEnabled - Flag to enable account syncing. * @param options.config.backupAndSync.isBackupAndSyncEnabled - Flag to enable backup and sync. - * @param options.config.accountOrderCallbacks - Callbacks to migrate hidden and pinned account information from the account order controller. - * @param options.config.accountOrderCallbacks.isHiddenAccount - Callback to check if an account is hidden. - * @param options.config.accountOrderCallbacks.isPinnedAccount - Callback to check if an account is pinned. * @returns An object containing the controller instance and the messenger. */ function setup({ @@ -305,10 +302,6 @@ function setup({ isBackupAndSyncEnabled: true, onBackupAndSyncEvent: jest.fn(), }, - accountOrderCallbacks: { - isHiddenAccount: jest.fn().mockReturnValue(false), - isPinnedAccount: jest.fn().mockReturnValue(false), - }, }, }: { state?: Partial; @@ -323,10 +316,6 @@ function setup({ event: BackupAndSyncAnalyticsEventPayload, ) => void; }; - accountOrderCallbacks?: { - isHiddenAccount?: (accountId: AccountId) => boolean; - isPinnedAccount?: (accountId: AccountId) => boolean; - }; }; } = {}): { controller: AccountTreeController; @@ -6137,256 +6126,82 @@ describe('AccountTreeController', () => { }); }); - describe('migrating account order callbacks', () => { + describe('account group pinned and hidden metadata', () => { const mockAccount1 = { ...MOCK_HD_ACCOUNT_1, id: 'test-account-1' as AccountId, address: '0x123', }; - describe('basic functionality', () => { - it('initializes without callbacks and use default metadata values', () => { - const { controller } = setup({ - accounts: [mockAccount1], - config: { - backupAndSync: { - isAccountSyncingEnabled: true, - isBackupAndSyncEnabled: true, - onBackupAndSyncEvent: jest.fn(), - }, - // No accountOrderCallbacks provided - }, - }); - - controller.init(); - - const wallets = Object.values(controller.state.accountTree.wallets); - expect(wallets).toHaveLength(1); - - const groups = Object.values(wallets[0].groups); - expect(groups).toHaveLength(1); - expect(groups[0].accounts).toContain(mockAccount1.id); - expect(groups[0].metadata.pinned).toBe(false); - expect(groups[0].metadata.hidden).toBe(false); - - // Verify that metadata was persisted with default values - const groupId = groups[0].id; - expect(controller.state.accountGroupsMetadata[groupId]).toStrictEqual({ - name: { - value: expect.any(String), - lastUpdatedAt: expect.any(Number), - }, - pinned: { - value: false, - lastUpdatedAt: 0, - }, - hidden: { - value: false, - lastUpdatedAt: 0, - }, - lastSelected: 0, - }); - }); - - it('handles only pinned callback provided', () => { - const mockCallbacks = { - isPinnedAccount: jest.fn().mockReturnValue(true), - }; - - const { controller } = setup({ - accounts: [mockAccount1], - config: { - backupAndSync: { - isAccountSyncingEnabled: true, - isBackupAndSyncEnabled: true, - onBackupAndSyncEvent: jest.fn(), - }, - accountOrderCallbacks: mockCallbacks, - }, - }); - - controller.init(); - - const wallets = Object.values(controller.state.accountTree.wallets); - expect(wallets).toHaveLength(1); - - const groups = Object.values(wallets[0].groups); - expect(groups).toHaveLength(1); - expect(groups[0].accounts).toContain(mockAccount1.id); - expect(groups[0].metadata.pinned).toBe(true); - expect(groups[0].metadata.hidden).toBe(false); - expect(mockCallbacks.isPinnedAccount).toHaveBeenCalledWith( - mockAccount1.id, - ); - - // Verify that metadata was persisted correctly - const groupId = groups[0].id; - expect(controller.state.accountGroupsMetadata[groupId]).toStrictEqual({ - name: { - value: expect.any(String), - lastUpdatedAt: expect.any(Number), - }, - pinned: { - value: true, - lastUpdatedAt: 0, - }, - hidden: { - value: false, - lastUpdatedAt: 0, - }, - lastSelected: 0, - }); + it('initializes with default pinned and hidden metadata values', () => { + const { controller } = setup({ + accounts: [mockAccount1], }); - it('handles only hidden callback provided', () => { - const mockCallbacks = { - isHiddenAccount: jest.fn().mockReturnValue(true), - }; - - const { controller } = setup({ - accounts: [mockAccount1], - config: { - backupAndSync: { - isAccountSyncingEnabled: true, - isBackupAndSyncEnabled: true, - onBackupAndSyncEvent: jest.fn(), - }, - accountOrderCallbacks: mockCallbacks, - }, - }); - - controller.init(); + controller.init(); - const wallets = Object.values(controller.state.accountTree.wallets); - expect(wallets).toHaveLength(1); + const wallets = Object.values(controller.state.accountTree.wallets); + expect(wallets).toHaveLength(1); - const groups = Object.values(wallets[0].groups); - expect(groups).toHaveLength(1); - expect(groups[0].accounts).toContain(mockAccount1.id); - expect(groups[0].metadata.pinned).toBe(false); - expect(groups[0].metadata.hidden).toBe(true); - expect(mockCallbacks.isHiddenAccount).toHaveBeenCalledWith( - mockAccount1.id, - ); + const groups = Object.values(wallets[0].groups); + expect(groups).toHaveLength(1); + expect(groups[0].accounts).toContain(mockAccount1.id); + expect(groups[0].metadata.pinned).toBe(false); + expect(groups[0].metadata.hidden).toBe(false); - // Verify that metadata was persisted correctly - const groupId = groups[0].id; - expect(controller.state.accountGroupsMetadata[groupId]).toStrictEqual({ - name: { - value: expect.any(String), - lastUpdatedAt: expect.any(Number), - }, - pinned: { - value: false, - lastUpdatedAt: 0, - }, - hidden: { - value: true, - lastUpdatedAt: 0, - }, - lastSelected: 0, - }); + const groupId = groups[0].id; + expect(controller.state.accountGroupsMetadata[groupId]).toStrictEqual({ + name: { + value: expect.any(String), + lastUpdatedAt: expect.any(Number), + }, + pinned: { + value: false, + lastUpdatedAt: 0, + }, + hidden: { + value: false, + lastUpdatedAt: 0, + }, + lastSelected: 0, }); + }); - it('prefers persisted metadata over callbacks', () => { - const mockIsHiddenAccount = jest.fn().mockReturnValue(true); - const mockIsPinnedAccount = jest.fn().mockReturnValue(true); - - const walletId = toMultichainAccountWalletId( - mockAccount1.options.entropy.id, - ); - const groupId = toMultichainAccountGroupId( - walletId, - mockAccount1.options.entropy.groupIndex, - ); + it('uses persisted pinned and hidden metadata', () => { + const walletId = toMultichainAccountWalletId( + mockAccount1.options.entropy.id, + ); + const groupId = toMultichainAccountGroupId( + walletId, + mockAccount1.options.entropy.groupIndex, + ); - const { controller } = setup({ - accounts: [mockAccount1], - keyrings: [MOCK_HD_KEYRING_1], - state: { - accountGroupsMetadata: { - [groupId]: { - pinned: { - value: false, - lastUpdatedAt: Date.now(), - }, - hidden: { - value: false, - lastUpdatedAt: Date.now(), - }, + const { controller } = setup({ + accounts: [mockAccount1], + keyrings: [MOCK_HD_KEYRING_1], + state: { + accountGroupsMetadata: { + [groupId]: { + pinned: { + value: true, + lastUpdatedAt: Date.now(), }, - }, - }, - config: { - backupAndSync: { - isAccountSyncingEnabled: true, - isBackupAndSyncEnabled: true, - onBackupAndSyncEvent: jest.fn(), - }, - accountOrderCallbacks: { - isHiddenAccount: mockIsHiddenAccount, - isPinnedAccount: mockIsPinnedAccount, - }, - }, - }); - - controller.init(); - - // Verify callbacks were NOT called because persisted metadata takes precedence - expect(mockIsHiddenAccount).not.toHaveBeenCalled(); - expect(mockIsPinnedAccount).not.toHaveBeenCalled(); - - const wallets = Object.values(controller.state.accountTree.wallets); - const groups = Object.values(wallets[0].groups); - expect(groups[0].accounts).toContain(mockAccount1.id); - expect(groups[0].metadata.pinned).toBe(false); // Persisted value used - expect(groups[0].metadata.hidden).toBe(false); // Persisted value used - }); - - it('uses persisted metadata when no callbacks are provided', () => { - const walletId = toMultichainAccountWalletId( - mockAccount1.options.entropy.id, - ); - const groupId = toMultichainAccountGroupId( - walletId, - mockAccount1.options.entropy.groupIndex, - ); - - const { controller } = setup({ - accounts: [mockAccount1], - keyrings: [MOCK_HD_KEYRING_1], - state: { - accountGroupsMetadata: { - [groupId]: { - pinned: { - value: true, // Persisted as pinned - lastUpdatedAt: Date.now(), - }, - hidden: { - value: true, // Persisted as hidden - lastUpdatedAt: Date.now(), - }, + hidden: { + value: true, + lastUpdatedAt: Date.now(), }, }, }, - config: { - backupAndSync: { - isAccountSyncingEnabled: true, - isBackupAndSyncEnabled: true, - onBackupAndSyncEvent: jest.fn(), - }, - // No accountOrderCallbacks provided - }, - }); + }, + }); - controller.init(); + controller.init(); - const wallets = Object.values(controller.state.accountTree.wallets); - const groups = Object.values(wallets[0].groups); - expect(groups[0].accounts).toContain(mockAccount1.id); - expect(groups[0].metadata.pinned).toBe(true); - expect(groups[0].metadata.hidden).toBe(true); - }); + const wallets = Object.values(controller.state.accountTree.wallets); + const groups = Object.values(wallets[0].groups); + expect(groups[0].accounts).toContain(mockAccount1.id); + expect(groups[0].metadata.pinned).toBe(true); + expect(groups[0].metadata.hidden).toBe(true); }); }); }); diff --git a/packages/account-tree-controller/src/AccountTreeController.ts b/packages/account-tree-controller/src/AccountTreeController.ts index f2460bb3fc..ba0ff30b15 100644 --- a/packages/account-tree-controller/src/AccountTreeController.ts +++ b/packages/account-tree-controller/src/AccountTreeController.ts @@ -165,13 +165,6 @@ export class AccountTreeController extends BaseController< readonly #backupAndSyncConfig: AccountTreeControllerInternalBackupAndSyncConfig; - /** - * Callbacks to migrate hidden and pinned account information from the account order controller - */ - readonly #accountOrderCallbacks: - | AccountTreeControllerConfig['accountOrderCallbacks'] - | undefined; - #initialized: boolean; /** @@ -233,9 +226,6 @@ export class AccountTreeController extends BaseController< }, }; - // Used when migrating initial hidden/pinned state for groups (if available). - this.#accountOrderCallbacks = config?.accountOrderCallbacks; - // Initialize the backup and sync service this.#backupAndSyncService = new BackupAndSyncService( this.#createBackupAndSyncContext(), @@ -677,9 +667,7 @@ export class AccountTreeController extends BaseController< /** * Applies group metadata updates (name, pinned, hidden flags) by checking - * the persistent state first, and then fallbacks to default values (based - * on the wallet's - * type). + * the persistent state first, and then falling back to default values. * * @param state Controller state to update for persistence. * @param walletId The wallet ID containing the group. @@ -748,37 +736,21 @@ export class AccountTreeController extends BaseController< if (persistedGroupMetadata?.pinned?.value !== undefined) { group.metadata.pinned = persistedGroupMetadata.pinned.value; } else { - let isPinned = false; - - if (this.#accountOrderCallbacks?.isPinnedAccount) { - isPinned = group.accounts.some((account) => - this.#accountOrderCallbacks?.isPinnedAccount?.(account), - ); - } state.accountGroupsMetadata[groupId].pinned = { - value: isPinned, + value: false, lastUpdatedAt: 0, }; - // If any accounts was previously pinned, then we consider the group to be pinned as well. - group.metadata.pinned = isPinned; + group.metadata.pinned = false; } if (persistedGroupMetadata?.hidden?.value !== undefined) { group.metadata.hidden = persistedGroupMetadata.hidden.value; } else { - let isHidden = false; - - if (this.#accountOrderCallbacks?.isHiddenAccount) { - isHidden = group.accounts.some((account) => - this.#accountOrderCallbacks?.isHiddenAccount?.(account), - ); - } state.accountGroupsMetadata[groupId].hidden = { - value: isHidden, + value: false, lastUpdatedAt: 0, }; - // If any accounts was previously hidden, then we consider the group to be hidden as well. - group.metadata.hidden = isHidden; + group.metadata.hidden = false; } // Apply persisted lastSelected (plain number, not synced). diff --git a/packages/account-tree-controller/src/types.ts b/packages/account-tree-controller/src/types.ts index 4fff1d42f6..dd020f3206 100644 --- a/packages/account-tree-controller/src/types.ts +++ b/packages/account-tree-controller/src/types.ts @@ -1,6 +1,5 @@ import type { AccountGroupId, AccountWalletId } from '@metamask/account-api'; import type { - AccountId, AccountsControllerAccountsAddedEvent, AccountsControllerAccountsRemovedEvent, AccountsControllerGetAccountAction, @@ -179,10 +178,6 @@ export type AccountTreeControllerConfig = { backupAndSync?: { onBackupAndSyncEvent?: (event: BackupAndSyncAnalyticsEventPayload) => void; }; - accountOrderCallbacks?: { - isHiddenAccount?: (accountId: AccountId) => boolean; - isPinnedAccount?: (accountId: AccountId) => boolean; - }; }; export type AccountTreeControllerInternalBackupAndSyncConfig = {