feat(api): add workflow trigger identifier parity (#6657)

Author: djabarovgeorge

apps/api/migrations/integration-scheme-update/add-integration-identifier-migration.ts

@@ -1,10 +1,10 @@
 // June 27th, 2023
 
 import shortid from 'shortid';
-import slugify from 'slugify';
 
 import { providers } from '@novu/shared';
 import { EnvironmentRepository, IntegrationEntity, IntegrationRepository } from '@novu/dal';
+import { slugifyName } from '@novu/application-generic';
 
 export const ENVIRONMENT_NAME_TO_SHORT_NAME = { ['Development']: 'dev', ['Production']: 'prod', ['undefined']: '' };
 
@@ -101,7 +101,7 @@ export function genIntegrationIdentificationDetails({
   const defaultName = providers.find((provider) => provider.id === providerId)?.displayName ?? providerIdCapitalized;
 
   const name = existingName ?? defaultName;
-  const identifier = existingIdentifier ?? `${slugify(name, { lower: true, strict: true })}-${shortid.generate()}`;
+  const identifier = existingIdentifier ?? `${slugifyName(name)}-${shortid.generate()}`;
 
   return { name, identifier };
 }

apps/api/migrations/layout-identifier-update/add-layout-identifier-migration.ts

@@ -1,9 +1,9 @@
 // August 14th, 2023
 
 import shortid from 'shortid';
-import slugify from 'slugify';
 
 import { LayoutRepository, OrganizationRepository } from '@novu/dal';
+import { slugifyName } from '@novu/application-generic';
 
 export async function addLayoutIdentifierMigration() {
   // eslint-disable-next-line no-console
@@ -27,7 +27,7 @@ export async function addLayoutIdentifierMigration() {
     const bulkWriteOps = layouts
       .map((layout) => {
         const { _id, name } = layout;
-        const identifier = `${slugify(name, { lower: true, strict: true })}-${shortid.generate()}`;
+        const identifier = `${slugifyName(name)}-${shortid.generate()}`;
 
         return [
           {

apps/api/migrations/novu-integrations/novu-integrations.migration.ts

@@ -9,7 +9,7 @@ import {
 import { EmailProviderIdEnum, SmsProviderIdEnum } from '@novu/shared';
 import { NestFactory } from '@nestjs/core';
 import { AppModule } from '../../src/app.module';
-import slugify from 'slugify';
+import { slugifyName } from '@novu/application-generic';
 import shortid from 'shortid';
 
 const organizationRepository = new OrganizationRepository();
@@ -47,11 +47,11 @@ const createNovuIntegration = async (
     providerId,
     channel,
     name,
-    identifier: `${slugify(name, { lower: true, strict: true })}-${shortid.generate()}`,
+    identifier: `${slugifyName(name)}-${shortid.generate()}`,
     active: countChannelIntegrations === 0,
   });
 
-  console.log('Created Integration' + response._id);
+  console.log(`Created Integration${response._id}`);
 };
 
 export async function createNovuIntegrations() {
@@ -78,10 +78,10 @@ export async function createNovuIntegrations() {
       await createNovuIntegration(environment, ChannelTypeEnum.SMS);
       await createNovuIntegration(environment, ChannelTypeEnum.EMAIL);
 
-      console.log('Processed environment' + environment._id);
+      console.log(`Processed environment${environment._id}`);
     }
 
-    console.log('Processed organization' + organization._id);
+    console.log(`Processed organization${organization._id}`);
   }
 
   // eslint-disable-next-line no-console

apps/api/src/app/integrations/usecases/create-integration/create-integration.usecase.ts

@@ -1,6 +1,5 @@
 import { BadRequestException, ConflictException, Inject, Injectable } from '@nestjs/common';
 import shortid from 'shortid';
-import slugify from 'slugify';
 import { IntegrationEntity, IntegrationRepository, DalException, IntegrationQuery } from '@novu/dal';
 import {
   ChannelTypeEnum,
@@ -17,6 +16,7 @@ import {
   InvalidateCacheService,
   areNovuSmsCredentialsSet,
   areNovuEmailCredentialsSet,
+  slugifyName,
 } from '@novu/application-generic';
 
 import { CreateIntegrationCommand } from './create-integration.command';
@@ -148,7 +148,7 @@ export class CreateIntegration {
       const defaultName =
         providers.find((provider) => provider.id === command.providerId)?.displayName ?? providerIdCapitalized;
       const name = command.name ?? defaultName;
-      const identifier = command.identifier ?? `${slugify(name, { lower: true, strict: true })}-${shortid.generate()}`;
+      const identifier = command.identifier ?? `${slugifyName(name)}-${shortid.generate()}`;
 
       const query: IntegrationQuery = {
         name,

apps/api/src/app/workflows-v1/notification-template.controller.ts

@@ -106,7 +106,7 @@ export class NotificationTemplateController {
         name: body.name,
         tags: body.tags,
         description: body.description,
-        identifier: body.identifier,
+        workflowId: body.identifier,
         critical: body.critical,
         preferenceSettings: body.preferenceSettings,
         steps: body.steps,

apps/api/src/app/workflows-v1/workflow-v1.controller.ts

@@ -111,7 +111,7 @@ export class WorkflowControllerV1 {
         name: body.name,
         tags: body.tags,
         description: body.description,
-        identifier: body.identifier,
+        workflowId: body.identifier,
         critical: body.critical,
         preferenceSettings: body.preferenceSettings,
         steps: body.steps,

apps/api/src/app/workflows-v2/mappers/notification-template-mapper.ts

@@ -31,6 +31,7 @@ export function toResponseWorkflowDto(
     preferences: preferencesDto,
     steps: getSteps(template, stepIdToControlValuesMap),
     name: template.name,
+    workflowId: template.triggers[0].identifier,
     description: template.description,
     origin: template.origin || WorkflowOriginEnum.EXTERNAL,
     type: template.type || ('MISSING-TYPE-ISSUE' as unknown as WorkflowTypeEnum),

apps/api/src/app/workflows-v2/usecases/upsert-workflow/upsert-workflow.usecase.ts

@@ -21,6 +21,7 @@ import {
   UpsertControlValuesUseCase,
   UpsertPreferences,
   UpsertUserWorkflowPreferencesCommand,
+  slugifyName,
 } from '@novu/application-generic';
 import {
   CreateWorkflowDto,
@@ -33,7 +34,6 @@ import {
   WorkflowTypeEnum,
 } from '@novu/shared';
 import { UpsertWorkflowCommand } from './upsert-workflow.command';
-import { WorkflowAlreadyExistException } from '../../exceptions/workflow-already-exist';
 import { StepUpsertMechanismFailedMissingIdException } from '../../exceptions/step-upsert-mechanism-failed-missing-id.exception';
 import { toResponseWorkflowDto } from '../../mappers/notification-template-mapper';
 
@@ -64,11 +64,11 @@ export class UpsertWorkflowUseCase {
     private getPreferencesUseCase: GetPreferences
   ) {}
   async execute(command: UpsertWorkflowCommand): Promise<WorkflowResponseDto> {
-    const workflowForUpdate = await this.getWorkflowIfUpdateAndExist(command);
-    if (!workflowForUpdate && (await this.workflowExistByExternalId(command))) {
-      throw new WorkflowAlreadyExistException(command);
-    }
-    const workflow = await this.createOrUpdateWorkflow(workflowForUpdate, command);
+    const existingWorkflow = await this.notificationTemplateRepository.findOne({
+      _id: command.workflowDatabaseIdForUpdate,
+      _environmentId: command.user.environmentId,
+    });
+    const workflow = await this.createOrUpdateWorkflow(existingWorkflow, command);
     const stepIdToControlValuesMap = await this.upsertControlValues(workflow, command);
     const preferences = await this.upsertPreference(command, workflow);
 
@@ -188,28 +188,10 @@ export class UpsertWorkflowUseCase {
       description: workflowDto.description || '',
       tags: workflowDto.tags || [],
       critical: false,
+      triggerIdentifier: slugifyName(workflowDto.name),
     };
   }
 
-  private async getWorkflowIfUpdateAndExist(upsertCommand: UpsertWorkflowCommand) {
-    if (upsertCommand.workflowDatabaseIdForUpdate) {
-      return await this.notificationTemplateRepository.findByIdQuery({
-        id: upsertCommand.workflowDatabaseIdForUpdate,
-        environmentId: upsertCommand.user.environmentId,
-      });
-    }
-  }
-
-  private async workflowExistByExternalId(upsertCommand: UpsertWorkflowCommand) {
-    const { environmentId } = upsertCommand.user;
-    const workflowByDbId = await this.notificationTemplateRepository.findByTriggerIdentifier(
-      environmentId,
-      upsertCommand.workflowDto.name
-    );
-
-    return !!workflowByDbId;
-  }
-
   private convertCreateToUpdateCommand(
     command: UpsertWorkflowCommand,
     existingWorkflow: NotificationTemplateEntity
@@ -229,6 +211,7 @@ export class UpsertWorkflowUseCase {
       description: workflowDto.description,
       tags: workflowDto.tags,
       active: workflowDto.active ?? true,
+      workflowId: workflowDto.workflowId,
     };
   }
 

apps/api/src/app/workflows-v2/workflow.controller.e2e.ts

@@ -16,6 +16,7 @@ import {
 } from '@novu/shared';
 import { randomBytes } from 'crypto';
 import { JsonSchema } from '@novu/framework';
+import { slugifyName } from '@novu/application-generic';
 
 const v2Prefix = '/v2';
 const PARTIAL_UPDATED_NAME = 'Updated';
@@ -59,13 +60,14 @@ describe('Workflow Controller E2E API Testing', () => {
   });
 
   describe('Create Workflow Permutations', () => {
-    it('should not allow creating two workflows for the same user with the same name', async () => {
+    it('should allow creating two workflows for the same user with the same name', async () => {
       const nameSuffix = `Test Workflow${new Date().toString()}`;
       await createWorkflowAndValidate(nameSuffix);
       const createWorkflowDto: CreateWorkflowDto = buildCreateWorkflowDto(nameSuffix);
       const res = await session.testAgent.post(`${v2Prefix}/workflows`).send(createWorkflowDto);
-      expect(res.status).to.be.equal(400);
-      expect(res.text).to.contain('Workflow with the same name already exists');
+      expect(res.status).to.be.equal(201);
+      const workflowCreated: WorkflowResponseDto = res.body.data;
+      expect(workflowCreated.workflowId).to.include(`${slugifyName(nameSuffix)}-`);
     });
   });
 
@@ -147,7 +149,7 @@ describe('Workflow Controller E2E API Testing', () => {
       });
     });
 
-    it('should return  results without query', async () => {
+    it('should return results without query', async () => {
       const uuid = generateUUID();
       await create10Workflows(uuid);
       const listWorkflowResponse = await getAllAndValidate({
@@ -243,6 +245,7 @@ function buildCreateWorkflowDto(nameSuffix: string): CreateWorkflowDto {
   return {
     __source: WorkflowCreationSourceEnum.EDITOR,
     name: TEST_WORKFLOW_NAME + nameSuffix,
+    workflowId: `${slugifyName(TEST_WORKFLOW_NAME + nameSuffix)}`,
     description: 'This is a test workflow',
     active: true,
     tags: TEST_TAGS,
@@ -585,5 +588,10 @@ function buildUpdateRequest(workflowCreated: WorkflowResponseDto): UpdateWorkflo
     'type'
   ) as UpdateWorkflowDto;
 
-  return { ...updateRequest, name: TEST_WORKFLOW_UPDATED_NAME, steps };
+  return {
+    ...updateRequest,
+    name: TEST_WORKFLOW_UPDATED_NAME,
+    workflowId: `${slugifyName(TEST_WORKFLOW_UPDATED_NAME)}`,
+    steps,
+  };
 }

libs/application-generic/src/usecases/create-workflow/create-workflow.command.ts

@@ -94,6 +94,16 @@ export class CreateWorkflowCommand extends EnvironmentWithUserCommand {
   type: WorkflowTypeEnum;
 
   origin: WorkflowOriginEnum;
+
+  /**
+   * Optional identifier for the workflow trigger.
+   * This allows overriding the default trigger identifier generation strategy in the use case.
+   * If provided, the use case will use this value instead of generating one.
+   * If not provided, the use case will generate a trigger identifier based on its internal logic.
+   */
+  @IsOptional()
+  @IsString()
+  triggerIdentifier?: string;
 }
 
 export class ChannelCTACommand {

libs/application-generic/src/usecases/create-workflow/create-workflow.usecase.ts

@@ -7,7 +7,6 @@ import {
   NotFoundException,
 } from '@nestjs/common';
 import { ModuleRef } from '@nestjs/core';
-import slugify from 'slugify';
 import shortid from 'shortid';
 
 import {
@@ -43,6 +42,7 @@ import {
   CreateMessageTemplateCommand,
 } from '../message-template';
 import { ApiException, PlatformException } from '../../utils/exceptions';
+import { slugifyName } from '../../utils';
 
 @Injectable()
 export class CreateWorkflow {
@@ -63,23 +63,7 @@ export class CreateWorkflow {
     const command = blueprintCommand ?? usecaseCommand;
     this.validatePayload(command);
 
-    let triggerIdentifier: string;
-    if (command.type === WorkflowTypeEnum.BRIDGE)
-      /*
-       * Bridge workflows need to have the identifier preserved to ensure that
-       * the Framework-defined identifier is the source of truth.
-       */
-      triggerIdentifier = command.name;
-    else {
-      /**
-       * For non-bridge workflows, we use a slugified version of the workflow name
-       * as the trigger identifier to provide a better trigger DX.
-       */
-      triggerIdentifier = `${slugify(command.name, {
-        lower: true,
-        strict: true,
-      })}`;
-    }
+    const triggerIdentifier = this.generateTriggerIdentifier(command);
 
     const parentChangeId: string =
       NotificationTemplateRepository.createObjectId();
@@ -136,6 +120,29 @@ export class CreateWorkflow {
     return storedWorkflow;
   }
 
+  private generateTriggerIdentifier(command: CreateWorkflowCommand) {
+    if (command.triggerIdentifier) {
+      return command.triggerIdentifier;
+    }
+
+    let triggerIdentifier: string;
+    if (command.type === WorkflowTypeEnum.BRIDGE)
+      /*
+       * Bridge workflows need to have the identifier preserved to ensure that
+       * the Framework-defined identifier is the source of truth.
+       */
+      triggerIdentifier = command.name;
+    else {
+      /**
+       * For non-bridge workflows, we use a slugified version of the workflow name
+       * as the trigger identifier to provide a better trigger DX.
+       */
+      triggerIdentifier = slugifyName(command.name);
+    }
+
+    return triggerIdentifier;
+  }
+
   private validatePayload(command: CreateWorkflowCommand) {
     const variants = command.steps
       ? command.steps?.flatMap((step) => step.variants || [])

libs/application-generic/src/usecases/workflow/update-workflow/update-workflow.command.ts

@@ -42,7 +42,7 @@ export class UpdateWorkflowCommand extends EnvironmentWithUserCommand {
 
   @IsString()
   @IsOptional()
-  identifier?: string;
+  workflowId?: string;
 
   @IsBoolean()
   @IsOptional()

libs/application-generic/src/usecases/workflow/update-workflow/update-workflow.usecase.ts

@@ -93,19 +93,19 @@ export class UpdateWorkflow {
       updatePayload.description = command.description;
     }
 
-    if (command.identifier) {
+    if (command.workflowId) {
       const isExistingIdentifier =
         await this.notificationTemplateRepository.findByTriggerIdentifier(
           command.environmentId,
-          command.identifier,
+          command.workflowId,
         );
 
       if (isExistingIdentifier && isExistingIdentifier._id !== command.id) {
         throw new BadRequestException(
-          `Workflow with identifier ${command.identifier} already exists`,
+          `Workflow with identifier ${command.workflowId} already exists`,
         );
       } else {
-        updatePayload['triggers.0.identifier'] = command.identifier;
+        updatePayload['triggers.0.identifier'] = command.workflowId;
       }
     }
 

libs/application-generic/src/utils/index.ts

@@ -11,3 +11,4 @@ export * from './bridge';
 export * from './subscriber';
 export * from './variants';
 export * from './deepmerge';
+export * from './slugify-name';

libs/application-generic/src/utils/slugify-name.ts

@@ -0,0 +1,8 @@
+import slugify from 'slugify';
+
+export const slugifyName = (identifier: string): string => {
+  return slugify(identifier, {
+    lower: true,
+    strict: true,
+  });
+};

packages/shared/src/dto/workflows/workflow-commons-fields.ts

@@ -64,6 +64,10 @@ export class WorkflowCommonsFields {
   @IsDefined()
   name: string;
 
+  @IsString()
+  @IsDefined()
+  workflowId: string;
+
   @IsString()
   @IsOptional()
   description?: string;