From b620680a6cdfc9f047aa1158cedbedda51808e20 Mon Sep 17 00:00:00 2001 From: GalT <39020298+tatarco@users.noreply.github.com> Date: Thu, 7 Nov 2024 22:47:55 +0100 Subject: [PATCH] bug(api): change sync to resolve values correctly --- .../sync-to-environment.usecase.ts | 159 +++++++++++------- ...ate-and-persist-workflow-issues.usecase.ts | 20 ++- .../workflows-v2/workflow.controller.e2e.ts | 3 +- 3 files changed, 116 insertions(+), 66 deletions(-) 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 5db647f185ed..9205f323ec3f 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 @@ -3,24 +3,21 @@ import { CreateWorkflowDto, PreferencesTypeEnum, StepCreateDto, - StepTypeEnum, + StepDataDto, + StepResponseDto, StepUpdateDto, UpdateWorkflowDto, WorkflowCreationSourceEnum, WorkflowPreferences, WorkflowResponseDto, } from '@novu/shared'; -import { - NotificationStepEntity, - NotificationTemplateEntity, - PreferencesEntity, - PreferencesRepository, -} from '@novu/dal'; +import { PreferencesEntity, PreferencesRepository } from '@novu/dal'; import { SyncToEnvironmentCommand } from './sync-to-environment.command'; -import { GetWorkflowByIdsUseCase } from '../get-workflow-by-ids/get-workflow-by-ids.usecase'; import { GetWorkflowByIdsCommand } from '../get-workflow-by-ids/get-workflow-by-ids.command'; import { UpsertWorkflowUseCase } from '../upsert-workflow/upsert-workflow.usecase'; import { UpsertWorkflowCommand } from '../upsert-workflow/upsert-workflow.command'; +import { GetWorkflowUseCase } from '../get-workflow/get-workflow.usecase'; +import { GetStepDataUsecase } from '../get-step-schema/get-step-data.usecase'; /** * This usecase is used to sync a workflow from one environment to another. @@ -34,9 +31,10 @@ import { UpsertWorkflowCommand } from '../upsert-workflow/upsert-workflow.comman @Injectable() export class SyncToEnvironmentUseCase { constructor( - private getWorkflowByIdsUseCase: GetWorkflowByIdsUseCase, + private getWorkflowUseCase: GetWorkflowUseCase, private preferencesRepository: PreferencesRepository, - private upsertWorkflowUseCase: UpsertWorkflowUseCase + private upsertWorkflowUseCase: UpsertWorkflowUseCase, + private getStepData: GetStepDataUsecase ) {} async execute(command: SyncToEnvironmentCommand): Promise { @@ -45,27 +43,35 @@ export class SyncToEnvironmentUseCase { } const workflowToClone = await this.getWorkflowToClone(command); - const preferencesToClone = await this.getWorkflowPreferences(workflowToClone._id, workflowToClone._environmentId); - const externalId = workflowToClone.triggers[0].identifier; + const preferencesToClone = await this.getWorkflowPreferences(workflowToClone._id, command.user.environmentId); + const externalId = workflowToClone.workflowId; const existingWorkflow = await this.findWorkflowInTargetEnvironment(command, externalId); + const workflowDto = await this.buildRequestDto(workflowToClone, preferencesToClone, command, existingWorkflow); - const workflowDto = existingWorkflow - ? await this.mapWorkflowToUpdateWorkflowDto(workflowToClone, existingWorkflow, preferencesToClone) - : await this.mapWorkflowToCreateWorkflowDto(workflowToClone, preferencesToClone); - - const upsertedWorkflow = await this.upsertWorkflowUseCase.execute( + return await this.upsertWorkflowUseCase.execute( UpsertWorkflowCommand.create({ user: { ...command.user, environmentId: command.targetEnvironmentId }, identifierOrInternalId: existingWorkflow?._id, workflowDto, }) ); + } + + private async buildRequestDto( + workflowToClone: WorkflowResponseDto, + preferencesToClone: PreferencesEntity[], + command: SyncToEnvironmentCommand, + existingWorkflow?: WorkflowResponseDto + ) { + if (existingWorkflow) { + return await this.mapWorkflowToUpdateWorkflowDto(workflowToClone, existingWorkflow, preferencesToClone, command); + } - return upsertedWorkflow; + return await this.mapWorkflowToCreateWorkflowDto(workflowToClone, preferencesToClone, command); } - private async getWorkflowToClone(command: SyncToEnvironmentCommand): Promise { - return this.getWorkflowByIdsUseCase.execute( + private async getWorkflowToClone(command: SyncToEnvironmentCommand): Promise { + return this.getWorkflowUseCase.execute( GetWorkflowByIdsCommand.create({ user: command.user, identifierOrInternalId: command.identifierOrInternalId, @@ -76,9 +82,9 @@ export class SyncToEnvironmentUseCase { private async findWorkflowInTargetEnvironment( command: SyncToEnvironmentCommand, externalId: string - ): Promise { + ): Promise { try { - return await this.getWorkflowByIdsUseCase.execute( + return await this.getWorkflowUseCase.execute( GetWorkflowByIdsCommand.create({ user: { ...command.user, environmentId: command.targetEnvironmentId }, identifierOrInternalId: externalId, @@ -90,56 +96,93 @@ export class SyncToEnvironmentUseCase { } private async mapWorkflowToCreateWorkflowDto( - workflow: NotificationTemplateEntity, - preferences: PreferencesEntity[] + workflowToClone: WorkflowResponseDto, + preferences: PreferencesEntity[], + command: SyncToEnvironmentCommand ): Promise { return { - workflowId: workflow.triggers[0].identifier, - name: workflow.name, - active: workflow.active, - tags: workflow.tags, - description: workflow.description, + workflowId: workflowToClone.workflowId, + name: workflowToClone.name, + active: workflowToClone.active, + tags: workflowToClone.tags, + description: workflowToClone.description, __source: WorkflowCreationSourceEnum.DASHBOARD, - steps: this.mapStepsToDto(workflow.steps), + steps: await this.mapStepsToDto(workflowToClone.steps, command), preferences: this.mapPreferences(preferences), }; } private async mapWorkflowToUpdateWorkflowDto( - workflow: NotificationTemplateEntity, - existingWorkflow: NotificationTemplateEntity, - preferences: PreferencesEntity[] + originWorkflow: WorkflowResponseDto, + existingWorkflowInProd: WorkflowResponseDto | undefined, + preferencesToClone: PreferencesEntity[], + command: SyncToEnvironmentCommand ): Promise { return { - workflowId: workflow.triggers[0].identifier, - name: workflow.name, - active: workflow.active, - tags: workflow.tags, - description: workflow.description, - steps: this.mapStepsToDto(workflow.steps, existingWorkflow.steps), - preferences: this.mapPreferences(preferences), + workflowId: originWorkflow.workflowId, + name: originWorkflow.name, + active: originWorkflow.active, + tags: originWorkflow.tags, + description: originWorkflow.description, + steps: await this.mapStepsToDto(originWorkflow.steps, command, existingWorkflowInProd?.steps), + preferences: this.mapPreferences(preferencesToClone), }; } - private mapStepsToDto( - steps: NotificationStepEntity[], - existingWorkflowSteps?: NotificationStepEntity[] - ): StepUpdateDto[] | StepCreateDto[] { - return steps.map((step) => ({ - /* - * If we are updating an existing workflow, we need to map the updated steps to the existing steps - * (!) 'existingWorkflowSteps' are from a different environment than 'steps' - the only thing that doesn't change - * in steps across environments is the stepId (TODO) - */ - ...(existingWorkflowSteps && { - _id: - existingWorkflowSteps.find((existingStep) => existingStep.stepId === step.stepId)?._templateId ?? - step._templateId, - }), + private async mapStepsToDto( + steps: StepResponseDto[], + command: SyncToEnvironmentCommand, + existingWorkflowSteps?: StepResponseDto[] + ): Promise<(StepUpdateDto | StepCreateDto)[]> { + const augmentedSteps: (StepUpdateDto | StepCreateDto)[] = []; + for (const step of steps) { + const idAsOptionalObject = this.prodDbIdAsOptionalObject(existingWorkflowSteps, step); + const stepDataDto = await this.getStepData.execute({ + identifierOrInternalId: command.identifierOrInternalId, + stepId: step.stepId, + user: command.user, + }); + + augmentedSteps.push(this.buildSingleStepRequest(idAsOptionalObject, step, stepDataDto)); + } + + return augmentedSteps; + } + /* + * If we are updating an existing workflow, we need to map the updated steps to the existing steps + * (!) 'existingWorkflowSteps' are from a different environment than 'steps' - the only thing that doesn't change + * in steps across environments is the stepId (TODO) + */ + private buildSingleStepRequest( + idAsOptionalObject: { _id: string } | {}, + step: StepResponseDto, + stepDataDto: StepDataDto + ): StepUpdateDto | StepCreateDto { + return { + ...idAsOptionalObject, name: step.name ?? '', - type: step.template?.type ?? StepTypeEnum.TRIGGER, - controlValues: step.controlVariables ?? {}, - })); + type: step.type, + controlValues: stepDataDto.controls.values ?? {}, + }; + } + + private prodDbIdAsOptionalObject(existingWorkflowSteps: StepResponseDto[] | undefined, step: StepResponseDto) { + const prodDatabaseId = this.findDatabaseIdInProdByExternalId(existingWorkflowSteps, step); + + if (prodDatabaseId) { + return { + _id: prodDatabaseId, + }; + } else { + return {}; + } + } + + private findDatabaseIdInProdByExternalId( + existingWorkflowSteps: StepResponseDto[] | undefined, + step: StepResponseDto + ) { + return existingWorkflowSteps?.find((existingStep) => existingStep.stepId === step.stepId)?._id ?? step._id; } private mapPreferences(preferences: PreferencesEntity[]): { diff --git a/apps/api/src/app/workflows-v2/usecases/upsert-workflow/validate-and-persist-workflow-issues.usecase.ts b/apps/api/src/app/workflows-v2/usecases/upsert-workflow/validate-and-persist-workflow-issues.usecase.ts index d00a51bbb561..1cd3114743dc 100644 --- a/apps/api/src/app/workflows-v2/usecases/upsert-workflow/validate-and-persist-workflow-issues.usecase.ts +++ b/apps/api/src/app/workflows-v2/usecases/upsert-workflow/validate-and-persist-workflow-issues.usecase.ts @@ -37,6 +37,7 @@ export class ValidateAndPersistWorkflowIssuesUsecase { private async persistWorkflow(command: ValidateWorkflowCommand, workflowWithIssues: NotificationTemplateEntity) { const isGoodWorkflow = this.isWorkflowCompleteAndValid(workflowWithIssues); + const status = this.calculateStatus(isGoodWorkflow, workflowWithIssues); await this.notificationTemplateRepository.update( { _id: command.workflow._id, @@ -44,7 +45,7 @@ export class ValidateAndPersistWorkflowIssuesUsecase { }, { ...workflowWithIssues, - status: this.calculateStatus(isGoodWorkflow, workflowWithIssues), + status, } ); } @@ -62,15 +63,22 @@ export class ValidateAndPersistWorkflowIssuesUsecase { } private isWorkflowCompleteAndValid(workflowWithIssues: NotificationTemplateEntity) { - const workflowIssues = workflowWithIssues.issues && Object.keys(workflowWithIssues.issues).length; - const hasStepIssues = + const workflowIssues = workflowWithIssues.issues && Object.keys(workflowWithIssues.issues).length > 0; + const hasInnerIssues = workflowWithIssues.steps .map((step) => step.issues) .filter((issue) => issue != null) - .filter((issue) => issue.body && Object.keys(issue.body).length > 0) - .filter((issue) => issue.controls && Object.keys(issue.controls).length > 0).length > 0; + .filter((issue) => this.hasBodyIssues(issue) || this.hasControlIssues(issue)).length > 0; - return !hasStepIssues && !workflowIssues; + return !hasInnerIssues && !workflowIssues; + } + + private hasControlIssues(issue: StepIssues) { + return issue.controls && Object.keys(issue.controls).length > 0; + } + + private hasBodyIssues(issue: StepIssues) { + return issue.body && Object.keys(issue.body).length > 0; } private async getWorkflow(command: ValidateWorkflowCommand) { 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 17ae7d1a0eea..570937ddf7f5 100644 --- a/apps/api/src/app/workflows-v2/workflow.controller.e2e.ts +++ b/apps/api/src/app/workflows-v2/workflow.controller.e2e.ts @@ -111,7 +111,7 @@ describe('Workflow Controller E2E API Testing', () => { { steps: [{ ...buildEmailStep(), controlValues: {} }] }, 0 ); - expect(status).to.equal(WorkflowStatusEnum.ERROR); + expect(status, JSON.stringify(issues)).to.equal(WorkflowStatusEnum.ERROR); expect(issues).to.be.ok; if (issues.controls) { expect(issues.controls?.emailEditor).to.be.ok; @@ -728,7 +728,6 @@ describe('Workflow Controller E2E API Testing', () => { expect(workflowResponseDto.createdAt, workflowAsString(workflowResponseDto)).to.be.ok; expect(workflowResponseDto.preferences, workflowAsString(workflowResponseDto)).to.be.ok; expect(workflowResponseDto.status, workflowAsString(workflowResponseDto)).to.be.ok; - expect(workflowResponseDto.status, workflowAsString(workflowResponseDto)).to.equal(WorkflowStatusEnum.ACTIVE); expect(workflowResponseDto.origin, workflowAsString(workflowResponseDto)).to.be.eq(WorkflowOriginEnum.NOVU_CLOUD); expect(Object.keys(workflowResponseDto.issues || {}).length, workflowAsString(workflowResponseDto)).to.be.equal(0); }