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): preview usecase #7330

Merged
merged 18 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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 @@ -117,10 +117,11 @@ export class HydrateEmailSchemaUseCase {
node,
placeholderAggregation: PlaceholderAggregation
) {
const resolvedValue = this.getValueByPath(masterPayload, node.attrs.id);
const { fallback } = node.attrs;
const variableName = node.attrs.id;
const buildLiquidJSDefault = (mailyFallback: string) => (mailyFallback ? ` | default: '${mailyFallback}'` : '');
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider moving all the variable calculations from start to end into the buildLiquidJSVariable function defined once either on the class or as a single util. That is combine line 122 and 133.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also add an extra step of sanitization in the above function that caters for weird cases where the variableName ends with |.

Copy link
Contributor

Choose a reason for hiding this comment

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

Lastly, this transformation should happena cross all steps, not just email.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consider moving all the variable calculations from start to end into the buildLiquidJSVariable function defined once either on the class or as a single util. That is combine line 122 and 133.

If you refer to the show resolver, i totally agree, this was postponed until we have full show support.

Let's also add an extra step of sanitization in the above function that caters for weird cases where the variableName ends with |.

I love the idea but i believe we can postpone such sanitization because liquid js does not care about filter suffixes.
image

Lastly, this transformation should happena cross all steps, not just email.

can you elaborate please what do you mean by this transformation

const finalValue = `{{ ${variableName} ${buildLiquidJSDefault(fallback)} }}`;

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

return finalValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Injectable } from '@nestjs/common';
import { render as mailyRender } from '@maily-to/render';
import { Instrument, InstrumentUsecase } from '@novu/application-generic';
import isEmpty from 'lodash/isEmpty';
import { Liquid } from 'liquidjs';
import { FullPayloadForRender, RenderCommand } from './render-command';
import { ExpandEmailEditorSchemaUsecase } from './expand-email-editor-schema.usecase';
import { emailStepControlZodSchema } from '../../../workflows-v2/shared';
Expand All @@ -21,10 +22,26 @@ export class RenderEmailOutputUsecase {
return { subject, body: '' };
}

const expandedSchema = this.transformForAndShowLogic(body, renderCommand.fullPayloadForRender);
const htmlRendered = await this.renderEmail(expandedSchema);
const expandedMailyContent = this.transformForAndShowLogic(body, renderCommand.fullPayloadForRender);
const parsedTipTap = await this.parseTipTapNodeByLiquid(expandedMailyContent, renderCommand);
const renderedHtml = await this.renderEmail(parsedTipTap);

return { subject, body: htmlRendered };
return { subject, body: renderedHtml };
}

private async parseTipTapNodeByLiquid(
value: TipTapNode,
renderCommand: RenderEmailOutputCommand
): Promise<TipTapNode> {
const client = new Liquid();
const templateString = client.parse(JSON.stringify(value));
const parsedTipTap = await client.render(templateString, {
payload: renderCommand.fullPayloadForRender.payload,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get rid of the fullPayloadForRender extra nesting? It will make the code way more beautiful.

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 believe we can, but this refactor is focused on doing the minimum needed to make the code work with what we currently have. We'll try to avoid major changes as much as possible.

The goal is to ensure the use cases—like sending a simple email, "show" component, then "for" components—work as expected with minimal refactoring. If that's not possible, we'll consider a bigger refactor.

Once we have a working solution, we can refactor further if needed. One potential improvement could be flattening the data as you suggested.

subscriber: renderCommand.fullPayloadForRender.subscriber,
steps: renderCommand.fullPayloadForRender.steps,
});

return JSON.parse(parsedTipTap);
}

@Instrument()
Expand Down
177 changes: 175 additions & 2 deletions apps/api/src/app/workflows-v2/e2e/generate-preview.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,15 +409,43 @@ describe('Workflow Step Preview - POST /:workflowId/step/:stepId/preview', () =>
result: {
preview: {
subject: 'Welcome John',
body: 'Hello John, your order #{{payload.orderId}} is ready!', // orderId is not defined in the payload schema
body: 'Hello John, your order #undefined is ready!', // orderId is not defined in the payload schema or clientVariablesExample
},
type: 'in_app',
},
previewPayloadExample: {
payload: {
lastName: '{{payload.lastName}}',
organizationName: '{{payload.organizationName}}',
orderId: '{{payload.orderId}}',
firstName: 'John',
},
},
});

