From 054fe9745ff6864f9088aa4cd66ed9e7869520d5 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour <4711238+mutdmour@users.noreply.github.com> Date: Mon, 21 Oct 2024 13:34:57 +0200 Subject: [PATCH] fix: Ensure NDV params don't get cut off early and scrolled to the top (#11252) --- .../2372-ado-prevent-clipping-params.cy.ts | 86 ++++++++++ cypress/e2e/6-code-node.cy.ts | 24 +-- .../Test-workflow-with-long-parameters.json | 150 ++++++++++++++++++ cypress/pages/ndv.ts | 1 + .../src/components/NDVSubConnections.spec.ts | 126 +++++++++++++++ .../src/components/NDVSubConnections.vue | 2 +- .../src/composables/useExpressionEditor.ts | 2 +- 7 files changed, 377 insertions(+), 14 deletions(-) create mode 100644 cypress/e2e/2372-ado-prevent-clipping-params.cy.ts create mode 100644 cypress/fixtures/Test-workflow-with-long-parameters.json create mode 100644 packages/editor-ui/src/components/NDVSubConnections.spec.ts diff --git a/cypress/e2e/2372-ado-prevent-clipping-params.cy.ts b/cypress/e2e/2372-ado-prevent-clipping-params.cy.ts new file mode 100644 index 0000000000..260c6e48c9 --- /dev/null +++ b/cypress/e2e/2372-ado-prevent-clipping-params.cy.ts @@ -0,0 +1,86 @@ +import { NDV, WorkflowPage } from '../pages'; + +const workflowPage = new WorkflowPage(); +const ndv = new NDV(); + +describe('ADO-2362 ADO-2350 NDV Prevent clipping long parameters and scrolling to expression', () => { + it('should show last parameters and open at scroll top of parameters', () => { + workflowPage.actions.visit(); + cy.createFixtureWorkflow('Test-workflow-with-long-parameters.json'); + workflowPage.actions.openNode('Schedule Trigger'); + + ndv.getters.inlineExpressionEditorInput().should('be.visible'); + + ndv.actions.close(); + + workflowPage.actions.openNode('Edit Fields1'); + + // first parameter should be visible + ndv.getters.inputLabel().eq(0).should('include.text', 'Mode'); + ndv.getters.inputLabel().eq(0).should('be.visible'); + + ndv.getters.inlineExpressionEditorInput().should('have.length', 2); + + // last parameter in view should be visible + ndv.getters.inlineExpressionEditorInput().eq(0).should('have.text', 'should be visible!'); + ndv.getters.inlineExpressionEditorInput().eq(0).should('be.visible'); + + // next parameter in view should not be visible + ndv.getters.inlineExpressionEditorInput().eq(1).should('have.text', 'not visible'); + ndv.getters.inlineExpressionEditorInput().eq(1).should('not.be.visible'); + + ndv.actions.close(); + workflowPage.actions.openNode('Schedule Trigger'); + + // first parameter (notice) should be visible + ndv.getters.nthParam(0).should('include.text', 'This workflow will run on the schedule '); + ndv.getters.inputLabel().eq(0).should('be.visible'); + + ndv.getters.inlineExpressionEditorInput().should('have.length', 2); + + // last parameter in view should be visible + ndv.getters.inlineExpressionEditorInput().eq(0).should('have.text', 'should be visible'); + ndv.getters.inlineExpressionEditorInput().eq(0).should('be.visible'); + + // next parameter in view should not be visible + ndv.getters.inlineExpressionEditorInput().eq(1).should('have.text', 'not visible'); + ndv.getters.inlineExpressionEditorInput().eq(1).should('not.be.visible'); + + ndv.actions.close(); + workflowPage.actions.openNode('Slack'); + + // first field (credentials) should be visible + ndv.getters.nodeCredentialsLabel().should('be.visible'); + + // last parameter in view should be visible + ndv.getters.inlineExpressionEditorInput().eq(0).should('have.text', 'should be visible'); + ndv.getters.inlineExpressionEditorInput().eq(0).should('be.visible'); + + // next parameter in view should not be visible + ndv.getters.inlineExpressionEditorInput().eq(1).should('have.text', 'not visible'); + ndv.getters.inlineExpressionEditorInput().eq(1).should('not.be.visible'); + }); + + it('NODE-1272 ensure expressions scrolled to top, not middle', () => { + workflowPage.actions.visit(); + cy.createFixtureWorkflow('Test-workflow-with-long-parameters.json'); + workflowPage.actions.openNode('With long expression'); + + ndv.getters.inlineExpressionEditorInput().eq(0).should('be.visible'); + // should be scrolled at top + ndv.getters + .inlineExpressionEditorInput() + .eq(0) + .find('.cm-line') + .eq(0) + .should('have.text', '1 visible!'); + ndv.getters.inlineExpressionEditorInput().eq(0).find('.cm-line').eq(0).should('be.visible'); + ndv.getters + .inlineExpressionEditorInput() + .eq(0) + .find('.cm-line') + .eq(6) + .should('have.text', '7 not visible!'); + ndv.getters.inlineExpressionEditorInput().eq(0).find('.cm-line').eq(6).should('not.be.visible'); + }); +}); diff --git a/cypress/e2e/6-code-node.cy.ts b/cypress/e2e/6-code-node.cy.ts index 5a6182c25a..5bc7d05ee2 100644 --- a/cypress/e2e/6-code-node.cy.ts +++ b/cypress/e2e/6-code-node.cy.ts @@ -162,21 +162,21 @@ return [] cy.get('#tab-code').should('have.class', 'is-active'); }); - it('should show error based on status code', () => { - const prompt = nanoid(20); - cy.get('#tab-ask-ai').click(); - ndv.actions.executePrevious(); + const handledCodes = [ + { code: 400, message: 'Code generation failed due to an unknown reason' }, + { code: 413, message: 'Your workflow data is too large for AI to process' }, + { code: 429, message: "We've hit our rate limit with our AI partner" }, + { code: 500, message: 'Code generation failed due to an unknown reason' }, + ]; - cy.getByTestId('ask-ai-prompt-input').type(prompt); + handledCodes.forEach(({ code, message }) => { + it(`should show error based on status code ${code}`, () => { + const prompt = nanoid(20); + cy.get('#tab-ask-ai').click(); + ndv.actions.executePrevious(); - const handledCodes = [ - { code: 400, message: 'Code generation failed due to an unknown reason' }, - { code: 413, message: 'Your workflow data is too large for AI to process' }, - { code: 429, message: "We've hit our rate limit with our AI partner" }, - { code: 500, message: 'Code generation failed due to an unknown reason' }, - ]; + cy.getByTestId('ask-ai-prompt-input').type(prompt); - handledCodes.forEach(({ code, message }) => { cy.intercept('POST', '/rest/ai/ask-ai', { statusCode: code, status: code, diff --git a/cypress/fixtures/Test-workflow-with-long-parameters.json b/cypress/fixtures/Test-workflow-with-long-parameters.json new file mode 100644 index 0000000000..d4d052f6f0 --- /dev/null +++ b/cypress/fixtures/Test-workflow-with-long-parameters.json @@ -0,0 +1,150 @@ +{ + "meta": { + "instanceId": "777c68374367604fdf2a0bcfe9b1b574575ddea61aa8268e4bf034434bd7c894" + }, + "nodes": [ + { + "parameters": { + "assignments": { + "assignments": [ + { + "id": "0effebfc-fa8c-4d41-8a37-6d5695dfc9ee", + "name": "test", + "value": "test", + "type": "string" + }, + { + "id": "beb8723f-6333-4186-ab88-41d4e2338866", + "name": "test", + "value": "test", + "type": "string" + }, + { + "id": "85095836-4e94-442f-9270-e1a89008c129", + "name": "test", + "value": "test", + "type": "string" + }, + { + "id": "b6163f8a-bca6-4364-8b38-182df37c55cd", + "name": "=should be visible!", + "value": "=not visible", + "type": "string" + } + ] + }, + "options": {} + }, + "id": "950fcdc1-9e92-410f-8377-d4240e9bf6ff", + "name": "Edit Fields1", + "type": "n8n-nodes-base.set", + "typeVersion": 3.4, + "position": [ + 680, + 460 + ] + }, + { + "parameters": { + "messageType": "block", + "blocksUi": "blocks", + "text": "=should be visible", + "otherOptions": { + "sendAsUser": "=not visible" + } + }, + "id": "dcf7410d-0f8e-4cdb-9819-ae275558bdaa", + "name": "Slack", + "type": "n8n-nodes-base.slack", + "typeVersion": 2.2, + "position": [ + 900, + 460 + ], + "webhookId": "002b502e-31e5-4fdb-ac43-a56cfde8f82a" + }, + { + "parameters": { + "rule": { + "interval": [ + {}, + { + "field": "=should be visible" + }, + { + "field": "=not visible" + } + ] + } + }, + "id": "4c948a3f-19d4-4b08-a8be-f7d2964a21f4", + "name": "Schedule Trigger", + "type": "n8n-nodes-base.scheduleTrigger", + "typeVersion": 1.2, + "position": [ + 460, + 460 + ] + }, + { + "parameters": { + "assignments": { + "assignments": [ + { + "id": "5dcaab37-1146-49c6-97a3-3b2f73483270", + "name": "object", + "value": "=1 visible!\n2 {\n3 \"str\": \"two\",\n4 \"str_date\": \"{{ $now }}\",\n5 \"str_int\": \"1\",\n6 \"str_float\": \"1.234\",\n7 not visible!\n \"str_bool\": \"true\",\n \"str_email\": \"david@thedavid.com\",\n \"str_with_email\":\"My email is david@n8n.io\",\n \"str_json_single\":\"{'one':'two'}\",\n \"str_json_double\":\"{\\\"one\\\":\\\"two\\\"}\",\n \"bool\": true,\n \"list\": [1, 2, 3],\n \"decimal\": 1.234,\n \"timestamp1\": 1708695471,\n \"timestamp2\": 1708695471000,\n \"timestamp3\": 1708695471000000,\n \"num_one\": 1\n}", + "type": "object" + } + ] + }, + "includeOtherFields": true, + "options": {} + }, + "id": "a41dfb0d-38aa-42d2-b3e2-1854090bd319", + "name": "With long expression", + "type": "n8n-nodes-base.set", + "typeVersion": 3.3, + "position": [ + 1100, + 460 + ] + } + ], + "connections": { + "Edit Fields1": { + "main": [ + [ + { + "node": "Slack", + "type": "main", + "index": 0 + } + ] + ] + }, + "Slack": { + "main": [ + [ + { + "node": "With long expression", + "type": "main", + "index": 0 + } + ] + ] + }, + "Schedule Trigger": { + "main": [ + [ + { + "node": "Edit Fields1", + "type": "main", + "index": 0 + } + ] + ] + } + }, + "pinData": {} +} diff --git a/cypress/pages/ndv.ts b/cypress/pages/ndv.ts index ab2a88896c..4504552e26 100644 --- a/cypress/pages/ndv.ts +++ b/cypress/pages/ndv.ts @@ -64,6 +64,7 @@ export class NDV extends BasePage { nodeRenameInput: () => cy.getByTestId('node-rename-input'), executePrevious: () => cy.getByTestId('execute-previous-node'), httpRequestNotice: () => cy.getByTestId('node-parameters-http-notice'), + nodeCredentialsLabel: () => cy.getByTestId('credentials-label'), nthParam: (n: number) => cy.getByTestId('node-parameters').find('.parameter-item').eq(n), inputRunSelector: () => this.getters.inputPanel().findChildByTestId('run-selector'), inputLinkRun: () => this.getters.inputPanel().findChildByTestId('link-run'), diff --git a/packages/editor-ui/src/components/NDVSubConnections.spec.ts b/packages/editor-ui/src/components/NDVSubConnections.spec.ts new file mode 100644 index 0000000000..c269e8e7f1 --- /dev/null +++ b/packages/editor-ui/src/components/NDVSubConnections.spec.ts @@ -0,0 +1,126 @@ +import { render, waitFor } from '@testing-library/vue'; +import NDVSubConnections from '@/components/NDVSubConnections.vue'; +import { setActivePinia } from 'pinia'; +import { createTestingPinia } from '@pinia/testing'; +import type { INodeUi } from '@/Interface'; +import type { INodeTypeDescription, WorkflowParameters } from 'n8n-workflow'; +import { NodeConnectionType, Workflow } from 'n8n-workflow'; + +const nodeType: INodeTypeDescription = { + displayName: 'OpenAI', + name: '@n8n/n8n-nodes-langchain.openAi', + version: [1], + inputs: [ + { type: NodeConnectionType.Main }, + { type: NodeConnectionType.AiTool, displayName: 'Tools' }, + ], + outputs: [NodeConnectionType.Main], + credentials: [ + { + name: 'openAiApi', + required: true, + }, + ], + properties: [], + defaults: { color: '', name: '' }, + group: [], + description: '', +}; + +const node: INodeUi = { + parameters: { + resource: 'assistant', + assistantId: { + __rl: true, + mode: 'list', + value: '', + }, + options: {}, + }, + id: 'f30c2cbc-c1b1-4014-87f7-22e6ae7afcc8', + name: 'OpenAI', + type: '@n8n/n8n-nodes-langchain.openAi', + typeVersion: 1.6, + position: [1300, 540], +}; + +const workflow: WorkflowParameters = { + nodes: [node], + connections: {}, + pinData: {}, + active: false, + nodeTypes: { + getByName: vi.fn(), + getByNameAndVersion: vi.fn(), + getKnownTypes: vi.fn(), + }, +}; + +const getNodeType = vi.fn(); + +vi.mock('@/stores/nodeTypes.store', () => ({ + useNodeTypesStore: vi.fn(() => ({ + getNodeType, + })), +})); + +vi.mock('@/stores/workflows.store', () => ({ + useWorkflowsStore: vi.fn(() => ({ + getCurrentWorkflow: vi.fn(() => new Workflow(workflow)), + getNodeByName: vi.fn(() => node), + })), +})); + +describe('NDVSubConnections', () => { + beforeAll(() => { + vi.useFakeTimers(); + setActivePinia(createTestingPinia()); + vi.restoreAllMocks(); + }); + + it('should render container if possible connections', async () => { + getNodeType.mockReturnValue(nodeType); + const { getByTestId, html } = render(NDVSubConnections, { + props: { + rootNode: node, + }, + }); + vi.advanceTimersByTime(1000); // Event debounce time + + await waitFor(() => {}); + expect(getByTestId('subnode-connection-group-ai_tool')).toBeVisible(); + expect(html()).toEqual( + `
+
+
+
Tools +
+
+
+ + + +
+ +
+
+
+
+
+
`, + ); + }); + + it('should not render container if no possible connections', async () => { + getNodeType.mockReturnValue(null); + const component = render(NDVSubConnections, { + props: { + rootNode: node, + }, + }); + vi.advanceTimersByTime(1000); // Event debounce time + + await waitFor(() => {}); + expect(component.html()).toEqual(''); + }); +}); diff --git a/packages/editor-ui/src/components/NDVSubConnections.vue b/packages/editor-ui/src/components/NDVSubConnections.vue index ed1fb2f228..2ceaa11b5d 100644 --- a/packages/editor-ui/src/components/NDVSubConnections.vue +++ b/packages/editor-ui/src/components/NDVSubConnections.vue @@ -191,7 +191,7 @@ defineExpose({