From bb2da97dd1112914381152cd81226b997bd7d9ee Mon Sep 17 00:00:00 2001 From: Denis Kralj Date: Fri, 3 May 2024 18:42:06 +0200 Subject: [PATCH 01/10] refactor: Remove unused feature flag Remove unused injections Move single use helper to class using it Remove unused imports Tidy up imports --- apps/api/src/app/events/events.controller.ts | 4 ++-- apps/api/src/app/events/events.module.ts | 1 - .../trigger-broadcast.usecase.ts | 8 -------- .../trigger-event/trigger-event.usecase.ts | 17 ++++++++++++++--- .../trigger-multicast.usecase.ts | 1 - 5 files changed, 16 insertions(+), 15 deletions(-) diff --git a/apps/api/src/app/events/events.controller.ts b/apps/api/src/app/events/events.controller.ts index 72fdf9b6f61..50a132226e1 100644 --- a/apps/api/src/app/events/events.controller.ts +++ b/apps/api/src/app/events/events.controller.ts @@ -1,6 +1,6 @@ +import { v4 as uuidv4 } from 'uuid'; import { Body, Controller, Delete, Param, Post, Scope, UseGuards } from '@nestjs/common'; import { ApiExcludeEndpoint, ApiOperation, ApiTags } from '@nestjs/swagger'; -import { v4 as uuidv4 } from 'uuid'; import { AddressingTypeEnum, ApiRateLimitCategoryEnum, @@ -21,6 +21,7 @@ import { CancelDelayed, CancelDelayedCommand } from './usecases/cancel-delayed'; import { ParseEventRequest, ParseEventRequestMulticastCommand } from './usecases/parse-event-request'; import { ProcessBulkTrigger, ProcessBulkTriggerCommand } from './usecases/process-bulk-trigger'; import { TriggerEventToAll, TriggerEventToAllCommand } from './usecases/trigger-event-to-all'; +import { SendTestEmail, SendTestEmailCommand } from './usecases/send-test-email'; import { UserSession } from '../shared/framework/user.decorator'; import { ExternalApiAccessible } from '../auth/framework/external-api.decorator'; @@ -28,7 +29,6 @@ import { UserAuthGuard } from '../auth/framework/user.auth.guard'; import { ApiCommonResponses, ApiResponse, ApiOkResponse } from '../shared/framework/response.decorator'; import { DataBooleanDto } from '../shared/dtos/data-wrapper-dto'; import { ThrottlerCategory, ThrottlerCost } from '../rate-limiting/guards'; -import { SendTestEmail, SendTestEmailCommand } from './usecases/send-test-email'; import { ResourceCategory } from '../resource-limiting/guards'; @ThrottlerCategory(ApiRateLimitCategoryEnum.TRIGGER) diff --git a/apps/api/src/app/events/events.module.ts b/apps/api/src/app/events/events.module.ts index 5d11d1ec0f1..fe9c4567af8 100644 --- a/apps/api/src/app/events/events.module.ts +++ b/apps/api/src/app/events/events.module.ts @@ -22,7 +22,6 @@ import { ExecutionDetailsModule } from '../execution-details/execution-details.m import { TopicsModule } from '../topics/topics.module'; import { LayoutsModule } from '../layouts/layouts.module'; import { TenantModule } from '../tenant/tenant.module'; -import { JobTopicNameEnum } from '@novu/shared'; import { SendTestEmail } from './usecases/send-test-email'; const PROVIDERS = [ diff --git a/packages/application-generic/src/usecases/trigger-broadcast/trigger-broadcast.usecase.ts b/packages/application-generic/src/usecases/trigger-broadcast/trigger-broadcast.usecase.ts index 3279d27dfd8..632c6e44be6 100644 --- a/packages/application-generic/src/usecases/trigger-broadcast/trigger-broadcast.usecase.ts +++ b/packages/application-generic/src/usecases/trigger-broadcast/trigger-broadcast.usecase.ts @@ -15,16 +15,12 @@ import { SubscriberSourceEnum, } from '@novu/shared'; -import { ProcessSubscriber } from '../process-subscriber'; -import { PinoLogger } from '../../logging'; import { Instrument, InstrumentUsecase } from '../../instrumentation'; import { buildNotificationTemplateIdentifierKey, CachedEntity, } from '../../services/cache'; import { ApiException } from '../../utils/exceptions'; -import { ProcessTenant } from '../process-tenant'; -import { MapTriggerRecipients } from '../map-trigger-recipients/map-trigger-recipients.use-case'; import { SubscriberProcessQueueService } from '../../services/queues/subscriber-process-queue.service'; import { TriggerBroadcastCommand } from './trigger-broadcast.command'; import { IProcessSubscriberBulkJobDto } from '../../dtos'; @@ -35,14 +31,10 @@ const QUEUE_CHUNK_SIZE = Number(process.env.BROADCAST_QUEUE_CHUNK_SIZE) || 100; @Injectable() export class TriggerBroadcast { constructor( - private processSubscriber: ProcessSubscriber, private integrationRepository: IntegrationRepository, private subscriberRepository: SubscriberRepository, private jobRepository: JobRepository, private notificationTemplateRepository: NotificationTemplateRepository, - private processTenant: ProcessTenant, - private logger: PinoLogger, - private mapTriggerRecipients: MapTriggerRecipients, private subscriberProcessQueueService: SubscriberProcessQueueService ) {} diff --git a/packages/application-generic/src/usecases/trigger-event/trigger-event.usecase.ts b/packages/application-generic/src/usecases/trigger-event/trigger-event.usecase.ts index fca542a7bb8..9fb470b0a65 100644 --- a/packages/application-generic/src/usecases/trigger-event/trigger-event.usecase.ts +++ b/packages/application-generic/src/usecases/trigger-event/trigger-event.usecase.ts @@ -14,6 +14,7 @@ import { ISubscribersDefine, ITenantDefine, ProvidersIdEnum, + TriggerRecipientSubscriber, TriggerTenantContext, } from '@novu/shared'; @@ -30,7 +31,6 @@ import { } from '../../services/cache'; import { ApiException } from '../../utils/exceptions'; import { ProcessTenant, ProcessTenantCommand } from '../process-tenant'; -import { MapTriggerRecipients } from '../map-trigger-recipients/map-trigger-recipients.use-case'; import { TriggerBroadcast } from '../trigger-broadcast/trigger-broadcast.usecase'; import { TriggerBroadcastCommand } from '../trigger-broadcast/trigger-broadcast.command'; import { @@ -49,7 +49,6 @@ export class TriggerEvent { private notificationTemplateRepository: NotificationTemplateRepository, private processTenant: ProcessTenant, private logger: PinoLogger, - private mapTriggerRecipients: MapTriggerRecipients, private triggerBroadcast: TriggerBroadcast, private triggerMulticast: TriggerMulticast ) {} @@ -60,7 +59,7 @@ export class TriggerEvent { const mappedCommand = { ...command, tenant: this.mapTenant(command.tenant), - actor: this.mapTriggerRecipients.mapActor(command.actor), + actor: this.mapActor(command.actor), }; Logger.debug(mappedCommand.actor); @@ -271,4 +270,16 @@ export class TriggerEvent { return tenant; } + + private mapActor( + subscriber: TriggerRecipientSubscriber + ): ISubscribersDefine | null { + if (!subscriber) return null; + + if (typeof subscriber === 'string') { + return { subscriberId: subscriber }; + } + + return subscriber; + } } diff --git a/packages/application-generic/src/usecases/trigger-multicast/trigger-multicast.usecase.ts b/packages/application-generic/src/usecases/trigger-multicast/trigger-multicast.usecase.ts index f8c0ae1f8ca..f521a781421 100644 --- a/packages/application-generic/src/usecases/trigger-multicast/trigger-multicast.usecase.ts +++ b/packages/application-generic/src/usecases/trigger-multicast/trigger-multicast.usecase.ts @@ -39,7 +39,6 @@ const isTopic = (recipient: TriggerRecipient): recipient is ITopic => @Injectable() export class TriggerMulticast { constructor( - private logger: PinoLogger, private subscriberProcessQueueService: SubscriberProcessQueueService, private topicSubscribersRepository: TopicSubscribersRepository, private topicRepository: TopicRepository, From f57217334a7d2d03648d1cb5cfe6adcee1066db8 Mon Sep 17 00:00:00 2001 From: Denis Kralj Date: Fri, 3 May 2024 20:19:59 +0200 Subject: [PATCH 02/10] refactor: Remove unused feature flag Remove map-trigger-recipients use case Remove associated e2e tests Remove setting of feature flag in e2e tests Remove feature flag from env validator Remove feature flag from env configurations Remove feature flag from list of feature flags Remove feature flag read and false handling from trigger-multicast use case --- apps/api/src/.env.development | 1 - apps/api/src/.env.production | 1 - apps/api/src/.env.test | 2 - apps/api/src/.example.env | 1 - .../events/e2e/map-trigger-recipients.e2e.ts | 670 ------------------ .../app/events/e2e/trigger-event-topic.e2e.ts | 4 - .../app/events/e2e/trigger-multicast.e2e.ts | 1 - apps/api/src/config/env-validator.ts | 5 - apps/worker/src/.env.test | 1 - .../src/types/feature-flags/feature-flags.ts | 1 - packages/application-generic/src/.env.test | 2 - .../usecases/map-trigger-recipients/index.ts | 2 - .../map-trigger-recipients.command.ts | 14 - .../map-trigger-recipients.use-case.ts | 197 ----- .../trigger-multicast.usecase.ts | 18 - 15 files changed, 920 deletions(-) delete mode 100644 apps/api/src/app/events/e2e/map-trigger-recipients.e2e.ts delete mode 100644 packages/application-generic/src/usecases/map-trigger-recipients/index.ts delete mode 100644 packages/application-generic/src/usecases/map-trigger-recipients/map-trigger-recipients.command.ts delete mode 100644 packages/application-generic/src/usecases/map-trigger-recipients/map-trigger-recipients.use-case.ts diff --git a/apps/api/src/.env.development b/apps/api/src/.env.development index 4156ccbd790..07d0e6f88af 100644 --- a/apps/api/src/.env.development +++ b/apps/api/src/.env.development @@ -46,7 +46,6 @@ LOGGING_LEVEL=info VERCEL_REDIRECT_URI=https://dev.web.novu.co/auth/login VERCEL_BASE_URL=https://api.vercel.com -IS_TOPIC_NOTIFICATION_ENABLED=true FF_IS_DISTRIBUTED_LOCK_LOGGING_ENABLED=false IS_TRANSLATION_MANAGER_ENABLED=false diff --git a/apps/api/src/.env.production b/apps/api/src/.env.production index 4a2ba2f9fe4..4795f07677a 100644 --- a/apps/api/src/.env.production +++ b/apps/api/src/.env.production @@ -33,7 +33,6 @@ LOGGING_LEVEL=info VERCEL_REDIRECT_URI=https://web.novu.co/auth/login VERCEL_BASE_URL=https://api.vercel.com -IS_TOPIC_NOTIFICATION_ENABLED=true FF_IS_DISTRIBUTED_LOCK_LOGGING_ENABLED=false IS_TRANSLATION_MANAGER_ENABLED=false diff --git a/apps/api/src/.env.test b/apps/api/src/.env.test index f0d92bfb6bf..125aa6deca6 100644 --- a/apps/api/src/.env.test +++ b/apps/api/src/.env.test @@ -73,8 +73,6 @@ VERCEL_CLIENT_SECRET= VERCEL_REDIRECT_URI=http://127.0.0.1:4200/auth/login VERCEL_BASE_URL=https://api.vercel.com -IS_TOPIC_NOTIFICATION_ENABLED=true - STORE_NOTIFICATION_CONTENT=true MAX_NOVU_INTEGRATION_MAIL_REQUESTS=300 diff --git a/apps/api/src/.example.env b/apps/api/src/.example.env index 347bc9201f4..c9de1be0225 100644 --- a/apps/api/src/.example.env +++ b/apps/api/src/.example.env @@ -54,7 +54,6 @@ VERCEL_CLIENT_SECRET= VERCEL_REDIRECT_URI=http://127.0.0.1:4200/auth/login VERCEL_BASE_URL=https://api.vercel.com -IS_TOPIC_NOTIFICATION_ENABLED=true IS_TRANSLATION_MANAGER_ENABLED=false STORE_NOTIFICATION_CONTENT=true diff --git a/apps/api/src/app/events/e2e/map-trigger-recipients.e2e.ts b/apps/api/src/app/events/e2e/map-trigger-recipients.e2e.ts deleted file mode 100644 index 95aeac3fc30..00000000000 --- a/apps/api/src/app/events/e2e/map-trigger-recipients.e2e.ts +++ /dev/null @@ -1,670 +0,0 @@ -import { Test } from '@nestjs/testing'; -import { SubscribersService, UserSession } from '@novu/testing'; -import { - FeatureFlagsService, - GetTopicSubscribersUseCase, - MapTriggerRecipients, - MapTriggerRecipientsCommand, -} from '@novu/application-generic'; -import { - SubscriberEntity, - SubscriberRepository, - TopicEntity, - TopicRepository, - CreateTopicSubscribersEntity, - TopicSubscribersRepository, -} from '@novu/dal'; -import { - ISubscribersDefine, - ITopic, - SubscriberSourceEnum, - TopicId, - TopicKey, - TopicName, - TriggerRecipientsPayload, - TriggerRecipientsTypeEnum, -} from '@novu/shared'; -import { expect } from 'chai'; -import { v4 as uuid } from 'uuid'; -import { SharedModule } from '../../shared/shared.module'; -import { EventsModule } from '../events.module'; - -const originalLaunchDarklySdkKey = process.env.LAUNCH_DARKLY_SDK_KEY; - -describe('MapTriggerRecipientsUseCase', () => { - let session: UserSession; - let subscribersService: SubscribersService; - let topicRepository: TopicRepository; - let topicSubscribersRepository: TopicSubscribersRepository; - let useCase: MapTriggerRecipients; - - describe('When feature disabled', () => { - before(async () => { - const featureFlagsService = new FeatureFlagsService(); - await featureFlagsService.initialize(); - - process.env.LAUNCH_DARKLY_SDK_KEY = ''; - process.env.IS_TOPIC_NOTIFICATION_ENABLED = 'false'; - - const moduleRef = await Test.createTestingModule({ - imports: [SharedModule, EventsModule], - providers: [MapTriggerRecipients, GetTopicSubscribersUseCase], - }).compile(); - - session = new UserSession(); - await session.initialize(); - - useCase = moduleRef.get(MapTriggerRecipients); - subscribersService = new SubscribersService(session.organization._id, session.environment._id); - topicRepository = new TopicRepository(); - topicSubscribersRepository = new TopicSubscribersRepository(); - }); - - after(() => { - process.env.LAUNCH_DARKLY_SDK_KEY = originalLaunchDarklySdkKey; - }); - - it('should map properly a single subscriber id as string', async () => { - const transactionId = uuid(); - const subscriberId = SubscriberRepository.createObjectId(); - - const command = buildCommand(session, transactionId, subscriberId); - const result = await useCase.execute(command); - - expect(result).to.be.eql([{ subscriberId, _subscriberSource: SubscriberSourceEnum.SINGLE }]); - }); - - it('should map properly a single subscriber defined payload', async () => { - const transactionId = uuid(); - - const subscriberId = SubscriberRepository.createObjectId(); - const recipient: ISubscribersDefine = { - subscriberId, - firstName: 'Test Name', - lastName: 'Last of name', - email: 'test@email.novu', - }; - - const command = buildCommand(session, transactionId, recipient); - - const result = await useCase.execute(command); - - expect(result).to.be.eql([{ ...recipient, _subscriberSource: SubscriberSourceEnum.SINGLE }]); - }); - - it('should only process the subscriber id and the subscriber recipients and ignore topics', async () => { - const firstTopicKey = 'topic-key-mixed-recipients-1'; - const firstTopicName = 'topic-key-mixed-recipients-1-name'; - const secondTopicKey = 'topic-key-mixed-recipients-2'; - const secondTopicName = 'topic-key-mixed-recipients-2-name'; - const transactionId = uuid(); - - const firstTopic = await createTopicEntity( - session, - topicRepository, - topicSubscribersRepository, - firstTopicKey, - firstTopicName - ); - const firstTopicId = firstTopic._id; - const firstSubscriber = await subscribersService.createSubscriber(); - const secondSubscriber = await subscribersService.createSubscriber(); - await addSubscribersToTopic(session, topicRepository, topicSubscribersRepository, firstTopicId, firstTopicKey, [ - firstSubscriber, - secondSubscriber, - ]); - - const secondTopic = await createTopicEntity( - session, - topicRepository, - topicSubscribersRepository, - secondTopicKey, - secondTopicName - ); - const secondTopicId = secondTopic._id; - const thirdSubscriber = await subscribersService.createSubscriber(); - await addSubscribersToTopic(session, topicRepository, topicSubscribersRepository, secondTopicId, secondTopicKey, [ - thirdSubscriber, - ]); - - const firstTopicRecipient: ITopic = { - type: TriggerRecipientsTypeEnum.TOPIC, - topicKey: firstTopic._id, - }; - const secondTopicRecipient: ITopic = { - type: TriggerRecipientsTypeEnum.TOPIC, - topicKey: secondTopic._id, - }; - - const singleSubscriberId = SubscriberRepository.createObjectId(); - const subscribersDefineSubscriberId = SubscriberRepository.createObjectId(); - const singleSubscribersDefine: ISubscribersDefine = { - subscriberId: subscribersDefineSubscriberId, - firstName: 'Test Name', - lastName: 'Last of name', - email: 'test@email.novu', - }; - - const recipients = [firstTopicRecipient, singleSubscriberId, secondTopicRecipient, singleSubscribersDefine]; - - const command = buildCommand(session, transactionId, recipients); - - const result = await useCase.execute(command); - - expect(result).to.be.eql([ - { subscriberId: singleSubscriberId, _subscriberSource: SubscriberSourceEnum.SINGLE }, - { ...singleSubscribersDefine, _subscriberSource: SubscriberSourceEnum.SINGLE }, - ]); - }); - - it('should map properly multiple duplicated recipients of different types and deduplicate them', async () => { - const transactionId = uuid(); - const firstSubscriberId = SubscriberRepository.createObjectId(); - const secondSubscriberId = SubscriberRepository.createObjectId(); - - const firstRecipient: ISubscribersDefine = { - subscriberId: firstSubscriberId, - firstName: 'Test Name', - lastName: 'Last of name', - email: 'test@email.novu', - }; - - const secondRecipient: ISubscribersDefine = { - subscriberId: secondSubscriberId, - firstName: 'Test Name', - lastName: 'Last of name', - email: 'test@email.novu', - }; - - const command = buildCommand(session, transactionId, [ - firstSubscriberId, - secondSubscriberId, - firstRecipient, - secondRecipient, - secondSubscriberId, - firstSubscriberId, - ]); - const result = await useCase.execute(command); - - expect(result).to.be.eql([ - { subscriberId: firstSubscriberId, _subscriberSource: SubscriberSourceEnum.SINGLE }, - { subscriberId: secondSubscriberId, _subscriberSource: SubscriberSourceEnum.SINGLE }, - ]); - }); - }); - - describe('When feature enabled', () => { - before(async () => { - process.env.LAUNCH_DARKLY_SDK_KEY = ''; - process.env.IS_TOPIC_NOTIFICATION_ENABLED = 'true'; - - const moduleRef = await Test.createTestingModule({ - imports: [SharedModule, EventsModule], - providers: [MapTriggerRecipients, GetTopicSubscribersUseCase], - }).compile(); - - session = new UserSession(); - await session.initialize(); - - useCase = moduleRef.get(MapTriggerRecipients, { strict: false }); - subscribersService = new SubscribersService(session.organization._id, session.environment._id); - topicRepository = new TopicRepository(); - topicSubscribersRepository = new TopicSubscribersRepository(); - }); - - after(() => { - process.env.LAUNCH_DARKLY_SDK_KEY = originalLaunchDarklySdkKey; - }); - - it('should map properly a single subscriber id as string', async () => { - const transactionId = uuid(); - const subscriberId = SubscriberRepository.createObjectId(); - - const command = buildCommand(session, transactionId, subscriberId); - const result = await useCase.execute(command); - - expect(result).to.be.eql([{ subscriberId, _subscriberSource: SubscriberSourceEnum.SINGLE }]); - }); - - it('should map properly a single subscriber defined payload', async () => { - const transactionId = uuid(); - - const subscriberId = SubscriberRepository.createObjectId(); - const recipient: ISubscribersDefine = { - subscriberId, - firstName: 'Test Name', - lastName: 'Last of name', - email: 'test@email.novu', - }; - - const command = buildCommand(session, transactionId, recipient); - - const result = await useCase.execute(command); - - expect(result).to.be.eql([{ ...recipient, _subscriberSource: SubscriberSourceEnum.SINGLE }]); - }); - - it('should map properly a single topic', async () => { - const topicKey = 'topic-key-single-recipient'; - const topicName = 'topic-key-single-recipient-name'; - const transactionId = uuid(); - - const topic = await createTopicEntity(session, topicRepository, topicSubscribersRepository, topicKey, topicName); - const topicId = topic._id; - const firstSubscriber = await subscribersService.createSubscriber(); - const secondSubscriber = await subscribersService.createSubscriber(); - await addSubscribersToTopic(session, topicRepository, topicSubscribersRepository, topicId, topicKey, [ - firstSubscriber, - secondSubscriber, - ]); - - const recipient: ITopic = { - type: TriggerRecipientsTypeEnum.TOPIC, - topicKey: topic.key, - }; - - const command = buildCommand(session, transactionId, [recipient]); - - const result = await useCase.execute(command); - - expect(result).to.include.deep.members([ - { subscriberId: firstSubscriber.subscriberId, _subscriberSource: SubscriberSourceEnum.TOPIC }, - { subscriberId: secondSubscriber.subscriberId, _subscriberSource: SubscriberSourceEnum.TOPIC }, - ]); - }); - - it('should throw an error if providing a topic that does not exist', async () => { - const topicKey = 'topic-key-single-recipient'; - const topicName = 'topic-key-single-recipient-name'; - const transactionId = uuid(); - - const recipient: ITopic = { - type: TriggerRecipientsTypeEnum.TOPIC, - topicKey: TopicRepository.createObjectId(), - }; - - const command = buildCommand(session, transactionId, [recipient]); - - let error; - - try { - const result = await useCase.execute(command); - } catch (e) { - error = e; - } - - expect(error).to.be.ok; - expect(error.message).to.contain('not found in current environment'); - }); - - it('should map properly a mixed recipients list with a string, a subscribers define interface and two topics', async () => { - const firstTopicKey = 'topic-key-mixed-recipients-1'; - const firstTopicName = 'topic-key-mixed-recipients-1-name'; - const secondTopicKey = 'topic-key-mixed-recipients-2'; - const secondTopicName = 'topic-key-mixed-recipients-2-name'; - const transactionId = uuid(); - - const firstTopic = await createTopicEntity( - session, - topicRepository, - topicSubscribersRepository, - firstTopicKey, - firstTopicName - ); - const firstTopicId = firstTopic._id; - const firstSubscriber = await subscribersService.createSubscriber(); - const secondSubscriber = await subscribersService.createSubscriber(); - await addSubscribersToTopic(session, topicRepository, topicSubscribersRepository, firstTopicId, firstTopicKey, [ - firstSubscriber, - secondSubscriber, - ]); - - const secondTopic = await createTopicEntity( - session, - topicRepository, - topicSubscribersRepository, - secondTopicKey, - secondTopicName - ); - const secondTopicId = secondTopic._id; - const thirdSubscriber = await subscribersService.createSubscriber(); - await addSubscribersToTopic(session, topicRepository, topicSubscribersRepository, secondTopicId, secondTopicKey, [ - thirdSubscriber, - ]); - - const firstTopicRecipient: ITopic = { - type: TriggerRecipientsTypeEnum.TOPIC, - topicKey: firstTopic.key, - }; - const secondTopicRecipient: ITopic = { - type: TriggerRecipientsTypeEnum.TOPIC, - topicKey: secondTopic.key, - }; - - const singleSubscriberId = SubscriberRepository.createObjectId(); - const subscribersDefineSubscriberId = SubscriberRepository.createObjectId(); - const singleSubscribersDefine: ISubscribersDefine = { - subscriberId: subscribersDefineSubscriberId, - firstName: 'Test Name', - lastName: 'Last of name', - email: 'test@email.novu', - }; - - const recipients = [firstTopicRecipient, singleSubscriberId, secondTopicRecipient, singleSubscribersDefine]; - - const command = buildCommand(session, transactionId, recipients); - - const result = await useCase.execute(command); - - expect(result).to.include.deep.members([ - { subscriberId: singleSubscriberId, _subscriberSource: SubscriberSourceEnum.SINGLE }, - { ...singleSubscribersDefine, _subscriberSource: SubscriberSourceEnum.SINGLE }, - { subscriberId: firstSubscriber.subscriberId, _subscriberSource: SubscriberSourceEnum.TOPIC }, - { subscriberId: secondSubscriber.subscriberId, _subscriberSource: SubscriberSourceEnum.TOPIC }, - { subscriberId: thirdSubscriber.subscriberId, _subscriberSource: SubscriberSourceEnum.TOPIC }, - ]); - }); - - it('should map properly multiple duplicated recipients of different types and deduplicate them', async () => { - const transactionId = uuid(); - const firstSubscriberId = SubscriberRepository.createObjectId(); - const secondSubscriberId = SubscriberRepository.createObjectId(); - - const firstRecipient: ISubscribersDefine = { - subscriberId: firstSubscriberId, - firstName: 'Test Name', - lastName: 'Last of name', - email: 'test@email.novu', - }; - - const secondRecipient: ISubscribersDefine = { - subscriberId: secondSubscriberId, - firstName: 'Test Name', - lastName: 'Last of name', - email: 'test@email.novu', - }; - - const command = buildCommand(session, transactionId, [ - firstSubscriberId, - secondSubscriberId, - firstRecipient, - secondRecipient, - secondSubscriberId, - firstSubscriberId, - ]); - const result = await useCase.execute(command); - - expect(result).to.be.eql([ - { subscriberId: firstSubscriberId, _subscriberSource: SubscriberSourceEnum.SINGLE }, - { subscriberId: secondSubscriberId, _subscriberSource: SubscriberSourceEnum.SINGLE }, - ]); - }); - - it('should map properly multiple duplicated recipients of different types and deduplicate them but with different order', async () => { - const transactionId = uuid(); - const firstSubscriberId = SubscriberRepository.createObjectId(); - const secondSubscriberId = SubscriberRepository.createObjectId(); - - const firstRecipient: ISubscribersDefine = { - subscriberId: firstSubscriberId, - firstName: 'Test Name', - lastName: 'Last of name', - email: 'test@email.novu', - }; - - const secondRecipient: ISubscribersDefine = { - subscriberId: secondSubscriberId, - firstName: 'Test Name', - lastName: 'Last of name', - email: 'test@email.novu', - }; - - const command = buildCommand(session, transactionId, [ - firstRecipient, - secondRecipient, - firstSubscriberId, - secondSubscriberId, - secondSubscriberId, - firstSubscriberId, - secondRecipient, - firstRecipient, - ]); - const result = await useCase.execute(command); - - expect(result).to.be.eql([ - { ...firstRecipient, _subscriberSource: SubscriberSourceEnum.SINGLE }, - { ...secondRecipient, _subscriberSource: SubscriberSourceEnum.SINGLE }, - ]); - }); - - it('should map properly multiple topics and deduplicate them', async () => { - const firstTopicKey = 'topic-key-mixed-topics-1'; - const firstTopicName = 'topic-key-mixed-topics-1-name'; - const secondTopicKey = 'topic-key-mixed-topics-2'; - const secondTopicName = 'topic-key-mixed-topics-2-name'; - const thirdTopicKey = 'topic-key-mixed-topics-3'; - const thirdTopicName = 'topic-key-mixed-topics-3-name'; - const transactionId = uuid(); - - const firstSubscriber = await subscribersService.createSubscriber(); - const secondSubscriber = await subscribersService.createSubscriber(); - const thirdSubscriber = await subscribersService.createSubscriber(); - const fourthSubscriber = await subscribersService.createSubscriber(); - - const firstTopic = await createTopicEntity( - session, - topicRepository, - topicSubscribersRepository, - firstTopicKey, - firstTopicName - ); - const firstTopicId = firstTopic._id; - await addSubscribersToTopic(session, topicRepository, topicSubscribersRepository, firstTopicId, firstTopicKey, [ - firstSubscriber, - secondSubscriber, - ]); - - const secondTopic = await createTopicEntity( - session, - topicRepository, - topicSubscribersRepository, - secondTopicKey, - secondTopicName - ); - const secondTopicId = secondTopic._id; - await addSubscribersToTopic(session, topicRepository, topicSubscribersRepository, secondTopicId, secondTopicKey, [ - thirdSubscriber, - ]); - - const thirdTopic = await createTopicEntity( - session, - topicRepository, - topicSubscribersRepository, - thirdTopicKey, - thirdTopicName - ); - const thirdTopicId = thirdTopic._id; - await addSubscribersToTopic(session, topicRepository, topicSubscribersRepository, thirdTopicId, thirdTopicKey, [ - firstSubscriber, - fourthSubscriber, - ]); - - const firstTopicRecipient: ITopic = { - type: TriggerRecipientsTypeEnum.TOPIC, - topicKey: firstTopic.key, - }; - const secondTopicRecipient: ITopic = { - type: TriggerRecipientsTypeEnum.TOPIC, - topicKey: secondTopic.key, - }; - const thirdTopicRecipient: ITopic = { - type: TriggerRecipientsTypeEnum.TOPIC, - topicKey: thirdTopic.key, - }; - - const command = buildCommand(session, transactionId, [ - thirdTopicRecipient, - firstTopicRecipient, - secondTopicRecipient, - thirdTopicRecipient, - secondTopicRecipient, - firstTopicRecipient, - ]); - const result = await useCase.execute(command); - - expect(result).to.include.deep.members([ - { subscriberId: firstSubscriber.subscriberId, _subscriberSource: SubscriberSourceEnum.TOPIC }, - { subscriberId: fourthSubscriber.subscriberId, _subscriberSource: SubscriberSourceEnum.TOPIC }, - { subscriberId: secondSubscriber.subscriberId, _subscriberSource: SubscriberSourceEnum.TOPIC }, - { subscriberId: thirdSubscriber.subscriberId, _subscriberSource: SubscriberSourceEnum.TOPIC }, - ]); - }); - - it('should map properly multiple duplicated recipients of different types and topics and deduplicate them', async () => { - const firstTopicKey = 'topic-key-mixed-recipients-deduplication-1'; - const firstTopicName = 'topic-key-mixed-recipients-deduplication-1-name'; - const secondTopicKey = 'topic-key-mixed-recipients-deduplication-2'; - const secondTopicName = 'topic-key-mixed-recipients-deduplication-2-name'; - const transactionId = uuid(); - - const firstSubscriber = await subscribersService.createSubscriber(); - const secondSubscriber = await subscribersService.createSubscriber(); - const thirdSubscriber = await subscribersService.createSubscriber(); - - const firstRecipient: ISubscribersDefine = { - subscriberId: firstSubscriber.subscriberId, - firstName: 'Test Name', - lastName: 'Last of name', - email: 'test@email.novu', - }; - - const secondRecipient: ISubscribersDefine = { - subscriberId: secondSubscriber.subscriberId, - firstName: 'Test Name', - lastName: 'Last of name', - email: 'test@email.novu', - }; - - const firstTopic = await createTopicEntity( - session, - topicRepository, - topicSubscribersRepository, - firstTopicKey, - firstTopicName - ); - const firstTopicId = firstTopic._id; - await addSubscribersToTopic(session, topicRepository, topicSubscribersRepository, firstTopicId, firstTopicKey, [ - firstSubscriber, - secondSubscriber, - ]); - - const secondTopic = await createTopicEntity( - session, - topicRepository, - topicSubscribersRepository, - secondTopicKey, - secondTopicName - ); - const secondTopicId = secondTopic._id; - await addSubscribersToTopic(session, topicRepository, topicSubscribersRepository, secondTopicId, secondTopicKey, [ - thirdSubscriber, - ]); - - const firstTopicRecipient: ITopic = { - type: TriggerRecipientsTypeEnum.TOPIC, - topicKey: firstTopic.key, - }; - const secondTopicRecipient: ITopic = { - type: TriggerRecipientsTypeEnum.TOPIC, - topicKey: secondTopic.key, - }; - - const command = buildCommand(session, transactionId, [ - secondTopicRecipient, - firstRecipient, - firstSubscriber.subscriberId, - secondSubscriber.subscriberId, - firstTopicRecipient, - secondRecipient, - thirdSubscriber.subscriberId, - ]); - const result = await useCase.execute(command); - - expect(result.length).to.equal(3); - - // We process first recipients that are not topics, so they will take precedence when deduplicating - expect(result).to.include.deep.members([ - { ...firstRecipient, _subscriberSource: SubscriberSourceEnum.SINGLE }, - { subscriberId: secondSubscriber.subscriberId, _subscriberSource: SubscriberSourceEnum.SINGLE }, - { subscriberId: thirdSubscriber.subscriberId, _subscriberSource: SubscriberSourceEnum.SINGLE }, - ]); - }); - }); -}); - -const createTopicEntity = async ( - session: UserSession, - topicRepository: TopicRepository, - topicSubscribersRepository: TopicSubscribersRepository, - topicKey: TopicKey, - topicName: TopicName -): Promise => { - const environmentId = session.environment._id; - const organizationId = session.organization._id; - - const topicEntity = { - _environmentId: environmentId, - key: topicKey, - name: topicName, - _organizationId: organizationId, - }; - const topic = await topicRepository.create(topicEntity); - - expect(topic).to.exist; - expect(topic.key).to.be.eql(topicKey); - expect(topic.name).to.be.eql(topicName); - - return topic; -}; - -const addSubscribersToTopic = async ( - session: UserSession, - topicRepository: TopicRepository, - topicSubscribersRepository: TopicSubscribersRepository, - topicId: TopicId, - topicKey: TopicKey, - subscribers: SubscriberEntity[] -): Promise => { - const _environmentId = session.environment._id; - const _organizationId = session.organization._id; - const _topicId = topicId; - - const entities: CreateTopicSubscribersEntity[] = subscribers.map((subscriber) => ({ - _environmentId, - _organizationId, - _subscriberId: subscriber._id, - _topicId, - topicKey, - externalSubscriberId: subscriber.subscriberId, - })); - await topicSubscribersRepository.addSubscribers(entities); - - const result = await topicRepository.findTopic(topicKey, _environmentId); - - expect(result?.subscribers.length).to.be.eql(subscribers.length); - expect(result?.subscribers).to.have.members(subscribers.map((subscriber) => subscriber.subscriberId)); -}; - -const buildCommand = ( - session: UserSession, - transactionId: string, - recipients: TriggerRecipientsPayload -): MapTriggerRecipientsCommand => { - return MapTriggerRecipientsCommand.create({ - organizationId: session.organization._id, - environmentId: session.environment._id, - recipients, - transactionId, - userId: session.user._id, - }); -}; diff --git a/apps/api/src/app/events/e2e/trigger-event-topic.e2e.ts b/apps/api/src/app/events/e2e/trigger-event-topic.e2e.ts index d355834d34d..1c8ca26422c 100644 --- a/apps/api/src/app/events/e2e/trigger-event-topic.e2e.ts +++ b/apps/api/src/app/events/e2e/trigger-event-topic.e2e.ts @@ -49,7 +49,6 @@ describe('Topic Trigger Event', () => { beforeEach(async () => { process.env.LAUNCH_DARKLY_SDK_KEY = ''; - process.env.IS_TOPIC_NOTIFICATION_ENABLED = 'true'; session = new UserSession(); await session.initialize(); @@ -70,7 +69,6 @@ describe('Topic Trigger Event', () => { afterEach(() => { process.env.LAUNCH_DARKLY_SDK_KEY = originalLaunchDarklySdkKey; - process.env.IS_TOPIC_NOTIFICATION_ENABLED = 'false'; }); it('should trigger an event successfully', async () => { @@ -323,7 +321,6 @@ describe('Topic Trigger Event', () => { beforeEach(async () => { process.env.LAUNCH_DARKLY_SDK_KEY = ''; - process.env.IS_TOPIC_NOTIFICATION_ENABLED = 'true'; session = new UserSession(); await session.initialize(); @@ -377,7 +374,6 @@ describe('Topic Trigger Event', () => { afterEach(() => { process.env.LAUNCH_DARKLY_SDK_KEY = originalLaunchDarklySdkKey; - process.env.IS_TOPIC_NOTIFICATION_ENABLED = 'false'; }); it('should trigger an event successfully', async () => { diff --git a/apps/api/src/app/events/e2e/trigger-multicast.e2e.ts b/apps/api/src/app/events/e2e/trigger-multicast.e2e.ts index 48308f40524..c84d825f7ad 100644 --- a/apps/api/src/app/events/e2e/trigger-multicast.e2e.ts +++ b/apps/api/src/app/events/e2e/trigger-multicast.e2e.ts @@ -141,7 +141,6 @@ describe('TriggerMulticast', () => { beforeEach(async () => { process.env.LAUNCH_DARKLY_SDK_KEY = ''; - process.env.IS_TOPIC_NOTIFICATION_ENABLED = 'true'; session = new UserSession(); await session.initialize(); template = await session.createTemplate(); diff --git a/apps/api/src/config/env-validator.ts b/apps/api/src/config/env-validator.ts index 02afe9b1a7f..6952a1c5ba9 100644 --- a/apps/api/src/config/env-validator.ts +++ b/apps/api/src/config/env-validator.ts @@ -51,11 +51,6 @@ const validators: { [K in keyof any]: ValidatorSpec } = { NEW_RELIC_LICENSE_KEY: str({ default: '', }), - IS_TOPIC_NOTIFICATION_ENABLED: bool({ - desc: 'This is the environment variable used to enable the feature to send notifications to a topic', - default: true, - choices: [false, true], - }), REDIS_CACHE_SERVICE_HOST: str({ default: '', }), diff --git a/apps/worker/src/.env.test b/apps/worker/src/.env.test index f7eea5a5d35..75042df4b1b 100644 --- a/apps/worker/src/.env.test +++ b/apps/worker/src/.env.test @@ -79,7 +79,6 @@ LAUNCH_DARKLY_SDK_KEY= AUTO_CREATE_INDEXES=true IS_USE_MERGED_DIGEST_ID_ENABLED=true -IS_TOPIC_NOTIFICATION_ENABLED=true BROADCAST_QUEUE_CHUNK_SIZE=100 MULTICAST_QUEUE_CHUNK_SIZE=100 diff --git a/libs/shared/src/types/feature-flags/feature-flags.ts b/libs/shared/src/types/feature-flags/feature-flags.ts index 767490c7be0..a47f3a1ec64 100644 --- a/libs/shared/src/types/feature-flags/feature-flags.ts +++ b/libs/shared/src/types/feature-flags/feature-flags.ts @@ -1,6 +1,5 @@ export enum FeatureFlagsKeysEnum { IS_TEMPLATE_STORE_ENABLED = 'IS_TEMPLATE_STORE_ENABLED', - IS_TOPIC_NOTIFICATION_ENABLED = 'IS_TOPIC_NOTIFICATION_ENABLED', IS_MULTI_TENANCY_ENABLED = 'IS_MULTI_TENANCY_ENABLED', IS_USE_MERGED_DIGEST_ID_ENABLED = 'IS_USE_MERGED_DIGEST_ID_ENABLED', IS_API_RATE_LIMITING_ENABLED = 'IS_API_RATE_LIMITING_ENABLED', diff --git a/packages/application-generic/src/.env.test b/packages/application-generic/src/.env.test index b5ae17df600..d59f3dde7c8 100644 --- a/packages/application-generic/src/.env.test +++ b/packages/application-generic/src/.env.test @@ -74,8 +74,6 @@ VERCEL_CLIENT_SECRET= VERCEL_REDIRECT_URI=http://127.0.0.1:4200/auth/login VERCEL_BASE_URL=https://api.vercel.com -IS_TOPIC_NOTIFICATION_ENABLED=true - STORE_NOTIFICATION_CONTENT=true MAX_NOVU_INTEGRATION_MAIL_REQUESTS=300 diff --git a/packages/application-generic/src/usecases/map-trigger-recipients/index.ts b/packages/application-generic/src/usecases/map-trigger-recipients/index.ts deleted file mode 100644 index 9594dba6d7b..00000000000 --- a/packages/application-generic/src/usecases/map-trigger-recipients/index.ts +++ /dev/null @@ -1,2 +0,0 @@ -export * from './map-trigger-recipients.command'; -export * from './map-trigger-recipients.use-case'; diff --git a/packages/application-generic/src/usecases/map-trigger-recipients/map-trigger-recipients.command.ts b/packages/application-generic/src/usecases/map-trigger-recipients/map-trigger-recipients.command.ts deleted file mode 100644 index d578a27d539..00000000000 --- a/packages/application-generic/src/usecases/map-trigger-recipients/map-trigger-recipients.command.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { IsDefined, IsOptional } from 'class-validator'; -import { ISubscribersDefine, TriggerRecipientsPayload } from '@novu/shared'; -import { EnvironmentWithUserCommand } from '../../commands'; - -export class MapTriggerRecipientsCommand extends EnvironmentWithUserCommand { - @IsDefined() - recipients: TriggerRecipientsPayload; - - @IsDefined() - transactionId: string; - - @IsOptional() - actor?: ISubscribersDefine | null; -} diff --git a/packages/application-generic/src/usecases/map-trigger-recipients/map-trigger-recipients.use-case.ts b/packages/application-generic/src/usecases/map-trigger-recipients/map-trigger-recipients.use-case.ts deleted file mode 100644 index 01bda19aad5..00000000000 --- a/packages/application-generic/src/usecases/map-trigger-recipients/map-trigger-recipients.use-case.ts +++ /dev/null @@ -1,197 +0,0 @@ -import { Injectable } from '@nestjs/common'; -import { - EnvironmentId, - FeatureFlagsKeysEnum, - ISubscribersDefine, - ISubscribersSource, - ITopic, - OrganizationId, - SubscriberSourceEnum, - TopicSubscribersDto, - TriggerRecipient, - TriggerRecipients, - TriggerRecipientsTypeEnum, - TriggerRecipientSubscriber, - TriggerRecipientTopics, - UserId, -} from '@novu/shared'; - -import { MapTriggerRecipientsCommand } from './map-trigger-recipients.command'; -import { - GetTopicSubscribersCommand, - GetTopicSubscribersUseCase, -} from '../get-topic-subscribers'; -import { GetFeatureFlag, GetFeatureFlagCommand } from '../get-feature-flag'; -import { InstrumentUsecase } from '../../instrumentation'; - -const isNotTopic = ( - recipient: TriggerRecipient -): recipient is TriggerRecipientSubscriber => !isTopic(recipient); - -const isTopic = (recipient: TriggerRecipient): recipient is ITopic => - (recipient as ITopic).type && - (recipient as ITopic).type === TriggerRecipientsTypeEnum.TOPIC; - -@Injectable() -export class MapTriggerRecipients { - constructor( - private getTopicSubscribers: GetTopicSubscribersUseCase, - private getFeatureFlag: GetFeatureFlag - ) {} - - @InstrumentUsecase() - async execute( - command: MapTriggerRecipientsCommand - ): Promise { - const { - environmentId, - organizationId, - recipients, - transactionId, - userId, - actor, - } = command; - - const mappedRecipients = Array.isArray(recipients) - ? recipients - : [recipients]; - - const simpleSubscribers: ISubscribersSource[] = this.findSubscribers( - mappedRecipients, - SubscriberSourceEnum.SINGLE - ); - - let topicSubscribers: ISubscribersSource[] = - await this.getSubscribersFromAllTopics( - environmentId, - organizationId, - userId, - mappedRecipients - ); - - if (actor) { - topicSubscribers = this.excludeActorFromTopicSubscribers( - topicSubscribers, - actor - ); - } - - return this.deduplicateSubscribers([ - ...simpleSubscribers, - ...topicSubscribers, - ]); - } - - /** - * Time complexity: O(n) - */ - private deduplicateSubscribers( - subscribers: ISubscribersSource[] - ): ISubscribersSource[] { - const uniqueSubscribers = new Set(); - - return subscribers.filter((el) => { - const isDuplicate = uniqueSubscribers.has(el.subscriberId); - uniqueSubscribers.add(el.subscriberId); - - return !isDuplicate; - }); - } - - private excludeActorFromTopicSubscribers( - subscribers: ISubscribersSource[], - actor: ISubscribersDefine - ): ISubscribersSource[] { - return subscribers.filter( - (subscriber) => subscriber.subscriberId !== actor?.subscriberId - ); - } - - private async getSubscribersFromAllTopics( - environmentId: EnvironmentId, - organizationId: OrganizationId, - userId: UserId, - recipients: TriggerRecipients - ): Promise { - const featureFlagCommand = GetFeatureFlagCommand.create({ - environmentId, - organizationId, - userId, - key: FeatureFlagsKeysEnum.IS_TOPIC_NOTIFICATION_ENABLED, - }); - const isEnabled = await this.getFeatureFlag.execute(featureFlagCommand); - - if (isEnabled) { - const topics = this.findTopics(recipients); - - const subscribers: ISubscribersSource[] = []; - - for (const topic of topics) { - const getTopicSubscribersCommand = GetTopicSubscribersCommand.create({ - environmentId, - topicKey: topic.topicKey, - organizationId, - }); - const topicSubscribers = await this.getTopicSubscribers.execute( - getTopicSubscribersCommand - ); - - topicSubscribers.forEach((subscriber: TopicSubscribersDto) => - subscribers.push({ - subscriberId: subscriber.externalSubscriberId, - _subscriberSource: SubscriberSourceEnum.TOPIC, - }) - ); - } - - return subscribers; - } - - return []; - } - - public mapActor( - subscriber: TriggerRecipientSubscriber - ): ISubscribersDefine | null { - if (!subscriber) return null; - - if (typeof subscriber === 'string') { - return { subscriberId: subscriber }; - } - - return subscriber; - } - - public mapSubscriber( - subscriber: TriggerRecipientSubscriber, - source: SubscriberSourceEnum - ): ISubscribersSource | null { - if (!subscriber) return null; - - let mappedSubscriber: Partial; - - if (typeof subscriber === 'string') { - mappedSubscriber = { subscriberId: subscriber }; - } else { - mappedSubscriber = subscriber; - } - - return { - ...mappedSubscriber, - _subscriberSource: source, - } as ISubscribersSource; - } - - private findSubscribers( - recipients: TriggerRecipients, - source: SubscriberSourceEnum - ): ISubscribersSource[] { - return recipients - .filter(isNotTopic) - .map((subscriber) => this.mapSubscriber(subscriber, source)); - } - - private findTopics(recipients: TriggerRecipients): TriggerRecipientTopics { - return recipients.filter(isTopic); - } -} diff --git a/packages/application-generic/src/usecases/trigger-multicast/trigger-multicast.usecase.ts b/packages/application-generic/src/usecases/trigger-multicast/trigger-multicast.usecase.ts index f521a781421..84138acc3b4 100644 --- a/packages/application-generic/src/usecases/trigger-multicast/trigger-multicast.usecase.ts +++ b/packages/application-generic/src/usecases/trigger-multicast/trigger-multicast.usecase.ts @@ -72,24 +72,6 @@ export class TriggerMulticast { ); } - const isEnabled = await this.getFeatureFlag.execute( - GetFeatureFlagCommand.create({ - environmentId, - organizationId, - userId, - key: FeatureFlagsKeysEnum.IS_TOPIC_NOTIFICATION_ENABLED, - }) - ); - - if (!isEnabled) { - Logger.log( - `The IS_TOPIC_NOTIFICATION_ENABLED feature flag is disabled, skipping trigger multicast`, - LOG_CONTEXT - ); - - return; - } - const topics = await this.getTopicsByTopicKeys( organizationId, environmentId, From 293d41ce751d58d6f32adb424de7f749591644f4 Mon Sep 17 00:00:00 2001 From: Denis Kralj Date: Fri, 3 May 2024 21:07:07 +0200 Subject: [PATCH 03/10] refactor: Remove unused feature flag Add (ugly) solution to verify matching of enum name and value --- .../src/types/feature-flags/flags.types.spec.ts | 4 ++-- libs/shared/src/types/feature-flags/flags.types.ts | 14 +++++++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/libs/shared/src/types/feature-flags/flags.types.spec.ts b/libs/shared/src/types/feature-flags/flags.types.spec.ts index 9ecc2f93e03..b87bbb823ed 100644 --- a/libs/shared/src/types/feature-flags/flags.types.spec.ts +++ b/libs/shared/src/types/feature-flags/flags.types.spec.ts @@ -46,13 +46,13 @@ testFlagEnumValidity(ValidFlagsEnum); enum InvalidKeyFlagsEnum { IS_SOMETHING_ENABLED = 'IS_SOMETHING_ENABLED', - INVALID_ENABLED = 'IS_INVALID_ENABLED', + INVALID_ENABLED = 'INVALID_ENABLED', } // @ts-expect-error - Invalid key - INVALID_ENABLED testFlagEnumValidity(InvalidKeyFlagsEnum); enum InvalidValueFlagsEnum { IS_SOMETHING_ENABLED = 'IS_SOMETHING_ENABLED', - IS_INVALID_ENABLED = 'INVALID_ENABLED', + INVALID_ENABLED = 'INVALID_ENABLED', } // @ts-expect-error - Invalid value on IS_INVALID_ENABLED: 'INVALID_ENABLED' testFlagEnumValidity(InvalidValueFlagsEnum); diff --git a/libs/shared/src/types/feature-flags/flags.types.ts b/libs/shared/src/types/feature-flags/flags.types.ts index 274e73ae0b0..449431c2f10 100644 --- a/libs/shared/src/types/feature-flags/flags.types.ts +++ b/libs/shared/src/types/feature-flags/flags.types.ts @@ -17,4 +17,16 @@ export type IFlagKey = `IS_${Uppercase}_ENABLED`; */ export function testFlagEnumValidity>( testEnum: TEnum & Record, ['Key must follow `IFlagKey` format']> -) {} +) { + for (const key in testEnum) { + if (testEnum.hasOwnProperty(key) && isIFlagKey(key) && isIFlagKey(testEnum[key])) { + if (key !== testEnum[key]) { + throw Error('Enum name must match the value'); + } + } + } +} + +function isIFlagKey(value: unknown): value is IFlagKey { + return typeof value === 'string'; +} From d03423e6a97b61e3bda0acc88bc2e1d3c3c75f5f Mon Sep 17 00:00:00 2001 From: Denis Kralj Date: Mon, 6 May 2024 09:09:49 +0200 Subject: [PATCH 04/10] refactor: Remove unused feature flag Remove unused export --- packages/application-generic/src/usecases/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/application-generic/src/usecases/index.ts b/packages/application-generic/src/usecases/index.ts index 825855e7cda..6e91354852e 100644 --- a/packages/application-generic/src/usecases/index.ts +++ b/packages/application-generic/src/usecases/index.ts @@ -29,7 +29,6 @@ export * from './switch-environment'; export * from './switch-organization'; export * from './create-user'; export * from './get-subscriber-global-preference'; -export * from './map-trigger-recipients'; export * from './get-topic-subscribers'; export * from './subscriber-job-bound/subscriber-job-bound.usecase'; export * from './subscriber-job-bound/subscriber-job-bound.command'; From 24bf4bd41f87de220955b2b9eef868e8a9d54a58 Mon Sep 17 00:00:00 2001 From: Denis Kralj Date: Mon, 6 May 2024 09:14:19 +0200 Subject: [PATCH 05/10] refactor: Remove unused feature flag ignore example env config for spell check --- .cspell.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.cspell.json b/.cspell.json index e4ec6fe1c3b..101159aafa9 100644 --- a/.cspell.json +++ b/.cspell.json @@ -639,7 +639,7 @@ "websockets", "whatsapp", "whatsappbusiness", - "parens" + "parens", ], "flagWords": [], "patterns": [ @@ -731,5 +731,6 @@ ".env.local", ".env.production", ".env.test", + ".example.env", ] } From 456912d1cc81e08ce2f99bd37c40fb01c5bc3f40 Mon Sep 17 00:00:00 2001 From: Denis Kralj Date: Mon, 6 May 2024 09:32:07 +0200 Subject: [PATCH 06/10] refactor: Remove unused feature flag Remove unused import --- apps/worker/src/app/workflow/workflow.module.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/apps/worker/src/app/workflow/workflow.module.ts b/apps/worker/src/app/workflow/workflow.module.ts index a31401889a3..d7576d12f35 100644 --- a/apps/worker/src/app/workflow/workflow.module.ts +++ b/apps/worker/src/app/workflow/workflow.module.ts @@ -21,7 +21,6 @@ import { ConditionsFilter, TriggerEvent, SelectVariant, - MapTriggerRecipients, GetTopicSubscribersUseCase, getFeatureFlag, SubscriberJobBound, @@ -131,7 +130,6 @@ const USE_CASES = [ TriggerEvent, UpdateJobStatus, WebhookFilterBackoffStrategy, - MapTriggerRecipients, GetTopicSubscribersUseCase, getFeatureFlag, SubscriberJobBound, From 1dbf6aa5dfd871f1a1b76179460fc424fe8b7be0 Mon Sep 17 00:00:00 2001 From: Denis Kralj Date: Mon, 6 May 2024 09:44:34 +0200 Subject: [PATCH 07/10] refactor: Remove unused feature flag Add alternative checking of flag name match check --- .../src/types/feature-flags/flags.types.spec.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libs/shared/src/types/feature-flags/flags.types.spec.ts b/libs/shared/src/types/feature-flags/flags.types.spec.ts index b87bbb823ed..62d83cee5d5 100644 --- a/libs/shared/src/types/feature-flags/flags.types.spec.ts +++ b/libs/shared/src/types/feature-flags/flags.types.spec.ts @@ -62,3 +62,15 @@ testFlagEnumValidity(InvalidValueFlagsEnum); */ testFlagEnumValidity(FeatureFlagsKeysEnum); testFlagEnumValidity(SystemCriticalFlagsEnum); + +// Ensure that the keys and values of FeatureFlagsKeysEnum match +type ValidateFeatureFlagsKeysEnum = { + [K in keyof typeof FeatureFlagsKeysEnum]: K extends IFlagKey ? K : `Value doesn't match key`; +}; +const validateFeatureFlagsKeysEnum: ValidateFeatureFlagsKeysEnum = FeatureFlagsKeysEnum; + +// Ensure that the keys and values of SystemCriticalFlagsEnum match +type ValidateSystemCriticalFlagsEnum = { + [K in keyof typeof SystemCriticalFlagsEnum]: K extends IFlagKey ? K : `Value doesn't match key`; +}; +const validateSystemCriticalFlagsEnum: ValidateSystemCriticalFlagsEnum = SystemCriticalFlagsEnum; From b0c9be85679517304e4bb67e4d16820c7a9bc42f Mon Sep 17 00:00:00 2001 From: Denis Kralj Date: Mon, 6 May 2024 11:08:06 +0200 Subject: [PATCH 08/10] refactor: Remove unused feature flag Remove non typescript check option Name variable underscore to imply non-use Refactor tests for flag test specs --- .../types/feature-flags/flags.types.spec.ts | 27 ++++++++++--------- .../src/types/feature-flags/flags.types.ts | 16 ++--------- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/libs/shared/src/types/feature-flags/flags.types.spec.ts b/libs/shared/src/types/feature-flags/flags.types.spec.ts index 62d83cee5d5..132aa4d7a1c 100644 --- a/libs/shared/src/types/feature-flags/flags.types.spec.ts +++ b/libs/shared/src/types/feature-flags/flags.types.spec.ts @@ -44,33 +44,36 @@ enum ValidFlagsEnum { } testFlagEnumValidity(ValidFlagsEnum); -enum InvalidKeyFlagsEnum { - IS_SOMETHING_ENABLED = 'IS_SOMETHING_ENABLED', +enum InvalidFlagsEnum { INVALID_ENABLED = 'INVALID_ENABLED', } -// @ts-expect-error - Invalid key - INVALID_ENABLED -testFlagEnumValidity(InvalidKeyFlagsEnum); -enum InvalidValueFlagsEnum { - IS_SOMETHING_ENABLED = 'IS_SOMETHING_ENABLED', - INVALID_ENABLED = 'INVALID_ENABLED', +// @ts-expect-error - not matching pattern +testFlagEnumValidity(InvalidFlagsEnum); + +enum NonMatchingKeyValueEnum { + IS_SOMETHING_ENABLED = 'IS_SOMETHING_ELSE_ENABLED', } -// @ts-expect-error - Invalid value on IS_INVALID_ENABLED: 'INVALID_ENABLED' -testFlagEnumValidity(InvalidValueFlagsEnum); + +// Ensure that the keys and values of FeatureFlagsKeysEnum match +type ValidateNonMatchingKeyValueEnum = { + [K in keyof typeof NonMatchingKeyValueEnum]: K extends IFlagKey ? K : `Value doesn't match key`; +}; +// @ts-expect-error - non matching key-value pair in enum +const validateNonMatchingKeyValueEnum: ValidateNonMatchingKeyValueEnum = NonMatchingKeyValueEnum; /** * Verifying declared FlagEnums */ -testFlagEnumValidity(FeatureFlagsKeysEnum); -testFlagEnumValidity(SystemCriticalFlagsEnum); - // Ensure that the keys and values of FeatureFlagsKeysEnum match type ValidateFeatureFlagsKeysEnum = { [K in keyof typeof FeatureFlagsKeysEnum]: K extends IFlagKey ? K : `Value doesn't match key`; }; const validateFeatureFlagsKeysEnum: ValidateFeatureFlagsKeysEnum = FeatureFlagsKeysEnum; +testFlagEnumValidity(FeatureFlagsKeysEnum); // Ensure that the keys and values of SystemCriticalFlagsEnum match type ValidateSystemCriticalFlagsEnum = { [K in keyof typeof SystemCriticalFlagsEnum]: K extends IFlagKey ? K : `Value doesn't match key`; }; const validateSystemCriticalFlagsEnum: ValidateSystemCriticalFlagsEnum = SystemCriticalFlagsEnum; +testFlagEnumValidity(SystemCriticalFlagsEnum); diff --git a/libs/shared/src/types/feature-flags/flags.types.ts b/libs/shared/src/types/feature-flags/flags.types.ts index 449431c2f10..8b3807a7d67 100644 --- a/libs/shared/src/types/feature-flags/flags.types.ts +++ b/libs/shared/src/types/feature-flags/flags.types.ts @@ -16,17 +16,5 @@ export type IFlagKey = `IS_${Uppercase}_ENABLED`; * @param testEnum - the Enum to type check */ export function testFlagEnumValidity>( - testEnum: TEnum & Record, ['Key must follow `IFlagKey` format']> -) { - for (const key in testEnum) { - if (testEnum.hasOwnProperty(key) && isIFlagKey(key) && isIFlagKey(testEnum[key])) { - if (key !== testEnum[key]) { - throw Error('Enum name must match the value'); - } - } - } -} - -function isIFlagKey(value: unknown): value is IFlagKey { - return typeof value === 'string'; -} + _: TEnum & Record, ['Key must follow `IFlagKey` format']> +) {} From e39da358c402b942fd390261ef84fe7840ad721a Mon Sep 17 00:00:00 2001 From: Denis Kralj Date: Mon, 6 May 2024 14:35:55 +0200 Subject: [PATCH 09/10] refactor: Remove unused feature flag Remove Remove flag setting as per MR suggestion --- .../src/app/events/e2e/trigger-event-topic.e2e.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/apps/api/src/app/events/e2e/trigger-event-topic.e2e.ts b/apps/api/src/app/events/e2e/trigger-event-topic.e2e.ts index 1c8ca26422c..7a3d4323195 100644 --- a/apps/api/src/app/events/e2e/trigger-event-topic.e2e.ts +++ b/apps/api/src/app/events/e2e/trigger-event-topic.e2e.ts @@ -31,8 +31,6 @@ const axiosInstance = axios.create(); const TOPIC_PATH = '/v1/topics'; const TRIGGER_ENDPOINT = '/v1/events/trigger'; -const originalLaunchDarklySdkKey = process.env.LAUNCH_DARKLY_SDK_KEY; - describe('Topic Trigger Event', () => { describe('Trigger event for a topic - /v1/events/trigger (POST)', () => { let session: UserSession; @@ -48,7 +46,6 @@ describe('Topic Trigger Event', () => { const messageRepository = new MessageRepository(); beforeEach(async () => { - process.env.LAUNCH_DARKLY_SDK_KEY = ''; session = new UserSession(); await session.initialize(); @@ -67,10 +64,6 @@ describe('Topic Trigger Event', () => { to = [{ type: TriggerRecipientsTypeEnum.TOPIC, topicKey: createdTopicDto.key }]; }); - afterEach(() => { - process.env.LAUNCH_DARKLY_SDK_KEY = originalLaunchDarklySdkKey; - }); - it('should trigger an event successfully', async () => { const response = await axiosInstance.post( triggerEndpointUrl, @@ -320,7 +313,6 @@ describe('Topic Trigger Event', () => { const logRepository = new LogRepository(); beforeEach(async () => { - process.env.LAUNCH_DARKLY_SDK_KEY = ''; session = new UserSession(); await session.initialize(); @@ -372,10 +364,6 @@ describe('Topic Trigger Event', () => { ]; }); - afterEach(() => { - process.env.LAUNCH_DARKLY_SDK_KEY = originalLaunchDarklySdkKey; - }); - it('should trigger an event successfully', async () => { const response = await axiosInstance.post( triggerEndpointUrl, From 300fa2c8dc1a0795e078441505c8236b70f2a956 Mon Sep 17 00:00:00 2001 From: Denis Kralj Date: Mon, 6 May 2024 14:50:57 +0200 Subject: [PATCH 10/10] refactor: Remove unused feature flag Remove additional occurrences of feature flag that isn't required --- apps/api/src/app/events/e2e/trigger-event.e2e.ts | 3 --- apps/api/src/app/events/e2e/trigger-multicast.e2e.ts | 1 - .../api/src/app/integrations/e2e/get-active-integration.e2e.ts | 1 - 3 files changed, 5 deletions(-) diff --git a/apps/api/src/app/events/e2e/trigger-event.e2e.ts b/apps/api/src/app/events/e2e/trigger-event.e2e.ts index b44df90a837..cf0789ceee9 100644 --- a/apps/api/src/app/events/e2e/trigger-event.e2e.ts +++ b/apps/api/src/app/events/e2e/trigger-event.e2e.ts @@ -67,7 +67,6 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { describe(`Trigger Event - ${eventTriggerPath} (POST)`, function () { beforeEach(async () => { - process.env.LAUNCH_DARKLY_SDK_KEY = ''; session = new UserSession(); await session.initialize(); template = await session.createTemplate(); @@ -2550,7 +2549,6 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { describe('filters logic', () => { beforeEach(async () => { - process.env.LAUNCH_DARKLY_SDK_KEY = ''; session = new UserSession(); await session.initialize(); subscriberService = new SubscribersService(session.organization._id, session.environment._id); @@ -3189,7 +3187,6 @@ describe(`Trigger event - ${eventTriggerPath} (POST)`, function () { describe('workflow override', () => { beforeEach(async () => { - process.env.LAUNCH_DARKLY_SDK_KEY = ''; session = new UserSession(); await session.initialize(); diff --git a/apps/api/src/app/events/e2e/trigger-multicast.e2e.ts b/apps/api/src/app/events/e2e/trigger-multicast.e2e.ts index c84d825f7ad..166e79f1576 100644 --- a/apps/api/src/app/events/e2e/trigger-multicast.e2e.ts +++ b/apps/api/src/app/events/e2e/trigger-multicast.e2e.ts @@ -140,7 +140,6 @@ describe('TriggerMulticast', () => { } beforeEach(async () => { - process.env.LAUNCH_DARKLY_SDK_KEY = ''; session = new UserSession(); await session.initialize(); template = await session.createTemplate(); diff --git a/apps/api/src/app/integrations/e2e/get-active-integration.e2e.ts b/apps/api/src/app/integrations/e2e/get-active-integration.e2e.ts index 26c5ffafb56..f20c0c8b9b9 100644 --- a/apps/api/src/app/integrations/e2e/get-active-integration.e2e.ts +++ b/apps/api/src/app/integrations/e2e/get-active-integration.e2e.ts @@ -11,7 +11,6 @@ describe('Get Active Integrations - Multi-Provider Configuration - /integrations beforeEach(async () => { session = new UserSession(); await session.initialize(); - process.env.LAUNCH_DARKLY_SDK_KEY = ''; }); it('should get active integrations', async function () {