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(Execute Workflow Node): Move Type Conversion functionality to ResourceMapper #12268

Merged
merged 15 commits into from
Dec 19, 2024
1 change: 1 addition & 0 deletions cypress/e2e/48-subworkflow-inputs.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ function validateAndReturnToParent(targetChild: string, offset: number, fields:
// Due to our workaround to remain in the same tab we need to select the correct tab manually
navigateWorkflowSelectionDropdown(offset, targetChild);

// This fails, pointing to `usePushConnection` `const triggerNode = subWorkflow?.nodes.find` being `undefined.find()`I <think>
CharlieKolb marked this conversation as resolved.
Show resolved Hide resolved
ndv.actions.execute();

getOutputTableHeaders().should('have.length', fields.length + 1);
Expand Down
18 changes: 14 additions & 4 deletions cypress/fixtures/Test_Subworkflow-Inputs.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,23 @@
},
{
"parameters": {
"workflowId": {},
"workflowInputs": {
"mappingMode": "defineBelow",
"value": {},
"matchingColumns": [],
"schema": [],
"ignoreTypeMismatchErrors": false,
"attemptToConvertTypes": false,
"convertFieldsToString": true
},
"options": {}
},
"id": "ddc82976-edd9-4488-a5a5-7f558a7d905b",
"name": "Execute Workflow",
"type": "n8n-nodes-base.executeWorkflow",
"typeVersion": 1.1,
"position": [500, 240]
"typeVersion": 1.2,
"position": [500, 240],
"id": "6b6e2e34-c6ab-4083-b8e3-6b0d56be5453",
"name": "Execute Workflow"
}
],
"connections": {
Expand Down
13 changes: 8 additions & 5 deletions packages/core/src/node-execution-context/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ const validateResourceMapperValue = (
for (let i = 0; i < paramValueNames.length; i++) {
const key = paramValueNames[i];
const resolvedValue = paramValues[key];
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call
const schemaEntry = schema.find((s) => s.id === key);

if (
Expand All @@ -99,15 +98,19 @@ const validateResourceMapperValue = (
};
}

// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
if (schemaEntry?.type) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
const validationResult = validateFieldType(key, resolvedValue, schemaEntry.type, {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
valueOptions: schemaEntry.options,
strict: !resourceMapperField.attemptToConvertTypes,
parseStrings: !!resourceMapperField.convertFieldsToString,
});

if (!validationResult.valid) {
return { ...validationResult, fieldName: key };
if (!resourceMapperField.ignoreTypeMismatchErrors) {
return { ...validationResult, fieldName: key };
} else {
paramValues[key] = resolvedValue;
}
} else {
// If it's valid, set the casted value
paramValues[key] = validationResult.newValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
INodeParameters,
INodeProperties,
INodeTypeDescription,
NodeParameterValueType,
ResourceMapperField,
ResourceMapperValue,
} from 'n8n-workflow';
Expand Down Expand Up @@ -52,6 +53,12 @@ const state = reactive({
value: {},
matchingColumns: [] as string[],
schema: [] as ResourceMapperField[],
ignoreTypeMismatchErrors: false,
attemptToConvertTypes: false,
// This should always be true if `showTypeConversionOptions` is provided
// It's used to avoid accepting any value as string without casting it
// Which is the legacy behavior without these type options.
convertFieldsToString: false,
} as ResourceMapperValue,
parameterValues: {} as INodeParameters,
loading: false,
Expand Down Expand Up @@ -97,6 +104,10 @@ onMounted(async () => {
...state.parameterValues,
parameters: props.node.parameters,
};

if (showTypeConversionOptions.value) {
state.paramValue.convertFieldsToString = true;
}
}
const params = state.parameterValues.parameters as INodeParameters;
const parameterName = props.parameter.name;
Expand Down Expand Up @@ -161,6 +172,10 @@ const showMappingModeSelect = computed<boolean>(() => {
return props.parameter.typeOptions?.resourceMapper?.supportAutoMap !== false;
});

const showTypeConversionOptions = computed<boolean>(() => {
return props.parameter.typeOptions?.resourceMapper?.showTypeConversionOptions === true;
});

const showMatchingColumnsSelector = computed<boolean>(() => {
return (
!state.loading &&
Expand Down Expand Up @@ -568,5 +583,50 @@ defineExpose({
})
}}
</N8nNotice>
<div
v-if="showTypeConversionOptions && state.paramValue.schema.length > 0"
:class="$style.typeConversionOptions"
>
<ParameterInputFull
:parameter="{
name: 'attemptToConvertTypes',
type: 'boolean',
displayName: 'Attempt to convert types',
default: false,
description: 'Attempt to convert types when mapping fields',
CharlieKolb marked this conversation as resolved.
Show resolved Hide resolved
}"
:path="props.path + '.attemptToConvertTypes'"
:value="state.paramValue.attemptToConvertTypes"
@update="
(x: IUpdateInformation<NodeParameterValueType>) => {
state.paramValue.attemptToConvertTypes = x.value as boolean;
emitValueChanged();
}
"
/>
<ParameterInputFull
:parameter="{
name: 'ignoreTypeMismatchErrors',
type: 'boolean',
displayName: 'Ignore Type Mismatch Errors',
default: false,
description: 'Whether type mismatches should be ignored, rather than returning an Error',
}"
:path="props.path + '.ignoreTypeMismatchErrors'"
:value="state.paramValue.ignoreTypeMismatchErrors"
@update="
(x: IUpdateInformation<NodeParameterValueType>) => {
state.paramValue.ignoreTypeMismatchErrors = x.value as boolean;
emitValueChanged();
}
"
/>
</div>
</div>
</template>

<style module lang="scss">
.typeConversionOptions {
CharlieKolb marked this conversation as resolved.
Show resolved Hide resolved
margin-top: var(--spacing-m);
}
</style>
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import _ from 'lodash';
import { NodeConnectionType, NodeOperationError } from 'n8n-workflow';
import type {
ExecuteWorkflowData,
FieldValueOption,
IDataObject,
IExecuteFunctions,
INodeExecutionData,
Expand All @@ -13,9 +13,8 @@ import type {
import { getWorkflowInfo } from './GenericFunctions';
import { loadWorkflowInputMappings } from './methods/resourceMapping';
import { generatePairedItemData } from '../../../utils/utilities';
import { getWorkflowInputData } from '../GenericFunctions';

function getWorkflowInputValues(this: IExecuteFunctions) {
function getWorkflowInputValues(this: IExecuteFunctions): INodeExecutionData[] {
const inputData = this.getInputData();

return inputData.map((item, itemIndex) => {
Expand All @@ -39,18 +38,22 @@ function getWorkflowInputValues(this: IExecuteFunctions) {
}

function getCurrentWorkflowInputData(this: IExecuteFunctions) {
const inputData = getWorkflowInputValues.call(this);
const inputData: INodeExecutionData[] = getWorkflowInputValues.call(this);

const schema = this.getNodeParameter('workflowInputs.schema', 0, []) as ResourceMapperField[];

if (schema.length === 0) {
return inputData;
} else {
const newParams = schema
.filter((x) => !x.removed)
.map((x) => ({ name: x.displayName, type: x.type ?? 'any' })) as FieldValueOption[];
const removedKeys = new Set(schema.filter((x) => x.removed).map((x) => x.displayName));

return getWorkflowInputData.call(this, inputData, newParams);
const filteredInputData: INodeExecutionData[] = inputData.map((item, index) => ({
index,
pairedItem: { item: index },
json: _.pickBy(item.json, (_v, key) => !removedKeys.has(key)),
}));

return filteredInputData;
}
}

Expand Down Expand Up @@ -84,6 +87,13 @@ export class ExecuteWorkflow implements INodeType {
},
],
},
{
displayName: 'This node is out of date. Please upgrade by removing it and adding a new one',
name: 'outdatedVersionWarning',
type: 'notice',
displayOptions: { show: { '@version': [{ _cnd: { lte: 1.1 } }] } },
default: '',
},
{
displayName: 'Source',
name: 'source',
Expand Down Expand Up @@ -254,6 +264,7 @@ export class ExecuteWorkflow implements INodeType {
addAllFields: true,
multiKeyMatch: false,
supportAutoMap: false,
showTypeConversionOptions: true,
},
},
displayOptions: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { mock } from 'jest-mock-extended';
import type { FieldValueOption, IExecuteFunctions, INode, INodeExecutionData } from 'n8n-workflow';

import { ExecuteWorkflowTrigger } from './ExecuteWorkflowTrigger.node';
import { getFieldEntries, getWorkflowInputData } from '../GenericFunctions';
import { getFieldEntries } from '../GenericFunctions';

jest.mock('../GenericFunctions');

Expand Down Expand Up @@ -32,14 +32,10 @@ describe('ExecuteWorkflowTrigger', () => {
{ name: 'value2', type: 'number' },
] as FieldValueOption[];
const getFieldEntriesMock = (getFieldEntries as jest.Mock).mockReturnValue(mockNewParams);
const getWorkflowInputDataMock = (getWorkflowInputData as jest.Mock).mockReturnValue(
mockInputData,
);

const result = await new ExecuteWorkflowTrigger().execute.call(executeFns);

expect(result).toEqual([mockInputData]);
expect(getFieldEntriesMock).toHaveBeenCalledWith(executeFns);
expect(getWorkflowInputDataMock).toHaveBeenCalledWith(mockInputData, mockNewParams);
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import _ from 'lodash';
import {
type INodeExecutionData,
NodeConnectionType,
type IExecuteFunctions,
type INodeType,
Expand All @@ -10,11 +12,11 @@ import {
WORKFLOW_INPUTS,
JSON_EXAMPLE,
VALUES,
INPUT_OPTIONS,
TYPE_OPTIONS,
PASSTHROUGH,
FALLBACK_DEFAULT_VALUE,
} from '../constants';
import { getFieldEntries, getWorkflowInputData } from '../GenericFunctions';
import { getFieldEntries } from '../GenericFunctions';

export class ExecuteWorkflowTrigger implements INodeType {
description: INodeTypeDescription = {
Expand Down Expand Up @@ -63,6 +65,23 @@ export class ExecuteWorkflowTrigger implements INodeType {
],
default: 'worklfow_call',
},
{
displayName:
"When an ‘execute workflow’ node calls this workflow, the execution starts here. Any data passed into the 'execute workflow' node will be output by this node.",
name: 'notice',
type: 'notice',
default: '',
displayOptions: {
show: { '@version': [{ _cnd: { eq: 1 } }] },
},
},
{
displayName: 'This node is out of date. Please upgrade by removing it and adding a new one',
name: 'outdatedVersionWarning',
type: 'notice',
displayOptions: { show: { '@version': [{ _cnd: { eq: 1 } }] } },
default: '',
},
{
displayName: 'Input Source',
name: INPUT_SOURCE,
Expand Down Expand Up @@ -163,52 +182,40 @@ export class ExecuteWorkflowTrigger implements INodeType {
},
],
},
{
displayName: 'Input Options',
name: INPUT_OPTIONS,
placeholder: 'Options',
type: 'collection',
description: 'Options controlling how input data is handled, converted and rejected',
displayOptions: {
show: { '@version': [{ _cnd: { gte: 1.1 } }] },
},
default: {},
// Note that, while the defaults are true, the user has to add these in the first place
// We default to false if absent in the execute function below
options: [
{
displayName: 'Attempt to Convert Types',
name: 'attemptToConvertTypes',
type: 'boolean',
default: true,
description:
'Whether to attempt conversion on type mismatch, rather than directly returning an Error',
noDataExpression: true,
},
{
displayName: 'Ignore Type Mismatch Errors',
name: 'ignoreTypeErrors',
type: 'boolean',
default: true,
description:
'Whether type mismatches should be ignored, rather than returning an Error',
noDataExpression: true,
},
],
},
],
};

async execute(this: IExecuteFunctions) {
const inputData = this.getInputData();
const inputSource = this.getNodeParameter(INPUT_SOURCE, 0, PASSTHROUGH) as string;

// Note on the data we receive from ExecuteWorkflow caller:
//
// The ExecuteWorkflow node typechecks all fields explicitly provided by the user here via the resourceMapper
// and removes all fields that are in the schema, but `removed` in the resourceMapper.
//
// In passthrough and legacy node versions, inputData will line up since the resourceMapper is empty,
// in which case all input is passed through.
// In other cases we will already have matching types and fields provided by the resource mapper,
// so we just need to be permissive on this end,
// while ensuring we provide default values for fields in our schema, which are removed in the resourceMapper.

if (inputSource === PASSTHROUGH) {
return [inputData];
} else {
const newParams = getFieldEntries(this);
const newKeys = new Set(newParams.map((x) => x.name));
const itemsInSchema: INodeExecutionData[] = inputData.map((row, index) => ({
json: {
...Object.fromEntries(newParams.map((x) => [x.name, FALLBACK_DEFAULT_VALUE])),
// Need to trim to the expected schema to support legacy Execute Workflow callers passing through all their data
// which we do not want to expose past this node.
..._.pickBy(row.json, (_value, key) => newKeys.has(key)),
},
index,
}));

return [getWorkflowInputData.call(this, inputData, newParams)];
return [itemsInSchema];
}
}
}
Loading
Loading