Skip to content

Commit

Permalink
Check whether the notification feature flag is enabled before calling…
Browse files Browse the repository at this point in the history 
… notification v2
  • Loading branch information
@PooyaRaki
PooyaRaki committed Nov 25, 2024
1 parent eea1b3f commit 156d8e4
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 51 deletions.
88 changes: 62 additions & 26 deletions src/routes/notifications/v1/notifications.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe('Notifications Controller (Unit)', () => {
let safeConfigUrl: string;
let networkService: jest.MockedObjectDeep<INetworkService>;
let notificationServiceV2: jest.MockedObjectDeep<NotificationsServiceV2>;
let isNotificationsV2Enabled: boolean;

beforeEach(async () => {
jest.resetAllMocks();
Expand Down Expand Up @@ -75,6 +76,9 @@ describe('Notifications Controller (Unit)', () => {
);
notificationServiceV2 = moduleFixture.get(NotificationsServiceV2);
safeConfigUrl = configurationService.getOrThrow('safeConfig.baseUri');
isNotificationsV2Enabled = configurationService.getOrThrow(
'features.pushNotifications',
);
networkService = moduleFixture.get(NetworkService);

app = await new TestAppProvider().provide(moduleFixture);
Expand Down Expand Up @@ -153,18 +157,20 @@ describe('Notifications Controller (Unit)', () => {
(safeRegistration) => safeRegistration.safes.length > 0,
);

expect(notificationServiceV2.upsertSubscriptions).toHaveBeenCalledTimes(
safeRegistrationsWithSafe.length,
);

for (const [
index,
upsertSubscriptionsV2,
] of upsertSubscriptionsV2Dto.entries()) {
const nthCall = index + 1; // Convert zero-based index to a one-based call number
if (isNotificationsV2Enabled) {
expect(
notificationServiceV2.upsertSubscriptions,
).toHaveBeenNthCalledWith(nthCall, upsertSubscriptionsV2);
).toHaveBeenCalledTimes(safeRegistrationsWithSafe.length);

for (const [
index,
upsertSubscriptionsV2,
] of upsertSubscriptionsV2Dto.entries()) {
const nthCall = index + 1; // Convert zero-based index to a one-based call number
expect(
notificationServiceV2.upsertSubscriptions,
).toHaveBeenNthCalledWith(nthCall, upsertSubscriptionsV2);
}
}
},
);
Expand Down Expand Up @@ -206,7 +212,11 @@ describe('Notifications Controller (Unit)', () => {
}),
);

expect(notificationServiceV2.upsertSubscriptions).not.toHaveBeenCalled();
if (isNotificationsV2Enabled) {
expect(
notificationServiceV2.upsertSubscriptions,
).not.toHaveBeenCalled();
}
});

it('Server errors returned from provider', async () => {
Expand Down Expand Up @@ -244,7 +254,11 @@ describe('Notifications Controller (Unit)', () => {
error: 'Internal Server Error',
});

expect(notificationServiceV2.upsertSubscriptions).not.toHaveBeenCalled();
if (isNotificationsV2Enabled) {
expect(
notificationServiceV2.upsertSubscriptions,
).not.toHaveBeenCalled();
}
});

it('Both client and server errors returned from provider', async () => {
Expand Down Expand Up @@ -297,7 +311,11 @@ describe('Notifications Controller (Unit)', () => {
error: 'Internal Server Error',
});

expect(notificationServiceV2.upsertSubscriptions).not.toHaveBeenCalled();
if (isNotificationsV2Enabled) {
expect(
notificationServiceV2.upsertSubscriptions,
).not.toHaveBeenCalled();
}
});

it('No status code errors returned from provider', async () => {
Expand Down Expand Up @@ -333,7 +351,11 @@ describe('Notifications Controller (Unit)', () => {
error: 'Internal Server Error',
});

expect(notificationServiceV2.upsertSubscriptions).not.toHaveBeenCalled();
if (isNotificationsV2Enabled) {
expect(
notificationServiceV2.upsertSubscriptions,
).not.toHaveBeenCalled();
}
});
});

Expand Down Expand Up @@ -362,8 +384,10 @@ describe('Notifications Controller (Unit)', () => {
url: expectedProviderURL,
});

expect(notificationServiceV2.deleteDevice).toHaveBeenCalledTimes(1);
expect(notificationServiceV2.deleteDevice).toHaveBeenCalledWith(uuid);
if (isNotificationsV2Enabled) {
expect(notificationServiceV2.deleteDevice).toHaveBeenCalledTimes(1);
expect(notificationServiceV2.deleteDevice).toHaveBeenCalledWith(uuid);
}
});

it('Failure: Config API fails', async () => {
Expand All @@ -380,7 +404,9 @@ describe('Notifications Controller (Unit)', () => {
.expect(503);
expect(networkService.delete).toHaveBeenCalledTimes(0);

expect(notificationServiceV2.deleteDevice).not.toHaveBeenCalled();
if (isNotificationsV2Enabled) {
expect(notificationServiceV2.deleteDevice).not.toHaveBeenCalled();
}
});

it('Failure: Transaction API fails', async () => {
Expand All @@ -403,7 +429,9 @@ describe('Notifications Controller (Unit)', () => {
.expect(503);
expect(networkService.delete).toHaveBeenCalledTimes(1);

expect(notificationServiceV2.deleteDevice).not.toHaveBeenCalled();
if (isNotificationsV2Enabled) {
expect(notificationServiceV2.deleteDevice).not.toHaveBeenCalled();
}
});
});

Expand Down Expand Up @@ -436,12 +464,16 @@ describe('Notifications Controller (Unit)', () => {
url: expectedProviderURL,
});

expect(notificationServiceV2.deleteSubscription).toHaveBeenCalledTimes(1);
expect(notificationServiceV2.deleteSubscription).toHaveBeenCalledWith({
deviceUuid: uuid,
chainId: chain.chainId,
safeAddress: getAddress(safeAddress),
});
if (isNotificationsV2Enabled) {
expect(notificationServiceV2.deleteSubscription).toHaveBeenCalledTimes(
1,
);
expect(notificationServiceV2.deleteSubscription).toHaveBeenCalledWith({
deviceUuid: uuid,
chainId: chain.chainId,
safeAddress: getAddress(safeAddress),
});
}
});

