Skip to content

Commit

Permalink
Update NotificationServicesController to accommodate for Snaps Noti…
Browse files Browse the repository at this point in the history
…fications (#4809)

## Explanation

* What is the current state of things and why does it need to change? We
currently have snap types living in the extension, which should exist in
this repo instead. Also, the extension is using two controllers to
handle notifications, the `NotificationController` solely exists for the
purpose of snap notifications. With the revamping of the notifications
system, it is best to house all notifications in one place.
* What is the solution your changes offer and how does it work? Add snap
logic to the `NotificationServicesController`
* Are there any changes whose purpose might not obvious to those
unfamiliar with the domain? The `updateMetamaskNotificationsList`
function will be the gateway for snaps to add notifications to the
controller.

## Changelog

### `@metamask/notification-services-controller`

- **ADDED**: `getNotificationsByType` to grab a list of notifications by
type.
- **ADDED**: `deleteNotificationById` to delete a notification in the
controller's state (to be only used by notifications that live in this
controller which currently is just snaps).
- **CHANGED**: `fetchAndUpdateMetamaskNotifications` to grab snaps from
state before repopulating with a new list of other notifications.
- **CHANGED**: `markNotificaftionsAsRead` to account for snaps
notifications.
- **CHANGED**: `updateMetamaskNotificationsList` to assign a processed
notification to state instead of the originally passed in notification
as the `processSnapNotification` function adds on extra properties to
the raw notification.

## 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
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
  • Loading branch information
hmalik88 authored Oct 21, 2024
1 parent c3ae77e commit d75a971
Show file tree
Hide file tree
Showing 14 changed files with 507 additions and 37 deletions.
1 change: 1 addition & 0 deletions packages/notification-services-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
"@contentful/rich-text-html-renderer": "^16.5.2",
"@metamask/base-controller": "^7.0.1",
"@metamask/controller-utils": "^11.3.0",
"@metamask/utils": "^9.1.0",
"bignumber.js": "^4.1.0",
"firebase": "^10.11.0",
"loglevel": "^1.8.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
import type { UserStorageController } from '@metamask/profile-sync-controller';
import { AuthenticationController } from '@metamask/profile-sync-controller';

import { createMockSnapNotification } from './__fixtures__';
import {
createMockFeatureAnnouncementAPIResult,
createMockFeatureAnnouncementRaw,
Expand All @@ -25,6 +26,7 @@ import {
mockMarkNotificationsAsRead,
} from './__fixtures__/mockServices';
import { waitFor } from './__fixtures__/test-utils';
import { TRIGGER_TYPES } from './constants';
import NotificationServicesController, {
defaultState,
} from './NotificationServicesController';
Expand All @@ -35,7 +37,9 @@ import type {
NotificationServicesPushControllerDisablePushNotifications,
NotificationServicesPushControllerUpdateTriggerPushNotifications,
} from './NotificationServicesController';
import { processFeatureAnnouncement } from './processors';
import { processNotification } from './processors/process-notifications';
import { processSnapNotification } from './processors/process-snap-notifications';
import * as OnChainNotifications from './services/onchain-notifications';
import type { UserStorage } from './types/user-storage/user-storage';
import * as Utils from './utils/utils';
Expand Down Expand Up @@ -472,23 +476,30 @@ describe('metamask-notifications - fetchAndUpdateMetamaskNotifications()', () =>
};
};

