From 4ea2d7fe1d7a580c08a90632bde496eea64903d7 Mon Sep 17 00:00:00 2001 From: Gosha Date: Fri, 6 Dec 2024 15:47:40 +0200 Subject: [PATCH 1/2] feat(api): revert preview tests --- .../app/workflows-v2/generate-preview.e2e.ts | 257 +++++++++++++++++- 1 file changed, 256 insertions(+), 1 deletion(-) diff --git a/apps/api/src/app/workflows-v2/generate-preview.e2e.ts b/apps/api/src/app/workflows-v2/generate-preview.e2e.ts index 4e267cb70e1..0b7ccf09032 100644 --- a/apps/api/src/app/workflows-v2/generate-preview.e2e.ts +++ b/apps/api/src/app/workflows-v2/generate-preview.e2e.ts @@ -13,7 +13,6 @@ import { GeneratePreviewResponseDto, HttpError, NovuRestResult, - PreviewIssueEnum, RedirectTargetEnum, StepTypeEnum, WorkflowCreationSourceEnum, @@ -51,6 +50,23 @@ describe('Generate Preview', () => { describe('Generate Preview', () => { describe('Hydration testing', () => { + it(` should hydrate previous step in iterator email --> digest`, async () => { + const { workflowId, emailStepDatabaseId, digestStepId } = await createWorkflowWithEmailLookingAtDigestResult(); + const requestDto = { + controlValues: getTestControlValues(digestStepId)[StepTypeEnum.EMAIL], + previewPayload: { payload: { subject: PLACEHOLDER_SUBJECT_INAPP_PAYLOAD_VALUE } }, + }; + const previewResponseDto = await generatePreview(workflowId, emailStepDatabaseId, requestDto, 'testing steps'); + expect(previewResponseDto.result!.preview).to.exist; + expect(previewResponseDto.previewPayloadExample).to.exist; + expect(previewResponseDto.previewPayloadExample?.steps?.[digestStepId]).to.be.ok; + if (previewResponseDto.result!.type !== ChannelTypeEnum.EMAIL) { + throw new Error('Expected email'); + } + const preview = previewResponseDto.result!.preview.body; + expect(previewResponseDto.result!.preview.body).to.contain('{{item.payload.country}}'); + }); + it(` should hydrate previous step in iterator sms looking at inApp`, async () => { const { workflowId, smsDatabaseStepId, inAppStepId } = await createWorkflowWithSmsLookingAtInAppResult(); const requestDto = buildDtoNoPayload(StepTypeEnum.SMS, inAppStepId); @@ -63,6 +79,26 @@ describe('Generate Preview', () => { } }); }); + + it(`IN_APP :should match the body in the preview response`, async () => { + const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId(StepTypeEnum.IN_APP); + const controlValues = buildInAppControlValues(); + const requestDto = { + controlValues, + previewPayload: { payload: { subject: PLACEHOLDER_SUBJECT_INAPP_PAYLOAD_VALUE } }, + }; + const previewResponseDto = await generatePreview(workflowId, stepDatabaseId, requestDto, StepTypeEnum.IN_APP); + expect(previewResponseDto.result!.preview).to.exist; + controlValues.subject = controlValues.subject!.replace( + PLACEHOLDER_SUBJECT_INAPP, + PLACEHOLDER_SUBJECT_INAPP_PAYLOAD_VALUE + ); + if (previewResponseDto.result?.type !== 'in_app') { + throw new Error('should have a inapp redview '); + } + expect(previewResponseDto.result.preview.subject).to.deep.equal(controlValues.subject); + }); + describe('Happy Path, no payload, expected same response as requested', () => { // TODO: this test is not working as expected it(`${StepTypeEnum.IN_APP}: should match the body in the preview response`, async () => { @@ -80,6 +116,54 @@ describe('Generate Preview', () => { expect(previewResponseDto.result!.preview).to.deep.equal(previewRequestWithoutTheRedirect); }); + it(`${StepTypeEnum.SMS}: should match the body in the preview response`, async () => { + const previewResponseDto = await createWorkflowAndPreview(StepTypeEnum.SMS, 'SMS'); + + expect(previewResponseDto.result!.preview).to.exist; + expect(previewResponseDto.issues).to.exist; + expect(previewResponseDto.previewPayloadExample).to.exist; + expect(previewResponseDto.previewPayloadExample.subscriber, 'Expecting to find subscriber in the payload').to + .exist; + + expect(previewResponseDto.result!.preview).to.deep.equal(getTestControlValues()[StepTypeEnum.SMS]); + }); + + it(`${StepTypeEnum.PUSH}: should match the body in the preview response`, async () => { + const previewResponseDto = await createWorkflowAndPreview(StepTypeEnum.PUSH, 'Push'); + + expect(previewResponseDto.result!.preview).to.exist; + expect(previewResponseDto.issues).to.exist; + expect(previewResponseDto.previewPayloadExample).to.exist; + expect(previewResponseDto.previewPayloadExample.subscriber, 'Expecting to find subscriber in the payload').to + .exist; + + expect(previewResponseDto.result!.preview).to.deep.equal(getTestControlValues()[StepTypeEnum.PUSH]); + }); + + it(`${StepTypeEnum.CHAT}: should match the body in the preview response`, async () => { + const previewResponseDto = await createWorkflowAndPreview(StepTypeEnum.CHAT, 'Chat'); + + expect(previewResponseDto.result!.preview).to.exist; + expect(previewResponseDto.issues).to.exist; + expect(previewResponseDto.previewPayloadExample).to.exist; + expect(previewResponseDto.previewPayloadExample.subscriber, 'Expecting to find subscriber in the payload').to + .exist; + + expect(previewResponseDto.result!.preview).to.deep.equal(getTestControlValues()[StepTypeEnum.CHAT]); + }); + + it(`${StepTypeEnum.EMAIL}: should match the body in the preview response`, async () => { + const previewResponseDto = await createWorkflowAndPreview(StepTypeEnum.EMAIL, 'Email'); + + expect(previewResponseDto.result!.preview).to.exist; + expect(previewResponseDto.issues).to.exist; + expect(previewResponseDto.previewPayloadExample).to.exist; + expect(previewResponseDto.previewPayloadExample.subscriber, 'Expecting to find subscriber in the payload').to + .exist; + + assertEmail(previewResponseDto); + }); + async function createWorkflowAndPreview(type, description) { const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId(type); const requestDto = buildDtoNoPayload(type); @@ -189,8 +273,115 @@ 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('In App Partial control values', () => { + it('Should show issue about bad URLs without failing ', async () => { + const steps = [{ name: 'IN_APP_STEP_SHOULD_NOT_FAIL', type: StepTypeEnum.IN_APP }]; + const createDto = buildCreateWorkflowDto('', { steps }); + const novuRestResult = await workflowsClient.createWorkflow(createDto); + if (!novuRestResult.isSuccessResult()) { + throw new Error('should create workflow'); + } + const controlValues = { + subject: `{{subscriber.firstName}} Hello, World! ${PLACEHOLDER_SUBJECT_INAPP}`, + body: `Hello, World! {{payload.placeholder.body}}`, + avatar: 'https://www.example.com/avatar.png', + primaryAction: { + label: '{{payload.secondaryUrl}}', + redirect: { + url: 'notAUrl.com', + target: RedirectTargetEnum.BLANK, + }, + }, + secondaryAction: { + label: 'some label', + redirect: { + url: 'ftp://notAUrl.com', + target: RedirectTargetEnum.BLANK, + }, + }, + redirect: { + target: RedirectTargetEnum.BLANK, + url: 'not a url', + }, + }; + const workflowSlug = novuRestResult.value?.slug; + const stepSlug = novuRestResult.value?.steps[0].slug; + await patchStepWithControlValues(workflowSlug, stepSlug, controlValues); + const generatePreviewResponseDto = await generatePreview( + workflowSlug, + stepSlug, + { + controlValues, + }, + '' + ); + expect(generatePreviewResponseDto.issues['redirect.url'][0].issueType).to.equal('INVALID_URL'); + expect(generatePreviewResponseDto.issues['secondaryAction.redirect.url'][0].issueType).to.equal( + 'INVALID_URL' + ); + expect(generatePreviewResponseDto.issues['primaryAction.redirect.url'][0].issueType).to.equal('INVALID_URL'); + if (generatePreviewResponseDto.result?.type === ChannelTypeEnum.IN_APP) { + const { preview } = generatePreviewResponseDto.result; + expect(JSON.stringify(preview.primaryAction)).to.contain('not-good-url-please-replace'); + expect(JSON.stringify(preview.secondaryAction)).to.contain('not-good-url-please-replace'); + expect(JSON.stringify(preview.redirect)).to.contain('not-good-url-please-replace'); + } + }); + it('Should not fail if inApp is providing partial URL in redirect', async () => { const steps = [{ name: 'IN_APP_STEP_SHOULD_NOT_FAIL', type: StepTypeEnum.IN_APP }]; const createDto = buildCreateWorkflowDto('', { steps }); @@ -239,6 +430,70 @@ describe('Generate Preview', () => { ); } }); + + it('Should not fail if inApp url ref is a placeholder without payload', async () => { + const steps = [{ name: 'IN_APP_STEP_SHOULD_NOT_FAIL', type: StepTypeEnum.IN_APP }]; + const createDto = buildCreateWorkflowDto('', { steps }); + const novuRestResult = await workflowsClient.createWorkflow(createDto); + if (!novuRestResult.isSuccessResult()) { + throw new Error('should create workflow'); + } + const workflowSlug = novuRestResult.value?.slug; + const stepSlug = novuRestResult.value?.steps[0].slug; + const stepDataDto = await patchStepWithControlValues( + workflowSlug, + stepSlug, + buildInAppControlValueWithAPlaceholderInTheUrl() + ); + const generatePreviewResponseDto = await generatePreview( + workflowSlug, + stepSlug, + { controlValues: buildInAppControlValueWithAPlaceholderInTheUrl() }, + '' + ); + + if (generatePreviewResponseDto.result?.type === ChannelTypeEnum.IN_APP) { + expect(generatePreviewResponseDto.result.preview.body).to.equal( + { + subject: `{{subscriber.firstName}} Hello, World! ${PLACEHOLDER_SUBJECT_INAPP}`, + body: `Hello, World! {{payload.placeholder.body}}`, + avatar: 'https://www.example.com/avatar.png', + primaryAction: { + label: '{{payload.secondaryUrl}}', + redirect: { + target: RedirectTargetEnum.BLANK, + }, + }, + secondaryAction: null, + redirect: { + target: RedirectTargetEnum.BLANK, + url: ' ', + }, + }.body + ); + expect(generatePreviewResponseDto.result.preview.primaryAction?.redirect?.url).to.be.ok; + expect(generatePreviewResponseDto.result.preview.primaryAction?.redirect?.url).to.contain('https'); + } + }); + }); + }); + describe('Missing Required ControlValues', () => { + const channelTypes = [{ type: StepTypeEnum.IN_APP, description: 'InApp' }]; + + channelTypes.forEach(({ type, description }) => { + it(`${type}: should assign default values to missing elements`, async () => { + const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId(type); + const requestDto = buildDtoWithMissingControlValues(type, stepId); + const previewResponseDto = await generatePreview(workflowId, stepDatabaseId, requestDto, description); + if (previewResponseDto.result!.type !== ChannelTypeEnum.IN_APP) { + throw new Error('Expected email'); + } + expect(previewResponseDto.result!.preview.body).to.exist; + expect(previewResponseDto.result!.preview.body).to.equal('PREVIEW_ISSUE:REQUIRED_CONTROL_VALUE_IS_MISSING'); + const { issues } = previewResponseDto; + expect(issues).to.exist; + expect(issues.body).to.exist; + }); }); }); }); From a5fcba61690eff06afd31ffbd29a0323b050c453 Mon Sep 17 00:00:00 2001 From: Gosha Date: Sun, 8 Dec 2024 12:35:23 +0200 Subject: [PATCH 2/2] fix(api): align tests to work with workflow endpoint --- .../app/workflows-v2/generate-preview.e2e.ts | 456 ++++++++---------- .../build-payload-schema.usecase.ts | 5 +- .../generate-preview.usecase.ts | 4 +- .../workflows-v2/workflow.controller.e2e.ts | 92 +++- 4 files changed, 305 insertions(+), 252 deletions(-) diff --git a/apps/api/src/app/workflows-v2/generate-preview.e2e.ts b/apps/api/src/app/workflows-v2/generate-preview.e2e.ts index 0b7ccf09032..fd6024fd280 100644 --- a/apps/api/src/app/workflows-v2/generate-preview.e2e.ts +++ b/apps/api/src/app/workflows-v2/generate-preview.e2e.ts @@ -24,6 +24,7 @@ import { InAppControlType } from './shared'; const SUBJECT_TEST_PAYLOAD = '{{payload.subject.test.payload}}'; const PLACEHOLDER_SUBJECT_INAPP = '{{payload.subject}}'; const PLACEHOLDER_SUBJECT_INAPP_PAYLOAD_VALUE = 'this is the replacement text for the placeholder'; + describe('Generate Preview', () => { let session: UserSession; let workflowsClient: ReturnType; @@ -56,7 +57,13 @@ describe('Generate Preview', () => { controlValues: getTestControlValues(digestStepId)[StepTypeEnum.EMAIL], previewPayload: { payload: { subject: PLACEHOLDER_SUBJECT_INAPP_PAYLOAD_VALUE } }, }; - const previewResponseDto = await generatePreview(workflowId, emailStepDatabaseId, requestDto, 'testing steps'); + const previewResponseDto = await generatePreview( + workflowsClient, + workflowId, + emailStepDatabaseId, + requestDto, + 'testing steps' + ); expect(previewResponseDto.result!.preview).to.exist; expect(previewResponseDto.previewPayloadExample).to.exist; expect(previewResponseDto.previewPayloadExample?.steps?.[digestStepId]).to.be.ok; @@ -70,7 +77,13 @@ describe('Generate Preview', () => { it(` should hydrate previous step in iterator sms looking at inApp`, async () => { const { workflowId, smsDatabaseStepId, inAppStepId } = await createWorkflowWithSmsLookingAtInAppResult(); const requestDto = buildDtoNoPayload(StepTypeEnum.SMS, inAppStepId); - const previewResponseDto = await generatePreview(workflowId, smsDatabaseStepId, requestDto, 'testing steps'); + const previewResponseDto = await generatePreview( + workflowsClient, + workflowId, + smsDatabaseStepId, + requestDto, + 'testing steps' + ); expect(previewResponseDto.result!.preview).to.exist; expect(previewResponseDto.previewPayloadExample).to.exist; expect(previewResponseDto.previewPayloadExample?.steps).to.be.ok; @@ -81,13 +94,22 @@ describe('Generate Preview', () => { }); it(`IN_APP :should match the body in the preview response`, async () => { - const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId(StepTypeEnum.IN_APP); + const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId( + workflowsClient, + StepTypeEnum.IN_APP + ); const controlValues = buildInAppControlValues(); const requestDto = { controlValues, previewPayload: { payload: { subject: PLACEHOLDER_SUBJECT_INAPP_PAYLOAD_VALUE } }, }; - const previewResponseDto = await generatePreview(workflowId, stepDatabaseId, requestDto, StepTypeEnum.IN_APP); + const previewResponseDto = await generatePreview( + workflowsClient, + workflowId, + stepDatabaseId, + requestDto, + StepTypeEnum.IN_APP + ); expect(previewResponseDto.result!.preview).to.exist; controlValues.subject = controlValues.subject!.replace( PLACEHOLDER_SUBJECT_INAPP, @@ -101,7 +123,7 @@ describe('Generate Preview', () => { describe('Happy Path, no payload, expected same response as requested', () => { // TODO: this test is not working as expected - it(`${StepTypeEnum.IN_APP}: should match the body in the preview response`, async () => { + it('in_app: should match the body in the preview response', async () => { const previewResponseDto = await createWorkflowAndPreview(StepTypeEnum.IN_APP, 'InApp'); expect(previewResponseDto.result).to.exist; @@ -116,11 +138,10 @@ describe('Generate Preview', () => { expect(previewResponseDto.result!.preview).to.deep.equal(previewRequestWithoutTheRedirect); }); - it(`${StepTypeEnum.SMS}: should match the body in the preview response`, async () => { + it('sms: should match the body in the preview response', async () => { const previewResponseDto = await createWorkflowAndPreview(StepTypeEnum.SMS, 'SMS'); expect(previewResponseDto.result!.preview).to.exist; - expect(previewResponseDto.issues).to.exist; expect(previewResponseDto.previewPayloadExample).to.exist; expect(previewResponseDto.previewPayloadExample.subscriber, 'Expecting to find subscriber in the payload').to .exist; @@ -128,11 +149,10 @@ describe('Generate Preview', () => { expect(previewResponseDto.result!.preview).to.deep.equal(getTestControlValues()[StepTypeEnum.SMS]); }); - it(`${StepTypeEnum.PUSH}: should match the body in the preview response`, async () => { + it('push: should match the body in the preview response', async () => { const previewResponseDto = await createWorkflowAndPreview(StepTypeEnum.PUSH, 'Push'); expect(previewResponseDto.result!.preview).to.exist; - expect(previewResponseDto.issues).to.exist; expect(previewResponseDto.previewPayloadExample).to.exist; expect(previewResponseDto.previewPayloadExample.subscriber, 'Expecting to find subscriber in the payload').to .exist; @@ -140,11 +160,10 @@ describe('Generate Preview', () => { expect(previewResponseDto.result!.preview).to.deep.equal(getTestControlValues()[StepTypeEnum.PUSH]); }); - it(`${StepTypeEnum.CHAT}: should match the body in the preview response`, async () => { + it('chat: should match the body in the preview response', async () => { const previewResponseDto = await createWorkflowAndPreview(StepTypeEnum.CHAT, 'Chat'); expect(previewResponseDto.result!.preview).to.exist; - expect(previewResponseDto.issues).to.exist; expect(previewResponseDto.previewPayloadExample).to.exist; expect(previewResponseDto.previewPayloadExample.subscriber, 'Expecting to find subscriber in the payload').to .exist; @@ -152,11 +171,10 @@ describe('Generate Preview', () => { expect(previewResponseDto.result!.preview).to.deep.equal(getTestControlValues()[StepTypeEnum.CHAT]); }); - it(`${StepTypeEnum.EMAIL}: should match the body in the preview response`, async () => { + it('email: should match the body in the preview response', async () => { const previewResponseDto = await createWorkflowAndPreview(StepTypeEnum.EMAIL, 'Email'); expect(previewResponseDto.result!.preview).to.exist; - expect(previewResponseDto.issues).to.exist; expect(previewResponseDto.previewPayloadExample).to.exist; expect(previewResponseDto.previewPayloadExample.subscriber, 'Expecting to find subscriber in the payload').to .exist; @@ -164,18 +182,23 @@ describe('Generate Preview', () => { assertEmail(previewResponseDto); }); - async function createWorkflowAndPreview(type, description) { - const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId(type); + async function createWorkflowAndPreview(type: StepTypeEnum, description: string) { + const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId(workflowsClient, type); const requestDto = buildDtoNoPayload(type); - return await generatePreview(workflowId, stepDatabaseId, requestDto, description); + return await generatePreview(workflowsClient, workflowId, stepDatabaseId, requestDto, description); } }); + describe('email specific features', () => { describe('show', () => { it('show -> should hide element based on payload', async () => { - const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId(StepTypeEnum.EMAIL); + const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId( + workflowsClient, + StepTypeEnum.EMAIL + ); const previewResponseDto = await generatePreview( + workflowsClient, workflowId, stepDatabaseId, { @@ -192,8 +215,12 @@ describe('Generate Preview', () => { expect(preview).to.not.contain('should be the fallback value'); }); it('show -> should show element based on payload - string', async () => { - const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId(StepTypeEnum.EMAIL); + const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId( + workflowsClient, + StepTypeEnum.EMAIL + ); const previewResponseDto = await generatePreview( + workflowsClient, workflowId, stepDatabaseId, { @@ -210,8 +237,12 @@ describe('Generate Preview', () => { expect(preview).to.contain('should be the fallback value'); }); it('show -> should show element based on payload - boolean', async () => { - const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId(StepTypeEnum.EMAIL); + const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId( + workflowsClient, + StepTypeEnum.EMAIL + ); const previewResponseDto = await generatePreview( + workflowsClient, workflowId, stepDatabaseId, { @@ -227,8 +258,12 @@ describe('Generate Preview', () => { expect(preview).to.contain('should be the fallback value'); }); it('show -> should show element if payload is missing', async () => { - const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId(StepTypeEnum.EMAIL); + const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId( + workflowsClient, + StepTypeEnum.EMAIL + ); const previewResponseDto = await generatePreview( + workflowsClient, workflowId, stepDatabaseId, { @@ -247,10 +282,11 @@ describe('Generate Preview', () => { }); describe('for', () => { it('should populate for if payload exist with actual values', async () => { - const { stepDatabaseId, workflowId } = await createWorkflowAndReturnId(StepTypeEnum.EMAIL); + const { stepDatabaseId, workflowId } = await createWorkflowAndReturnId(workflowsClient, StepTypeEnum.EMAIL); const name1 = 'ball is round'; const name2 = 'square is square'; const previewResponseDto = await generatePreview( + workflowsClient, workflowId, stepDatabaseId, { @@ -275,224 +311,153 @@ 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 { stepDatabaseId, workflowId } = await createWorkflowAndReturnId(workflowsClient, StepTypeEnum.SMS); const requestDto = { controlValues: { - body: 'This is a legal placeholder with a pipe [{{payload.variableName | upper}}the pipe should show in the preview]', + body: 'This is a legal placeholder with a pipe [{{payload.variableName | upcase}}the pipe should show in the preview]', }, }; - const previewResponseDto = await generatePreview(workflowId, stepDatabaseId, requestDto, 'email'); + const previewResponseDto = await generatePreview( + workflowsClient, + 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.result!.preview.body).to.contain('{{PAYLOAD.VARIABLENAME | UPCASE}}'); expect(previewResponseDto.previewPayloadExample).to.exist; expect(previewResponseDto?.previewPayloadExample?.payload?.variableName).to.equal( - '{{payload.variableName | upper}}' + '{{payload.variableName | upcase}}' ); }); - }); - 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]'); + it('Should not fail if inApp is providing partial URL in redirect', async () => { + const steps = [{ name: 'IN_APP_STEP_SHOULD_NOT_FAIL', type: StepTypeEnum.IN_APP }]; + const createDto = buildCreateWorkflowDto('', { steps }); + const novuRestResult = await workflowsClient.createWorkflow(createDto); + if (!novuRestResult.isSuccessResult()) { + throw new Error('should create workflow'); } - - 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('In App Partial control values', () => { - it('Should show issue about bad URLs without failing ', async () => { - const steps = [{ name: 'IN_APP_STEP_SHOULD_NOT_FAIL', type: StepTypeEnum.IN_APP }]; - const createDto = buildCreateWorkflowDto('', { steps }); - const novuRestResult = await workflowsClient.createWorkflow(createDto); - if (!novuRestResult.isSuccessResult()) { - throw new Error('should create workflow'); - } - const controlValues = { - subject: `{{subscriber.firstName}} Hello, World! ${PLACEHOLDER_SUBJECT_INAPP}`, - body: `Hello, World! {{payload.placeholder.body}}`, - avatar: 'https://www.example.com/avatar.png', - primaryAction: { - label: '{{payload.secondaryUrl}}', - redirect: { - url: 'notAUrl.com', - target: RedirectTargetEnum.BLANK, - }, - }, - secondaryAction: { - label: 'some label', - redirect: { - url: 'ftp://notAUrl.com', - target: RedirectTargetEnum.BLANK, - }, - }, + const controlValues = { + subject: `{{subscriber.firstName}} Hello, World! ${PLACEHOLDER_SUBJECT_INAPP}`, + body: `Hello, World! {{payload.placeholder.body}}`, + avatar: 'https://www.example.com/avatar.png', + primaryAction: { + label: '{{payload.secondaryUrl}}', redirect: { target: RedirectTargetEnum.BLANK, - url: 'not a url', }, - }; - const workflowSlug = novuRestResult.value?.slug; - const stepSlug = novuRestResult.value?.steps[0].slug; - await patchStepWithControlValues(workflowSlug, stepSlug, controlValues); - const generatePreviewResponseDto = await generatePreview( - workflowSlug, - stepSlug, + }, + secondaryAction: null, + redirect: { + target: RedirectTargetEnum.BLANK, + url: ' ', + }, + }; + const workflowSlug = novuRestResult.value?.slug; + const stepSlug = novuRestResult.value?.steps[0].slug; + const stepDataDto = await patchStepWithControlValues(workflowSlug, stepSlug, controlValues); + const generatePreviewResponseDto = await generatePreview( + workflowsClient, + workflowSlug, + stepSlug, + { controlValues }, + '' + ); + if (generatePreviewResponseDto.result?.type === ChannelTypeEnum.IN_APP) { + expect(generatePreviewResponseDto.result.preview.body).to.equal( { - controlValues, - }, - '' - ); - expect(generatePreviewResponseDto.issues['redirect.url'][0].issueType).to.equal('INVALID_URL'); - expect(generatePreviewResponseDto.issues['secondaryAction.redirect.url'][0].issueType).to.equal( - 'INVALID_URL' - ); - expect(generatePreviewResponseDto.issues['primaryAction.redirect.url'][0].issueType).to.equal('INVALID_URL'); - if (generatePreviewResponseDto.result?.type === ChannelTypeEnum.IN_APP) { - const { preview } = generatePreviewResponseDto.result; - expect(JSON.stringify(preview.primaryAction)).to.contain('not-good-url-please-replace'); - expect(JSON.stringify(preview.secondaryAction)).to.contain('not-good-url-please-replace'); - expect(JSON.stringify(preview.redirect)).to.contain('not-good-url-please-replace'); - } - }); - - it('Should not fail if inApp is providing partial URL in redirect', async () => { - const steps = [{ name: 'IN_APP_STEP_SHOULD_NOT_FAIL', type: StepTypeEnum.IN_APP }]; - const createDto = buildCreateWorkflowDto('', { steps }); - const novuRestResult = await workflowsClient.createWorkflow(createDto); - if (!novuRestResult.isSuccessResult()) { - throw new Error('should create workflow'); - } - const controlValues = { - subject: `{{subscriber.firstName}} Hello, World! ${PLACEHOLDER_SUBJECT_INAPP}`, - body: `Hello, World! {{payload.placeholder.body}}`, - avatar: 'https://www.example.com/avatar.png', - primaryAction: { - label: '{{payload.secondaryUrl}}', - redirect: { - target: RedirectTargetEnum.BLANK, - }, - }, - secondaryAction: null, - redirect: { - target: RedirectTargetEnum.BLANK, - url: ' ', - }, - }; - const workflowSlug = novuRestResult.value?.slug; - const stepSlug = novuRestResult.value?.steps[0].slug; - const stepDataDto = await patchStepWithControlValues(workflowSlug, stepSlug, controlValues); - const generatePreviewResponseDto = await generatePreview(workflowSlug, stepSlug, { controlValues }, ''); - if (generatePreviewResponseDto.result?.type === ChannelTypeEnum.IN_APP) { - expect(generatePreviewResponseDto.result.preview.body).to.equal( - { - subject: `{{subscriber.firstName}} Hello, World! ${PLACEHOLDER_SUBJECT_INAPP}`, - body: `Hello, World! {{payload.placeholder.body}}`, - avatar: 'https://www.example.com/avatar.png', - primaryAction: { - label: '{{payload.secondaryUrl}}', - redirect: { - target: RedirectTargetEnum.BLANK, - }, - }, - secondaryAction: null, + subject: `{{subscriber.firstName}} Hello, World! ${PLACEHOLDER_SUBJECT_INAPP}`, + body: `Hello, World! {{payload.placeholder.body}}`, + avatar: 'https://www.example.com/avatar.png', + primaryAction: { + label: '{{payload.secondaryUrl}}', redirect: { target: RedirectTargetEnum.BLANK, - url: ' ', }, - }.body - ); - } - }); - - it('Should not fail if inApp url ref is a placeholder without payload', async () => { - const steps = [{ name: 'IN_APP_STEP_SHOULD_NOT_FAIL', type: StepTypeEnum.IN_APP }]; - const createDto = buildCreateWorkflowDto('', { steps }); - const novuRestResult = await workflowsClient.createWorkflow(createDto); - if (!novuRestResult.isSuccessResult()) { - throw new Error('should create workflow'); - } - const workflowSlug = novuRestResult.value?.slug; - const stepSlug = novuRestResult.value?.steps[0].slug; - const stepDataDto = await patchStepWithControlValues( - workflowSlug, - stepSlug, - buildInAppControlValueWithAPlaceholderInTheUrl() - ); - const generatePreviewResponseDto = await generatePreview( - workflowSlug, - stepSlug, - { controlValues: buildInAppControlValueWithAPlaceholderInTheUrl() }, - '' + }, + secondaryAction: null, + redirect: { + target: RedirectTargetEnum.BLANK, + url: ' ', + }, + }.body ); + } + }); - if (generatePreviewResponseDto.result?.type === ChannelTypeEnum.IN_APP) { - expect(generatePreviewResponseDto.result.preview.body).to.equal( - { - subject: `{{subscriber.firstName}} Hello, World! ${PLACEHOLDER_SUBJECT_INAPP}`, - body: `Hello, World! {{payload.placeholder.body}}`, - avatar: 'https://www.example.com/avatar.png', - primaryAction: { - label: '{{payload.secondaryUrl}}', - redirect: { - target: RedirectTargetEnum.BLANK, - }, - }, - secondaryAction: null, + it('Should not fail if inApp url ref is a placeholder without payload', async () => { + const steps = [{ name: 'IN_APP_STEP_SHOULD_NOT_FAIL', type: StepTypeEnum.IN_APP }]; + const createDto = buildCreateWorkflowDto('', { steps }); + const novuRestResult = await workflowsClient.createWorkflow(createDto); + if (!novuRestResult.isSuccessResult()) { + throw new Error('should create workflow'); + } + const workflowSlug = novuRestResult.value?.slug; + const stepSlug = novuRestResult.value?.steps[0].slug; + const stepDataDto = await patchStepWithControlValues( + workflowSlug, + stepSlug, + buildInAppControlValueWithAPlaceholderInTheUrl() + ); + const generatePreviewResponseDto = await generatePreview( + workflowsClient, + workflowSlug, + stepSlug, + { controlValues: buildInAppControlValueWithAPlaceholderInTheUrl() }, + '' + ); + + if (generatePreviewResponseDto.result?.type === ChannelTypeEnum.IN_APP) { + expect(generatePreviewResponseDto.result.preview.body).to.equal( + { + subject: `{{subscriber.firstName}} Hello, World! ${PLACEHOLDER_SUBJECT_INAPP}`, + body: `Hello, World! {{payload.placeholder.body}}`, + avatar: 'https://www.example.com/avatar.png', + primaryAction: { + label: '{{payload.secondaryUrl}}', redirect: { target: RedirectTargetEnum.BLANK, - url: ' ', }, - }.body - ); - expect(generatePreviewResponseDto.result.preview.primaryAction?.redirect?.url).to.be.ok; - expect(generatePreviewResponseDto.result.preview.primaryAction?.redirect?.url).to.contain('https'); - } - }); + }, + secondaryAction: null, + redirect: { + target: RedirectTargetEnum.BLANK, + url: ' ', + }, + }.body + ); + expect(generatePreviewResponseDto.result.preview.primaryAction?.redirect?.url).to.be.ok; + expect(generatePreviewResponseDto.result.preview.primaryAction?.redirect?.url).to.contain('https'); + } }); }); + describe('Missing Required ControlValues', () => { const channelTypes = [{ type: StepTypeEnum.IN_APP, description: 'InApp' }]; channelTypes.forEach(({ type, description }) => { - it(`${type}: should assign default values to missing elements`, async () => { - const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId(type); + it(`[${type}] should assign default values to missing elements`, async () => { + const { stepDatabaseId, workflowId, stepId } = await createWorkflowAndReturnId(workflowsClient, type); const requestDto = buildDtoWithMissingControlValues(type, stepId); - const previewResponseDto = await generatePreview(workflowId, stepDatabaseId, requestDto, description); + + const previewResponseDto = await generatePreview( + workflowsClient, + workflowId, + stepDatabaseId, + requestDto, + description + ); + if (previewResponseDto.result!.type !== ChannelTypeEnum.IN_APP) { throw new Error('Expected email'); } expect(previewResponseDto.result!.preview.body).to.exist; expect(previewResponseDto.result!.preview.body).to.equal('PREVIEW_ISSUE:REQUIRED_CONTROL_VALUE_IS_MISSING'); - const { issues } = previewResponseDto; - expect(issues).to.exist; - expect(issues.body).to.exist; }); }); }); @@ -505,34 +470,6 @@ describe('Generate Preview', () => { }; } - async function generatePreview( - workflowId: string, - stepDatabaseId: string, - dto: GeneratePreviewRequestDto, - description: string - ): Promise { - const novuRestResult = await workflowsClient.generatePreview(workflowId, stepDatabaseId, dto); - if (novuRestResult.isSuccessResult()) { - return novuRestResult.value; - } - - throw await assertHttpError(description, novuRestResult, dto); - } - - async function createWorkflowAndReturnId(type: StepTypeEnum) { - const createWorkflowDto = buildCreateWorkflowDto(`${type}:${randomUUID()}`); - createWorkflowDto.steps[0].type = type; - const workflowResult = await workflowsClient.createWorkflow(createWorkflowDto); - if (!workflowResult.isSuccessResult()) { - throw new Error(`Failed to create workflow ${JSON.stringify(workflowResult.error)}`); - } - - return { - workflowId: workflowResult.value._id, - stepDatabaseId: workflowResult.value.steps[0]._id, - stepId: workflowResult.value.steps[0].stepId, - }; - } async function createWorkflowWithEmailLookingAtDigestResult() { const createWorkflowDto: CreateWorkflowDto = { tags: [], @@ -601,20 +538,6 @@ function buildDtoNoPayload(stepTypeEnum: StepTypeEnum, stepId?: string): Generat }; } -function buildDtoWithMissingControlValues(stepTypeEnum: StepTypeEnum, stepId: string): GeneratePreviewRequestDto { - const stepTypeToElement = getTestControlValues(stepId)[stepTypeEnum]; - if (stepTypeEnum === StepTypeEnum.EMAIL) { - delete stepTypeToElement.subject; - } else { - delete stepTypeToElement.body; - } - - return { - controlValues: stepTypeToElement, - previewPayload: { payload: { subject: PLACEHOLDER_SUBJECT_INAPP_PAYLOAD_VALUE } }, - }; -} - function buildEmailControlValuesPayload(stepId?: string): EmailStepControlSchemaDto { return { subject: `Hello, World! ${SUBJECT_TEST_PAYLOAD}`, @@ -745,3 +668,50 @@ function assertEmail(dto: GeneratePreviewResponseDto) { expect(preview).to.contain('should be the fallback value'); } } + +export async function createWorkflowAndReturnId( + workflowsClient: ReturnType, + type: StepTypeEnum +) { + const createWorkflowDto = buildCreateWorkflowDto(`${type}:${randomUUID()}`); + createWorkflowDto.steps[0].type = type; + const workflowResult = await workflowsClient.createWorkflow(createWorkflowDto); + if (!workflowResult.isSuccessResult()) { + throw new Error(`Failed to create workflow ${JSON.stringify(workflowResult.error)}`); + } + + return { + workflowId: workflowResult.value._id, + stepDatabaseId: workflowResult.value.steps[0]._id, + stepId: workflowResult.value.steps[0].stepId, + }; +} + +export async function generatePreview( + workflowsClient: ReturnType, + workflowId: string, + stepDatabaseId: string, + dto: GeneratePreviewRequestDto, + description: string +): Promise { + const novuRestResult = await workflowsClient.generatePreview(workflowId, stepDatabaseId, dto); + if (novuRestResult.isSuccessResult()) { + return novuRestResult.value; + } + + throw await assertHttpError(description, novuRestResult, dto); +} + +function buildDtoWithMissingControlValues(stepTypeEnum: StepTypeEnum, stepId: string): GeneratePreviewRequestDto { + const stepTypeToElement = getTestControlValues(stepId)[stepTypeEnum]; + if (stepTypeEnum === StepTypeEnum.EMAIL) { + delete stepTypeToElement.subject; + } else { + delete stepTypeToElement.body; + } + + return { + controlValues: stepTypeToElement, + previewPayload: { payload: { subject: PLACEHOLDER_SUBJECT_INAPP_PAYLOAD_VALUE } }, + }; +} diff --git a/apps/api/src/app/workflows-v2/usecases/build-payload-schema/build-payload-schema.usecase.ts b/apps/api/src/app/workflows-v2/usecases/build-payload-schema/build-payload-schema.usecase.ts index 20d589540d4..1c4bcbf717e 100644 --- a/apps/api/src/app/workflows-v2/usecases/build-payload-schema/build-payload-schema.usecase.ts +++ b/apps/api/src/app/workflows-v2/usecases/build-payload-schema/build-payload-schema.usecase.ts @@ -61,6 +61,9 @@ export class BuildPayloadSchema { private extractTemplateVariables(controlValues: Record[]): string[] { const controlValuesString = controlValues.map(flattenObjectValues).flat().join(' '); - return extractLiquidTemplateVariables(controlValuesString).validVariables.map((variable) => variable.name); + const test = extractLiquidTemplateVariables(controlValuesString); + const test2 = test.validVariables.map((variable) => variable.name); + + return test2; } } diff --git a/apps/api/src/app/workflows-v2/usecases/generate-preview/generate-preview.usecase.ts b/apps/api/src/app/workflows-v2/usecases/generate-preview/generate-preview.usecase.ts index 472ba83708d..6a11fca21db 100644 --- a/apps/api/src/app/workflows-v2/usecases/generate-preview/generate-preview.usecase.ts +++ b/apps/api/src/app/workflows-v2/usecases/generate-preview/generate-preview.usecase.ts @@ -58,13 +58,13 @@ export class GeneratePreviewUsecase { }) ); - const res = this.buildVariablesSchema(stepData.variables, payloadSchema); + const variableSchema = this.buildVariablesSchema(stepData.variables, payloadSchema); const preparedAndValidatedContent = await this.prepareAndValidateContentUsecase.execute({ user: command.user, previewPayloadFromDto: commandVariablesExample, controlValues, controlDataSchema: stepData.controls.dataSchema || {}, - variableSchema: res, + variableSchema, }); const variablesExample = this.buildVariablesExample( workflow, diff --git a/apps/api/src/app/workflows-v2/workflow.controller.e2e.ts b/apps/api/src/app/workflows-v2/workflow.controller.e2e.ts index 657d03db48b..564aa93d44d 100644 --- a/apps/api/src/app/workflows-v2/workflow.controller.e2e.ts +++ b/apps/api/src/app/workflows-v2/workflow.controller.e2e.ts @@ -44,6 +44,13 @@ const TEST_WORKFLOW_NAME = 'Test Workflow Name'; const TEST_TAGS = ['test']; let session: UserSession; +function getHeaders(overrideEnv?: string): HeadersInit { + return { + Authorization: session.token, + 'Novu-Environment-Id': overrideEnv || session.environment._id, + }; +} + describe('Workflow Controller E2E API Testing', () => { let workflowsClient: ReturnType; @@ -55,12 +62,6 @@ describe('Workflow Controller E2E API Testing', () => { after(async () => { await sleep(1000); }); - function getHeaders(overrideEnv?: string): HeadersInit { - return { - Authorization: session.token, // Fixed space - 'Novu-Environment-Id': overrideEnv || session.environment._id, - }; - } it('Smoke Testing', async () => { const workflowCreated = await createWorkflowAndValidate(); @@ -1134,6 +1135,85 @@ describe('Workflow Controller E2E API Testing', () => { steps: [updatedStep, newStep], }; } + + describe('Workflow Step Issues', () => { + it('should show issues for illegal variables in control values', async () => { + const createWorkflowDto: CreateWorkflowDto = buildCreateWorkflowDto('test-issues', { + steps: [ + { + name: 'Email Test Step', + type: StepTypeEnum.EMAIL, + }, + ], + }); + + const res = await workflowsClient.createWorkflow(createWorkflowDto); + expect(res.isSuccessResult()).to.be.true; + if (res.isSuccessResult()) { + const workflow = res.value; + await workflowsClient.patchWorkflowStepData(workflow._id, workflow.steps[0]._id, { + controlValues: { body: 'Welcome {{}}' }, + }); + + const stepData = await getStepData(workflow._id, workflow.steps[0]._id); + expect(stepData.issues, 'Step data should have issues').to.exist; + expect(stepData.issues?.controls?.body, 'Step data should have body issues').to.exist; + expect(stepData.issues?.controls?.body?.[0]?.variableName).to.equal('{{}}'); + expect(stepData.issues?.controls?.body?.[0]?.issueType).to.equal('ILLEGAL_VARIABLE_IN_CONTROL_VALUE'); + } + }); + + it('should show issues for invalid URLs', async () => { + const createWorkflowDto: CreateWorkflowDto = buildCreateWorkflowDto('test-issues', { + steps: [ + { + name: 'In-App Test Step', + type: StepTypeEnum.IN_APP, + }, + ], + }); + + const res = await workflowsClient.createWorkflow(createWorkflowDto); + expect(res.isSuccessResult()).to.be.true; + if (res.isSuccessResult()) { + const workflow = res.value; + await workflowsClient.patchWorkflowStepData(workflow._id, workflow.steps[0]._id, { + controlValues: { + redirect: { url: 'not-good-url-please-replace' }, + primaryAction: { redirect: { url: 'not-good-url-please-replace' } }, + secondaryAction: { redirect: { url: 'not-good-url-please-replace' } }, + }, + }); + + const stepData = await getStepData(workflow._id, workflow.steps[0]._id); + expect(stepData.issues, 'Step data should have issues').to.exist; + expect(stepData.issues?.controls?.['redirect.url']?.[0]?.issueType).to.equal('INVALID_URL'); + expect(stepData.issues?.controls?.['primaryAction.redirect.url']?.[0]?.issueType).to.equal('INVALID_URL'); + expect(stepData.issues?.controls?.['secondaryAction.redirect.url']?.[0]?.issueType).to.equal('INVALID_URL'); + } + }); + + it('should show issues for missing required control values', async () => { + const createWorkflowDto: CreateWorkflowDto = buildCreateWorkflowDto('test-issues', { + steps: [ + { + name: 'In-App Test Step', + type: StepTypeEnum.IN_APP, + }, + ], + }); + + const res = await workflowsClient.createWorkflow(createWorkflowDto); + expect(res.isSuccessResult()).to.be.true; + if (res.isSuccessResult()) { + const workflow = res.value; + const stepData = await getStepData(workflow._id, workflow.steps[0]._id); + expect(stepData.issues, 'Step data should have issues').to.exist; + expect(stepData.issues?.controls, 'Step data should have control issues').to.exist; + expect(stepData.issues?.controls?.body?.[0]?.issueType).to.equal('MISSING_VALUE'); + } + }); + }); }); function buildEmailStep(): StepCreateDto {