Skip to content

Commit

Permalink
fix(framework, node): Make the payload property optional during tri…
Browse files Browse the repository at this point in the history
…gger (#6384)
  • Loading branch information
rifont authored Aug 28, 2024
1 parent 7fdff4c commit dae6a0c
Show file tree
Hide file tree
Showing 13 changed files with 345 additions and 24 deletions.
3 changes: 3 additions & 0 deletions packages/framework/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,6 @@ logs

# Misc
.DS_Store

# Vitest
tsconfig.vitest-temp.json
4 changes: 2 additions & 2 deletions packages/framework/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
"README.md"
],
"scripts": {
"test": "vitest",
"test:watch": "vitest --watch",
"test": "vitest --typecheck",
"test:watch": "vitest --typecheck --watch",
"lint": "eslint src --ext .ts --config .eslintrc.js",
"lint:fix": "eslint src --ext .ts --fix --config .eslintrc.js",
"format": "prettier --check --ignore-path .gitignore .",
Expand Down
4 changes: 3 additions & 1 deletion packages/framework/scripts/devtool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ import $RefParser from '@apidevtools/json-schema-ref-parser';
const main = async () => {
const parser = new $RefParser();
const schema = await parser.dereference(__dirname + '/schema_input.json');
// eslint-disable-next-line multiline-comment-style
// Edit this path to target the JSON schema that the send message endpoint uses
const body = schema['components']['schemas']['api.v2010.account.message'];
// @ts-expect-error - components does not exist on the schema
const body = schema.components.schemas['api.v2010.account.message'];
writeFileSync(__dirname + '/schema_output.json', JSON.stringify(body, null, 2));

console.log('schema.json updated');
Expand Down
2 changes: 1 addition & 1 deletion packages/framework/src/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ describe('Novu Client', () => {
}),
{
providers: {
slack: async ({ controls }) => {
slack: async () => {
return {
blocks: [
{
Expand Down
5 changes: 3 additions & 2 deletions packages/framework/src/client.validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Client } from './client';
import { z } from 'zod';
import { workflow } from './resources/workflow';
import { ExecutionStateControlsInvalidError } from './errors';
import { PostActionEnum } from './constants';

describe('validation', () => {
let client: Client;
Expand Down Expand Up @@ -160,7 +161,7 @@ describe('validation', () => {

try {
await client.executeWorkflow({
action: 'execute',
action: PostActionEnum.EXECUTE,
workflowId: 'zod-validation',
inputs: {
foo: '341',
Expand Down Expand Up @@ -344,7 +345,7 @@ describe('validation', () => {

try {
await client.executeWorkflow({
action: 'execute',
action: PostActionEnum.EXECUTE,
workflowId: 'json-schema-validation',
inputs: {
foo: '341',
Expand Down
1 change: 0 additions & 1 deletion packages/framework/src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ export class NovuRequestHandler<Input extends any[] = any[], Output = any> {
transactionId: triggerEvent.transactionId,
overrides: triggerEvent.overrides || {},
...(triggerEvent.actor && { actor: triggerEvent.actor }),
...(triggerEvent.tenant && { tenant: triggerEvent.tenant }),
...(triggerEvent.bridgeUrl && { bridgeUrl: triggerEvent.bridgeUrl }),
...(triggerEvent.controls && { controls: triggerEvent.controls }),
};
Expand Down
71 changes: 70 additions & 1 deletion packages/framework/src/resources/workflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe('workflow function', () => {
type: 'object',
properties: {
foo: { type: 'number' },
bar: { type: 'string' },
bar: { type: 'string', default: 'baz' },
},
required: ['foo', 'bar'],
additionalProperties: false,
Expand Down Expand Up @@ -171,6 +171,75 @@ describe('workflow function', () => {
});
});

it('should not compile when the payload is not specified and the payloadSchema declares required properties', async () => {
const testWorkflow = workflow(
'test-workflow',
async ({ step, payload }) => {
await step.custom('custom', async () => ({
foo: 'bar',
}));
},
{
payloadSchema: {
type: 'object',
properties: {
foo: { type: 'string' },
},
required: ['foo'],
additionalProperties: false,
} as const,
}
);

// Capture in a test function to avoid throwing execution errors
const testFn = () =>
testWorkflow.trigger({
// @ts-expect-error - payload is missing from the trigger
payload: undefined,
to: '[email protected]',
});
});

it('should compile when the payload is not specified and the payloadSchema does not declare required properties', async () => {
const testWorkflow = workflow(
'test-workflow',
async ({ step, payload }) => {
await step.custom('custom', async () => ({
foo: 'bar',
}));
},
{
payloadSchema: {
type: 'object',
properties: {
foo: { type: 'string' },
},
additionalProperties: false,
} as const,
}
);

// Capture in a test function to avoid throwing execution errors
const testFn = () =>
testWorkflow.trigger({
to: '[email protected]',
});
});

it('should compile when the payload is not specified and the payloadSchema is not specified', async () => {
const testWorkflow = workflow('test-workflow', async ({ step, payload }) => {
await step.custom('custom', async () => ({
foo: 'bar',
}));
});

// Capture in a test function to avoid throwing execution errors
const testFn = () =>
testWorkflow.trigger({
to: '[email protected]',
});
});

it('should throw an error when the NOVU_SECRET_KEY is not set', async () => {
const originalEnv = process.env.NOVU_SECRET_KEY;
delete process.env.NOVU_SECRET_KEY;
Expand Down
4 changes: 2 additions & 2 deletions packages/framework/src/resources/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,10 @@ export function workflow<
throw new MissingSecretKeyError();
}

const unvalidatedData = (event.payload || {}) as T_PayloadUnvalidated;
let validatedData: T_PayloadValidated;
if (options.payloadSchema) {
const validationResult = await validateData(options.payloadSchema, event.payload);
const validationResult = await validateData(options.payloadSchema, unvalidatedData);
if (validationResult.success === false) {
throw new WorkflowPayloadInvalidError(workflowId, validationResult.errors);
}
Expand All @@ -68,7 +69,6 @@ export function workflow<
...(event.transactionId && { transactionId: event.transactionId }),
...(event.overrides && { overrides: event.overrides }),
...(event.actor && { actor: event.actor }),
...(event.tenant && { tenant: event.tenant }),
...(bridgeUrl && { bridgeUrl }),
};

Expand Down
22 changes: 10 additions & 12 deletions packages/framework/src/types/event.types.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
import type {
ITenantDefine,
ITriggerPayload,
TriggerEventStatusEnum,
TriggerRecipientsPayload,
TriggerRecipientSubscriber,
} from '@novu/shared';
import { ConditionalPartial, PickRequiredKeys } from './util.types';

// eslint-disable-next-line @typescript-eslint/ban-types
type EventPayload = ITriggerPayload & {};

// eslint-disable-next-line @typescript-eslint/ban-types
type Actor = TriggerRecipientSubscriber & {};

type Tenant = ITenantDefine & Record<string, never>;

// eslint-disable-next-line @typescript-eslint/ban-types
type Recipients = TriggerRecipientsPayload & {};

Expand Down Expand Up @@ -45,14 +43,6 @@ export type EventTriggerParams<T_Payload = EventPayload> = {
* Bridge url to trigger the workflow to
*/
bridgeUrl?: string;
/**
* Payload to trigger the workflow with
*/
payload: T_Payload;
/**
* Tenant to trigger the workflow with
*/
tenant?: Tenant;
/**
* Transaction id for trigger
*/
Expand All @@ -69,7 +59,15 @@ export type EventTriggerParams<T_Payload = EventPayload> = {
[stepId: string]: Record<string, unknown>;
};
};
};
} & ConditionalPartial<
{
/**
* Payload to trigger the workflow with
*/
payload: T_Payload;
},
PickRequiredKeys<T_Payload> extends never ? true : false
>;

export type EventTriggerResponse = {
/**
Expand Down
Loading

0 comments on commit dae6a0c

Please sign in to comment.