feat(api): update after pr comments

novuhq · Dec 6, 2024 · 2214a0c · 2214a0c
1 parent 7c0f81e
commit 2214a0c
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 67 deletions.
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
@@ -28,34 +28,39 @@ export class BuildPayloadSchema {
     const variablesExample = pathsToObject(templateVars, {
       valuePrefix: '{{',
       valueSuffix: '}}',
-    }).payload as Record<string, unknown>;
+    }).payload;
 
     return convertJsonToSchemaWithDefaults(variablesExample);
   }
 
   private async buildControlValues(command: BuildPayloadSchemaCommand) {
-    let aggregateControlValues = command.controlValues ? [command.controlValues] : [];
-
-    if (!aggregateControlValues.length) {
-      aggregateControlValues = (
-        await this.controlValuesRepository.find({
-          _environmentId: command.environmentId,
-          _organizationId: command.organizationId,
-          _workflowId: command.workflowId,
-          level: ControlValuesLevelEnum.STEP_CONTROLS,
-        })
-      )
-        .map((item) => item.controls)
-        .filter((control): control is NonNullable<typeof control> => control != null);
+    let controlValues = command.controlValues ? [command.controlValues] : [];
+
+    if (!controlValues.length) {
+      controlValues = (
+        await this.controlValuesRepository.find(
+          {
+            _environmentId: command.environmentId,
+            _organizationId: command.organizationId,
+            _workflowId: command.workflowId,
+            level: ControlValuesLevelEnum.STEP_CONTROLS,
+            controls: { $ne: null },
+          },
+          {
+            controls: 1,
+            _id: 0,
+          }
+        )
+      ).map((item) => item.controls);
     }
 
-    return aggregateControlValues;
+    return controlValues;
   }
 
   @Instrument()
-  private extractTemplateVariables(aggregateControlValues: Record<string, unknown>[]): string[] {
-    const concatenatedControlValues = aggregateControlValues.map(flattenObjectValues).flat().join(' ');
+  private extractTemplateVariables(controlValues: Record<string, unknown>[]): string[] {
+    const controlValuesString = controlValues.map(flattenObjectValues).flat().join(' ');
 
-    return extractLiquidTemplateVariables(concatenatedControlValues).validVariables;
+    return extractLiquidTemplateVariables(controlValuesString).validVariables.map((variable) => variable.name);
   }
 }
diff --git a/...pp/workflows-v2/usecases/build-variable-schema/build-available-variable-schema.usecase.ts b/...pp/workflows-v2/usecases/build-variable-schema/build-available-variable-schema.usecase.ts
@@ -56,12 +56,7 @@ export class BuildAvailableVariableSchemaUsecase {
     command: BuildAvailableVariableSchemaCommand
   ): Promise<JSONSchemaDto> {
     if (workflow.payloadSchema) {
-      return (
-        parsePayloadSchema(workflow.payloadSchema, { safe: true }) || {
-          type: 'object',
-          description: 'Payload for the current step',
-        }
-      );
+      return parsePayloadSchema(workflow.payloadSchema, { safe: true }) || {};
     }
 
     return this.buildPayloadSchema.execute(

diff --git a/apps/api/src/app/workflows-v2/util/path-to-object.ts b/apps/api/src/app/workflows-v2/util/path-to-object.ts
@@ -12,8 +12,11 @@ import _ from 'lodash';
  * // Returns: { payload: { old: 'payload.old', new: 'payload.new' } }
  * // Note: 'payload' entry is ignored as it has no namespace
  */
-export function pathsToObject(keys: string[], { valuePrefix = '', valueSuffix = '' } = {}): Record<string, unknown> {
-  const result: Record<string, unknown> = {};
+export function pathsToObject(
+  keys: string[],
+  { valuePrefix = '', valueSuffix = '' } = {}
+): Record<string, Record<string, unknown>> {
+  const result: Record<string, Record<string, unknown>> = {};
 
   keys
     .filter((key) => key.includes('.'))

diff --git a/apps/api/src/app/workflows-v2/util/template-parser/liquid-parser.spec.ts b/apps/api/src/app/workflows-v2/util/template-parser/liquid-parser.spec.ts
@@ -4,58 +4,64 @@ import { extractLiquidTemplateVariables } from './liquid-parser';
 describe('parseLiquidVariables', () => {
   it('should extract simple variable names', () => {
     const template = '{{name}} {{age}}';
-    const { validVariables: variables } = extractLiquidTemplateVariables(template);
+    const { validVariables } = extractLiquidTemplateVariables(template);
+    const validVariablesNames = validVariables.map((variable) => variable.name);
 
-    expect(variables).to.have.members(['name', 'age']);
+    expect(validVariablesNames).to.have.members(['name', 'age']);
   });
 
   it('should extract nested object paths', () => {
     const template = 'Hello {{user.profile.name}}, your address is {{user.address.street}}';
-    const { validVariables: variables } = extractLiquidTemplateVariables(template);
+    const { validVariables } = extractLiquidTemplateVariables(template);
+    const validVariablesNames = validVariables.map((variable) => variable.name);
 
-    expect(variables).to.have.members(['user.profile.name', 'user.address.street']);
+    expect(validVariablesNames).to.have.members(['user.profile.name', 'user.address.street']);
   });
 
   it('should handle multiple occurrences of the same variable', () => {
     const template = '{{user.name}} {{user.name}} {{user.name}}';
-    const { validVariables: variables } = extractLiquidTemplateVariables(template);
+    const { validVariables } = extractLiquidTemplateVariables(template);
+    const validVariablesNames = validVariables.map((variable) => variable.name);
 
-    expect(variables).to.have.members(['user.name']);
+    expect(validVariablesNames).to.have.members(['user.name']);
   });
 
   it('should handle mixed content with HTML and variables', () => {
     const template = '<div>Hello {{user.name}}</div><span>{{status}}</span>';
-    const { validVariables: variables } = extractLiquidTemplateVariables(template);
+    const { validVariables } = extractLiquidTemplateVariables(template);
+    const validVariablesNames = validVariables.map((variable) => variable.name);
 
-    expect(variables).to.have.members(['user.name', 'status']);
+    expect(validVariablesNames).to.have.members(['user.name', 'status']);
   });
 
   it('should handle whitespace in template syntax', () => {
     const template = '{{ user.name }} {{  status  }}';
-    const { validVariables: variables } = extractLiquidTemplateVariables(template);
+    const { validVariables } = extractLiquidTemplateVariables(template);
+    const validVariablesNames = validVariables.map((variable) => variable.name);
 
-    expect(variables).to.have.members(['user.name', 'status']);
+    expect(validVariablesNames).to.have.members(['user.name', 'status']);
   });
 
   it('should handle empty template string', () => {
     const template = '';
-    const { validVariables: variables } = extractLiquidTemplateVariables(template);
+    const { validVariables } = extractLiquidTemplateVariables(template);
 
-    expect(variables.length).to.equal(0);
+    expect(validVariables).to.have.lengthOf(0);
   });
 
   it('should handle template with no variables', () => {
     const template = 'Hello World!';
-    const { validVariables: variables } = extractLiquidTemplateVariables(template);
+    const { validVariables } = extractLiquidTemplateVariables(template);
 
-    expect(variables).to.have.lengthOf(0);
+    expect(validVariables).to.have.lengthOf(0);
   });
 
   it('should handle special characters in variable names', () => {
     const template = '{{special_var_1}} {{data-point}}';
-    const { validVariables: variables } = extractLiquidTemplateVariables(template);
+    const { validVariables } = extractLiquidTemplateVariables(template);
+    const validVariablesNames = validVariables.map((variable) => variable.name);
 
-    expect(variables).to.have.members(['special_var_1', 'data-point']);
+    expect(validVariablesNames).to.have.members(['special_var_1', 'data-point']);
   });
 
   describe('Error handling', () => {
@@ -67,31 +73,35 @@ describe('parseLiquidVariables', () => {
       expect(variables).to.have.lengthOf(0);
       expect(errors).to.have.lengthOf(2);
       expect(errors[0].message).to.contain('expected "|" before filter');
-      expect(errors[0].variable).to.equal('{{invalid..syntax}}');
-      expect(errors[1].variable).to.equal('{{invalid2..syntax}}');
+      expect(errors[0].name).to.equal('{{invalid..syntax}}');
+      expect(errors[1].name).to.equal('{{invalid2..syntax}}');
     });
 
     it('should handle invalid liquid syntax gracefully, return valid variables', () => {
       const { validVariables, invalidVariables: errors } = extractLiquidTemplateVariables(
         '{{subscriber.name}} {{invalid..syntax}}'
       );
+      const validVariablesNames = validVariables.map((variable) => variable.name);
 
-      expect(validVariables).to.have.members(['subscriber.name']);
+      expect(validVariablesNames).to.have.members(['subscriber.name']);
       expect(errors[0].message).to.contain('expected "|" before filter');
-      expect(errors[0].variable).to.equal('{{invalid..syntax}}');
+      expect(errors[0].name).to.equal('{{invalid..syntax}}');
     });
 
     it('should handle undefined input gracefully', () => {
       expect(() => extractLiquidTemplateVariables(undefined as any)).to.not.throw();
-      expect(extractLiquidTemplateVariables(undefined as any)).to.deep.equal({
-        validVariables: [],
-        invalidVariables: [],
-      });
+      const { validVariables } = extractLiquidTemplateVariables(undefined as any);
+      const validVariablesNames = validVariables.map((variable) => variable.name);
+
+      expect(validVariablesNames).to.have.lengthOf(0);
     });
 
     it('should handle non-string input gracefully', () => {
       expect(() => extractLiquidTemplateVariables({} as any)).to.not.throw();
-      expect(extractLiquidTemplateVariables({} as any)).to.deep.equal({ validVariables: [], invalidVariables: [] });
+      const { validVariables } = extractLiquidTemplateVariables({} as any);
+      const validVariablesNames = validVariables.map((variable) => variable.name);
+
+      expect(validVariablesNames).to.have.lengthOf(0);
     });
   });
 });
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
@@ -1,5 +1,4 @@
 import { Template, Liquid, RenderError, LiquidError } from 'liquidjs';
-import { TemplateParseResult, InvalidVariable } from './parser-types';
 import { isValidTemplate, extractLiquidExpressions } from './parser-utils';
 
 const LIQUID_CONFIG = {
@@ -9,6 +8,17 @@ const LIQUID_CONFIG = {
   catchAllErrors: true,
 } as const;
 
+export type Variable = {
+  context?: string;
+  message?: string;
+  name: string;
+};
+
+export type TemplateParseResult = {
+  validVariables: Variable[];
+  invalidVariables: Variable[];
+};
+
 /**
  * Copy of LiquidErrors type from liquidjs since it's not exported.
  * Used to handle multiple render errors that can occur during template parsing.
@@ -65,28 +75,28 @@ export function extractLiquidTemplateVariables(template: string): TemplateParseR
 }
 
 function processLiquidRawOutput(rawOutputs: string[]): TemplateParseResult {
-  const variables = new Set<string>();
-  const invalidVariables: InvalidVariable[] = [];
+  const validVariables = new Set<string>();
+  const invalidVariables: Variable[] = [];
 
   for (const rawOutput of rawOutputs) {
     try {
       const parsedVars = parseByLiquid(rawOutput);
-      parsedVars.forEach((variable) => variables.add(variable));
+      parsedVars.forEach((variable) => validVariables.add(variable));
     } catch (error: unknown) {
       if (isLiquidErrors(error)) {
         invalidVariables.push(
           ...error.errors.map((e: RenderError) => ({
             context: e.context,
             message: e.message,
-            variable: rawOutput,
+            name: rawOutput,
           }))
         );
       }
     }
   }
 
   return {
-    validVariables: Array.from(variables),
+    validVariables: [...validVariables].map((name) => ({ name })),
     invalidVariables,
   };
 }

diff --git a/apps/api/src/app/workflows-v2/util/template-parser/parser-types.ts b/apps/api/src/app/workflows-v2/util/template-parser/parser-types.ts