Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(api): add full step data to workflow dto; refactor #7235

Merged
merged 4 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {
PreferencesResponseDto,
RuntimeIssue,
ShortIsPrefixEnum,
StepResponseDto,
StepDataDto,
StepTypeEnum,
WorkflowCreateAndUpdateKeys,
WorkflowListResponseDto,
Expand All @@ -15,7 +15,10 @@ import { NotificationStepEntity, NotificationTemplateEntity } from '@novu/dal';
import { WorkflowInternalResponseDto } from '@novu/application-generic';
import { buildSlug } from '../../shared/helpers/build-slug';

export function toResponseWorkflowDto(workflow: WorkflowInternalResponseDto): WorkflowResponseDto {
export function toResponseWorkflowDto(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the point was letting this map to dto, if you send the DTO pre cooked I think it's taking the string out of the funciton no? I would expect to get the entity list and maps of the additinoal data you want to bake in

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still maps the workflow to DTO as name of the method suggests, only the steps are coming in already in "DTO form".

We simply don't need to do any mapping for the steps for now, its already in viable form, which is good. Each layer of mapping just adds an overhead

workflow: WorkflowInternalResponseDto,
steps: StepDataDto[]
): WorkflowResponseDto {
const preferencesDto: PreferencesResponseDto = {
user: workflow.userPreferences,
default: workflow.defaultPreferences,
Expand All @@ -30,7 +33,7 @@ export function toResponseWorkflowDto(workflow: WorkflowInternalResponseDto): Wo
tags: workflow.tags,
active: workflow.active,
preferences: preferencesDto,
steps: getSteps(workflow),
steps: steps,
description: workflow.description,
origin: computeOrigin(workflow),
updatedAt: workflow.updatedAt || 'Missing Updated At',
Expand All @@ -40,16 +43,6 @@ export function toResponseWorkflowDto(workflow: WorkflowInternalResponseDto): Wo
};
}

function getSteps(template: NotificationTemplateEntity) {
const steps: StepResponseDto[] = [];
for (const step of template.steps) {
const stepResponseDto = toStepResponseDto(step);
steps.push(stepResponseDto);
}

return steps;
}

function toMinifiedWorkflowDto(template: NotificationTemplateEntity): WorkflowListResponseDto {
const workflowName = template.name || 'Missing Name';

Expand All @@ -71,19 +64,6 @@ export function toWorkflowsMinifiedDtos(templates: NotificationTemplateEntity[])
return templates.map(toMinifiedWorkflowDto);
}

function toStepResponseDto(persistedStep: NotificationStepEntity): StepResponseDto {
const stepName = persistedStep.name || 'Missing Name';

return {
_id: persistedStep._templateId,
slug: buildSlug(stepName, ShortIsPrefixEnum.STEP, persistedStep._templateId),
name: stepName,
stepId: persistedStep.stepId || 'Missing Step Id',
type: persistedStep.template?.type || StepTypeEnum.EMAIL,
issues: persistedStep.issues,
} satisfies StepResponseDto;
}
Comment on lines -74 to -85
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this directly to the build-step-data use-case, because its always used together with the use-case.

If the usecase for unsanitized step data emerges, then we can split it again or create GetStepDataInternal usecase.

Now I don't think its even possible to create step without name or stepId or without type, it seems to be here only because of tests. These things should be validated upon creation and not rely on fallbacks anyway.


function buildStepTypeOverview(step: NotificationStepEntity): StepTypeEnum | undefined {
return step.template?.type;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { BadRequestException, Injectable } from '@nestjs/common';
import { ControlValuesLevelEnum, StepDataDto, WorkflowOriginEnum } from '@novu/shared';
import { ControlValuesLevelEnum, ShortIsPrefixEnum, StepDataDto, WorkflowOriginEnum } from '@novu/shared';
import { ControlValuesRepository, NotificationStepEntity, NotificationTemplateEntity } from '@novu/dal';
import { GetWorkflowByIdsUseCase, Instrument, InstrumentUsecase } from '@novu/application-generic';
import { BuildStepDataCommand } from './build-step-data.command';
import { InvalidStepException } from '../../exceptions/invalid-step.exception';
import { BuildAvailableVariableSchemaUsecase } from '../build-variable-schema';
import { buildSlug } from '../../../shared/helpers/build-slug';

@Injectable()
export class BuildStepDataUsecase {
Expand Down Expand Up @@ -43,6 +44,7 @@ export class BuildStepDataUsecase {
workflow,
}),
name: currentStep.name,
slug: buildSlug(currentStep.name, ShortIsPrefixEnum.STEP, currentStep._templateId),
_id: currentStep._templateId,
stepId: currentStep.stepId,
type: currentStep.template?.type,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,25 @@
import { Injectable } from '@nestjs/common';
import { Injectable, InternalServerErrorException } from '@nestjs/common';

import { WorkflowResponseDto } from '@novu/shared';
import { GetWorkflowByIdsCommand, GetWorkflowByIdsUseCase, InstrumentUsecase } from '@novu/application-generic';
import { StepDataDto, UserSessionData, WorkflowResponseDto } from '@novu/shared';
import {
GetWorkflowByIdsCommand,
GetWorkflowByIdsUseCase,
InstrumentUsecase,
WorkflowInternalResponseDto,
} from '@novu/application-generic';

import { GetWorkflowCommand } from './get-workflow.command';
import { toResponseWorkflowDto } from '../../mappers/notification-template-mapper';
import { BuildStepDataUsecase } from '../build-step-data/build-step-data.usecase';
import { BuildStepDataCommand } from '../build-step-data/build-step-data.command';
import { NotificationStepEntity } from '@novu/dal';

@Injectable()
export class GetWorkflowUseCase {
constructor(private getWorkflowByIdsUseCase: GetWorkflowByIdsUseCase) {}
constructor(
private getWorkflowByIdsUseCase: GetWorkflowByIdsUseCase,
private buildStepDataUsecase: BuildStepDataUsecase
) {}

@InstrumentUsecase()
async execute(command: GetWorkflowCommand): Promise<WorkflowResponseDto> {
Expand All @@ -21,6 +32,42 @@ export class GetWorkflowUseCase {
})
);

return toResponseWorkflowDto(workflowEntity);
const fullSteps = await this.getFullWorkflowSteps(workflowEntity, command.user);

return toResponseWorkflowDto(workflowEntity, fullSteps);
}

private async getFullWorkflowSteps(
workflow: WorkflowInternalResponseDto,
user: UserSessionData
): Promise<StepDataDto[]> {
tatarco marked this conversation as resolved.
Show resolved Hide resolved
const stepPromises = workflow.steps.map((step: NotificationStepEntity & { _id: string }) =>
this.buildStepForWorkflow(workflow, step, user)
);

return Promise.all(stepPromises);
}

private async buildStepForWorkflow(
workflow: WorkflowInternalResponseDto,
step: NotificationStepEntity & { _id: string },
user: UserSessionData
): Promise<StepDataDto> {
try {
return await this.buildStepDataUsecase.execute(
BuildStepDataCommand.create({
workflowIdOrInternalId: workflow._id,
stepIdOrInternalId: step._id,
user,
})
);
} catch (error) {
throw new InternalServerErrorException({
message: 'Failed to build workflow step',
workflowId: workflow._id,
stepId: step._id,
error: error.message,
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
PreferencesTypeEnum,
StepCreateDto,
StepDataDto,
StepResponseDto,
StepUpdateDto,
UpdateWorkflowDto,
WorkflowCreationSourceEnum,
Expand All @@ -16,7 +15,6 @@ import { Instrument, InstrumentUsecase } from '@novu/application-generic';
import { SyncToEnvironmentCommand } from './sync-to-environment.command';
import { GetWorkflowCommand, GetWorkflowUseCase } from '../get-workflow';
import { UpsertWorkflowCommand, UpsertWorkflowUseCase } from '../upsert-workflow';
import { BuildStepDataUsecase } from '../build-step-data';

/**
* This usecase is used to sync a workflow from one environment to another.
Expand All @@ -32,8 +30,7 @@ export class SyncToEnvironmentUseCase {
constructor(
private getWorkflowUseCase: GetWorkflowUseCase,
private preferencesRepository: PreferencesRepository,
private upsertWorkflowUseCase: UpsertWorkflowUseCase,
private buildStepDataUsecase: BuildStepDataUsecase
private upsertWorkflowUseCase: UpsertWorkflowUseCase
) {}

@InstrumentUsecase()
Expand All @@ -46,7 +43,7 @@ export class SyncToEnvironmentUseCase {
const preferencesToClone = await this.getWorkflowPreferences(originWorkflow._id, command.user.environmentId);
const externalId = originWorkflow.workflowId;
const targetWorkflow = await this.findWorkflowInTargetEnvironment(command, externalId);
const workflowDto = await this.buildRequestDto(originWorkflow, preferencesToClone, command, targetWorkflow);
const workflowDto = await this.buildRequestDto(originWorkflow, preferencesToClone, targetWorkflow);

return await this.upsertWorkflowUseCase.execute(
UpsertWorkflowCommand.create({
Expand All @@ -61,14 +58,13 @@ export class SyncToEnvironmentUseCase {
private async buildRequestDto(
originWorkflow: WorkflowResponseDto,
preferencesToClone: PreferencesEntity[],
command: SyncToEnvironmentCommand,
targetWorkflow?: WorkflowResponseDto
) {
if (targetWorkflow) {
return await this.mapWorkflowToUpdateWorkflowDto(originWorkflow, targetWorkflow, preferencesToClone, command);
return await this.mapWorkflowToUpdateWorkflowDto(originWorkflow, targetWorkflow, preferencesToClone);
}

return await this.mapWorkflowToCreateWorkflowDto(originWorkflow, preferencesToClone, command);
return await this.mapWorkflowToCreateWorkflowDto(originWorkflow, preferencesToClone);
}

@Instrument()
Expand All @@ -87,7 +83,7 @@ export class SyncToEnvironmentUseCase {
externalId: string
): Promise<WorkflowResponseDto | undefined> {
try {
return await this.getWorkflowUseCase.execute(
return this.getWorkflowUseCase.execute(
GetWorkflowCommand.create({
user: { ...command.user, environmentId: command.targetEnvironmentId },
workflowIdOrInternalId: externalId,
Expand All @@ -101,8 +97,7 @@ export class SyncToEnvironmentUseCase {
@Instrument()
private async mapWorkflowToCreateWorkflowDto(
originWorkflow: WorkflowResponseDto,
preferences: PreferencesEntity[],
command: SyncToEnvironmentCommand
preferences: PreferencesEntity[]
): Promise<CreateWorkflowDto> {
return {
workflowId: originWorkflow.workflowId,
Expand All @@ -111,83 +106,52 @@ export class SyncToEnvironmentUseCase {
tags: originWorkflow.tags,
description: originWorkflow.description,
__source: WorkflowCreationSourceEnum.DASHBOARD,
steps: await this.mapStepsToDto(originWorkflow.steps, command),
steps: await this.mapStepsToCreateOrUpdateDto(originWorkflow.steps),
preferences: this.mapPreferences(preferences),
};
}

@Instrument()
private async mapWorkflowToUpdateWorkflowDto(
originWorkflow: WorkflowResponseDto,
existingWorkflowInProd: WorkflowResponseDto | undefined,
preferencesToClone: PreferencesEntity[],
command: SyncToEnvironmentCommand
existingTargetEnvWorkflow: WorkflowResponseDto | undefined,
preferencesToClone: PreferencesEntity[]
): Promise<UpdateWorkflowDto> {
return {
workflowId: originWorkflow.workflowId,
name: originWorkflow.name,
active: originWorkflow.active,
tags: originWorkflow.tags,
description: originWorkflow.description,
steps: await this.mapStepsToDto(originWorkflow.steps, command, existingWorkflowInProd?.steps),
steps: await this.mapStepsToCreateOrUpdateDto(originWorkflow.steps, existingTargetEnvWorkflow?.steps),
preferences: this.mapPreferences(preferencesToClone),
};
}

@Instrument()
private async mapStepsToDto(
originSteps: StepResponseDto[],
command: SyncToEnvironmentCommand,
targetWorkflowSteps?: StepResponseDto[]
private async mapStepsToCreateOrUpdateDto(
originSteps: StepDataDto[],
targetEnvSteps?: StepDataDto[]
): Promise<(StepUpdateDto | StepCreateDto)[]> {
const augmentedSteps: (StepUpdateDto | StepCreateDto)[] = [];
for (const originStep of originSteps) {
const idAsOptionalObject = this.prodDbIdAsOptionalObject(originStep, targetWorkflowSteps);
const stepDataDto = await this.buildStepDataUsecase.execute({
workflowIdOrInternalId: command.workflowIdOrInternalId,
stepIdOrInternalId: originStep._id || originStep.stepId,
user: command.user,
});

augmentedSteps.push(this.buildSingleStepRequest(idAsOptionalObject, originStep, stepDataDto));
}
return originSteps.map((sourceStep) => {
// if we find matching step in target environment, we are updating
const targetEnvStepInternalId = targetEnvSteps?.find(
(targetStep) => targetStep.stepId === sourceStep.stepId
)?._id;

return augmentedSteps;
return this.buildStepCreateOrUpdateDto(sourceStep, targetEnvStepInternalId);
});
}
/*
* 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 {

private buildStepCreateOrUpdateDto(step: StepDataDto, existingInternalId?: string): StepUpdateDto | StepCreateDto {
return {
...idAsOptionalObject,
...(existingInternalId && { _id: existingInternalId }),
name: step.name ?? '',
type: step.type,
controlValues: stepDataDto.controls.values ?? {},
controlValues: step.controls.values ?? {},
};
}

private prodDbIdAsOptionalObject(originStep: StepResponseDto, targetWorkflowSteps?: StepResponseDto[]) {
const prodDatabaseId = this.findDatabaseIdInProdByExternalId(originStep, targetWorkflowSteps);

if (prodDatabaseId) {
return {
_id: prodDatabaseId,
};
} else {
return {};
}
}

private findDatabaseIdInProdByExternalId(originStep: StepResponseDto, targetWorkflowSteps?: StepResponseDto[]) {
return targetWorkflowSteps?.find((targetStep) => targetStep.stepId === originStep.stepId)?._id;
}

private mapPreferences(preferences: PreferencesEntity[]): {
user: WorkflowPreferences | null;
workflow: WorkflowPreferences | null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ import {
} from '@novu/application-generic';
import { BadRequestException, Injectable } from '@nestjs/common';
import { UpsertWorkflowCommand } from './upsert-workflow.command';
import { toResponseWorkflowDto } from '../../mappers/notification-template-mapper';
import { stepTypeToDefaultDashboardControlSchema } from '../../shared';
import { PatchStepUsecase } from '../patch-step-data';
import { PostProcessWorkflowUpdate } from '../post-process-workflow-update';
import { GetWorkflowUseCase } from '../get-workflow/get-workflow.usecase';
import { GetWorkflowCommand } from '../get-workflow/get-workflow.command';

@Injectable()
export class UpsertWorkflowUseCase {
Expand All @@ -41,6 +42,7 @@ export class UpsertWorkflowUseCase {
private notificationGroupRepository: NotificationGroupRepository,
private workflowUpdatePostProcess: PostProcessWorkflowUpdate,
private getWorkflowByIdsUseCase: GetWorkflowByIdsUseCase,
private getWorkflowUseCase: GetWorkflowUseCase,
private patchStepDataUsecase: PatchStepUsecase
) {}

Expand All @@ -54,9 +56,15 @@ export class UpsertWorkflowUseCase {
workflow: persistedWorkflow,
});
await this.persistWorkflow(validatedWorkflowWithIssues);
persistedWorkflow = await this.getWorkflow(validatedWorkflowWithIssues._id, command);

return toResponseWorkflowDto(persistedWorkflow);
const workflow = await this.getWorkflowUseCase.execute(
GetWorkflowCommand.create({
workflowIdOrInternalId: validatedWorkflowWithIssues._id,
user: command.user,
})
);

return workflow;
}

@Instrument()
Expand Down
Loading
Loading