From 75c9ffa4c4a483c2c50956b6827b5d518b43816c Mon Sep 17 00:00:00 2001 From: Richard Fontein <32132657+rifont@users.noreply.github.com> Date: Wed, 5 Jun 2024 16:04:18 +0100 Subject: [PATCH] refactor(api): improve notification and email handling (#5683) * refactor(api): improve notification and email handling * refactor(invites): update webhook DTO and use typed headers * feat(env): add HubSpot environment variables * refactor(invites): rename invite-nudge-webhook to invite-nudge --- .../parse-event-request.usecase.ts | 137 +++++++++++------- .../src/app/invites/dtos/invite-member.dto.ts | 11 +- .../api/src/app/invites/invites.controller.ts | 12 +- apps/api/src/app/invites/usecases/index.ts | 2 +- .../invite-nudge-command.ts | 8 - .../invite-nudge/invite-nudge.command.ts | 11 ++ .../invite-nudge.usecase.ts} | 18 +-- apps/api/src/types/env.d.ts | 3 + .../services/cache/key-builders/entities.ts | 11 ++ .../cache/key-builders/identifiers.ts | 1 + libs/dal/src/repositories/user/user.entity.ts | 4 +- 11 files changed, 135 insertions(+), 83 deletions(-) delete mode 100644 apps/api/src/app/invites/usecases/invite-nudge-webhook/invite-nudge-command.ts create mode 100644 apps/api/src/app/invites/usecases/invite-nudge/invite-nudge.command.ts rename apps/api/src/app/invites/usecases/{invite-nudge-webhook/invite-nudge-usecase.ts => invite-nudge/invite-nudge.usecase.ts} (70%) diff --git a/apps/api/src/app/events/usecases/parse-event-request/parse-event-request.usecase.ts b/apps/api/src/app/events/usecases/parse-event-request/parse-event-request.usecase.ts index 051dbad398e..62aaafd6eb5 100644 --- a/apps/api/src/app/events/usecases/parse-event-request/parse-event-request.usecase.ts +++ b/apps/api/src/app/events/usecases/parse-event-request/parse-event-request.usecase.ts @@ -5,6 +5,7 @@ import { merge } from 'lodash'; import { v4 as uuidv4 } from 'uuid'; import { buildNotificationTemplateIdentifierKey, + buildHasNotificationKey, CachedEntity, Instrument, InstrumentUsecase, @@ -15,6 +16,7 @@ import { AnalyticsService, GetFeatureFlag, GetFeatureFlagCommand, + InvalidateCacheService, } from '@novu/application-generic'; import { FeatureFlagsKeysEnum, @@ -55,11 +57,12 @@ export class ParseEventRequest { private tenantRepository: TenantRepository, private workflowOverrideRepository: WorkflowOverrideRepository, private analyticsService: AnalyticsService, - private getFeatureFlag: GetFeatureFlag + private getFeatureFlag: GetFeatureFlag, + private invalidateCacheService: InvalidateCacheService ) {} @InstrumentUsecase() - async execute(command: ParseEventRequestCommand) { + public async execute(command: ParseEventRequestCommand) { const transactionId = command.transactionId || uuidv4(); const template = await this.getNotificationTemplateByTriggerIdentifier({ @@ -155,18 +158,7 @@ export class ParseEventRequest { transactionId, }; - const isEnabled = await this.getFeatureFlag.execute( - GetFeatureFlagCommand.create({ - key: FeatureFlagsKeysEnum.IS_TEAM_MEMBER_INVITE_NUDGE_ENABLED, - organizationId: command.organizationId, - userId: 'system', - environmentId: 'system', - }) - ); - - if (isEnabled && (process.env.NODE_ENV === 'dev' || process.env.NODE_ENV === 'production')) { - await this.sendInAppNudgeForTeamMemberInvite(command); - } + await this.sendInAppNudgeForTeamMemberInvite(command); await this.workflowQueueService.add({ name: transactionId, data: jobData, groupId: command.organizationId }); @@ -223,7 +215,7 @@ export class ParseEventRequest { } } - private modifyAttachments(command: ParseEventRequestCommand) { + private modifyAttachments(command: ParseEventRequestCommand): void { command.payload.attachments = command.payload.attachments.map((attachment) => ({ ...attachment, name: attachment.name, @@ -232,52 +224,89 @@ export class ParseEventRequest { })); } - public getReservedVariablesTypes(template: NotificationTemplateEntity): TriggerContextTypeEnum[] { + private getReservedVariablesTypes(template: NotificationTemplateEntity): TriggerContextTypeEnum[] { const reservedVariables = template.triggers[0].reservedVariables; return reservedVariables?.map((reservedVariable) => reservedVariable.type) || []; } - public async sendInAppNudgeForTeamMemberInvite(command: ParseEventRequestCommand) { + @Instrument() + @CachedEntity({ + builder: (command: ParseEventRequestCommand) => + buildHasNotificationKey({ + _organizationId: command.organizationId, + }), + }) + private async getNotificationCount(command: ParseEventRequestCommand): Promise { + return await this.notificationRepository.count( + { + _organizationId: command.organizationId, + }, + 1 + ); + } + + @Instrument() + private async sendInAppNudgeForTeamMemberInvite(command: ParseEventRequestCommand): Promise { + const isEnabled = await this.getFeatureFlag.execute( + GetFeatureFlagCommand.create({ + key: FeatureFlagsKeysEnum.IS_TEAM_MEMBER_INVITE_NUDGE_ENABLED, + organizationId: command.organizationId, + userId: 'system', + environmentId: 'system', + }) + ); + + if (!isEnabled) return; + // check if this is first trigger - const notification = await this.notificationRepository.findOne({ - _organizationId: command.organizationId, - _environmentId: command.environmentId, - }); + const notificationCount = await this.getNotificationCount(command); + + if (notificationCount > 0) return; - if (notification) return; + // After the first trigger, we invalidate the cache to ensure the next event trigger + // will update the cache with a count of 1. + this.invalidateCacheService.invalidateByKey({ + key: buildHasNotificationKey({ + _organizationId: command.organizationId, + }), + }); // check if user is using personal email const user = await this.userRepository.findOne({ _id: command.userId, }); - if (this.checkEmail(user?.email)) return; + if (!user) throw new ApiException('User not found'); + + if (this.isBlockedEmail(user.email)) return; // check if organization has more than 1 member - const membersCount = await this.memberRepository.count({ - _organizationId: command.organizationId, - }); + const membersCount = await this.memberRepository.count( + { + _organizationId: command.organizationId, + }, + 2 + ); if (membersCount > 1) return; - if ((process.env.NODE_ENV === 'dev' || process.env.NODE_ENV === 'production') && process.env.NOVU_API_KEY) { + Logger.log('No notification found', LOG_CONTEXT); + + if (process.env.NOVU_API_KEY) { if (!command.payload[INVITE_TEAM_MEMBER_NUDGE_PAYLOAD_KEY]) { const novu = new Novu(process.env.NOVU_API_KEY); - novu.trigger( - process.env.NOVU_INVITE_TEAM_MEMBER_NUDGE_TRIGGER_IDENTIFIER || 'in-app-invite-team-member-nudge', - { - to: { - subscriberId: command.userId, - email: user?.email as string, - }, - payload: { - [INVITE_TEAM_MEMBER_NUDGE_PAYLOAD_KEY]: true, - webhookUrl: `${process.env.API_ROOT_URL}/v1/invites/webhook`, - }, - } - ); + await novu.trigger(process.env.NOVU_INVITE_TEAM_MEMBER_NUDGE_TRIGGER_IDENTIFIER, { + to: { + subscriberId: command.userId, + email: user?.email as string, + }, + payload: { + [INVITE_TEAM_MEMBER_NUDGE_PAYLOAD_KEY]: true, + webhookUrl: `${process.env.API_ROOT_URL}/v1/invites/webhook`, + }, + }); this.analyticsService.track('Invite Nudge Sent', command.userId, { _organization: command.organizationId, @@ -286,19 +315,19 @@ export class ParseEventRequest { } } - public checkEmail(email) { - const includedDomains = [ - '@gmail', - '@outlook', - '@yahoo', - '@icloud', - '@mail', - '@hotmail', - '@protonmail', - '@gmx', - '@novu', - ]; - - return includedDomains.some((domain) => email.includes(domain)); + private isBlockedEmail(email: string): boolean { + return BLOCKED_DOMAINS.some((domain) => email.includes(domain)); } } + +const BLOCKED_DOMAINS = [ + '@gmail', + '@outlook', + '@yahoo', + '@icloud', + '@mail', + '@hotmail', + '@protonmail', + '@gmx', + '@novu', +]; diff --git a/apps/api/src/app/invites/dtos/invite-member.dto.ts b/apps/api/src/app/invites/dtos/invite-member.dto.ts index fa69023b90e..d353853cd2b 100644 --- a/apps/api/src/app/invites/dtos/invite-member.dto.ts +++ b/apps/api/src/app/invites/dtos/invite-member.dto.ts @@ -1,7 +1,16 @@ -import { IsEmail, IsNotEmpty } from 'class-validator'; +import { IsEmail, IsNotEmpty, IsObject, ValidateNested } from 'class-validator'; +import { Type } from 'class-transformer'; +import { SubscriberEntity } from '@novu/dal'; export class InviteMemberDto { @IsEmail() @IsNotEmpty() email: string; } + +export class InviteWebhookDto { + @IsObject() + @ValidateNested() + @Type(() => SubscriberEntity) + subscriber: SubscriberEntity; +} diff --git a/apps/api/src/app/invites/invites.controller.ts b/apps/api/src/app/invites/invites.controller.ts index 3d7170077af..e9ab29f0c1c 100644 --- a/apps/api/src/app/invites/invites.controller.ts +++ b/apps/api/src/app/invites/invites.controller.ts @@ -20,7 +20,7 @@ import { UserSession } from '../shared/framework/user.decorator'; import { GetInviteCommand } from './usecases/get-invite/get-invite.command'; import { AcceptInviteCommand } from './usecases/accept-invite/accept-invite.command'; import { Roles } from '../auth/framework/roles.decorator'; -import { InviteMemberDto } from './dtos/invite-member.dto'; +import { InviteMemberDto, InviteWebhookDto } from './dtos/invite-member.dto'; import { InviteMemberCommand } from './usecases/invite-member/invite-member.command'; import { BulkInviteMembersDto } from './dtos/bulk-invite-members.dto'; import { BulkInviteCommand } from './usecases/bulk-invite/bulk-invite.command'; @@ -35,8 +35,8 @@ import { ApiExcludeController, ApiTags } from '@nestjs/swagger'; import { ThrottlerCost } from '../rate-limiting/guards'; import { ApiCommonResponses } from '../shared/framework/response.decorator'; import { UserAuthGuard } from '../auth/framework/user.auth.guard'; -import { InviteNudgeWebhookCommand } from './usecases/invite-nudge-webhook/invite-nudge-command'; -import { InviteNudgeWebhook } from './usecases/invite-nudge-webhook/invite-nudge-usecase'; +import { InviteNudgeWebhookCommand } from './usecases/invite-nudge/invite-nudge.command'; +import { InviteNudgeWebhook } from './usecases/invite-nudge/invite-nudge.usecase'; @UseInterceptors(ClassSerializerInterceptor) @ApiCommonResponses() @@ -134,10 +134,10 @@ export class InvitesController { } @Post('/webhook') - async inviteCheckWebhook(@Headers() headers: Record, @Body() body: Record) { + async inviteCheckWebhook(@Headers('nv-hmac-256') hmacHeader: string, @Body() body: InviteWebhookDto) { const command = InviteNudgeWebhookCommand.create({ - headers, - body, + hmacHeader, + subscriber: body.subscriber, }); const response = await this.inviteNudgeWebhookUsecase.execute(command); diff --git a/apps/api/src/app/invites/usecases/index.ts b/apps/api/src/app/invites/usecases/index.ts index d93100f3583..bb924b94162 100644 --- a/apps/api/src/app/invites/usecases/index.ts +++ b/apps/api/src/app/invites/usecases/index.ts @@ -3,6 +3,6 @@ import { GetInvite } from './get-invite/get-invite.usecase'; import { BulkInvite } from './bulk-invite/bulk-invite.usecase'; import { InviteMember } from './invite-member/invite-member.usecase'; import { ResendInvite } from './resend-invite/resend-invite.usecase'; -import { InviteNudgeWebhook } from './invite-nudge-webhook/invite-nudge-usecase'; +import { InviteNudgeWebhook } from './invite-nudge/invite-nudge.usecase'; export const USE_CASES = [AcceptInvite, GetInvite, BulkInvite, InviteMember, ResendInvite, InviteNudgeWebhook]; diff --git a/apps/api/src/app/invites/usecases/invite-nudge-webhook/invite-nudge-command.ts b/apps/api/src/app/invites/usecases/invite-nudge-webhook/invite-nudge-command.ts deleted file mode 100644 index 16510256985..00000000000 --- a/apps/api/src/app/invites/usecases/invite-nudge-webhook/invite-nudge-command.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { IsString } from 'class-validator'; -import { BaseCommand } from '../../../shared/commands/base.command'; - -export class InviteNudgeWebhookCommand extends BaseCommand { - headers?: Record; - - body?: Record; -} diff --git a/apps/api/src/app/invites/usecases/invite-nudge/invite-nudge.command.ts b/apps/api/src/app/invites/usecases/invite-nudge/invite-nudge.command.ts new file mode 100644 index 00000000000..4c9a4e66c7b --- /dev/null +++ b/apps/api/src/app/invites/usecases/invite-nudge/invite-nudge.command.ts @@ -0,0 +1,11 @@ +import { IsObject, IsString } from 'class-validator'; +import { SubscriberEntity } from '@novu/dal'; +import { BaseCommand } from '../../../shared/commands/base.command'; + +export class InviteNudgeWebhookCommand extends BaseCommand { + @IsString() + hmacHeader: string; + + @IsObject() + subscriber: SubscriberEntity; +} diff --git a/apps/api/src/app/invites/usecases/invite-nudge-webhook/invite-nudge-usecase.ts b/apps/api/src/app/invites/usecases/invite-nudge/invite-nudge.usecase.ts similarity index 70% rename from apps/api/src/app/invites/usecases/invite-nudge-webhook/invite-nudge-usecase.ts rename to apps/api/src/app/invites/usecases/invite-nudge/invite-nudge.usecase.ts index ebb23634915..be13d3de6fa 100644 --- a/apps/api/src/app/invites/usecases/invite-nudge-webhook/invite-nudge-usecase.ts +++ b/apps/api/src/app/invites/usecases/invite-nudge/invite-nudge.usecase.ts @@ -4,7 +4,7 @@ import { GetFeatureFlag, GetFeatureFlagCommand, createHash } from '@novu/applica import { FeatureFlagsKeysEnum } from '@novu/shared'; import axios from 'axios'; -import { InviteNudgeWebhookCommand } from './invite-nudge-command'; +import { InviteNudgeWebhookCommand } from './invite-nudge.command'; const axiosInstance = axios.create(); @@ -18,33 +18,29 @@ export class InviteNudgeWebhook { const isEnabled = await this.getFeatureFlag.execute( GetFeatureFlagCommand.create({ key: FeatureFlagsKeysEnum.IS_TEAM_MEMBER_INVITE_NUDGE_ENABLED, - organizationId: command.body?.subscriber?._organizationId, + organizationId: command.subscriber._organizationId, userId: 'system', environmentId: 'system', }) ); - if ( - isEnabled && - (process.env.NODE_ENV === 'dev' || process.env.NODE_ENV === 'production') && - process.env.NOVU_API_KEY - ) { - const hmacHash = createHash(process.env.NOVU_API_KEY || '', command?.body?.subscriber?._environmentId || ''); - const hmacHashFromWebhook = command?.headers?.['nv-hmac-256']; + if (isEnabled && process.env.NOVU_API_KEY) { + const hmacHash = createHash(process.env.NOVU_API_KEY, command.subscriber._environmentId); + const hmacHashFromWebhook = command.hmacHeader; if (hmacHash !== hmacHashFromWebhook) { throw new Error('Unauthorized request'); } const membersCount = await this.memberRepository.count({ - _organizationId: command?.body?.subscriber?._organizationId, + _organizationId: command.subscriber._organizationId, }); if (membersCount === 1) { await axiosInstance.post( `https://api.hubapi.com/contacts/v1/lists/${process.env.HUBSPOT_INVITE_NUDGE_EMAIL_USER_LIST_ID}/add`, { - emails: [command?.body?.subscriber?.email], + emails: [command.subscriber.email], }, { headers: { diff --git a/apps/api/src/types/env.d.ts b/apps/api/src/types/env.d.ts index 36d09996600..d0065b184cd 100644 --- a/apps/api/src/types/env.d.ts +++ b/apps/api/src/types/env.d.ts @@ -28,6 +28,9 @@ declare global { NOTIFICATION_RETENTION_DAYS?: number; MESSAGE_GENERIC_RETENTION_DAYS?: number; MESSAGE_IN_APP_RETENTION_DAYS?: number; + NOVU_INVITE_TEAM_MEMBER_NUDGE_TRIGGER_IDENTIFIER: string; + HUBSPOT_INVITE_NUDGE_EMAIL_USER_LIST_ID: string; + HUBSPOT_PRIVATE_APP_ACCESS_TOKEN: string; } } } diff --git a/libs/application-generic/src/services/cache/key-builders/entities.ts b/libs/application-generic/src/services/cache/key-builders/entities.ts index c09308a92c2..b38b9e44deb 100644 --- a/libs/application-generic/src/services/cache/key-builders/entities.ts +++ b/libs/application-generic/src/services/cache/key-builders/entities.ts @@ -143,6 +143,17 @@ export const buildEvaluateApiRateLimitKey = ({ identifier: apiRateLimitCategory, }); +export const buildHasNotificationKey = ({ + _organizationId, +}: { + _organizationId: string; +}): string => + buildOrganizationScopedKey({ + type: CacheKeyTypeEnum.ENTITY, + keyEntity: CacheKeyPrefixEnum.HAS_NOTIFICATION, + organizationId: _organizationId, + }); + export const buildUsageKey = ({ _organizationId, resourceType, diff --git a/libs/application-generic/src/services/cache/key-builders/identifiers.ts b/libs/application-generic/src/services/cache/key-builders/identifiers.ts index 5500fc21341..aa942a6fa37 100644 --- a/libs/application-generic/src/services/cache/key-builders/identifiers.ts +++ b/libs/application-generic/src/services/cache/key-builders/identifiers.ts @@ -28,6 +28,7 @@ export enum CacheKeyPrefixEnum { SERVICE_CONFIG = 'service_config', SUBSCRIPTION = 'subscription', USAGE = 'usage', + HAS_NOTIFICATION = 'has_notification', } /** diff --git a/libs/dal/src/repositories/user/user.entity.ts b/libs/dal/src/repositories/user/user.entity.ts index 8f4f6b34471..157a6e73a61 100644 --- a/libs/dal/src/repositories/user/user.entity.ts +++ b/libs/dal/src/repositories/user/user.entity.ts @@ -26,11 +26,11 @@ export class UserEntity { resetTokenCount?: IUserResetTokenCount; - firstName?: string | null; + firstName: string; lastName?: string | null; - email?: string | null; + email: string; profilePicture?: string | null;