Skip to content

Commit

Permalink
fix: Load remote resources even if expressions in non requried parame…
Browse files Browse the repository at this point in the history
…ters resolve (#6987)

Github issue / Community forum post (link here to close automatically):
  • Loading branch information
mutdmour authored Aug 31, 2023
1 parent 8cd4db0 commit 8a8d4e8
Show file tree
Hide file tree
Showing 18 changed files with 478 additions and 25 deletions.
17 changes: 16 additions & 1 deletion cypress/e2e/26-resource-locator.cy.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { WorkflowPage, NDV, CredentialsModal } from '../pages';
import { getVisibleSelect } from '../utils';
import { getPopper, getVisiblePopper, getVisibleSelect } from '../utils';

const workflowPage = new WorkflowPage();
const ndv = new NDV();
Expand Down Expand Up @@ -51,4 +51,19 @@ describe('Resource Locator', () => {
ndv.actions.setRLCValue('documentId', '321');
ndv.getters.resourceLocatorInput('sheetName').should('have.value', '');
});

// unlike RMC and remote options, RLC does not support loadOptionDependsOn
it('should retrieve list options when other params throw errors', () => {
workflowPage.actions.addInitialNodeToCanvas('E2e Test', {action: 'Resource Locator'});

ndv.getters.resourceLocatorInput('rlc').click();
getVisiblePopper().should('have.length', 1).findChildByTestId('rlc-item').should('have.length', 5);

ndv.actions.setInvalidExpression('fieldId');

ndv.getters.container().click(); // remove focus from input, hide expression preview

ndv.getters.resourceLocatorInput('rlc').click();
getVisiblePopper().should('have.length', 1).findChildByTestId('rlc-item').should('have.length', 5);
});
});
32 changes: 32 additions & 0 deletions cypress/e2e/28-resource-mapper.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { WorkflowPage, NDV } from '../pages';

const workflowPage = new WorkflowPage();
const ndv = new NDV();

describe('Resource Mapper', () => {
beforeEach(() => {
workflowPage.actions.visit();
});

it('should not retrieve list options when required params throw errors', () => {
workflowPage.actions.addInitialNodeToCanvas('E2e Test', {action: 'Resource Mapping Component'});

ndv.getters.resourceMapperFieldsContainer().should('be.visible').findChildByTestId('parameter-input').should('have.length', 2);

ndv.actions.setInvalidExpression('fieldId');

ndv.actions.refreshResourceMapperColumns();
ndv.getters.resourceMapperFieldsContainer().should('not.exist');
});

it('should retrieve list options when optional params throw errors', () => {
workflowPage.actions.addInitialNodeToCanvas('E2e Test', {action: 'Resource Mapping Component'});

ndv.getters.resourceMapperFieldsContainer().should('be.visible').findChildByTestId('parameter-input').should('have.length', 2);

ndv.actions.setInvalidExpression('otherField');

ndv.actions.refreshResourceMapperColumns();
ndv.getters.resourceMapperFieldsContainer().should('be.visible').findChildByTestId('parameter-input').should('have.length', 2);
});
});
33 changes: 33 additions & 0 deletions cypress/e2e/5-ndv.cy.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { WorkflowPage, NDV } from '../pages';
import { v4 as uuid } from 'uuid';
import { getPopper, getVisiblePopper, getVisibleSelect } from '../utils';

const workflowPage = new WorkflowPage();
const ndv = new NDV();
Expand Down Expand Up @@ -289,6 +290,38 @@ describe('NDV', () => {
});
});

it('should not retrieve remote options when required params throw errors', () => {
workflowPage.actions.addInitialNodeToCanvas('E2e Test', {action: 'Remote Options'});

ndv.getters.parameterInput('remoteOptions').click();
getVisibleSelect().find('.el-select-dropdown__item').should('have.length', 3);

ndv.actions.setInvalidExpression('fieldId');

ndv.getters.container().click(); // remove focus from input, hide expression preview

ndv.getters.parameterInput('remoteOptions').click();
getPopper().should('not.be.visible');

ndv.getters.parameterInputIssues('remoteOptions').realHover();
getVisiblePopper().should('include.text', `node doesn't exist`);
});

it('should retrieve remote options when non-required params throw errors', () => {
workflowPage.actions.addInitialNodeToCanvas('E2e Test', {action: 'Remote Options'});

ndv.getters.parameterInput('remoteOptions').click();
getVisibleSelect().find('.el-select-dropdown__item').should('have.length', 3);
ndv.getters.parameterInput('remoteOptions').click();

ndv.actions.setInvalidExpression('otherField');

ndv.getters.container().click(); // remove focus from input, hide expression preview

ndv.getters.parameterInput('remoteOptions').click();
getVisibleSelect().find('.el-select-dropdown__item').should('have.length', 3);
});

