From 6b7bf840635f9c6b5502937b6c3a8ced65da9e3b Mon Sep 17 00:00:00 2001 From: Hassan Malik <41640681+hmalik88@users.noreply.github.com> Date: Tue, 24 Sep 2024 10:22:40 -0400 Subject: [PATCH] feat: Expand notification args (#4682) ## 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 --- .../src/NotificationController.test.ts | 67 +++++++++++-------- .../src/NotificationController.ts | 28 +++++++- 2 files changed, 63 insertions(+), 32 deletions(-) diff --git a/packages/notification-controller/src/NotificationController.test.ts b/packages/notification-controller/src/NotificationController.test.ts index d3ce348821..ac1638be2e 100644 --- a/packages/notification-controller/src/NotificationController.test.ts +++ b/packages/notification-controller/src/NotificationController.test.ts @@ -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); @@ -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); @@ -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', ]), @@ -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); @@ -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', ]), @@ -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); @@ -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); }); diff --git a/packages/notification-controller/src/NotificationController.ts b/packages/notification-controller/src/NotificationController.ts index afe8b28a1d..31fb22eca9 100644 --- a/packages/notification-controller/src/NotificationController.ts +++ b/packages/notification-controller/src/NotificationController.ts @@ -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< @@ -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( @@ -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;