fix(editor): Fix new, unsaved workflow sharing (#16740)

packages/frontend/editor-ui/src/components/DuplicateWorkflowDialog.vue

@@ -105,7 +105,7 @@ const save = async (): Promise<void> => {
 			);
 		}
 
-		const saved = await workflowSaving.saveAsNewWorkflow({
+		const workflowId = await workflowSaving.saveAsNewWorkflow({
 			name: workflowName,
 			data: workflowToUpdate,
 			tags: currentTagIds.value,
@@ -115,7 +115,7 @@ const save = async (): Promise<void> => {
 			parentFolderId,
 		});
 
-		if (saved) {
+		if (!!workflowId) {
 			closeDialog();
 			telemetry.track('User duplicated workflow', {
 				old_workflow_id: currentWorkflowId,

packages/frontend/editor-ui/src/components/WorkflowShareModal.ee.test.ts

@@ -0,0 +1,137 @@
+import { reactive } from 'vue';
+import { createTestingPinia } from '@pinia/testing';
+import userEvent from '@testing-library/user-event';
+import { waitFor } from '@testing-library/vue';
+import { useRouter } from 'vue-router';
+import type { FrontendSettings } from '@n8n/api-types';
+import { createProjectListItem } from '@/__tests__/data/projects';
+import type { MockedStore } from '@/__tests__/utils';
+import { mockedStore, getDropdownItems } from '@/__tests__/utils';
+import { createComponentRenderer } from '@/__tests__/render';
+import WorkflowShareModal from './WorkflowShareModal.ee.vue';
+import { PLACEHOLDER_EMPTY_WORKFLOW_ID } from '@/constants';
+import { useSettingsStore } from '@/stores/settings.store';
+import { useWorkflowsStore } from '@/stores/workflows.store';
+import { useWorkflowsEEStore } from '@/stores/workflows.ee.store';
+import { useProjectsStore } from '@/stores/projects.store';
+import { useRolesStore } from '@/stores/roles.store';
+import { useWorkflowSaving } from '@/composables/useWorkflowSaving';
+
+vi.mock('vue-router', async (importOriginal) => {
+	const query = reactive({});
+	return {
+		...(await importOriginal()),
+		useRoute: () => ({
+			query,
+		}),
+	};
+});
+vi.mock('@/composables/useToast', () => ({
+	useToast: () => ({
+		showMessage: vi.fn(),
+		showError: vi.fn(),
+	}),
+}));
+vi.mock('@/composables/useMessage', () => ({
+	useMessage: () => ({
+		confirm: vi.fn().mockResolvedValue(true),
+	}),
+}));
+vi.mock('@/composables/useWorkflowSaving', () => {
+	const saveAsNewWorkflow = vi.fn().mockResolvedValue('abc123');
+	return {
+		useWorkflowSaving: () => ({
+			saveAsNewWorkflow,
+		}),
+	};
+});
+vi.mock('@n8n/permissions', () => ({
+	getResourcePermissions: () => ({
+		workflow: { share: true },
+	}),
+}));
+vi.mock('@n8n/utils/event-bus', () => ({
+	createEventBus: () => ({
+		emit: vi.fn(),
+	}),
+}));
+
+const renderComponent = createComponentRenderer(WorkflowShareModal, {
+	pinia: createTestingPinia(),
+	global: {
+		stubs: {
+			Modal: {
+				template:
+					'<div role="dialog"><slot name="header" /><slot name="content" /><slot name="footer" /></div>',
+			},
+		},
+	},
+});
+
+let settingsStore: MockedStore<typeof useSettingsStore>;
+let workflowsStore: MockedStore<typeof useWorkflowsStore>;
+let workflowsEEStore: MockedStore<typeof useWorkflowsEEStore>;
+let projectsStore: MockedStore<typeof useProjectsStore>;
+let rolesStore: MockedStore<typeof useRolesStore>;
+let workflowSaving: ReturnType<typeof useWorkflowSaving>;
+
+describe('WorkflowShareModal.ee.vue', () => {
+	beforeEach(() => {
+		settingsStore = mockedStore(useSettingsStore);
+		workflowsStore = mockedStore(useWorkflowsStore);
+		workflowsEEStore = mockedStore(useWorkflowsEEStore);
+		projectsStore = mockedStore(useProjectsStore);
+		rolesStore = mockedStore(useRolesStore);
+
+		// Set up default store state
+		settingsStore.settings.enterprise = { sharing: true } as FrontendSettings['enterprise'];
+		workflowsEEStore.getWorkflowOwnerName = vi.fn(() => 'Owner Name');
+		projectsStore.personalProjects = [createProjectListItem()];
+		rolesStore.processedWorkflowRoles = [
+			{ name: 'Editor', role: 'workflow:editor', scopes: [], licensed: false },
+			{ name: 'Owner', role: 'workflow:owner', scopes: [], licensed: false },
+		];
+
+		workflowSaving = useWorkflowSaving({ router: useRouter() });
+	});
+
+	it('should share new, unsaved workflow after saving it first', async () => {
+		workflowsStore.workflow = {
+			id: PLACEHOLDER_EMPTY_WORKFLOW_ID,
+			name: 'My workflow',
+			active: false,
+			isArchived: false,
+			createdAt: new Date().toISOString(),
+			updatedAt: new Date().toISOString(),
+			versionId: '',
+			scopes: [],
+			nodes: [],
+			connections: {},
+		};
+
+		const saveWorkflowSharedWithSpy = vi.spyOn(workflowsEEStore, 'saveWorkflowSharedWith');
+
+		const props = {
+			data: { id: PLACEHOLDER_EMPTY_WORKFLOW_ID },
+		};
+		const { getByTestId, getByRole, getByText } = renderComponent({ props });
+
+		expect(getByRole('button', { name: 'Save' })).toBeDisabled();
+
+		const projectSelect = getByTestId('project-sharing-select');
+		const projectSelectDropdownItems = await getDropdownItems(projectSelect);
+		await userEvent.click(projectSelectDropdownItems[0]);
+
+		expect(getByText('You made changes')).toBeVisible();
+		expect(getByRole('button', { name: 'Save' })).toBeEnabled();
+
+		await userEvent.click(getByRole('button', { name: 'Save' }));
+		await waitFor(() => {
+			expect(workflowSaving.saveAsNewWorkflow).toHaveBeenCalled();
+			expect(saveWorkflowSharedWithSpy).toHaveBeenCalledWith({
+				workflowId: 'abc123',
+				sharedWithProjects: [projectsStore.personalProjects[0]],
+			});
+		});
+	});
+});

packages/frontend/editor-ui/src/components/WorkflowShareModal.ee.vue

@@ -1,7 +1,7 @@
 <script setup lang="ts">
 import { computed, watch, onMounted, ref } from 'vue';
+import { useRouter, useRoute } from 'vue-router';
 import { createEventBus } from '@n8n/utils/event-bus';
-
 import Modal from './Modal.vue';
 import {
 	EnterpriseEditionFeature,
@@ -12,7 +12,6 @@ import {
 import { getResourcePermissions } from '@n8n/permissions';
 import { useMessage } from '@/composables/useMessage';
 import { useToast } from '@/composables/useToast';
-import { nodeViewEventBus } from '@/event-bus';
 import { useSettingsStore } from '@/stores/settings.store';
 import { useUIStore } from '@/stores/ui.store';
 import { useUsersStore } from '@/stores/users.store';
@@ -28,6 +27,7 @@ import { useRolesStore } from '@/stores/roles.store';
 import { usePageRedirectionHelper } from '@/composables/usePageRedirectionHelper';
 import { useI18n } from '@n8n/i18n';
 import { telemetry } from '@/plugins/telemetry';
+import { useWorkflowSaving } from '@/composables/useWorkflowSaving';
 
 const props = defineProps<{
 	data: {
@@ -49,6 +49,9 @@ const toast = useToast();
 const message = useMessage();
 const pageRedirectionHelper = usePageRedirectionHelper();
 const i18n = useI18n();
+const router = useRouter();
+const route = useRoute();
+const workflowSaving = useWorkflowSaving({ router });
 
 const workflow = ref(
 	data.id === PLACEHOLDER_EMPTY_WORKFLOW_ID
@@ -141,15 +144,16 @@ const onSave = async () => {
 	loading.value = true;
 
 	const saveWorkflowPromise = async () => {
-		return await new Promise<string>((resolve) => {
 		if (workflow.value.id === PLACEHOLDER_EMPTY_WORKFLOW_ID) {
-				nodeViewEventBus.emit('saveWorkflow', () => {
+			const parentFolderId = route.query.folderId as string | undefined;
-					resolve(workflow.value.id);
+			const workflowId = await workflowSaving.saveAsNewWorkflow({ parentFolderId });
-				});
+			if (!workflowId) {
-			} else {
+				throw new Error(i18n.baseText('workflows.shareModal.onSave.error.title'));
-				resolve(workflow.value.id);
+			}
+			return workflowId;
+		} else {
+			return workflow.value.id;
 		}
-		});
 	};
 
 	try {

packages/frontend/editor-ui/src/composables/useWorkflowSaving.ts

@@ -15,7 +15,7 @@ import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers';
 import { useWorkflowsStore } from '@/stores/workflows.store';
 import { useSourceControlStore } from '@/stores/sourceControl.store';
 import { useCanvasStore } from '@/stores/canvas.store';
-import type { IUpdateInformation, NotificationOptions } from '@/Interface';
+import type { IUpdateInformation, IWorkflowDb, NotificationOptions } from '@/Interface';
 import type { ITag } from '@n8n/rest-api-client/api/tags';
 import type { WorkflowDataCreate, WorkflowDataUpdate } from '@n8n/rest-api-client/api/workflows';
 import type { IDataObject, INode, IWorkflowSettings } from 'n8n-workflow';
@@ -173,7 +173,7 @@ export function useWorkflowSaving({ router }: { router: ReturnType<typeof useRou
 		const parentFolderId = router.currentRoute.value.query.parentFolderId as string;
 
 		if (!currentWorkflow || ['new', PLACEHOLDER_EMPTY_WORKFLOW_ID].includes(currentWorkflow)) {
-			return await saveAsNewWorkflow({ name, tags, parentFolderId }, redirect);
+			return !!(await saveAsNewWorkflow({ name, tags, parentFolderId }, redirect));
 		}
 
 		// Workflow exists already so update it
@@ -305,7 +305,7 @@ export function useWorkflowSaving({ router }: { router: ReturnType<typeof useRou
 			data?: WorkflowDataCreate;
 		} = {},
 		redirect = true,
-	): Promise<boolean> {
+	): Promise<IWorkflowDb['id'] | null> {
 		try {
 			uiStore.addActiveAction('workflowSaving');
 
@@ -353,7 +353,7 @@ export function useWorkflowSaving({ router }: { router: ReturnType<typeof useRou
 				});
 				window.open(routeData.href, '_blank');
 				uiStore.removeActiveAction('workflowSaving');
-				return true;
+				return workflowData.id;
 			}
 
 			// workflow should not be active if there is live webhook with the same path
@@ -411,7 +411,7 @@ export function useWorkflowSaving({ router }: { router: ReturnType<typeof useRou
 			void useExternalHooks().run('workflow.afterUpdate', { workflowData });
 
 			getCurrentWorkflow(true); // refresh cache
-			return true;
+			return workflowData.id;
 		} catch (e) {
 			uiStore.removeActiveAction('workflowSaving');
 
@@ -421,7 +421,7 @@ export function useWorkflowSaving({ router }: { router: ReturnType<typeof useRou
 				type: 'error',
 			});
 
-			return false;
+			return null;
 		}
 	}
 

packages/frontend/editor-ui/src/event-bus/node-view.ts

@@ -1,9 +1,6 @@
 import { createEventBus } from '@n8n/utils/event-bus';
 import type { IDataObject } from 'n8n-workflow';
 
-/** Callback function called after workflow has been save */
-export type OnSaveWorkflowFn = () => void;
-
 export interface NodeViewEventBusEvents {
 	/** Command to create a new workflow */
 	newWorkflow: never;
@@ -11,9 +8,6 @@ export interface NodeViewEventBusEvents {
 	/** Command to open the chat */
 	openChat: never;
 
-	/** Command to save the current workflow */
-	saveWorkflow: OnSaveWorkflowFn;
-
 	/** Command to import a workflow from given data */
 	importWorkflowData: IDataObject;