From 652b8d170b9624d47b5f2d8d679c165cc14ea548 Mon Sep 17 00:00:00 2001 From: Michael Kret <88898367+michael-radency@users.noreply.github.com> Date: Mon, 20 Jan 2025 11:41:50 +0200 Subject: [PATCH 001/105] fix(Wait Node): Fix for hasNextPage in waiting forms (#12636) --- .../node-execution-context.ts | 12 ++++-- .../nodes-base/nodes/Form/test/utils.test.ts | 42 ++++++++++++++++++- packages/nodes-base/nodes/Form/utils.ts | 15 +++++-- packages/workflow/src/Interfaces.ts | 6 ++- 4 files changed, 66 insertions(+), 9 deletions(-) diff --git a/packages/core/src/execution-engine/node-execution-context/node-execution-context.ts b/packages/core/src/execution-engine/node-execution-context/node-execution-context.ts index f5f3730244..79225554d6 100644 --- a/packages/core/src/execution-engine/node-execution-context/node-execution-context.ts +++ b/packages/core/src/execution-engine/node-execution-context/node-execution-context.ts @@ -79,18 +79,24 @@ export abstract class NodeExecutionContext implements Omit { beforeEach(() => { @@ -721,3 +728,36 @@ describe('resolveRawData', () => { ); }); }); + +describe('FormTrigger, isFormConnected', () => { + it('should return false if Wait node is connected but resume parameter is not form', async () => { + const result = isFormConnected([ + mock({ + type: 'n8n-nodes-base.wait', + parameters: { + resume: 'timeInterval', + }, + }), + ]); + expect(result).toBe(false); + }); + it('should return true if Wait node is connected and resume parameter is form', async () => { + const result = isFormConnected([ + mock({ + type: 'n8n-nodes-base.wait', + parameters: { + resume: 'form', + }, + }), + ]); + expect(result).toBe(true); + }); + it('should return true if Form node is connected', async () => { + const result = isFormConnected([ + mock({ + type: 'n8n-nodes-base.form', + }), + ]); + expect(result).toBe(true); + }); +}); diff --git a/packages/nodes-base/nodes/Form/utils.ts b/packages/nodes-base/nodes/Form/utils.ts index 87dda98f88..6fc414f622 100644 --- a/packages/nodes-base/nodes/Form/utils.ts +++ b/packages/nodes-base/nodes/Form/utils.ts @@ -326,6 +326,13 @@ export function renderForm({ res.render('form-trigger', data); } +export const isFormConnected = (nodes: NodeTypeAndVersion[]) => { + return nodes.some( + (n) => + n.type === FORM_NODE_TYPE || (n.type === WAIT_NODE_TYPE && n.parameters?.resume === 'form'), + ); +}; + export async function formWebhook( context: IWebhookFunctions, authProperty = FORM_TRIGGER_AUTHENTICATION_PROPERTY, @@ -403,10 +410,10 @@ export async function formWebhook( } if (!redirectUrl && node.type !== FORM_TRIGGER_NODE_TYPE) { - const connectedNodes = context.getChildNodes(context.getNode().name); - const hasNextPage = connectedNodes.some( - (n) => n.type === FORM_NODE_TYPE || n.type === WAIT_NODE_TYPE, - ); + const connectedNodes = context.getChildNodes(context.getNode().name, { + includeNodeParameters: true, + }); + const hasNextPage = isFormConnected(connectedNodes); if (hasNextPage) { redirectUrl = context.evaluateExpression('{{ $execution.resumeFormUrl }}') as string; diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 2161569b7d..10fc8e9b11 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -852,6 +852,7 @@ export type NodeTypeAndVersion = { type: string; typeVersion: number; disabled: boolean; + parameters?: INodeParameters; }; export interface FunctionsBase { @@ -869,7 +870,10 @@ export interface FunctionsBase { getRestApiUrl(): string; getInstanceBaseUrl(): string; getInstanceId(): string; - getChildNodes(nodeName: string): NodeTypeAndVersion[]; + getChildNodes( + nodeName: string, + options?: { includeNodeParameters?: boolean }, + ): NodeTypeAndVersion[]; getParentNodes(nodeName: string): NodeTypeAndVersion[]; getKnownNodeTypes(): IDataObject; getMode?: () => WorkflowExecuteMode; From 1eeb788d327287d21eab7ad6f2156453ab7642c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20G=C3=B3mez=20Morales?= Date: Mon, 20 Jan 2025 10:59:15 +0100 Subject: [PATCH 002/105] feat(editor): VariablesView Reskin - Add Filters for missing values (#12611) --- cypress/e2e/23-variables.cy.ts | 7 +- cypress/pages/variables.ts | 5 +- .../src/components/N8nActionBox/ActionBox.vue | 4 +- .../src/components/N8nDatatable/Datatable.vue | 95 ++-- .../__snapshots__/Datatable.test.ts.snap | 146 ++--- .../src/components/N8nFormInput/FormInput.vue | 3 + .../src/components/VariablesForm.vue | 132 +++++ .../src/components/VariablesRow.test.ts | 106 ---- .../editor-ui/src/components/VariablesRow.vue | 278 ---------- .../components/VariablesUsageBadge.test.ts | 31 ++ .../src/components/VariablesUsageBadge.vue | 44 ++ .../components/layouts/PageViewLayoutList.vue | 6 +- .../layouts/ResourcesListLayout.vue | 10 +- .../src/plugins/i18n/locales/en.json | 2 + .../editor-ui/src/views/VariablesView.test.ts | 285 ++++++++-- .../editor-ui/src/views/VariablesView.vue | 519 ++++++++---------- 16 files changed, 832 insertions(+), 841 deletions(-) create mode 100644 packages/editor-ui/src/components/VariablesForm.vue delete mode 100644 packages/editor-ui/src/components/VariablesRow.test.ts delete mode 100644 packages/editor-ui/src/components/VariablesRow.vue create mode 100644 packages/editor-ui/src/components/VariablesUsageBadge.test.ts create mode 100644 packages/editor-ui/src/components/VariablesUsageBadge.vue diff --git a/cypress/e2e/23-variables.cy.ts b/cypress/e2e/23-variables.cy.ts index c481f25128..a0af3c09ac 100644 --- a/cypress/e2e/23-variables.cy.ts +++ b/cypress/e2e/23-variables.cy.ts @@ -65,8 +65,11 @@ describe('Variables', () => { const editingRow = variablesPage.getters.variablesEditableRows().eq(0); variablesPage.actions.setRowValue(editingRow, 'key', key); variablesPage.actions.setRowValue(editingRow, 'value', value); - editingRow.should('contain', 'This field may contain only letters'); - variablesPage.getters.editableRowSaveButton(editingRow).should('be.disabled'); + variablesPage.actions.saveRowEditing(editingRow); + variablesPage.getters + .variablesEditableRows() + .eq(0) + .should('contain', 'This field may contain only letters'); variablesPage.actions.cancelRowEditing(editingRow); variablesPage.getters.variablesRows().should('have.length', 3); diff --git a/cypress/pages/variables.ts b/cypress/pages/variables.ts index 5fb0a64d9a..1bf344cbfc 100644 --- a/cypress/pages/variables.ts +++ b/cypress/pages/variables.ts @@ -68,7 +68,10 @@ export class VariablesPage extends BasePage { }, setRowValue: (row: Chainable>, field: 'key' | 'value', value: string) => { row.within(() => { - cy.getByTestId(`variable-row-${field}-input`).type('{selectAll}{del}').type(value); + cy.getByTestId(`variable-row-${field}-input`) + .find('input, textarea') + .type('{selectAll}{del}') + .type(value); }); }, cancelRowEditing: (row: Chainable>) => { diff --git a/packages/design-system/src/components/N8nActionBox/ActionBox.vue b/packages/design-system/src/components/N8nActionBox/ActionBox.vue index 02de38143c..b04ccb1049 100644 --- a/packages/design-system/src/components/N8nActionBox/ActionBox.vue +++ b/packages/design-system/src/components/N8nActionBox/ActionBox.vue @@ -10,8 +10,8 @@ import N8nText from '../N8nText'; interface ActionBoxProps { emoji: string; heading: string; - buttonText: string; - buttonType: ButtonType; + buttonText?: string; + buttonType?: ButtonType; buttonDisabled?: boolean; buttonIcon?: string; description: string; diff --git a/packages/design-system/src/components/N8nDatatable/Datatable.vue b/packages/design-system/src/components/N8nDatatable/Datatable.vue index 905aef4671..d3447229d1 100644 --- a/packages/design-system/src/components/N8nDatatable/Datatable.vue +++ b/packages/design-system/src/components/N8nDatatable/Datatable.vue @@ -1,5 +1,5 @@ - diff --git a/packages/editor-ui/src/components/VariablesRow.test.ts b/packages/editor-ui/src/components/VariablesRow.test.ts deleted file mode 100644 index 7d7013054f..0000000000 --- a/packages/editor-ui/src/components/VariablesRow.test.ts +++ /dev/null @@ -1,106 +0,0 @@ -import VariablesRow from './VariablesRow.vue'; -import { fireEvent } from '@testing-library/vue'; -import { setupServer } from '@/__tests__/server'; -import { afterAll, beforeAll } from 'vitest'; -import { useSettingsStore } from '@/stores/settings.store'; -import { useUsersStore } from '@/stores/users.store'; -import { createComponentRenderer } from '@/__tests__/render'; -import { createTestingPinia } from '@pinia/testing'; -import { STORES } from '@/constants'; - -const renderComponent = createComponentRenderer(VariablesRow, { - pinia: createTestingPinia({ - initialState: { - [STORES.SETTINGS]: { - settings: { - enterprise: { - variables: true, - }, - }, - }, - }, - }), - global: { - stubs: ['n8n-tooltip'], - }, -}); - -describe('VariablesRow', () => { - let server: ReturnType; - - beforeAll(() => { - server = setupServer(); - }); - - beforeEach(async () => { - await useSettingsStore().getSettings(); - await useUsersStore().loginWithCookie(); - }); - - afterAll(() => { - server.shutdown(); - }); - - const environmentVariable = { - id: '1', - name: 'key', - value: 'value', - }; - - it('should render correctly', () => { - const wrapper = renderComponent({ - props: { - data: environmentVariable, - }, - }); - - expect(wrapper.container.querySelectorAll('td')).toHaveLength(4); - }); - - it('should show edit and delete buttons on hover', async () => { - const wrapper = renderComponent({ - props: { - data: environmentVariable, - }, - }); - - await fireEvent.mouseEnter(wrapper.container); - - expect(wrapper.getByTestId('variable-row-edit-button')).toBeVisible(); - expect(wrapper.getByTestId('variable-row-delete-button')).toBeVisible(); - }); - - it('should show key and value inputs in edit mode', async () => { - const wrapper = renderComponent({ - props: { - data: environmentVariable, - editing: true, - }, - }); - - await fireEvent.mouseEnter(wrapper.container); - - expect(wrapper.getByTestId('variable-row-key-input')).toBeVisible(); - expect(wrapper.getByTestId('variable-row-key-input').querySelector('input')).toHaveValue( - environmentVariable.name, - ); - expect(wrapper.getByTestId('variable-row-value-input')).toBeVisible(); - expect(wrapper.getByTestId('variable-row-value-input').querySelector('input')).toHaveValue( - environmentVariable.value, - ); - }); - - it('should show cancel and save buttons in edit mode', async () => { - const wrapper = renderComponent({ - props: { - data: environmentVariable, - editing: true, - }, - }); - - await fireEvent.mouseEnter(wrapper.container); - - expect(wrapper.getByTestId('variable-row-cancel-button')).toBeVisible(); - expect(wrapper.getByTestId('variable-row-save-button')).toBeVisible(); - }); -}); diff --git a/packages/editor-ui/src/components/VariablesRow.vue b/packages/editor-ui/src/components/VariablesRow.vue deleted file mode 100644 index f970bd91ae..0000000000 --- a/packages/editor-ui/src/components/VariablesRow.vue +++ /dev/null @@ -1,278 +0,0 @@ - - - - - diff --git a/packages/editor-ui/src/components/VariablesUsageBadge.test.ts b/packages/editor-ui/src/components/VariablesUsageBadge.test.ts new file mode 100644 index 0000000000..a325ada897 --- /dev/null +++ b/packages/editor-ui/src/components/VariablesUsageBadge.test.ts @@ -0,0 +1,31 @@ +import { createComponentRenderer } from '@/__tests__/render'; +import VariablesUsageBadge from './VariablesUsageBadge.vue'; +import userEvent from '@testing-library/user-event'; + +const renderComponent = createComponentRenderer(VariablesUsageBadge); + +const showMessage = vi.fn(); +vi.mock('@/composables/useToast', () => ({ + useToast: () => ({ showMessage }), +})); + +const copy = vi.fn(); +vi.mock('@/composables/useClipboard', () => ({ + useClipboard: () => ({ copy }), +})); + +describe('VariablesUsageBadge', () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + it('should copy to the clipboard', async () => { + const name = 'myVar'; + const output = `$vars.${name}`; + const { getByText } = renderComponent({ props: { name } }); + await userEvent.click(getByText(output)); + + expect(showMessage).toHaveBeenCalledWith(expect.objectContaining({ type: 'success' })); + expect(copy).toHaveBeenCalledWith(output); + }); +}); diff --git a/packages/editor-ui/src/components/VariablesUsageBadge.vue b/packages/editor-ui/src/components/VariablesUsageBadge.vue new file mode 100644 index 0000000000..043d7e5d17 --- /dev/null +++ b/packages/editor-ui/src/components/VariablesUsageBadge.vue @@ -0,0 +1,44 @@ + + + + + diff --git a/packages/editor-ui/src/components/layouts/PageViewLayoutList.vue b/packages/editor-ui/src/components/layouts/PageViewLayoutList.vue index b46f58d284..963da7842e 100644 --- a/packages/editor-ui/src/components/layouts/PageViewLayoutList.vue +++ b/packages/editor-ui/src/components/layouts/PageViewLayoutList.vue @@ -1,11 +1,9 @@ -
+
diff --git a/packages/editor-ui/src/views/TestDefinition/tests/TestDefinitionEditView.test.ts b/packages/editor-ui/src/views/TestDefinition/tests/TestDefinitionEditView.test.ts index 3e85f86425..9824dda7d9 100644 --- a/packages/editor-ui/src/views/TestDefinition/tests/TestDefinitionEditView.test.ts +++ b/packages/editor-ui/src/views/TestDefinition/tests/TestDefinitionEditView.test.ts @@ -147,21 +147,6 @@ describe('TestDefinitionEditView', () => { expect(createTestMock).toHaveBeenCalled(); }); - it('should update test and show success message on save if testId is present', async () => { - vi.mocked(useRoute).mockReturnValue({ - params: { testId: '1' }, - name: VIEWS.TEST_DEFINITION_EDIT, - } as unknown as ReturnType); - - const { getByTestId } = renderComponentWithFeatureEnabled(); - - const saveButton = getByTestId('run-test-button'); - saveButton.click(); - await nextTick(); - - expect(updateTestMock).toHaveBeenCalledWith('1'); - }); - it('should show error message on failed test creation', async () => { createTestMock.mockRejectedValue(new Error('Save failed')); @@ -180,53 +165,39 @@ describe('TestDefinitionEditView', () => { expect(showErrorMock).toHaveBeenCalledWith(expect.any(Error), expect.any(String)); }); - it('should display "Save Test" button when editing test without eval workflow and tags', async () => { + it('should display disabled "run test" button when editing test without tags', async () => { vi.mocked(useRoute).mockReturnValue({ params: { testId: '1' }, name: VIEWS.TEST_DEFINITION_EDIT, } as unknown as ReturnType); - const { getByTestId } = renderComponentWithFeatureEnabled(); + const { getByTestId, mockedTestDefinitionStore } = renderComponentWithFeatureEnabled(); + + mockedTestDefinitionStore.getFieldIssues = vi + .fn() + .mockReturnValue([{ field: 'tags', message: 'Tag is required' }]); await nextTick(); + const updateButton = getByTestId('run-test-button'); - expect(updateButton.textContent?.toLowerCase()).toContain('save'); - }); + expect(updateButton.textContent?.toLowerCase()).toContain('run test'); + expect(updateButton).toHaveClass('disabled'); - it('should display "Save Test" button when creating new test', async () => { - vi.mocked(useRoute).mockReturnValue({ - params: {}, - name: VIEWS.NEW_TEST_DEFINITION, - } as unknown as ReturnType); - - const { getByTestId } = renderComponentWithFeatureEnabled(); - - const saveButton = getByTestId('run-test-button'); - expect(saveButton.textContent?.toLowerCase()).toContain('save test'); + mockedTestDefinitionStore.getFieldIssues = vi.fn().mockReturnValue([]); + await nextTick(); + expect(updateButton).not.toHaveClass('disabled'); }); it('should apply "has-issues" class to inputs with issues', async () => { - vi.mocked(useTestDefinitionForm).mockReturnValue({ - ...vi.mocked(useTestDefinitionForm)(), - fieldsIssues: ref([ - { field: 'name', message: 'Name is required' }, - { field: 'tags', message: 'Tag is required' }, - ]), - } as unknown as ReturnType); - - const { container } = renderComponentWithFeatureEnabled(); - + const { container, mockedTestDefinitionStore } = renderComponentWithFeatureEnabled(); + mockedTestDefinitionStore.getFieldIssues = vi + .fn() + .mockReturnValue([{ field: 'tags', message: 'Tag is required' }]); await nextTick(); const issueElements = container.querySelectorAll('.has-issues'); expect(issueElements.length).toBeGreaterThan(0); }); - it('should fetch all tags on mount', async () => { - renderComponentWithFeatureEnabled(); - await nextTick(); - expect(mockedStore(useAnnotationTagsStore).fetchAll).toHaveBeenCalled(); - }); - describe('Test Runs functionality', () => { it('should display test runs table when runs exist', async () => { vi.mocked(useRoute).mockReturnValue({ diff --git a/packages/editor-ui/src/views/TestDefinition/tests/TestDefinitionListView.test.ts b/packages/editor-ui/src/views/TestDefinition/tests/TestDefinitionListView.test.ts index bf23b73a9e..c4b427f4bd 100644 --- a/packages/editor-ui/src/views/TestDefinition/tests/TestDefinitionListView.test.ts +++ b/packages/editor-ui/src/views/TestDefinition/tests/TestDefinitionListView.test.ts @@ -6,21 +6,22 @@ import { createComponentRenderer } from '@/__tests__/render'; import TestDefinitionListView from '@/views/TestDefinition/TestDefinitionListView.vue'; import { useRoute, useRouter } from 'vue-router'; import { useToast } from '@/composables/useToast'; -import { useAnnotationTagsStore } from '@/stores/tags.store'; +import { useMessage } from '@/composables/useMessage'; import { useTestDefinitionStore } from '@/stores/testDefinition.store.ee'; import { nextTick, ref } from 'vue'; import { mockedStore, waitAllPromises } from '@/__tests__/utils'; -import { VIEWS } from '@/constants'; +import { MODAL_CONFIRM, VIEWS } from '@/constants'; import type { TestDefinitionRecord } from '@/api/testDefinition.ee'; vi.mock('vue-router'); vi.mock('@/composables/useToast'); - +vi.mock('@/composables/useMessage'); describe('TestDefinitionListView', () => { const renderComponent = createComponentRenderer(TestDefinitionListView); let showMessageMock: Mock; let showErrorMock: Mock; + let confirmMock: Mock; let startTestRunMock: Mock; let fetchTestRunsMock: Mock; let deleteByIdMock: Mock; @@ -65,6 +66,7 @@ describe('TestDefinitionListView', () => { showMessageMock = vi.fn(); showErrorMock = vi.fn(); + confirmMock = vi.fn().mockResolvedValue(MODAL_CONFIRM); startTestRunMock = vi.fn().mockResolvedValue({ success: true }); fetchTestRunsMock = vi.fn(); deleteByIdMock = vi.fn(); @@ -74,6 +76,10 @@ describe('TestDefinitionListView', () => { showMessage: showMessageMock, showError: showErrorMock, } as unknown as ReturnType); + + vi.mocked(useMessage).mockReturnValue({ + confirm: confirmMock, + } as unknown as ReturnType); }); afterEach(() => { @@ -89,7 +95,6 @@ describe('TestDefinitionListView', () => { setActivePinia(pinia); const testDefinitionStore = mockedStore(useTestDefinitionStore); - // const tagsStore = mockedStore(useAnnotationTagsStore); testDefinitionStore.isFeatureEnabled = true; testDefinitionStore.fetchAll = fetchAllMock; testDefinitionStore.startTestRun = startTestRunMock; @@ -120,7 +125,6 @@ describe('TestDefinitionListView', () => { expect(testDefinitionStore.fetchAll).toHaveBeenCalledWith({ workflowId: 'workflow1', }); - expect(mockedStore(useAnnotationTagsStore).fetchAll).toHaveBeenCalled(); }); it('should start test run and show success message', async () => { @@ -150,13 +154,12 @@ describe('TestDefinitionListView', () => { }); it('should delete test and show success message', async () => { - const { getByTestId, testDefinitionStore } = await renderComponentWithFeatureEnabled(); - + const { getByTestId } = await renderComponentWithFeatureEnabled(); const deleteButton = getByTestId('delete-test-button-1'); deleteButton.click(); - await nextTick(); + await waitAllPromises(); - expect(testDefinitionStore.deleteById).toHaveBeenCalledWith('1'); + expect(deleteByIdMock).toHaveBeenCalledWith('1'); expect(showMessageMock).toHaveBeenCalledWith({ title: expect.any(String), type: 'success', From 3434682e415378a0dbb61edc56d275b8c151c407 Mon Sep 17 00:00:00 2001 From: Cornelius Suermann Date: Mon, 20 Jan 2025 15:26:12 +0100 Subject: [PATCH 007/105] chore: Bump License-SDK to v2.14.1 (no-changelog) (#12724) --- packages/cli/package.json | 2 +- pnpm-lock.yaml | 42 ++++++++++++++++----------------------- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 4920b7994a..8374e06ddc 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -96,7 +96,7 @@ "@n8n/task-runner": "workspace:*", "@n8n/typeorm": "0.3.20-12", "@n8n_io/ai-assistant-sdk": "1.13.0", - "@n8n_io/license-sdk": "2.14.0", + "@n8n_io/license-sdk": "2.14.1", "@oclif/core": "4.0.7", "@rudderstack/rudder-sdk-node": "2.0.9", "@sentry/node": "catalog:", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c46e5d10c8..593b2451d1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -443,7 +443,7 @@ importers: version: 3.666.0(@aws-sdk/client-sts@3.666.0) '@getzep/zep-cloud': specifier: 1.0.12 - version: 1.0.12(@langchain/core@0.3.30(openai@4.78.1(encoding@0.1.13)(zod@3.24.1)))(encoding@0.1.13)(langchain@0.3.11(uhxpxbd3xjubkjdqqkxxpkezmi)) + version: 1.0.12(@langchain/core@0.3.30(openai@4.78.1(encoding@0.1.13)(zod@3.24.1)))(encoding@0.1.13)(langchain@0.3.11(zeor2kpdk42qkakkadld3jqwca)) '@getzep/zep-js': specifier: 0.9.0 version: 0.9.0 @@ -470,7 +470,7 @@ importers: version: 0.3.2(@aws-sdk/client-sso-oidc@3.666.0(@aws-sdk/client-sts@3.666.0))(@langchain/core@0.3.30(openai@4.78.1(encoding@0.1.13)(zod@3.24.1)))(encoding@0.1.13) '@langchain/community': specifier: 0.3.24 - version: 0.3.24(xbnzedcvjhnriori3dst4asz2q) + version: 0.3.24(nebpow57ma55uutbjwa5f4w6tq) '@langchain/core': specifier: 'catalog:' version: 0.3.30(openai@4.78.1(encoding@0.1.13)(zod@3.24.1)) @@ -557,7 +557,7 @@ importers: version: 23.0.1 langchain: specifier: 0.3.11 - version: 0.3.11(uhxpxbd3xjubkjdqqkxxpkezmi) + version: 0.3.11(zeor2kpdk42qkakkadld3jqwca) lodash: specifier: 'catalog:' version: 4.17.21 @@ -803,8 +803,8 @@ importers: specifier: 1.13.0 version: 1.13.0 '@n8n_io/license-sdk': - specifier: 2.14.0 - version: 2.14.0 + specifier: 2.14.1 + version: 2.14.1 '@oclif/core': specifier: 4.0.7 version: 4.0.7 @@ -4494,8 +4494,8 @@ packages: resolution: {integrity: sha512-16kftFTeX3/lBinHJaBK0OL1lB4FpPaUoHX4h25AkvgHvmjUHpWNY2ZtKos0rY89+pkzDsNxMZqSUkeKU45iRg==} engines: {node: '>=20.15', pnpm: '>=8.14'} - '@n8n_io/license-sdk@2.14.0': - resolution: {integrity: sha512-cQ1gBEksgVMlUdbDeDx5lctf8/nlJGNQWLv+MWJpfdqg/aJ56HzgzzSLppdmPdT0mRnfWEYmwijFBDfBZRcx2g==} + '@n8n_io/license-sdk@2.14.1': + resolution: {integrity: sha512-NLr/BuRp2t0sQaHmgeEPt9kYruZTm6DMvqcxBZqhfjDT7hzM7fJDUKRI/ocUUElnuXNdZ8dXY1wYEyX+NwPfMw==} engines: {node: '>=18.12.1'} '@n8n_io/riot-tmpl@4.0.0': @@ -15992,7 +15992,7 @@ snapshots: '@gar/promisify@1.1.3': optional: true - '@getzep/zep-cloud@1.0.12(@langchain/core@0.3.30(openai@4.78.1(encoding@0.1.13)(zod@3.24.1)))(encoding@0.1.13)(langchain@0.3.11(uhxpxbd3xjubkjdqqkxxpkezmi))': + '@getzep/zep-cloud@1.0.12(@langchain/core@0.3.30(openai@4.78.1(encoding@0.1.13)(zod@3.24.1)))(encoding@0.1.13)(langchain@0.3.11(zeor2kpdk42qkakkadld3jqwca))': dependencies: form-data: 4.0.0 node-fetch: 2.7.0(encoding@0.1.13) @@ -16001,7 +16001,7 @@ snapshots: zod: 3.24.1 optionalDependencies: '@langchain/core': 0.3.30(openai@4.78.1(encoding@0.1.13)(zod@3.24.1)) - langchain: 0.3.11(uhxpxbd3xjubkjdqqkxxpkezmi) + langchain: 0.3.11(zeor2kpdk42qkakkadld3jqwca) transitivePeerDependencies: - encoding @@ -16540,7 +16540,7 @@ snapshots: - aws-crt - encoding - '@langchain/community@0.3.24(xbnzedcvjhnriori3dst4asz2q)': + '@langchain/community@0.3.24(nebpow57ma55uutbjwa5f4w6tq)': dependencies: '@browserbasehq/stagehand': 1.9.0(@playwright/test@1.49.1)(deepmerge@4.3.1)(dotenv@16.4.5)(encoding@0.1.13)(openai@4.78.1(encoding@0.1.13)(zod@3.24.1))(zod@3.24.1) '@ibm-cloud/watsonx-ai': 1.1.2 @@ -16551,7 +16551,7 @@ snapshots: flat: 5.0.2 ibm-cloud-sdk-core: 5.1.0 js-yaml: 4.1.0 - langchain: 0.3.11(uhxpxbd3xjubkjdqqkxxpkezmi) + langchain: 0.3.11(zeor2kpdk42qkakkadld3jqwca) langsmith: 0.2.15(openai@4.78.1(encoding@0.1.13)(zod@3.24.1)) openai: 4.78.1(encoding@0.1.13)(zod@3.24.1) uuid: 10.0.0 @@ -16566,7 +16566,7 @@ snapshots: '@aws-sdk/credential-provider-node': 3.666.0(@aws-sdk/client-sso-oidc@3.666.0(@aws-sdk/client-sts@3.666.0))(@aws-sdk/client-sts@3.666.0) '@azure/storage-blob': 12.18.0(encoding@0.1.13) '@browserbasehq/sdk': 2.0.0(encoding@0.1.13) - '@getzep/zep-cloud': 1.0.12(@langchain/core@0.3.30(openai@4.78.1(encoding@0.1.13)(zod@3.24.1)))(encoding@0.1.13)(langchain@0.3.11(uhxpxbd3xjubkjdqqkxxpkezmi)) + '@getzep/zep-cloud': 1.0.12(@langchain/core@0.3.30(openai@4.78.1(encoding@0.1.13)(zod@3.24.1)))(encoding@0.1.13)(langchain@0.3.11(zeor2kpdk42qkakkadld3jqwca)) '@getzep/zep-js': 0.9.0 '@google-ai/generativelanguage': 2.6.0(encoding@0.1.13) '@google-cloud/storage': 7.12.1(encoding@0.1.13) @@ -16944,7 +16944,7 @@ snapshots: '@n8n_io/ai-assistant-sdk@1.13.0': {} - '@n8n_io/license-sdk@2.14.0': + '@n8n_io/license-sdk@2.14.1': dependencies: crypto-js: 4.2.0 node-machine-id: 1.1.12 @@ -19867,14 +19867,6 @@ snapshots: transitivePeerDependencies: - debug - axios@1.7.4(debug@4.3.7): - dependencies: - follow-redirects: 1.15.6(debug@4.3.7) - form-data: 4.0.0 - proxy-from-env: 1.1.0 - transitivePeerDependencies: - - debug - axios@1.7.7: dependencies: follow-redirects: 1.15.6(debug@4.3.6) @@ -22738,7 +22730,7 @@ snapshots: '@types/debug': 4.1.12 '@types/node': 18.16.16 '@types/tough-cookie': 4.0.2 - axios: 1.7.4(debug@4.3.7) + axios: 1.7.4 camelcase: 6.3.0 debug: 4.3.7 dotenv: 16.4.5 @@ -22748,7 +22740,7 @@ snapshots: isstream: 0.1.2 jsonwebtoken: 9.0.2 mime-types: 2.1.35 - retry-axios: 2.6.0(axios@1.7.4) + retry-axios: 2.6.0(axios@1.7.4(debug@4.3.7)) tough-cookie: 4.1.3 transitivePeerDependencies: - supports-color @@ -23753,7 +23745,7 @@ snapshots: kuler@2.0.0: {} - langchain@0.3.11(uhxpxbd3xjubkjdqqkxxpkezmi): + langchain@0.3.11(zeor2kpdk42qkakkadld3jqwca): dependencies: '@langchain/core': 0.3.30(openai@4.78.1(encoding@0.1.13)(zod@3.24.1)) '@langchain/openai': 0.3.17(@langchain/core@0.3.30(openai@4.78.1(encoding@0.1.13)(zod@3.24.1)))(encoding@0.1.13) @@ -26153,7 +26145,7 @@ snapshots: ret@0.1.15: {} - retry-axios@2.6.0(axios@1.7.4): + retry-axios@2.6.0(axios@1.7.4(debug@4.3.7)): dependencies: axios: 1.7.4 From f167578b3251e553a4d000e731e1bb60348916ad Mon Sep 17 00:00:00 2001 From: Dana <152518854+dana-gill@users.noreply.github.com> Date: Mon, 20 Jan 2025 16:52:06 +0100 Subject: [PATCH 008/105] feat(n8n Form Trigger Node): Form Improvements (#12590) --- .../cli/templates/form-trigger.handlebars | 7 +- .../nodes/Form/common.descriptions.ts | 2 +- packages/nodes-base/nodes/Form/interfaces.ts | 1 + .../Form/test/FormTriggerV2.node.test.ts | 1 + .../nodes-base/nodes/Form/test/utils.test.ts | 111 ++++++++++++++---- packages/nodes-base/nodes/Form/utils.ts | 39 +++++- packages/nodes-base/package.json | 2 + .../utils/sendAndWait/test/util.test.ts | 2 + pnpm-lock.yaml | 6 + 9 files changed, 148 insertions(+), 23 deletions(-) diff --git a/packages/cli/templates/form-trigger.handlebars b/packages/cli/templates/form-trigger.handlebars index ee868b072f..695ef79472 100644 --- a/packages/cli/templates/form-trigger.handlebars +++ b/packages/cli/templates/form-trigger.handlebars @@ -2,6 +2,11 @@ + + + + +

{{formTitle}}

-

{{formDescription}}

+

{{{formDescription}}}

diff --git a/packages/nodes-base/nodes/Form/common.descriptions.ts b/packages/nodes-base/nodes/Form/common.descriptions.ts index 299284c59f..dc0d8bf076 100644 --- a/packages/nodes-base/nodes/Form/common.descriptions.ts +++ b/packages/nodes-base/nodes/Form/common.descriptions.ts @@ -29,7 +29,7 @@ export const formDescription: INodeProperties = { default: '', placeholder: "e.g. We'll get back to you soon", description: - 'Shown underneath the Form Title. Can be used to prompt the user on how to complete the form.', + 'Shown underneath the Form Title. Can be used to prompt the user on how to complete the form. Accepts HTML.', typeOptions: { rows: 2, }, diff --git a/packages/nodes-base/nodes/Form/interfaces.ts b/packages/nodes-base/nodes/Form/interfaces.ts index 1cf5f64c92..b04d30d1d3 100644 --- a/packages/nodes-base/nodes/Form/interfaces.ts +++ b/packages/nodes-base/nodes/Form/interfaces.ts @@ -22,6 +22,7 @@ export type FormTriggerData = { validForm: boolean; formTitle: string; formDescription?: string; + formDescriptionMetadata?: string; formSubmittedHeader?: string; formSubmittedText?: string; redirectUrl?: string; diff --git a/packages/nodes-base/nodes/Form/test/FormTriggerV2.node.test.ts b/packages/nodes-base/nodes/Form/test/FormTriggerV2.node.test.ts index 63dbfd4986..9c69179066 100644 --- a/packages/nodes-base/nodes/Form/test/FormTriggerV2.node.test.ts +++ b/packages/nodes-base/nodes/Form/test/FormTriggerV2.node.test.ts @@ -50,6 +50,7 @@ describe('FormTrigger', () => { appendAttribution: false, buttonLabel: 'Submit', formDescription: 'Test Description', + formDescriptionMetadata: 'Test Description', formFields: [ { defaultValue: '', diff --git a/packages/nodes-base/nodes/Form/test/utils.test.ts b/packages/nodes-base/nodes/Form/test/utils.test.ts index b37a3993e8..65decaa285 100644 --- a/packages/nodes-base/nodes/Form/test/utils.test.ts +++ b/packages/nodes-base/nodes/Form/test/utils.test.ts @@ -10,19 +10,58 @@ import type { import { formWebhook, + createDescriptionMetadata, prepareFormData, prepareFormReturnItem, resolveRawData, isFormConnected, } from '../utils'; +describe('FormTrigger, parseFormDescription', () => { + it('should remove HTML tags and truncate to 150 characters', () => { + const descriptions = [ + { description: '

This is a test description

', expected: 'This is a test description' }, + { description: 'Test description', expected: 'Test description' }, + { + description: + 'Beneath the golden hues of a setting sun, waves crashed against the rugged shore, carrying whispers of ancient tales etched in natures timeless and soothing song.', + expected: + 'Beneath the golden hues of a setting sun, waves crashed against the rugged shore, carrying whispers of ancient tales etched in natures timeless and so', + }, + { + description: + '

Beneath the golden hues of a setting sun, waves crashed against the rugged shore, carrying whispers of ancient tales etched in natures timeless and soothing song.

', + expected: + 'Beneath the golden hues of a setting sun, waves crashed against the rugged shore, carrying whispers of ancient tales etched in natures timeless and so', + }, + ]; + + descriptions.forEach(({ description, expected }) => { + expect(createDescriptionMetadata(description)).toBe(expected); + }); + }); +}); + describe('FormTrigger, formWebhook', () => { + const executeFunctions = mock(); + executeFunctions.getNode.mockReturnValue({ typeVersion: 2.1 } as any); + executeFunctions.getNodeParameter.calledWith('options').mockReturnValue({}); + executeFunctions.getNodeParameter.calledWith('formTitle').mockReturnValue('Test Form'); + executeFunctions.getNodeParameter + .calledWith('formDescription') + .mockReturnValue('Test Description'); + executeFunctions.getNodeParameter.calledWith('responseMode').mockReturnValue('onReceived'); + executeFunctions.getRequestObject.mockReturnValue({ method: 'GET', query: {} } as any); + executeFunctions.getMode.mockReturnValue('manual'); + executeFunctions.getInstanceId.mockReturnValue('instanceId'); + executeFunctions.getBodyData.mockReturnValue({ data: {}, files: {} }); + executeFunctions.getChildNodes.mockReturnValue([]); + beforeEach(() => { jest.clearAllMocks(); }); it('should call response render', async () => { - const executeFunctions = mock(); const mockRender = jest.fn(); const formFields: FormFieldsParameter = [ @@ -43,20 +82,8 @@ describe('FormTrigger, formWebhook', () => { }, ]; - executeFunctions.getNode.mockReturnValue({ typeVersion: 2.1 } as any); - executeFunctions.getNodeParameter.calledWith('options').mockReturnValue({}); - executeFunctions.getNodeParameter.calledWith('formTitle').mockReturnValue('Test Form'); - executeFunctions.getNodeParameter - .calledWith('formDescription') - .mockReturnValue('Test Description'); - executeFunctions.getNodeParameter.calledWith('responseMode').mockReturnValue('onReceived'); executeFunctions.getNodeParameter.calledWith('formFields.values').mockReturnValue(formFields); executeFunctions.getResponseObject.mockReturnValue({ render: mockRender } as any); - executeFunctions.getRequestObject.mockReturnValue({ method: 'GET', query: {} } as any); - executeFunctions.getMode.mockReturnValue('manual'); - executeFunctions.getInstanceId.mockReturnValue('instanceId'); - executeFunctions.getBodyData.mockReturnValue({ data: {}, files: {} }); - executeFunctions.getChildNodes.mockReturnValue([]); await formWebhook(executeFunctions); @@ -64,6 +91,7 @@ describe('FormTrigger, formWebhook', () => { appendAttribution: true, buttonLabel: 'Submit', formDescription: 'Test Description', + formDescriptionMetadata: 'Test Description', formFields: [ { defaultValue: '', @@ -117,8 +145,55 @@ describe('FormTrigger, formWebhook', () => { }); }); + it('should sanitize form descriptions', async () => { + const mockRender = jest.fn(); + + const formDescription = [ + { description: 'Test Description', expected: 'Test Description' }, + { description: 'hello', expected: 'hello' }, + { description: '', expected: '' }, + ]; + const formFields: FormFieldsParameter = [ + { fieldLabel: 'Name', fieldType: 'text', requiredField: true }, + ]; + + executeFunctions.getNodeParameter.calledWith('formFields.values').mockReturnValue(formFields); + executeFunctions.getResponseObject.mockReturnValue({ render: mockRender } as any); + + for (const { description, expected } of formDescription) { + executeFunctions.getNodeParameter.calledWith('formDescription').mockReturnValue(description); + + await formWebhook(executeFunctions); + + expect(mockRender).toHaveBeenCalledWith('form-trigger', { + appendAttribution: true, + buttonLabel: 'Submit', + formDescription: expected, + formDescriptionMetadata: createDescriptionMetadata(expected), + formFields: [ + { + defaultValue: '', + errorId: 'error-field-0', + id: 'field-0', + inputRequired: 'form-required', + isInput: true, + label: 'Name', + placeholder: undefined, + type: 'text', + }, + ], + formSubmittedText: 'Your response has been recorded', + formTitle: 'Test Form', + n8nWebsiteLink: + 'https://n8n.io/?utm_source=n8n-internal&utm_medium=form-trigger&utm_campaign=instanceId', + testRun: true, + useResponseData: false, + validForm: true, + }); + } + }); + it('should return workflowData on POST request', async () => { - const executeFunctions = mock(); const mockStatus = jest.fn(); const mockEnd = jest.fn(); @@ -132,15 +207,9 @@ describe('FormTrigger, formWebhook', () => { 'field-1': '30', }; - executeFunctions.getNode.mockReturnValue({ typeVersion: 2.1 } as any); - executeFunctions.getNodeParameter.calledWith('options').mockReturnValue({}); - executeFunctions.getNodeParameter.calledWith('responseMode').mockReturnValue('onReceived'); - executeFunctions.getChildNodes.mockReturnValue([]); executeFunctions.getNodeParameter.calledWith('formFields.values').mockReturnValue(formFields); executeFunctions.getResponseObject.mockReturnValue({ status: mockStatus, end: mockEnd } as any); executeFunctions.getRequestObject.mockReturnValue({ method: 'POST' } as any); - executeFunctions.getMode.mockReturnValue('manual'); - executeFunctions.getInstanceId.mockReturnValue('instanceId'); executeFunctions.getBodyData.mockReturnValue({ data: bodyData, files: {} }); const result = await formWebhook(executeFunctions); @@ -213,6 +282,7 @@ describe('FormTrigger, prepareFormData', () => { validForm: true, formTitle: 'Test Form', formDescription: 'This is a test form', + formDescriptionMetadata: 'This is a test form', formSubmittedText: 'Thank you for your submission', n8nWebsiteLink: 'https://n8n.io/?utm_source=n8n-internal&utm_medium=form-trigger&utm_campaign=test-instance', @@ -292,6 +362,7 @@ describe('FormTrigger, prepareFormData', () => { validForm: true, formTitle: 'Test Form', formDescription: 'This is a test form', + formDescriptionMetadata: 'This is a test form', formSubmittedText: 'Your response has been recorded', n8nWebsiteLink: 'https://n8n.io/?utm_source=n8n-internal&utm_medium=form-trigger', formFields: [ diff --git a/packages/nodes-base/nodes/Form/utils.ts b/packages/nodes-base/nodes/Form/utils.ts index 6fc414f622..e4b46c72fd 100644 --- a/packages/nodes-base/nodes/Form/utils.ts +++ b/packages/nodes-base/nodes/Form/utils.ts @@ -16,6 +16,7 @@ import { WAIT_NODE_TYPE, jsonParse, } from 'n8n-workflow'; +import sanitize from 'sanitize-html'; import type { FormTriggerData, FormTriggerInput } from './interfaces'; import { FORM_TRIGGER_AUTHENTICATION_PROPERTY } from './interfaces'; @@ -23,6 +24,41 @@ import { getResolvables } from '../../utils/utilities'; import { WebhookAuthorizationError } from '../Webhook/error'; import { validateWebhookAuthentication } from '../Webhook/utils'; +function sanitizeHtml(text: string) { + return sanitize(text, { + allowedTags: [ + 'b', + 'i', + 'em', + 'strong', + 'a', + 'h1', + 'h2', + 'h3', + 'h4', + 'h5', + 'h6', + 'u', + 'sub', + 'sup', + 'code', + 'pre', + 'span', + 'br', + ], + allowedAttributes: { + a: ['href', 'target', 'rel'], + }, + nonBooleanAttributes: ['*'], + }); +} + +export function createDescriptionMetadata(description: string) { + return description === '' + ? 'n8n form' + : description.replace(/^\s*\n+|<\/?[^>]+(>|$)/g, '').slice(0, 150); +} + export function prepareFormData({ formTitle, formDescription, @@ -63,6 +99,7 @@ export function prepareFormData({ validForm, formTitle, formDescription, + formDescriptionMetadata: createDescriptionMetadata(formDescription), formSubmittedHeader, formSubmittedText, n8nWebsiteLink, @@ -380,7 +417,7 @@ export async function formWebhook( //Show the form on GET request if (method === 'GET') { const formTitle = context.getNodeParameter('formTitle', '') as string; - const formDescription = context.getNodeParameter('formDescription', '') as string; + const formDescription = sanitizeHtml(context.getNodeParameter('formDescription', '') as string); const responseMode = context.getNodeParameter('responseMode', '') as string; let formSubmittedText; diff --git a/packages/nodes-base/package.json b/packages/nodes-base/package.json index 9a42eae713..309d93c310 100644 --- a/packages/nodes-base/package.json +++ b/packages/nodes-base/package.json @@ -841,6 +841,7 @@ "@types/nodemailer": "^6.4.14", "@types/promise-ftp": "^1.3.4", "@types/rfc2047": "^2.0.1", + "@types/sanitize-html": "^2.11.0", "@types/showdown": "^1.9.4", "@types/snowflake-sdk": "^1.6.24", "@types/ssh2-sftp-client": "^5.1.0", @@ -906,6 +907,7 @@ "rhea": "1.0.24", "rrule": "2.8.1", "rss-parser": "3.13.0", + "sanitize-html": "2.12.1", "semver": "7.5.4", "showdown": "2.1.0", "simple-git": "3.17.0", diff --git a/packages/nodes-base/utils/sendAndWait/test/util.test.ts b/packages/nodes-base/utils/sendAndWait/test/util.test.ts index 5194887f2f..39a6f16859 100644 --- a/packages/nodes-base/utils/sendAndWait/test/util.test.ts +++ b/packages/nodes-base/utils/sendAndWait/test/util.test.ts @@ -240,6 +240,7 @@ describe('Send and Wait utils tests', () => { validForm: true, formTitle: '', formDescription: 'Test message', + formDescriptionMetadata: 'Test message', formSubmittedHeader: 'Got it, thanks', formSubmittedText: 'This page can be closed now', n8nWebsiteLink: 'https://n8n.io/?utm_source=n8n-internal&utm_medium=form-trigger', @@ -318,6 +319,7 @@ describe('Send and Wait utils tests', () => { validForm: true, formTitle: 'Test title', formDescription: 'Test description', + formDescriptionMetadata: 'Test description', formSubmittedHeader: 'Got it, thanks', formSubmittedText: 'This page can be closed now', n8nWebsiteLink: 'https://n8n.io/?utm_source=n8n-internal&utm_medium=form-trigger', diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 593b2451d1..af9b0caa4d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1851,6 +1851,9 @@ importers: rss-parser: specifier: 3.13.0 version: 3.13.0 + sanitize-html: + specifier: 2.12.1 + version: 2.12.1 semver: specifier: ^7.5.4 version: 7.6.0 @@ -1936,6 +1939,9 @@ importers: '@types/rfc2047': specifier: ^2.0.1 version: 2.0.1 + '@types/sanitize-html': + specifier: ^2.11.0 + version: 2.11.0 '@types/showdown': specifier: ^1.9.4 version: 1.9.4 From 967ee4b89b94b92fc3955c56bf4c9cca0bd64eac Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Mon, 20 Jan 2025 16:53:55 +0100 Subject: [PATCH 009/105] feat: Synchronize deletions when pulling from source control (#12170) Co-authored-by: r00gm --- packages/cli/src/commands/ldap/reset.ts | 2 +- .../cli/src/controllers/users.controller.ts | 2 +- .../src/credentials/credentials.controller.ts | 2 +- .../src/credentials/credentials.service.ts | 22 +- .../source-control-import.service.ee.test.ts | 3 + .../__tests__/source-control.service.test.ts | 173 +++++++++++++- .../source-control-import.service.ee.ts | 31 +++ .../source-control.controller.ee.ts | 2 +- .../source-control.service.ee.ts | 220 +++++++++++------- .../source-control/source-control.handler.ts | 2 +- .../cli/src/services/project.service.ee.ts | 2 +- .../cli/src/workflows/workflow.service.ts | 7 + .../source-control-import.service.test.ts | 3 + .../MainSidebarSourceControl.test.ts | 90 ------- .../components/MainSidebarSourceControl.vue | 100 +------- .../SourceControlPullModal.ee.test.ts | 114 +++++++++ .../components/SourceControlPullModal.ee.vue | 201 +++++++++++----- .../components/SourceControlPushModal.ee.vue | 26 +-- .../src/plugins/i18n/locales/en.json | 2 +- .../src/utils/sourceControlUtils.test.ts | 119 ++++++++++ .../editor-ui/src/utils/sourceControlUtils.ts | 142 +++++++++++ 21 files changed, 900 insertions(+), 365 deletions(-) create mode 100644 packages/editor-ui/src/components/SourceControlPullModal.ee.test.ts create mode 100644 packages/editor-ui/src/utils/sourceControlUtils.test.ts create mode 100644 packages/editor-ui/src/utils/sourceControlUtils.ts diff --git a/packages/cli/src/commands/ldap/reset.ts b/packages/cli/src/commands/ldap/reset.ts index edbf988434..8d3eb8da94 100644 --- a/packages/cli/src/commands/ldap/reset.ts +++ b/packages/cli/src/commands/ldap/reset.ts @@ -110,7 +110,7 @@ export class Reset extends BaseCommand { } for (const credential of ownedCredentials) { - await Container.get(CredentialsService).delete(credential); + await Container.get(CredentialsService).delete(owner, credential.id); } await Container.get(AuthProviderSyncHistoryRepository).delete({ providerType: 'ldap' }); diff --git a/packages/cli/src/controllers/users.controller.ts b/packages/cli/src/controllers/users.controller.ts index 3177c2c23b..80d38fcfcd 100644 --- a/packages/cli/src/controllers/users.controller.ts +++ b/packages/cli/src/controllers/users.controller.ts @@ -240,7 +240,7 @@ export class UsersController { } for (const credential of ownedCredentials) { - await this.credentialsService.delete(credential); + await this.credentialsService.delete(userToDelete, credential.id); } await this.userService.getManager().transaction(async (trx) => { diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 73888e1977..6b4fd8472a 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -251,7 +251,7 @@ export class CredentialsController { ); } - await this.credentialsService.delete(credential); + await this.credentialsService.delete(req.user, credential.id); this.eventService.emit('credentials-deleted', { user: req.user, diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 52a1a3e88d..18d01a198a 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -406,10 +406,26 @@ export class CredentialsService { return result; } - async delete(credentials: CredentialsEntity) { - await this.externalHooks.run('credentials.delete', [credentials.id]); + /** + * Deletes a credential. + * + * If the user does not have permission to delete the credential this does + * nothing and returns void. + */ + async delete(user: User, credentialId: string) { + await this.externalHooks.run('credentials.delete', [credentialId]); - await this.credentialsRepository.remove(credentials); + const credential = await this.sharedCredentialsRepository.findCredentialForUser( + credentialId, + user, + ['credential:delete'], + ); + + if (!credential) { + return; + } + + await this.credentialsRepository.remove(credential); } async test(user: User, credentials: ICredentialsDecrypted) { diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts index fff60bd566..cc817368cd 100644 --- a/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control-import.service.ee.test.ts @@ -26,6 +26,9 @@ describe('SourceControlImportService', () => { mock(), workflowRepository, mock(), + mock(), + mock(), + mock(), mock({ n8nFolder: '/mock/n8n' }), ); diff --git a/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts b/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts index 56b58646c9..43a13ed6ba 100644 --- a/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts +++ b/packages/cli/src/environments.ee/source-control/__tests__/source-control.service.test.ts @@ -1,10 +1,19 @@ +import type { SourceControlledFile } from '@n8n/api-types'; import { Container } from '@n8n/di'; import { mock } from 'jest-mock-extended'; import { InstanceSettings } from 'n8n-core'; +import type { TagEntity } from '@/databases/entities/tag-entity'; +import type { User } from '@/databases/entities/user'; +import type { Variables } from '@/databases/entities/variables'; +import type { TagRepository } from '@/databases/repositories/tag.repository'; import { SourceControlPreferencesService } from '@/environments.ee/source-control/source-control-preferences.service.ee'; import { SourceControlService } from '@/environments.ee/source-control/source-control.service.ee'; +import type { SourceControlImportService } from '../source-control-import.service.ee'; +import type { ExportableCredential } from '../types/exportable-credential'; +import type { SourceControlWorkflowVersionId } from '../types/source-control-workflow-version-id'; + describe('SourceControlService', () => { const preferencesService = new SourceControlPreferencesService( Container.get(InstanceSettings), @@ -13,20 +22,25 @@ describe('SourceControlService', () => { mock(), mock(), ); + const sourceControlImportService = mock(); + const tagRepository = mock(); const sourceControlService = new SourceControlService( mock(), mock(), preferencesService, mock(), - mock(), - mock(), + sourceControlImportService, + tagRepository, mock(), ); + beforeEach(() => { + jest.resetAllMocks(); + jest.spyOn(sourceControlService, 'sanityCheck').mockResolvedValue(undefined); + }); + describe('pushWorkfolder', () => { it('should throw an error if a file is given that is not in the workfolder', async () => { - jest.spyOn(sourceControlService, 'sanityCheck').mockResolvedValue(undefined); - await expect( sourceControlService.pushWorkfolder({ fileNames: [ @@ -46,4 +60,155 @@ describe('SourceControlService', () => { ).rejects.toThrow('File path /etc/passwd is invalid'); }); }); + + describe('pullWorkfolder', () => { + it('does not filter locally created credentials', async () => { + // ARRANGE + const user = mock(); + const statuses = [ + mock({ + status: 'created', + location: 'local', + type: 'credential', + }), + mock({ + status: 'created', + location: 'local', + type: 'workflow', + }), + ]; + jest.spyOn(sourceControlService, 'getStatus').mockResolvedValueOnce(statuses); + + // ACT + const result = await sourceControlService.pullWorkfolder(user, {}); + + // ASSERT + expect(result).toMatchObject({ statusCode: 409, statusResult: statuses }); + }); + + it('does not filter remotely deleted credentials', async () => { + // ARRANGE + const user = mock(); + const statuses = [ + mock({ + status: 'deleted', + location: 'remote', + type: 'credential', + }), + mock({ + status: 'created', + location: 'local', + type: 'workflow', + }), + ]; + jest.spyOn(sourceControlService, 'getStatus').mockResolvedValueOnce(statuses); + + // ACT + const result = await sourceControlService.pullWorkfolder(user, {}); + + // ASSERT + expect(result).toMatchObject({ statusCode: 409, statusResult: statuses }); + }); + + it('should throw an error if a file is given that is not in the workfolder', async () => { + await expect( + sourceControlService.pushWorkfolder({ + fileNames: [ + { + file: '/etc/passwd', + id: 'test', + name: 'secret-file', + type: 'file', + status: 'modified', + location: 'local', + conflict: false, + updatedAt: new Date().toISOString(), + pushed: false, + }, + ], + }), + ).rejects.toThrow('File path /etc/passwd is invalid'); + }); + }); + + describe('getStatus', () => { + it('conflict depends on the value of `direction`', async () => { + // ARRANGE + + // Define a credential that does only exist locally. + // Pulling this would delete it so it should be marked as a conflict. + // Pushing this is conflict free. + sourceControlImportService.getRemoteVersionIdsFromFiles.mockResolvedValue([]); + sourceControlImportService.getLocalVersionIdsFromDb.mockResolvedValue([ + mock(), + ]); + + // Define a credential that does only exist locally. + // Pulling this would delete it so it should be marked as a conflict. + // Pushing this is conflict free. + sourceControlImportService.getRemoteCredentialsFromFiles.mockResolvedValue([]); + sourceControlImportService.getLocalCredentialsFromDb.mockResolvedValue([ + mock(), + ]); + + // Define a variable that does only exist locally. + // Pulling this would delete it so it should be marked as a conflict. + // Pushing this is conflict free. + sourceControlImportService.getRemoteVariablesFromFile.mockResolvedValue([]); + sourceControlImportService.getLocalVariablesFromDb.mockResolvedValue([mock()]); + + // Define a tag that does only exist locally. + // Pulling this would delete it so it should be marked as a conflict. + // Pushing this is conflict free. + const tag = mock({ updatedAt: new Date() }); + tagRepository.find.mockResolvedValue([tag]); + sourceControlImportService.getRemoteTagsAndMappingsFromFile.mockResolvedValue({ + tags: [], + mappings: [], + }); + sourceControlImportService.getLocalTagsAndMappingsFromDb.mockResolvedValue({ + tags: [tag], + mappings: [], + }); + + // ACT + const pullResult = await sourceControlService.getStatus({ + direction: 'pull', + verbose: false, + preferLocalVersion: false, + }); + + const pushResult = await sourceControlService.getStatus({ + direction: 'push', + verbose: false, + preferLocalVersion: false, + }); + + // ASSERT + console.log(pullResult); + console.log(pushResult); + + if (!Array.isArray(pullResult)) { + fail('Expected pullResult to be an array.'); + } + if (!Array.isArray(pushResult)) { + fail('Expected pushResult to be an array.'); + } + + expect(pullResult).toHaveLength(4); + expect(pushResult).toHaveLength(4); + + expect(pullResult.find((i) => i.type === 'workflow')).toHaveProperty('conflict', true); + expect(pushResult.find((i) => i.type === 'workflow')).toHaveProperty('conflict', false); + + expect(pullResult.find((i) => i.type === 'credential')).toHaveProperty('conflict', true); + expect(pushResult.find((i) => i.type === 'credential')).toHaveProperty('conflict', false); + + expect(pullResult.find((i) => i.type === 'variables')).toHaveProperty('conflict', true); + expect(pushResult.find((i) => i.type === 'variables')).toHaveProperty('conflict', false); + + expect(pullResult.find((i) => i.type === 'tags')).toHaveProperty('conflict', true); + expect(pushResult.find((i) => i.type === 'tags')).toHaveProperty('conflict', false); + }); + }); }); diff --git a/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts index 21640dad0e..3c416c0b4c 100644 --- a/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts @@ -9,9 +9,11 @@ import { readFile as fsReadFile } from 'node:fs/promises'; import path from 'path'; import { ActiveWorkflowManager } from '@/active-workflow-manager'; +import { CredentialsService } from '@/credentials/credentials.service'; import type { Project } from '@/databases/entities/project'; import { SharedCredentials } from '@/databases/entities/shared-credentials'; import type { TagEntity } from '@/databases/entities/tag-entity'; +import type { User } from '@/databases/entities/user'; import type { Variables } from '@/databases/entities/variables'; import type { WorkflowTagMapping } from '@/databases/entities/workflow-tag-mapping'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; @@ -25,7 +27,9 @@ import { WorkflowTagMappingRepository } from '@/databases/repositories/workflow- import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import type { IWorkflowToImport } from '@/interfaces'; import { isUniqueConstraintError } from '@/response-helper'; +import { TagService } from '@/services/tag.service'; import { assertNever } from '@/utils'; +import { WorkflowService } from '@/workflows/workflow.service'; import { SOURCE_CONTROL_CREDENTIAL_EXPORT_FOLDER, @@ -62,6 +66,9 @@ export class SourceControlImportService { private readonly variablesRepository: VariablesRepository, private readonly workflowRepository: WorkflowRepository, private readonly workflowTagMappingRepository: WorkflowTagMappingRepository, + private readonly workflowService: WorkflowService, + private readonly credentialsService: CredentialsService, + private readonly tagService: TagService, instanceSettings: InstanceSettings, ) { this.gitFolder = path.join(instanceSettings.n8nFolder, SOURCE_CONTROL_GIT_FOLDER); @@ -500,6 +507,30 @@ export class SourceControlImportService { return result; } + async deleteWorkflowsNotInWorkfolder(user: User, candidates: SourceControlledFile[]) { + for (const candidate of candidates) { + await this.workflowService.delete(user, candidate.id); + } + } + + async deleteCredentialsNotInWorkfolder(user: User, candidates: SourceControlledFile[]) { + for (const candidate of candidates) { + await this.credentialsService.delete(user, candidate.id); + } + } + + async deleteVariablesNotInWorkfolder(candidates: SourceControlledFile[]) { + for (const candidate of candidates) { + await this.variablesService.delete(candidate.id); + } + } + + async deleteTagsNotInWorkfolder(candidates: SourceControlledFile[]) { + for (const candidate of candidates) { + await this.tagService.delete(candidate.id); + } + } + private async findOrCreateOwnerProject(owner: ResourceOwner): Promise { if (typeof owner === 'string' || owner.type === 'personal') { const email = typeof owner === 'string' ? owner : owner.personalEmail; diff --git a/packages/cli/src/environments.ee/source-control/source-control.controller.ee.ts b/packages/cli/src/environments.ee/source-control/source-control.controller.ee.ts index a7dd00d199..1f243a1447 100644 --- a/packages/cli/src/environments.ee/source-control/source-control.controller.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control.controller.ee.ts @@ -191,7 +191,7 @@ export class SourceControlController { @Body payload: PullWorkFolderRequestDto, ): Promise { try { - const result = await this.sourceControlService.pullWorkfolder(req.user.id, payload); + const result = await this.sourceControlService.pullWorkfolder(req.user, payload); res.statusCode = result.statusCode; return result.statusResult; } catch (error) { diff --git a/packages/cli/src/environments.ee/source-control/source-control.service.ee.ts b/packages/cli/src/environments.ee/source-control/source-control.service.ee.ts index 3b330fffa3..2bd040ee3a 100644 --- a/packages/cli/src/environments.ee/source-control/source-control.service.ee.ts +++ b/packages/cli/src/environments.ee/source-control/source-control.service.ee.ts @@ -322,8 +322,44 @@ export class SourceControlService { }; } + private getConflicts(files: SourceControlledFile[]): SourceControlledFile[] { + return files.filter((file) => file.conflict || file.status === 'modified'); + } + + private getWorkflowsToImport(files: SourceControlledFile[]): SourceControlledFile[] { + return files.filter((e) => e.type === 'workflow' && e.status !== 'deleted'); + } + + private getWorkflowsToDelete(files: SourceControlledFile[]): SourceControlledFile[] { + return files.filter((e) => e.type === 'workflow' && e.status === 'deleted'); + } + + private getCredentialsToImport(files: SourceControlledFile[]): SourceControlledFile[] { + return files.filter((e) => e.type === 'credential' && e.status !== 'deleted'); + } + + private getCredentialsToDelete(files: SourceControlledFile[]): SourceControlledFile[] { + return files.filter((e) => e.type === 'credential' && e.status === 'deleted'); + } + + private getTagsToImport(files: SourceControlledFile[]): SourceControlledFile | undefined { + return files.find((e) => e.type === 'tags' && e.status !== 'deleted'); + } + + private getTagsToDelete(files: SourceControlledFile[]): SourceControlledFile[] { + return files.filter((e) => e.type === 'tags' && e.status === 'deleted'); + } + + private getVariablesToImport(files: SourceControlledFile[]): SourceControlledFile | undefined { + return files.find((e) => e.type === 'variables' && e.status !== 'deleted'); + } + + private getVariablesToDelete(files: SourceControlledFile[]): SourceControlledFile[] { + return files.filter((e) => e.type === 'variables' && e.status === 'deleted'); + } + async pullWorkfolder( - userId: User['id'], + user: User, options: PullWorkFolderRequestDto, ): Promise<{ statusCode: number; statusResult: SourceControlledFile[] }> { await this.sanityCheck(); @@ -334,58 +370,51 @@ export class SourceControlService { preferLocalVersion: false, })) as SourceControlledFile[]; - // filter out items that will not effect a local change and thus should not - // trigger a conflict warning in the frontend - const filteredResult = statusResult.filter((e) => { - // locally created credentials will not create a conflict on pull - if (e.status === 'created' && e.location === 'local') { - return false; - } - // remotely deleted credentials will not delete local credentials - if (e.type === 'credential' && e.status === 'deleted') { - return false; - } - return true; - }); - - if (!options.force) { - const possibleConflicts = filteredResult?.filter( - (file) => (file.conflict || file.status === 'modified') && file.type === 'workflow', - ); + if (options.force !== true) { + const possibleConflicts = this.getConflicts(statusResult); if (possibleConflicts?.length > 0) { await this.gitService.resetBranch(); return { statusCode: 409, - statusResult: filteredResult, + statusResult, }; } } - const workflowsToBeImported = statusResult.filter( - (e) => e.type === 'workflow' && e.status !== 'deleted', - ); + const workflowsToBeImported = this.getWorkflowsToImport(statusResult); await this.sourceControlImportService.importWorkflowFromWorkFolder( workflowsToBeImported, - userId, + user.id, ); - - const credentialsToBeImported = statusResult.filter( - (e) => e.type === 'credential' && e.status !== 'deleted', + const workflowsToBeDeleted = this.getWorkflowsToDelete(statusResult); + await this.sourceControlImportService.deleteWorkflowsNotInWorkfolder( + user, + workflowsToBeDeleted, ); + const credentialsToBeImported = this.getCredentialsToImport(statusResult); await this.sourceControlImportService.importCredentialsFromWorkFolder( credentialsToBeImported, - userId, + user.id, + ); + const credentialsToBeDeleted = this.getCredentialsToDelete(statusResult); + await this.sourceControlImportService.deleteCredentialsNotInWorkfolder( + user, + credentialsToBeDeleted, ); - const tagsToBeImported = statusResult.find((e) => e.type === 'tags'); + const tagsToBeImported = this.getTagsToImport(statusResult); if (tagsToBeImported) { await this.sourceControlImportService.importTagsFromWorkFolder(tagsToBeImported); } + const tagsToBeDeleted = this.getTagsToDelete(statusResult); + await this.sourceControlImportService.deleteTagsNotInWorkfolder(tagsToBeDeleted); - const variablesToBeImported = statusResult.find((e) => e.type === 'variables'); + const variablesToBeImported = this.getVariablesToImport(statusResult); if (variablesToBeImported) { await this.sourceControlImportService.importVariablesFromWorkFolder(variablesToBeImported); } + const variablesToBeDeleted = this.getVariablesToDelete(statusResult); + await this.sourceControlImportService.deleteVariablesNotInWorkfolder(variablesToBeDeleted); // #region Tracking Information this.eventService.emit( @@ -396,7 +425,7 @@ export class SourceControlService { return { statusCode: 200, - statusResult: filteredResult, + statusResult, }; } @@ -536,7 +565,7 @@ export class SourceControlService { type: 'workflow', status: options.direction === 'push' ? 'created' : 'deleted', location: options.direction === 'push' ? 'local' : 'remote', - conflict: false, + conflict: options.direction === 'push' ? false : true, file: item.filename, updatedAt: item.updatedAt ?? new Date().toISOString(), }); @@ -617,7 +646,7 @@ export class SourceControlService { type: 'credential', status: options.direction === 'push' ? 'created' : 'deleted', location: options.direction === 'push' ? 'local' : 'remote', - conflict: false, + conflict: options.direction === 'push' ? false : true, file: item.filename, updatedAt: new Date().toISOString(), }); @@ -669,26 +698,47 @@ export class SourceControlService { } }); - if ( - varMissingInLocal.length > 0 || - varMissingInRemote.length > 0 || - varModifiedInEither.length > 0 - ) { - if (options.direction === 'pull' && varRemoteIds.length === 0) { - // if there's nothing to pull, don't show difference as modified - } else { - sourceControlledFiles.push({ - id: 'variables', - name: 'variables', - type: 'variables', - status: 'modified', - location: options.direction === 'push' ? 'local' : 'remote', - conflict: false, - file: getVariablesPath(this.gitFolder), - updatedAt: new Date().toISOString(), - }); - } - } + varMissingInLocal.forEach((item) => { + sourceControlledFiles.push({ + id: item.id, + name: item.key, + type: 'variables', + status: options.direction === 'push' ? 'deleted' : 'created', + location: options.direction === 'push' ? 'local' : 'remote', + conflict: false, + file: getVariablesPath(this.gitFolder), + updatedAt: new Date().toISOString(), + }); + }); + + varMissingInRemote.forEach((item) => { + sourceControlledFiles.push({ + id: item.id, + name: item.key, + type: 'variables', + status: options.direction === 'push' ? 'created' : 'deleted', + location: options.direction === 'push' ? 'local' : 'remote', + // if the we pull and the file is missing in the remote, we will delete + // it locally, which is communicated by marking this as a conflict + conflict: options.direction === 'push' ? false : true, + file: getVariablesPath(this.gitFolder), + updatedAt: new Date().toISOString(), + }); + }); + + varModifiedInEither.forEach((item) => { + sourceControlledFiles.push({ + id: item.id, + name: item.key, + type: 'variables', + status: 'modified', + location: options.direction === 'push' ? 'local' : 'remote', + conflict: true, + file: getVariablesPath(this.gitFolder), + updatedAt: new Date().toISOString(), + }); + }); + return { varMissingInLocal, varMissingInRemote, @@ -743,32 +793,44 @@ export class SourceControlService { ) === -1, ); - if ( - tagsMissingInLocal.length > 0 || - tagsMissingInRemote.length > 0 || - tagsModifiedInEither.length > 0 || - mappingsMissingInLocal.length > 0 || - mappingsMissingInRemote.length > 0 - ) { - if ( - options.direction === 'pull' && - tagMappingsRemote.tags.length === 0 && - tagMappingsRemote.mappings.length === 0 - ) { - // if there's nothing to pull, don't show difference as modified - } else { - sourceControlledFiles.push({ - id: 'mappings', - name: 'tags', - type: 'tags', - status: 'modified', - location: options.direction === 'push' ? 'local' : 'remote', - conflict: false, - file: getTagsPath(this.gitFolder), - updatedAt: lastUpdatedTag[0]?.updatedAt.toISOString(), - }); - } - } + tagsMissingInLocal.forEach((item) => { + sourceControlledFiles.push({ + id: item.id, + name: item.name, + type: 'tags', + status: options.direction === 'push' ? 'deleted' : 'created', + location: options.direction === 'push' ? 'local' : 'remote', + conflict: false, + file: getTagsPath(this.gitFolder), + updatedAt: lastUpdatedTag[0]?.updatedAt.toISOString(), + }); + }); + tagsMissingInRemote.forEach((item) => { + sourceControlledFiles.push({ + id: item.id, + name: item.name, + type: 'tags', + status: options.direction === 'push' ? 'created' : 'deleted', + location: options.direction === 'push' ? 'local' : 'remote', + conflict: options.direction === 'push' ? false : true, + file: getTagsPath(this.gitFolder), + updatedAt: lastUpdatedTag[0]?.updatedAt.toISOString(), + }); + }); + + tagsModifiedInEither.forEach((item) => { + sourceControlledFiles.push({ + id: item.id, + name: item.name, + type: 'tags', + status: 'modified', + location: options.direction === 'push' ? 'local' : 'remote', + conflict: true, + file: getTagsPath(this.gitFolder), + updatedAt: lastUpdatedTag[0]?.updatedAt.toISOString(), + }); + }); + return { tagsMissingInLocal, tagsMissingInRemote, diff --git a/packages/cli/src/public-api/v1/handlers/source-control/source-control.handler.ts b/packages/cli/src/public-api/v1/handlers/source-control/source-control.handler.ts index 09f5a1bc85..4a0729ef6e 100644 --- a/packages/cli/src/public-api/v1/handlers/source-control/source-control.handler.ts +++ b/packages/cli/src/public-api/v1/handlers/source-control/source-control.handler.ts @@ -36,7 +36,7 @@ export = { try { const payload = PullWorkFolderRequestDto.parse(req.body); const sourceControlService = Container.get(SourceControlService); - const result = await sourceControlService.pullWorkfolder(req.user.id, payload); + const result = await sourceControlService.pullWorkfolder(req.user, payload); if (result.statusCode === 200) { Container.get(EventService).emit('source-control-user-pulled-api', { diff --git a/packages/cli/src/services/project.service.ee.ts b/packages/cli/src/services/project.service.ee.ts index 43605939a3..9eb3f8efa1 100644 --- a/packages/cli/src/services/project.service.ee.ts +++ b/packages/cli/src/services/project.service.ee.ts @@ -130,7 +130,7 @@ export class ProjectService { ); } else { for (const sharedCredential of ownedCredentials) { - await credentialsService.delete(sharedCredential.credentials); + await credentialsService.delete(user, sharedCredential.credentials.id); } } diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index 6460348b23..fdb53c1832 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -272,6 +272,13 @@ export class WorkflowService { return updatedWorkflow; } + /** + * Deletes a workflow and returns it. + * + * If the workflow is active this will deactivate the workflow. + * If the user does not have the permissions to delete the workflow this does + * nothing and returns void. + */ async delete(user: User, workflowId: string): Promise { await this.externalHooks.run('workflow.delete', [workflowId]); diff --git a/packages/cli/test/integration/environments/source-control-import.service.test.ts b/packages/cli/test/integration/environments/source-control-import.service.test.ts index 8b774f8fa9..5bfd5d0e79 100644 --- a/packages/cli/test/integration/environments/source-control-import.service.test.ts +++ b/packages/cli/test/integration/environments/source-control-import.service.test.ts @@ -50,6 +50,9 @@ describe('SourceControlImportService', () => { mock(), mock(), mock(), + mock(), + mock(), + mock(), mock({ n8nFolder: '/some-path' }), ); }); diff --git a/packages/editor-ui/src/components/MainSidebarSourceControl.test.ts b/packages/editor-ui/src/components/MainSidebarSourceControl.test.ts index dc3e805b05..15ed1a103c 100644 --- a/packages/editor-ui/src/components/MainSidebarSourceControl.test.ts +++ b/packages/editor-ui/src/components/MainSidebarSourceControl.test.ts @@ -166,95 +166,5 @@ describe('MainSidebarSourceControl', () => { ), ); }); - - it("should show user's feedback when pulling", async () => { - vi.spyOn(sourceControlStore, 'pullWorkfolder').mockResolvedValueOnce([ - { - id: '014da93897f146d2b880-baa374b9d02d', - name: 'vuelfow2', - type: 'workflow', - status: 'created', - location: 'remote', - conflict: false, - file: '/014da93897f146d2b880-baa374b9d02d.json', - updatedAt: '2025-01-09T13:12:24.580Z', - }, - { - id: 'a102c0b9-28ac-43cb-950e-195723a56d54', - name: 'Gmail account', - type: 'credential', - status: 'created', - location: 'remote', - conflict: false, - file: '/a102c0b9-28ac-43cb-950e-195723a56d54.json', - updatedAt: '2025-01-09T13:12:24.586Z', - }, - { - id: 'variables', - name: 'variables', - type: 'variables', - status: 'modified', - location: 'remote', - conflict: false, - file: '/variable_stubs.json', - updatedAt: '2025-01-09T13:12:24.588Z', - }, - { - id: 'mappings', - name: 'tags', - type: 'tags', - status: 'modified', - location: 'remote', - conflict: false, - file: '/tags.json', - updatedAt: '2024-12-16T12:53:12.155Z', - }, - ]); - - const { getAllByRole } = renderComponent({ - pinia, - props: { isCollapsed: false }, - }); - - await userEvent.click(getAllByRole('button')[0]); - await waitFor(() => { - expect(showToast).toHaveBeenNthCalledWith( - 1, - expect.objectContaining({ - title: 'Finish setting up your new variables to use in workflows', - }), - ); - expect(showToast).toHaveBeenNthCalledWith( - 2, - expect.objectContaining({ - title: 'Finish setting up your new credentials to use in workflows', - }), - ); - expect(showToast).toHaveBeenNthCalledWith( - 3, - expect.objectContaining({ - message: '1 Workflow, 1 Credential, Variables, and Tags were pulled', - }), - ); - }); - }); - - it('should show feedback where there are no change to pull', async () => { - vi.spyOn(sourceControlStore, 'pullWorkfolder').mockResolvedValueOnce([]); - - const { getAllByRole } = renderComponent({ - pinia, - props: { isCollapsed: false }, - }); - - await userEvent.click(getAllByRole('button')[0]); - await waitFor(() => { - expect(showMessage).toHaveBeenCalledWith( - expect.objectContaining({ - title: 'Up to date', - }), - ); - }); - }); }); }); diff --git a/packages/editor-ui/src/components/MainSidebarSourceControl.vue b/packages/editor-ui/src/components/MainSidebarSourceControl.vue index af861e93c9..32c1638f96 100644 --- a/packages/editor-ui/src/components/MainSidebarSourceControl.vue +++ b/packages/editor-ui/src/components/MainSidebarSourceControl.vue @@ -1,5 +1,5 @@ { const rootStore = useRootStore(); const settingsStore = useSettingsStore(); - const state = reactive(DEFAULT_STATE); + const state = reactive({ ...DEFAULT_STATE }); const planName = computed(() => state.data.license.planName || DEFAULT_PLAN_NAME); const planId = computed(() => state.data.license.planId); diff --git a/packages/editor-ui/src/views/NodeView.v2.vue b/packages/editor-ui/src/views/NodeView.v2.vue index eee1bf0357..41b765a2d0 100644 --- a/packages/editor-ui/src/views/NodeView.v2.vue +++ b/packages/editor-ui/src/views/NodeView.v2.vue @@ -349,7 +349,9 @@ async function initializeRoute(force = false) { if (!isAlreadyInitialized) { historyStore.reset(); - await loadCredentials(); + if (!isDemoRoute.value) { + await loadCredentials(); + } // If there is no workflow id, treat it as a new workflow if (isNewWorkflowRoute.value || !workflowId.value) { From e05608ac90c7d6d525022c552e1147965249654f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Fri, 24 Jan 2025 14:50:07 +0100 Subject: [PATCH 061/105] refactor(core): Alllow using S3 compatible object stores over http (#12812) --- .../src/configs/external-storage.config.ts | 5 +- packages/@n8n/config/src/index.ts | 1 + packages/@n8n/config/test/config.test.ts | 1 + packages/cli/src/commands/base-command.ts | 34 +--- .../__tests__/object-store.service.test.ts | 171 ++++++++++-------- .../object-store/object-store.service.ee.ts | 106 +++++------ .../src/binary-data/object-store/types.ts | 4 - 7 files changed, 153 insertions(+), 169 deletions(-) diff --git a/packages/@n8n/config/src/configs/external-storage.config.ts b/packages/@n8n/config/src/configs/external-storage.config.ts index 6e5fbd64d8..aff2447d40 100644 --- a/packages/@n8n/config/src/configs/external-storage.config.ts +++ b/packages/@n8n/config/src/configs/external-storage.config.ts @@ -23,11 +23,14 @@ class S3CredentialsConfig { } @Config -class S3Config { +export class S3Config { /** Host of the n8n bucket in S3-compatible external storage @example "s3.us-east-1.amazonaws.com" */ @Env('N8N_EXTERNAL_STORAGE_S3_HOST') host: string = ''; + @Env('N8N_EXTERNAL_STORAGE_S3_PROTOCOL') + protocol: 'http' | 'https' = 'https'; + @Nested bucket: S3BucketConfig; diff --git a/packages/@n8n/config/src/index.ts b/packages/@n8n/config/src/index.ts index f462ef9424..00abdf6d7c 100644 --- a/packages/@n8n/config/src/index.ts +++ b/packages/@n8n/config/src/index.ts @@ -30,6 +30,7 @@ export { TaskRunnersConfig } from './configs/runners.config'; export { SecurityConfig } from './configs/security.config'; export { ExecutionsConfig } from './configs/executions.config'; export { FrontendBetaFeatures, FrontendConfig } from './configs/frontend.config'; +export { S3Config } from './configs/external-storage.config'; export { LOG_SCOPES } from './configs/logging.config'; export type { LogScope } from './configs/logging.config'; diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index d9499d7849..aa115364d9 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -138,6 +138,7 @@ describe('GlobalConfig', () => { externalStorage: { s3: { host: '', + protocol: 'https', bucket: { name: '', region: '', diff --git a/packages/cli/src/commands/base-command.ts b/packages/cli/src/commands/base-command.ts index 9bfd992b3d..5e07b8e350 100644 --- a/packages/cli/src/commands/base-command.ts +++ b/packages/cli/src/commands/base-command.ts @@ -189,42 +189,10 @@ export abstract class BaseCommand extends Command { private async _initObjectStoreService(options = { isReadOnly: false }) { const objectStoreService = Container.get(ObjectStoreService); - const { host, bucket, credentials } = this.globalConfig.externalStorage.s3; - - if (host === '') { - throw new ApplicationError( - 'External storage host not configured. Please set `N8N_EXTERNAL_STORAGE_S3_HOST`.', - ); - } - - if (bucket.name === '') { - throw new ApplicationError( - 'External storage bucket name not configured. Please set `N8N_EXTERNAL_STORAGE_S3_BUCKET_NAME`.', - ); - } - - if (bucket.region === '') { - throw new ApplicationError( - 'External storage bucket region not configured. Please set `N8N_EXTERNAL_STORAGE_S3_BUCKET_REGION`.', - ); - } - - if (credentials.accessKey === '') { - throw new ApplicationError( - 'External storage access key not configured. Please set `N8N_EXTERNAL_STORAGE_S3_ACCESS_KEY`.', - ); - } - - if (credentials.accessSecret === '') { - throw new ApplicationError( - 'External storage access secret not configured. Please set `N8N_EXTERNAL_STORAGE_S3_ACCESS_SECRET`.', - ); - } - this.logger.debug('Initializing object store service'); try { - await objectStoreService.init(host, bucket, credentials); + await objectStoreService.init(); objectStoreService.setReadonly(options.isReadOnly); this.logger.debug('Object store init completed'); diff --git a/packages/core/src/binary-data/object-store/__tests__/object-store.service.test.ts b/packages/core/src/binary-data/object-store/__tests__/object-store.service.test.ts index f5d2924eb5..ca8f4c0c94 100644 --- a/packages/core/src/binary-data/object-store/__tests__/object-store.service.test.ts +++ b/packages/core/src/binary-data/object-store/__tests__/object-store.service.test.ts @@ -1,3 +1,4 @@ +import type { S3Config } from '@n8n/config'; import axios from 'axios'; import { mock } from 'jest-mock-extended'; import { Readable } from 'stream'; @@ -18,6 +19,12 @@ const mockError = new Error('Something went wrong!'); const fileId = 'workflows/ObogjVbqpNOQpiyV/executions/999/binary_data/71f6209b-5d48-41a2-a224-80d529d8bb32'; const mockBuffer = Buffer.from('Test data'); +const s3Config = mock({ + host: mockHost, + bucket: mockBucket, + credentials: mockCredentials, + protocol: 'https', +}); const toDeletionXml = (filename: string) => ` ${filename} @@ -25,10 +32,13 @@ const toDeletionXml = (filename: string) => ` let objectStoreService: ObjectStoreService; +const now = new Date('2024-02-01T01:23:45.678Z'); +jest.useFakeTimers({ now }); + beforeEach(async () => { - objectStoreService = new ObjectStoreService(mock()); + objectStoreService = new ObjectStoreService(mock(), s3Config); mockAxios.request.mockResolvedValueOnce({ status: 200 }); // for checkConnection - await objectStoreService.init(mockHost, mockBucket, mockCredentials); + await objectStoreService.init(); jest.restoreAllMocks(); }); @@ -40,17 +50,17 @@ describe('checkConnection()', () => { await objectStoreService.checkConnection(); - expect(mockAxios.request).toHaveBeenCalledWith( - expect.objectContaining({ - method: 'HEAD', - url: `https://${mockHost}/${mockBucket.name}`, - headers: expect.objectContaining({ - 'X-Amz-Content-Sha256': expect.any(String), - 'X-Amz-Date': expect.any(String), - Authorization: expect.any(String), - }), - }), - ); + expect(mockAxios.request).toHaveBeenCalledWith({ + method: 'HEAD', + url: 'https://s3.us-east-1.amazonaws.com/test-bucket', + headers: { + Host: 's3.us-east-1.amazonaws.com', + 'X-Amz-Content-Sha256': 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855', + 'X-Amz-Date': '20240201T012345Z', + Authorization: + 'AWS4-HMAC-SHA256 Credential=mock-access-key/20240201/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=a5240c11a706e9e6c60e7033a848fc934911b12330e5a4609b0b943f97d9781b', + }, + }); }); it('should throw an error on request failure', async () => { @@ -70,18 +80,17 @@ describe('getMetadata()', () => { await objectStoreService.getMetadata(fileId); - expect(mockAxios.request).toHaveBeenCalledWith( - expect.objectContaining({ - method: 'HEAD', - url: `${mockUrl}/${fileId}`, - headers: expect.objectContaining({ - Host: mockHost, - 'X-Amz-Content-Sha256': expect.any(String), - 'X-Amz-Date': expect.any(String), - Authorization: expect.any(String), - }), - }), - ); + expect(mockAxios.request).toHaveBeenCalledWith({ + method: 'HEAD', + url: `${mockUrl}/${fileId}`, + headers: { + Host: mockHost, + 'X-Amz-Content-Sha256': 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855', + 'X-Amz-Date': '20240201T012345Z', + Authorization: + 'AWS4-HMAC-SHA256 Credential=mock-access-key/20240201/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=60e11c39580ad7dd3a3d549523e7115cdff018540f24c6412ed40053e52a21d0', + }, + }); }); it('should throw an error on request failure', async () => { @@ -101,19 +110,22 @@ describe('put()', () => { await objectStoreService.put(fileId, mockBuffer, metadata); - expect(mockAxios.request).toHaveBeenCalledWith( - expect.objectContaining({ - method: 'PUT', - url: `${mockUrl}/${fileId}`, - headers: expect.objectContaining({ - 'Content-Length': mockBuffer.length, - 'Content-MD5': expect.any(String), - 'x-amz-meta-filename': metadata.fileName, - 'Content-Type': metadata.mimeType, - }), - data: mockBuffer, - }), - ); + expect(mockAxios.request).toHaveBeenCalledWith({ + method: 'PUT', + url: 'https://s3.us-east-1.amazonaws.com/test-bucket/workflows/ObogjVbqpNOQpiyV/executions/999/binary_data/71f6209b-5d48-41a2-a224-80d529d8bb32', + headers: { + 'Content-Length': 9, + 'Content-MD5': 'yh6gLBC3w39CW5t92G1eEQ==', + 'x-amz-meta-filename': 'file.txt', + 'Content-Type': 'text/plain', + Host: 's3.us-east-1.amazonaws.com', + 'X-Amz-Content-Sha256': 'e27c8214be8b7cf5bccc7c08247e3cb0c1514a48ee1f63197fe4ef3ef51d7e6f', + 'X-Amz-Date': '20240201T012345Z', + Authorization: + 'AWS4-HMAC-SHA256 Credential=mock-access-key/20240201/us-east-1/s3/aws4_request, SignedHeaders=content-length;content-md5;content-type;host;x-amz-content-sha256;x-amz-date;x-amz-meta-filename, Signature=6b0fbb51a35dbfa73ac79a964ffc7203b40517a062efc5b01f5f9b7ad553fa7a', + }, + data: mockBuffer, + }); }); it('should block if read-only', async () => { @@ -152,13 +164,18 @@ describe('get()', () => { const result = await objectStoreService.get(fileId, { mode: 'buffer' }); - expect(mockAxios.request).toHaveBeenCalledWith( - expect.objectContaining({ - method: 'GET', - url: `${mockUrl}/${fileId}`, - responseType: 'arraybuffer', - }), - ); + expect(mockAxios.request).toHaveBeenCalledWith({ + method: 'GET', + url: `${mockUrl}/${fileId}`, + responseType: 'arraybuffer', + headers: { + Authorization: + 'AWS4-HMAC-SHA256 Credential=mock-access-key/20240201/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=5f69680786e0ad9f0a0324eb5e4b8fe8c78562afc924489ea423632a2ad2187d', + Host: 's3.us-east-1.amazonaws.com', + 'X-Amz-Content-Sha256': 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855', + 'X-Amz-Date': '20240201T012345Z', + }, + }); expect(Buffer.isBuffer(result)).toBe(true); }); @@ -168,13 +185,18 @@ describe('get()', () => { const result = await objectStoreService.get(fileId, { mode: 'stream' }); - expect(mockAxios.request).toHaveBeenCalledWith( - expect.objectContaining({ - method: 'GET', - url: `${mockUrl}/${fileId}`, - responseType: 'stream', - }), - ); + expect(mockAxios.request).toHaveBeenCalledWith({ + method: 'GET', + url: `${mockUrl}/${fileId}`, + responseType: 'stream', + headers: { + Authorization: + 'AWS4-HMAC-SHA256 Credential=mock-access-key/20240201/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=3ef579ebe2ae89303a89c0faf3ce8ef8e907295dc538d59e95bcf35481c0d03e', + Host: 's3.us-east-1.amazonaws.com', + 'X-Amz-Content-Sha256': 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855', + 'X-Amz-Date': '20240201T012345Z', + }, + }); expect(result instanceof Readable).toBe(true); }); @@ -194,12 +216,17 @@ describe('deleteOne()', () => { await objectStoreService.deleteOne(fileId); - expect(mockAxios.request).toHaveBeenCalledWith( - expect.objectContaining({ - method: 'DELETE', - url: `${mockUrl}/${fileId}`, - }), - ); + expect(mockAxios.request).toHaveBeenCalledWith({ + method: 'DELETE', + url: `${mockUrl}/${fileId}`, + headers: { + Authorization: + 'AWS4-HMAC-SHA256 Credential=mock-access-key/20240201/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=4ad61b1b4da335c6c49772d28e54a301f787d199c9403055b217f890f7aec7fc', + Host: 's3.us-east-1.amazonaws.com', + 'X-Amz-Content-Sha256': 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855', + 'X-Amz-Date': '20240201T012345Z', + }, + }); }); it('should throw an error on request failure', async () => { @@ -232,19 +259,21 @@ describe('deleteMany()', () => { await objectStoreService.deleteMany(prefix); - expect(objectStoreService.list).toHaveBeenCalledWith(prefix); - expect(mockAxios.request).toHaveBeenCalledWith( - expect.objectContaining({ - method: 'POST', - url: `${mockUrl}/?delete`, - headers: expect.objectContaining({ - 'Content-Type': 'application/xml', - 'Content-Length': expect.any(Number), - 'Content-MD5': expect.any(String), - }), - data: toDeletionXml(fileName), - }), - ); + expect(mockAxios.request).toHaveBeenCalledWith({ + method: 'POST', + url: `${mockUrl}?delete=`, + headers: { + 'Content-Type': 'application/xml', + 'Content-Length': 55, + 'Content-MD5': 'ybYDrpQxwYvNIGBQs7PJNA==', + Host: 's3.us-east-1.amazonaws.com', + 'X-Amz-Content-Sha256': '5708e5c935cb75eb528e41ef1548e08b26c5b3b7504b67dc911abc1ff1881f76', + 'X-Amz-Date': '20240201T012345Z', + Authorization: + 'AWS4-HMAC-SHA256 Credential=mock-access-key/20240201/us-east-1/s3/aws4_request, SignedHeaders=content-length;content-md5;content-type;host;x-amz-content-sha256;x-amz-date, Signature=039168f10927b31624f3a5edae8eb4c89405f7c594eb2d6e00257c1462363f99', + }, + data: toDeletionXml(fileName), + }); }); it('should not send a deletion request if no prefix match', async () => { diff --git a/packages/core/src/binary-data/object-store/object-store.service.ee.ts b/packages/core/src/binary-data/object-store/object-store.service.ee.ts index 508477d50e..5561bd61db 100644 --- a/packages/core/src/binary-data/object-store/object-store.service.ee.ts +++ b/packages/core/src/binary-data/object-store/object-store.service.ee.ts @@ -1,6 +1,7 @@ +import { S3Config } from '@n8n/config'; import { Service } from '@n8n/di'; import { sign } from 'aws4'; -import type { Request as Aws4Options, Credentials as Aws4Credentials } from 'aws4'; +import type { Request as Aws4Options } from 'aws4'; import axios from 'axios'; import type { AxiosRequestConfig, AxiosResponse, InternalAxiosRequestConfig, Method } from 'axios'; import { ApplicationError } from 'n8n-workflow'; @@ -9,43 +10,41 @@ import type { Readable } from 'stream'; import { Logger } from '@/logging/logger'; -import type { - Bucket, - ConfigSchemaCredentials, - ListPage, - MetadataResponseHeaders, - RawListPage, - RequestOptions, -} from './types'; +import type { ListPage, MetadataResponseHeaders, RawListPage, RequestOptions } from './types'; import { isStream, parseXml, writeBlockedMessage } from './utils'; import type { BinaryData } from '../types'; @Service() export class ObjectStoreService { - private host = ''; - - private bucket: Bucket = { region: '', name: '' }; - - private credentials: Aws4Credentials = { accessKeyId: '', secretAccessKey: '' }; + private baseUrl: URL; private isReady = false; private isReadOnly = false; - constructor(private readonly logger: Logger) {} + constructor( + private readonly logger: Logger, + private readonly s3Config: S3Config, + ) { + const { host, bucket, protocol } = s3Config; - async init(host: string, bucket: Bucket, credentials: ConfigSchemaCredentials) { - this.host = host; - this.bucket.name = bucket.name; - this.bucket.region = bucket.region; + if (host === '') { + throw new ApplicationError( + 'External storage host not configured. Please set `N8N_EXTERNAL_STORAGE_S3_HOST`.', + ); + } - this.credentials = { - accessKeyId: credentials.accessKey, - secretAccessKey: credentials.accessSecret, - }; + if (bucket.name === '') { + throw new ApplicationError( + 'External storage bucket name not configured. Please set `N8N_EXTERNAL_STORAGE_S3_BUCKET_NAME`.', + ); + } + this.baseUrl = new URL(`${protocol}://${host}/${bucket.name}`); + } + + async init() { await this.checkConnection(); - this.setReady(true); } @@ -65,7 +64,7 @@ export class ObjectStoreService { async checkConnection() { if (this.isReady) return; - return await this.request('HEAD', this.host, this.bucket.name); + return await this.request('HEAD', ''); } /** @@ -84,9 +83,7 @@ export class ObjectStoreService { if (metadata.fileName) headers['x-amz-meta-filename'] = metadata.fileName; if (metadata.mimeType) headers['Content-Type'] = metadata.mimeType; - const path = `/${this.bucket.name}/${filename}`; - - return await this.request('PUT', this.host, path, { headers, body: buffer }); + return await this.request('PUT', filename, { headers, body: buffer }); } /** @@ -97,9 +94,7 @@ export class ObjectStoreService { async get(fileId: string, { mode }: { mode: 'buffer' }): Promise; async get(fileId: string, { mode }: { mode: 'stream' }): Promise; async get(fileId: string, { mode }: { mode: 'stream' | 'buffer' }) { - const path = `${this.bucket.name}/${fileId}`; - - const { data } = await this.request('GET', this.host, path, { + const { data } = await this.request('GET', fileId, { responseType: mode === 'buffer' ? 'arraybuffer' : 'stream', }); @@ -116,9 +111,7 @@ export class ObjectStoreService { * @doc https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingMetadata.html */ async getMetadata(fileId: string) { - const path = `${this.bucket.name}/${fileId}`; - - const response = await this.request('HEAD', this.host, path); + const response = await this.request('HEAD', fileId); return response.headers as MetadataResponseHeaders; } @@ -129,9 +122,7 @@ export class ObjectStoreService { * @doc https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObject.html */ async deleteOne(fileId: string) { - const path = `${this.bucket.name}/${fileId}`; - - return await this.request('DELETE', this.host, path); + return await this.request('DELETE', fileId); } /** @@ -154,9 +145,7 @@ export class ObjectStoreService { 'Content-MD5': createHash('md5').update(body).digest('base64'), }; - const path = `${this.bucket.name}/?delete`; - - return await this.request('POST', this.host, path, { headers, body }); + return await this.request('POST', '', { headers, body, qs: { delete: '' } }); } /** @@ -192,7 +181,7 @@ export class ObjectStoreService { if (nextPageToken) qs['continuation-token'] = nextPageToken; - const { data } = await this.request('GET', this.host, this.bucket.name, { qs }); + const { data } = await this.request('GET', '', { qs }); if (typeof data !== 'string') { throw new TypeError(`Expected XML string but received ${typeof data}`); @@ -215,18 +204,6 @@ export class ObjectStoreService { return page as ListPage; } - private toPath(rawPath: string, qs?: Record) { - const path = rawPath.startsWith('/') ? rawPath : `/${rawPath}`; - - if (!qs) return path; - - const qsParams = Object.entries(qs) - .map(([key, value]) => `${key}=${value}`) - .join('&'); - - return path.concat(`?${qsParams}`); - } - private async blockWrite(filename: string): Promise { const logMessage = writeBlockedMessage(filename); @@ -243,28 +220,37 @@ export class ObjectStoreService { private async request( method: Method, - host: string, rawPath = '', { qs, headers, body, responseType }: RequestOptions = {}, ) { - const path = this.toPath(rawPath, qs); + const url = new URL(this.baseUrl); + if (rawPath && rawPath !== '/') { + url.pathname = `${url.pathname}/${rawPath}`; + } + Object.entries(qs ?? {}).forEach(([key, value]) => { + url.searchParams.set(key, String(value)); + }); const optionsToSign: Aws4Options = { method, service: 's3', - region: this.bucket.region, - host, - path, + region: this.s3Config.bucket.region, + host: this.s3Config.host, + path: `${url.pathname}${url.search}`, }; if (headers) optionsToSign.headers = headers; if (body) optionsToSign.body = body; - const signedOptions = sign(optionsToSign, this.credentials); + const { accessKey, accessSecret } = this.s3Config.credentials; + const signedOptions = sign(optionsToSign, { + accessKeyId: accessKey, + secretAccessKey: accessSecret, + }); const config: AxiosRequestConfig = { method, - url: `https://${host}${path}`, + url: url.toString(), headers: signedOptions.headers, }; diff --git a/packages/core/src/binary-data/object-store/types.ts b/packages/core/src/binary-data/object-store/types.ts index 49726f5c43..20390cf243 100644 --- a/packages/core/src/binary-data/object-store/types.ts +++ b/packages/core/src/binary-data/object-store/types.ts @@ -24,8 +24,6 @@ type Item = { export type ListPage = Omit & { contents: Item[] }; -export type Bucket = { region: string; name: string }; - export type RequestOptions = { qs?: Record; headers?: Record; @@ -38,5 +36,3 @@ export type MetadataResponseHeaders = AxiosResponseHeaders & { 'content-type'?: string; 'x-amz-meta-filename'?: string; } & BinaryData.PreWriteMetadata; - -export type ConfigSchemaCredentials = { accessKey: string; accessSecret: string }; From d48cc36061e1069dd92edc65c0c1fbc32cf89489 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Fri, 24 Jan 2025 15:55:48 +0200 Subject: [PATCH 062/105] feat(editor): Remove bug reporting button from new canvas (no-changelog) (#12831) --- .../src/components/canvas/Canvas.vue | 2 -- .../src/components/canvas/WorkflowCanvas.vue | 2 -- .../buttons/CanvasControlButtons.test.ts | 22 -------------- .../elements/buttons/CanvasControlButtons.vue | 18 ------------ .../CanvasControlButtons.test.ts.snap | 29 ------------------- .../src/plugins/i18n/locales/en.json | 1 - packages/editor-ui/src/views/NodeView.v2.vue | 1 - 7 files changed, 75 deletions(-) diff --git a/packages/editor-ui/src/components/canvas/Canvas.vue b/packages/editor-ui/src/components/canvas/Canvas.vue index be51e1f81f..44a5ceaed4 100644 --- a/packages/editor-ui/src/components/canvas/Canvas.vue +++ b/packages/editor-ui/src/components/canvas/Canvas.vue @@ -85,7 +85,6 @@ const props = withDefaults( readOnly?: boolean; executing?: boolean; keyBindings?: boolean; - showBugReportingButton?: boolean; loading?: boolean; }>(), { @@ -771,7 +770,6 @@ provide(CanvasKey, { :class="$style.canvasControls" :position="controlsPosition" :show-interactive="false" - :show-bug-reporting-button="showBugReportingButton" :zoom="viewport.zoom" @zoom-to-fit="onFitView" @zoom-in="onZoomIn" diff --git a/packages/editor-ui/src/components/canvas/WorkflowCanvas.vue b/packages/editor-ui/src/components/canvas/WorkflowCanvas.vue index d32b974a40..1e6ee9972b 100644 --- a/packages/editor-ui/src/components/canvas/WorkflowCanvas.vue +++ b/packages/editor-ui/src/components/canvas/WorkflowCanvas.vue @@ -23,7 +23,6 @@ const props = withDefaults( eventBus?: EventBus; readOnly?: boolean; executing?: boolean; - showBugReportingButton?: boolean; }>(), { id: 'canvas', @@ -70,7 +69,6 @@ onNodesInitialized(() => { :id="id" :nodes="mappedNodes" :connections="mappedConnections" - :show-bug-reporting-button="showBugReportingButton" :event-bus="eventBus" :read-only="readOnly" v-bind="$attrs" diff --git a/packages/editor-ui/src/components/canvas/elements/buttons/CanvasControlButtons.test.ts b/packages/editor-ui/src/components/canvas/elements/buttons/CanvasControlButtons.test.ts index 9b157da1b8..235f63e085 100644 --- a/packages/editor-ui/src/components/canvas/elements/buttons/CanvasControlButtons.test.ts +++ b/packages/editor-ui/src/components/canvas/elements/buttons/CanvasControlButtons.test.ts @@ -3,12 +3,6 @@ import CanvasControlButtons from './CanvasControlButtons.vue'; import { setActivePinia } from 'pinia'; import { createTestingPinia } from '@pinia/testing'; -const MOCK_URL = 'mock-url'; - -vi.mock('@/composables/useBugReporting', () => ({ - useBugReporting: () => ({ getReportingURL: () => MOCK_URL }), -})); - const renderComponent = createComponentRenderer(CanvasControlButtons); describe('CanvasControlButtons', () => { @@ -17,27 +11,11 @@ describe('CanvasControlButtons', () => { }); it('should render correctly', () => { - const wrapper = renderComponent({ - props: { - showBugReportingButton: true, - }, - }); - - expect(wrapper.getByTestId('zoom-in-button')).toBeVisible(); - expect(wrapper.getByTestId('zoom-out-button')).toBeVisible(); - expect(wrapper.getByTestId('zoom-to-fit')).toBeVisible(); - expect(wrapper.getByTestId('report-bug')).toBeVisible(); - - expect(wrapper.html()).toMatchSnapshot(); - }); - - it('should render correctly without bug reporting button', () => { const wrapper = renderComponent(); expect(wrapper.getByTestId('zoom-in-button')).toBeVisible(); expect(wrapper.getByTestId('zoom-out-button')).toBeVisible(); expect(wrapper.getByTestId('zoom-to-fit')).toBeVisible(); - expect(wrapper.queryByTestId('report-bug')).not.toBeInTheDocument(); expect(wrapper.html()).toMatchSnapshot(); }); diff --git a/packages/editor-ui/src/components/canvas/elements/buttons/CanvasControlButtons.vue b/packages/editor-ui/src/components/canvas/elements/buttons/CanvasControlButtons.vue index 72e1e6251c..5d9a7a3435 100644 --- a/packages/editor-ui/src/components/canvas/elements/buttons/CanvasControlButtons.vue +++ b/packages/editor-ui/src/components/canvas/elements/buttons/CanvasControlButtons.vue @@ -2,18 +2,14 @@ import { Controls } from '@vue-flow/controls'; import KeyboardShortcutTooltip from '@/components/KeyboardShortcutTooltip.vue'; import { computed } from 'vue'; -import { useBugReporting } from '@/composables/useBugReporting'; -import { useTelemetry } from '@/composables/useTelemetry'; import { useI18n } from '@/composables/useI18n'; const props = withDefaults( defineProps<{ zoom?: number; - showBugReportingButton?: boolean; }>(), { zoom: 1, - showBugReportingButton: false, }, ); @@ -24,8 +20,6 @@ const emit = defineEmits<{ 'zoom-to-fit': []; }>(); -const { getReportingURL } = useBugReporting(); -const telemetry = useTelemetry(); const i18n = useI18n(); const isResetZoomVisible = computed(() => props.zoom !== 1); @@ -45,10 +39,6 @@ function onZoomOut() { function onZoomToFit() { emit('zoom-to-fit'); } - -function trackBugReport() { - telemetry.track('User clicked bug report button in canvas', {}, { withPostHog: true }); -} diff --git a/packages/editor-ui/src/components/canvas/elements/buttons/__snapshots__/CanvasControlButtons.test.ts.snap b/packages/editor-ui/src/components/canvas/elements/buttons/__snapshots__/CanvasControlButtons.test.ts.snap index cf8ea70a93..d5e55528b1 100644 --- a/packages/editor-ui/src/components/canvas/elements/buttons/__snapshots__/CanvasControlButtons.test.ts.snap +++ b/packages/editor-ui/src/components/canvas/elements/buttons/__snapshots__/CanvasControlButtons.test.ts.snap @@ -20,35 +20,6 @@ exports[`CanvasControlButtons > should render correctly 1`] = ` - - - -
" -`; - -exports[`CanvasControlButtons > should render correctly without bug reporting button 1`] = ` -"
- - - - - - - - -
" `; diff --git a/packages/editor-ui/src/plugins/i18n/locales/en.json b/packages/editor-ui/src/plugins/i18n/locales/en.json index be4c0e5fec..25eebc86ae 100644 --- a/packages/editor-ui/src/plugins/i18n/locales/en.json +++ b/packages/editor-ui/src/plugins/i18n/locales/en.json @@ -1319,7 +1319,6 @@ "nodeView.redirecting": "Redirecting", "nodeView.refresh": "Refresh", "nodeView.resetZoom": "Reset Zoom", - "nodeView.reportBug": "Report a bug", "nodeView.runButtonText.executeWorkflow": "Test workflow", "nodeView.runButtonText.executingWorkflow": "Executing workflow", "nodeView.runButtonText.waitingForTriggerEvent": "Waiting for trigger event", diff --git a/packages/editor-ui/src/views/NodeView.v2.vue b/packages/editor-ui/src/views/NodeView.v2.vue index 41b765a2d0..459812e946 100644 --- a/packages/editor-ui/src/views/NodeView.v2.vue +++ b/packages/editor-ui/src/views/NodeView.v2.vue @@ -1702,7 +1702,6 @@ onBeforeUnmount(() => { :event-bus="canvasEventBus" :read-only="isCanvasReadOnly" :executing="isWorkflowRunning" - :show-bug-reporting-button="!isDemoRoute || !!executionsStore.activeExecution" :key-bindings="keyBindingsEnabled" @update:nodes:position="onUpdateNodesPosition" @update:node:position="onUpdateNodePosition" From a197fbb21b5642843d8bc3e657049aca99e0729d Mon Sep 17 00:00:00 2001 From: Michael Kret <88898367+michael-radency@users.noreply.github.com> Date: Fri, 24 Jan 2025 15:59:43 +0200 Subject: [PATCH 063/105] feat(Send Email Node): New operation sendAndWait (#12775) --- cypress/e2e/4-node-creator.cy.ts | 1 - .../test/v2/sendAndWait.operation.test.ts | 68 ++++++++++++++ .../nodes/EmailSend/v2/EmailSendV2.node.ts | 30 +++++- .../nodes/EmailSend/v2/descriptions.ts | 23 +++++ .../nodes/EmailSend/v2/send.operation.ts | 94 +------------------ .../EmailSend/v2/sendAndWait.operation.ts | 53 +++++++++++ .../nodes-base/nodes/EmailSend/v2/utils.ts | 70 ++++++++++++++ .../nodes/Google/Gmail/v2/GmailV2.node.ts | 4 +- .../test/v2/node/message/sendAndWait.test.ts | 3 + .../Outlook/v2/actions/node.description.ts | 4 +- .../Microsoft/Outlook/v2/actions/router.ts | 12 +-- .../nodes-base/nodes/Slack/V2/SlackV2.node.ts | 4 +- .../nodes/Telegram/Telegram.node.ts | 4 +- .../utils/sendAndWait/descriptions.ts | 2 +- 14 files changed, 261 insertions(+), 111 deletions(-) create mode 100644 packages/nodes-base/nodes/EmailSend/test/v2/sendAndWait.operation.test.ts create mode 100644 packages/nodes-base/nodes/EmailSend/v2/descriptions.ts create mode 100644 packages/nodes-base/nodes/EmailSend/v2/sendAndWait.operation.ts create mode 100644 packages/nodes-base/nodes/EmailSend/v2/utils.ts diff --git a/cypress/e2e/4-node-creator.cy.ts b/cypress/e2e/4-node-creator.cy.ts index b70e121fd0..a33f16156f 100644 --- a/cypress/e2e/4-node-creator.cy.ts +++ b/cypress/e2e/4-node-creator.cy.ts @@ -135,7 +135,6 @@ describe('Node Creator', () => { 'OpenThesaurus', 'Spontit', 'Vonage', - 'Send Email', 'Toggl Trigger', ]; const doubleActionNode = 'OpenWeatherMap'; diff --git a/packages/nodes-base/nodes/EmailSend/test/v2/sendAndWait.operation.test.ts b/packages/nodes-base/nodes/EmailSend/test/v2/sendAndWait.operation.test.ts new file mode 100644 index 0000000000..f0dfea2042 --- /dev/null +++ b/packages/nodes-base/nodes/EmailSend/test/v2/sendAndWait.operation.test.ts @@ -0,0 +1,68 @@ +import type { MockProxy } from 'jest-mock-extended'; +import { mock } from 'jest-mock-extended'; +import { SEND_AND_WAIT_OPERATION, type IExecuteFunctions } from 'n8n-workflow'; + +import { EmailSendV2, versionDescription } from '../../v2/EmailSendV2.node'; +import * as utils from '../../v2/utils'; + +const transporter = { sendMail: jest.fn() }; + +jest.mock('../../v2/utils', () => { + const originalModule = jest.requireActual('../../v2/utils'); + return { + ...originalModule, + configureTransport: jest.fn(() => transporter), + }; +}); + +describe('Test EmailSendV2, email => sendAndWait', () => { + let emailSendV2: EmailSendV2; + let mockExecuteFunctions: MockProxy; + + beforeEach(() => { + emailSendV2 = new EmailSendV2(versionDescription); + mockExecuteFunctions = mock(); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should send message and put execution to wait', async () => { + const items = [{ json: { data: 'test' } }]; + //node + mockExecuteFunctions.getNodeParameter.mockReturnValueOnce(SEND_AND_WAIT_OPERATION); + + //operation + mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('from@mail.com'); + mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('to@mail.com'); + mockExecuteFunctions.getInstanceId.mockReturnValue('instanceId'); + mockExecuteFunctions.getCredentials.mockResolvedValue({}); + mockExecuteFunctions.putExecutionToWait.mockImplementation(); + mockExecuteFunctions.getInputData.mockReturnValue(items); + + //getSendAndWaitConfig + mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('my message'); + mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('my subject'); + mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('http://localhost/waiting-webhook'); + mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('nodeID'); + mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); + mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('approval'); + + // configureWaitTillDate + mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); //options.limitWaitTime.values + + const result = await emailSendV2.execute.call(mockExecuteFunctions); + + expect(result).toEqual([items]); + expect(utils.configureTransport).toHaveBeenCalledTimes(1); + expect(mockExecuteFunctions.putExecutionToWait).toHaveBeenCalledTimes(1); + + expect(transporter.sendMail).toHaveBeenCalledWith({ + from: 'from@mail.com', + html: expect.stringContaining('href="http://localhost/waiting-webhook/nodeID?approved=true"'), + subject: 'my subject', + to: 'to@mail.com', + }); + }); +}); diff --git a/packages/nodes-base/nodes/EmailSend/v2/EmailSendV2.node.ts b/packages/nodes-base/nodes/EmailSend/v2/EmailSendV2.node.ts index 13caad5b5d..1b1a363875 100644 --- a/packages/nodes-base/nodes/EmailSend/v2/EmailSendV2.node.ts +++ b/packages/nodes-base/nodes/EmailSend/v2/EmailSendV2.node.ts @@ -5,11 +5,15 @@ import type { INodeTypeBaseDescription, INodeTypeDescription, } from 'n8n-workflow'; -import { NodeConnectionType } from 'n8n-workflow'; +import { NodeConnectionType, SEND_AND_WAIT_OPERATION } from 'n8n-workflow'; import * as send from './send.operation'; +import * as sendAndWait from './sendAndWait.operation'; +import { smtpConnectionTest } from './utils'; +import { sendAndWaitWebhooksDescription } from '../../../utils/sendAndWait/descriptions'; +import { sendAndWaitWebhook } from '../../../utils/sendAndWait/utils'; -const versionDescription: INodeTypeDescription = { +export const versionDescription: INodeTypeDescription = { displayName: 'Send Email', name: 'emailSend', icon: 'fa:envelope', @@ -30,6 +34,7 @@ const versionDescription: INodeTypeDescription = { testedBy: 'smtpConnectionTest', }, ], + webhooks: sendAndWaitWebhooksDescription, properties: [ { displayName: 'Resource', @@ -47,7 +52,7 @@ const versionDescription: INodeTypeDescription = { { displayName: 'Operation', name: 'operation', - type: 'hidden', + type: 'options', noDataExpression: true, default: 'send', options: [ @@ -56,9 +61,15 @@ const versionDescription: INodeTypeDescription = { value: 'send', action: 'Send an Email', }, + { + name: 'Send and Wait for Response', + value: SEND_AND_WAIT_OPERATION, + action: 'Send message and wait for response', + }, ], }, ...send.description, + ...sendAndWait.description, ], }; @@ -73,13 +84,22 @@ export class EmailSendV2 implements INodeType { } methods = { - credentialTest: { smtpConnectionTest: send.smtpConnectionTest }, + credentialTest: { smtpConnectionTest }, }; + webhook = sendAndWaitWebhook; + async execute(this: IExecuteFunctions): Promise { let returnData: INodeExecutionData[][] = []; + const operation = this.getNodeParameter('operation', 0) as string; - returnData = await send.execute.call(this); + if (operation === SEND_AND_WAIT_OPERATION) { + returnData = await sendAndWait.execute.call(this); + } + + if (operation === 'send') { + returnData = await send.execute.call(this); + } return returnData; } diff --git a/packages/nodes-base/nodes/EmailSend/v2/descriptions.ts b/packages/nodes-base/nodes/EmailSend/v2/descriptions.ts new file mode 100644 index 0000000000..0416813ae3 --- /dev/null +++ b/packages/nodes-base/nodes/EmailSend/v2/descriptions.ts @@ -0,0 +1,23 @@ +import type { INodeProperties } from 'n8n-workflow'; + +export const fromEmailProperty: INodeProperties = { + displayName: 'From Email', + name: 'fromEmail', + type: 'string', + default: '', + required: true, + placeholder: 'admin@example.com', + description: + 'Email address of the sender. You can also specify a name: Nathan Doe <nate@n8n.io>.', +}; + +export const toEmailProperty: INodeProperties = { + displayName: 'To Email', + name: 'toEmail', + type: 'string', + default: '', + required: true, + placeholder: 'info@example.com', + description: + 'Email address of the recipient. You can also specify a name: Nathan Doe <nate@n8n.io>.', +}; diff --git a/packages/nodes-base/nodes/EmailSend/v2/send.operation.ts b/packages/nodes-base/nodes/EmailSend/v2/send.operation.ts index b8f065a8b7..32f33566fb 100644 --- a/packages/nodes-base/nodes/EmailSend/v2/send.operation.ts +++ b/packages/nodes-base/nodes/EmailSend/v2/send.operation.ts @@ -1,43 +1,22 @@ import type { - ICredentialsDecrypted, - ICredentialTestFunctions, IDataObject, IExecuteFunctions, - INodeCredentialTestResult, INodeExecutionData, INodeProperties, JsonObject, } from 'n8n-workflow'; import { NodeApiError } from 'n8n-workflow'; -import { createTransport } from 'nodemailer'; -import type SMTPTransport from 'nodemailer/lib/smtp-transport'; import { updateDisplayOptions } from '@utils/utilities'; +import { fromEmailProperty, toEmailProperty } from './descriptions'; +import { configureTransport, type EmailSendOptions } from './utils'; import { appendAttributionOption } from '../../../utils/descriptions'; const properties: INodeProperties[] = [ // TODO: Add choice for text as text or html (maybe also from name) - { - displayName: 'From Email', - name: 'fromEmail', - type: 'string', - default: '', - required: true, - placeholder: 'admin@example.com', - description: - 'Email address of the sender. You can also specify a name: Nathan Doe <nate@n8n.io>.', - }, - { - displayName: 'To Email', - name: 'toEmail', - type: 'string', - default: '', - required: true, - placeholder: 'info@example.com', - description: - 'Email address of the recipient. You can also specify a name: Nathan Doe <nate@n8n.io>.', - }, + fromEmailProperty, + toEmailProperty, { displayName: 'Subject', @@ -194,72 +173,11 @@ const displayOptions = { export const description = updateDisplayOptions(displayOptions, properties); -type EmailSendOptions = { - appendAttribution?: boolean; - allowUnauthorizedCerts?: boolean; - attachments?: string; - ccEmail?: string; - bccEmail?: string; - replyTo?: string; -}; - -function configureTransport(credentials: IDataObject, options: EmailSendOptions) { - const connectionOptions: SMTPTransport.Options = { - host: credentials.host as string, - port: credentials.port as number, - secure: credentials.secure as boolean, - }; - - if (credentials.secure === false) { - connectionOptions.ignoreTLS = credentials.disableStartTls as boolean; - } - - if (typeof credentials.hostName === 'string' && credentials.hostName) { - connectionOptions.name = credentials.hostName; - } - - if (credentials.user || credentials.password) { - connectionOptions.auth = { - user: credentials.user as string, - pass: credentials.password as string, - }; - } - - if (options.allowUnauthorizedCerts === true) { - connectionOptions.tls = { - rejectUnauthorized: false, - }; - } - - return createTransport(connectionOptions); -} - -export async function smtpConnectionTest( - this: ICredentialTestFunctions, - credential: ICredentialsDecrypted, -): Promise { - const credentials = credential.data!; - const transporter = configureTransport(credentials, {}); - try { - await transporter.verify(); - return { - status: 'OK', - message: 'Connection successful!', - }; - } catch (error) { - return { - status: 'Error', - message: error.message, - }; - } finally { - transporter.close(); - } -} - export async function execute(this: IExecuteFunctions): Promise { const items = this.getInputData(); const nodeVersion = this.getNode().typeVersion; const instanceId = this.getInstanceId(); + const credentials = await this.getCredentials('smtp'); const returnData: INodeExecutionData[] = []; let item: INodeExecutionData; @@ -274,8 +192,6 @@ export async function execute(this: IExecuteFunctions): Promise { + const fromEmail = this.getNodeParameter('fromEmail', 0) as string; + const toEmail = this.getNodeParameter('toEmail', 0) as string; + + const config = getSendAndWaitConfig(this); + const buttons: string[] = []; + for (const option of config.options) { + buttons.push(createButton(config.url, option.label, option.value, option.style)); + } + + const instanceId = this.getInstanceId(); + + const htmlBody = createEmailBody(config.message, buttons.join('\n'), instanceId); + + const mailOptions: IDataObject = { + from: fromEmail, + to: toEmail, + subject: config.title, + html: htmlBody, + }; + + const credentials = await this.getCredentials('smtp'); + const transporter = configureTransport(credentials, {}); + + await transporter.sendMail(mailOptions); + + const waitTill = configureWaitTillDate(this); + + await this.putExecutionToWait(waitTill); + return [this.getInputData()]; +} diff --git a/packages/nodes-base/nodes/EmailSend/v2/utils.ts b/packages/nodes-base/nodes/EmailSend/v2/utils.ts new file mode 100644 index 0000000000..ce680ab1ab --- /dev/null +++ b/packages/nodes-base/nodes/EmailSend/v2/utils.ts @@ -0,0 +1,70 @@ +import type { + IDataObject, + ICredentialsDecrypted, + ICredentialTestFunctions, + INodeCredentialTestResult, +} from 'n8n-workflow'; +import { createTransport } from 'nodemailer'; +import type SMTPTransport from 'nodemailer/lib/smtp-transport'; + +export type EmailSendOptions = { + appendAttribution?: boolean; + allowUnauthorizedCerts?: boolean; + attachments?: string; + ccEmail?: string; + bccEmail?: string; + replyTo?: string; +}; + +export function configureTransport(credentials: IDataObject, options: EmailSendOptions) { + const connectionOptions: SMTPTransport.Options = { + host: credentials.host as string, + port: credentials.port as number, + secure: credentials.secure as boolean, + }; + + if (credentials.secure === false) { + connectionOptions.ignoreTLS = credentials.disableStartTls as boolean; + } + + if (typeof credentials.hostName === 'string' && credentials.hostName) { + connectionOptions.name = credentials.hostName; + } + + if (credentials.user || credentials.password) { + connectionOptions.auth = { + user: credentials.user as string, + pass: credentials.password as string, + }; + } + + if (options.allowUnauthorizedCerts === true) { + connectionOptions.tls = { + rejectUnauthorized: false, + }; + } + + return createTransport(connectionOptions); +} + +export async function smtpConnectionTest( + this: ICredentialTestFunctions, + credential: ICredentialsDecrypted, +): Promise { + const credentials = credential.data!; + const transporter = configureTransport(credentials, {}); + try { + await transporter.verify(); + return { + status: 'OK', + message: 'Connection successful!', + }; + } catch (error) { + return { + status: 'Error', + message: error.message, + }; + } finally { + transporter.close(); + } +} diff --git a/packages/nodes-base/nodes/Google/Gmail/v2/GmailV2.node.ts b/packages/nodes-base/nodes/Google/Gmail/v2/GmailV2.node.ts index b6c117296c..7c55c11eff 100644 --- a/packages/nodes-base/nodes/Google/Gmail/v2/GmailV2.node.ts +++ b/packages/nodes-base/nodes/Google/Gmail/v2/GmailV2.node.ts @@ -13,7 +13,7 @@ import { labelFields, labelOperations } from './LabelDescription'; import { getGmailAliases, getLabels, getThreadMessages } from './loadOptions'; import { messageFields, messageOperations } from './MessageDescription'; import { threadFields, threadOperations } from './ThreadDescription'; -import { sendAndWaitWebhooks } from '../../../../utils/sendAndWait/descriptions'; +import { sendAndWaitWebhooksDescription } from '../../../../utils/sendAndWait/descriptions'; import type { IEmail } from '../../../../utils/sendAndWait/interfaces'; import { configureWaitTillDate, @@ -69,7 +69,7 @@ const versionDescription: INodeTypeDescription = { }, }, ], - webhooks: sendAndWaitWebhooks, + webhooks: sendAndWaitWebhooksDescription, properties: [ { displayName: 'Authentication', diff --git a/packages/nodes-base/nodes/Microsoft/Outlook/test/v2/node/message/sendAndWait.test.ts b/packages/nodes-base/nodes/Microsoft/Outlook/test/v2/node/message/sendAndWait.test.ts index 7446118275..0375f1d40f 100644 --- a/packages/nodes-base/nodes/Microsoft/Outlook/test/v2/node/message/sendAndWait.test.ts +++ b/packages/nodes-base/nodes/Microsoft/Outlook/test/v2/node/message/sendAndWait.test.ts @@ -51,6 +51,9 @@ describe('Test MicrosoftOutlookV2, message => sendAndWait', () => { mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('approval'); + // configureWaitTillDate + mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); //options.limitWaitTime.values + const result = await microsoftOutlook.execute.call(mockExecuteFunctions); expect(result).toEqual([items]); diff --git a/packages/nodes-base/nodes/Microsoft/Outlook/v2/actions/node.description.ts b/packages/nodes-base/nodes/Microsoft/Outlook/v2/actions/node.description.ts index 5b9596c9d1..1c163d731d 100644 --- a/packages/nodes-base/nodes/Microsoft/Outlook/v2/actions/node.description.ts +++ b/packages/nodes-base/nodes/Microsoft/Outlook/v2/actions/node.description.ts @@ -9,7 +9,7 @@ import * as folder from './folder'; import * as folderMessage from './folderMessage'; import * as message from './message'; import * as messageAttachment from './messageAttachment'; -import { sendAndWaitWebhooks } from '../../../../../utils/sendAndWait/descriptions'; +import { sendAndWaitWebhooksDescription } from '../../../../../utils/sendAndWait/descriptions'; export const description: INodeTypeDescription = { displayName: 'Microsoft Outlook', @@ -31,7 +31,7 @@ export const description: INodeTypeDescription = { required: true, }, ], - webhooks: sendAndWaitWebhooks, + webhooks: sendAndWaitWebhooksDescription, properties: [ { displayName: 'Resource', diff --git a/packages/nodes-base/nodes/Microsoft/Outlook/v2/actions/router.ts b/packages/nodes-base/nodes/Microsoft/Outlook/v2/actions/router.ts index 64b1bffc65..8a05975e56 100644 --- a/packages/nodes-base/nodes/Microsoft/Outlook/v2/actions/router.ts +++ b/packages/nodes-base/nodes/Microsoft/Outlook/v2/actions/router.ts @@ -1,10 +1,5 @@ import type { IExecuteFunctions, INodeExecutionData } from 'n8n-workflow'; -import { - NodeApiError, - NodeOperationError, - SEND_AND_WAIT_OPERATION, - WAIT_INDEFINITELY, -} from 'n8n-workflow'; +import { NodeApiError, NodeOperationError, SEND_AND_WAIT_OPERATION } from 'n8n-workflow'; import * as calendar from './calendar'; import * as contact from './contact'; @@ -15,6 +10,7 @@ import * as folderMessage from './folderMessage'; import * as message from './message'; import * as messageAttachment from './messageAttachment'; import type { MicrosoftOutlook } from './node.type'; +import { configureWaitTillDate } from '../../../../../utils/sendAndWait/utils'; export async function router(this: IExecuteFunctions) { const items = this.getInputData(); @@ -36,7 +32,9 @@ export async function router(this: IExecuteFunctions) { ) { await message[microsoftOutlook.operation].execute.call(this, 0, items); - await this.putExecutionToWait(WAIT_INDEFINITELY); + const waitTill = configureWaitTillDate(this); + + await this.putExecutionToWait(waitTill); return [items]; } diff --git a/packages/nodes-base/nodes/Slack/V2/SlackV2.node.ts b/packages/nodes-base/nodes/Slack/V2/SlackV2.node.ts index 1ff61e3372..15d8e7bfbd 100644 --- a/packages/nodes-base/nodes/Slack/V2/SlackV2.node.ts +++ b/packages/nodes-base/nodes/Slack/V2/SlackV2.node.ts @@ -41,7 +41,7 @@ import { reactionFields, reactionOperations } from './ReactionDescription'; import { starFields, starOperations } from './StarDescription'; import { userFields, userOperations } from './UserDescription'; import { userGroupFields, userGroupOperations } from './UserGroupDescription'; -import { sendAndWaitWebhooks } from '../../../utils/sendAndWait/descriptions'; +import { sendAndWaitWebhooksDescription } from '../../../utils/sendAndWait/descriptions'; import { configureWaitTillDate, getSendAndWaitProperties, @@ -81,7 +81,7 @@ export class SlackV2 implements INodeType { }, }, ], - webhooks: sendAndWaitWebhooks, + webhooks: sendAndWaitWebhooksDescription, properties: [ { displayName: 'Authentication', diff --git a/packages/nodes-base/nodes/Telegram/Telegram.node.ts b/packages/nodes-base/nodes/Telegram/Telegram.node.ts index a51a5f3f3b..6f7e4906e3 100644 --- a/packages/nodes-base/nodes/Telegram/Telegram.node.ts +++ b/packages/nodes-base/nodes/Telegram/Telegram.node.ts @@ -21,7 +21,7 @@ import { getPropertyName, } from './GenericFunctions'; import { appendAttributionOption } from '../../utils/descriptions'; -import { sendAndWaitWebhooks } from '../../utils/sendAndWait/descriptions'; +import { sendAndWaitWebhooksDescription } from '../../utils/sendAndWait/descriptions'; import { configureWaitTillDate, getSendAndWaitProperties, @@ -49,7 +49,7 @@ export class Telegram implements INodeType { required: true, }, ], - webhooks: sendAndWaitWebhooks, + webhooks: sendAndWaitWebhooksDescription, properties: [ { displayName: 'Resource', diff --git a/packages/nodes-base/utils/sendAndWait/descriptions.ts b/packages/nodes-base/utils/sendAndWait/descriptions.ts index 3ec38e6ddd..1549a6cc98 100644 --- a/packages/nodes-base/utils/sendAndWait/descriptions.ts +++ b/packages/nodes-base/utils/sendAndWait/descriptions.ts @@ -1,6 +1,6 @@ import type { IWebhookDescription } from 'n8n-workflow'; -export const sendAndWaitWebhooks: IWebhookDescription[] = [ +export const sendAndWaitWebhooksDescription: IWebhookDescription[] = [ { name: 'default', httpMethod: 'GET', From 31281a3f6b314f3e0efad9f8c02935bd782a5d49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Fri, 24 Jan 2025 16:33:00 +0100 Subject: [PATCH 064/105] ci: Run Frontend tests on `master` only on node 20 (no-changelog) (#12833) --- .github/workflows/ci-master.yml | 1 + .github/workflows/units-tests-dispatch.yml | 6 ++++++ .github/workflows/units-tests-reusable.yml | 5 +++++ 3 files changed, 12 insertions(+) diff --git a/.github/workflows/ci-master.yml b/.github/workflows/ci-master.yml index b6972e1932..c7015c60ef 100644 --- a/.github/workflows/ci-master.yml +++ b/.github/workflows/ci-master.yml @@ -48,6 +48,7 @@ jobs: cacheKey: ${{ github.sha }}-base:build collectCoverage: ${{ matrix.node-version == '20.x' }} ignoreTurboCache: ${{ matrix.node-version == '20.x' }} + skipFrontendTests: ${{ matrix.node-version != '20.x' }} secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/.github/workflows/units-tests-dispatch.yml b/.github/workflows/units-tests-dispatch.yml index 5ad63f000e..72a9db5b6a 100644 --- a/.github/workflows/units-tests-dispatch.yml +++ b/.github/workflows/units-tests-dispatch.yml @@ -12,6 +12,11 @@ on: description: 'PR number to run tests for.' required: false type: number + skipFrontendTests: + description: 'Skip Frontend tests' + required: false + default: false + type: boolean jobs: prepare: @@ -37,3 +42,4 @@ jobs: uses: ./.github/workflows/units-tests-reusable.yml with: ref: ${{ needs.prepare.outputs.branch }} + skipFrontendTests: ${{ inputs.skipFrontendTests }} diff --git a/.github/workflows/units-tests-reusable.yml b/.github/workflows/units-tests-reusable.yml index 62eca74b15..06d560cdbe 100644 --- a/.github/workflows/units-tests-reusable.yml +++ b/.github/workflows/units-tests-reusable.yml @@ -26,6 +26,10 @@ on: required: false default: false type: boolean + skipFrontendTests: + required: false + default: false + type: boolean secrets: CODECOV_TOKEN: description: 'Codecov upload token.' @@ -74,6 +78,7 @@ jobs: run: pnpm test:nodes - name: Test Frontend + if: ${{ !inputs.skipFrontendTests }} run: pnpm test:frontend - name: Upload coverage to Codecov From 6f44004354907a8f95bef109e7dc2fb4ddd335bb Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Fri, 24 Jan 2025 20:30:27 +0000 Subject: [PATCH 065/105] ci: Fix manually dispatching e2e test workflows (#12841) --- .github/workflows/e2e-tests.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 2f63f61bc6..e7400adecb 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -27,11 +27,6 @@ on: description: 'URL to call after workflow is done.' required: false default: '' - node_view_version: - description: 'Node View version to run tests with.' - required: false - default: '1' - type: string jobs: calls-start-url: @@ -51,7 +46,6 @@ jobs: branch: ${{ github.event.inputs.branch || 'master' }} user: ${{ github.event.inputs.user || 'PR User' }} spec: ${{ github.event.inputs.spec || 'e2e/*' }} - node_view_version: ${{ github.event.inputs.node_view_version || '1' }} secrets: CYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }} From 03365f096d3d5c8e3a6537f37cda412959705346 Mon Sep 17 00:00:00 2001 From: Cornelius Suermann Date: Sat, 25 Jan 2025 16:33:40 +0100 Subject: [PATCH 066/105] fix(core): Display the last activated plan name when multiple are activated (#12835) --- packages/cli/src/__tests__/license.test.ts | 37 ++++++++++++++++------ packages/cli/src/license.ts | 4 ++- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/__tests__/license.test.ts b/packages/cli/src/__tests__/license.test.ts index 0e26d0d81c..fb82f312ee 100644 --- a/packages/cli/src/__tests__/license.test.ts +++ b/packages/cli/src/__tests__/license.test.ts @@ -16,6 +16,12 @@ const MOCK_ACTIVATION_KEY = 'activation-key'; const MOCK_FEATURE_FLAG = 'feat:sharing'; const MOCK_MAIN_PLAN_ID = '1b765dc4-d39d-4ffe-9885-c56dd67c4b26'; +function makeDateWithHourOffset(offsetInHours: number): Date { + const date = new Date(); + date.setHours(date.getHours() + offsetInHours); + return date; +} + const licenseConfig: GlobalConfig['license'] = { serverUrl: MOCK_SERVER_URL, autoRenewalEnabled: true, @@ -134,7 +140,7 @@ describe('License', () => { expect(LicenseManager.prototype.getManagementJwt).toHaveBeenCalled(); }); - test('getMainPlan() returns the right entitlement', async () => { + test('getMainPlan() returns the latest main entitlement', async () => { // mock entitlements response License.prototype.getCurrentEntitlements = jest.fn().mockReturnValue([ { @@ -143,8 +149,21 @@ describe('License', () => { productMetadata: {}, features: {}, featureOverrides: {}, - validFrom: new Date(), - validTo: new Date(), + validFrom: makeDateWithHourOffset(-3), + validTo: makeDateWithHourOffset(1), + }, + { + id: '95b9c852-1349-478d-9ad1-b3f55510e488', + productId: '670650f2-72d8-4397-898c-c249906e2cc2', + productMetadata: { + terms: { + isMainPlan: true, + }, + }, + features: {}, + featureOverrides: {}, + validFrom: makeDateWithHourOffset(-2), + validTo: makeDateWithHourOffset(1), }, { id: MOCK_MAIN_PLAN_ID, @@ -156,8 +175,8 @@ describe('License', () => { }, features: {}, featureOverrides: {}, - validFrom: new Date(), - validTo: new Date(), + validFrom: makeDateWithHourOffset(-1), // this is the LATEST / newest plan + validTo: makeDateWithHourOffset(1), }, ]); jest.fn(license.getMainPlan).mockReset(); @@ -175,8 +194,8 @@ describe('License', () => { productMetadata: {}, // has no `productMetadata.terms.isMainPlan`! features: {}, featureOverrides: {}, - validFrom: new Date(), - validTo: new Date(), + validFrom: makeDateWithHourOffset(-1), + validTo: makeDateWithHourOffset(1), }, { id: 'c1aae471-c24e-4874-ad88-b97107de486c', @@ -184,8 +203,8 @@ describe('License', () => { productMetadata: {}, // has no `productMetadata.terms.isMainPlan`! features: {}, featureOverrides: {}, - validFrom: new Date(), - validTo: new Date(), + validFrom: makeDateWithHourOffset(-1), + validTo: makeDateWithHourOffset(1), }, ]); jest.fn(license.getMainPlan).mockReset(); diff --git a/packages/cli/src/license.ts b/packages/cli/src/license.ts index 0b02e7da1a..7e1a443495 100644 --- a/packages/cli/src/license.ts +++ b/packages/cli/src/license.ts @@ -330,7 +330,7 @@ export class License { } /** - * Helper function to get the main plan for a license + * Helper function to get the latest main plan for a license */ getMainPlan(): TEntitlement | undefined { if (!this.manager) { @@ -342,6 +342,8 @@ export class License { return undefined; } + entitlements.sort((a, b) => b.validFrom.getTime() - a.validFrom.getTime()); + return entitlements.find( (entitlement) => (entitlement.productMetadata?.terms as { isMainPlan?: boolean })?.isMainPlan, ); From 02880cc5a61ce88b7533cce0519421b4bab34ef8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Mon, 27 Jan 2025 08:53:36 +0100 Subject: [PATCH 067/105] test(editor): Fix usePushConnection tests on node 18 (no-changelog) (#12832) --- packages/editor-ui/src/composables/usePushConnection.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/editor-ui/src/composables/usePushConnection.test.ts b/packages/editor-ui/src/composables/usePushConnection.test.ts index 63592afd79..76b9c0ee86 100644 --- a/packages/editor-ui/src/composables/usePushConnection.test.ts +++ b/packages/editor-ui/src/composables/usePushConnection.test.ts @@ -35,8 +35,6 @@ vi.mock('@/composables/useToast', () => { }; }); -vi.useFakeTimers(); - describe('usePushConnection()', () => { let router: ReturnType; let pushStore: ReturnType; From a24e4420bb9023f808acd756d125dffaea325968 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Mon, 27 Jan 2025 10:56:26 +0100 Subject: [PATCH 068/105] feat(core): Explicitly report external hook failures (#12830) --- packages/cli/src/external-hooks.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/external-hooks.ts b/packages/cli/src/external-hooks.ts index 8a0ba82c98..f6741dd544 100644 --- a/packages/cli/src/external-hooks.ts +++ b/packages/cli/src/external-hooks.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/no-var-requires */ import { Service } from '@n8n/di'; +import { ErrorReporter } from 'n8n-core'; import { ApplicationError } from 'n8n-workflow'; import config from '@/config'; @@ -20,6 +21,7 @@ export class ExternalHooks { private dbCollections: IExternalHooksFunctions['dbCollections']; constructor( + private readonly errorReporter: ErrorReporter, userRepository: UserRepository, settingsRepository: SettingsRepository, credentialsRepository: CredentialsRepository, @@ -94,7 +96,14 @@ export class ExternalHooks { }; for (const externalHookFunction of this.externalHooks[hookName]) { - await externalHookFunction.apply(externalHookFunctions, hookParameters); + try { + await externalHookFunction.apply(externalHookFunctions, hookParameters); + } catch (cause) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const error = new ApplicationError(`External hook "${hookName}" failed`, { cause }); + this.errorReporter.error(error, { level: 'fatal' }); + throw error; + } } } From 02df25c450a0a384a32d0815d8a2faec7562a8ae Mon Sep 17 00:00:00 2001 From: Eugene Date: Mon, 27 Jan 2025 13:29:14 +0300 Subject: [PATCH 069/105] fix(editor): Add notice when user hits the limit for execution metadata item length (#12676) --- .../utils/__tests__/execution-metadata.test.ts | 11 ++++------- .../utils/execution-metadata.ts | 4 ++-- .../nodes/ExecutionData/ExecutionData.node.ts | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/execution-metadata.test.ts b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/execution-metadata.test.ts index c673a2fcfe..b08e38da5b 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/__tests__/execution-metadata.test.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/__tests__/execution-metadata.test.ts @@ -205,15 +205,12 @@ describe('Execution Metadata functions', () => { }, } as IRunExecutionData; - setWorkflowExecutionMetadata( - executionData, - 'test1', - 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab', - ); + const longValue = 'a'.repeat(513); + + setWorkflowExecutionMetadata(executionData, 'test1', longValue); expect(metadata).toEqual({ - test1: - 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + test1: longValue.slice(0, 512), }); }); }); diff --git a/packages/core/src/execution-engine/node-execution-context/utils/execution-metadata.ts b/packages/core/src/execution-engine/node-execution-context/utils/execution-metadata.ts index 5e7c4954d6..29957b983a 100644 --- a/packages/core/src/execution-engine/node-execution-context/utils/execution-metadata.ts +++ b/packages/core/src/execution-engine/node-execution-context/utils/execution-metadata.ts @@ -38,9 +38,9 @@ export function setWorkflowExecutionMetadata( Logger.error('Custom data key over 50 characters long. Truncating to 50 characters.'); } if (val.length > 255) { - Logger.error('Custom data value over 255 characters long. Truncating to 255 characters.'); + Logger.error('Custom data value over 512 characters long. Truncating to 512 characters.'); } - executionData.resultData.metadata[key.slice(0, 50)] = val.slice(0, 255); + executionData.resultData.metadata[key.slice(0, 50)] = val.slice(0, 512); } export function setAllWorkflowExecutionMetadata( diff --git a/packages/nodes-base/nodes/ExecutionData/ExecutionData.node.ts b/packages/nodes-base/nodes/ExecutionData/ExecutionData.node.ts index f2d113948a..c450bff6dd 100644 --- a/packages/nodes-base/nodes/ExecutionData/ExecutionData.node.ts +++ b/packages/nodes-base/nodes/ExecutionData/ExecutionData.node.ts @@ -83,6 +83,22 @@ export class ExecutionData implements INodeType { ], }, ], + hints: [ + { + type: 'warning', + message: 'Some keys are longer than 50 characters. They will be truncated.', + displayCondition: '={{ $parameter.dataToSave.values.some((x) => x.key.length > 50) }}', + whenToDisplay: 'beforeExecution', + location: 'outputPane', + }, + { + type: 'warning', + message: 'Some values are longer than 512 characters. They will be truncated.', + displayCondition: '={{ $parameter.dataToSave.values.some((x) => x.value.length > 512) }}', + whenToDisplay: 'beforeExecution', + location: 'outputPane', + }, + ], }; async execute(this: IExecuteFunctions): Promise { From 663dfb48defd944f88f0ecc4f3347ea4f8a7c831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Mon, 27 Jan 2025 11:52:18 +0100 Subject: [PATCH 070/105] fix(Postgres PGVector Store Node): Release postgres connections back to the pool (#12723) Co-authored-by: Oleg Ivaniv --- .../VectorStoreInMemory.node.ts | 3 +- .../VectorStorePGVector.node.ts | 10 +- .../VectorStorePinecone.node.ts | 2 +- .../VectorStoreQdrant.node.ts | 2 +- .../VectorStoreSupabase.node.ts | 2 +- .../VectorStoreZep/VectorStoreZep.node.ts | 2 +- .../shared/createVectorStoreNode.ts | 153 ++++++++++-------- 7 files changed, 102 insertions(+), 72 deletions(-) diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreInMemory/VectorStoreInMemory.node.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreInMemory/VectorStoreInMemory.node.ts index 0323478ee8..d08bc2bab2 100644 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreInMemory/VectorStoreInMemory.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreInMemory/VectorStoreInMemory.node.ts @@ -1,3 +1,4 @@ +import type { MemoryVectorStore } from 'langchain/vectorstores/memory'; import type { INodeProperties } from 'n8n-workflow'; import { createVectorStoreNode } from '../shared/createVectorStoreNode'; @@ -20,7 +21,7 @@ const insertFields: INodeProperties[] = [ }, ]; -export class VectorStoreInMemory extends createVectorStoreNode({ +export class VectorStoreInMemory extends createVectorStoreNode({ meta: { displayName: 'In-Memory Vector Store', name: 'vectorStoreInMemory', diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStorePGVector/VectorStorePGVector.node.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStorePGVector/VectorStorePGVector.node.ts index 852453b622..7b2ab7664d 100644 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStorePGVector/VectorStorePGVector.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStorePGVector/VectorStorePGVector.node.ts @@ -213,7 +213,7 @@ class ExtendedPGVectorStore extends PGVectorStore { } } -export class VectorStorePGVector extends createVectorStoreNode({ +export class VectorStorePGVector extends createVectorStoreNode({ meta: { description: 'Work with your data in Postgresql with the PGVector extension', icon: 'file:postgres.svg', @@ -274,6 +274,7 @@ export class VectorStorePGVector extends createVectorStoreNode({ return await ExtendedPGVectorStore.initialize(embeddings, config); }, + async populateVectorStore(context, embeddings, documents, itemIndex) { // NOTE: if you are to create the HNSW index before use, you need to consider moving the distanceStrategy field to // shared fields, because you need that strategy when creating the index. @@ -307,6 +308,11 @@ export class VectorStorePGVector extends createVectorStoreNode({ metadataColumnName: 'metadata', }) as ColumnOptions; - await PGVectorStore.fromDocuments(documents, embeddings, config); + const vectorStore = await PGVectorStore.fromDocuments(documents, embeddings, config); + vectorStore.client?.release(); + }, + + releaseVectorStoreClient(vectorStore) { + vectorStore.client?.release(); }, }) {} diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStorePinecone/VectorStorePinecone.node.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStorePinecone/VectorStorePinecone.node.ts index 5a11acea24..61761a54ec 100644 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStorePinecone/VectorStorePinecone.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStorePinecone/VectorStorePinecone.node.ts @@ -51,7 +51,7 @@ const insertFields: INodeProperties[] = [ }, ]; -export class VectorStorePinecone extends createVectorStoreNode({ +export class VectorStorePinecone extends createVectorStoreNode({ meta: { displayName: 'Pinecone Vector Store', name: 'vectorStorePinecone', diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreQdrant/VectorStoreQdrant.node.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreQdrant/VectorStoreQdrant.node.ts index 988f607ad7..e18cc4988e 100644 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreQdrant/VectorStoreQdrant.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreQdrant/VectorStoreQdrant.node.ts @@ -79,7 +79,7 @@ const retrieveFields: INodeProperties[] = [ }, ]; -export class VectorStoreQdrant extends createVectorStoreNode({ +export class VectorStoreQdrant extends createVectorStoreNode({ meta: { displayName: 'Qdrant Vector Store', name: 'vectorStoreQdrant', diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreSupabase/VectorStoreSupabase.node.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreSupabase/VectorStoreSupabase.node.ts index a462ff8cf6..6ec3975ebd 100644 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreSupabase/VectorStoreSupabase.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreSupabase/VectorStoreSupabase.node.ts @@ -41,7 +41,7 @@ const retrieveFields: INodeProperties[] = [ const updateFields: INodeProperties[] = [...insertFields]; -export class VectorStoreSupabase extends createVectorStoreNode({ +export class VectorStoreSupabase extends createVectorStoreNode({ meta: { description: 'Work with your data in Supabase Vector Store', icon: 'file:supabase.svg', diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreZep/VectorStoreZep.node.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreZep/VectorStoreZep.node.ts index 1372d54f6e..5c973002b2 100644 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreZep/VectorStoreZep.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreZep/VectorStoreZep.node.ts @@ -46,7 +46,7 @@ const retrieveFields: INodeProperties[] = [ }, ]; -export class VectorStoreZep extends createVectorStoreNode({ +export class VectorStoreZep extends createVectorStoreNode({ meta: { displayName: 'Zep Vector Store', name: 'vectorStoreZep', diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/createVectorStoreNode.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/createVectorStoreNode.ts index e393d32e55..ecf2e64a81 100644 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/createVectorStoreNode.ts +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/createVectorStoreNode.ts @@ -49,7 +49,7 @@ interface NodeMeta { operationModes?: NodeOperationMode[]; } -export interface VectorStoreNodeConstructorArgs { +export interface VectorStoreNodeConstructorArgs { meta: NodeMeta; methods?: { listSearch?: { @@ -77,7 +77,8 @@ export interface VectorStoreNodeConstructorArgs { filter: Record | undefined, embeddings: Embeddings, itemIndex: number, - ) => Promise; + ) => Promise; + releaseVectorStoreClient?: (vectorStore: T) => void; } function transformDescriptionForOperationMode( @@ -90,11 +91,15 @@ function transformDescriptionForOperationMode( })); } -function isUpdateSupported(args: VectorStoreNodeConstructorArgs): boolean { +function isUpdateSupported( + args: VectorStoreNodeConstructorArgs, +): boolean { return args.meta.operationModes?.includes('update') ?? false; } -function getOperationModeOptions(args: VectorStoreNodeConstructorArgs): INodePropertyOptions[] { +function getOperationModeOptions( + args: VectorStoreNodeConstructorArgs, +): INodePropertyOptions[] { const enabledOperationModes = args.meta.operationModes ?? DEFAULT_OPERATION_MODES; const allOptions = [ @@ -137,7 +142,9 @@ function getOperationModeOptions(args: VectorStoreNodeConstructorArgs): INodePro ); } -export const createVectorStoreNode = (args: VectorStoreNodeConstructorArgs) => +export const createVectorStoreNode = ( + args: VectorStoreNodeConstructorArgs, +) => class VectorStoreNodeType implements INodeType { description: INodeTypeDescription = { displayName: args.meta.displayName, @@ -334,38 +341,42 @@ export const createVectorStoreNode = (args: VectorStoreNodeConstructorArgs) => embeddings, itemIndex, ); - const prompt = this.getNodeParameter('prompt', itemIndex) as string; - const topK = this.getNodeParameter('topK', itemIndex, 4) as number; + try { + const prompt = this.getNodeParameter('prompt', itemIndex) as string; + const topK = this.getNodeParameter('topK', itemIndex, 4) as number; - const embeddedPrompt = await embeddings.embedQuery(prompt); - const docs = await vectorStore.similaritySearchVectorWithScore( - embeddedPrompt, - topK, - filter, - ); + const embeddedPrompt = await embeddings.embedQuery(prompt); + const docs = await vectorStore.similaritySearchVectorWithScore( + embeddedPrompt, + topK, + filter, + ); - const includeDocumentMetadata = this.getNodeParameter( - 'includeDocumentMetadata', - itemIndex, - true, - ) as boolean; + const includeDocumentMetadata = this.getNodeParameter( + 'includeDocumentMetadata', + itemIndex, + true, + ) as boolean; - const serializedDocs = docs.map(([doc, score]) => { - const document = { - pageContent: doc.pageContent, - ...(includeDocumentMetadata ? { metadata: doc.metadata } : {}), - }; + const serializedDocs = docs.map(([doc, score]) => { + const document = { + pageContent: doc.pageContent, + ...(includeDocumentMetadata ? { metadata: doc.metadata } : {}), + }; - return { - json: { document, score }, - pairedItem: { - item: itemIndex, - }, - }; - }); + return { + json: { document, score }, + pairedItem: { + item: itemIndex, + }, + }; + }); - resultData.push(...serializedDocs); - logAiEvent(this, 'ai-vector-store-searched', { query: prompt }); + resultData.push(...serializedDocs); + logAiEvent(this, 'ai-vector-store-searched', { query: prompt }); + } finally { + args.releaseVectorStoreClient?.(vectorStore); + } } return [resultData]; @@ -427,24 +438,28 @@ export const createVectorStoreNode = (args: VectorStoreNodeConstructorArgs) => itemIndex, ); - const { processedDocuments, serializedDocuments } = await processDocument( - loader, - itemData, - itemIndex, - ); + try { + const { processedDocuments, serializedDocuments } = await processDocument( + loader, + itemData, + itemIndex, + ); - if (processedDocuments?.length !== 1) { - throw new NodeOperationError(this.getNode(), 'Single document per item expected'); + if (processedDocuments?.length !== 1) { + throw new NodeOperationError(this.getNode(), 'Single document per item expected'); + } + + resultData.push(...serializedDocuments); + + // Use ids option to upsert instead of insert + await vectorStore.addDocuments(processedDocuments, { + ids: [documentId], + }); + + logAiEvent(this, 'ai-vector-store-updated'); + } finally { + args.releaseVectorStoreClient?.(vectorStore); } - - resultData.push(...serializedDocuments); - - // Use ids option to upsert instead of insert - await vectorStore.addDocuments(processedDocuments, { - ids: [documentId], - }); - - logAiEvent(this, 'ai-vector-store-updated'); } return [resultData]; @@ -468,6 +483,9 @@ export const createVectorStoreNode = (args: VectorStoreNodeConstructorArgs) => const vectorStore = await args.getVectorStoreClient(this, filter, embeddings, itemIndex); return { response: logWrapper(vectorStore, this), + closeFunction: async () => { + args.releaseVectorStoreClient?.(vectorStore); + }, }; } @@ -491,23 +509,28 @@ export const createVectorStoreNode = (args: VectorStoreNodeConstructorArgs) => embeddings, itemIndex, ); - const embeddedPrompt = await embeddings.embedQuery(input); - const documents = await vectorStore.similaritySearchVectorWithScore( - embeddedPrompt, - topK, - filter, - ); - return documents - .map((document) => { - if (includeDocumentMetadata) { - return { type: 'text', text: JSON.stringify(document[0]) }; - } - return { - type: 'text', - text: JSON.stringify({ pageContent: document[0].pageContent }), - }; - }) - .filter((document) => !!document); + + try { + const embeddedPrompt = await embeddings.embedQuery(input); + const documents = await vectorStore.similaritySearchVectorWithScore( + embeddedPrompt, + topK, + filter, + ); + return documents + .map((document) => { + if (includeDocumentMetadata) { + return { type: 'text', text: JSON.stringify(document[0]) }; + } + return { + type: 'text', + text: JSON.stringify({ pageContent: document[0].pageContent }), + }; + }) + .filter((document) => !!document); + } finally { + args.releaseVectorStoreClient?.(vectorStore); + } }, }); From 648c6f9315b16b885e04716e7e0035a73b358fb0 Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Mon, 27 Jan 2025 12:32:03 +0100 Subject: [PATCH 071/105] fix(editor): Properly set active project in new canvas (#12810) --- cypress/e2e/39-projects.cy.ts | 61 ++++++++++++++++++++ packages/editor-ui/src/views/NodeView.v2.vue | 4 +- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/cypress/e2e/39-projects.cy.ts b/cypress/e2e/39-projects.cy.ts index 04c72c4b2a..492cdde975 100644 --- a/cypress/e2e/39-projects.cy.ts +++ b/cypress/e2e/39-projects.cy.ts @@ -1,3 +1,5 @@ +import { setCredentialValues } from '../composables/modals/credential-modal'; +import { clickCreateNewCredential, selectResourceLocatorItem } from '../composables/ndv'; import * as projects from '../composables/projects'; import { INSTANCE_ADMIN, @@ -534,6 +536,65 @@ describe('Projects', { disableAutoLogin: true }, () => { workflowPage.getters.canvasNodeByName(NOTION_NODE_NAME).should('be.visible').dblclick(); ndv.getters.credentialInput().find('input').should('be.enabled'); }); + + it('should create sub-workflow and credential in the sub-workflow in the same project', () => { + cy.signinAsOwner(); + cy.visit(workflowsPage.url); + + projects.createProject('Dev'); + projects.getProjectTabWorkflows().click(); + workflowsPage.getters.newWorkflowButtonCard().click(); + workflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); + workflowPage.actions.saveWorkflowOnButtonClick(); + workflowPage.actions.addNodeToCanvas('Execute Workflow', true, true); + + cy.window().then((win) => { + cy.stub(win, 'open').callsFake((url) => { + cy.visit(url); + }); + }); + + selectResourceLocatorItem('workflowId', 0, 'Create a'); + + workflowPage.actions.addNodeToCanvas(NOTION_NODE_NAME, true, true); + clickCreateNewCredential(); + setCredentialValues({ + apiKey: 'abc123', + }); + ndv.actions.close(); + workflowPage.actions.saveWorkflowOnButtonClick(); + + projects.getMenuItems().last().click(); + workflowsPage.getters.workflowCards().should('have.length', 2); + + projects.getProjectTabCredentials().click(); + credentialsPage.getters.credentialCards().should('have.length', 1); + }); + + it('should create credential from workflow in the correct project after editor page refresh', () => { + cy.signinAsOwner(); + cy.visit(workflowsPage.url); + + projects.createProject('Dev'); + projects.getProjectTabWorkflows().click(); + workflowsPage.getters.newWorkflowButtonCard().click(); + workflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); + workflowPage.actions.saveWorkflowOnButtonClick(); + + cy.reload(); + + workflowPage.actions.addNodeToCanvas(NOTION_NODE_NAME, true, true); + clickCreateNewCredential(); + setCredentialValues({ + apiKey: 'abc123', + }); + ndv.actions.close(); + workflowPage.actions.saveWorkflowOnButtonClick(); + + projects.getMenuItems().last().click(); + projects.getProjectTabCredentials().click(); + credentialsPage.getters.credentialCards().should('have.length', 1); + }); }); it('should set and update project icon', () => { diff --git a/packages/editor-ui/src/views/NodeView.v2.vue b/packages/editor-ui/src/views/NodeView.v2.vue index 459812e946..537cd11591 100644 --- a/packages/editor-ui/src/views/NodeView.v2.vue +++ b/packages/editor-ui/src/views/NodeView.v2.vue @@ -396,9 +396,7 @@ async function initializeWorkspaceForExistingWorkflow(id: string) { trackOpenWorkflowFromOnboardingTemplate(); } - await projectsStore.setProjectNavActiveIdByWorkflowHomeProject( - editableWorkflow.value.homeProject, - ); + await projectsStore.setProjectNavActiveIdByWorkflowHomeProject(workflowData.homeProject); } catch (error) { toast.showError(error, i18n.baseText('openWorkflow.workflowNotFoundError')); From eabf1609577cd94a6bad5020c34378d840a13bc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 27 Jan 2025 13:44:20 +0100 Subject: [PATCH 072/105] fix(core): Handle max stalled count error better (#12824) --- packages/cli/src/errors/max-stalled-count.error.ts | 11 +++++++---- packages/cli/src/scaling/scaling.service.ts | 5 ----- packages/cli/src/workflow-runner.ts | 10 ++++++++++ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/errors/max-stalled-count.error.ts b/packages/cli/src/errors/max-stalled-count.error.ts index 653ca18eac..38f73023a7 100644 --- a/packages/cli/src/errors/max-stalled-count.error.ts +++ b/packages/cli/src/errors/max-stalled-count.error.ts @@ -5,9 +5,12 @@ import { ApplicationError } from 'n8n-workflow'; */ export class MaxStalledCountError extends ApplicationError { constructor(cause: Error) { - super('The execution has reached the maximum number of attempts and will no longer retry.', { - level: 'warning', - cause, - }); + super( + 'This execution failed to be processed too many times and will no longer retry. To allow this execution to complete, please break down your workflow or scale up your workers or adjust your worker settings.', + { + level: 'warning', + cause, + }, + ); } } diff --git a/packages/cli/src/scaling/scaling.service.ts b/packages/cli/src/scaling/scaling.service.ts index f20d0764c6..7c48ce57e2 100644 --- a/packages/cli/src/scaling/scaling.service.ts +++ b/packages/cli/src/scaling/scaling.service.ts @@ -17,7 +17,6 @@ import config from '@/config'; import { HIGHEST_SHUTDOWN_PRIORITY, Time } from '@/constants'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; import { OnShutdown } from '@/decorators/on-shutdown'; -import { MaxStalledCountError } from '@/errors/max-stalled-count.error'; import { EventService } from '@/events/event.service'; import { OrchestrationService } from '@/services/orchestration.service'; import { assertNever } from '@/utils'; @@ -271,10 +270,6 @@ export class ScalingService { this.queue.on('error', (error: Error) => { if ('code' in error && error.code === 'ECONNREFUSED') return; // handled by RedisClientService.retryStrategy - if (error.message.includes('job stalled more than maxStalledCount')) { - throw new MaxStalledCountError(error); - } - /** * Non-recoverable error on worker start with Redis unavailable. * Even if Redis recovers, worker will remain unable to process jobs. diff --git a/packages/cli/src/workflow-runner.ts b/packages/cli/src/workflow-runner.ts index 148df7edcd..5d80459d93 100644 --- a/packages/cli/src/workflow-runner.ts +++ b/packages/cli/src/workflow-runner.ts @@ -37,6 +37,8 @@ import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-da import { generateFailedExecutionFromError } from '@/workflow-helpers'; import { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service'; +import { MaxStalledCountError } from './errors/max-stalled-count.error'; + @Service() export class WorkflowRunner { private scalingService: ScalingService; @@ -416,6 +418,13 @@ export class WorkflowRunner { try { await job.finished(); } catch (error) { + if ( + error instanceof Error && + error.message.includes('job stalled more than maxStalledCount') + ) { + error = new MaxStalledCountError(error); + } + // We use "getWorkflowHooksWorkerExecuter" as "getWorkflowHooksWorkerMain" does not contain the // "workflowExecuteAfter" which we require. const hooks = getWorkflowHooksWorkerExecuter( @@ -424,6 +433,7 @@ export class WorkflowRunner { data.workflowData, { retryOf: data.retryOf ? data.retryOf.toString() : undefined }, ); + await this.processError(error, new Date(), data.executionMode, executionId, hooks); reject(error); From 6dd90c8764f3c50fc1c44cffaf63c18cafa49803 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milorad=20FIlipovi=C4=87?= Date: Mon, 27 Jan 2025 15:36:59 +0100 Subject: [PATCH 073/105] feat(editor): Add more telemetry for workflow inputs (no-changelog) (#12862) --- .../components/FixedCollectionParameter.vue | 33 ++++++++++++++++++- .../src/components/ParameterInput.vue | 19 +++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/editor-ui/src/components/FixedCollectionParameter.vue b/packages/editor-ui/src/components/FixedCollectionParameter.vue index 59eacfdb10..04f8c9e773 100644 --- a/packages/editor-ui/src/components/FixedCollectionParameter.vue +++ b/packages/editor-ui/src/components/FixedCollectionParameter.vue @@ -18,6 +18,9 @@ import { } from 'n8n-design-system'; import ParameterInputList from './ParameterInputList.vue'; import Draggable from 'vuedraggable'; +import { useWorkflowsStore } from '@/stores/workflows.store'; +import { useNDVStore } from '@/stores/ndv.store'; +import { telemetry } from '@/plugins/telemetry'; const locale = useI18n(); @@ -44,6 +47,9 @@ const emit = defineEmits<{ valueChanged: [value: ValueChangedEvent]; }>(); +const workflowsStore = useWorkflowsStore(); +const ndvStore = useNDVStore(); + const getPlaceholderText = computed(() => { const placeholder = locale.nodeText().placeholder(props.parameter, props.path); return placeholder ? placeholder : locale.baseText('fixedCollectionParameter.choose'); @@ -127,6 +133,13 @@ const getOptionProperties = (optionName: string) => { return undefined; }; +const onAddButtonClick = (optionName: string) => { + optionSelected(optionName); + if (props.parameter.name === 'workflowInputs') { + trackWorkflowInputFieldAdded(); + } +}; + const optionSelected = (optionName: string) => { const option = getOptionProperties(optionName); if (option === undefined) { @@ -183,6 +196,9 @@ const optionSelected = (optionName: string) => { const valueChanged = (parameterData: IUpdateInformation) => { emit('valueChanged', parameterData); + if (props.parameter.name === 'workflowInputs') { + trackWorkflowInputFieldTypeChange(parameterData); + } }; const onDragChange = (optionName: string) => { const parameterData: ValueChangedEvent = { @@ -193,6 +209,21 @@ const onDragChange = (optionName: string) => { emit('valueChanged', parameterData); }; + +const trackWorkflowInputFieldTypeChange = (parameterData: IUpdateInformation) => { + telemetry.track('User changed workflow input field type', { + type: parameterData.value, + workflow_id: workflowsStore.workflow.id, + node_id: ndvStore.activeNode?.id, + }); +}; + +const trackWorkflowInputFieldAdded = () => { + telemetry.track('User added workflow input field', { + workflow_id: workflowsStore.workflow.id, + node_id: ndvStore.activeNode?.id, + }); +};