Skip to content
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.

Sign up for GitHub

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

feat: optimise push controller API calls #5358

Merged
merged 4 commits into from
Feb 18, 2025
Merged

feat: optimise push controller API calls #5358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c056db
feat: removes the get push links API call
Prithpal-Sooriya Feb 18, 2025
1e570fd
Merge branch 'main' into remove-get-push-links-call
Prithpal-Sooriya Feb 18, 2025
8fda7f5
chore: fix eslint warnings
Prithpal-Sooriya Feb 18, 2025
b1e904c
Merge branch 'main' into remove-get-push-links-call
Prithpal-Sooriya Feb 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 1 addition & 19 deletions ...troller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Messenger } from '@metamask/base-controller';
import type { AuthenticationController } from '@metamask/profile-sync-controller';
import log from 'loglevel';

import NotificationServicesPushController from './NotificationServicesPushController';
import type {
Expand All @@ -11,10 +10,6 @@ import type {
import * as services from './services/services';
import type { PushNotificationEnv } from './types';

// Testing util to clean up verbose logs when testing errors
const mockErrorLog = () =>
jest.spyOn(log, 'error').mockImplementation(jest.fn());

const MOCK_JWT = 'mockJwt';
const MOCK_FCM_TOKEN = 'mockFcmToken';
const MOCK_MOBILE_FCM_TOKEN = 'mockMobileFcmToken';
Expand Down Expand Up @@ -89,21 +84,9 @@ describe('NotificationServicesPushController', () => {
arrangeServicesMocks();
const { controller, messenger } = arrangeMockMessenger();
mockAuthBearerTokenCall(messenger);
await controller.disablePushNotifications(MOCK_TRIGGERS);
await controller.disablePushNotifications();
expect(controller.state.fcmToken).toBe('');
});

it('should fail if a jwt token is not provided', async () => {
arrangeServicesMocks();
mockErrorLog();
const { controller, messenger } = arrangeMockMessenger();
mockAuthBearerTokenCall(messenger).mockResolvedValue(
null as unknown as string,
);
await expect(controller.disablePushNotifications([])).rejects.toThrow(
expect.any(Error),
);
});
});

describe('updateTriggerPushNotifications', () => {
Expand All @@ -127,7 +110,6 @@ describe('NotificationServicesPushController', () => {
const args = spy.mock.calls[0][0];
expect(args.bearerToken).toBe(MOCK_JWT);
expect(args.triggers).toBe(MOCK_TRIGGERS);
expect(args.regToken).toBe(controller.state.fcmToken);
});
});
});
Expand Down
13 changes: 2 additions & 11 deletions ...s-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,26 +286,18 @@ export default class NotificationServicesPushController extends BaseController<

