From 63990f14e3991770c1b9fbfd56edd6d0f3abd54b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milorad=20FIlipovi=C4=87?= Date: Wed, 29 May 2024 16:13:54 +0200 Subject: [PATCH] fix(editor): Prevent updating node parameter value if it hasn't changed (#9535) --- cypress/e2e/29-sql-editor.cy.ts | 71 -------- cypress/e2e/41-editors.cy.ts | 169 ++++++++++++++++++ cypress/pages/ndv.ts | 1 + .../src/components/HtmlEditor/HtmlEditor.vue | 3 +- .../src/components/ParameterInput.vue | 7 + .../src/components/SqlEditor/SqlEditor.vue | 1 - 6 files changed, 178 insertions(+), 74 deletions(-) delete mode 100644 cypress/e2e/29-sql-editor.cy.ts create mode 100644 cypress/e2e/41-editors.cy.ts diff --git a/cypress/e2e/29-sql-editor.cy.ts b/cypress/e2e/29-sql-editor.cy.ts deleted file mode 100644 index 86299d5f67..0000000000 --- a/cypress/e2e/29-sql-editor.cy.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { WorkflowPage, NDV } from '../pages'; - -const workflowPage = new WorkflowPage(); -const ndv = new NDV(); - -describe('SQL editors', () => { - beforeEach(() => { - workflowPage.actions.visit(); - }); - - it('should preserve changes when opening-closing Postgres node', () => { - workflowPage.actions.addInitialNodeToCanvas('Postgres', { - action: 'Execute a SQL query', - keepNdvOpen: true, - }); - ndv.getters - .sqlEditorContainer() - .click() - .find('.cm-content') - .type('SELECT * FROM `testTable`') - .type('{esc}'); - ndv.actions.close(); - workflowPage.actions.openNode('Postgres'); - ndv.getters.sqlEditorContainer().find('.cm-content').type('{end} LIMIT 10').type('{esc}'); - ndv.actions.close(); - workflowPage.actions.openNode('Postgres'); - ndv.getters.sqlEditorContainer().should('contain', 'SELECT * FROM `testTable` LIMIT 10'); - }); - - it('should update expression output dropdown as the query is edited', () => { - workflowPage.actions.addInitialNodeToCanvas('MySQL', { - action: 'Execute a SQL query', - }); - ndv.actions.close(); - - workflowPage.actions.openNode('When clicking "Test workflow"'); - ndv.actions.setPinnedData([{ table: 'test_table' }]); - ndv.actions.close(); - - workflowPage.actions.openNode('MySQL'); - ndv.getters - .sqlEditorContainer() - .find('.cm-content') - .type('SELECT * FROM {{ $json.table }}', { parseSpecialCharSequences: false }); - workflowPage.getters - .inlineExpressionEditorOutput() - .should('have.text', 'SELECT * FROM test_table'); - }); - - it('should not push NDV header out with a lot of code in Postgres editor', () => { - workflowPage.actions.addInitialNodeToCanvas('Postgres', { - action: 'Execute a SQL query', - keepNdvOpen: true, - }); - cy.fixture('Dummy_javascript.txt').then((code) => { - ndv.getters.sqlEditorContainer().find('.cm-content').paste(code); - }); - ndv.getters.nodeExecuteButton().should('be.visible'); - }); - - it('should not push NDV header out with a lot of code in MySQL editor', () => { - workflowPage.actions.addInitialNodeToCanvas('MySQL', { - action: 'Execute a SQL query', - keepNdvOpen: true, - }); - cy.fixture('Dummy_javascript.txt').then((code) => { - ndv.getters.sqlEditorContainer().find('.cm-content').paste(code); - }); - ndv.getters.nodeExecuteButton().should('be.visible'); - }); -}); diff --git a/cypress/e2e/41-editors.cy.ts b/cypress/e2e/41-editors.cy.ts new file mode 100644 index 0000000000..b50aa66872 --- /dev/null +++ b/cypress/e2e/41-editors.cy.ts @@ -0,0 +1,169 @@ +import { WorkflowPage, NDV } from '../pages'; + +const workflowPage = new WorkflowPage(); +const ndv = new NDV(); + +// Update is debounced in editors, so adding typing delay to catch up +const TYPING_DELAY = 100; + +describe('Editors', () => { + beforeEach(() => { + workflowPage.actions.visit(); + }); + + describe('SQL Editor', () => { + + it('should preserve changes when opening-closing Postgres node', () => { + workflowPage.actions.addInitialNodeToCanvas('Postgres', { + action: 'Execute a SQL query', + keepNdvOpen: true, + }); + ndv.getters + .sqlEditorContainer() + .click() + .find('.cm-content') + .type('SELECT * FROM `testTable`', { delay: TYPING_DELAY }) + .type('{esc}'); + ndv.actions.close(); + workflowPage.actions.openNode('Postgres'); + ndv.getters.sqlEditorContainer().find('.cm-content').type('{end} LIMIT 10', { delay: TYPING_DELAY }).type('{esc}'); + ndv.actions.close(); + workflowPage.actions.openNode('Postgres'); + ndv.getters.sqlEditorContainer().should('contain', 'SELECT * FROM `testTable` LIMIT 10'); + }); + + it('should update expression output dropdown as the query is edited', () => { + workflowPage.actions.addInitialNodeToCanvas('MySQL', { + action: 'Execute a SQL query', + }); + ndv.actions.close(); + + workflowPage.actions.openNode('When clicking "Test workflow"'); + ndv.actions.setPinnedData([{ table: 'test_table' }]); + ndv.actions.close(); + + workflowPage.actions.openNode('MySQL'); + ndv.getters + .sqlEditorContainer() + .find('.cm-content') + .type('SELECT * FROM {{ $json.table }}', { parseSpecialCharSequences: false }); + workflowPage.getters + .inlineExpressionEditorOutput() + .should('have.text', 'SELECT * FROM test_table'); + }); + + it('should not push NDV header out with a lot of code in Postgres editor', () => { + workflowPage.actions.addInitialNodeToCanvas('Postgres', { + action: 'Execute a SQL query', + keepNdvOpen: true, + }); + cy.fixture('Dummy_javascript.txt').then((code) => { + ndv.getters.sqlEditorContainer().find('.cm-content').paste(code); + }); + ndv.getters.nodeExecuteButton().should('be.visible'); + }); + + it('should not push NDV header out with a lot of code in MySQL editor', () => { + workflowPage.actions.addInitialNodeToCanvas('MySQL', { + action: 'Execute a SQL query', + keepNdvOpen: true, + }); + cy.fixture('Dummy_javascript.txt').then((code) => { + ndv.getters.sqlEditorContainer().find('.cm-content').paste(code); + }); + ndv.getters.nodeExecuteButton().should('be.visible'); + }); + + it('should not trigger dirty flag if nothing is changed', () => { + workflowPage.actions.addInitialNodeToCanvas('Postgres', { + action: 'Execute a SQL query', + keepNdvOpen: true, + }); + ndv.actions.close(); + workflowPage.actions.saveWorkflowOnButtonClick(); + workflowPage.getters.isWorkflowSaved(); + workflowPage.actions.openNode('Postgres'); + ndv.actions.close(); + // Workflow should still be saved + workflowPage.getters.isWorkflowSaved(); + }); + + it('should trigger dirty flag if query is updated', () => { + workflowPage.actions.addInitialNodeToCanvas('Postgres', { + action: 'Execute a SQL query', + keepNdvOpen: true, + }); + ndv.actions.close(); + workflowPage.actions.saveWorkflowOnButtonClick(); + workflowPage.getters.isWorkflowSaved(); + workflowPage.actions.openNode('Postgres'); + ndv.getters + .sqlEditorContainer() + .click() + .find('.cm-content') + .type('SELECT * FROM `testTable`', { delay: TYPING_DELAY }) + .type('{esc}'); + ndv.actions.close(); + workflowPage.getters.isWorkflowSaved().should('not.be.true'); + }); + }); + + describe('HTML Editor', () => { + // Closing tags will be added by the editor + const TEST_ELEMENT_H1 = '

