fix(editor): Fix SQL editor issue (#7236)

Github issue / Community forum post (link here to close automatically):
n8n-io · Sep 22, 2023 · 647fc6c · 647fc6c
1 parent e8e44f6
commit 647fc6c
Show file tree

Hide file tree

Showing 7 changed files with 167 additions and 165 deletions.
diff --git a/cypress/e2e/13-pinning.cy.ts b/cypress/e2e/13-pinning.cy.ts
@@ -1,9 +1,9 @@
-// import {
-// 	HTTP_REQUEST_NODE_NAME,
-// 	MANUAL_TRIGGER_NODE_NAME,
-// 	PIPEDRIVE_NODE_NAME,
-// 	EDIT_FIELDS_SET_NODE_NAME,
-// } from '../constants';
+import {
+	HTTP_REQUEST_NODE_NAME,
+	MANUAL_TRIGGER_NODE_NAME,
+	PIPEDRIVE_NODE_NAME,
+	EDIT_FIELDS_SET_NODE_NAME,
+} from '../constants';
 import { WorkflowPage, NDV } from '../pages';
 
 const workflowPage = new WorkflowPage();
@@ -69,35 +69,33 @@ describe('Data pinning', () => {
 		ndv.getters.outputTbodyCell(1, 0).should('include.text', 1);
 	});
 
-	//TODO: Update Edit Fields (Set) node to a new version
-	// it('Should be able to reference paired items in a node located before pinned data', () => {
-	// 	workflowPage.actions.addInitialNodeToCanvas(MANUAL_TRIGGER_NODE_NAME);
-	// 	workflowPage.actions.addNodeToCanvas(HTTP_REQUEST_NODE_NAME, true, true);
-	// 	ndv.actions.setPinnedData([{ http: 123 }]);
-	// 	ndv.actions.close();
+	it('Should be able to reference paired items in a node located before pinned data', () => {
+		workflowPage.actions.addInitialNodeToCanvas(MANUAL_TRIGGER_NODE_NAME);
+		workflowPage.actions.addNodeToCanvas(HTTP_REQUEST_NODE_NAME, true, true);
+		ndv.actions.setPinnedData([{ http: 123 }]);
+		ndv.actions.close();
 
-	// 	workflowPage.actions.addNodeToCanvas(PIPEDRIVE_NODE_NAME, true, true);
-	// 	ndv.actions.setPinnedData(Array(3).fill({ pipedrive: 123 }));
-	// 	ndv.actions.close();
+		workflowPage.actions.addNodeToCanvas(PIPEDRIVE_NODE_NAME, true, true);
+		ndv.actions.setPinnedData(Array(3).fill({ pipedrive: 123 }));
+		ndv.actions.close();
 
-	// 	workflowPage.actions.addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME, true, true);
-	// 	setExpressionOnStringValueInSet(`{{ $('${HTTP_REQUEST_NODE_NAME}').item`);
+		workflowPage.actions.addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME, true, true);
+		setExpressionOnStringValueInSet(`{{ $('${HTTP_REQUEST_NODE_NAME}').item`);
 
-	// 	const output = '[Object: {"json": {"http": 123}, "pairedItem": {"item": 0}}]';
+		const output = '[Object: {"json": {"http": 123}, "pairedItem": {"item": 0}}]';
 
-	// 	cy.get('div').contains(output).should('be.visible');
-	// });
+		cy.get('div').contains(output).should('be.visible');
+	});
 });
 
-// function setExpressionOnStringValueInSet(expression: string) {
-// 	cy.get('button').contains('Execute node').click();
-// 	cy.get('input[placeholder="Add Value"]').click();
-// 	cy.get('span').contains('String').click();
+function setExpressionOnStringValueInSet(expression: string) {
+	cy.get('button').contains('Execute node').click();
+	cy.get('.fixed-collection-parameter > :nth-child(2) > .button > span').click();
 
-// 	ndv.getters.nthParam(3).contains('Expression').invoke('show').click();
+	ndv.getters.nthParam(4).contains('Expression').invoke('show').click();
 
-// 	ndv.getters
-// 		.inlineExpressionEditorInput()
-// 		.clear()
-// 		.type(expression, { parseSpecialCharSequences: false });
-// }
+	ndv.getters
+		.inlineExpressionEditorInput()
+		.clear()
+		.type(expression, { parseSpecialCharSequences: false });
+}
diff --git a/cypress/e2e/16-webhook-node.cy.ts b/cypress/e2e/16-webhook-node.cy.ts
@@ -1,7 +1,7 @@
 import { WorkflowPage, NDV, CredentialsModal } from '../pages';
 import { v4 as uuid } from 'uuid';
