From a4ec7b3047ab81f2a9f24cf44f31c2c2d14e082c Mon Sep 17 00:00:00 2001 From: Ali BARIN Date: Thu, 22 Aug 2024 16:20:18 +0200 Subject: [PATCH] feat(compute-parameters): add valueType with parse option and string by default (#2025) * feat(compute-parameters): add valueType with parse option and string by default * test(compute-parameters): write tests for valueType with parse and undefined * fix(compute-parameters): cover valueType = parse in nested objects * test(compute-parameters): cover valueType = 'parse' in nested non-primitives * fix(compute-parameters): mark fields optional --- .../apps/code/actions/run-javascript/index.js | 1 + .../backend/src/helpers/compute-parameters.js | 51 ++- .../src/helpers/compute-parameters.test.js | 390 ++++++++++++++---- packages/backend/src/models/step.js | 27 ++ packages/backend/src/services/action.js | 3 + packages/backend/test/factories/app.js | 72 ++++ 6 files changed, 448 insertions(+), 96 deletions(-) create mode 100644 packages/backend/test/factories/app.js diff --git a/packages/backend/src/apps/code/actions/run-javascript/index.js b/packages/backend/src/apps/code/actions/run-javascript/index.js index 3b940998..a90ab757 100644 --- a/packages/backend/src/apps/code/actions/run-javascript/index.js +++ b/packages/backend/src/apps/code/actions/run-javascript/index.js @@ -33,6 +33,7 @@ export default defineAction({ type: 'string', required: true, variables: true, + valueType: 'parse', }, ], }, diff --git a/packages/backend/src/helpers/compute-parameters.js b/packages/backend/src/helpers/compute-parameters.js index 2125e70d..ef22596b 100644 --- a/packages/backend/src/helpers/compute-parameters.js +++ b/packages/backend/src/helpers/compute-parameters.js @@ -6,10 +6,21 @@ function getParameterEntries(parameters) { return Object.entries(parameters); } -function computeParameterEntries(parameterEntries, executionSteps) { +function getFieldByKey(key, fields = []) { + return fields.find((field) => field.key === key); +}; + +function getParameterValueType(parameterKey, fields) { + const field = getFieldByKey(parameterKey, fields); + const defaultValueType = 'string'; + + return field?.valueType || defaultValueType; +} + +function computeParameterEntries(parameterEntries, fields, executionSteps) { const defaultComputedParameters = {}; return parameterEntries.reduce((result, [key, value]) => { - const parameterComputedValue = computeParameter(value, executionSteps); + const parameterComputedValue = computeParameter(key, value, fields, executionSteps); return { ...result, @@ -18,14 +29,21 @@ function computeParameterEntries(parameterEntries, executionSteps) { }, defaultComputedParameters); } -function computeParameter(value, executionSteps) { +function shouldAutoParse(key, fields) { + const parameterValueType = getParameterValueType(key, fields); + const shouldAutoParse = parameterValueType === 'parse'; + + return shouldAutoParse; +} + +function computeParameter(key, value, fields, executionSteps) { if (typeof value === 'string') { - const computedStringParameter = computeStringParameter(value, executionSteps); + const computedStringParameter = computeStringParameter(key, value, fields, executionSteps); return computedStringParameter; } if (Array.isArray(value)) { - const computedArrayParameter = computeArrayParameter(value, executionSteps); + const computedArrayParameter = computeArrayParameter(key, value, fields, executionSteps); return computedArrayParameter; } @@ -107,7 +125,7 @@ function autoParseComputedVariable(computedVariable) { } } -function computeStringParameter(stringValue, executionSteps) { +function computeStringParameter(key, stringValue, fields, executionSteps) { const parts = splitByVariable(stringValue); const computedValue = parts @@ -122,17 +140,26 @@ function computeStringParameter(stringValue, executionSteps) { }) .join(''); - const autoParsedValue = autoParseComputedVariable(computedValue); + const autoParse = shouldAutoParse(key, fields); + if (autoParse) { + const autoParsedValue = autoParseComputedVariable(computedValue); - return autoParsedValue; + return autoParsedValue; + } + + return computedValue; } -function computeArrayParameter(arrayValue, executionSteps) { - return arrayValue.map((item) => computeParameters(item, executionSteps)); +function computeArrayParameter(key, arrayValue, fields = [], executionSteps) { + return arrayValue.map((item) => { + const itemFields = fields.find((field) => field.key === key)?.fields; + + return computeParameters(item, itemFields, executionSteps); + }); } -export default function computeParameters(parameters, executionSteps) { +export default function computeParameters(parameters, fields, executionSteps) { const parameterEntries = getParameterEntries(parameters); - return computeParameterEntries(parameterEntries, executionSteps); + return computeParameterEntries(parameterEntries, fields, executionSteps); } diff --git a/packages/backend/src/helpers/compute-parameters.test.js b/packages/backend/src/helpers/compute-parameters.test.js index 4c6d19af..c45a680a 100644 --- a/packages/backend/src/helpers/compute-parameters.test.js +++ b/packages/backend/src/helpers/compute-parameters.test.js @@ -1,5 +1,6 @@ import { beforeEach, describe, it, expect } from 'vitest'; import { createExecutionStep } from '../../test/factories/execution-step.js'; +import { createDropdownArgument, createDynamicArgument, createStringArgument } from '../../test/factories/app.js'; import computeParameters from './compute-parameters.js'; const computeVariable = (stepId, path) => `{{step.${stepId}.${path}}}`; @@ -68,7 +69,7 @@ describe('Compute parameters helper', () => { key2: `"${computeVariable('non-existent-step-id', 'non-existent-key')}" is the value for non-existent-key`, }; - const computedParameters = computeParameters(parameters, executionSteps); + const computedParameters = computeParameters(parameters, [], executionSteps); const expectedParameters = { key1: '', key2: '"" is the value for non-existent-key', @@ -81,20 +82,29 @@ describe('Compute parameters helper', () => { it('should resolve empty object', () => { const parameters = {}; - const computedParameters = computeParameters(parameters, executionSteps); + const computedParameters = computeParameters(parameters, [], executionSteps); expect(computedParameters).toStrictEqual(parameters); }); }); describe('with string parameters', () => { + let stepArguments; + beforeEach(() => { + stepArguments = [ + createStringArgument({ + key: 'key1', + }), + ]; + }); + it('should resolve as-is without variables', () => { const parameters = { key1: 'plain text', key2: 'plain text', }; - const computedParameters = computeParameters(parameters, executionSteps); + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); expect(computedParameters).toStrictEqual(parameters); }); @@ -106,7 +116,7 @@ describe('Compute parameters helper', () => { key3: ' plain text', }; - const computedParameters = computeParameters(parameters, executionSteps); + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); expect(computedParameters).toStrictEqual(parameters); }); @@ -116,23 +126,291 @@ describe('Compute parameters helper', () => { key1: `static text ${computeVariable(executionStepOne.stepId, 'step1Key1')}`, }; - const computedParameters = computeParameters(parameters, executionSteps); + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); const expectedParameters = { key1: `static text plain text value for step1Key1`, }; expect(computedParameters).toStrictEqual(expectedParameters); }); + + describe('with variables containing JSON', () => { + describe('without explicit valueType defined', () => { + let stepArguments; + beforeEach(() => { + stepArguments = [ + createStringArgument({ + key: 'key1', + }), + ]; + }); + + it('should resolve text + JSON value as-is', () => { + const parameters = { + key1: 'prepended text {"key": "value"} ', + }; + + const computedParameters = computeParameters(parameters, executionSteps); + + expect(computedParameters).toStrictEqual(parameters); + }); + + it('should resolve stringified JSON parsed', () => { + const parameters = { + key1: '{"key1": "plain text", "key2": "119007199254740999"}', + }; + + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); + const expectedParameters = { + key1: '{"key1": "plain text", "key2": "119007199254740999"}', + }; + + expect(computedParameters).toStrictEqual(expectedParameters); + }); + + it('should handle arrays at root level', () => { + const parameters = { + key1: '["value1", "value2"]', + }; + + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); + const expectedParameters = { + key1: '["value1", "value2"]', + }; + + expect(computedParameters).toStrictEqual(expectedParameters); + }); + + it('should handle arrays in nested level', () => { + const parameters = { + key1: '{"items": ["value1", "value2"]}', + }; + + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); + const expectedParameters = { + key1: '{"items": ["value1", "value2"]}', + }; + + expect(computedParameters).toStrictEqual(expectedParameters); + }); + + it('should compute mix variables correctly', () => { + const parameters = { + key1: `another static text ${computeVariable(executionStepThree.stepId, 'step3Key4.step3Key4ChildKey4')}`, + }; + + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); + const expectedParameters = { + key1: `another static text ["value1","value2"]`, + }; + + expect(computedParameters).toStrictEqual(expectedParameters); + }); + + it('should compute variables correctly', () => { + const parameters = { + key1: `${computeVariable(executionStepThree.stepId, 'step3Key4.step3Key4ChildKey4')}`, + }; + + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); + const expectedParameters = { + key1: '["value1","value2"]', + }; + + expect(computedParameters).toStrictEqual(expectedParameters); + }); + + it('should not parse non-primitives in nested arrays', () => { + const stepArguments = [ + createDynamicArgument({ + key: 'inputs', + }) + ]; + + const parameters = { + inputs: [ + { + key: 'person', + value: '{ "name": "John Doe", "age": 32 }', + } + ], + }; + + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); + const expectedParameters = { + inputs: [ + { + key: 'person', + value: '{ "name": "John Doe", "age": 32 }', + } + ], + }; + + expect(computedParameters).toStrictEqual(expectedParameters); + }); + }); + + describe(`with valueType as 'parse'`, () => { + let stepArguments; + beforeEach(() => { + stepArguments = [ + createStringArgument({ + key: 'key1', + valueType: 'parse', + }), + ]; + }); + + it('should resolve text + JSON value as-is', () => { + const parameters = { + key1: 'prepended text {"key": "value"} ', + }; + + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); + + expect(computedParameters).toStrictEqual(parameters); + }); + + it('should resolve stringified JSON parsed', () => { + const parameters = { + key1: '{"key1": "plain text", "key2": "119007199254740999"}', + }; + + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); + const expectedParameters = { + key1: { + key1: 'plain text', + key2: '119007199254740999', + }, + }; + + expect(computedParameters).toStrictEqual(expectedParameters); + }); + + it('should handle arrays at root level', () => { + const parameters = { + key1: '["value1", "value2"]', + }; + + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); + const expectedParameters = { + key1: ['value1', 'value2'], + }; + + expect(computedParameters).toStrictEqual(expectedParameters); + }); + + it('should handle arrays in nested level', () => { + const parameters = { + key1: '{"items": ["value1", "value2"]}', + }; + + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); + const expectedParameters = { + key1: { + items: ['value1', 'value2'], + } + }; + + expect(computedParameters).toStrictEqual(expectedParameters); + }); + + it('should compute and parse mix variables correctly', () => { + const parameters = { + key1: `another static text ${computeVariable(executionStepThree.stepId, 'step3Key4.step3Key4ChildKey4')}`, + }; + + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); + const expectedParameters = { + key1: `another static text ["value1","value2"]`, + }; + + expect(computedParameters).toStrictEqual(expectedParameters); + }); + + it('should compute and parse variables correctly', () => { + const parameters = { + key1: `${computeVariable(executionStepThree.stepId, 'step3Key4.step3Key4ChildKey4')}`, + }; + + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); + const expectedParameters = { + key1: ["value1", "value2"], + }; + + expect(computedParameters).toStrictEqual(expectedParameters); + }); + + it('should compute and parse variables in nested arrays correctly', () => { + const stepArguments = [ + createDynamicArgument({ + key: 'inputs', + fields: [ + { + label: 'Key', + key: 'key', + required: true, + variables: true, + }, + { + label: 'Value', + key: 'value', + required: true, + variables: true, + valueType: 'parse', + } + ], + }) + ]; + + const parameters = { + inputs: [ + { + key: 'person', + value: '{ "name": "John Doe", "age": 32 }', + } + ], + }; + + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); + const expectedParameters = { + inputs: [ + { + key: 'person', + value: { + name: 'John Doe', + age: 32, + } + } + ], + }; + + expect(computedParameters).toStrictEqual(expectedParameters); + }); + }); + }); }); describe('with number parameters', () => { + let stepArguments; + beforeEach(() => { + stepArguments = [ + createStringArgument({ + key: 'key1', + }), + createStringArgument({ + key: 'key2', + }), + ]; + }); + it('should resolve number larger than MAX_SAFE_INTEGER correctly', () => { const parameters = { // eslint-disable-next-line no-loss-of-precision key1: 119007199254740999, }; - const computedParameters = computeParameters(parameters, executionSteps); + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); expect(computedParameters).toStrictEqual(parameters); expect(computedParameters.key1 > Number.MAX_SAFE_INTEGER).toBe(true); @@ -143,10 +421,9 @@ describe('Compute parameters helper', () => { key1: '119007199254740999', }; - const computedParameters = computeParameters(parameters, executionSteps); + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); expect(computedParameters).toStrictEqual(parameters); - expect(parseInt(parameters.key1)) }); it('should compute variables with int values correctly', () => { @@ -155,7 +432,7 @@ describe('Compute parameters helper', () => { key2: `${computeVariable(executionStepThree.stepId, 'step3Key3')}` }; - const computedParameters = computeParameters(parameters, executionSteps); + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); const expectedParameters = { key1: `another static text 123123`, key2: `123123` @@ -170,7 +447,7 @@ describe('Compute parameters helper', () => { key2: `${computeVariable(executionStepTwo.stepId, 'step2Key3')}` }; - const computedParameters = computeParameters(parameters, executionSteps); + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); const expectedParameters = { // The expected `key2` is computed wrongly. key1: `another static text 6502380617126796383`, @@ -181,85 +458,30 @@ describe('Compute parameters helper', () => { }); }); - describe('with JSON parameters', () => { - it('should resolve text + JSON value as-is', () => { + describe('with boolean parameters', () => { + let stepArguments; + beforeEach(() => { + stepArguments = [ + createDropdownArgument({ + key: 'key1', + }), + createDropdownArgument({ + key: 'key2', + }), + ]; + }); + + it('should resolve boolean as-is', () => { const parameters = { - key1: 'prepended text {"key": "value"} ', + key1: true, + key2: false, }; - const computedParameters = computeParameters(parameters, executionSteps); + const computedParameters = computeParameters(parameters, stepArguments, executionSteps); expect(computedParameters).toStrictEqual(parameters); - }); - - it('should resolve stringified JSON parsed', () => { - const parameters = { - key1: '{"key1": "plain text", "key2": "119007199254740999"}', - }; - - const computedParameters = computeParameters(parameters, executionSteps); - const expectedParameters = { - key1: { - key1: 'plain text', - key2: '119007199254740999', - }, - }; - - expect(computedParameters).toStrictEqual(expectedParameters); - }); - - it('should handle arrays at root level', () => { - const parameters = { - key1: '["value1", "value2"]', - }; - - const computedParameters = computeParameters(parameters, executionSteps); - const expectedParameters = { - key1: ['value1', 'value2'], - }; - - expect(computedParameters).toStrictEqual(expectedParameters); - }); - - it('should handle arrays in nested level', () => { - const parameters = { - key1: '{"items": ["value1", "value2"]}', - }; - - const computedParameters = computeParameters(parameters, executionSteps); - const expectedParameters = { - key1: { - items: ['value1', 'value2'], - } - }; - - expect(computedParameters).toStrictEqual(expectedParameters); - }); - - it('should compute mix variables correctly', () => { - const parameters = { - key1: `another static text ${computeVariable(executionStepThree.stepId, 'step3Key4.step3Key4ChildKey4')}`, - }; - - const computedParameters = computeParameters(parameters, executionSteps); - const expectedParameters = { - key1: `another static text ["value1","value2"]`, - }; - - expect(computedParameters).toStrictEqual(expectedParameters); - }); - - it('should compute variables correctly', () => { - const parameters = { - key1: `${computeVariable(executionStepThree.stepId, 'step3Key4.step3Key4ChildKey4')}`, - }; - - const computedParameters = computeParameters(parameters, executionSteps); - const expectedParameters = { - key1: ["value1", "value2"], - }; - - expect(computedParameters).toStrictEqual(expectedParameters); + expect(computedParameters.key1).toBe(true); + expect(computedParameters.key2).toBe(false); }); }); }); diff --git a/packages/backend/src/models/step.js b/packages/backend/src/models/step.js index 59ddb835..d2d9ce05 100644 --- a/packages/backend/src/models/step.js +++ b/packages/backend/src/models/step.js @@ -198,6 +198,30 @@ class Step extends Base { return existingArguments; } + async getSetupAndDynamicFields() { + const setupFields = await this.getSetupFields(); + const setupAndDynamicFields = []; + + for (const setupField of setupFields) { + setupAndDynamicFields.push(setupField); + + const additionalFields = setupField.additionalFields; + if (additionalFields) { + const keyArgument = additionalFields.arguments.find(argument => argument.name === 'key'); + const dynamicFieldsKey = keyArgument.value; + + const dynamicFields = await this.createDynamicFields( + dynamicFieldsKey, + this.parameters + ); + + setupAndDynamicFields.push(...dynamicFields); + } + } + + return setupAndDynamicFields; + } + async createDynamicFields(dynamicFieldsKey, parameters) { const connection = await this.$relatedQuery('connection'); const flow = await this.$relatedQuery('flow'); @@ -240,8 +264,11 @@ class Step extends Base { }) : []; + const setupAndDynamicFields = await this.getSetupAndDynamicFields(); + const computedParameters = computeParameters( $.step.parameters, + setupAndDynamicFields, priorExecutionSteps ); diff --git a/packages/backend/src/services/action.js b/packages/backend/src/services/action.js index 4bc5fac1..fdf70e12 100644 --- a/packages/backend/src/services/action.js +++ b/packages/backend/src/services/action.js @@ -31,8 +31,11 @@ export const processAction = async (options) => { execution_id: $.execution.id, }); + const stepSetupAndDynamicFields = await step.getSetupAndDynamicFields(); + const computedParameters = computeParameters( $.step.parameters, + stepSetupAndDynamicFields, priorExecutionSteps ); diff --git a/packages/backend/test/factories/app.js b/packages/backend/test/factories/app.js new file mode 100644 index 00000000..2869c430 --- /dev/null +++ b/packages/backend/test/factories/app.js @@ -0,0 +1,72 @@ +import { faker } from '@faker-js/faker'; + +export const createArgument = (params = {}) => { + const labelAndKey = faker.lorem.word(); + + const argument = { + label: labelAndKey, + key: labelAndKey, + required: false, + variables: true, + ...params, + }; + + return argument; +}; + +export const createStringArgument = (params = {}) => { + const stringArgument = createArgument({ + ...params, + type: 'string', + }); + + return stringArgument; +}; + +export const createDropdownArgument = (params = {}) => { + const dropdownArgument = createArgument({ + options: [ + { + label: 'Yes', + value: true, + }, + { + label: 'No', + value: false, + }, + ], + ...params, + type: 'dropdown', + }); + + return dropdownArgument; +}; + +export const createDynamicArgument = (params = {}) => { + const dynamicArgument = createArgument({ + value: [ + { + key: '', + value: '', + }, + ], + fields: [ + { + label: 'Key', + key: 'key', + required: true, + variables: true, + }, + { + label: 'Value', + key: 'value', + required: true, + variables: true, + } + ], + ...params, + type: 'dynamic', + }); + + return dynamicArgument; +};