it('Failure: Config API fails', async () => {
Expand All @@ -461,7 +493,9 @@ describe('Notifications Controller (Unit)', () => {
.expect(503);
expect(networkService.delete).toHaveBeenCalledTimes(0);

expect(notificationServiceV2.deleteSubscription).not.toHaveBeenCalled();
if (isNotificationsV2Enabled) {
expect(notificationServiceV2.deleteSubscription).not.toHaveBeenCalled();
}
});

it('Failure: Transaction API fails', async () => {
Expand All @@ -487,7 +521,9 @@ describe('Notifications Controller (Unit)', () => {
.expect(503);
expect(networkService.delete).toHaveBeenCalledTimes(1);

expect(notificationServiceV2.deleteSubscription).not.toHaveBeenCalled();
if (isNotificationsV2Enabled) {
expect(notificationServiceV2.deleteSubscription).not.toHaveBeenCalled();
}
});
});
});
66 changes: 41 additions & 25 deletions src/routes/notifications/v1/notifications.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,27 @@ import type { UUID } from 'crypto';
import { NotificationsServiceV2 } from '@/routes/notifications/v2/notifications.service';
import { recoverMessageAddress } from 'viem';
import { UuidSchema } from '@/validation/entities/schemas/uuid.schema';
import { IConfigurationService } from '@/config/configuration.service.interface';

@ApiTags('notifications')
@Controller({ path: '', version: '1' })
export class NotificationsController {
private isPushNotificationV2Enabled = false;
constructor(
// Adding NotificationServiceV2 to ensure compatibility with V1.
// @TODO Remove NotificationModuleV2 after all clients have migrated and compatibility is no longer needed.
@Inject(NotificationsServiceV2)
private readonly notificationServiceV2: NotificationsServiceV2,
private readonly notificationsService: NotificationsService,
) {}

@Inject(IConfigurationService)
private readonly configurationService: IConfigurationService,
) {
this.isPushNotificationV2Enabled =
this.configurationService.getOrThrow<boolean>(
'features.pushNotifications',
);
}

@ApiOkResponse()
@Post('register/notifications')
Expand All @@ -42,21 +52,23 @@ export class NotificationsController {
): Promise<void> {
await this.notificationsService.registerDevice(registerDeviceDto);

// Compatibility with V2
// @TODO Remove NotificationModuleV2 after all clients have migrated and compatibility is no longer needed.
const compatibleV2Requests =
await this.createV2RegisterDto(registerDeviceDto);

const v2Requests = [];
for (const compatibleV2Request of compatibleV2Requests) {
v2Requests.push(
await this.notificationServiceV2.upsertSubscriptions(
compatibleV2Request,
),
);
}
if (this.isPushNotificationV2Enabled) {
// Compatibility with V2
// @TODO Remove NotificationModuleV2 after all clients have migrated and compatibility is no longer needed.
const compatibleV2Requests =
await this.createV2RegisterDto(registerDeviceDto);

const v2Requests = [];
for (const compatibleV2Request of compatibleV2Requests) {
v2Requests.push(
await this.notificationServiceV2.upsertSubscriptions(
compatibleV2Request,
),
);
}

await Promise.all(v2Requests);
await Promise.all(v2Requests);
}
}

private async createV2RegisterDto(args: RegisterDeviceDto): Promise<
Expand Down Expand Up @@ -131,9 +143,11 @@ export class NotificationsController {
): Promise<void> {
await this.notificationsService.unregisterDevice({ chainId, uuid });

// Compatibility with V2
// @TODO Remove NotificationModuleV2 after all clients have migrated and compatibility is no longer needed.
await this.notificationServiceV2.deleteDevice(uuid);
if (this.isPushNotificationV2Enabled) {
// Compatibility with V2
// @TODO Remove NotificationModuleV2 after all clients have migrated and compatibility is no longer needed.
await this.notificationServiceV2.deleteDevice(uuid);
}
}

@Delete('chains/:chainId/notifications/devices/:uuid/safes/:safeAddress')
Expand All @@ -149,12 +163,14 @@ export class NotificationsController {
safeAddress,
});

// Compatibility with V2
// @TODO Remove NotificationModuleV2 after all clients have migrated and compatibility is no longer needed.
await this.notificationServiceV2.deleteSubscription({
deviceUuid: uuid,
chainId: chainId,
safeAddress: safeAddress,
});
if (this.isPushNotificationV2Enabled) {
// Compatibility with V2
// @TODO Remove NotificationModuleV2 after all clients have migrated and compatibility is no longer needed.
await this.notificationServiceV2.deleteSubscription({
deviceUuid: uuid,
chainId: chainId,
safeAddress: safeAddress,
});
}
}
}

0 comments on commit 156d8e4

Please sign in to comment.