From b6a7c684aa9baf794b5884b88262e98189467546 Mon Sep 17 00:00:00 2001 From: George Djabarov <39195835+djabarovgeorge@users.noreply.github.com> Date: Fri, 1 Nov 2024 11:06:34 +0200 Subject: [PATCH] feat(api): treat workflow name as editable, non-unique values (#6780) --- .vscode/tasks.json | 28 +++++++--- .../sync-to-environment.usecase.ts | 1 - .../upsert-workflow.usecase.ts | 52 ++++++++++++------- .../workflows-v2/workflow.controller.e2e.ts | 1 - .../steps/edit-step-sidebar.tsx | 2 +- libs/application-generic/package.json | 1 + .../create-workflow.usecase.ts | 48 +++++++++++++---- .../src/utils/generate-id.ts | 8 +++ libs/application-generic/src/utils/index.ts | 1 + .../src/dto/workflows/create-workflow-dto.ts | 3 ++ .../src/dto/workflows/update-workflow-dto.ts | 5 +- .../dto/workflows/workflow-commons-fields.ts | 4 -- .../dto/workflows/workflow-response-dto.ts | 4 ++ pnpm-lock.yaml | 29 ++++++----- 14 files changed, 131 insertions(+), 56 deletions(-) create mode 100644 libs/application-generic/src/utils/generate-id.ts diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 1591971d9ea..5918193398a 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -20,13 +20,8 @@ "beginsPattern": "Successfully compiled", "endsPattern": "Started application in NODE_ENV" } - }, - "dependsOn": [ - "SHARED", - "APPLICATION GENERIC", - "DAL" - ] - }, + } + }, { "type": "npm", "script": "start", @@ -56,6 +51,25 @@ ] }, { + "type": "npm", + "script": "start", + "isBackground": true, + "label": "DASHBOARD", + "path": "/apps/dashboard", + "icon": { + "id": "browser", + "color": "terminal.ansiGreen" + }, + "problemMatcher": { + "base": "$tsc-watch", + "owner": "typescript", + "background": { + "activeOnStart": true, + "beginsPattern": "Compiling...", + "endsPattern": "webpack compiled successfully" + } + } + }, { "type": "npm", "script": "start", "isBackground": true, diff --git a/apps/api/src/app/workflows-v2/usecases/sync-to-environment/sync-to-environment.usecase.ts b/apps/api/src/app/workflows-v2/usecases/sync-to-environment/sync-to-environment.usecase.ts index 6bc9e9a1333..5db647f185e 100644 --- a/apps/api/src/app/workflows-v2/usecases/sync-to-environment/sync-to-environment.usecase.ts +++ b/apps/api/src/app/workflows-v2/usecases/sync-to-environment/sync-to-environment.usecase.ts @@ -111,7 +111,6 @@ export class SyncToEnvironmentUseCase { preferences: PreferencesEntity[] ): Promise { return { - updatedAt: new Date().toISOString(), workflowId: workflow.triggers[0].identifier, name: workflow.name, active: workflow.active, diff --git a/apps/api/src/app/workflows-v2/usecases/upsert-workflow/upsert-workflow.usecase.ts b/apps/api/src/app/workflows-v2/usecases/upsert-workflow/upsert-workflow.usecase.ts index be01fe0d4f1..c0d02051579 100644 --- a/apps/api/src/app/workflows-v2/usecases/upsert-workflow/upsert-workflow.usecase.ts +++ b/apps/api/src/app/workflows-v2/usecases/upsert-workflow/upsert-workflow.usecase.ts @@ -5,6 +5,7 @@ import { NotificationGroupRepository, NotificationStepEntity, NotificationTemplateEntity, + NotificationTemplateRepository, PreferencesEntity, } from '@novu/dal'; import { @@ -25,9 +26,12 @@ import { import { CreateWorkflowDto, DEFAULT_WORKFLOW_PREFERENCES, + IdentifierOrInternalId, StepCreateDto, StepDto, StepUpdateDto, + UpdateWorkflowDto, + UserSessionData, WorkflowCreationSourceEnum, WorkflowOriginEnum, WorkflowPreferences, @@ -69,14 +73,7 @@ export class UpsertWorkflowUseCase { private getPreferencesUseCase: GetPreferences ) {} async execute(command: UpsertWorkflowCommand): Promise { - const workflowForUpdate: NotificationTemplateEntity | null = command.identifierOrInternalId - ? await this.getWorkflowByIdsUseCase.execute( - GetWorkflowByIdsCommand.create({ - ...command, - identifierOrInternalId: command.identifierOrInternalId, - }) - ) - : null; + const workflowForUpdate = await this.queryWorkflow(command); const workflow = await this.createOrUpdateWorkflow(workflowForUpdate, command); const stepIdToControlValuesMap = await this.upsertControlValues(workflow, command); const preferences = await this.upsertPreference(command, workflow); @@ -84,6 +81,19 @@ export class UpsertWorkflowUseCase { return toResponseWorkflowDto(workflow, preferences, stepIdToControlValuesMap); } + private async queryWorkflow(command: UpsertWorkflowCommand): Promise { + if (!command.identifierOrInternalId) { + return null; + } + + return await this.getWorkflowByIdsUseCase.execute( + GetWorkflowByIdsCommand.create({ + ...command, + identifierOrInternalId: command.identifierOrInternalId, + }) + ); + } + private async upsertControlValues(workflow: NotificationTemplateEntity, command: UpsertWorkflowCommand) { const stepIdToControlValuesMap: { [p: string]: ControlValuesEntity } = {}; for (const persistedStep of workflow.steps) { @@ -179,12 +189,14 @@ export class UpsertWorkflowUseCase { } private async createOrUpdateWorkflow( - existingWorkflow: NotificationTemplateEntity | null | undefined, + existingWorkflow: NotificationTemplateEntity | null, command: UpsertWorkflowCommand ): Promise { - if (existingWorkflow) { + if (existingWorkflow && isWorkflowUpdateDto(command.workflowDto, command.identifierOrInternalId)) { return await this.updateWorkflowUsecase.execute( - UpdateWorkflowCommand.create(this.convertCreateToUpdateCommand(command, existingWorkflow)) + UpdateWorkflowCommand.create( + this.convertCreateToUpdateCommand(command.workflowDto, command.user, existingWorkflow) + ) ); } @@ -219,30 +231,27 @@ export class UpsertWorkflowUseCase { description: workflowDto.description || '', tags: workflowDto.tags || [], critical: false, - triggerIdentifier: workflowDto.workflowId ?? slugify(workflowDto.name), + triggerIdentifier: slugify(workflowDto.name), }; } private convertCreateToUpdateCommand( - command: UpsertWorkflowCommand, + workflowDto: UpdateWorkflowDto, + user: UserSessionData, existingWorkflow: NotificationTemplateEntity ): UpdateWorkflowCommand { - const { workflowDto } = command; - const { user } = command; - return { id: existingWorkflow._id, environmentId: existingWorkflow._environmentId, organizationId: user.organizationId, userId: user._id, - name: command.workflowDto.name, + name: workflowDto.name, steps: this.mapSteps(workflowDto.steps, existingWorkflow), rawData: workflowDto, type: WorkflowTypeEnum.BRIDGE, description: workflowDto.description, tags: workflowDto.tags, active: workflowDto.active ?? true, - workflowId: workflowDto.workflowId, }; } @@ -337,3 +346,10 @@ export class UpsertWorkflowUseCase { )?._id; } } + +function isWorkflowUpdateDto( + workflowDto: CreateWorkflowDto | UpdateWorkflowDto, + id?: IdentifierOrInternalId +): workflowDto is UpdateWorkflowDto { + return !!id; +} diff --git a/apps/api/src/app/workflows-v2/workflow.controller.e2e.ts b/apps/api/src/app/workflows-v2/workflow.controller.e2e.ts index 481757d0215..57c0dcf252e 100644 --- a/apps/api/src/app/workflows-v2/workflow.controller.e2e.ts +++ b/apps/api/src/app/workflows-v2/workflow.controller.e2e.ts @@ -821,7 +821,6 @@ function buildUpdateRequest(workflowCreated: WorkflowResponseDto): UpdateWorkflo return { ...updateRequest, name: TEST_WORKFLOW_UPDATED_NAME, - workflowId: `${slugify(TEST_WORKFLOW_UPDATED_NAME)}`, steps, }; } diff --git a/apps/dashboard/src/components/workflow-editor/steps/edit-step-sidebar.tsx b/apps/dashboard/src/components/workflow-editor/steps/edit-step-sidebar.tsx index ca3f1a6896f..67e354b1df8 100644 --- a/apps/dashboard/src/components/workflow-editor/steps/edit-step-sidebar.tsx +++ b/apps/dashboard/src/components/workflow-editor/steps/edit-step-sidebar.tsx @@ -20,7 +20,7 @@ export const EditStepSidebar = () => { const { reset, setError } = form; const { workflow, error } = useFetchWorkflow({ - workflowId, + workflowSlug: workflowId, }); const step = useMemo(() => workflow?.steps.find((el) => el._id === stepId), [stepId, workflow]); diff --git a/libs/application-generic/package.json b/libs/application-generic/package.json index dc6bd039de0..024ae79359a 100644 --- a/libs/application-generic/package.json +++ b/libs/application-generic/package.json @@ -92,6 +92,7 @@ "rrule": "^2.7.2", "rxjs": "7.8.1", "sanitize-html": "^2.4.0", + "nanoid": "^3.1.20", "shortid": "^2.2.16" }, "optionalDependencies": { diff --git a/libs/application-generic/src/usecases/create-workflow/create-workflow.usecase.ts b/libs/application-generic/src/usecases/create-workflow/create-workflow.usecase.ts index 2074efb89bf..460a85c6e8b 100644 --- a/libs/application-generic/src/usecases/create-workflow/create-workflow.usecase.ts +++ b/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 shortid from 'shortid'; import { FeedRepository, @@ -43,6 +42,7 @@ import { CreateMessageTemplateCommand, } from '../message-template'; import { ApiException, PlatformException } from '../../utils/exceptions'; +import { shortId } from '../../utils/generate-id'; @Injectable() export class CreateWorkflow { @@ -169,18 +169,14 @@ export class CreateWorkflow { contentService.extractMessageVariables(command.steps); const subscriberVariables = contentService.extractSubscriberMessageVariables(command.steps); - - const templateCheckIdentifier = - await this.notificationTemplateRepository.findByTriggerIdentifier( - command.environmentId, - triggerIdentifier, - ); + const identifier = await this.generateUniqueIdentifier( + command, + triggerIdentifier, + ); const trigger: INotificationTrigger = { type: TriggerTypeEnum.EVENT, - identifier: `${triggerIdentifier}${ - !templateCheckIdentifier ? '' : `-${shortid.generate()}` - }`, + identifier, variables: variables.map((i) => { return { name: i.name, @@ -208,6 +204,38 @@ export class CreateWorkflow { return trigger; } + private async generateUniqueIdentifier( + command: CreateWorkflowCommand, + triggerIdentifier: string, + ) { + const maxAttempts = 3; + let identifier = ''; + + for (let attempt = 0; attempt < maxAttempts; attempt += 1) { + const candidateIdentifier = + attempt === 0 ? triggerIdentifier : `${triggerIdentifier}-${shortId()}`; + + const isIdentifierExist = + await this.notificationTemplateRepository.findByTriggerIdentifier( + command.environmentId, + candidateIdentifier, + ); + + if (!isIdentifierExist) { + identifier = candidateIdentifier; + break; + } + } + + if (!identifier) { + throw new ApiException( + `Unable to generate a unique identifier. Please provide a different workflow name.${command.name}`, + ); + } + + return identifier; + } + private sendTemplateCreationEvent( command: CreateWorkflowCommand, triggerIdentifier: string, diff --git a/libs/application-generic/src/utils/generate-id.ts b/libs/application-generic/src/utils/generate-id.ts new file mode 100644 index 00000000000..1f7bb06d4fe --- /dev/null +++ b/libs/application-generic/src/utils/generate-id.ts @@ -0,0 +1,8 @@ +import { customAlphabet } from 'nanoid'; + +export const ALPHABET = '0123456789abcdefghijklmnopqrstuvwxyz'; +const nanoid = customAlphabet(ALPHABET); + +export function shortId() { + return nanoid(4); +} diff --git a/libs/application-generic/src/utils/index.ts b/libs/application-generic/src/utils/index.ts index dc1cb77b7b7..11f207996b1 100644 --- a/libs/application-generic/src/utils/index.ts +++ b/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 './generate-id'; diff --git a/packages/shared/src/dto/workflows/create-workflow-dto.ts b/packages/shared/src/dto/workflows/create-workflow-dto.ts index 4e95bf54afe..460cb1a4f98 100644 --- a/packages/shared/src/dto/workflows/create-workflow-dto.ts +++ b/packages/shared/src/dto/workflows/create-workflow-dto.ts @@ -1,7 +1,10 @@ +import { IsDefined, IsNotEmpty, IsString } from 'class-validator'; import { PreferencesRequestDto, StepCreateDto, WorkflowCommonsFields } from './workflow-commons-fields'; import { WorkflowCreationSourceEnum } from '../../types'; export type CreateWorkflowDto = WorkflowCommonsFields & { + workflowId: string; + steps: StepCreateDto[]; __source: WorkflowCreationSourceEnum; diff --git a/packages/shared/src/dto/workflows/update-workflow-dto.ts b/packages/shared/src/dto/workflows/update-workflow-dto.ts index f251a09af74..2d3762f7403 100644 --- a/packages/shared/src/dto/workflows/update-workflow-dto.ts +++ b/packages/shared/src/dto/workflows/update-workflow-dto.ts @@ -1,7 +1,10 @@ import { PreferencesRequestDto, StepCreateDto, StepUpdateDto, WorkflowCommonsFields } from './workflow-commons-fields'; export type UpdateWorkflowDto = WorkflowCommonsFields & { - updatedAt: string; + /** + * We allow to update workflow id only for code first workflows + */ + workflowId?: string; steps: (StepCreateDto | StepUpdateDto)[]; diff --git a/packages/shared/src/dto/workflows/workflow-commons-fields.ts b/packages/shared/src/dto/workflows/workflow-commons-fields.ts index 23afe1464cd..34f9e5007d1 100644 --- a/packages/shared/src/dto/workflows/workflow-commons-fields.ts +++ b/packages/shared/src/dto/workflows/workflow-commons-fields.ts @@ -62,10 +62,6 @@ export class WorkflowCommonsFields { @IsDefined() name: string; - @IsString() - @IsDefined() - workflowId: string; - @IsString() @IsOptional() description?: string; diff --git a/packages/shared/src/dto/workflows/workflow-response-dto.ts b/packages/shared/src/dto/workflows/workflow-response-dto.ts index a6067205498..4719f34232f 100644 --- a/packages/shared/src/dto/workflows/workflow-response-dto.ts +++ b/packages/shared/src/dto/workflows/workflow-response-dto.ts @@ -40,4 +40,8 @@ export class WorkflowResponseDto extends WorkflowCommonsFields { @IsObject() @IsOptional() issues?: Record; + + @IsString() + @IsDefined() + workflowId: string; } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 375f394d278..54407ce066a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2625,6 +2625,9 @@ importers: mixpanel: specifier: ^0.17.0 version: 0.17.0 + nanoid: + specifier: ^3.1.31 + version: 3.3.7 nestjs-otel: specifier: 6.1.1 version: 6.1.1(@nestjs/common@10.4.1(class-transformer@0.5.1)(class-validator@0.14.1)(reflect-metadata@0.2.2)(rxjs@7.8.1))(@nestjs/core@10.4.1(@nestjs/common@10.4.1(class-transformer@0.5.1)(class-validator@0.14.1)(reflect-metadata@0.2.2)(rxjs@7.8.1))(@nestjs/platform-express@10.4.1)(@nestjs/websockets@10.4.1)(encoding@0.1.13)(reflect-metadata@0.2.2)(rxjs@7.8.1)) @@ -34952,8 +34955,8 @@ snapshots: dependencies: '@aws-crypto/sha256-browser': 3.0.0 '@aws-crypto/sha256-js': 3.0.0 - '@aws-sdk/client-sso-oidc': 3.575.0(@aws-sdk/client-sts@3.575.0) - '@aws-sdk/client-sts': 3.575.0 + '@aws-sdk/client-sso-oidc': 3.575.0 + '@aws-sdk/client-sts': 3.575.0(@aws-sdk/client-sso-oidc@3.575.0) '@aws-sdk/core': 3.575.0 '@aws-sdk/credential-provider-node': 3.575.0(@aws-sdk/client-sso-oidc@3.575.0)(@aws-sdk/client-sts@3.575.0) '@aws-sdk/middleware-host-header': 3.575.0 @@ -35154,8 +35157,8 @@ snapshots: '@aws-crypto/sha1-browser': 3.0.0 '@aws-crypto/sha256-browser': 3.0.0 '@aws-crypto/sha256-js': 3.0.0 - '@aws-sdk/client-sso-oidc': 3.575.0(@aws-sdk/client-sts@3.575.0) - '@aws-sdk/client-sts': 3.575.0 + '@aws-sdk/client-sso-oidc': 3.575.0 + '@aws-sdk/client-sts': 3.575.0(@aws-sdk/client-sso-oidc@3.575.0) '@aws-sdk/core': 3.575.0 '@aws-sdk/credential-provider-node': 3.575.0(@aws-sdk/client-sso-oidc@3.575.0)(@aws-sdk/client-sts@3.575.0) '@aws-sdk/middleware-bucket-endpoint': 3.575.0 @@ -35381,11 +35384,11 @@ snapshots: - aws-crt optional: true - '@aws-sdk/client-sso-oidc@3.575.0(@aws-sdk/client-sts@3.575.0)': + '@aws-sdk/client-sso-oidc@3.575.0': dependencies: '@aws-crypto/sha256-browser': 3.0.0 '@aws-crypto/sha256-js': 3.0.0 - '@aws-sdk/client-sts': 3.575.0 + '@aws-sdk/client-sts': 3.575.0(@aws-sdk/client-sso-oidc@3.575.0) '@aws-sdk/core': 3.575.0 '@aws-sdk/credential-provider-node': 3.575.0(@aws-sdk/client-sso-oidc@3.575.0)(@aws-sdk/client-sts@3.575.0) '@aws-sdk/middleware-host-header': 3.575.0 @@ -35424,7 +35427,6 @@ snapshots: '@smithy/util-utf8': 3.0.0 tslib: 2.7.0 transitivePeerDependencies: - - '@aws-sdk/client-sts' - aws-crt '@aws-sdk/client-sso-oidc@3.637.0(@aws-sdk/client-sts@3.637.0)': @@ -35809,11 +35811,11 @@ snapshots: - aws-crt optional: true - '@aws-sdk/client-sts@3.575.0': + '@aws-sdk/client-sts@3.575.0(@aws-sdk/client-sso-oidc@3.575.0)': dependencies: '@aws-crypto/sha256-browser': 3.0.0 '@aws-crypto/sha256-js': 3.0.0 - '@aws-sdk/client-sso-oidc': 3.575.0(@aws-sdk/client-sts@3.575.0) + '@aws-sdk/client-sso-oidc': 3.575.0 '@aws-sdk/core': 3.575.0 '@aws-sdk/credential-provider-node': 3.575.0(@aws-sdk/client-sso-oidc@3.575.0)(@aws-sdk/client-sts@3.575.0) '@aws-sdk/middleware-host-header': 3.575.0 @@ -35852,6 +35854,7 @@ snapshots: '@smithy/util-utf8': 3.0.0 tslib: 2.7.0 transitivePeerDependencies: + - '@aws-sdk/client-sso-oidc' - aws-crt '@aws-sdk/client-sts@3.637.0': @@ -36081,7 +36084,7 @@ snapshots: '@aws-sdk/credential-provider-ini@3.575.0(@aws-sdk/client-sso-oidc@3.575.0)(@aws-sdk/client-sts@3.575.0)': dependencies: - '@aws-sdk/client-sts': 3.575.0 + '@aws-sdk/client-sts': 3.575.0(@aws-sdk/client-sso-oidc@3.575.0) '@aws-sdk/credential-provider-env': 3.575.0 '@aws-sdk/credential-provider-process': 3.575.0 '@aws-sdk/credential-provider-sso': 3.575.0(@aws-sdk/client-sso-oidc@3.575.0) @@ -36392,7 +36395,7 @@ snapshots: '@aws-sdk/credential-provider-web-identity@3.575.0(@aws-sdk/client-sts@3.575.0)': dependencies: - '@aws-sdk/client-sts': 3.575.0 + '@aws-sdk/client-sts': 3.575.0(@aws-sdk/client-sso-oidc@3.575.0) '@aws-sdk/types': 3.575.0 '@smithy/property-provider': 3.1.3 '@smithy/types': 3.3.0 @@ -36913,7 +36916,7 @@ snapshots: '@aws-sdk/token-providers@3.575.0(@aws-sdk/client-sso-oidc@3.575.0)': dependencies: - '@aws-sdk/client-sso-oidc': 3.575.0(@aws-sdk/client-sts@3.575.0) + '@aws-sdk/client-sso-oidc': 3.575.0 '@aws-sdk/types': 3.575.0 '@smithy/property-provider': 3.1.3 '@smithy/shared-ini-file-loader': 3.1.4 @@ -36922,7 +36925,7 @@ snapshots: '@aws-sdk/token-providers@3.614.0(@aws-sdk/client-sso-oidc@3.575.0)': dependencies: - '@aws-sdk/client-sso-oidc': 3.575.0(@aws-sdk/client-sts@3.575.0) + '@aws-sdk/client-sso-oidc': 3.575.0 '@aws-sdk/types': 3.609.0 '@smithy/property-provider': 3.1.3 '@smithy/shared-ini-file-loader': 3.1.4