Skip to content

Commit

Permalink
feat: Expand notification args (#4682)
Browse files Browse the repository at this point in the history
## Explanation


* What is the current state of things and why does it need to change?
The notification controller cannot accommodate "expanded view" snaps
notifications
* What is the solution your changes offer and how does it work? The
extra properties being sent with the `show` method will now be persisted
as well so the extension can use it to show JSX content for snaps within
an expanded view of a notification.

## Changelog

### `@metamask/notification-controller`

- **CHANGED**: `show` function's arguments to be an object so we can
persist the extra properties needed to show JSX content in snap
notifications.

## 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
  • Loading branch information
hmalik88 committed Sep 24, 2024
1 parent 599ac25 commit 6b7bf84
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 32 deletions.
67 changes: 38 additions & 29 deletions packages/notification-controller/src/NotificationController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const origin = 'snap_test';
const message = 'foo';

describe('NotificationController', () => {
it('action: NotificationController:show', async () => {
it('action: NotificationController:show', () => {
const unrestricted = getUnrestrictedMessenger();
const messenger = getRestrictedMessenger(unrestricted);

Expand All @@ -49,22 +49,39 @@ describe('NotificationController', () => {
});

expect(
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/await-thenable
await unrestricted.call('NotificationController:show', origin, message),
unrestricted.call('NotificationController:show', origin, {
message,
}),
).toBeUndefined();

expect(
unrestricted.call('NotificationController:show', origin, {
message,
title: 'title',
interfaceId: '1',
}),
).toBeUndefined();
const notifications = Object.values(controller.state.notifications);
expect(notifications).toHaveLength(1);
expect(notifications).toContainEqual({
expect(notifications).toHaveLength(2);
expect(notifications[0]).toStrictEqual({
createdDate: expect.any(Number),
id: expect.any(String),
message,
origin,
readDate: null,
expandedView: null,
});
expect(notifications[1]).toStrictEqual({
createdDate: expect.any(Number),
id: expect.any(String),
message,
origin,
readDate: null,
expandedView: { title: 'title', interfaceId: '1' },
});
});

it('action: NotificationController:markViewed', async () => {
it('action: NotificationController:markViewed', () => {
const unrestricted = getUnrestrictedMessenger();
const messenger = getRestrictedMessenger(unrestricted);

Expand All @@ -73,16 +90,14 @@ describe('NotificationController', () => {
});

expect(
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/await-thenable
await unrestricted.call('NotificationController:show', origin, message),
unrestricted.call('NotificationController:show', origin, {
message,
}),
).toBeUndefined();
const notifications = Object.values(controller.state.notifications);
expect(notifications).toHaveLength(1);
expect(
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/await-thenable
await unrestricted.call('NotificationController:markRead', [
unrestricted.call('NotificationController:markRead', [
notifications[0].id,
'foo',
]),
Expand All @@ -97,7 +112,7 @@ describe('NotificationController', () => {
expect(newNotifications).toHaveLength(1);
});

it('action: NotificationController:dismiss', async () => {
it('action: NotificationController:dismiss', () => {
const unrestricted = getUnrestrictedMessenger();
const messenger = getRestrictedMessenger(unrestricted);

Expand All @@ -106,16 +121,14 @@ describe('NotificationController', () => {
});

expect(
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/await-thenable
await unrestricted.call('NotificationController:show', origin, message),
unrestricted.call('NotificationController:show', origin, {
message,
}),
).toBeUndefined();
const notifications = Object.values(controller.state.notifications);
expect(notifications).toHaveLength(1);
expect(
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/await-thenable
await unrestricted.call('NotificationController:dismiss', [
unrestricted.call('NotificationController:dismiss', [
notifications[0].id,
'foo',
]),
Expand All @@ -124,7 +137,7 @@ describe('NotificationController', () => {
expect(Object.values(controller.state.notifications)).toHaveLength(0);
});

it('action: NotificationController:clear', async () => {
it('action: NotificationController:clear', () => {
const unrestricted = getUnrestrictedMessenger();
const messenger = getRestrictedMessenger(unrestricted);

Expand All @@ -133,17 +146,13 @@ describe('NotificationController', () => {
});

expect(
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/await-thenable
await unrestricted.call('NotificationController:show', origin, message),
unrestricted.call('NotificationController:show', origin, {
message,
}),
).toBeUndefined();
const notifications = Object.values(controller.state.notifications);
expect(notifications).toHaveLength(1);
expect(
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/await-thenable
await unrestricted.call('NotificationController:clear'),
).toBeUndefined();
expect(unrestricted.call('NotificationController:clear')).toBeUndefined();

expect(Object.values(controller.state.notifications)).toHaveLength(0);
});
Expand Down
28 changes: 25 additions & 3 deletions packages/notification-controller/src/NotificationController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,20 @@ export type Notification = {
message: string;
};

/**
* @typedef NotificationOptions - Notification data to be used to display in the UI
* @property message - The notification message
* @property title - The notification's title displayed in expanded view
* @property interfaceId - The interface id of the content to be displayed in expanded view
* @property footerLink - An object holding link information to be used in the expanded view
*/
export type NotificationOptions = {
message: string;
title?: string;
interfaceId?: string;
footerLink?: { href: string; text: string };
};

const name = 'NotificationController';

export type NotificationControllerStateChange = ControllerStateChangeEvent<
Expand Down Expand Up @@ -117,7 +131,8 @@ export class NotificationController extends BaseController<

this.messagingSystem.registerActionHandler(
`${name}:show` as const,
(origin: string, message: string) => this.show(origin, message),
(origin: string, options: NotificationOptions) =>
this.show(origin, options),
);

this.messagingSystem.registerActionHandler(
Expand All @@ -139,16 +154,23 @@ export class NotificationController extends BaseController<
* Shows a notification.
*
* @param origin - The origin trying to send a notification
* @param message - A message to show on the notification
* @param options - Notification args object
* @param options.title - The title to show in an expanded view
* @param options.interfaceId - A interface id for snap content
* @param options.footerLink - Footer object
* @param options.footerLink.href - Footer href
* @param options.footerLink.text - Link text
*/
show(origin: string, message: string) {
show(origin: string, options: NotificationOptions) {
const id = nanoid();
const { message, ...expandedView } = options;
const notification = {
id,
origin,
createdDate: Date.now(),
readDate: null,
message,
expandedView: Object.keys(expandedView).length > 0 ? expandedView : null,
};
this.update((state) => {
state.notifications[id] = notification;
Expand Down

0 comments on commit 6b7bf84

Please sign in to comment.