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

fix(api): bug bash preview issues resolved #6904

Merged
merged 4 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,7 @@
"packages/shared/src/types/timezones/timezones.types.ts",
"*.riv",
"websockets",
"apps/api/src/app/workflows-v2/usecases/validate-content/validate-placeholders/validate-placeholder.usecase.ts",
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add /* cspell:disable */ on this file, i would not put here files that can be moved in the source code in the future.

".env",
".env.development",
".env.local",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ import { BaseCommand } from '@novu/application-generic';
import { FullPayloadForRender } from './render-command';

export class ExpandEmailEditorSchemaCommand extends BaseCommand {
body: string;
emailEditorJson: string;
fullPayloadForRender: FullPayloadForRender;
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export class ExpandEmailEditorSchemaUsecase {
}
private hydrate(command: ExpandEmailEditorSchemaCommand) {
const { hydratedEmailSchema } = this.hydrateEmailSchemaUseCase.execute({
emailEditor: command.body,
emailEditor: command.emailEditorJson,
fullPayloadForRender: command.fullPayloadForRender,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,51 +3,65 @@ import { Injectable } from '@nestjs/common';
import { PreviewPayload, TipTapNode } from '@novu/shared';
import { z } from 'zod';
import { HydrateEmailSchemaCommand } from './hydrate-email-schema.command';
import { PlaceholderAggregation } from '../../../workflows-v2/usecases';

@Injectable()
export class HydrateEmailSchemaUseCase {
execute(command: HydrateEmailSchemaCommand): {
hydratedEmailSchema: TipTapNode;
nestedPayload: Record<string, unknown>;
placeholderAggregation: PlaceholderAggregation;
} {
const defaultPayload: Record<string, unknown> = {};
const placeholderAggregation: PlaceholderAggregation = {
nestedForPlaceholders: {},
regularPlaceholdersToDefaultValue: {},
};
const emailEditorSchema: TipTapNode = TipTapSchema.parse(JSON.parse(command.emailEditor));
if (emailEditorSchema.content) {
this.transformContentInPlace(emailEditorSchema.content, defaultPayload, command.fullPayloadForRender);
this.transformContentInPlace(emailEditorSchema.content, command.fullPayloadForRender, placeholderAggregation);
}

return { hydratedEmailSchema: emailEditorSchema, nestedPayload: this.flattenToNested(defaultPayload) };
return {
hydratedEmailSchema: emailEditorSchema,
placeholderAggregation,
};
}

private variableLogic(
masterPayload: PreviewPayload,
node: TipTapNode & { attrs: { id: string } },
defaultPayload: Record<string, unknown>,
node: TipTapNode & {
attrs: { id: string };
},
content: TipTapNode[],
index: number
index: number,
placeholderAggregation: PlaceholderAggregation
) {
const resolvedValueRegularPlaceholder = this.getResolvedValueRegularPlaceholder(masterPayload, node);
defaultPayload[node.attrs.id] = resolvedValueRegularPlaceholder;
const resolvedValueRegularPlaceholder = this.getResolvedValueRegularPlaceholder(
masterPayload,
node,
placeholderAggregation
);
content[index] = {
type: 'text',
text: resolvedValueRegularPlaceholder,
};
}

private forNodeLogic(
node: TipTapNode & { attrs: { each: string } },
node: TipTapNode & {
attrs: { each: string };
},
masterPayload: PreviewPayload,
defaultPayload: Record<string, unknown>,
content: TipTapNode[],
index: number
index: number,
placeholderAggregation: PlaceholderAggregation
) {
const itemPointerToDefaultRecord = this.collectAllItemPlaceholders(node);
const resolvedValueForPlaceholder = this.getResolvedValueForPlaceholder(
masterPayload,
node,
itemPointerToDefaultRecord
itemPointerToDefaultRecord,
placeholderAggregation
);
defaultPayload[node.attrs.each] = resolvedValueForPlaceholder;
content[index] = {
type: 'for',
attrs: { each: resolvedValueForPlaceholder },
Expand All @@ -57,31 +71,31 @@ export class HydrateEmailSchemaUseCase {

private showLogic(
masterPayload: PreviewPayload,
node: TipTapNode & { attrs: { show: string } },
defaultPayload: Record<string, unknown>
node: TipTapNode & {
attrs: { show: string };
},
placeholderAggregation: PlaceholderAggregation
) {
const resolvedValueShowPlaceholder = this.getResolvedValueShowPlaceholder(masterPayload, node);
defaultPayload[node.attrs.show] = resolvedValueShowPlaceholder;
node.attrs.show = resolvedValueShowPlaceholder;
node.attrs.show = this.getResolvedValueShowPlaceholder(masterPayload, node, placeholderAggregation);
}

private transformContentInPlace(
content: TipTapNode[],
defaultPayload: Record<string, unknown>,
masterPayload: PreviewPayload
masterPayload: PreviewPayload,
placeholderAggregation: PlaceholderAggregation
) {
content.forEach((node, index) => {
if (this.isVariableNode(node)) {
this.variableLogic(masterPayload, node, defaultPayload, content, index);
this.variableLogic(masterPayload, node, content, index, placeholderAggregation);
}
if (this.isForNode(node)) {
this.forNodeLogic(node, masterPayload, defaultPayload, content, index);
this.forNodeLogic(node, masterPayload, content, index, placeholderAggregation);
}
if (this.isShowNode(node)) {
this.showLogic(masterPayload, node, defaultPayload);
this.showLogic(masterPayload, node, placeholderAggregation);
}
if (node.content) {
this.transformContentInPlace(node.content, defaultPayload, masterPayload);
this.transformContentInPlace(node.content, masterPayload, placeholderAggregation);
}
});
}
Expand All @@ -98,53 +112,65 @@ export class HydrateEmailSchemaUseCase {
return !!(node.type === 'variable' && node.attrs && 'id' in node.attrs && typeof node.attrs.id === 'string');
}

private getResolvedValueRegularPlaceholder(masterPayload: PreviewPayload, node) {
private getResolvedValueRegularPlaceholder(
masterPayload: PreviewPayload,
node,
placeholderAggregation: PlaceholderAggregation
) {
const resolvedValue = this.getValueByPath(masterPayload, node.attrs.id);
const { fallback } = node.attrs;

return resolvedValue || fallback || `{{${node.attrs.id}}}`;
const finalValue = resolvedValue || fallback || `{{${node.attrs.id}}}`;
placeholderAggregation.regularPlaceholdersToDefaultValue[`{{${node.attrs.id}}}`] = finalValue;

return finalValue;
}

private getResolvedValueShowPlaceholder(masterPayload: PreviewPayload, node) {
private getResolvedValueShowPlaceholder(
masterPayload: PreviewPayload,
node,
placeholderAggregation: PlaceholderAggregation
) {
const resolvedValue = this.getValueByPath(masterPayload, node.attrs.show);
const { fallback } = node.attrs;

return resolvedValue || fallback || `true`;
}
const finalValue = resolvedValue || fallback || `true`;
placeholderAggregation.regularPlaceholdersToDefaultValue[`{{${node.attrs.show}}}`] = finalValue;

private flattenToNested(flatJson: Record<string, any>): Record<string, any> {
const nestedJson: Record<string, any> = {};
// eslint-disable-next-line guard-for-in
for (const key in flatJson) {
const keys = key.split('.');
keys.reduce((acc, part, index) => {
if (index === keys.length - 1) {
acc[part] = flatJson[key];
} else if (!acc[part]) {
acc[part] = {};
}

return acc[part];
}, nestedJson);
}

return nestedJson;
return finalValue;
}

private getResolvedValueForPlaceholder(
masterPayload: PreviewPayload,
node: TipTapNode & { attrs: { each: string } },
itemPointerToDefaultRecord: Record<string, string>
node: TipTapNode & {
attrs: { each: string };
},
itemPointerToDefaultRecord: Record<string, string>,
placeholderAggregation: PlaceholderAggregation
) {
const resolvedValue = this.getValueByPath(masterPayload, node.attrs.each);
let resolvedValueIfFound = this.getValueByPath(masterPayload, node.attrs.each);

if (!resolvedValue) {
return [this.buildElement(itemPointerToDefaultRecord, '1'), this.buildElement(itemPointerToDefaultRecord, '2')];
if (!resolvedValueIfFound) {
resolvedValueIfFound = [
this.buildElement(itemPointerToDefaultRecord, '1'),
this.buildElement(itemPointerToDefaultRecord, '2'),
];
}
placeholderAggregation.nestedForPlaceholders[`{{${node.attrs.each}}}`] =
this.buildNestedVariableRecord(itemPointerToDefaultRecord);

return resolvedValue;
return resolvedValueIfFound;
}

private buildNestedVariableRecord(itemPointerToDefaultRecord: Record<string, string>) {
const transformedObj: Record<string, string> = {};

Object.entries(itemPointerToDefaultRecord).forEach(([key, value]) => {
transformedObj[value] = value;
});

return transformedObj;
}
private collectAllItemPlaceholders(nodeExt: TipTapNode) {
const payloadValues = {};
const traverse = (node: TipTapNode) => {
Expand All @@ -153,7 +179,7 @@ export class HydrateEmailSchemaUseCase {
}
if (this.isPayloadValue(node)) {
const { id } = node.attrs;
payloadValues[node.attrs.id] = node.attrs.fallback || `{{item.${id}}}`;
payloadValues[`${node.attrs.id}`] = node.attrs.fallback || `{{item.${id}}}`;
}
if (node.content && Array.isArray(node.content)) {
node.content.forEach(traverse);
Expand All @@ -164,18 +190,16 @@ export class HydrateEmailSchemaUseCase {
return payloadValues;
}

private getValueByPath(obj: Record<string, any>, path: string): any {
const keys = path.split('.');
private getValueByPath(masterPayload: Record<string, any>, placeholderRef: string): any {
const keys = placeholderRef.split('.');

return keys.reduce((currentObj, key) => {
if (currentObj && typeof currentObj === 'object' && key in currentObj) {
const nextObj = currentObj[key];

return nextObj;
return currentObj[key];
}

return undefined;
}, obj);
}, masterPayload);
}

private buildElement(itemPointerToDefaultRecord: Record<string, string>, suffix: string) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@ export class RenderEmailOutputUsecase {

async execute(renderCommand: RenderEmailOutputCommand): Promise<EmailRenderOutput> {
const { emailEditor, subject } = EmailStepControlSchema.parse(renderCommand.controlValues);
console.log('payload', JSON.stringify(renderCommand.fullPayloadForRender));
const expandedSchema = this.transformForAndShowLogic(emailEditor, renderCommand.fullPayloadForRender);
const htmlRendered = await render(expandedSchema);

return { subject, body: htmlRendered };
}

private transformForAndShowLogic(body: string, fullPayloadForRender: FullPayloadForRender) {
return this.expendEmailEditorSchemaUseCase.execute({ body, fullPayloadForRender });
return this.expendEmailEditorSchemaUseCase.execute({ emailEditorJson: body, fullPayloadForRender });
}
}

Expand Down
68 changes: 60 additions & 8 deletions apps/api/src/app/workflows-v2/generate-preview.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,59 @@ describe('Generate Preview', () => {
});
});
});
describe('payload sanitation', () => {
it('Should produce a correct payload when pipe is used etc {{payload.variable | upper}}', async () => {
const { stepDatabaseId, workflowId } = await createWorkflowAndReturnId(StepTypeEnum.SMS);
const requestDto = {
controlValues: {
body: 'This is a legal placeholder with a pipe [{{payload.variableName | upper}}the pipe should show in the preview]',
},
};
const previewResponseDto = await generatePreview(workflowId, stepDatabaseId, requestDto, 'email');
expect(previewResponseDto.result!.preview).to.exist;
if (previewResponseDto.result!.type !== 'sms') {
throw new Error('Expected sms');
}
expect(previewResponseDto.result!.preview.body).to.contain('{{payload.variableName | upper}}');
expect(previewResponseDto.previewPayloadExample).to.exist;
expect(previewResponseDto?.previewPayloadExample?.payload?.variableName).to.equal(
'{{payload.variableName | upper}}'
);
});
});

describe('Error Handling', () => {
it('Should not fail on illegal placeholder {{}} ', async () => {
const { stepDatabaseId, workflowId } = await createWorkflowAndReturnId(StepTypeEnum.SMS);
const requestDto = {
controlValues: { body: 'some text that illegal placeholder[{{}}this text should be alone in brackets]' },
};
const previewResponseDto = await generatePreview(workflowId, stepDatabaseId, requestDto, 'sms');
expect(previewResponseDto.result!.preview).to.exist;
if (previewResponseDto.result!.type === 'sms') {
expect(previewResponseDto.result!.preview.body).to.contain('[this text should be alone in brackets]');
}
const issue = previewResponseDto.issues.body;
expect(issue).to.exist;
expect(issue[0].variableName).to.equal('{{}}');
expect(issue[0].issueType).to.equal('ILLEGAL_VARIABLE_IN_CONTROL_VALUE');
});
it('Should return a clear error on illegal placeholder {{name}} ', async () => {
const { stepDatabaseId, workflowId } = await createWorkflowAndReturnId(StepTypeEnum.SMS);
const requestDto = {
controlValues: { body: 'some text that illegal placeholder[{{name}}this text should be alone in brackets]' },
};
const previewResponseDto = await generatePreview(workflowId, stepDatabaseId, requestDto, 'sms');
expect(previewResponseDto.result!.preview).to.exist;
if (previewResponseDto.result!.type === 'sms') {
expect(previewResponseDto.result!.preview.body).to.contain('[this text should be alone in brackets]');
}
const issue = previewResponseDto.issues.body;
expect(issue).to.exist;
expect(issue[0].variableName).to.equal('{{name}}');
expect(issue[0].issueType).to.equal('ILLEGAL_VARIABLE_IN_CONTROL_VALUE');
});
});
describe('Missing Required ControlValues', () => {
const channelTypes = [{ type: StepTypeEnum.IN_APP, description: 'InApp' }];

Expand Down Expand Up @@ -427,14 +479,14 @@ function assertEmail(dto: GeneratePreviewResponseDto) {
if (dto.result!.type === ChannelTypeEnum.EMAIL) {
const preview = dto.result!.preview.body;
expect(preview).to.exist;
expect(preview).to.contain('{{item.header}}1');
expect(preview).to.contain('{{item.header}}2');
expect(preview).to.contain('{{item.name}}1');
expect(preview).to.contain('{{item.name}}2');
expect(preview).to.contain('{{item.id}}1');
expect(preview).to.contain('{{item.id}}2');
expect(preview).to.contain('{{item.origin.country}}1');
expect(preview).to.contain('{{item.origin.country}}2');
expect(preview).to.contain('{{item.header}}-1');
expect(preview).to.contain('{{item.header}}-2');
expect(preview).to.contain('{{item.name}}-1');
expect(preview).to.contain('{{item.name}}-2');
expect(preview).to.contain('{{item.id}}-1');
expect(preview).to.contain('{{item.id}}-2');
expect(preview).to.contain('{{item.origin.country}}-1');
expect(preview).to.contain('{{item.origin.country}}-2');
expect(preview).to.contain('{{payload.body}}');
expect(preview).to.contain('should be the fallback value');
}
Expand Down
Empty file.
Loading
Loading