fix: Load remote resources even if expressions in non requried parameters resolve (#6987)

Github issue / Community forum post (link here to close automatically):
This commit is contained in:
Mutasem Aldmour
2023-08-31 16:40:20 +02:00
committed by GitHub
parent 8cd4db0ab7
commit 8a8d4e8bb3
18 changed files with 478 additions and 25 deletions

View File

@@ -31,6 +31,7 @@ import {
CUSTOM_API_CALL_NAME,
inTest,
CLI_DIR,
inE2ETests,
} from '@/constants';
import { CredentialsOverwrites } from '@/CredentialsOverwrites';
import { Service } from 'typedi';
@@ -64,6 +65,11 @@ export class LoadNodesAndCredentials implements INodesAndCredentials {
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
if (!inTest) module.constructor._initPaths();
if (!inE2ETests) {
this.excludeNodes = this.excludeNodes ?? [];
this.excludeNodes.push('n8n-nodes-base.e2eTest');
}
this.downloadFolder = UserSettings.getUserN8nFolderDownloadedNodesPath();
// Load nodes from `n8n-nodes-base`

View File

@@ -888,7 +888,8 @@ export default defineComponent({
if (
this.node === null ||
this.hasRemoteMethod === false ||
this.remoteParameterOptionsLoading
this.remoteParameterOptionsLoading ||
!this.parameter
) {
return;
}
@@ -900,7 +901,8 @@ export default defineComponent({
try {
const currentNodeParameters = (this.ndvStore.activeNode as INodeUi).parameters;
const resolvedNodeParameters = this.resolveParameter(
const resolvedNodeParameters = this.resolveRequiredParameters(
this.parameter,
currentNodeParameters,
) as INodeParameters;
const loadOptionsMethod = this.getArgument('loadOptionsMethod') as string | undefined;

View File

@@ -1,5 +1,5 @@
<template>
<div :class="$style['parameter-issues']" v-if="issues.length">
<div :class="$style['parameter-issues']" data-test-id="parameter-issues" v-if="issues.length">
<n8n-tooltip placement="top">
<template #content>
<titled-list :title="`${$locale.baseText('parameterInput.issues')}:`" :items="issues" />

View File

@@ -681,7 +681,10 @@ export default defineComponent({
});
}
const resolvedNodeParameters = this.resolveParameter(params.parameters) as INodeParameters;
const resolvedNodeParameters = this.resolveRequiredParameters(
this.parameter,
params.parameters,
) as INodeParameters;
const loadOptionsMethod = this.getPropertyArgument(this.currentMode, 'searchListMethod') as
| string
| undefined;

View File

@@ -18,6 +18,7 @@
@update:modelValue="onFilterInput"
ref="search"
:placeholder="$locale.baseText('resourceLocator.search.placeholder')"
data-test-id="rlc-search"
>
<template #prefix>
<font-awesome-icon :class="$style.searchIcon" icon="search" />
@@ -47,6 +48,7 @@
[$style.selected]: result.value === modelValue,
[$style.hovering]: hoverIndex === i,
}"
data-test-id="rlc-item"
@click="() => onItemClick(result.value)"
@mouseenter="() => onItemHover(i)"
@mouseleave="() => onItemHoverLeave()"

View File

@@ -1,6 +1,6 @@
<script setup lang="ts">
import type { IUpdateInformation, ResourceMapperReqParams } from '@/Interface';
import { resolveParameter } from '@/mixins/workflowHelpers';
import { resolveRequiredParameters } from '@/mixins/workflowHelpers';
import { useNodeTypesStore } from '@/stores/nodeTypes.store';
import type {
INode,
@@ -229,7 +229,10 @@ async function loadFieldsToMap(): Promise<void> {
name: props.node?.type,
version: props.node.typeVersion,
},
currentNodeParameters: resolveParameter(props.node.parameters) as INodeParameters,
currentNodeParameters: resolveRequiredParameters(
props.parameter,
props.node.parameters,
) as INodeParameters,
path: props.path,
methodName: props.parameter.typeOptions?.resourceMapper?.resourceMapperMethod,
credentials: props.node.credentials,

View File

@@ -26,7 +26,7 @@ describe('ResourceMapper.vue', () => {
.spyOn(nodeTypeStore, 'getResourceMapperFields')
.mockResolvedValue(MAPPING_COLUMNS_RESPONSE);
resolveParameterSpy = vi
.spyOn(workflowHelpers, 'resolveParameter')
.spyOn(workflowHelpers, 'resolveRequiredParameters')
.mockReturnValue(NODE_PARAMETER_VALUES);
});

View File

@@ -27,6 +27,8 @@ import type {
IExecuteData,
INodeConnection,
IWebhookDescription,
INodeProperties,
IWorkflowSettings,
} from 'n8n-workflow';
import { NodeHelpers } from 'n8n-workflow';
@@ -39,6 +41,7 @@ import type {
XYPosition,
ITag,
TargetItem,
ICredentialsResponse,
} from '../Interface';
import { externalHooks } from '@/mixins/externalHooks';
@@ -53,7 +56,6 @@ import { getSourceItems } from '@/utils';
import { useUIStore } from '@/stores/ui.store';
import { useWorkflowsStore } from '@/stores/workflows.store';
import { useRootStore } from '@/stores/n8nRoot.store';
import type { IWorkflowSettings } from 'n8n-workflow';
import { useNDVStore } from '@/stores/ndv.store';
import { useTemplatesStore } from '@/stores/templates.store';
import { useNodeTypesStore } from '@/stores/nodeTypes.store';
@@ -62,7 +64,6 @@ import { useEnvironmentsStore } from '@/stores/environments.ee.store';
import { useUsersStore } from '@/stores/users.store';
import { getWorkflowPermissions } from '@/permissions';
import type { IPermissions } from '@/permissions';
import type { ICredentialsResponse } from '@/Interface';
export function resolveParameter(
parameter: NodeParameterValue | INodeParameters | NodeParameterValue[] | INodeParameters[],
@@ -122,7 +123,7 @@ export function resolveParameter(
);
let runExecutionData: IRunExecutionData;
if (executionData === null || !executionData.data) {
if (!executionData?.data) {
runExecutionData = {
resultData: {
runData: {},
@@ -176,6 +177,47 @@ export function resolveParameter(
) as IDataObject;
}
export function resolveRequiredParameters(
currentParameter: INodeProperties,
parameters: INodeParameters,
opts: {
targetItem?: TargetItem;
inputNodeName?: string;
inputRunIndex?: number;
inputBranchIndex?: number;
} = {},
): IDataObject | null {
const loadOptionsDependsOn = currentParameter?.typeOptions?.loadOptionsDependsOn ?? [];
const initial: { required: INodeParameters; nonrequired: INodeParameters } = {
required: {},
nonrequired: {},
};
const { required, nonrequired }: INodeParameters = Object.keys(parameters).reduce(
(accu, name: string) => {
const required = loadOptionsDependsOn.includes(name);
accu[required ? 'required' : 'nonrequired'][name] = parameters[name];
return accu;
},
initial,
);
const resolvedRequired = resolveParameter(required, opts);
let resolvedNonrequired: IDataObject | null = {};
try {
resolvedNonrequired = resolveParameter(nonrequired, opts);
} catch (e) {
// ignore any expression errors for example
}
return {
...resolvedRequired,
...(resolvedNonrequired ?? {}),
};
}
function getCurrentWorkflow(copyData?: boolean): Workflow {
return useWorkflowsStore().getCurrentWorkflow(copyData);
}
@@ -210,7 +252,7 @@ function connectionInputData(
) {
connectionInputData = [];
} else {
connectionInputData = _executeData.data![inputName][nodeConnection.sourceIndex];
connectionInputData = _executeData.data[inputName][nodeConnection.sourceIndex];
if (connectionInputData !== null) {
// Update the pairedItem information on items
@@ -314,7 +356,7 @@ function executeData(
executeData.data = workflowRunData[parentNodeName][runIndex].data!;
if (workflowRunData[currentNode] && workflowRunData[currentNode][runIndex]) {
executeData.source = {
[inputName]: workflowRunData[currentNode][runIndex].source!,
[inputName]: workflowRunData[currentNode][runIndex].source,
};
} else {
// The current node did not get executed in UI yet so build data manually
@@ -357,6 +399,7 @@ export const workflowHelpers = defineComponent({
},
methods: {
resolveParameter,
resolveRequiredParameters,
getCurrentWorkflow,
getNodes,
getWorkflow,
@@ -572,9 +615,7 @@ export const workflowHelpers = defineComponent({
continue;
}
if (
this.displayParameter(node.parameters, credentialTypeDescription, '', node) === false
) {
if (!this.displayParameter(node.parameters, credentialTypeDescription, '', node)) {
// Credential should not be displayed so do also not save
continue;
}
@@ -661,7 +702,7 @@ export const workflowHelpers = defineComponent({
return null;
}
const obj = returnData['__xxxxxxx__'];
const obj = returnData.__xxxxxxx__;
if (typeof obj === 'object') {
const proxy = obj as { isProxy: boolean; toJSON?: () => unknown } | null;
if (proxy?.isProxy && proxy.toJSON) return JSON.stringify(proxy.toJSON());
@@ -921,7 +962,7 @@ export const workflowHelpers = defineComponent({
if (redirect) {
void this.$router.replace({
name: VIEWS.WORKFLOW,
params: { name: workflowData.id as string, action: 'workflowSave' },
params: { name: workflowData.id, action: 'workflowSave' },
});
}

View File

@@ -0,0 +1,18 @@
{
"node": "n8n-nodes-base.e2eTest",
"nodeVersion": "1.0",
"codexVersion": "1.0",
"details": "The is a test node for e2e testing.",
"categories": ["Core Nodes"],
"resources": {
"primaryDocumentation": [
{
"url": "https://docs.n8n.io/integrations/builtin/core-nodes/n8n-nodes-base.editimage/"
}
]
},
"subcategories": {
"Core Nodes": ["Helpers"]
},
"e2e": true
}

View File

@@ -0,0 +1,207 @@
import type {
IExecuteFunctions,
ILoadOptionsFunctions,
INodeExecutionData,
INodeListSearchResult,
INodePropertyOptions,
INodeType,
INodeTypeDescription,
ResourceMapperFields,
} from 'n8n-workflow';
import { remoteOptions, resourceMapperFields, returnData, searchOptions } from './mock';
export class E2eTest implements INodeType {
description: INodeTypeDescription = {
displayName: 'E2E Test',
name: 'e2eTest',
icon: 'fa:play',
group: ['output'],
version: 1,
subtitle: '={{$parameter["operation"]}}',
description: 'Dummy node used for e2e testing',
defaults: {
name: 'E2E Test',
},
inputs: ['main'],
outputs: ['main'],
properties: [
{
displayName: 'Operation',
name: 'operation',
type: 'options',
noDataExpression: true,
options: [
{
name: 'Remote Options',
value: 'remoteOptions',
},
{
name: 'Resource Locator',
value: 'resourceLocator',
},
{
name: 'Resource Mapping Component',
value: 'resourceMapper',
},
],
default: 'remoteOptions',
},
{
displayName: 'Field ID',
name: 'fieldId',
type: 'string',
default: '',
},
{
displayName: 'Remote Options Name or ID',
name: 'remoteOptions',
description:
'Remote options to load. Choose from the list, or specify an ID using an <a href="https://docs.n8n.io/code-examples/expressions/">expression</a>.',
type: 'options',
typeOptions: {
loadOptionsDependsOn: ['fieldId'],
loadOptionsMethod: 'getOptions',
},
required: true,
default: [],
displayOptions: {
show: {
operation: ['remoteOptions'],
},
},
},
{
displayName: 'Resource Locator',
name: 'rlc',
type: 'resourceLocator',
default: { mode: 'list', value: '' },
required: true,
displayOptions: {
show: {
operation: ['resourceLocator'],
},
},
modes: [
{
displayName: 'From List',
name: 'list',
type: 'list',
typeOptions: {
searchListMethod: 'optionsSearch',
searchable: true,
},
},
{
displayName: 'By URL',
name: 'url',
type: 'string',
placeholder: 'https://example.com/user/a4071e98-7d40-41fb-8911-ce3e7bf94fb2',
validation: [
{
type: 'regex',
properties: {
regex:
'https://example.com/user/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}',
errorMessage: 'Not a valid example URL',
},
},
],
extractValue: {
type: 'regex',
regex:
'https://example.com/user/([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})',
},
},
{
displayName: 'ID',
name: 'id',
type: 'string',
validation: [
{
type: 'regex',
properties: {
regex: '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}',
errorMessage: 'Not a valid UUI',
},
},
],
placeholder: 'a4071e98-7d40-41fb-8911-ce3e7bf94fb2',
},
],
},
{
displayName: 'Resource Mapping Component',
name: 'resourceMapper',
type: 'resourceMapper',
noDataExpression: true,
default: {
mappingMode: 'defineBelow',
value: null,
},
required: true,
typeOptions: {
loadOptionsDependsOn: ['fieldId'],
resourceMapper: {
resourceMapperMethod: 'getMappingColumns',
mode: 'upsert',
fieldWords: {
singular: 'column',
plural: 'columns',
},
addAllFields: true,
multiKeyMatch: false,
},
},
displayOptions: {
show: {
operation: ['resourceMapper'],
},
},
},
{
displayName: 'Other Non Important Field',
name: 'otherField',
type: 'string',
default: '',
},
],
};
methods = {
loadOptions: {
async getOptions(this: ILoadOptionsFunctions): Promise<INodePropertyOptions[]> {
return remoteOptions;
},
},
listSearch: {
async optionsSearch(
this: ILoadOptionsFunctions,
filter?: string,
paginationToken?: string,
): Promise<INodeListSearchResult> {
const pageSize = 5;
let results = searchOptions;
if (filter) {
results = results.filter((option) => option.name.includes(filter));
}
const offset = paginationToken ? parseInt(paginationToken, 10) : 0;
results = results.slice(offset, offset + pageSize);
return {
results,
paginationToken: offset + pageSize,
};
},
},
resourceMapping: {
async getMappingColumns(this: ILoadOptionsFunctions): Promise<ResourceMapperFields> {
return resourceMapperFields;
},
},
};
async execute(this: IExecuteFunctions): Promise<INodeExecutionData[][]> {
return this.prepareOutputData(returnData);
}
}

View File

@@ -0,0 +1,64 @@
import { array, name, uuid } from 'minifaker';
import 'minifaker/locales/en';
import type {
INodeExecutionData,
INodeListSearchResult,
INodePropertyOptions,
ResourceMapperFields,
} from 'n8n-workflow';
export const returnData: INodeExecutionData[] = [
{
json: {
id: '23423532',
name: 'Hello World',
},
},
];
export const remoteOptions: INodePropertyOptions[] = [
{
name: 'Resource 1',
value: 'resource1',
},
{
name: 'Resource 2',
value: 'resource2',
},
{
name: 'Resource 3',
value: 'resource3',
},
];
export const resourceMapperFields: ResourceMapperFields = {
fields: [
{
id: 'id',
displayName: 'ID',
defaultMatch: true,
canBeUsedToMatch: true,
required: true,
display: true,
type: 'string',
},
{
id: 'name',
displayName: 'Name',
defaultMatch: false,
canBeUsedToMatch: false,
required: false,
display: true,
type: 'string',
},
],
};
export const searchOptions: INodeListSearchResult['results'] = array(100, () => {
const value = uuid.v4();
return {
name: name(),
value,
url: 'https://example.com/user/' + value,
};
});

View File

@@ -466,6 +466,7 @@
"dist/nodes/Dropbox/Dropbox.node.js",
"dist/nodes/Dropcontact/Dropcontact.node.js",
"dist/nodes/EditImage/EditImage.node.js",
"dist/nodes/E2eTest/E2eTest.node.js",
"dist/nodes/Egoi/Egoi.node.js",
"dist/nodes/Elastic/Elasticsearch/Elasticsearch.node.js",
"dist/nodes/Elastic/ElasticSecurity/ElasticSecurity.node.js",