it('processes and shows feature announcements and wallet notifications', async () => {
it('processes and shows feature announcements, wallet and snap notifications', async () => {
const {
messenger,
mockFeatureAnnouncementAPIResult,
mockListNotificationsAPIResult,
} = arrangeMocks();

const snapNotification = createMockSnapNotification();
const processedSnapNotification = processSnapNotification(snapNotification);

const controller = new NotificationServicesController({
messenger,
env: { featureAnnouncements: featureAnnouncementsEnv },
state: { ...defaultState, isFeatureAnnouncementsEnabled: true },
state: {
...defaultState,
isFeatureAnnouncementsEnabled: true,
metamaskNotificationsList: [processedSnapNotification],
},
});

const result = await controller.fetchAndUpdateMetamaskNotifications();

// Should have 1 feature announcement and 1 wallet notification
expect(result).toHaveLength(2);
expect(result).toHaveLength(3);
expect(
result.find(
(n) => n.id === mockFeatureAnnouncementAPIResult.items?.[0].fields.id,
Expand All @@ -497,9 +508,10 @@ describe('metamask-notifications - fetchAndUpdateMetamaskNotifications()', () =>
expect(
result.find((n) => n.id === mockListNotificationsAPIResult[0].id),
).toBeDefined();
expect(result.find((n) => n.type === TRIGGER_TYPES.SNAP)).toBeDefined();

// State is also updated
expect(controller.state.metamaskNotificationsList).toHaveLength(2);
expect(controller.state.metamaskNotificationsList).toHaveLength(3);
});

it('only fetches and processes feature announcements if not authenticated', async () => {
Expand Down Expand Up @@ -529,6 +541,148 @@ describe('metamask-notifications - fetchAndUpdateMetamaskNotifications()', () =>
});
});

describe('metamask-notifications - getNotificationsByType', () => {
it('can fetch notifications by their type', async () => {
const { messenger } = mockNotificationMessenger();
const controller = new NotificationServicesController({
messenger,
env: { featureAnnouncements: featureAnnouncementsEnv },
});

const processedSnapNotification = processSnapNotification(
createMockSnapNotification(),
);
const processedFeatureAnnouncement = processFeatureAnnouncement(
createMockFeatureAnnouncementRaw(),
);

await controller.updateMetamaskNotificationsList(processedSnapNotification);
await controller.updateMetamaskNotificationsList(
processedFeatureAnnouncement,
);

expect(controller.state.metamaskNotificationsList).toHaveLength(2);

const filteredNotifications = controller.getNotificationsByType(
TRIGGER_TYPES.SNAP,
);

expect(filteredNotifications).toHaveLength(1);
expect(filteredNotifications).toStrictEqual([
{
type: TRIGGER_TYPES.SNAP,
id: expect.any(String),
createdAt: expect.any(String),
isRead: false,
readDate: null,
data: {
message: 'fooBar',
origin: '@metamask/example-snap',
detailedView: {
title: 'Detailed View',
interfaceId: '1',
footerLink: {
text: 'Go Home',
href: 'metamask://client/',
},
},
},
},
]);
});
});

describe('metamask-notifications - deleteNotificationsById', () => {
it('will delete a notification by its id', async () => {
const { messenger } = mockNotificationMessenger();
const processedSnapNotification = processSnapNotification(
createMockSnapNotification(),
);
const controller = new NotificationServicesController({
messenger,
env: { featureAnnouncements: featureAnnouncementsEnv },
state: { metamaskNotificationsList: [processedSnapNotification] },
});

await controller.deleteNotificationsById([processedSnapNotification.id]);

expect(controller.state.metamaskNotificationsList).toHaveLength(0);
});

it('will batch delete notifications', async () => {
const { messenger } = mockNotificationMessenger();
const processedSnapNotification1 = processSnapNotification(
createMockSnapNotification(),
);
const processedSnapNotification2 = processSnapNotification(
createMockSnapNotification(),
);
const controller = new NotificationServicesController({
messenger,
env: { featureAnnouncements: featureAnnouncementsEnv },
state: {
metamaskNotificationsList: [
processedSnapNotification1,
processedSnapNotification2,
],
},
});

await controller.deleteNotificationsById([
processedSnapNotification1.id,
processedSnapNotification2.id,
]);

expect(controller.state.metamaskNotificationsList).toHaveLength(0);
});

it('will throw if a notification is not found', async () => {
const { messenger } = mockNotificationMessenger();
const processedSnapNotification = processSnapNotification(
createMockSnapNotification(),
);
const controller = new NotificationServicesController({
messenger,
env: { featureAnnouncements: featureAnnouncementsEnv },
state: { metamaskNotificationsList: [processedSnapNotification] },
});

await expect(controller.deleteNotificationsById(['foo'])).rejects.toThrow(
'The notification to be deleted does not exist.',
);

expect(controller.state.metamaskNotificationsList).toHaveLength(1);
});

it('will throw if the notification to be deleted is not locally persisted', async () => {
const { messenger } = mockNotificationMessenger();
const processedSnapNotification = processSnapNotification(
createMockSnapNotification(),
);
const processedFeatureAnnouncement = processFeatureAnnouncement(
createMockFeatureAnnouncementRaw(),
);
const controller = new NotificationServicesController({
messenger,
env: { featureAnnouncements: featureAnnouncementsEnv },
state: {
metamaskNotificationsList: [
processedFeatureAnnouncement,
processedSnapNotification,
],
},
});

await expect(
controller.deleteNotificationsById([processedFeatureAnnouncement.id]),
).rejects.toThrow(
'The notification type of "features_announcement" is not locally persisted, only the following types can use this function: snap.',
);

expect(controller.state.metamaskNotificationsList).toHaveLength(2);
});
});

describe('metamask-notifications - markMetamaskNotificationsAsRead()', () => {
const arrangeMocks = (options?: { onChainMarkAsReadFails: boolean }) => {
const messengerMocks = mockNotificationMessenger();
Expand Down Expand Up @@ -576,6 +730,38 @@ describe('metamask-notifications - markMetamaskNotificationsAsRead()', () => {
// We can debate & change implementation if it makes sense to mark as read locally if external APIs fail.
expect(controller.state.metamaskNotificationsReadList).toHaveLength(1);
});

it('updates snap notifications as read', async () => {
const { messenger } = arrangeMocks();
const processedSnapNotification = processSnapNotification(
createMockSnapNotification(),
);
const controller = new NotificationServicesController({
messenger,
env: { featureAnnouncements: featureAnnouncementsEnv },
state: {
metamaskNotificationsList: [processedSnapNotification],
},
});

await controller.markMetamaskNotificationsAsRead([
{
type: TRIGGER_TYPES.SNAP,
id: processedSnapNotification.id,
isRead: false,
},
]);

// Should see 1 item in controller read state
expect(controller.state.metamaskNotificationsReadList).toHaveLength(1);

// The notification should have a read date
expect(
// @ts-expect-error readDate property is guaranteed to exist
// as we're dealing with a snap notification
controller.state.metamaskNotificationsList[0].readDate,
).not.toBeNull();
});
});

describe('metamask-notifications - enableMetamaskNotifications()', () => {
Expand Down Expand Up @@ -670,6 +856,42 @@ describe('metamask-notifications - disableMetamaskNotifications()', () => {
});
});

describe('metamask-notifications - updateMetamaskNotificationsList', () => {
it('can add and process a new notification to the notifications list', async () => {
const { messenger } = mockNotificationMessenger();
const controller = new NotificationServicesController({
messenger,
env: { featureAnnouncements: featureAnnouncementsEnv },
state: { isNotificationServicesEnabled: true },
});
const processedSnapNotification = processSnapNotification(
createMockSnapNotification(),
);
await controller.updateMetamaskNotificationsList(processedSnapNotification);
expect(controller.state.metamaskNotificationsList).toStrictEqual([
{
type: TRIGGER_TYPES.SNAP,
id: expect.any(String),
createdAt: expect.any(String),
readDate: null,
isRead: false,
data: {
message: 'fooBar',
origin: '@metamask/example-snap',
detailedView: {
title: 'Detailed View',
interfaceId: '1',
footerLink: {
text: 'Go Home',
href: 'metamask://client/',
},
},
},
},
]);
});
});

// Type-Computation - we are extracting args and parameters from a generic type utility
// Thus this `AnyFunc` can be used to help constrain the generic parameters correctly
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down
Loading

0 comments on commit d75a971

Please sign in to comment.