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-service): refactor issue error messages #7359

Merged
merged 24 commits into from
Dec 25, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6fefacb
feat(api): wip refactor issue error messages
djabarovgeorge Dec 23, 2024
18a0c79
Merge remote-tracking branch 'origin/next' into refactor-issue-error-…
djabarovgeorge Dec 23, 2024
366112a
fix: after next merge
djabarovgeorge Dec 23, 2024
5ae76ca
fix: in app schema
djabarovgeorge Dec 23, 2024
50c60f5
feat(api): add nested liquid js issues
djabarovgeorge Dec 23, 2024
0651c78
fix(api): preview output validation
djabarovgeorge Dec 23, 2024
8fe5c6f
refactor(api): renders
djabarovgeorge Dec 24, 2024
e755702
Merge remote-tracking branch 'origin/next' into refactor-issue-error-…
djabarovgeorge Dec 24, 2024
7b0bb08
Merge remote-tracking branch 'origin/next' into refactor-issue-error-…
djabarovgeorge Dec 24, 2024
9eb061e
Merge remote-tracking branch 'origin/next' into refactor-issue-error-…
djabarovgeorge Dec 24, 2024
59551dc
fix(api-service): test
djabarovgeorge Dec 24, 2024
c25a6e4
fix(api-service): update schema
djabarovgeorge Dec 24, 2024
a6ce376
fix(api-service): revert test
djabarovgeorge Dec 24, 2024
6ed6a94
fix(dashboard): remove controls validation from client
djabarovgeorge Dec 25, 2024
8cd987d
fix(api): framework workflow sanitization
djabarovgeorge Dec 25, 2024
442abbe
fix(api): sanitize framework workflow
djabarovgeorge Dec 25, 2024
8198d19
fix(api): tests
djabarovgeorge Dec 25, 2024
3bbbf13
fix(api): dto create
djabarovgeorge Dec 25, 2024
2671471
Merge remote-tracking branch 'origin/next' into refactor-issue-error-…
djabarovgeorge Dec 25, 2024
06d86d9
fix(api): dto update
djabarovgeorge Dec 25, 2024
2eb9552
fix(api): dto update
djabarovgeorge Dec 25, 2024
3fa7042
fix(api): tests
djabarovgeorge Dec 25, 2024
93c94bb
fix(api): tests
djabarovgeorge Dec 25, 2024
488e793
Merge remote-tracking branch 'origin/next' into refactor-issue-error-…
djabarovgeorge Dec 25, 2024
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
18 changes: 9 additions & 9 deletions apps/api/src/app/workflows-v2/shared/sanitize-control-values.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,24 +41,24 @@ type LookBackWindow = {
unit: string;
};

