From 28cd3ad56da80d68aa019cd778ac3f656e6a19ec Mon Sep 17 00:00:00 2001 From: Valentin Yanakiev Date: Mon, 17 Jan 2022 11:49:13 +0200 Subject: [PATCH 1/2] Notifications flag fix --- src/app.controller.ts | 8 ++++++-- .../notified.users.provider.interface.ts | 3 ++- .../notification/notification.service.spec.ts | 17 ++++++++++++++++ .../notification/notification.service.ts | 20 ++++++++++++++++++- 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/app.controller.ts b/src/app.controller.ts index 4779e8a8..6c459ed1 100644 --- a/src/app.controller.ts +++ b/src/app.controller.ts @@ -92,7 +92,7 @@ export class AppController { @Payload() eventPayload: any, @Ctx() context: RmqContext, // notificationBuilder: any, - sendNotifications: any, + sendNotificationsImpl: any, eventName: string ) { this.logger.verbose?.( @@ -105,11 +105,15 @@ export class AppController { if (!(await this.featureFlagProvider.areNotificationsEnabled())) { channel.ack(originalMsg); + this.logger.verbose?.( + 'Notifications are disabled. Returning...', + LogContext.NOTIFICATIONS + ); return; } // https://www.squaremobius.net/amqp.node/channel_api.html#channel_nack - sendNotifications + sendNotificationsImpl .then((x: any[]) => { const nacked = x.filter( (y: { status: string }) => y.status === 'rejected' diff --git a/src/core/contracts/notified.users.provider.interface.ts b/src/core/contracts/notified.users.provider.interface.ts index fb96f010..f98990a0 100644 --- a/src/core/contracts/notified.users.provider.interface.ts +++ b/src/core/contracts/notified.users.provider.interface.ts @@ -1,7 +1,8 @@ import { User } from '../models'; import { CredentialCriteria } from '../models/credential.criteria'; +import { IFeatureFlagProvider } from './feature.flag.provider.interface'; -export interface INotifiedUsersProvider { +export interface INotifiedUsersProvider extends IFeatureFlagProvider { getUser(userID: string): Promise; getUniqueUsersMatchingCredentialCriteria( credentialCriterias: CredentialCriteria[] diff --git a/src/services/domain/notification/notification.service.spec.ts b/src/services/domain/notification/notification.service.spec.ts index e22ff904..189488a0 100644 --- a/src/services/domain/notification/notification.service.spec.ts +++ b/src/services/domain/notification/notification.service.spec.ts @@ -59,6 +59,7 @@ describe('NotificationService', () => { { provide: ALKEMIO_CLIENT_ADAPTER, useValue: { + areNotificationsEnabled: jest.fn(), getUser: jest.fn(), getApplicationCreator: jest.fn(), getUsersMatchingCredentialCriteria: jest.fn(), @@ -108,6 +109,10 @@ describe('NotificationService', () => { ...testData.opportunityAdmins, ]; + jest + .spyOn(alkemioAdapter, 'areNotificationsEnabled') + .mockResolvedValue(true); + jest .spyOn(alkemioAdapter, 'getUniqueUsersMatchingCredentialCriteria') .mockResolvedValue(admins); @@ -140,5 +145,17 @@ describe('NotificationService', () => { ) ).rejects.toThrow(); }); + + it('Should not send notifications when notifications are disabled', async () => { + jest + .spyOn(alkemioAdapter, 'areNotificationsEnabled') + .mockResolvedValue(false); + + const res = await notificationService.sendApplicationCreatedNotifications( + testData.data as ApplicationCreatedEventPayload + ); + + expect(res.length).toBe(0); //shouldn't have any notifications sent + }); }); }); diff --git a/src/services/domain/notification/notification.service.ts b/src/services/domain/notification/notification.service.ts index 0a832d8a..d4168b91 100644 --- a/src/services/domain/notification/notification.service.ts +++ b/src/services/domain/notification/notification.service.ts @@ -1,7 +1,11 @@ import { Injectable, Inject, LoggerService } from '@nestjs/common'; import { WINSTON_MODULE_NEST_PROVIDER } from 'nest-winston'; import NotifmeSdk, { NotificationStatus } from 'notifme-sdk'; -import { LogContext, NOTIFICATIONS_PROVIDER } from '@src/common'; +import { + ALKEMIO_CLIENT_ADAPTER, + LogContext, + NOTIFICATIONS_PROVIDER, +} from '@src/common'; import { ApplicationCreatedEventPayload } from '@src/types/application.created.event.payload'; import { ApplicationCreatedNotificationBuilder } from '@src/services'; import { CommunicationDiscussionCreatedNotificationBuilder } from '../builders/communication-discussion-created/communication.discussion.created.notification.builder'; @@ -10,12 +14,15 @@ import { UserRegisteredNotificationBuilder } from '../builders/user-registered/u import { UserRegistrationEventPayload } from '@src/types'; import { CommunicationUpdateEventPayload } from '@src/types/communication.update.event.payload'; import { CommunicationDiscussionCreatedEventPayload } from '@src/types/communication.discussion.created.event.payload'; +import { AlkemioClientAdapter } from '@src/services/application/alkemio-client-adapter'; @Injectable() export class NotificationService { constructor( @Inject(WINSTON_MODULE_NEST_PROVIDER) private readonly logger: LoggerService, + @Inject(ALKEMIO_CLIENT_ADAPTER) + private readonly alkemioClientAdapter: AlkemioClientAdapter, @Inject(NOTIFICATIONS_PROVIDER) private readonly notifmeService: NotifmeSdk, private applicationCreatedNotificationBuilder: ApplicationCreatedNotificationBuilder, @@ -28,6 +35,17 @@ export class NotificationService { payload: any, notificationBuilder: any ): Promise[]> { + const notificationsEnabled = + await this.alkemioClientAdapter.areNotificationsEnabled(); + if (!notificationsEnabled) { + this.logger.verbose?.( + 'Notification disabled. No notifications are going to be built.', + LogContext.NOTIFICATIONS + ); + + return []; + } + return notificationBuilder .buildNotifications(payload) .then((x: any[]) => x.map((x: any) => this.sendNotification(x))) From 619bf844cb2c8c602875b3f00e277c4000ebdb2c Mon Sep 17 00:00:00 2001 From: Valentin Yanakiev Date: Mon, 17 Jan 2022 12:12:44 +0200 Subject: [PATCH 2/2] Addressed comments --- package-lock.json | 4 ++-- package.json | 2 +- src/app.controller.ts | 4 ---- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/package-lock.json b/package-lock.json index b61d7055..9bc3e25c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "alkemio-notifications", - "version": "0.4.5", + "version": "0.4.6", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "alkemio-notifications", - "version": "0.4.5", + "version": "0.4.6", "license": "EUPL-1.2", "dependencies": { "@alkemio/client-lib": "^0.10.4", diff --git a/package.json b/package.json index df43a473..1467615a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "alkemio-notifications", - "version": "0.4.5", + "version": "0.4.6", "description": "Alkemio notifications service", "author": "Alkemio Foundation", "private": false, diff --git a/src/app.controller.ts b/src/app.controller.ts index 6c459ed1..b11b2fe4 100644 --- a/src/app.controller.ts +++ b/src/app.controller.ts @@ -105,10 +105,6 @@ export class AppController { if (!(await this.featureFlagProvider.areNotificationsEnabled())) { channel.ack(originalMsg); - this.logger.verbose?.( - 'Notifications are disabled. Returning...', - LogContext.NOTIFICATIONS - ); return; }