/**
* Disables push notifications for the application.
* This method handles the process of disabling push notifications by:
* 1. Unregistering the service worker to stop listening for messages.
* 2. Sending a request to the server to unregister the device using the FCM token.
* 3. Removing the FCM token from the state to complete the process.
*
* @param UUIDs - An array of UUIDs for which push notifications should be disabled.
* This removes the registration token on this device, and ensures we unsubscribe from any listeners
*/
async disablePushNotifications(UUIDs: string[]) {
async disablePushNotifications() {
if (!this.#config.isPushEnabled) {
return;
}

const bearerToken = await this.#getAndAssertBearerToken();
let isPushNotificationsDisabled: boolean;

try {
// Send a request to the server to unregister the token/device
isPushNotificationsDisabled = await deactivatePushNotifications({
bearerToken,
triggers: UUIDs,
env: this.#env,
deleteRegToken,
regToken: this.state.fcmToken,
Expand Down Expand Up @@ -356,7 +348,6 @@ export default class NotificationServicesPushController extends BaseController<
createRegToken,
deleteRegToken,
platform: this.#config.platform,
regToken: this.state.fcmToken,
});

// update the state with the new FCM token
Expand Down
19 changes: 1 addition & 18 deletions ...n-services-controller/src/NotificationServicesPushController/__fixtures__/mockServices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,12 @@
import nock from 'nock';

import {
getMockRetrievePushNotificationLinksResponse,
getMockUpdatePushNotificationLinksResponse,
} from './mockResponse';
import { getMockUpdatePushNotificationLinksResponse } from './mockResponse';

type MockReply = {
status: nock.StatusCode;
body?: nock.Body;
};

export const mockEndpointGetPushNotificationLinks = (mockReply?: MockReply) => {
const mockResponse = getMockRetrievePushNotificationLinksResponse();
const reply = mockReply ?? {
status: 200,
body: mockResponse.response,
};

const mockEndpoint = nock(mockResponse.url)
.get('')
.reply(reply.status, reply.body);

return mockEndpoint;
};

export const mockEndpointUpdatePushNotificationLinks = (
mockReply?: MockReply,
) => {
Expand Down
113 changes: 7 additions & 106 deletions ...tion-services-controller/src/NotificationServicesPushController/services/services.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,11 @@ import * as PushWebModule from './push/push-web';
import {
activatePushNotifications,
deactivatePushNotifications,
getPushNotificationLinks,
listenToPushNotifications,
updateLinksAPI,
updateTriggerPushNotifications,
} from './services';
import {
mockEndpointGetPushNotificationLinks,
mockEndpointUpdatePushNotificationLinks,
} from '../__fixtures__/mockServices';
import { mockEndpointUpdatePushNotificationLinks } from '../__fixtures__/mockServices';
import type { PushNotificationEnv } from '../types/firebase';

// Testing util to clean up verbose logs when testing errors
Expand All @@ -26,24 +22,6 @@ const MOCK_TRIGGERS = ['1', '2', '3'];
const MOCK_JWT = 'MOCK_JWT';

describe('NotificationServicesPushController Services', () => {
describe('getPushNotificationLinks', () => {
it('should return reg token links', async () => {
const mockAPI = mockEndpointGetPushNotificationLinks();
const result = await getPushNotificationLinks(MOCK_JWT);
expect(mockAPI.isDone()).toBe(true);
expect(result?.registration_tokens).toBeDefined();
expect(result?.trigger_ids).toBeDefined();
});

it('should return null if given a bad response', async () => {
const mockAPI = mockEndpointGetPushNotificationLinks({ status: 500 });
mockErrorLog();
const result = await getPushNotificationLinks(MOCK_JWT);
expect(mockAPI.isDone()).toBe(true);
expect(result).toBeNull();
});
});

describe('updateLinksAPI', () => {
const act = async () =>
await updateLinksAPI(MOCK_JWT, MOCK_TRIGGERS, [
Expand Down Expand Up @@ -74,10 +52,7 @@ describe('NotificationServicesPushController Services', () => {
});

describe('activatePushNotifications', () => {
const arrangeMocks = (override?: {
mockGet?: { status: number };
mockPut?: { status: number };
}) => {
const arrangeMocks = (override?: { mockPut?: { status: number } }) => {
const params = {
bearerToken: MOCK_JWT,
triggers: MOCK_TRIGGERS,
Expand All @@ -96,7 +71,6 @@ describe('NotificationServicesPushController Services', () => {
params,
mobileParams,
apis: {
mockGet: mockEndpointGetPushNotificationLinks(override?.mockGet),
mockPut: mockEndpointUpdatePushNotificationLinks(override?.mockPut),
},
};
Expand All @@ -106,7 +80,6 @@ describe('NotificationServicesPushController Services', () => {
const { params, apis } = arrangeMocks();
const result = await activatePushNotifications(params);

expect(apis.mockGet.isDone()).toBe(true);
expect(params.createRegToken).toHaveBeenCalled();
expect(apis.mockPut.isDone()).toBe(true);

Expand All @@ -118,32 +91,18 @@ describe('NotificationServicesPushController Services', () => {
mockErrorLog();
const result = await activatePushNotifications(mobileParams);

expect(apis.mockGet.isDone()).toBe(true);
expect(mobileParams.createRegToken).not.toHaveBeenCalled();
expect(apis.mockPut.isDone()).toBe(true);

expect(result).toBe(MOCK_MOBILE_FCM_TOKEN);
});

it('should return null if unable to get links from API', async () => {
const { params, apis } = arrangeMocks({ mockGet: { status: 500 } });
mockErrorLog();
const result = await activatePushNotifications(params);

expect(apis.mockGet.isDone()).toBe(true);
expect(params.createRegToken).not.toHaveBeenCalled();
expect(apis.mockPut.isDone()).toBe(false);

expect(result).toBeNull();
});

it('should return null if unable to create new registration token', async () => {
const { params, apis } = arrangeMocks();
params.createRegToken.mockRejectedValue(new Error('MOCK ERROR'));

const result = await activatePushNotifications(params);

expect(apis.mockGet.isDone()).toBe(true);
expect(params.createRegToken).toHaveBeenCalled();
expect(apis.mockPut.isDone()).toBe(false);

Expand All @@ -152,10 +111,7 @@ describe('NotificationServicesPushController Services', () => {
});

describe('deactivatePushNotifications', () => {
const arrangeMocks = (override?: {
mockGet?: { status: number };
mockPut?: { status: number };
}) => {
const arrangeMocks = () => {
const params = {
regToken: MOCK_REG_TOKEN,
bearerToken: MOCK_JWT,
Expand All @@ -166,80 +122,41 @@ describe('NotificationServicesPushController Services', () => {

return {
params,
apis: {
mockGet: mockEndpointGetPushNotificationLinks(override?.mockGet),
mockPut: mockEndpointUpdatePushNotificationLinks(override?.mockPut),
},
};
};

it('should successfully delete the registration token', async () => {
const { params, apis } = arrangeMocks();
const { params } = arrangeMocks();
const result = await deactivatePushNotifications(params);

expect(apis.mockGet.isDone()).toBe(true);
expect(apis.mockPut.isDone()).toBe(true);
expect(params.deleteRegToken).toHaveBeenCalled();

expect(result).toBe(true);
});

it('should return early when there is no registration token to delete', async () => {
const { params, apis } = arrangeMocks();
const { params } = arrangeMocks();
mockErrorLog();
const result = await deactivatePushNotifications({
...params,
regToken: '',
});

expect(apis.mockGet.isDone()).toBe(false);
expect(apis.mockPut.isDone()).toBe(false);
expect(params.deleteRegToken).not.toHaveBeenCalled();

expect(result).toBe(true);
});

it('should return false when unable to get links api', async () => {
const { params, apis } = arrangeMocks({ mockGet: { status: 500 } });
mockErrorLog();
const result = await deactivatePushNotifications(params);

expect(apis.mockGet.isDone()).toBe(true);
expect(apis.mockPut.isDone()).toBe(false);
expect(params.deleteRegToken).not.toHaveBeenCalled();

expect(result).toBe(false);
});

it('should return false when unable to update links api', async () => {
const { params, apis } = arrangeMocks({ mockPut: { status: 500 } });
const result = await deactivatePushNotifications(params);

expect(apis.mockGet.isDone()).toBe(true);
expect(apis.mockPut.isDone()).toBe(true);
expect(params.deleteRegToken).not.toHaveBeenCalled();

expect(result).toBe(false);
});

it('should return false when unable to delete the existing reg token', async () => {
const { params, apis } = arrangeMocks();
const { params } = arrangeMocks();
params.deleteRegToken.mockResolvedValue(false);
const result = await deactivatePushNotifications(params);

expect(apis.mockGet.isDone()).toBe(true);
expect(apis.mockPut.isDone()).toBe(true);
expect(params.deleteRegToken).toHaveBeenCalled();

expect(result).toBe(false);
});
});

describe('updateTriggerPushNotifications', () => {
const arrangeMocks = (override?: {
mockGet?: { status: number };
mockPut?: { status: number };
}) => {
const arrangeMocks = (override?: { mockPut?: { status: number } }) => {
const params = {
regToken: MOCK_REG_TOKEN,
bearerToken: MOCK_JWT,
Expand All @@ -253,7 +170,6 @@ describe('NotificationServicesPushController Services', () => {
return {
params,
apis: {
mockGet: mockEndpointGetPushNotificationLinks(override?.mockGet),
mockPut: mockEndpointUpdatePushNotificationLinks(override?.mockPut),
},
};
Expand All @@ -264,7 +180,6 @@ describe('NotificationServicesPushController Services', () => {
mockErrorLog();
const result = await updateTriggerPushNotifications(params);

expect(apis.mockGet.isDone()).toBe(true);
expect(params.deleteRegToken).toHaveBeenCalled();
expect(params.createRegToken).toHaveBeenCalled();
expect(apis.mockPut.isDone()).toBe(true);
Expand All @@ -273,20 +188,6 @@ describe('NotificationServicesPushController Services', () => {
expect(result.isTriggersLinkedToPushNotifications).toBe(true);
});

it('should return early if fails to get links api', async () => {
const { params, apis } = arrangeMocks({ mockGet: { status: 500 } });
mockErrorLog();
const result = await updateTriggerPushNotifications(params);

expect(apis.mockGet.isDone()).toBe(true);
expect(params.deleteRegToken).not.toHaveBeenCalled();
expect(params.createRegToken).not.toHaveBeenCalled();
expect(apis.mockPut.isDone()).toBe(false);

expect(result.fcmToken).toBeUndefined();
expect(result.isTriggersLinkedToPushNotifications).toBe(false);
});

it('should throw error if fails to create reg token', async () => {
const { params } = arrangeMocks();
params.createRegToken.mockResolvedValue(null);
Expand Down
Loading
Loading