const response2 = await session.testAgent.post(`/v2/workflows/${workflow._id}/step/${stepId}/preview`).send({
controlValues,
previewPayload: {
payload: {
firstName: 'John',
orderId: '123456', // orderId is will override the variable example that driven by workflow payload schema
},
},
});

expect(response2.status).to.equal(201);
expect(response2.body.data).to.deep.equal({
result: {
preview: {
subject: 'Welcome John',
body: 'Hello John, your order #123456 is ready!', // orderId is not defined in the payload schema
},
type: 'in_app',
},
previewPayloadExample: {
payload: {
lastName: '{{payload.lastName}}',
organizationName: '{{payload.organizationName}}',
orderId: '123456',
firstName: 'John',
},
},
Expand Down Expand Up @@ -492,6 +520,151 @@ describe('Workflow Step Preview - POST /:workflowId/step/:stepId/preview', () =>
});
});

it('should transform tip tap node to liquid variables', async () => {
const workflow = await createWorkflow();

const stepId = workflow.steps[1]._id; // Using the email step (second step)
const bodyControlValue = {
type: 'doc',
content: [
{
type: 'heading',
attrs: { textAlign: 'left', level: 1 },
content: [
{ type: 'text', text: 'New Maily Email Editor ' },
{ type: 'variable', attrs: { id: 'payload.foo', label: null, fallback: null, showIfKey: null } },
{ type: 'text', text: ' ' },
],
},
{
type: 'paragraph',
attrs: { textAlign: 'left' },
content: [
{ type: 'text', text: 'free text last name is: ' },
{
type: 'variable',
attrs: { id: 'subscriber.lastName', label: null, fallback: null, showIfKey: `payload.show` },
},
{ type: 'text', text: ' ' },
{ type: 'hardBreak' },
{ type: 'text', text: 'extra data : ' },
{ type: 'variable', attrs: { id: 'payload.extraData', label: null, fallback: null, showIfKey: null } },
{ type: 'text', text: ' ' },
],
},
],
};
const controlValues = {
subject: 'Hello {{subscriber.firstName}} World!',
body: JSON.stringify(bodyControlValue),
};

const { status, body } = await session.testAgent.post(`/v2/workflows/${workflow._id}/step/${stepId}/preview`).send({
controlValues,
previewPayload: {},
});

expect(status).to.equal(201);
expect(body.data.result.type).to.equal('email');
expect(body.data.result.preview.subject).to.equal('Hello {{subscriber.firstName}} World!');
expect(body.data.result.preview.body).to.include('{{subscriber.lastName}}');
expect(body.data.result.preview.body).to.include('{{payload.foo}}');
// expect(body.data.result.preview.body).to.include('{{payload.show}}');
expect(body.data.result.preview.body).to.include('{{payload.extraData}}');
expect(body.data.previewPayloadExample).to.deep.equal({
subscriber: {
firstName: '{{subscriber.firstName}}',
lastName: '{{subscriber.lastName}}',
},
payload: {
foo: '{{payload.foo}}',
show: '{{payload.show}}',
extraData: '{{payload.extraData}}',
},
});
});

