feat(api): wip refactor issue error messages

novuhq · Dec 23, 2024 · 6fefacb · 6fefacb
1 parent d2f993b
commit 6fefacb
Show file tree

Hide file tree

Showing 11 changed files with 107 additions and 49 deletions.
diff --git a/apps/api/src/app/workflows-v2/shared/sanitize-control-values.ts b/apps/api/src/app/workflows-v2/shared/sanitize-control-values.ts
@@ -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),
   };
 }
@@ -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);

diff --git a/apps/api/src/app/workflows-v2/shared/schemas/chat-control.schema.ts b/apps/api/src/app/workflows-v2/shared/schemas/chat-control.schema.ts
@@ -3,6 +3,7 @@ import { zodToJsonSchema } from 'zod-to-json-schema';
 
 import { JSONSchemaDto, UiComponentEnum, UiSchema, UiSchemaGroupEnum } from '@novu/shared';
 import { skipStepUiSchema, skipZodSchema } from './skip-control.schema';
+import { defaultOptions } from './shared';
 
 export const chatControlZodSchema = z
   .object({
@@ -13,7 +14,7 @@ export const chatControlZodSchema = z
 
 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: {

diff --git a/apps/api/src/app/workflows-v2/shared/schemas/delay-control.schema.ts b/apps/api/src/app/workflows-v2/shared/schemas/delay-control.schema.ts
@@ -9,6 +9,7 @@ import {
   UiSchemaGroupEnum,
 } from '@novu/shared';
 import { skipStepUiSchema, skipZodSchema } from './skip-control.schema';
+import { defaultOptions } from './shared';
 
 export const delayControlZodSchema = z
   .object({
@@ -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: {

diff --git a/apps/api/src/app/workflows-v2/shared/schemas/digest-control.schema.ts b/apps/api/src/app/workflows-v2/shared/schemas/digest-control.schema.ts
@@ -9,6 +9,7 @@ import {
   UiSchemaGroupEnum,
 } from '@novu/shared';
 import { skipStepUiSchema, skipZodSchema } from './skip-control.schema';
+import { defaultOptions } from './shared';
 
 const digestRegularControlZodSchema = z
   .object({
@@ -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);

diff --git a/apps/api/src/app/workflows-v2/shared/schemas/email-control.schema.ts b/apps/api/src/app/workflows-v2/shared/schemas/email-control.schema.ts
@@ -3,6 +3,7 @@ import { z } from 'zod';
 import { zodToJsonSchema } from 'zod-to-json-schema';
 import { TipTapSchema } from '../../../environments-v1/usecases/output-renderers';
 import { skipZodSchema, skipStepUiSchema } from './skip-control.schema';
+import { defaultOptions } from './shared';
 
 export const emailControlZodSchema = z
   .object({
@@ -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: {

diff --git a/apps/api/src/app/workflows-v2/shared/schemas/in-app-control.schema.ts b/apps/api/src/app/workflows-v2/shared/schemas/in-app-control.schema.ts
@@ -2,45 +2,59 @@ import { z } from 'zod';
 import { zodToJsonSchema } from 'zod-to-json-schema';
 import { JSONSchemaDto, UiComponentEnum, UiSchema, UiSchemaGroupEnum } from '@novu/shared';
 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]*)*)$/;
 
 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: skipZodSchema,
-    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: {

diff --git a/apps/api/src/app/workflows-v2/shared/schemas/push-control.schema.ts b/apps/api/src/app/workflows-v2/shared/schemas/push-control.schema.ts
@@ -3,6 +3,7 @@ import { zodToJsonSchema } from 'zod-to-json-schema';
 
 import { JSONSchemaDto, UiComponentEnum, UiSchema, UiSchemaGroupEnum } from '@novu/shared';
 import { skipZodSchema, skipStepUiSchema } from './skip-control.schema';
+import { defaultOptions } from './shared';
 
 export const pushControlZodSchema = z
   .object({
@@ -14,7 +15,7 @@ export const pushControlZodSchema = z
 
 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: {

diff --git a/apps/api/src/app/workflows-v2/shared/schemas/shared.ts b/apps/api/src/app/workflows-v2/shared/schemas/shared.ts
@@ -0,0 +1,5 @@
+import { Targets, Options } from 'zod-to-json-schema';
+
+export const defaultOptions: Partial<Options<Targets>> = {
+  $refStrategy: 'none',
+};
diff --git a/apps/api/src/app/workflows-v2/shared/schemas/sms-control.schema.ts b/apps/api/src/app/workflows-v2/shared/schemas/sms-control.schema.ts
@@ -3,6 +3,7 @@ import { zodToJsonSchema } from 'zod-to-json-schema';
 
 import { JSONSchemaDto, UiComponentEnum, UiSchema, UiSchemaGroupEnum } from '@novu/shared';
 import { skipZodSchema, skipStepUiSchema } from './skip-control.schema';
+import { defaultOptions } from './shared';
 
 export const smsControlZodSchema = z
   .object({
@@ -13,7 +14,7 @@ export const smsControlZodSchema = z
 
 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: {

diff --git a/apps/api/src/app/workflows-v2/usecases/upsert-workflow/upsert-workflow.usecase.ts b/apps/api/src/app/workflows-v2/usecases/upsert-workflow/upsert-workflow.usecase.ts
@@ -30,7 +30,6 @@ import {
   WorkflowStatusEnum,
   StepIssues,
   ControlSchemas,
-  DigestUnitEnum,
 } from '@novu/shared';
 import {
   CreateWorkflow as CreateWorkflowGeneric,
@@ -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 {
@@ -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 };
@@ -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,
+        };
+      });
     }
   }
 
@@ -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,
           });
@@ -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(
@@ -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';
+}
diff --git a/apps/api/src/app/workflows-v2/util/template-parser/liquid-parser.ts b/apps/api/src/app/workflows-v2/util/template-parser/liquid-parser.ts
@@ -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' };
   }
 
   const validProps: string[] = [];
@@ -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]})`,
+    };
   }
 
   return { valid: true, props: validProps };