Test'; + const TEST_ELEMENT_P = '

Test'; + + it('should preserve changes when opening-closing HTML node', () => { + workflowPage.actions.addInitialNodeToCanvas('HTML', { + action: 'Generate HTML template', + keepNdvOpen: true, + }); + ndv.getters + .htmlEditorContainer() + .click() + .find('.cm-content') + .type(`{selectall}${TEST_ELEMENT_H1}`, { delay: TYPING_DELAY, force: true }) + .type('{esc}'); + ndv.actions.close(); + workflowPage.actions.openNode('HTML'); + ndv.getters.htmlEditorContainer().find('.cm-content').type(`{end}${TEST_ELEMENT_P}`, { delay: TYPING_DELAY, force: true }).type('{esc}'); + ndv.actions.close(); + workflowPage.actions.openNode('HTML'); + ndv.getters.htmlEditorContainer().should('contain', TEST_ELEMENT_H1); + ndv.getters.htmlEditorContainer().should('contain', TEST_ELEMENT_P); + }); + + it('should not trigger dirty flag if nothing is changed', () => { + workflowPage.actions.addInitialNodeToCanvas('HTML', { + action: 'Generate HTML template', + keepNdvOpen: true, + }); + ndv.actions.close(); + workflowPage.actions.saveWorkflowOnButtonClick(); + workflowPage.getters.isWorkflowSaved(); + workflowPage.actions.openNode('HTML'); + ndv.actions.close(); + // Workflow should still be saved + workflowPage.getters.isWorkflowSaved(); + }); + + it('should trigger dirty flag if query is updated', () => { + workflowPage.actions.addInitialNodeToCanvas('HTML', { + action: 'Generate HTML template', + keepNdvOpen: true, + }); + ndv.actions.close(); + workflowPage.actions.saveWorkflowOnButtonClick(); + workflowPage.getters.isWorkflowSaved(); + workflowPage.actions.openNode('HTML'); + ndv.getters + .htmlEditorContainer() + .click() + .find('.cm-content') + .type(`{selectall}${TEST_ELEMENT_H1}`, { delay: TYPING_DELAY, force: true }) + .type('{esc}'); + ndv.actions.close(); + workflowPage.getters.isWorkflowSaved().should('not.be.true'); + }); + }); +}); diff --git a/cypress/pages/ndv.ts b/cypress/pages/ndv.ts index 32cc4329b3..23a9882747 100644 --- a/cypress/pages/ndv.ts +++ b/cypress/pages/ndv.ts @@ -84,6 +84,7 @@ export class NDV extends BasePage { cy.getByTestId('columns-parameter-input-options-container'), resourceMapperRemoveAllFieldsOption: () => cy.getByTestId('action-removeAllFields'), sqlEditorContainer: () => cy.getByTestId('sql-editor-container'), + htmlEditorContainer: () => cy.getByTestId('html-editor-container'), filterComponent: (paramName: string) => cy.getByTestId(`filter-${paramName}`), filterCombinator: (paramName: string, index = 0) => this.getters.filterComponent(paramName).getByTestId('filter-combinator-select').eq(index), diff --git a/packages/editor-ui/src/components/HtmlEditor/HtmlEditor.vue b/packages/editor-ui/src/components/HtmlEditor/HtmlEditor.vue index 4f6e89526c..ca84e0c20f 100644 --- a/packages/editor-ui/src/components/HtmlEditor/HtmlEditor.vue +++ b/packages/editor-ui/src/components/HtmlEditor/HtmlEditor.vue @@ -1,6 +1,6 @@ @@ -244,7 +244,6 @@ onMounted(() => { onBeforeUnmount(() => { htmlEditorEventBus.off('format-html', formatHtml); - emit('update:model-value', readEditorValue()); }); diff --git a/packages/editor-ui/src/components/ParameterInput.vue b/packages/editor-ui/src/components/ParameterInput.vue index 2ae5d748c6..f6d6cde517 100644 --- a/packages/editor-ui/src/components/ParameterInput.vue +++ b/packages/editor-ui/src/components/ParameterInput.vue @@ -1189,6 +1189,13 @@ function valueChanged(value: NodeParameterValueType | {} | Date) { if (remoteParameterOptionsLoading.value) { return; } + // Only update the value if it has changed + const oldValue = node.value?.parameters + ? nodeHelpers.getParameterValue(node.value?.parameters, props.parameter.name, '') + : undefined; + if (oldValue !== undefined && oldValue === value) { + return; + } if (props.parameter.name === 'nodeCredentialType') { activeCredentialType.value = value as string; diff --git a/packages/editor-ui/src/components/SqlEditor/SqlEditor.vue b/packages/editor-ui/src/components/SqlEditor/SqlEditor.vue index 5c57108868..0a3890277a 100644 --- a/packages/editor-ui/src/components/SqlEditor/SqlEditor.vue +++ b/packages/editor-ui/src/components/SqlEditor/SqlEditor.vue @@ -163,7 +163,6 @@ onMounted(() => { onBeforeUnmount(() => { codeNodeEditorEventBus.off('error-line-number', highlightLine); - emit('update:model-value', readEditorValue()); }); function line(lineNumber: number): Line | null {