it('should flag issues as soon as params are set', () => {
workflowPage.actions.addInitialNodeToCanvas('Webhook');
workflowPage.getters.canvasNodes().first().dblclick();
Expand Down
22 changes: 19 additions & 3 deletions cypress/pages/ndv.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BasePage } from './base';
import { getVisibleSelect } from '../utils';
import { getVisiblePopper, getVisibleSelect } from '../utils';

export class NDV extends BasePage {
getters = {
Expand Down Expand Up @@ -39,6 +39,7 @@ export class NDV extends BasePage {
inlineExpressionEditorInput: () => cy.getByTestId('inline-expression-editor-input'),
nodeParameters: () => cy.getByTestId('node-parameters'),
parameterInput: (parameterName: string) => cy.getByTestId(`parameter-input-${parameterName}`),
parameterInputIssues: (parameterName: string) => cy.getByTestId(`parameter-input-${parameterName}`).should('have.length', 1).findChildByTestId('parameter-issues'),
parameterExpressionPreview: (parameterName: string) =>
this.getters
.nodeParameters()
Expand All @@ -64,6 +65,8 @@ export class NDV extends BasePage {
resourceLocatorErrorMessage: () => cy.getByTestId('rlc-error-container'),
resourceLocatorModeSelector: (paramName: string) =>
this.getters.resourceLocator(paramName).find('[data-test-id="rlc-mode-selector"]'),
resourceMapperFieldsContainer: () => cy.getByTestId('mapping-fields-container'),
resourceMapperSelectColumn: () => cy.getByTestId('matching-column-select'),
};

actions = {
Expand Down Expand Up @@ -99,8 +102,8 @@ export class NDV extends BasePage {
clearParameterInput: (parameterName: string) => {
this.getters.parameterInput(parameterName).type(`{selectall}{backspace}`);
},
typeIntoParameterInput: (parameterName: string, content: string) => {
this.getters.parameterInput(parameterName).type(content);
typeIntoParameterInput: (parameterName: string, content: string, opts?: { parseSpecialCharSequences: boolean }) => {
this.getters.parameterInput(parameterName).type(content, opts);
},
selectOptionInParameterDropdown: (parameterName: string, content: string) => {
getVisibleSelect().find('.option-headline').contains(content).click();
Expand Down Expand Up @@ -171,6 +174,19 @@ export class NDV extends BasePage {
.find('span')
.should('include.html', asEncodedHTML(value));
},

refreshResourceMapperColumns: () => {
this.getters.resourceMapperSelectColumn().realHover();
this.getters.resourceMapperSelectColumn().findChildByTestId('action-toggle').should('have.length', 1).click();

getVisiblePopper().find('li').last().click();
},

setInvalidExpression: (fieldName: string, invalidExpression?: string) => {
this.actions.typeIntoParameterInput(fieldName, "=");
this.actions.typeIntoParameterInput(fieldName, invalidExpression ?? "{{ $('unknown')", { parseSpecialCharSequences: false });
this.actions.validateExpressionPreview(fieldName, `node doesn't exist`);
}
};
}

Expand Down
12 changes: 9 additions & 3 deletions cypress/pages/workflow.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { META_KEY } from '../constants';
import { BasePage } from './base';
import { getVisibleSelect } from '../utils';
import { NodeCreator } from './features/node-creator';

const nodeCreator = new NodeCreator();
export class WorkflowPage extends BasePage {
url = '/workflow/new';
getters = {
Expand Down Expand Up @@ -130,12 +132,16 @@ export class WorkflowPage extends BasePage {
win.preventNodeViewBeforeUnload = preventNodeViewUnload;
});
},
addInitialNodeToCanvas: (nodeDisplayName: string, { keepNdvOpen } = { keepNdvOpen: false }) => {
addInitialNodeToCanvas: (nodeDisplayName: string, opts?: { keepNdvOpen?: boolean, action?: string }) => {
this.getters.canvasPlusButton().click();
this.getters.nodeCreatorSearchBar().type(nodeDisplayName);
this.getters.nodeCreatorSearchBar().type('{enter}');
if (keepNdvOpen) return;
cy.get('body').type('{esc}');
if (opts?.action) {
nodeCreator.getters.getCreatorItem(opts.action).click();
}
else if (!opts?.keepNdvOpen) {
cy.get('body').type('{esc}');
}
},
addNodeToCanvas: (
nodeDisplayName: string,
Expand Down
6 changes: 5 additions & 1 deletion cypress/utils/popper.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
export function getPopper() {
return cy.get('.el-popper');
}

export function getVisiblePopper() {
return cy.get('.el-popper').filter(':visible');
return getPopper().filter(':visible');
}

export function getVisibleSelect() {
Expand Down
6 changes: 6 additions & 0 deletions packages/cli/src/LoadNodesAndCredentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
CUSTOM_API_CALL_NAME,
inTest,
CLI_DIR,
inE2ETests,
} from '@/constants';
import { CredentialsOverwrites } from '@/CredentialsOverwrites';
import { Service } from 'typedi';
Expand Down Expand Up @@ -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`
Expand Down
6 changes: 4 additions & 2 deletions packages/editor-ui/src/components/ParameterInput.vue
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,8 @@ export default defineComponent({
if (
this.node === null ||
this.hasRemoteMethod === false ||
this.remoteParameterOptionsLoading
this.remoteParameterOptionsLoading ||
!this.parameter
) {
return;
}
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/editor-ui/src/components/ParameterIssues.vue
Original file line number Diff line number Diff line change
@@ -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" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down Expand Up @@ -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()"
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down
Loading

0 comments on commit 8a8d4e8

Please sign in to comment.