-
-
Notifications
You must be signed in to change notification settings - Fork 203
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.
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
Upgrade PreferencesController to BaseControllerV2 #3708
Labels
Comments
3 tasks
Gudahtt
added a commit
that referenced
this issue
Jan 4, 2024
The `PreferencesController` has been migrated to `BaseControllerV2`. As part of this migration, the unused `name` state property has also been removed. Closes #3708
3 tasks
Gudahtt
added a commit
that referenced
this issue
Jan 4, 2024
The `PreferencesController` has been migrated to `BaseControllerV2`. As part of this migration, the unused `name` state property has also been removed. Closes #3708
Gudahtt
added a commit
that referenced
this issue
Jan 4, 2024
The `PreferencesController` has been migrated to `BaseControllerV2`. As part of this migration, the unused `name` state property has also been removed. Closes #3708
Gudahtt
added a commit
that referenced
this issue
Jan 4, 2024
## Explanation The `PreferencesController` types are now declared as types rather than interfaces, in accordance with our conventions, and they are now fully documented. The state properties have also been alphabetized so that they are easier to maintain. Additionally, the old "ContactEntry" type has been renamed to "Identity", which is a more accurate name for what it is being used for here. ## References This was an effort to reduce the scope of the BaseControllerV2 migration, tracked by #3708 ## Changelog ### `@metamask/preferences-controller` #### Changed - **BREAKING**: Replace `ContactEntry` interface with `Identity` type - **BREAKING:** Convert `PreferencesState` from an interface to a type ## 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 - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
Gudahtt
added a commit
that referenced
this issue
Jan 5, 2024
The `AccountTrackerController` tests have been updated to remove instances of the `PreferencesController`. Mocks are now used instead of the real controller. It was useful to have a reference in tests to the default `PreferencesController` state, so that has been added as an export. Relates to #3708
3 tasks
Gudahtt
added a commit
that referenced
this issue
Jan 5, 2024
The `AccountTrackerController` tests have been updated to remove instances of the `PreferencesController`. Mocks are now used instead of the real controller. It was useful to have a reference in tests to the default `PreferencesController` state, so that has been added as an export. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 5, 2024
The `AccountTrackerController` tests have been updated to remove instances of the `PreferencesController`. Mocks are now used instead of the real controller. It was useful to have a reference in tests to the default `PreferencesController` state, so I added a `getDefaultState` static method. This was added as a method rather than an export to protect against accidental mutations. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 5, 2024
The `AssetsContractController` tests now use mocks rather than real `PreferencesController` instances. This should make the tests easier to read, and it decouples the `PreferencesController` further from the asset controllers. Relates to #3708
3 tasks
Gudahtt
added a commit
that referenced
this issue
Jan 5, 2024
The `NftController` tests now use mocks instead of real `AssetsContractController` and `PreferencesController` instances. This should make the tests easier to read, and it further decouples them from those controllers. Relates to #3708
3 tasks
Gudahtt
added a commit
that referenced
this issue
Jan 5, 2024
The `NftController` tests now use mocks instead of real `AssetsContractController` and `PreferencesController` instances. This should make the tests easier to read, and it further decouples them from those controllers. Additionally, the `setupController` helper method has been updated to accept a partial set of controller options rather than custom options. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 5, 2024
The `NftController` tests now use mocks instead of real `AssetsContractController` and `PreferencesController` instances. This should make the tests easier to read, and it further decouples them from those controllers. Additionally, the `setupController` helper method has been updated to accept a partial set of controller options rather than custom options. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 5, 2024
The `NftController` tests now use mocks instead of real `AssetsContractController` and `PreferencesController` instances. This should make the tests easier to read, and it further decouples them from those controllers. Additionally, the `setupController` helper method has been updated to accept a partial set of controller options rather than custom options. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 5, 2024
The `NftController` tests now use mocks instead of real `AssetsContractController` and `PreferencesController` instances. This should make the tests easier to read, and it further decouples them from those controllers. Additionally, the `setupController` helper method has been updated to accept a partial set of controller options rather than custom options. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 5, 2024
The `NftController` tests now use mocks instead of real `AssetsContractController` and `PreferencesController` instances. This should make the tests easier to read, and it further decouples them from those controllers. Additionally, the `setupController` helper method has been updated to accept a partial set of controller options rather than custom options. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 5, 2024
The `NftController` tests now use mocks instead of real `AssetsContractController` and `PreferencesController` instances. This should make the tests easier to read, and it further decouples them from those controllers. Additionally, the `setupController` helper method has been updated to accept a partial set of controller options rather than custom options. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 5, 2024
The `NftController` tests now use mocks instead of real `AssetsContractController` and `PreferencesController` instances. This should make the tests easier to read, and it further decouples them from those controllers. Additionally, the `setupController` helper method has been updated to accept a partial set of controller options rather than custom options. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
…ts and add `getDefaultPreferencesState` (#3736) ## Explanation The `AccountTrackerController` tests have been updated to remove instances of the `PreferencesController`. Mocks are now used instead of the real controller. It was useful to have a reference in tests to the default `PreferencesController` state, so I added a `getDefaultPreferencesState` function. This was added as a function to protect against accidental mutations. ## References Relates to #3708 ## Changelog ### `@metamask/preferences-controller` #### Added - Add `getDefaultPreferencesState` function ## 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 - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `AssetsContractController` tests now use mocks rather than real `PreferencesController` instances. This should make the tests easier to read, and it decouples the `PreferencesController` further from the asset controllers. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `NftController` tests now use mocks instead of real `AssetsContractController` and `PreferencesController` instances. This should make the tests easier to read, and it further decouples them from those controllers. Additionally, the `setupController` helper method has been updated to accept a partial set of controller options rather than custom options. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `NftDetectionController` tests have been refactored to use mocks rather than full `AssetsContractController`, `PreferencesController`, and `NftController` references. The `withController` pattern has been introduced as well, to simplify the controller setup and ensure all polling has stopped after each test. A few tests were found to be testing functionality that was internal to the `NftController`, so they have been removed. Coverage for the `NftDetectionController` remains unchanged. Relates to #3708
3 tasks
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `TokenBalancesController` tests have been refactored to use mocks instead of `NetworkController` and `PreferencesController` instances. This should make the tests easier to read, and it decouples the tests further from the other controllers. Relates to #3708
3 tasks
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `TokenDetectionController` tests have been refactored to use mocks instead of real controller instances, and to use the `withController` pattern to simplify controller setup and ensure that polling has stopped after each test. A `getDefaultTokenListState` function has been added to the `TokenListController` to enable us to use that default state in tests. Relates to #3708
3 tasks
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `TokenDetectionController` tests have been refactored to use mocks instead of real controller instances, and to use the `withController` pattern to simplify controller setup and ensure that polling has stopped after each test. A `getDefaultTokenListState` function has been added to the `TokenListController` to enable us to use that default state in tests. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `TokensController` tests have been refactored to use mocks instead of a real `PreferencesController` instance. This should make the tests easier to read, and it will decouple them from the `PreferencesController`. Relates to #3708
3 tasks
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
…ts (#3737) ## Explanation The `AssetsContractController` tests now use mocks rather than real `PreferencesController` instances. This should make the tests easier to read, and it decouples the `PreferencesController` further from the asset controllers. ## References Relates to #3708 ## Changelog No changes ## 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 - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `NftController` tests now use mocks instead of real `AssetsContractController` and `PreferencesController` instances. This should make the tests easier to read, and it further decouples them from those controllers. Additionally, the `setupController` helper method has been updated to accept a partial set of controller options rather than custom options. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
## Explanation The `NftController` tests now use mocks instead of real `AssetsContractController` and `PreferencesController` instances. This should make the tests easier to read, and it further decouples them from those controllers. ## References Relates to #3708 ## Changelog No changes ## 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 - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `TokensController` tests have been refactored to use mocks instead of a real `PreferencesController` instance. This should make the tests easier to read, and it will decouple them from the `PreferencesController`. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
## Explanation The `TokensController` tests have been refactored to use mocks instead of a real `PreferencesController` instance. This should make the tests easier to read, and it will decouple them from the `PreferencesController`. ## References Relates to #3708 ## Changelog No changes ## 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 - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `TokenBalancesController` tests have been refactored to use mocks instead of `NetworkController` and `PreferencesController` instances. This should make the tests easier to read, and it decouples the tests further from the other controllers. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
#3743) ## Explanation The `TokenBalancesController` tests have been refactored to use mocks instead of `NetworkController` and `PreferencesController` instances. This should make the tests easier to read, and it decouples the tests further from the other controllers. ## References Relates to #3708 ## Changelog ### `@metamask/assets-controllers` #### Added - Add `getDefaultTokensState` function to the `TokensController` ## 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 - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `NftDetectionController` tests have been refactored to use mocks rather than full `AssetsContractController`, `PreferencesController`, and `NftController` references. The `withController` pattern has been introduced as well, to simplify the controller setup and ensure all polling has stopped after each test. A few tests were found to be testing functionality that was internal to the `NftController`, so they have been removed. Coverage for the `NftDetectionController` remains unchanged. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `TokenDetectionController` tests have been refactored to use mocks instead of real controller instances, and to use the `withController` pattern to simplify controller setup and ensure that polling has stopped after each test. A `getDefaultTokenListState` function has been added to the `TokenListController` to enable us to use that default state in tests. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
…#3742) ## Explanation The `NftDetectionController` tests have been refactored to use mocks rather than full `AssetsContractController`, `PreferencesController`, and `NftController` references. The `withController` pattern has been introduced as well, to simplify the controller setup and ensure all polling has stopped after each test. A few tests were found to be testing functionality that was internal to the `NftController`, so they have been removed. Coverage for the `NftDetectionController` remains unchanged. ## References Relates to #3708 ## Changelog ### `@metamask/assets-controllers` #### Added - Add `getDefaultNftState` function to the `NftController` ## 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 - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `TokenDetectionController` tests have been refactored to use mocks instead of real controller instances, and to use the `withController` pattern to simplify controller setup and ensure that polling has stopped after each test. A `getDefaultTokenListState` function has been added to the `TokenListController` to enable us to use that default state in tests. Relates to #3708
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
…ts (#3744) ## Explanation The `TokenDetectionController` tests have been refactored to use mocks instead of real controller instances, and to use the `withController` pattern to simplify controller setup and ensure that polling has stopped after each test. A `getDefaultTokenListState` function has been added to the `TokenListController` to enable us to use that default state in tests. One test was added to the `TokensController` tests to prevent a coverage drop (it was previously being tested indirectly via the `TokenDetectionController` tests). ## References Relates to #3708 ## Changelog ### `@metamask/assets-controllers` #### Added - Add `getDefaultTokenListState` function to `TokenListController ## 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 - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `PreferencesController` has been migrated to `BaseControllerV2`. As part of this migration, the unused `name` state property has also been removed. Closes #3708
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `PreferencesController` has been migrated to `BaseControllerV2`. As part of this migration, the unused `name` state property has also been removed. Closes #3708
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `PreferencesController` has been migrated to `BaseControllerV2`. As part of this migration, the unused `name` state property has also been removed. Closes #3708
Gudahtt
added a commit
that referenced
this issue
Jan 8, 2024
The `PreferencesController` has been migrated to `BaseControllerV2`. As part of this migration, the unused `name` state property has also been removed. Closes #3708
Gudahtt
added a commit
that referenced
this issue
Jan 9, 2024
The `PreferencesController` has been migrated to `BaseControllerV2`. As part of this migration, the unused `name` state property has also been removed. Closes #3708
Gudahtt
added a commit
that referenced
this issue
Jan 9, 2024
The `PreferencesController` has been migrated to `BaseControllerV2`. As part of this migration, the unused `name` state property has also been removed. Closes #3708
Gudahtt
added a commit
that referenced
this issue
Jan 9, 2024
## Explanation The `PreferencesController` has been migrated to `BaseControllerV2`. As part of this migration, the unused `name` state property has also been removed. ## References Closes #3708 ## Changelog ### `@metamask/preferences-controller` #### Changed - **BREAKING:** Convert to `BaseControllerV2` - The constructor parameters have changed; rather than accepting an empty "config" parameter and a "state" parameter, there is now just a single object for all constructor arguments. This object has a mandatory `messenger` and an optional `state` property. - Additional type exports have been added for the controller messenger and associated types ## 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 - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
29 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There should not be too many changes necessary to upgrade this controller to BaseControllerV2, as it does not emit any events. Mainly we would just need to upgrade places in which state is updated to use BaseControllerV2's syntax.
The text was updated successfully, but these errors were encountered: