From 7ce09733864c68d11f97c4e7fa22afb9c37cfdcb Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Wed, 13 May 2026 20:19:54 +0100 Subject: [PATCH] fix: prevent first user template popup when following shared link (#12024) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary When a user who has not used the app before first loads up, they are presented with the template selection dialog. This conflicts when the first-time user visits the app via a share link - both the share & template dialog are triggered. ## Changes - **What**: - Skip the templates browser when share param is in URL - Tests - Add `url` to `setup`/`goto` to allow specifying the `share` parameter ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12024-fix-prevent-first-user-template-popup-when-following-shared-link-3586d73d365081cbbcecdba45a1ad1ea) by [Unito](https://www.unito.io) --- browser_tests/fixtures/ComfyPage.ts | 10 ++- browser_tests/tests/templates.spec.ts | 43 +++++++++++ .../useWorkflowPersistenceV2.test.ts | 77 ++++++++++++++++++- .../composables/useWorkflowPersistenceV2.ts | 12 ++- 4 files changed, 133 insertions(+), 9 deletions(-) diff --git a/browser_tests/fixtures/ComfyPage.ts b/browser_tests/fixtures/ComfyPage.ts index 6e94756e73..fc767db226 100644 --- a/browser_tests/fixtures/ComfyPage.ts +++ b/browser_tests/fixtures/ComfyPage.ts @@ -285,10 +285,12 @@ export class ComfyPage { async setup({ clearStorage = true, - mockReleases = true + mockReleases = true, + url }: { clearStorage?: boolean mockReleases?: boolean + url?: string } = {}) { // Mock release endpoint to prevent changelog popups (before navigation) if (mockReleases) { @@ -320,7 +322,7 @@ export class ComfyPage { }, this.id) } - await this.goto() + await this.goto({ url }) await this.page.waitForFunction(() => document.fonts.ready) await this.waitForAppReady() @@ -347,8 +349,8 @@ export class ComfyPage { return assetPath(fileName) } - async goto() { - await this.page.goto(this.url) + async goto({ url }: { url?: string } = {}) { + await this.page.goto(url ? new URL(url, this.url).toString() : this.url) } async nextFrame() { diff --git a/browser_tests/tests/templates.spec.ts b/browser_tests/tests/templates.spec.ts index 54973e44dc..06b53a9bbc 100644 --- a/browser_tests/tests/templates.spec.ts +++ b/browser_tests/tests/templates.spec.ts @@ -106,6 +106,49 @@ test.describe('Templates', { tag: ['@slow', '@workflow'] }, () => { await expect(comfyPage.templates.content).toBeVisible() }) + test('dialog should not be shown when first-time user opens a shared workflow link', async ({ + comfyPage + }) => { + await comfyPage.page.route( + '**/workflows/published/test-share-id', + async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + share_id: 'test-share-id', + workflow_id: 'wf-1', + name: 'Shared Workflow', + listed: true, + publish_time: new Date().toISOString(), + workflow_json: { + version: 0.4, + nodes: [], + links: [], + groups: [], + config: {}, + extra: {} + }, + assets: [] + }) + }) + } + ) + + await comfyPage.settings.setSetting('Comfy.TutorialCompleted', false) + + await comfyPage.setup({ + clearStorage: true, + url: '/?share=test-share-id' + }) + + await expect( + comfyPage.page.getByRole('heading', { name: 'Open shared workflow' }) + ).toBeVisible() + + await expect(comfyPage.templates.content).toBeHidden() + }) + test('Uses proper locale files for templates', async ({ comfyPage }) => { await comfyPage.settings.setSetting('Comfy.Locale', 'fr') diff --git a/src/platform/workflow/persistence/composables/useWorkflowPersistenceV2.test.ts b/src/platform/workflow/persistence/composables/useWorkflowPersistenceV2.test.ts index a5fe0fe6e5..852b3c9beb 100644 --- a/src/platform/workflow/persistence/composables/useWorkflowPersistenceV2.test.ts +++ b/src/platform/workflow/persistence/composables/useWorkflowPersistenceV2.test.ts @@ -76,15 +76,25 @@ vi.mock( }) ) +const commandStoreMocks = vi.hoisted(() => ({ + execute: vi.fn() +})) + vi.mock('@/stores/commandStore', () => ({ useCommandStore: () => ({ - execute: vi.fn() + execute: commandStoreMocks.execute }) })) +const routeMocks = vi.hoisted(() => ({ + query: {} as Record +})) + vi.mock('vue-router', () => ({ useRoute: () => ({ - query: {} + get query() { + return routeMocks.query + } }), useRouter: () => ({ replace: vi.fn() @@ -97,13 +107,30 @@ vi.mock('@/composables/auth/useCurrentUser', () => ({ }) })) +const preservedQueryMocks = vi.hoisted(() => ({ + payloads: {} as Record | undefined> +})) + vi.mock('@/platform/navigation/preservedQueryManager', () => ({ hydratePreservedQuery: vi.fn(), - mergePreservedQueryIntoQuery: vi.fn(() => null) + mergePreservedQueryIntoQuery: vi.fn( + (namespace: string, query: Record = {}) => { + const payload = preservedQueryMocks.payloads[namespace] + if (!payload) return undefined + const next: Record = { ...query } + let changed = false + for (const [key, value] of Object.entries(payload)) { + if (typeof next[key] === 'string') continue + next[key] = value + changed = true + } + return changed ? next : undefined + } + ) })) vi.mock('@/platform/navigation/preservedQueryNamespaces', () => ({ - PRESERVED_QUERY_NAMESPACES: { TEMPLATE: 'template' } + PRESERVED_QUERY_NAMESPACES: { TEMPLATE: 'template', SHARE: 'share' } })) vi.mock('@/platform/distribution/types', () => ({ @@ -178,6 +205,9 @@ describe('useWorkflowPersistenceV2', () => { mocks.apiMock.removeEventListener.mockImplementation(() => {}) openWorkflowMock.mockReset() loadBlankWorkflowMock.mockReset() + commandStoreMocks.execute.mockReset() + routeMocks.query = {} + preservedQueryMocks.payloads = {} }) afterEach(() => { @@ -357,4 +387,43 @@ describe('useWorkflowPersistenceV2', () => { expect(openWorkflowMock).not.toHaveBeenCalled() }) }) + + describe('loadDefaultWorkflow', () => { + it('opens templates browser for first-time users', async () => { + const { initializeWorkflow } = useWorkflowPersistenceV2() + await initializeWorkflow() + + expect(loadBlankWorkflowMock).toHaveBeenCalled() + expect(commandStoreMocks.execute).toHaveBeenCalledWith( + 'Comfy.BrowseTemplates' + ) + }) + + it('does not open templates browser when share param is in URL', async () => { + routeMocks.query = { share: 'test-share-id' } + + const { initializeWorkflow } = useWorkflowPersistenceV2() + await initializeWorkflow() + + expect(loadBlankWorkflowMock).toHaveBeenCalled() + expect(commandStoreMocks.execute).not.toHaveBeenCalledWith( + 'Comfy.BrowseTemplates' + ) + }) + + it('does not open templates browser when share intent is preserved across /user-select redirect', async () => { + // No-local-user flow: ?share=... was captured into sessionStorage and the + // URL query was dropped during the /user-select redirect before + // initializeWorkflow() runs. + preservedQueryMocks.payloads.share = { share: 'test-share-id' } + + const { initializeWorkflow } = useWorkflowPersistenceV2() + await initializeWorkflow() + + expect(loadBlankWorkflowMock).toHaveBeenCalled() + expect(commandStoreMocks.execute).not.toHaveBeenCalledWith( + 'Comfy.BrowseTemplates' + ) + }) + }) }) diff --git a/src/platform/workflow/persistence/composables/useWorkflowPersistenceV2.ts b/src/platform/workflow/persistence/composables/useWorkflowPersistenceV2.ts index 4091472d94..561399bccf 100644 --- a/src/platform/workflow/persistence/composables/useWorkflowPersistenceV2.ts +++ b/src/platform/workflow/persistence/composables/useWorkflowPersistenceV2.ts @@ -48,6 +48,7 @@ export function useWorkflowPersistenceV2() { const sharedWorkflowUrlLoader = useSharedWorkflowUrlLoader() const templateUrlLoader = useTemplateUrlLoader() const TEMPLATE_NAMESPACE = PRESERVED_QUERY_NAMESPACES.TEMPLATE + const SHARE_NAMESPACE = PRESERVED_QUERY_NAMESPACES.SHARE const draftStore = useWorkflowDraftStoreV2() const tabState = useWorkflowTabState() const toast = useToast() @@ -160,11 +161,20 @@ export function useWorkflowPersistenceV2() { }) } + const hasSharedWorkflowIntent = () => { + if (typeof route.query.share === 'string') return true + hydratePreservedQuery(SHARE_NAMESPACE) + const merged = mergePreservedQueryIntoQuery(SHARE_NAMESPACE, route.query) + return typeof merged?.share === 'string' + } + const loadDefaultWorkflow = async () => { if (!settingStore.get('Comfy.TutorialCompleted')) { await settingStore.set('Comfy.TutorialCompleted', true) await useWorkflowService().loadBlankWorkflow() - await useCommandStore().execute('Comfy.BrowseTemplates') + if (!hasSharedWorkflowIntent()) { + await useCommandStore().execute('Comfy.BrowseTemplates') + } } else { await comfyApp.loadGraphData() }