-// import { cowBase64 } from '../support/binaryTestFiles';
-import { BACKEND_BASE_URL } from '../constants';
+import { cowBase64 } from '../support/binaryTestFiles';
+import { BACKEND_BASE_URL, EDIT_FIELDS_SET_NODE_NAME } from '../constants';
 import { getVisibleSelect } from '../utils';
 
 const workflowPage = new WorkflowPage();
@@ -102,39 +102,31 @@ describe('Webhook Trigger node', async () => {
 		simpleWebhookCall({ method: 'PUT', webhookPath: uuid(), executeNow: true });
 	});
 
-	//TODO: Update Edit Fields (Set) node to a new version
-	// it('should listen for a GET request and respond with Respond to Webhook node', () => {
-	// 	const webhookPath = uuid();
-	// 	simpleWebhookCall({
-	// 		method: 'GET',
-	// 		webhookPath,
-	// 		executeNow: false,
-	// 		respondWith: 'Respond to Webhook',
-	// 	});
-
-	// 	ndv.getters.backToCanvas().click();
-
-	// 	workflowPage.actions.addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME);
-	// 	workflowPage.actions.openNode(EDIT_FIELDS_SET_NODE_NAME);
-	// 	cy.get('.add-option').click();
-	// 	getVisibleSelect().find('.el-select-dropdown__item').contains('Number').click();
-	// 	cy.get('.fixed-collection-parameter')
-	// 		.getByTestId('parameter-input-name')
-	// 		.clear()
-	// 		.type('MyValue');
-	// 	cy.get('.fixed-collection-parameter').getByTestId('parameter-input-value').clear().type('1234');
-	// 	ndv.getters.backToCanvas().click({ force: true });
-
-	// 	workflowPage.actions.addNodeToCanvas('Respond to Webhook');
-
-	// 	workflowPage.actions.executeWorkflow();
-	// 	cy.wait(waitForWebhook);
-
-	// 	cy.request('GET', `${BACKEND_BASE_URL}/webhook-test/${webhookPath}`).then((response) => {
-	// 		expect(response.status).to.eq(200);
-	// 		expect(response.body.MyValue).to.eq(1234);
-	// 	});
-	// });
+	it('should listen for a GET request and respond with Respond to Webhook node', () => {
+		const webhookPath = uuid();
+		simpleWebhookCall({
+			method: 'GET',
+			webhookPath,
+			executeNow: false,
+			respondWith: 'Respond to Webhook',
+		});
+
+		ndv.getters.backToCanvas().click();
+
+		addEditFields();
+
+		ndv.getters.backToCanvas().click({ force: true });
+
+		workflowPage.actions.addNodeToCanvas('Respond to Webhook');
+
+		workflowPage.actions.executeWorkflow();
+		cy.wait(waitForWebhook);
+
+		cy.request('GET', `${BACKEND_BASE_URL}/webhook-test/${webhookPath}`).then((response) => {
+			expect(response.status).to.eq(200);
+			expect(response.body.MyValue).to.eq(1234);
+		});
+	});
 
 	it('should listen for a GET request and respond custom status code 201', () => {
 		const webhookPath = uuid();
@@ -153,83 +145,64 @@ describe('Webhook Trigger node', async () => {
 		});
 	});
 
