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.

Sign up for GitHub

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

fix(api-service): translation groups causing long response time in v2 create/update workflow #7450

Merged
merged 1 commit into from
Jan 7, 2025
Merged

fix(api-service): translation groups causing long response time in v2 create/update workflow #7450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 8 additions & 2 deletions libs/application-generic/src/usecases/create-workflow/create-workflow.usecase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import {
WorkflowInternalResponseDto,
GetWorkflowByIdsUseCase,
} from '../workflow';
import { Instrument, InstrumentUsecase } from '../../instrumentation';

/**
* @deprecated - use `UpsertWorkflow` instead
Expand All @@ -80,6 +81,7 @@ export class CreateWorkflow {
private getWorkflowByIdsUseCase: GetWorkflowByIdsUseCase,
) {}

@InstrumentUsecase()
async execute(
usecaseCommand: CreateWorkflowCommand,
): Promise<WorkflowInternalResponseDto> {
Expand Down Expand Up @@ -114,8 +116,9 @@ export class CreateWorkflow {

try {
if (
process.env.NOVU_ENTERPRISE === 'true' ||
process.env.CI_EE_TEST === 'true'
(process.env.NOVU_ENTERPRISE === 'true' ||
process.env.CI_EE_TEST === 'true') &&
storedWorkflow.origin === WorkflowOriginEnum.NOVU_CLOUD_V1
Comment on lines +119 to +121
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only run the translation analytics code in v1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally, we should not load the translations during the message sent in the SendMessageBase usecase, but I haven't added any logic there because there is no workflow information at that stage, like the NotificationStepEntity is missing the workflow info and particularly the origin field.
To do that, it might require more work.

) {
if (!require('@novu/ee-shared-services')?.TranslationsService) {
throw new PlatformException('Translation module is not loaded');
Expand Down Expand Up @@ -199,6 +202,7 @@ export class CreateWorkflow {
}
}

@Instrument()
private async createNotificationTrigger(
command: CreateWorkflowCommand,
triggerIdentifier: string,
Expand Down Expand Up @@ -316,6 +320,7 @@ export class CreateWorkflow {
}
}

@Instrument()
private async storeWorkflow(
command: CreateWorkflowCommand,
templateSteps: INotificationTemplateStep[],
Expand Down Expand Up @@ -412,6 +417,7 @@ export class CreateWorkflow {
);
}

@Instrument()
private async storeTemplateSteps(
command: CreateWorkflowCommand,
parentChangeId: string,
Expand Down
12 changes: 10 additions & 2 deletions libs/application-generic/src/usecases/workflow/update-workflow/update-workflow.usecase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
ControlValuesLevelEnum,
isBridgeWorkflow,
PreferencesTypeEnum,
WorkflowOriginEnum,
} from '@novu/shared';

import {
Expand Down Expand Up @@ -61,6 +62,7 @@ import {
UpdateMessageTemplate,
UpdateMessageTemplateCommand,
} from '../../message-template';
import { Instrument, InstrumentUsecase } from '../../../instrumentation';

/**
* @deprecated - use `UpsertWorkflow` instead
Expand Down Expand Up @@ -92,6 +94,7 @@ export class UpdateWorkflow {
private controlValuesRepository: ControlValuesRepository,
) {}

@InstrumentUsecase()
async execute(
command: UpdateWorkflowCommand,
): Promise<WorkflowInternalResponseDto> {
Expand Down Expand Up @@ -370,8 +373,10 @@ export class UpdateWorkflow {

try {
if (
process.env.NOVU_ENTERPRISE === 'true' ||
process.env.CI_EE_TEST === 'true'
(process.env.NOVU_ENTERPRISE === 'true' ||
process.env.CI_EE_TEST === 'true') &&
notificationTemplateWithStepTemplate.origin ===
WorkflowOriginEnum.NOVU_CLOUD_V1
) {
if (!require('@novu/ee-shared-services')?.TranslationsService) {
throw new PlatformException('Translation module is not loaded');
Expand Down Expand Up @@ -420,6 +425,7 @@ export class UpdateWorkflow {
}
}

@Instrument()
private async updateMessageTemplates(
steps: NotificationStep[],
command: UpdateWorkflowCommand,
Expand Down Expand Up @@ -518,6 +524,7 @@ export class UpdateWorkflow {
return templateMessages;
}

@Instrument()
private updateTriggers(
updatePayload: Partial<WorkflowInternalResponseDto>,
steps: NotificationStep[],
Expand Down Expand Up @@ -735,6 +742,7 @@ export class UpdateWorkflow {
return variantsList;
}

@Instrument()
private async deleteRemovedSteps(
existingSteps: NotificationStepEntity[] | StepVariantEntity[] | undefined,
command: UpdateWorkflowCommand,
Expand Down
Loading