Skip to content

Commit

Permalink
refactor: Decouple the data deletion controller
Browse files Browse the repository at this point in the history
The `DataDeletionController` has been decoupled from the
`MetaMetricsController` by updating the constructor parameters to
expect just a function for getting the MetaMetrics ID, rather than the
entire `MetaMetricsController` state. This vastly simplifies the API
surface, making it easier to audit and test. It also prevents the
`DataDeletionController` from having the capability to make changes to
the `MetaMetricsController` state.

This is an improvement for #24503
  • Loading branch information
Gudahtt committed May 29, 2024
1 parent d15ca11 commit 3e61f0b
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import {
BaseController,
RestrictedControllerMessenger,
} from '@metamask/base-controller';
import { ObservableStore } from '@metamask/obs-store';
import { MetaMetricsControllerState } from '../metametrics';
import type { DataDeletionService } from '../../services/data-deletion-service';

// Unique name for the controller
Expand Down Expand Up @@ -104,10 +102,10 @@ export default class MetaMetricsDataDeletionController extends BaseController<
MetaMetricsDataDeletionState,
MetaMetricsDataDeletionControllerMessenger
> {
private metaMetricsId;

#dataDeletionService: DataDeletionService;

#getMetaMetricsId: () => string

/**
* Creates a MetaMetricsDataDeletionController instance.
*
Expand All @@ -121,12 +119,12 @@ export default class MetaMetricsDataDeletionController extends BaseController<
dataDeletionService,
messenger,
state,
metaMetricsStore,
getMetaMetricsId,
}: {
dataDeletionService: DataDeletionService;
messenger: MetaMetricsDataDeletionControllerMessenger;
state?: MetaMetricsDataDeletionState;
metaMetricsStore: ObservableStore<MetaMetricsControllerState>;
getMetaMetricsId: () => string;
}) {
// Call the constructor of BaseControllerV2
super({
Expand All @@ -135,7 +133,7 @@ export default class MetaMetricsDataDeletionController extends BaseController<
name: controllerName,
state: { ...defaultState, ...state },
});
this.metaMetricsId = metaMetricsStore.getState().metaMetricsId;
this.#getMetaMetricsId = getMetaMetricsId;
this.#dataDeletionService = dataDeletionService;
}

Expand All @@ -154,13 +152,14 @@ export default class MetaMetricsDataDeletionController extends BaseController<
*
*/
async createMetaMetricsDataDeletionTask(): Promise<void> {
if (!this.metaMetricsId) {
const metaMetricsId = this.#getMetaMetricsId();
if (!metaMetricsId) {
throw new Error('MetaMetrics ID not found');
}

const { data } =
await this.#dataDeletionService.createDataDeletionRegulationTask(
this.metaMetricsId,
metaMetricsId,
);
this.update((state) => {
state.metaMetricsDataDeletionId = data?.regulateId;
Expand Down
2 changes: 1 addition & 1 deletion app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ export default class MetamaskController extends EventEmitter {
dataDeletionService,
messenger: metaMetricsDataDeletionMessenger,
state: initState.metaMetricsDataDeletionController,
metaMetricsStore: this.metaMetricsController.store,
getMetaMetricsId: () => this.metaMetricsController.getMetaMetricsId(),
});

const gasFeeMessenger = this.controllerMessenger.getRestricted({
Expand Down

0 comments on commit 3e61f0b

Please sign in to comment.