-	//TODO: Update Edit Fields (Set) node to a new version
-	// it('should listen for a GET request and respond with last node', () => {
-	// 	const webhookPath = uuid();
-	// 	simpleWebhookCall({
-	// 		method: 'GET',
-	// 		webhookPath,
-	// 		executeNow: false,
-	// 		respondWith: 'Last Node',
-	// 	});
-	// 	ndv.getters.backToCanvas().click();
-
-	// 	workflowPage.actions.addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME);
-	// 	workflowPage.actions.openNode(EDIT_FIELDS_SET_NODE_NAME);
-	// 	cy.get('.add-option').click();
-	// 	getVisibleSelect().find('.el-select-dropdown__item').contains('Number').click();
-	// 	cy.get('.fixed-collection-parameter')
-	// 		.getByTestId('parameter-input-name')
-	// 		.find('input')
-	// 		.clear()
-	// 		.type('MyValue');
-	// 	cy.get('.fixed-collection-parameter')
-	// 		.getByTestId('parameter-input-value')
-	// 		.find('input')
-	// 		.clear()
-	// 		.type('1234');
-	// 	ndv.getters.backToCanvas().click({ force: true });
-
-	// 	workflowPage.actions.executeWorkflow();
-	// 	cy.wait(waitForWebhook);
-
-	// 	cy.request('GET', `${BACKEND_BASE_URL}/webhook-test/${webhookPath}`).then((response) => {
-	// 		expect(response.status).to.eq(200);
-	// 		expect(response.body.MyValue).to.eq(1234);
-	// 	});
-	// });
-
-	//TODO: Update Edit Fields (Set) node to a new version
-	// it('should listen for a GET request and respond with last node binary data', () => {
-	// 	const webhookPath = uuid();
-	// 	simpleWebhookCall({
-	// 		method: 'GET',
-	// 		webhookPath,
-	// 		executeNow: false,
-	// 		respondWith: 'Last Node',
-	// 		responseData: 'First Entry Binary',
-	// 	});
-	// 	ndv.getters.backToCanvas().click();
-
-	// 	workflowPage.actions.addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME);
-	// 	workflowPage.actions.openNode(EDIT_FIELDS_SET_NODE_NAME);
-	// 	cy.get('.add-option').click();
-	// 	getVisibleSelect().find('.el-select-dropdown__item').contains('String').click();
-	// 	cy.get('.fixed-collection-parameter').getByTestId('parameter-input-name').clear().type('data');
-	// 	cy.get('.fixed-collection-parameter')
-	// 		.getByTestId('parameter-input-value')
-	// 		.clear()
-	// 		.find('input')
-	// 		.invoke('val', cowBase64)
-	// 		.trigger('blur');
-	// 	ndv.getters.backToCanvas().click();
-
-	// 	workflowPage.actions.addNodeToCanvas('Move Binary Data');
-	// 	workflowPage.actions.zoomToFit();
-
-	// 	workflowPage.actions.openNode('Move Binary Data');
-	// 	cy.getByTestId('parameter-input-mode').click();
-	// 	getVisibleSelect().find('.option-headline').contains('JSON to Binary').click();
-	// 	ndv.getters.backToCanvas().click();
-
-	// 	workflowPage.actions.executeWorkflow();
-	// 	cy.wait(waitForWebhook);
-
-	// 	cy.request('GET', `${BACKEND_BASE_URL}/webhook-test/${webhookPath}`).then((response) => {
-	// 		expect(response.status).to.eq(200);
-	// 		expect(Object.keys(response.body).includes('data')).to.be.true;
-	// 	});
-	// });
+	it('should listen for a GET request and respond with last node', () => {
+		const webhookPath = uuid();
+		simpleWebhookCall({
+			method: 'GET',
+			webhookPath,
+			executeNow: false,
+			respondWith: 'Last Node',
+		});
+		ndv.getters.backToCanvas().click();
+
+		addEditFields();
+
+		ndv.getters.backToCanvas().click({ force: true });
+
+		workflowPage.actions.executeWorkflow();
+		cy.wait(waitForWebhook);
+
+		cy.request('GET', `${BACKEND_BASE_URL}/webhook-test/${webhookPath}`).then((response) => {
+			expect(response.status).to.eq(200);
+			expect(response.body.MyValue).to.eq(1234);
+		});
+	});
+
+	it('should listen for a GET request and respond with last node binary data', () => {
+		const webhookPath = uuid();
+		simpleWebhookCall({
+			method: 'GET',
+			webhookPath,
+			executeNow: false,
+			respondWith: 'Last Node',
+			responseData: 'First Entry Binary',
+		});
+		ndv.getters.backToCanvas().click();
+
+		workflowPage.actions.addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME);
+		workflowPage.actions.openNode(EDIT_FIELDS_SET_NODE_NAME);
+		cy.get('.fixed-collection-parameter > :nth-child(2) > .button > span').click();
+		ndv.getters.nthParam(2).type('data');
+		ndv.getters.nthParam(4).invoke('val', cowBase64).trigger('blur');
+
+		ndv.getters.backToCanvas().click();
+
+		workflowPage.actions.addNodeToCanvas('Move Binary Data');
+		workflowPage.actions.zoomToFit();
+
+		workflowPage.actions.openNode('Move Binary Data');
+		cy.getByTestId('parameter-input-mode').click();
+		getVisibleSelect().find('.option-headline').contains('JSON to Binary').click();
+		ndv.getters.backToCanvas().click();
+
+		workflowPage.actions.executeWorkflow();
+		cy.wait(waitForWebhook);
+
+		cy.request('GET', `${BACKEND_BASE_URL}/webhook-test/${webhookPath}`).then((response) => {
+			expect(response.status).to.eq(200);
+			expect(Object.keys(response.body).includes('data')).to.be.true;
+		});
+	});
 
 	it('should listen for a GET request and respond with an empty body', () => {
 		const webhookPath = uuid();
@@ -332,3 +305,13 @@ describe('Webhook Trigger node', async () => {
 			});
 	});
 });
