Skip to content

Commit

Permalink
fix(framework): Stop requiring default properties to be specified in …
Browse files Browse the repository at this point in the history
…outputs (novuhq#6373)
  • Loading branch information
rifont authored Aug 22, 2024
1 parent 4e038ff commit cb80926
Show file tree
Hide file tree
Showing 12 changed files with 227 additions and 68 deletions.
2 changes: 2 additions & 0 deletions .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,8 @@
"unpublish",
"unsub",
"untracked",
"unvalidated",
"Unvalidated",
"upsert",
"upserted",
"upstash",
Expand Down
16 changes: 8 additions & 8 deletions libs/application-generic/src/utils/bridge.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { DigestTypeEnum } from '@novu/shared';
import {
DigestOutput,
digestRegularOutput,
digestTimedOutput,
DigestRegularOutput,
DigestTimedOutput,
} from '@novu/framework';

export function getDigestType(outputs: DigestOutput): DigestTypeEnum {
Expand All @@ -17,21 +17,21 @@ export function getDigestType(outputs: DigestOutput): DigestTypeEnum {

export const isTimedDigestOutput = (
outputs: DigestOutput | undefined
): outputs is digestTimedOutput => {
return (outputs as digestTimedOutput)?.cron != null;
): outputs is DigestTimedOutput => {
return (outputs as DigestTimedOutput)?.cron != null;
};

export const isLookBackDigestOutput = (
outputs: DigestOutput
): outputs is digestRegularOutput => {
): outputs is DigestRegularOutput => {
return (
(outputs as digestRegularOutput)?.lookBackWindow?.amount != null &&
(outputs as digestRegularOutput)?.lookBackWindow?.unit != null
(outputs as DigestRegularOutput)?.lookBackWindow?.amount != null &&
(outputs as DigestRegularOutput)?.lookBackWindow?.unit != null
);
};

export const isRegularDigestOutput = (
outputs: DigestOutput
): outputs is digestRegularOutput => {
): outputs is DigestRegularOutput => {
return !isTimedDigestOutput(outputs) && !isLookBackDigestOutput(outputs);
};
13 changes: 6 additions & 7 deletions packages/framework/src/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ import {
WorkflowNotFoundError,
} from './errors';
import { workflow } from './resources';
import { Event, Step, FromSchema } from './types';
import { delayOutputSchema, channelStepSchemas } from './schemas';
import { FRAMEWORK_VERSION, SDK_VERSION, ChannelStepEnum, PostActionEnum } from './constants';
import { Event, Step } from './types';
import { FRAMEWORK_VERSION, SDK_VERSION, PostActionEnum } from './constants';

describe('Novu Client', () => {
let client: Client;
Expand Down Expand Up @@ -444,11 +443,11 @@ describe('Novu Client', () => {

describe('executeWorkflow method', () => {
it('should execute workflow successfully when action is execute and payload is provided', async () => {
const delayConfiguration: FromSchema<typeof delayOutputSchema> = { type: 'regular', unit: 'seconds', amount: 1 };
const emailConfiguration: FromSchema<(typeof channelStepSchemas)[ChannelStepEnum.EMAIL]['output']> = {
const delayConfiguration = { unit: 'seconds', amount: 1 } as const;
const emailConfiguration = {
body: 'Test Body',
subject: 'Subject',
};
} as const;
const newWorkflow = workflow('test-workflow', async ({ step }) => {
await step.email('send-email', async () => emailConfiguration);
await step.delay('delay', async () => delayConfiguration);
Expand Down Expand Up @@ -515,7 +514,7 @@ describe('Novu Client', () => {
expect(amount).toBe(delayConfiguration.amount);
expect(delayExecutionResult.providers).toEqual({});
const type = delayExecutionResult.outputs.type;
expect(type).toBe(delayConfiguration.type);
expect(type).toBe('regular');
});

it('should compile default control variable', async () => {
Expand Down
67 changes: 67 additions & 0 deletions packages/framework/src/resources/workflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,41 @@ describe('workflow function', () => {
result?.foo === 'custom';
});
});

it('should compile when returning undefined for a built-in step property that has a default value', async () => {
const delayType = undefined;
workflow('built-in-default-test', async ({ step }) => {
await step.delay('custom', async () => ({
type: delayType,
amount: 1,
unit: 'seconds',
}));
});
});

it('should compile when returning undefined for a custom step property that has a default value', async () => {
const delayType = undefined;
workflow('custom-default-test', async ({ step }) => {
const data = await step.custom(
'custom',
async () => ({
withDefault: undefined,
withoutDefault: 'bar',
}),
{
outputSchema: {
type: 'object',
properties: {
withDefault: { type: 'string', default: 'bar' },
withoutDefault: { type: 'string' },
},
required: ['withoutDefault'],
additionalProperties: false,
} as const,
}
);
});
});
});

describe('trigger', () => {
Expand Down Expand Up @@ -104,6 +139,38 @@ describe('workflow function', () => {
});
});

it('should compile when returning undefined for a payload property that has a default value', async () => {
const testWorkflow = workflow(
'test-workflow',
async ({ step }) => {
await step.custom('custom', async () => ({
foo: 'bar',
}));
},
{
payloadSchema: {
type: 'object',
properties: {
withDefault: { type: 'string', default: 'bar' },
withoutDefault: { type: 'string' },
},
required: ['withoutDefault'],
additionalProperties: false,
} as const,
}
);

// Capture in a test function to avoid throwing execution errors
const testFn = () =>
testWorkflow.trigger({
payload: {
withDefault: undefined,
withoutDefault: 'bar',
},
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
17 changes: 11 additions & 6 deletions packages/framework/src/resources/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
ChannelStep,
ActionStep,
StepOutput,
FromSchemaUnvalidated,
} from '../types';
import { WithPassthrough } from '../types/provider.types';
import { EMOJI, getBridgeUrl, initApiClient, log } from '../utils';
Expand All @@ -28,29 +29,33 @@ import { transformSchema, validateData } from '../validators';
export function workflow<
T_PayloadSchema extends Schema,
T_ControlSchema extends Schema,
T_Payload extends Record<string, unknown> = FromSchema<T_PayloadSchema>,
T_PayloadValidated extends Record<string, unknown> = FromSchema<T_PayloadSchema>,
T_PayloadUnvalidated extends Record<string, unknown> = FromSchemaUnvalidated<T_PayloadSchema>,
T_Controls extends Record<string, unknown> = FromSchema<T_ControlSchema>
>(
workflowId: string,
execute: Execute<T_Payload, T_Controls>,
execute: Execute<T_PayloadValidated, T_Controls>,
workflowOptions?: WorkflowOptions<T_PayloadSchema, T_ControlSchema>
): Workflow<T_Payload> {
): Workflow<T_PayloadUnvalidated> {
const options = workflowOptions ? workflowOptions : {};

const apiClient = initApiClient(process.env.NOVU_SECRET_KEY as string);

const trigger: Workflow<T_Payload>['trigger'] = async (event) => {
const trigger: Workflow<T_PayloadUnvalidated>['trigger'] = async (event) => {
if (!process.env.NOVU_SECRET_KEY) {
throw new MissingSecretKeyError();
}

let validatedData: T_Payload = event.payload;
let validatedData: T_PayloadValidated;
if (options.payloadSchema) {
const validationResult = await validateData(options.payloadSchema, event.payload);
if (validationResult.success === false) {
throw new WorkflowPayloadInvalidError(workflowId, validationResult.errors);
}
validatedData = validationResult.data;
} else {
// This type coercion provides support to trigger Workflows without a payload schema
validatedData = event.payload as unknown as T_PayloadValidated;
}
const bridgeUrl = await getBridgeUrl();

Expand Down Expand Up @@ -116,7 +121,7 @@ export function workflow<
};

execute({
payload: {} as T_Payload,
payload: {} as T_PayloadValidated,
subscriber: {},
environment: {},
controls: {} as T_Controls,
Expand Down
4 changes: 2 additions & 2 deletions packages/framework/src/types/provider.types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { providerSchemas } from '../schemas';
import type { FromSchema } from './schema.types';
import type { FromSchemaUnvalidated } from './schema.types';
import { Awaitable, Prettify } from './util.types';

export type Passthrough = {
Expand Down Expand Up @@ -29,5 +29,5 @@ export type Providers<T_StepType extends keyof typeof providerSchemas, T_Control
// eslint-disable-next-line multiline-comment-style
// TODO: fix the typing for `type` to use the keyof providerSchema[channelType]
// @ts-expect-error - Types of parameters 'options' and 'options' are incompatible.
}) => Awaitable<WithPassthrough<FromSchema<(typeof providerSchemas)[T_StepType][K]['output']>>>;
}) => Awaitable<WithPassthrough<FromSchemaUnvalidated<(typeof providerSchemas)[T_StepType][K]['output']>>>;
};
46 changes: 46 additions & 0 deletions packages/framework/src/types/schema.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,54 @@ import zod from 'zod';

export type JsonSchema = JSONSchema;

/**
* A schema used to validate a JSON object.
*
* Supported schemas:
* - JSONSchema
* - ZodSchema
*/
export type Schema = JsonSchema | zod.ZodSchema;

/**
* Infer the type of a Schema for unvalidated data.
*
* The resulting type has default properties set to optional,
* reflecting the fact that the data is unvalidated and has
* not had default properties set.
*
* @example
* ```ts
* type MySchema = FromSchemaUnvalidated<typeof mySchema>;
* ```
*/
export type FromSchemaUnvalidated<T extends Schema> =
/*
* Handle each Schema's type inference individually until
* all Schema types are exhausted.
*/

// JSONSchema
T extends JSONSchema
? JsonSchemaInfer<T, { keepDefaultedPropertiesOptional: true }>
: // ZodSchema
T extends zod.ZodSchema
? zod.input<T>
: // All schema types exhausted.
never;

/**
* Infer the type of a Schema for validated data.
*
* The resulting type has default properties set to required,
* reflecting the fact that the data has been validated and
* default properties have been set.
*
* @example
* ```ts
* type MySchema = FromSchema<typeof mySchema>;
* ```
*/
export type FromSchema<T extends Schema> =
/*
* Handle each Schema's type inference individually until
Expand Down
Loading

0 comments on commit cb80926

Please sign in to comment.