From ea360de42a6a0edb47ede140e5e0b0963e085051 Mon Sep 17 00:00:00 2001 From: Sokratis Vidros Date: Thu, 12 Dec 2024 23:31:59 +0200 Subject: [PATCH 1/2] chore(api): Remove usused workflow-v2 usecases --- .../usecases/patch-step-data/index.ts | 2 - .../patch-step-data/patch-step.command.ts | 21 ---- .../patch-step-data/patch-step.usecase.ts | 113 ------------------ .../upsert-workflow.usecase.ts | 3 +- .../app/workflows-v2/workflow.controller.ts | 43 +------ .../src/app/workflows-v2/workflow.module.ts | 2 - packages/shared/src/dto/workflows/step.dto.ts | 5 - 7 files changed, 2 insertions(+), 187 deletions(-) delete mode 100644 apps/api/src/app/workflows-v2/usecases/patch-step-data/index.ts delete mode 100644 apps/api/src/app/workflows-v2/usecases/patch-step-data/patch-step.command.ts delete mode 100644 apps/api/src/app/workflows-v2/usecases/patch-step-data/patch-step.usecase.ts diff --git a/apps/api/src/app/workflows-v2/usecases/patch-step-data/index.ts b/apps/api/src/app/workflows-v2/usecases/patch-step-data/index.ts deleted file mode 100644 index c0bfb3e1108..00000000000 --- a/apps/api/src/app/workflows-v2/usecases/patch-step-data/index.ts +++ /dev/null @@ -1,2 +0,0 @@ -export * from './patch-step.command'; -export * from './patch-step.usecase'; diff --git a/apps/api/src/app/workflows-v2/usecases/patch-step-data/patch-step.command.ts b/apps/api/src/app/workflows-v2/usecases/patch-step-data/patch-step.command.ts deleted file mode 100644 index e754b027026..00000000000 --- a/apps/api/src/app/workflows-v2/usecases/patch-step-data/patch-step.command.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { EnvironmentWithUserObjectCommand, MAX_NAME_LENGTH } from '@novu/application-generic'; -import { IsNotEmpty, IsObject, IsOptional, IsString, Length } from 'class-validator'; - -export class PatchStepCommand extends EnvironmentWithUserObjectCommand { - @IsString() - @IsNotEmpty() - workflowIdOrInternalId: string; - - @IsString() - @IsNotEmpty() - stepIdOrInternalId: string; - - @IsOptional() - @Length(1, MAX_NAME_LENGTH) - @IsString() - name?: string; - - @IsOptional() - @IsObject() - controlValues?: Record; -} diff --git a/apps/api/src/app/workflows-v2/usecases/patch-step-data/patch-step.usecase.ts b/apps/api/src/app/workflows-v2/usecases/patch-step-data/patch-step.usecase.ts deleted file mode 100644 index 26b2126c0d5..00000000000 --- a/apps/api/src/app/workflows-v2/usecases/patch-step-data/patch-step.usecase.ts +++ /dev/null @@ -1,113 +0,0 @@ -/* eslint-disable no-param-reassign */ -import { BadRequestException, Injectable } from '@nestjs/common'; -import { StepDataDto, UserSessionData } from '@novu/shared'; -import { NotificationStepEntity, NotificationTemplateEntity, NotificationTemplateRepository } from '@novu/dal'; -import { - GetWorkflowByIdsUseCase, - UpsertControlValuesCommand, - UpsertControlValuesUseCase, -} from '@novu/application-generic'; -import { PatchStepCommand } from './patch-step.command'; -import { BuildStepDataUsecase } from '../build-step-data'; -import { PostProcessWorkflowUpdate } from '../post-process-workflow-update'; - -type ValidNotificationWorkflow = { - currentStep: NonNullable; - workflow: NotificationTemplateEntity; -}; -@Injectable() -export class PatchStepUsecase { - constructor( - private getWorkflowByIdsUseCase: GetWorkflowByIdsUseCase, - private buildStepDataUsecase: BuildStepDataUsecase, - private notificationTemplateRepository: NotificationTemplateRepository, - private upsertControlValuesUseCase: UpsertControlValuesUseCase, - private postProcessWorkflowUpdate: PostProcessWorkflowUpdate - ) {} - - async execute(command: PatchStepCommand): Promise { - const persistedItems = await this.loadPersistedItems(command); - await this.patchFieldsOnPersistedItems(command, persistedItems); - const updatedWorkflow = await this.postProcessWorkflowUpdate.execute({ - workflow: persistedItems.workflow, - user: command.user, - }); - await this.persistWorkflow(updatedWorkflow, command.user); - - return await this.buildStepDataUsecase.execute(command); - } - - private async patchFieldsOnPersistedItems(command: PatchStepCommand, persistedItems: ValidNotificationWorkflow) { - if (command.name !== undefined) { - await this.updateName(persistedItems, command); - } - - if (command.controlValues !== undefined) { - await this.updateControlValues(persistedItems, command); - } - } - - private async loadPersistedItems(command: PatchStepCommand) { - const workflow = await this.fetchWorkflow(command); - - const { currentStep } = await this.findStepByStepId(command, workflow); - - return { workflow, currentStep }; - } - - private async updateName(persistedItems: ValidNotificationWorkflow, command: PatchStepCommand) { - persistedItems.currentStep.name = command.name; - } - - private async persistWorkflow( - workflowWithIssues: Partial, - userSessionData: UserSessionData - ) { - await this.notificationTemplateRepository.update( - { - _id: workflowWithIssues._id, - _environmentId: userSessionData.environmentId, - }, - { - ...workflowWithIssues, - } - ); - } - - private async fetchWorkflow(command: PatchStepCommand) { - return await this.getWorkflowByIdsUseCase.execute({ - workflowIdOrInternalId: command.workflowIdOrInternalId, - environmentId: command.user.environmentId, - organizationId: command.user.organizationId, - userId: command.user._id, - }); - } - - private async findStepByStepId(command: PatchStepCommand, workflow: NotificationTemplateEntity) { - const currentStep = workflow.steps.find( - (stepItem) => stepItem._id === command.stepIdOrInternalId || stepItem.stepId === command.stepIdOrInternalId - ); - - if (!currentStep) { - throw new BadRequestException({ - message: 'No step found', - stepIdOrInternalId: command.stepIdOrInternalId, - workflowId: command.workflowIdOrInternalId, - }); - } - - return { currentStep }; - } - - private async updateControlValues(persistedItems: ValidNotificationWorkflow, command: PatchStepCommand) { - return await this.upsertControlValuesUseCase.execute( - UpsertControlValuesCommand.create({ - organizationId: command.user.organizationId, - environmentId: command.user.environmentId, - notificationStepEntity: persistedItems.currentStep, - workflowId: persistedItems.workflow._id, - newControlValues: command.controlValues || {}, - }) - ); - } -} 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 a34b5f91014..20087c9b564 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 @@ -42,8 +42,7 @@ export class UpsertWorkflowUseCase { private notificationGroupRepository: NotificationGroupRepository, private workflowUpdatePostProcess: PostProcessWorkflowUpdate, private getWorkflowByIdsUseCase: GetWorkflowByIdsUseCase, - private getWorkflowUseCase: GetWorkflowUseCase, - private patchStepDataUsecase: PatchStepUsecase + private getWorkflowUseCase: GetWorkflowUseCase ) {} @InstrumentUsecase() diff --git a/apps/api/src/app/workflows-v2/workflow.controller.ts b/apps/api/src/app/workflows-v2/workflow.controller.ts index 17cee824ea7..bf71403ab4b 100644 --- a/apps/api/src/app/workflows-v2/workflow.controller.ts +++ b/apps/api/src/app/workflows-v2/workflow.controller.ts @@ -20,9 +20,7 @@ import { GeneratePreviewResponseDto, GetListQueryParams, ListWorkflowResponse, - PatchStepDataDto, PatchWorkflowDto, - StepDataDto, SyncWorkflowDto, UpdateWorkflowDto, UserSessionData, @@ -43,16 +41,9 @@ import { SyncToEnvironmentCommand } from './usecases/sync-to-environment/sync-to import { GeneratePreviewUsecase } from './usecases/generate-preview/generate-preview.usecase'; import { ParseSlugIdPipe } from './pipes/parse-slug-id.pipe'; import { ParseSlugEnvironmentIdPipe } from './pipes/parse-slug-env-id.pipe'; -import { - BuildStepDataCommand, - BuildStepDataUsecase, - BuildWorkflowTestDataUseCase, - WorkflowTestDataCommand, -} from './usecases'; +import { BuildWorkflowTestDataUseCase, WorkflowTestDataCommand } from './usecases'; import { GeneratePreviewCommand } from './usecases/generate-preview/generate-preview.command'; -import { PatchStepCommand } from './usecases/patch-step-data'; import { PatchWorkflowCommand, PatchWorkflowUsecase } from './usecases/patch-workflow'; -import { PatchStepUsecase } from './usecases/patch-step-data/patch-step.usecase'; @ApiCommonResponses() @Controller({ path: `/workflows`, version: '2' }) @@ -68,8 +59,6 @@ export class WorkflowController { private syncToEnvironmentUseCase: SyncToEnvironmentUseCase, private generatePreviewUseCase: GeneratePreviewUsecase, private buildWorkflowTestDataUseCase: BuildWorkflowTestDataUseCase, - private buildStepDataUsecase: BuildStepDataUsecase, - private patchStepDataUsecase: PatchStepUsecase, private patchWorkflowUsecase: PatchWorkflowUsecase ) {} @@ -189,36 +178,6 @@ export class WorkflowController { ); } - @Get('/:workflowId/steps/:stepId') - @UseGuards(UserAuthGuard) - async getWorkflowStepData( - @UserSession(ParseSlugEnvironmentIdPipe) user: UserSessionData, - @Param('workflowId', ParseSlugIdPipe) workflowIdOrInternalId: string, - @Param('stepId', ParseSlugIdPipe) stepIdOrInternalId: string - ): Promise { - return await this.buildStepDataUsecase.execute( - BuildStepDataCommand.create({ user, workflowIdOrInternalId, stepIdOrInternalId }) - ); - } - - @Patch('/:workflowId/steps/:stepId') - @UseGuards(UserAuthGuard) - async patchWorkflowStepData( - @UserSession(ParseSlugEnvironmentIdPipe) user: UserSessionData, - @Param('workflowId', ParseSlugIdPipe) workflowIdOrInternalId: string, - @Param('stepId', ParseSlugIdPipe) stepIdOrInternalId: string, - @Body() patchStepDataDto: PatchStepDataDto - ): Promise { - return await this.patchStepDataUsecase.execute( - PatchStepCommand.create({ - user, - workflowIdOrInternalId, - stepIdOrInternalId, - ...patchStepDataDto, - }) - ); - } - @Patch('/:workflowId') @UseGuards(UserAuthGuard) async patchWorkflow( diff --git a/apps/api/src/app/workflows-v2/workflow.module.ts b/apps/api/src/app/workflows-v2/workflow.module.ts index 4bd4396ea5f..dca385f1e14 100644 --- a/apps/api/src/app/workflows-v2/workflow.module.ts +++ b/apps/api/src/app/workflows-v2/workflow.module.ts @@ -38,7 +38,6 @@ import { BridgeModule } from '../bridge'; import { HydrateEmailSchemaUseCase } from '../environments-v1/usecases/output-renderers'; import { OverloadContentDataOnWorkflowUseCase } from './usecases/overload-content-data'; import { PatchWorkflowUsecase } from './usecases/patch-workflow'; -import { PatchStepUsecase } from './usecases/patch-step-data/patch-step.usecase'; import { BuildPayloadSchema } from './usecases/build-payload-schema/build-payload-schema.usecase'; const DAL_REPOSITORIES = [CommunityOrganizationRepository]; @@ -71,7 +70,6 @@ const DAL_REPOSITORIES = [CommunityOrganizationRepository]; PrepareAndValidateContentUsecase, ValidatePlaceholderUsecase, ExtractDefaultValuesFromSchemaUsecase, - PatchStepUsecase, PostProcessWorkflowUpdate, OverloadContentDataOnWorkflowUseCase, PatchWorkflowUsecase, diff --git a/packages/shared/src/dto/workflows/step.dto.ts b/packages/shared/src/dto/workflows/step.dto.ts index 156dda463e3..60518ac4284 100644 --- a/packages/shared/src/dto/workflows/step.dto.ts +++ b/packages/shared/src/dto/workflows/step.dto.ts @@ -24,11 +24,6 @@ export type StepCreateDto = StepDto & { controlValues?: Record; }; -export type PatchStepDataDto = { - name?: string; - controlValues?: Record; -}; - export type StepDto = { name: string; type: StepTypeEnum; From 4d4cba9458b2bb95fa2d3f64e4c5b2394d91004f Mon Sep 17 00:00:00 2001 From: Sokratis Vidros Date: Fri, 13 Dec 2024 00:35:46 +0200 Subject: [PATCH 2/2] chore(api): Simplify patch workflow endpoint Currently patch should only toggle the workflow active status --- .../patch-workflow/patch-workflow.command.ts | 14 +--- .../patch-workflow/patch-workflow.usecase.ts | 77 ++++++------------- .../upsert-workflow.usecase.ts | 46 +---------- .../shared/src/clients/workflows-client.ts | 10 --- 4 files changed, 28 insertions(+), 119 deletions(-) diff --git a/apps/api/src/app/workflows-v2/usecases/patch-workflow/patch-workflow.command.ts b/apps/api/src/app/workflows-v2/usecases/patch-workflow/patch-workflow.command.ts index c7ef8a7b4f3..daae2a392d0 100644 --- a/apps/api/src/app/workflows-v2/usecases/patch-workflow/patch-workflow.command.ts +++ b/apps/api/src/app/workflows-v2/usecases/patch-workflow/patch-workflow.command.ts @@ -1,5 +1,5 @@ import { EnvironmentWithUserObjectCommand } from '@novu/application-generic'; -import { IsArray, IsBoolean, IsNotEmpty, IsOptional, IsString } from 'class-validator'; +import { IsBoolean, IsNotEmpty, IsOptional, IsString } from 'class-validator'; export class PatchWorkflowCommand extends EnvironmentWithUserObjectCommand { @IsString() @@ -9,16 +9,4 @@ export class PatchWorkflowCommand extends EnvironmentWithUserObjectCommand { @IsBoolean() @IsOptional() active?: boolean; - - @IsString() - @IsOptional() - name?: string; - - @IsString() - @IsOptional() - description?: string; - - @IsArray() - @IsOptional() - tags?: string[]; } diff --git a/apps/api/src/app/workflows-v2/usecases/patch-workflow/patch-workflow.usecase.ts b/apps/api/src/app/workflows-v2/usecases/patch-workflow/patch-workflow.usecase.ts index be67d748646..b4ef2ea14c1 100644 --- a/apps/api/src/app/workflows-v2/usecases/patch-workflow/patch-workflow.usecase.ts +++ b/apps/api/src/app/workflows-v2/usecases/patch-workflow/patch-workflow.usecase.ts @@ -1,78 +1,51 @@ -import { Injectable } from '@nestjs/common'; -import { UserSessionData, WorkflowResponseDto } from '@novu/shared'; -import { NotificationTemplateEntity, NotificationTemplateRepository } from '@novu/dal'; -import { GetWorkflowByIdsUseCase, WorkflowInternalResponseDto } from '@novu/application-generic'; -import { PostProcessWorkflowUpdate } from '../post-process-workflow-update'; +import { Injectable, NotFoundException } from '@nestjs/common'; +import { WorkflowResponseDto } from '@novu/shared'; +import { NotificationTemplateRepository } from '@novu/dal'; import { PatchWorkflowCommand } from './patch-workflow.command'; import { GetWorkflowUseCase } from '../get-workflow'; @Injectable() export class PatchWorkflowUsecase { constructor( - private getWorkflowByIdsUseCase: GetWorkflowByIdsUseCase, private notificationTemplateRepository: NotificationTemplateRepository, - private postProcessWorkflowUpdate: PostProcessWorkflowUpdate, private getWorkflowUseCase: GetWorkflowUseCase ) {} + // NOTICE: The TODOS in this usecase explore a different way to fetch and update a resource, leveraging model classes async execute(command: PatchWorkflowCommand): Promise { - const persistedWorkflow = await this.fetchWorkflow(command); - let transientWorkflow = this.patchWorkflowFields(persistedWorkflow, command); - - transientWorkflow = await this.postProcessWorkflowUpdate.execute({ - workflow: transientWorkflow, - user: command.user, + // TODO: Return a Mongoose model + const workflow = await this.notificationTemplateRepository.findOne({ + _id: command.workflowIdOrInternalId, + _environmentId: command.user.environmentId, }); - await this.persistWorkflow(transientWorkflow, command.user); - - return await this.getWorkflowUseCase.execute({ - workflowIdOrInternalId: command.workflowIdOrInternalId, - user: command.user, - }); - } - - private patchWorkflowFields( - persistedWorkflow: WorkflowInternalResponseDto, - command: PatchWorkflowCommand - ): WorkflowInternalResponseDto { - const transientWorkflow = { ...persistedWorkflow }; - if (command.active !== undefined && command.active !== null) { - transientWorkflow.active = command.active; - } - - if (command.name !== undefined && command.name !== null) { - transientWorkflow.name = command.name; - } - if (command.description !== undefined && command.description !== null) { - transientWorkflow.description = command.description; + if (!workflow) { + throw new NotFoundException({ + message: 'Workflow cannot be found', + workflowId: command.workflowIdOrInternalId, + }); } - if (command.tags !== undefined && command.tags !== null) { - transientWorkflow.tags = command.tags; + if (typeof command.active === 'boolean') { + workflow.active = command.active; } - return transientWorkflow; - } - - private async persistWorkflow(workflowWithIssues: NotificationTemplateEntity, userSessionData: UserSessionData) { + // TODO: Update the workflow using the Mongoose model.save() await this.notificationTemplateRepository.update( { - _id: workflowWithIssues._id, - _environmentId: userSessionData.environmentId, + _id: workflow._id, + _environmentId: command.user.environmentId, }, - { - ...workflowWithIssues, - } + workflow ); - } - private async fetchWorkflow(command: PatchWorkflowCommand): Promise { - return await this.getWorkflowByIdsUseCase.execute({ + /* + * TODO: Convert to a serializer that also enriches the workflow with necessary data from other collections + * such as preferences, message templates, etc... + */ + return await this.getWorkflowUseCase.execute({ workflowIdOrInternalId: command.workflowIdOrInternalId, - environmentId: command.user.environmentId, - organizationId: command.user.organizationId, - userId: command.user._id, + user: command.user, }); } } 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 20087c9b564..d52e6d7d373 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 @@ -29,7 +29,6 @@ import { import { BadRequestException, Injectable } from '@nestjs/common'; import { UpsertWorkflowCommand } from './upsert-workflow.command'; import { stepTypeToControlSchema } from '../../shared'; -import { PatchStepUsecase } from '../patch-step-data'; import { PostProcessWorkflowUpdate } from '../post-process-workflow-update'; import { GetWorkflowCommand, GetWorkflowUseCase } from '../get-workflow'; import { UpsertWorkflowDataCommand } from './upsert-workflow-data.command'; @@ -48,22 +47,19 @@ export class UpsertWorkflowUseCase { @InstrumentUsecase() async execute(command: UpsertWorkflowCommand): Promise { const workflowForUpdate = await this.queryWorkflow(command); - let persistedWorkflow = await this.createOrUpdateWorkflow(workflowForUpdate, command); - persistedWorkflow = await this.upsertControlValues(persistedWorkflow, command); + const persistedWorkflow = await this.createOrUpdateWorkflow(workflowForUpdate, command); const validatedWorkflowWithIssues = await this.workflowUpdatePostProcess.execute({ user: command.user, workflow: persistedWorkflow, }); await this.persistWorkflow(validatedWorkflowWithIssues); - const workflow = await this.getWorkflowUseCase.execute( + return await this.getWorkflowUseCase.execute( GetWorkflowCommand.create({ workflowIdOrInternalId: validatedWorkflowWithIssues._id, user: command.user, }) ); - - return workflow; } @Instrument() @@ -286,44 +282,6 @@ export class UpsertWorkflowUseCase { ) )?._id; } - - /** - * @deprecated This method will be removed in future versions. - * Please use `the patch step data instead, do not add here anything` instead. - */ - @Instrument() - private async upsertControlValues( - workflow: NotificationTemplateEntity, - command: UpsertWorkflowCommand - ): Promise { - for (const step of workflow.steps) { - const controlValues = this.findControlValueInRequest(step, command.workflowDto.steps); - if (!controlValues) { - continue; - } - await this.patchStepDataUsecase.execute({ - controlValues, - workflowIdOrInternalId: workflow._id, - stepIdOrInternalId: step._templateId, - user: command.user, - }); - } - - return await this.getWorkflow(workflow._id, command); - } - - private findControlValueInRequest( - step: NotificationStepEntity, - steps: (StepCreateDto | StepUpdateDto)[] | StepCreateDto[] - ): Record | undefined { - return steps.find((stepRequest) => { - if (this.isStepUpdateDto(stepRequest)) { - return stepRequest._id === step._templateId; - } - - return stepRequest.name === step.name; - })?.controlValues; - } } function isWorkflowUpdateDto(workflowDto: UpsertWorkflowDataCommand, id?: string): workflowDto is UpdateWorkflowDto { diff --git a/packages/shared/src/clients/workflows-client.ts b/packages/shared/src/clients/workflows-client.ts index 19c7b258bd3..eb4ba574446 100644 --- a/packages/shared/src/clients/workflows-client.ts +++ b/packages/shared/src/clients/workflows-client.ts @@ -5,7 +5,6 @@ import { GeneratePreviewResponseDto, GetListQueryParams, ListWorkflowResponse, - PatchStepDataDto, PatchWorkflowDto, StepDataDto, SyncWorkflowDto, @@ -48,14 +47,6 @@ export const createWorkflowClient = (baseUrl: string, headers: HeadersInit = {}) return await baseClient.safeGet(`/v2/workflows/${workflowId}/steps/${stepId}`); }; - const patchWorkflowStepData = async ( - workflowId: string, - stepId: string, - patchStepDataDto: PatchStepDataDto - ): Promise> => { - return await baseClient.safePatch(`/v2/workflows/${workflowId}/steps/${stepId}`, patchStepDataDto); - }; - const patchWorkflow = async ( workflowId: string, patchWorkflowDto: PatchWorkflowDto @@ -147,7 +138,6 @@ export const createWorkflowClient = (baseUrl: string, headers: HeadersInit = {}) searchWorkflows, getWorkflowTestData, getWorkflowStepData, - patchWorkflowStepData, patchWorkflow, searchWorkflowsV1, createWorkflowsV1,