function sanitizeRedirect(redirect: Redirect | undefined) {
if (!redirect?.url || !redirect?.target) {
return undefined;
function sanitizeRedirect(redirect: Redirect | undefined, isOptional: boolean = false) {
if (isOptional && (!redirect?.url || !redirect?.target)) {
return null;
}

return {
url: redirect.url || 'https://example.com',
target: redirect.target || '_self',
url: redirect?.url as string,
target: redirect?.target as '_self' | '_blank' | '_parent' | '_top' | '_unfencedTop',
};
}

function sanitizeAction(action: Action) {
if (!action?.label) {
return undefined;
if (!action?.label && !action?.redirect?.url && !action?.redirect?.target) {
return null;
}

return {
label: action.label,
label: action.label as string,
redirect: sanitizeRedirect(action.redirect),
};
}
Expand All @@ -84,7 +84,7 @@ function sanitizeInApp(controlValues: InAppControlType) {
}

if (controlValues.redirect) {
normalized.redirect = sanitizeRedirect(controlValues.redirect as Redirect);
normalized.redirect = sanitizeRedirect(controlValues.redirect as Redirect, true);
}

return filterNullishValues(normalized);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@ import { z } from 'zod';
import { zodToJsonSchema } from 'zod-to-json-schema';

import { JSONSchemaDto, UiComponentEnum, UiSchema, UiSchemaGroupEnum } from '@novu/shared';
import { skipStepUiSchema } from './skip-control.schema';
import { skipStepUiSchema, skipZodSchema } from './skip-control.schema';
import { defaultOptions } from './shared';

export const chatControlZodSchema = z
.object({
skip: z.object({}).catchall(z.unknown()).optional(),
skip: skipZodSchema,
body: z.string(),
})
.strict();

export type ChatControlType = z.infer<typeof chatControlZodSchema>;

export const chatControlSchema = zodToJsonSchema(chatControlZodSchema) as JSONSchemaDto;
export const chatControlSchema = zodToJsonSchema(chatControlZodSchema, defaultOptions) as JSONSchemaDto;
export const chatUiSchema: UiSchema = {
group: UiSchemaGroupEnum.CHAT,
properties: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import {
UiSchema,
UiSchemaGroupEnum,
} from '@novu/shared';
import { skipStepUiSchema } from './skip-control.schema';
import { skipStepUiSchema, skipZodSchema } from './skip-control.schema';
import { defaultOptions } from './shared';

export const delayControlZodSchema = z
.object({
skip: z.object({}).catchall(z.unknown()).optional(),
skip: skipZodSchema,
type: z.enum(['regular']).default('regular'),
amount: z.union([z.number().min(1), z.string()]),
unit: z.nativeEnum(TimeUnitEnum),
Expand All @@ -21,7 +22,7 @@ export const delayControlZodSchema = z

export type DelayControlType = z.infer<typeof delayControlZodSchema>;

export const delayControlSchema = zodToJsonSchema(delayControlZodSchema) as JSONSchemaDto;
export const delayControlSchema = zodToJsonSchema(delayControlZodSchema, defaultOptions) as JSONSchemaDto;
export const delayUiSchema: UiSchema = {
group: UiSchemaGroupEnum.DELAY,
properties: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import {
UiSchema,
UiSchemaGroupEnum,
} from '@novu/shared';
import { skipStepUiSchema } from './skip-control.schema';
import { skipStepUiSchema, skipZodSchema } from './skip-control.schema';
import { defaultOptions } from './shared';

const digestRegularControlZodSchema = z
.object({
skip: z.object({}).catchall(z.unknown()).optional(),
skip: skipZodSchema,
amount: z.union([z.number().min(1), z.string().min(1)]),
unit: z.nativeEnum(TimeUnitEnum),
digestKey: z.string().optional(),
Expand All @@ -38,7 +39,7 @@ export type DigestTimedControlType = z.infer<typeof digestTimedControlZodSchema>
export type DigestControlSchemaType = z.infer<typeof digestControlZodSchema>;

export const digestControlZodSchema = z.union([digestRegularControlZodSchema, digestTimedControlZodSchema]);
export const digestControlSchema = zodToJsonSchema(digestControlZodSchema) as JSONSchemaDto;
export const digestControlSchema = zodToJsonSchema(digestControlZodSchema, defaultOptions) as JSONSchemaDto;

export function isDigestRegularControl(data: unknown): data is DigestRegularControlType {
const result = digestRegularControlZodSchema.safeParse(data);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import { JSONSchemaDto, UiComponentEnum, UiSchema, UiSchemaGroupEnum } from '@no
import { z } from 'zod';
import { zodToJsonSchema } from 'zod-to-json-schema';
import { TipTapSchema } from '../../../environments-v1/usecases/output-renderers';
import { skipStepUiSchema } from './skip-control.schema';
import { skipZodSchema, skipStepUiSchema } from './skip-control.schema';
import { defaultOptions } from './shared';

export const emailControlZodSchema = z
.object({
skip: z.object({}).catchall(z.unknown()).optional(),
skip: skipZodSchema,
/*
* todo: we need to validate the email editor (body) by type and not string,
* updating it to TipTapSchema will break the existing upsert issues generation
Expand All @@ -18,7 +19,7 @@ export const emailControlZodSchema = z

export type EmailControlType = z.infer<typeof emailControlZodSchema>;

export const emailControlSchema = zodToJsonSchema(emailControlZodSchema) as JSONSchemaDto;
export const emailControlSchema = zodToJsonSchema(emailControlZodSchema, defaultOptions) as JSONSchemaDto;
export const emailUiSchema: UiSchema = {
group: UiSchemaGroupEnum.EMAIL,
properties: {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,46 +1,60 @@
import { z } from 'zod';
import { zodToJsonSchema } from 'zod-to-json-schema';
import { JSONSchemaDto, UiComponentEnum, UiSchema, UiSchemaGroupEnum } from '@novu/shared';
import { skipStepUiSchema } from './skip-control.schema';
import { skipStepUiSchema, skipZodSchema } from './skip-control.schema';
import { defaultOptions } from './shared';

/**
* Regex pattern for validating URLs with template variables. Matches three cases:
*
* 1. ^(\{\{[^}]*\}\}.*)
* - Matches URLs that start with template variables like {{variable}}
* - Example: {{variable}}, {{variable}}/path
*
* 2. ^(?!mailto:)(?:(https?):\/\/[^\s/$.?#].[^\s]*(?:\{\{[^}]*\}\}[^\s]*)*)
* - Matches full URLs that may contain template variables
* - Excludes mailto: links
* - Example: https://example.com, https://example.com/{{variable}}, https://{{variable}}.com
*
* 3. ^(\/[^\s]*(?:\{\{[^}]*\}\}[^\s]*)*)$
* - Matches partial URLs (paths) that may contain template variables
* - Example: /path/to/page, /path/{{variable}}/page
*/
const templateUrlPattern =
/^(\{\{[^}]*\}\}.*)|^(?!mailto:)(?:(https?):\/\/[^\s/$.?#].[^\s]*(?:\{\{[^}]*\}\}[^\s]*)*)|^(\/[^\s]*(?:\{\{[^}]*\}\}[^\s]*)*)$/;
Fixed Show fixed Hide fixed

const redirectZodSchema = z
.object({
url: z.string().optional(),
url: z.string().regex(templateUrlPattern),
target: z.enum(['_self', '_blank', '_parent', '_top', '_unfencedTop']).default('_blank'),
})
.strict()
.optional()
.nullable();

const actionZodSchema = z
.object({
label: z.string().optional(),
redirect: redirectZodSchema.optional(),
label: z.string(),
redirect: redirectZodSchema,
})
.strict()
.optional()
.nullable();

export const inAppControlZodSchema = z
.object({
skip: z.object({}).catchall(z.unknown()).optional(),
subject: z.string().optional(),
body: z.string(),
avatar: z.string().optional(),
primaryAction: actionZodSchema,
secondaryAction: actionZodSchema,
data: z.object({}).catchall(z.unknown()).optional(),
redirect: redirectZodSchema,
})
.strict();
export const inAppControlZodSchema = z.object({
skip: skipZodSchema,
subject: z.string().optional(),
body: z.string(),
avatar: z.string().regex(templateUrlPattern).optional(),
primaryAction: actionZodSchema,
secondaryAction: actionZodSchema,
data: z.object({}).catchall(z.unknown()).optional(),
redirect: redirectZodSchema.optional(),
});

export type InAppRedirectType = z.infer<typeof redirectZodSchema>;
export type InAppActionType = z.infer<typeof actionZodSchema>;
export type InAppControlType = z.infer<typeof inAppControlZodSchema>;

export const inAppRedirectSchema = zodToJsonSchema(redirectZodSchema) as JSONSchemaDto;
export const inAppActionSchema = zodToJsonSchema(actionZodSchema) as JSONSchemaDto;
export const inAppControlSchema = zodToJsonSchema(inAppControlZodSchema) as JSONSchemaDto;
export const inAppRedirectSchema = zodToJsonSchema(redirectZodSchema, defaultOptions) as JSONSchemaDto;
export const inAppActionSchema = zodToJsonSchema(actionZodSchema, defaultOptions) as JSONSchemaDto;
export const inAppControlSchema = zodToJsonSchema(inAppControlZodSchema, defaultOptions) as JSONSchemaDto;

const redirectPlaceholder = {
url: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,20 @@ import { z } from 'zod';
import { zodToJsonSchema } from 'zod-to-json-schema';

import { JSONSchemaDto, UiComponentEnum, UiSchema, UiSchemaGroupEnum } from '@novu/shared';
import { skipStepUiSchema } from './skip-control.schema';
import { skipZodSchema, skipStepUiSchema } from './skip-control.schema';
import { defaultOptions } from './shared';

export const pushControlZodSchema = z
.object({
skip: z.object({}).catchall(z.unknown()).optional(),
skip: skipZodSchema,
subject: z.string(),
body: z.string(),
})
.strict();

export type PushControlType = z.infer<typeof pushControlZodSchema>;

export const pushControlSchema = zodToJsonSchema(pushControlZodSchema) as JSONSchemaDto;
export const pushControlSchema = zodToJsonSchema(pushControlZodSchema, defaultOptions) as JSONSchemaDto;
export const pushUiSchema: UiSchema = {
group: UiSchemaGroupEnum.PUSH,
properties: {
Expand Down
5 changes: 5 additions & 0 deletions apps/api/src/app/workflows-v2/shared/schemas/shared.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { Targets, Options } from 'zod-to-json-schema';

export const defaultOptions: Partial<Options<Targets>> = {
$refStrategy: 'none',
};
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@ import { z } from 'zod';
import { zodToJsonSchema } from 'zod-to-json-schema';

import { JSONSchemaDto, UiComponentEnum, UiSchema, UiSchemaGroupEnum } from '@novu/shared';
import { skipStepUiSchema } from './skip-control.schema';
import { skipZodSchema, skipStepUiSchema } from './skip-control.schema';
import { defaultOptions } from './shared';

export const smsControlZodSchema = z
.object({
skip: z.object({}).catchall(z.unknown()).optional(),
skip: skipZodSchema,
body: z.string(),
})
.strict();

export type SmsControlType = z.infer<typeof smsControlZodSchema>;

export const smsControlSchema = zodToJsonSchema(smsControlZodSchema) as JSONSchemaDto;
export const smsControlSchema = zodToJsonSchema(smsControlZodSchema, defaultOptions) as JSONSchemaDto;
export const smsUiSchema: UiSchema = {
group: UiSchemaGroupEnum.SMS,
properties: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import {
WorkflowStatusEnum,
StepIssues,
ControlSchemas,
DigestUnitEnum,
} from '@novu/shared';
import {
CreateWorkflow as CreateWorkflowGeneric,
Expand All @@ -57,6 +56,7 @@ import { stepTypeToControlSchema } from '../../shared';
import { GetWorkflowCommand, GetWorkflowUseCase } from '../get-workflow';
import { buildVariables } from '../../util/build-variables';
import { BuildAvailableVariableSchemaCommand, BuildAvailableVariableSchemaUsecase } from '../build-variable-schema';
import { sanitizeControlValues } from '../../shared/sanitize-control-values';

@Injectable()
export class UpsertWorkflowUseCase {
Expand Down Expand Up @@ -312,7 +312,8 @@ export class UpsertWorkflowUseCase {
)?.controls;
}

const controlIssues = processControlValuesBySchema(controlSchemas?.schema, controlValueLocal || {});
const sanitizedControlValues = controlValueLocal ? sanitizeControlValues(controlValueLocal, step.type) : {};
const controlIssues = processControlValuesBySchema(controlSchemas?.schema, sanitizedControlValues || {});
const liquidTemplateIssues = processControlValuesByLiquid(variableSchema, controlValueLocal || {});
const customIssues = await this.processCustomControlValues(user, step.type, controlValueLocal || {});
const customControlIssues = _.isEmpty(customIssues) ? {} : { controls: customIssues };
Expand Down Expand Up @@ -475,11 +476,15 @@ function processControlValuesByLiquid(
if (liquidTemplateIssues.invalidVariables.length > 0) {
issues.controls = issues.controls || {};

issues.controls[controlKey] = liquidTemplateIssues.invalidVariables.map((error) => ({
message: `${error.message}, variable: ${error.output}`,
issueType: StepContentIssueEnum.ILLEGAL_VARIABLE_IN_CONTROL_VALUE,
variableName: error.output,
}));
issues.controls[controlKey] = liquidTemplateIssues.invalidVariables.map((error) => {
const message = error.message ? error.message[0].toUpperCase() + error.message.slice(1).split(' line:')[0] : '';

return {
message: `${message} variable: ${error.output}`,
issueType: StepContentIssueEnum.ILLEGAL_VARIABLE_IN_CONTROL_VALUE,
variableName: error.output,
};
});
}
}

Expand Down Expand Up @@ -512,9 +517,8 @@ function processControlValuesBySchema(
if (!acc[path]) {
acc[path] = [];
}

acc[path].push({
message: error.message || 'Invalid value',
message: mapAjvErrorToMessage(error),
issueType: mapAjvErrorToIssueType(error),
variableName: path,
});
Expand All @@ -538,7 +542,16 @@ function processControlValuesBySchema(
* Example: "/foo/bar" becomes "foo.bar"
*/
function getErrorPath(error: ErrorObject): string {
return (error.instancePath.substring(1) || error.params.missingProperty)?.replace(/\//g, '.');
const path = error.instancePath.substring(1);
const { missingProperty } = error.params;

if (!path || path.trim().length === 0) {
return missingProperty;
}

const fullPath = missingProperty ? `${path}/${missingProperty}` : path;

return fullPath?.replace(/\//g, '.');
}

function cleanObject(
Expand All @@ -564,3 +577,19 @@ function mapAjvErrorToIssueType(error: ErrorObject): StepContentIssueEnum {
return StepContentIssueEnum.MISSING_VALUE;
}
}

function mapAjvErrorToMessage(error: ErrorObject<string, Record<string, unknown>, unknown>): string {
if (error.keyword === 'required') {
return `${_.capitalize(error.params.missingProperty)} is required`;
}
if (
error.keyword === 'pattern' &&
error.message?.includes('must match pattern') &&
error.message?.includes('mailto') &&
error.message?.includes('https')
) {
return 'Invalid URL format. Must be a valid absolute URL, path, or contain valid template variables';
}

return error.message || 'Invalid value';
}
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ function extractProps(template: any): { valid: boolean; props: string[]; error?:
* Invalid: {{user.first name}} - postfix length would be 2 due to space
*/
if (initial.postfix.length > 1) {
return { valid: false, props: [], error: 'Novu does not support variables with spaces' };
return { valid: false, props: [], error: 'Variables with spaces are not supported' };
djabarovgeorge marked this conversation as resolved.
Show resolved Hide resolved
}

const validProps: string[] = [];
Expand All @@ -181,7 +181,11 @@ function extractProps(template: any): { valid: boolean; props: string[]; error?:
* Invalid: {{firstName}} - No namespace
*/
if (validProps.length === 1) {
return { valid: false, props: [], error: 'Novu variables must include a namespace (e.g. user.firstName)' };
return {
valid: false,
props: [],
error: `Variables must include a namespace (e.g. payload.${validProps[0]})`,
djabarovgeorge marked this conversation as resolved.
Show resolved Hide resolved
};
}

return { valid: true, props: validProps };
Expand Down
Loading