diff --git a/cypress/e2e/39-projects.cy.ts b/cypress/e2e/39-projects.cy.ts index af3542154c..0f484771c6 100644 --- a/cypress/e2e/39-projects.cy.ts +++ b/cypress/e2e/39-projects.cy.ts @@ -1,6 +1,10 @@ import { createResource } from '../composables/create'; import { setCredentialValues } from '../composables/modals/credential-modal'; -import { clickCreateNewCredential, selectResourceLocatorItem } from '../composables/ndv'; +import { + clickCreateNewCredential, + getNdvContainer, + selectResourceLocatorItem, +} from '../composables/ndv'; import * as projects from '../composables/projects'; import { EDIT_FIELDS_SET_NODE_NAME, @@ -302,7 +306,8 @@ describe('Projects', { disableAutoLogin: true }, () => { }); selectResourceLocatorItem('workflowId', 0, 'Create a'); - + // Need to wait for the trigger node to auto-open after a delay + getNdvContainer().should('be.visible'); cy.get('body').type('{esc}'); workflowPage.actions.addNodeToCanvas(NOTION_NODE_NAME, true, true); clickCreateNewCredential(); diff --git a/cypress/e2e/44-routing.cy.ts b/cypress/e2e/44-routing.cy.ts index c3129c4e9d..1b2d77ff0e 100644 --- a/cypress/e2e/44-routing.cy.ts +++ b/cypress/e2e/44-routing.cy.ts @@ -3,8 +3,9 @@ import { getCloseSaveChangesButton, getSaveChangesModal, } from '../composables/modals/save-changes-modal'; +import { getNdvContainer } from '../composables/ndv'; import { getHomeButton } from '../composables/projects'; -import { addNodeToCanvas } from '../composables/workflow'; +import { addNodeToCanvas, saveWorkflowOnButtonClick } from '../composables/workflow'; import { getCreateWorkflowButton, getNewWorkflowCardButton, @@ -12,6 +13,7 @@ import { visitWorkflowsPage, } from '../composables/workflowsPage'; import { EDIT_FIELDS_SET_NODE_NAME } from '../constants'; +import { warningToast } from '../pages/notifications'; describe('Workflows', () => { beforeEach(() => { @@ -42,11 +44,11 @@ describe('Workflows', () => { getCreateWorkflowButton().click(); cy.createFixtureWorkflow('Test_workflow_1.json', 'Empty State Card Workflow'); - addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME); + addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME, true, true); // Here we go back via browser rather than the home button // As this already updates the route - cy.go(-1); + cy.go(-2); cy.url().should('include', getWorkflowsPageUrl()); @@ -56,4 +58,79 @@ describe('Workflows', () => { // Confirm the url is back to the workflow cy.url().should('include', '/workflow/'); }); + + it('should correct route when opening and closing NDV via browser button', () => { + getCreateWorkflowButton().click(); + saveWorkflowOnButtonClick(); + cy.url().then((startUrl) => { + cy.createFixtureWorkflow('Test_workflow_1.json', 'Empty State Card Workflow'); + cy.url().should('equal', startUrl); + + addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME, true, true); + + // Getting the generated nodeId is awkward, so we just ensure the URL changed + cy.url().should('not.equal', startUrl); + + // Here we go back via browser rather than the home button + // As this already updates the route + cy.go(-1); + + cy.url().should('equal', startUrl); + }); + }); + + it('should correct route when opening and closing NDV via browser button', () => { + getCreateWorkflowButton().click(); + saveWorkflowOnButtonClick(); + cy.url().then((startUrl) => { + cy.createFixtureWorkflow('Test_workflow_1.json', 'Empty State Card Workflow'); + cy.url().should('equal', startUrl); + + addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME, true, true); + + // Getting the generated nodeId is awkward, so we just ensure the URL changed + cy.url().should('not.equal', startUrl); + + cy.get('body').type('{esc}'); + + cy.url().should('equal', startUrl); + }); + }); + + it('should open ndv via URL', () => { + getCreateWorkflowButton().click(); + saveWorkflowOnButtonClick(); + cy.createFixtureWorkflow('Test_workflow_1.json', 'Empty State Card Workflow'); + + addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME, true, true); + cy.url().then((ndvUrl) => { + cy.get('body').type('{esc}'); + saveWorkflowOnButtonClick(); + + getNdvContainer().should('not.be.visible'); + + cy.visit(ndvUrl); + + getNdvContainer().should('be.visible'); + }); + }); + + it('should open show warning and drop nodeId from URL if it contained an unknown nodeId', () => { + getCreateWorkflowButton().click(); + saveWorkflowOnButtonClick(); + cy.createFixtureWorkflow('Test_workflow_1.json', 'Empty State Card Workflow'); + + addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME, true, true); + cy.url().then((ndvUrl) => { + cy.get('body').type('{esc}'); + saveWorkflowOnButtonClick(); + + getNdvContainer().should('not.be.visible'); + + cy.visit(ndvUrl + 'thisMessesUpTheNodeId'); + + warningToast().should('be.visible'); + cy.url().should('equal', ndvUrl.split('/').slice(0, -1).join('/')); + }); + }); }); diff --git a/packages/@n8n/nodes-langchain/nodes/tools/ToolWorkflow/v2/methods/localResourceMapping.ts b/packages/@n8n/nodes-langchain/nodes/tools/ToolWorkflow/v2/methods/localResourceMapping.ts index 74de9baae2..47f06b570a 100644 --- a/packages/@n8n/nodes-langchain/nodes/tools/ToolWorkflow/v2/methods/localResourceMapping.ts +++ b/packages/@n8n/nodes-langchain/nodes/tools/ToolWorkflow/v2/methods/localResourceMapping.ts @@ -7,8 +7,10 @@ export async function loadSubWorkflowInputs( const { fields, subworkflowInfo, dataMode } = await loadWorkflowInputMappings.bind(this)(); let emptyFieldsNotice: string | undefined; if (fields.length === 0) { - const subworkflowLink = subworkflowInfo?.id - ? `sub-workflow’s trigger` + const { triggerId, workflowId } = subworkflowInfo ?? {}; + const path = (workflowId ?? '') + (triggerId ? `/${triggerId.slice(0, 6)}` : ''); + const subworkflowLink = workflowId + ? `sub-workflow’s trigger` : 'sub-workflow’s trigger'; switch (dataMode) { diff --git a/packages/frontend/editor-ui/src/components/WorkflowSelectorParameterInput/WorkflowSelectorParameterInput.vue b/packages/frontend/editor-ui/src/components/WorkflowSelectorParameterInput/WorkflowSelectorParameterInput.vue index 6104a3a9e7..905263f71e 100644 --- a/packages/frontend/editor-ui/src/components/WorkflowSelectorParameterInput/WorkflowSelectorParameterInput.vue +++ b/packages/frontend/editor-ui/src/components/WorkflowSelectorParameterInput/WorkflowSelectorParameterInput.vue @@ -21,7 +21,7 @@ import { useWorkflowResourcesLocator } from './useWorkflowResourcesLocator'; import { useProjectsStore } from '@/stores/projects.store'; import { useTelemetry } from '@/composables/useTelemetry'; import { VIEWS } from '@/constants'; -import { SAMPLE_SUBWORKFLOW_WORKFLOW } from '@/constants.workflows'; +import { SAMPLE_SUBWORKFLOW_TRIGGER_ID, SAMPLE_SUBWORKFLOW_WORKFLOW } from '@/constants.workflows'; import type { IWorkflowDataCreate } from '@/Interface'; import { useDocumentVisibility } from '@/composables/useDocumentVisibility'; @@ -260,7 +260,10 @@ const onAddResourceClicked = async () => { telemetry.track('User clicked create new sub-workflow button', {}, { withPostHog: true }); const newWorkflow = await workflowsStore.createNewWorkflow(workflow); - const { href } = router.resolve({ name: VIEWS.WORKFLOW, params: { name: newWorkflow.id } }); + const { href } = router.resolve({ + name: VIEWS.WORKFLOW, + params: { name: newWorkflow.id, nodeId: SAMPLE_SUBWORKFLOW_TRIGGER_ID }, + }); await reloadWorkflows(); onInputChange(newWorkflow.id); hideDropdown(); diff --git a/packages/frontend/editor-ui/src/constants.workflows.ts b/packages/frontend/editor-ui/src/constants.workflows.ts index 4f5fa86596..7c28a179b7 100644 --- a/packages/frontend/editor-ui/src/constants.workflows.ts +++ b/packages/frontend/editor-ui/src/constants.workflows.ts @@ -1,11 +1,12 @@ import { NodeConnectionTypes } from 'n8n-workflow'; import type { INodeUi, IWorkflowDataCreate } from './Interface'; +export const SAMPLE_SUBWORKFLOW_TRIGGER_ID = 'c055762a-8fe7-4141-a639-df2372f30060'; export const SAMPLE_SUBWORKFLOW_WORKFLOW: IWorkflowDataCreate = { name: 'My Sub-Workflow', nodes: [ { - id: 'c055762a-8fe7-4141-a639-df2372f30060', + id: SAMPLE_SUBWORKFLOW_TRIGGER_ID, typeVersion: 1.1, name: 'When Executed by Another Workflow', type: 'n8n-nodes-base.executeWorkflowTrigger', diff --git a/packages/frontend/editor-ui/src/plugins/i18n/locales/en.json b/packages/frontend/editor-ui/src/plugins/i18n/locales/en.json index 1fc90bcfa6..bf51f8ab61 100644 --- a/packages/frontend/editor-ui/src/plugins/i18n/locales/en.json +++ b/packages/frontend/editor-ui/src/plugins/i18n/locales/en.json @@ -1463,6 +1463,8 @@ "nodeView.showMessage.debug.content": "You can make edits and re-execute. Once you're done, unpin the the first node.", "nodeView.showMessage.debug.missingNodes.title": "Some execution data wasn't imported", "nodeView.showMessage.debug.missingNodes.content": "Some nodes have been deleted or renamed or added to the workflow since the execution ran.", + "nodeView.showMessage.ndvUrl.missingNodes.title": "Node not found", + "nodeView.showMessage.ndvUrl.missingNodes.content": "URL contained a reference to an unknown node. Maybe the node was deleted?", "nodeView.stopCurrentExecution": "Stop current execution", "nodeView.stopWaitingForWebhookCall": "Stop waiting for webhook call", "nodeView.stoppingCurrentExecution": "Stopping current execution", diff --git a/packages/frontend/editor-ui/src/router.test.ts b/packages/frontend/editor-ui/src/router.test.ts index 8af15d9da5..ca577e9ece 100644 --- a/packages/frontend/editor-ui/src/router.test.ts +++ b/packages/frontend/editor-ui/src/router.test.ts @@ -45,6 +45,8 @@ describe('router', () => { ['/workflow', VIEWS.NEW_WORKFLOW], ['/workflow/new', VIEWS.NEW_WORKFLOW], ['/workflow/R9JFXwkUCL1jZBuw', VIEWS.WORKFLOW], + ['/workflow/R9JFXwkUCL1jZBuw/myNodeId', VIEWS.WORKFLOW], + ['/workflow/R9JFXwkUCL1jZBuw/398-1ewq213', VIEWS.WORKFLOW], ['/workflow/R9JFXwkUCL1jZBuw/executions/29021', VIEWS.EXECUTION_PREVIEW], ['/workflows/templates/R9JFXwkUCL1jZBuw', VIEWS.TEMPLATE_IMPORT], ['/workflows/demo', VIEWS.DEMO], diff --git a/packages/frontend/editor-ui/src/router.ts b/packages/frontend/editor-ui/src/router.ts index 1b11163e81..4c525d1389 100644 --- a/packages/frontend/editor-ui/src/router.ts +++ b/packages/frontend/editor-ui/src/router.ts @@ -389,7 +389,7 @@ export const routes: RouteRecordRaw[] = [ }, }, { - path: '/workflow/:name', + path: '/workflow/:name/:nodeId?', name: VIEWS.WORKFLOW, components: { default: NodeView, diff --git a/packages/frontend/editor-ui/src/stores/workflows.store.test.ts b/packages/frontend/editor-ui/src/stores/workflows.store.test.ts index f57047cba6..04ac8e7f90 100644 --- a/packages/frontend/editor-ui/src/stores/workflows.store.test.ts +++ b/packages/frontend/editor-ui/src/stores/workflows.store.test.ts @@ -815,6 +815,44 @@ describe('useWorkflowsStore', () => { ); }, ); + + describe('findNodeByPartialId', () => { + test.each([ + [[], 'D', undefined], + [['A', 'B', 'C'], 'D', undefined], + [['A', 'B', 'C'], 'B', 1], + [['AA', 'BB', 'CC'], 'B', 1], + [['AA', 'BB', 'BC'], 'B', 1], + [['AA', 'BB', 'BC'], 'BC', 2], + ] as Array<[string[], string, number | undefined]>)( + 'with input %s , %s returns node with index %s', + (ids, id, expectedIndex) => { + workflowsStore.workflow.nodes = ids.map((x) => ({ id: x }) as never); + + expect(workflowsStore.findNodeByPartialId(id)).toBe( + workflowsStore.workflow.nodes[expectedIndex ?? -1], + ); + }, + ); + }); + + describe('getPartialIdForNode', () => { + test.each([ + [[], 'Alphabet', 'Alphabet'], + [['Alphabet'], 'Alphabet', 'Alphab'], + [['Alphabet', 'Alphabeta'], 'Alphabeta', 'Alphabeta'], + [['Alphabet', 'Alphabeta', 'Alphabetagamma'], 'Alphabet', 'Alphabet'], + [['Alphabet', 'Alphabeta', 'Alphabetagamma'], 'Alphabeta', 'Alphabeta'], + [['Alphabet', 'Alphabeta', 'Alphabetagamma'], 'Alphabetagamma', 'Alphabetag'], + ] as Array<[string[], string, string]>)( + 'with input %s , %s returns %s', + (ids, id, expected) => { + workflowsStore.workflow.nodes = ids.map((x) => ({ id: x }) as never); + + expect(workflowsStore.getPartialIdForNode(id)).toBe(expected); + }, + ); + }); }); function getMockEditFieldsNode() { diff --git a/packages/frontend/editor-ui/src/stores/workflows.store.ts b/packages/frontend/editor-ui/src/stores/workflows.store.ts index aad599961f..4fb3892515 100644 --- a/packages/frontend/editor-ui/src/stores/workflows.store.ts +++ b/packages/frontend/editor-ui/src/stores/workflows.store.ts @@ -343,6 +343,22 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { return workflow.value.nodes.find((node) => node.id === nodeId); } + // Finds the full id for a given partial id for a node, relying on order for uniqueness in edge cases + function findNodeByPartialId(partialId: string): INodeUi | undefined { + return workflow.value.nodes.find((node) => node.id.startsWith(partialId)); + } + + // Finds a uniquely identifying partial id for a node, relying on order for uniqueness in edge cases + function getPartialIdForNode(fullId: string): string { + for (let length = 6; length < fullId.length; ++length) { + const partialId = fullId.slice(0, length); + if (workflow.value.nodes.filter((x) => x.id.startsWith(partialId)).length === 1) { + return partialId; + } + } + return fullId; + } + function getNodesByIds(nodeIds: string[]): INodeUi[] { return nodeIds.map(getNodeById).filter(isPresent); } @@ -1875,6 +1891,8 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { setNodes, setConnections, markExecutionAsStopped, + findNodeByPartialId, + getPartialIdForNode, totalWorkflowCount, }; }); diff --git a/packages/frontend/editor-ui/src/views/NodeView.vue b/packages/frontend/editor-ui/src/views/NodeView.vue index 86cd2f07ea..f48ec1a7e6 100644 --- a/packages/frontend/editor-ui/src/views/NodeView.vue +++ b/packages/frontend/editor-ui/src/views/NodeView.vue @@ -234,6 +234,7 @@ const workflowId = computed(() => { ? undefined : workflowIdParam; }); +const routeNodeId = computed(() => route.params.nodeId as string | undefined); const isNewWorkflowRoute = computed(() => route.name === VIEWS.NEW_WORKFLOW || !workflowId.value); const isWorkflowRoute = computed(() => !!route?.meta?.nodeView || isDemoRoute.value); @@ -1606,6 +1607,23 @@ function showAddFirstStepIfEnabled() { * Routing */ +function updateNodeRoute(nodeId: string) { + const nodeUi = workflowsStore.findNodeByPartialId(nodeId); + if (nodeUi) { + setNodeActive(nodeUi.id); + } else { + toast.showToast({ + title: i18n.baseText('nodeView.showMessage.ndvUrl.missingNodes.title'), + message: i18n.baseText('nodeView.showMessage.ndvUrl.missingNodes.content'), + type: 'warning', + }); + void router.replace({ + name: route.name, + params: { name: workflowId.value }, + }); + } +} + watch( () => route.name, async (newRouteName, oldRouteName) => { @@ -1617,6 +1635,35 @@ watch( }, ); +// This keeps the selected node in sync if the URL is updated +watch( + () => route.params.nodeId, + async (newId) => { + if (typeof newId !== 'string' || newId === '') ndvStore.activeNodeName = null; + else { + updateNodeRoute(newId); + } + }, +); + +// This keeps URL in sync if the activeNode is changed +watch( + () => ndvStore.activeNode, + async (val) => { + // This is just out of caution + if (!([VIEWS.WORKFLOW] as string[]).includes(String(route.name))) return; + + // Route params default to '' instead of undefined if not present + const nodeId = val?.id ? workflowsStore.getPartialIdForNode(val?.id) : ''; + + if (nodeId !== route.params.nodeId) { + await router.push({ + name: route.name, + params: { name: workflowId.value, nodeId }, + }); + } + }, +); onBeforeRouteLeave(async (to, from, next) => { const toNodeViewTab = getNodeViewTab(to); @@ -1689,6 +1736,13 @@ onMounted(() => { void externalHooks.run('nodeView.mount').catch(() => {}); + // A delay here makes opening the NDV a bit less jarring + setTimeout(() => { + if (routeNodeId.value) { + updateNodeRoute(routeNodeId.value); + } + }, 500); + emitPostMessageReady(); }); diff --git a/packages/nodes-base/nodes/ExecuteWorkflow/ExecuteWorkflow/methods/localResourceMapping.ts b/packages/nodes-base/nodes/ExecuteWorkflow/ExecuteWorkflow/methods/localResourceMapping.ts index e6770dde6f..dd323ad5e4 100644 --- a/packages/nodes-base/nodes/ExecuteWorkflow/ExecuteWorkflow/methods/localResourceMapping.ts +++ b/packages/nodes-base/nodes/ExecuteWorkflow/ExecuteWorkflow/methods/localResourceMapping.ts @@ -8,8 +8,10 @@ export async function loadSubWorkflowInputs( const { fields, dataMode, subworkflowInfo } = await loadWorkflowInputMappings.bind(this)(); let emptyFieldsNotice: string | undefined; if (fields.length === 0) { - const subworkflowLink = subworkflowInfo?.id - ? `sub-workflow’s trigger` + const { triggerId, workflowId } = subworkflowInfo ?? {}; + const path = (workflowId ?? '') + (triggerId ? `/${triggerId.slice(0, 6)}` : ''); + const subworkflowLink = workflowId + ? `sub-workflow’s trigger` : 'sub-workflow’s trigger'; switch (dataMode) { diff --git a/packages/nodes-base/utils/workflowInputsResourceMapping/GenericFunctions.ts b/packages/nodes-base/utils/workflowInputsResourceMapping/GenericFunctions.ts index 1ca9e7034e..d05e3bb769 100644 --- a/packages/nodes-base/utils/workflowInputsResourceMapping/GenericFunctions.ts +++ b/packages/nodes-base/utils/workflowInputsResourceMapping/GenericFunctions.ts @@ -96,7 +96,12 @@ export function getFieldEntries(context: IWorkflowNodeContext): { if (Array.isArray(result)) { const dataMode = String(inputSource); const workflow = context.getWorkflow(); - return { fields: result, dataMode, subworkflowInfo: { id: workflow.id } }; + const node = context.getNode(); + return { + fields: result, + dataMode, + subworkflowInfo: { workflowId: workflow.id, triggerId: node.id }, + }; } throw new NodeOperationError(context.getNode(), result); } @@ -153,7 +158,7 @@ export async function loadWorkflowInputMappings( const nodeLoadContext = await this.getWorkflowNodeContext(EXECUTE_WORKFLOW_TRIGGER_NODE_TYPE); let fields: ResourceMapperField[] = []; let dataMode: string = PASSTHROUGH; - let subworkflowInfo: { id?: string } | undefined; + let subworkflowInfo: { workflowId?: string; triggerId?: string } | undefined; if (nodeLoadContext) { const fieldValues = getFieldEntries(nodeLoadContext); diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index dc9cc5530e..8c6ee6917e 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -2658,7 +2658,7 @@ export interface ResourceMapperFields { export interface WorkflowInputsData { fields: ResourceMapperField[]; dataMode: string; - subworkflowInfo?: { id?: string }; + subworkflowInfo?: { workflowId?: string; triggerId?: string }; } export interface ResourceMapperField {