From 8a8d4e8bb32588e79c3fcda2317c491ade9b3637 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour <4711238+mutdmour@users.noreply.github.com> Date: Thu, 31 Aug 2023 16:40:20 +0200 Subject: [PATCH] fix: Load remote resources even if expressions in non requried parameters resolve (#6987) Github issue / Community forum post (link here to close automatically): --- cypress/e2e/26-resource-locator.cy.ts | 17 +- cypress/e2e/28-resource-mapper.cy.ts | 32 +++ cypress/e2e/5-ndv.cy.ts | 33 +++ cypress/pages/ndv.ts | 22 +- cypress/pages/workflow.ts | 12 +- cypress/utils/popper.ts | 6 +- packages/cli/src/LoadNodesAndCredentials.ts | 6 + .../src/components/ParameterInput.vue | 6 +- .../src/components/ParameterIssues.vue | 2 +- .../ResourceLocator/ResourceLocator.vue | 5 +- .../ResourceLocatorDropdown.vue | 2 + .../ResourceMapper/ResourceMapper.vue | 7 +- .../__tests__/ResourceMapper.test.ts | 2 +- .../editor-ui/src/mixins/workflowHelpers.ts | 61 +++++- .../nodes/E2eTest/E2eTest.node.json | 18 ++ .../nodes-base/nodes/E2eTest/E2eTest.node.ts | 207 ++++++++++++++++++ packages/nodes-base/nodes/E2eTest/mock.ts | 64 ++++++ packages/nodes-base/package.json | 1 + 18 files changed, 478 insertions(+), 25 deletions(-) create mode 100644 cypress/e2e/28-resource-mapper.cy.ts create mode 100644 packages/nodes-base/nodes/E2eTest/E2eTest.node.json create mode 100644 packages/nodes-base/nodes/E2eTest/E2eTest.node.ts create mode 100644 packages/nodes-base/nodes/E2eTest/mock.ts diff --git a/cypress/e2e/26-resource-locator.cy.ts b/cypress/e2e/26-resource-locator.cy.ts index 5231d3fe29..219a8b8fe7 100644 --- a/cypress/e2e/26-resource-locator.cy.ts +++ b/cypress/e2e/26-resource-locator.cy.ts @@ -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(); @@ -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); + }); }); diff --git a/cypress/e2e/28-resource-mapper.cy.ts b/cypress/e2e/28-resource-mapper.cy.ts new file mode 100644 index 0000000000..64e1179365 --- /dev/null +++ b/cypress/e2e/28-resource-mapper.cy.ts @@ -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); + }); +}); diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts index a729ddffac..7d7dbbd0de 100644 --- a/cypress/e2e/5-ndv.cy.ts +++ b/cypress/e2e/5-ndv.cy.ts @@ -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(); @@ -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(); diff --git a/cypress/pages/ndv.ts b/cypress/pages/ndv.ts index 0cd8e0913b..f8be5b991b 100644 --- a/cypress/pages/ndv.ts +++ b/cypress/pages/ndv.ts @@ -1,5 +1,5 @@ import { BasePage } from './base'; -import { getVisibleSelect } from '../utils'; +import { getVisiblePopper, getVisibleSelect } from '../utils'; export class NDV extends BasePage { getters = { @@ -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() @@ -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 = { @@ -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(); @@ -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`); + } }; } diff --git a/cypress/pages/workflow.ts b/cypress/pages/workflow.ts index a92736e179..622aa9d98d 100644 --- a/cypress/pages/workflow.ts +++ b/cypress/pages/workflow.ts @@ -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 = { @@ -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, diff --git a/cypress/utils/popper.ts b/cypress/utils/popper.ts index 846b2ec88e..5743c70f3e 100644 --- a/cypress/utils/popper.ts +++ b/cypress/utils/popper.ts @@ -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() { diff --git a/packages/cli/src/LoadNodesAndCredentials.ts b/packages/cli/src/LoadNodesAndCredentials.ts index 0494b7e7b8..269e6ce867 100644 --- a/packages/cli/src/LoadNodesAndCredentials.ts +++ b/packages/cli/src/LoadNodesAndCredentials.ts @@ -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` diff --git a/packages/editor-ui/src/components/ParameterInput.vue b/packages/editor-ui/src/components/ParameterInput.vue index 9ff9004163..53d5bb0617 100644 --- a/packages/editor-ui/src/components/ParameterInput.vue +++ b/packages/editor-ui/src/components/ParameterInput.vue @@ -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; diff --git a/packages/editor-ui/src/components/ParameterIssues.vue b/packages/editor-ui/src/components/ParameterIssues.vue index 96da86f7b7..8cbb73cb95 100644 --- a/packages/editor-ui/src/components/ParameterIssues.vue +++ b/packages/editor-ui/src/components/ParameterIssues.vue @@ -1,5 +1,5 @@