+
+const addEditFields = () => {
+	workflowPage.actions.addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME);
+	workflowPage.actions.openNode(EDIT_FIELDS_SET_NODE_NAME);
+	cy.get('.fixed-collection-parameter > :nth-child(2) > .button > span').click();
+	ndv.getters.nthParam(2).type('MyValue');
+	ndv.getters.nthParam(3).click();
+	cy.get('div').contains('Number').click();
+	ndv.getters.nthParam(4).type('1234');
+};
diff --git a/cypress/e2e/29-sql-editor.cy.ts b/cypress/e2e/29-sql-editor.cy.ts
@@ -0,0 +1,26 @@
+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('Manual');
+		workflowPage.actions.addNodeToCanvas('Postgres');
+		workflowPage.actions.openNode('Postgres');
+		ndv.getters.parameterInput('operation').click();
+		cy.get('div').contains('Execute Query').click();
+		cy.get('div.cm-activeLine').type('SELECT * FROM `testTable`');
+		ndv.actions.close();
+		workflowPage.actions.openNode('Postgres');
+		cy.get('div.cm-activeLine').type('{end} LIMIT 10');
+		ndv.actions.close();
+		workflowPage.actions.openNode('Postgres');
+
+		cy.get('div.cm-activeLine').contains('SELECT * FROM `testTable` LIMIT 10');
+	});
+});
diff --git a/packages/editor-ui/src/components/CodeNodeEditor/CodeNodeEditor.vue b/packages/editor-ui/src/components/CodeNodeEditor/CodeNodeEditor.vue
@@ -320,7 +320,7 @@ export default defineComponent({
 			...readOnlyEditorExtensions,
 			EditorState.readOnly.of(isReadOnly),
 			EditorView.editable.of(!isReadOnly),
-			codeNodeEditorTheme({ isReadOnly }),
+			codeNodeEditorTheme({ isReadOnly, customMinHeight: this.rows }),
 		];
 
 		if (!isReadOnly) {
@@ -354,16 +354,8 @@ export default defineComponent({
 		const [languageSupport, ...otherExtensions] = this.languageExtensions;
 		extensions.push(this.languageCompartment.of(languageSupport), ...otherExtensions);
 
-		let doc = this.modelValue ?? this.placeholder;
-
-		const lines = doc.split('\n');
-
-		if (lines.length < this.rows) {
-			doc += '\n'.repeat(this.rows - lines.length);
-		}
-
 		const state = EditorState.create({
-			doc,
+			doc: this.modelValue ?? this.placeholder,
 			extensions,
 		});
 

diff --git a/packages/editor-ui/src/components/CodeNodeEditor/theme.ts b/packages/editor-ui/src/components/CodeNodeEditor/theme.ts
@@ -32,9 +32,14 @@ const cssStyleDeclaration = getComputedStyle(document.documentElement);
 interface ThemeSettings {
 	isReadOnly?: boolean;
 	customMaxHeight?: string;
+	customMinHeight?: number;
 }
 
-export const codeNodeEditorTheme = ({ isReadOnly, customMaxHeight }: ThemeSettings) => [
+export const codeNodeEditorTheme = ({
+	isReadOnly,
+	customMaxHeight,
+	customMinHeight,
+}: ThemeSettings) => [
 	EditorView.theme({
 		'&': {
 			'font-size': BASE_STYLING.fontSize,
@@ -82,7 +87,9 @@ export const codeNodeEditorTheme = ({ isReadOnly, customMaxHeight }: ThemeSettin
 			overflow: 'auto',
 
 			maxHeight: customMaxHeight ?? '100%',
-			...(isReadOnly ? {} : { minHeight: '1.3em' }),
+			...(isReadOnly
+				? {}
+				: { minHeight: customMinHeight ? `${Number(customMinHeight) * 1.3}em` : '10em' }),
 		},
 		'.cm-diagnosticAction': {
 			backgroundColor: BASE_STYLING.diagnosticButton.backgroundColor,

diff --git a/packages/editor-ui/src/components/ParameterInput.vue b/packages/editor-ui/src/components/ParameterInput.vue
@@ -120,7 +120,7 @@
 					:dialect="getArgument('sqlDialect')"
 					:isReadOnly="isReadOnly"
 					:rows="getArgument('rows')"
-					@valueChanged="valueChangedDebounced"
+					@update:modelValue="valueChangedDebounced"
 				/>
 
 				<code-node-editor