From 8bf07365ac01ac8b7639ab76353dd4ffc6b5337f Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Fri, 12 Apr 2024 14:37:11 +0200 Subject: [PATCH 1/4] Fix parameter reset on credential change in Discord node --- cypress/e2e/5-ndv.cy.ts | 21 ++++++++++++-- .../editor-ui/src/components/NodeSettings.vue | 28 +++++++++++++++---- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts index 7ce95aba055f1..1989a5b010c61 100644 --- a/cypress/e2e/5-ndv.cy.ts +++ b/cypress/e2e/5-ndv.cy.ts @@ -3,6 +3,8 @@ import { getVisibleSelect } from '../utils'; import { MANUAL_TRIGGER_NODE_DISPLAY_NAME } from '../constants'; import { NDV, WorkflowPage } from '../pages'; import { NodeCreator } from '../pages/features/node-creator'; +import { clickCreateNewCredential } from '../composables/ndv'; +import { setCredentialValues } from '../composables/modals/credential-modal'; const workflowPage = new WorkflowPage(); const ndv = new NDV(); @@ -633,7 +635,7 @@ describe('NDV', () => { ndv.getters.nodeRunErrorIndicator().should('exist'); }); - it('Should handle mismatched option attributes', () => { + it.only('Should clear mismatched collection parameters', () => { workflowPage.actions.addInitialNodeToCanvas('LDAP', { keepNdvOpen: true, action: 'Create a new entry', @@ -645,7 +647,7 @@ describe('NDV', () => { cy.getByTestId('parameter-item').contains('Currently no items exist').should('exist'); }); - it('Should keep RLC values after operation change', () => { + it.only('Should keep RLC values after operation change', () => { const TEST_DOC_ID = '1111'; workflowPage.actions.addInitialNodeToCanvas('Google Sheets', { keepNdvOpen: true, @@ -656,6 +658,21 @@ describe('NDV', () => { ndv.getters.resourceLocatorInput('documentId').find('input').should('have.value', TEST_DOC_ID); }); + it.only('Should not clear resource/operation after credential change', () => { + workflowPage.actions.addInitialNodeToCanvas('Discord', { + keepNdvOpen: true, + action: 'Send a message', + }); + + clickCreateNewCredential(); + setCredentialValues({ + botToken: 'sk_test_123', + }); + + ndv.getters.parameterInput('resource').find('input').should('have.value', 'Message'); + ndv.getters.parameterInput('operation').find('input').should('have.value', 'Send'); + }); + it('Should open appropriate node creator after clicking on connection hint link', () => { const nodeCreator = new NodeCreator(); const hintMapper = { diff --git a/packages/editor-ui/src/components/NodeSettings.vue b/packages/editor-ui/src/components/NodeSettings.vue index 3b464f9dc9bb3..af2060a8b1726 100644 --- a/packages/editor-ui/src/components/NodeSettings.vue +++ b/packages/editor-ui/src/components/NodeSettings.vue @@ -187,7 +187,14 @@ import type { NodeParameterValue, ConnectionTypes, } from 'n8n-workflow'; -import { NodeHelpers, NodeConnectionType, deepCopy } from 'n8n-workflow'; +import { + NodeHelpers, + NodeConnectionType, + deepCopy, + isINodePropertyCollectionList, + isINodePropertiesList, + isINodePropertyOptionsList, +} from 'n8n-workflow'; import type { INodeUi, INodeUpdatePropertiesInformation, @@ -997,13 +1004,22 @@ export default defineComponent({ return; } // Every value should be a possible option - const hasValidOptions = Object.keys(nodeParameterValues).every( - (key) => (prop.options ?? []).find((option) => option.name === key) !== undefined, - ); + let hasValidOptions = true; + + if (isINodePropertyCollectionList(prop.options) || isINodePropertiesList(prop.options)) { + hasValidOptions = Object.keys(nodeParameterValues).every( + (key) => (prop.options ?? []).find((option) => option.name === key) !== undefined, + ); + } else if (isINodePropertyOptionsList(prop.options)) { + hasValidOptions = !!prop.options.find( + (option) => option.value === nodeParameterValues[prop.name], + ); + } + if ( !hasValidOptions || - showCondition !== updatedParameter.value || - hideCondition === updatedParameter.value + (showCondition && !showCondition.includes(updatedParameter.value)) || + (hideCondition && hideCondition.includes(updatedParameter.value)) ) { unset(nodeParameterValues as object, prop.name); } From 3b7091e1ceaa69de26f11c939841a76bce41e9de Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Fri, 12 Apr 2024 14:40:22 +0200 Subject: [PATCH 2/4] Remove .only --- cypress/e2e/5-ndv.cy.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts index 1989a5b010c61..b01b0edf5b0a3 100644 --- a/cypress/e2e/5-ndv.cy.ts +++ b/cypress/e2e/5-ndv.cy.ts @@ -635,7 +635,7 @@ describe('NDV', () => { ndv.getters.nodeRunErrorIndicator().should('exist'); }); - it.only('Should clear mismatched collection parameters', () => { + it('Should clear mismatched collection parameters', () => { workflowPage.actions.addInitialNodeToCanvas('LDAP', { keepNdvOpen: true, action: 'Create a new entry', @@ -647,7 +647,7 @@ describe('NDV', () => { cy.getByTestId('parameter-item').contains('Currently no items exist').should('exist'); }); - it.only('Should keep RLC values after operation change', () => { + it('Should keep RLC values after operation change', () => { const TEST_DOC_ID = '1111'; workflowPage.actions.addInitialNodeToCanvas('Google Sheets', { keepNdvOpen: true, @@ -658,7 +658,7 @@ describe('NDV', () => { ndv.getters.resourceLocatorInput('documentId').find('input').should('have.value', TEST_DOC_ID); }); - it.only('Should not clear resource/operation after credential change', () => { + it('Should not clear resource/operation after credential change', () => { workflowPage.actions.addInitialNodeToCanvas('Discord', { keepNdvOpen: true, action: 'Send a message', From 2fba2557bb46b96792476ee2fd85bbf2911c1c9f Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Fri, 12 Apr 2024 16:52:03 +0200 Subject: [PATCH 3/4] Fix reset of operation --- cypress/e2e/5-ndv.cy.ts | 10 +++++----- packages/editor-ui/src/components/NodeSettings.vue | 12 +++++------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts index b01b0edf5b0a3..03760d6d219b0 100644 --- a/cypress/e2e/5-ndv.cy.ts +++ b/cypress/e2e/5-ndv.cy.ts @@ -635,7 +635,7 @@ describe('NDV', () => { ndv.getters.nodeRunErrorIndicator().should('exist'); }); - it('Should clear mismatched collection parameters', () => { + it.only('Should clear mismatched collection parameters', () => { workflowPage.actions.addInitialNodeToCanvas('LDAP', { keepNdvOpen: true, action: 'Create a new entry', @@ -647,7 +647,7 @@ describe('NDV', () => { cy.getByTestId('parameter-item').contains('Currently no items exist').should('exist'); }); - it('Should keep RLC values after operation change', () => { + it.only('Should keep RLC values after operation change', () => { const TEST_DOC_ID = '1111'; workflowPage.actions.addInitialNodeToCanvas('Google Sheets', { keepNdvOpen: true, @@ -658,10 +658,10 @@ describe('NDV', () => { ndv.getters.resourceLocatorInput('documentId').find('input').should('have.value', TEST_DOC_ID); }); - it('Should not clear resource/operation after credential change', () => { + it.only('Should not clear resource/operation after credential change', () => { workflowPage.actions.addInitialNodeToCanvas('Discord', { keepNdvOpen: true, - action: 'Send a message', + action: 'Delete a message', }); clickCreateNewCredential(); @@ -670,7 +670,7 @@ describe('NDV', () => { }); ndv.getters.parameterInput('resource').find('input').should('have.value', 'Message'); - ndv.getters.parameterInput('operation').find('input').should('have.value', 'Send'); + ndv.getters.parameterInput('operation').find('input').should('have.value', 'Delete'); }); it('Should open appropriate node creator after clicking on connection hint link', () => { diff --git a/packages/editor-ui/src/components/NodeSettings.vue b/packages/editor-ui/src/components/NodeSettings.vue index af2060a8b1726..1f5b54a7f1d97 100644 --- a/packages/editor-ui/src/components/NodeSettings.vue +++ b/packages/editor-ui/src/components/NodeSettings.vue @@ -194,6 +194,7 @@ import { isINodePropertyCollectionList, isINodePropertiesList, isINodePropertyOptionsList, + displayParameter, } from 'n8n-workflow'; import type { INodeUi, @@ -997,15 +998,16 @@ export default defineComponent({ if (!nodeParameterValues?.hasOwnProperty(prop.name) || !displayOptions || !prop.options) { return; } - // Only process the parameters that should be hidden + // Only process the parameters that depend on the updated parameter const showCondition = displayOptions.show?.[updatedParameter.name]; const hideCondition = displayOptions.hide?.[updatedParameter.name]; if (showCondition === undefined && hideCondition === undefined) { return; } - // Every value should be a possible option + let hasValidOptions = true; + // Every value should be a possible option if (isINodePropertyCollectionList(prop.options) || isINodePropertiesList(prop.options)) { hasValidOptions = Object.keys(nodeParameterValues).every( (key) => (prop.options ?? []).find((option) => option.name === key) !== undefined, @@ -1016,11 +1018,7 @@ export default defineComponent({ ); } - if ( - !hasValidOptions || - (showCondition && !showCondition.includes(updatedParameter.value)) || - (hideCondition && hideCondition.includes(updatedParameter.value)) - ) { + if (!hasValidOptions && displayParameter(nodeParameterValues, prop, this.node)) { unset(nodeParameterValues as object, prop.name); } }); From fc1d3265e3ee3b6bf0dfd48ef54cfa3d7dfff7d0 Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Fri, 12 Apr 2024 17:00:26 +0200 Subject: [PATCH 4/4] Remove .only --- cypress/e2e/5-ndv.cy.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts index 03760d6d219b0..6103dbbc039f6 100644 --- a/cypress/e2e/5-ndv.cy.ts +++ b/cypress/e2e/5-ndv.cy.ts @@ -635,7 +635,7 @@ describe('NDV', () => { ndv.getters.nodeRunErrorIndicator().should('exist'); }); - it.only('Should clear mismatched collection parameters', () => { + it('Should clear mismatched collection parameters', () => { workflowPage.actions.addInitialNodeToCanvas('LDAP', { keepNdvOpen: true, action: 'Create a new entry', @@ -647,7 +647,7 @@ describe('NDV', () => { cy.getByTestId('parameter-item').contains('Currently no items exist').should('exist'); }); - it.only('Should keep RLC values after operation change', () => { + it('Should keep RLC values after operation change', () => { const TEST_DOC_ID = '1111'; workflowPage.actions.addInitialNodeToCanvas('Google Sheets', { keepNdvOpen: true, @@ -658,7 +658,7 @@ describe('NDV', () => { ndv.getters.resourceLocatorInput('documentId').find('input').should('have.value', TEST_DOC_ID); }); - it.only('Should not clear resource/operation after credential change', () => { + it('Should not clear resource/operation after credential change', () => { workflowPage.actions.addInitialNodeToCanvas('Discord', { keepNdvOpen: true, action: 'Delete a message',