it('should render tip tap node with api client variables example', async () => {
const workflow = await createWorkflow();

const stepId = workflow.steps[1]._id; // Using the email step (second step)
const bodyControlValue = {
type: 'doc',
content: [
{
type: 'heading',
attrs: { textAlign: 'left', level: 1 },
content: [
{ type: 'text', text: 'New Maily Email Editor ' },
{ type: 'variable', attrs: { id: 'payload.foo', label: null, fallback: null, showIfKey: null } },
{ type: 'text', text: ' ' },
],
},
{
type: 'paragraph',
attrs: { textAlign: 'left' },
content: [
{ type: 'text', text: 'free text last name is: ' },
{
type: 'variable',
attrs: { id: 'subscriber.lastName', label: null, fallback: null, showIfKey: `payload.show` },
},
{ type: 'text', text: ' ' },
{ type: 'hardBreak' },
{ type: 'text', text: 'extra data : ' },
{
type: 'variable',
attrs: {
id: 'payload.extraData',
label: null,
fallback: 'fallback extra data is awesome',
showIfKey: null,
},
},
{ type: 'text', text: ' ' },
],
},
],
};
const controlValues = {
subject: 'Hello {{subscriber.firstName}} World!',
body: JSON.stringify(bodyControlValue),
};

const { status, body } = await session.testAgent.post(`/v2/workflows/${workflow._id}/step/${stepId}/preview`).send({
controlValues,
previewPayload: {
subscriber: {
firstName: 'John',
// lastName: 'Doe',
},
payload: {
foo: 'foo from client',
show: false,
extraData: '',
},
},
});

expect(status).to.equal(201);
expect(body.data.result.type).to.equal('email');
expect(body.data.result.preview.subject).to.equal('Hello John World!');
expect(body.data.result.preview.body).to.include('{{subscriber.lastName}}');
expect(body.data.result.preview.body).to.include('foo from client');
expect(body.data.result.preview.body).to.include('fallback extra data is awesome');
expect(body.data.previewPayloadExample).to.deep.equal({
subscriber: {
firstName: 'John',
lastName: '{{subscriber.lastName}}',
},
payload: {
foo: 'foo from client',
show: false,
extraData: '',
},
});
});

async function createWorkflow(overrides: Partial<NotificationTemplateEntity> = {}): Promise<WorkflowResponseDto> {
const createWorkflowDto: CreateWorkflowDto = {
__source: WorkflowCreationSourceEnum.EDITOR,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { Injectable } from '@nestjs/common';
import { ControlValuesEntity, ControlValuesRepository } from '@novu/dal';
import { ControlValuesRepository } from '@novu/dal';
import { ControlValuesLevelEnum, JSONSchemaDto } from '@novu/shared';
import { Instrument, InstrumentUsecase } from '@novu/application-generic';
import { flattenObjectValues } from '../../util/utils';
import { pathsToObject } from '../../util/path-to-object';
import { extractLiquidTemplateVariables } from '../../util/template-parser/liquid-parser';
import { convertJsonToSchemaWithDefaults } from '../../util/jsonToSchema';
import { BuildPayloadSchemaCommand } from './build-payload-schema.command';
import { transformMailyContentToLiquid } from '../generate-preview/transform-maily-content-to-liquid';
import { isStringTipTapNode } from '../../util/tip-tap.util';

@Injectable()
export class BuildPayloadSchema {
Expand All @@ -24,7 +26,7 @@ export class BuildPayloadSchema {
};
}

const templateVars = this.extractTemplateVariables(controlValues);
const templateVars = await this.processControlValues(controlValues);
if (templateVars.length === 0) {
return {
type: 'object',
Expand Down Expand Up @@ -66,12 +68,31 @@ export class BuildPayloadSchema {
}

@Instrument()
private extractTemplateVariables(controlValues: Record<string, unknown>[]): string[] {
const controlValuesString = controlValues.map(flattenObjectValues).flat().join(' ');
private async processControlValues(controlValues: Record<string, unknown>[]): Promise<string[]> {
const allVariables: string[] = [];

const test = extractLiquidTemplateVariables(controlValuesString);
const test2 = test.validVariables.map((variable) => variable.name);
for (const controlValue of controlValues) {
const processedControlValue = await this.processControlValue(controlValue);
const controlValuesString = flattenObjectValues(processedControlValue).join(' ');
const templateVariables = extractLiquidTemplateVariables(controlValuesString);
allVariables.push(...templateVariables.validVariables.map((variable) => variable.name));
}

return [...new Set(allVariables)];
}

@Instrument()
private async processControlValue(controlValue: Record<string, unknown>): Promise<Record<string, unknown>> {
const processedValue: Record<string, unknown> = {};

for (const [key, value] of Object.entries(controlValue)) {
if (isStringTipTapNode(value)) {
processedValue[key] = transformMailyContentToLiquid(JSON.parse(value));
} else {
processedValue[key] = value;
}
}

return test2;
return processedValue;
}
}
Loading
Loading