From 9e5eb79cc46b2d6fe806be00a078225905ee77f5 Mon Sep 17 00:00:00 2001 From: "n8n-cat-bot[bot]" <283985454+n8n-cat-bot[bot]@users.noreply.github.com> Date: Fri, 2 Oct 2026 10:03:04 +0000 Subject: [PATCH] fix: Keep workflow sharing open until changes are saved (#40137) Co-authored-by: n8n-cat-bot[bot] --- .../components/WorkflowShareModal.ee.test.ts | 88 ++++++++++++++++++- .../components/WorkflowShareModal.ee.vue | 10 ++- 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/packages/frontend/editor-ui/src/features/workflows/components/WorkflowShareModal.ee.test.ts b/packages/frontend/editor-ui/src/features/workflows/components/WorkflowShareModal.ee.test.ts index 4dc26f8cdd6..dea8c667f5c 100644 --- a/packages/frontend/editor-ui/src/features/workflows/components/WorkflowShareModal.ee.test.ts +++ b/packages/frontend/editor-ui/src/features/workflows/components/WorkflowShareModal.ee.test.ts @@ -14,6 +14,7 @@ import { createComponentRenderer } from '@/__tests__/render'; import WorkflowShareModal from './WorkflowShareModal.ee.vue'; import { useSettingsStore } from '@n8n/stores/settings.store'; import { useWorkflowsStore } from '@/app/stores/workflows.store'; +import { useWorkflowsListStore } from '@/app/stores/workflowsList.store'; import { useWorkflowsEEStore } from '@/app/stores/workflows.ee.store'; import { useProjectsStore } from '@/features/collaboration/projects/projects.store'; import { useRolesStore } from '@n8n/stores/roles.store'; @@ -22,6 +23,8 @@ import type { ProjectSharingData, } from '@/features/collaboration/projects/projects.types'; import { DEFAULT_PROJECT_SEARCH_PAGE_SIZE } from '@/features/collaboration/projects/projects.utils'; +import { createTestWorkflow } from '@/__tests__/mocks'; +import { MODAL_CONFIRM } from '@/app/constants'; const mockWorkflowDocumentState = reactive({ homeProject: null as ProjectSharingData | null, @@ -39,6 +42,11 @@ vi.mock('@/app/stores/workflowDocument.store', () => ({ })); const mockRouteQuery = reactive>({}); +const { modalBusEmitMock, showErrorMock, confirmMock } = vi.hoisted(() => ({ + modalBusEmitMock: vi.fn(), + showErrorMock: vi.fn(), + confirmMock: vi.fn().mockResolvedValue(true), +})); vi.mock('vue-router', async (importOriginal) => { return { ...(await importOriginal()), @@ -50,12 +58,12 @@ vi.mock('vue-router', async (importOriginal) => { vi.mock('@n8n/composables/useToast', () => ({ useToast: () => ({ showMessage: vi.fn(), - showError: vi.fn(), + showError: showErrorMock, }), })); vi.mock('@/app/composables/useMessage', () => ({ useMessage: () => ({ - confirm: vi.fn().mockResolvedValue(true), + confirm: confirmMock, }), })); const saveAsNewWorkflowMock = vi.fn().mockResolvedValue('abc123'); @@ -73,7 +81,7 @@ vi.mock('@n8n/permissions', async (importOriginal) => ({ })); vi.mock('@n8n/utils/event-bus', () => ({ createEventBus: () => ({ - emit: vi.fn(), + emit: modalBusEmitMock, }), })); @@ -82,8 +90,9 @@ const renderComponent = createComponentRenderer(WorkflowShareModal, { global: { stubs: { Modal: { + props: ['beforeClose'], template: - '
', + '
', }, }, }, @@ -91,6 +100,7 @@ const renderComponent = createComponentRenderer(WorkflowShareModal, { let settingsStore: MockedStore; let workflowsStore: MockedStore; +let workflowsListStore: MockedStore; let workflowsEEStore: MockedStore; let projectsStore: MockedStore; let rolesStore: MockedStore; @@ -98,6 +108,7 @@ describe('WorkflowShareModal.ee.vue', () => { beforeEach(() => { settingsStore = mockedStore(useSettingsStore); workflowsStore = mockedStore(useWorkflowsStore); + workflowsListStore = mockedStore(useWorkflowsListStore); workflowsEEStore = mockedStore(useWorkflowsEEStore); projectsStore = mockedStore(useProjectsStore); rolesStore = mockedStore(useRolesStore); @@ -110,6 +121,8 @@ describe('WorkflowShareModal.ee.vue', () => { mockWorkflowDocumentState.name = ''; // Set up default store state + workflowsStore.isWorkflowSaved = {}; + workflowsListStore.fetchWorkflow.mockReset(); settingsStore.settings.enterprise = { sharing: true } as FrontendSettings['enterprise']; workflowsEEStore.getWorkflowOwnerName = vi.fn(() => 'Owner Name'); projectsStore.personalProjects = [createProjectListItem()]; @@ -139,6 +152,9 @@ describe('WorkflowShareModal.ee.vue', () => { ]; saveAsNewWorkflowMock.mockClear(); + modalBusEmitMock.mockClear(); + showErrorMock.mockClear(); + confirmMock.mockReset().mockResolvedValue(true); }); it('should share new, unsaved workflow after saving it first', async () => { @@ -183,6 +199,70 @@ describe('WorkflowShareModal.ee.vue', () => { }); }); + it('does not close the sharing modal when saving a new workflow fails', async () => { + // IAM-1480: A competing canvas save can make the first workflow create fail. + mockRouteQuery.new = 'true'; + workflowsStore.workflowId = ''; + mockWorkflowDocumentState.homeProject = { + id: 'personal-project-id', + name: 'Personal Project', + type: ProjectTypes.Personal, + icon: null, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }; + saveAsNewWorkflowMock.mockRejectedValueOnce(new Error('Workflow create failed (400)')); + + const saveWorkflowSharedWithSpy = vi.spyOn(workflowsEEStore, 'saveWorkflowSharedWith'); + const { getByTestId, getByRole } = renderComponent({ props: { data: { id: '' } } }); + const projectSelectDropdownItems = await getDropdownItems( + getByTestId('project-sharing-select'), + ); + await userEvent.click(projectSelectDropdownItems[0]); + await userEvent.click(getByRole('button', { name: 'Save' })); + + await waitFor(() => expect(showErrorMock).toHaveBeenCalledOnce()); + expect(saveAsNewWorkflowMock).toHaveBeenCalledOnce(); + expect(saveWorkflowSharedWithSpy).not.toHaveBeenCalled(); + expect(modalBusEmitMock).not.toHaveBeenCalledWith('close'); + }); + + it('saves sharing changes after initial loading when closing the modal', async () => { + const workflowId = 'workflow-1'; + const fetch = Promise.withResolvers>(); + confirmMock.mockResolvedValueOnce(MODAL_CONFIRM); + workflowsStore.isWorkflowSaved = { [workflowId]: true }; + workflowsListStore.fetchWorkflow.mockReturnValue(fetch.promise); + mockWorkflowDocumentState.homeProject = { + id: 'personal-project-id', + name: 'Personal Project', + type: ProjectTypes.Personal, + icon: null, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }; + + const { getByTestId, getByText } = renderComponent({ props: { data: { id: workflowId } } }); + const projectSelectDropdownItems = await getDropdownItems( + getByTestId('project-sharing-select'), + ); + await userEvent.click(projectSelectDropdownItems[0]); + expect(getByText('You made changes')).toBeVisible(); + await userEvent.click(getByTestId('attempt-close')); + + await waitFor(() => expect(confirmMock).toHaveBeenCalledOnce()); + expect(workflowsEEStore.saveWorkflowSharedWith).not.toHaveBeenCalled(); + fetch.resolve(createTestWorkflow({ id: workflowId })); + + await waitFor(() => { + expect(workflowsEEStore.saveWorkflowSharedWith).toHaveBeenCalledWith({ + workflowId, + sharedWithProjects: [projectsStore.personalProjects[0]], + }); + expect(modalBusEmitMock).toHaveBeenCalledWith('close'); + }); + }); + // Covers the quarantined e2e journey // "Workflow Sharing > should share workflow with another user via UI". it('should offer a peer personal project when team projects fill the first page of sharing candidates', async () => { diff --git a/packages/frontend/editor-ui/src/features/workflows/components/WorkflowShareModal.ee.vue b/packages/frontend/editor-ui/src/features/workflows/components/WorkflowShareModal.ee.vue index 4f9c6bd94aa..c5f5ce2c225 100644 --- a/packages/frontend/editor-ui/src/features/workflows/components/WorkflowShareModal.ee.vue +++ b/packages/frontend/editor-ui/src/features/workflows/components/WorkflowShareModal.ee.vue @@ -76,6 +76,7 @@ const workflowSharedWithProjects = computed( workflowDocumentStore.value?.sharedWithProjects ?? workflowListEntry.value?.sharedWithProjects, ); const loading = ref(true); +let initializationPromise: Promise | undefined; const isDirty = ref(false); const modalBus = createEventBus(); const sharedWithProjects = ref([ @@ -209,10 +210,10 @@ const onSave = async () => { title: i18n.baseText('workflows.shareModal.onSave.success.title'), }); isDirty.value = false; + modalBus.emit('close'); } catch (error) { toast.showError(error, i18n.baseText('workflows.shareModal.onSave.error.title')); } finally { - modalBus.emit('close'); loading.value = false; } }; @@ -230,7 +231,9 @@ const onCloseModal = async () => { ); if (shouldSave === MODAL_CONFIRM) { - return await onSave(); + await initializationPromise; + await onSave(); + return false; } } @@ -257,7 +260,8 @@ const initialize = async () => { }; onMounted(async () => { - await initialize(); + initializationPromise = initialize(); + await initializationPromise; }); watch(