From 063e390c5d7c27261414d4a025cec93aa22e732e Mon Sep 17 00:00:00 2001 From: Coding On Star <447357187@qq.com> Date: Thu, 9 Jul 2026 15:40:55 +0800 Subject: [PATCH] fix(web): preserve attribution from auth redirect (#38583) Co-authored-by: CodingOnStar Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- eslint-suppressions.json | 5 -- .../external-attribution-recorder.spec.tsx | 26 ++++++++++ .../create-app-modal/__tests__/index.spec.tsx | 26 ++++++++++ .../components/app/create-app-modal/index.tsx | 7 ++- .../base/amplitude/__tests__/utils.spec.ts | 34 ++++++++++-- web/app/components/base/amplitude/utils.ts | 12 +++-- .../external-attribution-recorder.tsx | 48 +++++++++++++++-- .../set-password/__tests__/page.spec.tsx | 17 +++--- .../__tests__/create-app-tracking.spec.ts | 52 ++++++++++++++++--- web/utils/create-app-tracking.ts | 52 +++++++++---------- 10 files changed, 224 insertions(+), 55 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index e47650125fe..8cc4e96a268 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -967,11 +967,6 @@ "count": 2 } }, - "web/app/components/base/amplitude/utils.ts": { - "ts/no-explicit-any": { - "count": 2 - } - }, "web/app/components/base/app-icon-picker/ImageInput.tsx": { "react/no-create-ref": { "count": 1 diff --git a/web/app/components/__tests__/external-attribution-recorder.spec.tsx b/web/app/components/__tests__/external-attribution-recorder.spec.tsx index dbf16cffed6..763d7e3ea47 100644 --- a/web/app/components/__tests__/external-attribution-recorder.spec.tsx +++ b/web/app/components/__tests__/external-attribution-recorder.spec.tsx @@ -58,6 +58,23 @@ describe('ExternalAttributionRecorder', () => { expect(firstArg?.searchParams?.get('slug')).toBe('get-started-with-dify') }) + it('seeds attribution from the redirect_url when auth redirects away from the landing url', async () => { + setSearchParams(`redirect_url=${encodeURIComponent('/apps?utm_source=dify_blog&slug=buildaisupportassistantwithdify')}`) + + render() + + await waitFor(() => { + expect(getUtmInfoCookie()).toEqual({ + utm_source: 'dify_blog', + slug: 'buildaisupportassistantwithdify', + }) + }) + expect(mockRememberCreateAppExternalAttribution).toHaveBeenCalledTimes(1) + const firstArg = mockRememberCreateAppExternalAttribution.mock.calls[0]?.[0] + expect(firstArg?.searchParams?.get('utm_source')).toBe('dify_blog') + expect(firstArg?.searchParams?.get('slug')).toBe('buildaisupportassistantwithdify') + }) + it('does nothing without a utm_source', () => { setSearchParams('slug=get-started-with-dify') @@ -67,6 +84,15 @@ describe('ExternalAttributionRecorder', () => { expect(mockRememberCreateAppExternalAttribution).not.toHaveBeenCalled() }) + it('does nothing for cross-origin redirect_url attribution params', () => { + setSearchParams(`redirect_url=${encodeURIComponent('https://example.com/apps?utm_source=dify_blog&slug=get-started-with-dify')}`) + + render() + + expect(getUtmInfoCookie()).toBeNull() + expect(mockRememberCreateAppExternalAttribution).not.toHaveBeenCalled() + }) + it('overwrites a stale utm_info cookie with the latest campaign params', async () => { Cookies.set('utm_info', JSON.stringify({ utm_source: 'newsletter', slug: 'launch-week' })) setSearchParams('utm_source=dify_blog&slug=get-started-with-dify') diff --git a/web/app/components/app/create-app-modal/__tests__/index.spec.tsx b/web/app/components/app/create-app-modal/__tests__/index.spec.tsx index 941f055969c..ccc78f4a9c6 100644 --- a/web/app/components/app/create-app-modal/__tests__/index.spec.tsx +++ b/web/app/components/app/create-app-modal/__tests__/index.spec.tsx @@ -195,6 +195,32 @@ describe('CreateAppModal', () => { ) }) + it('waits for create_app tracking before redirecting after blank app creation', async () => { + const mockApp: Partial = { id: 'app-1', mode: AppModeEnum.ADVANCED_CHAT, maintainer: 'user-1' } + let resolveTracking: (() => void) | undefined + mockCreateApp.mockResolvedValue(mockApp as App) + mockTrackCreateApp.mockReturnValue(new Promise((resolve) => { + resolveTracking = resolve + })) + renderModal() + + fireEvent.change(screen.getByPlaceholderText('app.newApp.appNamePlaceholder'), { + target: { value: 'Tracked App' }, + }) + fireEvent.click(screen.getByRole('button', { name: /app\.newApp\.Create/ })) + + await waitFor(() => { + expect(mockTrackCreateApp).toHaveBeenCalledWith({ source: 'studio_blank', appMode: AppModeEnum.ADVANCED_CHAT }) + }) + expect(mockGetRedirection).not.toHaveBeenCalled() + + resolveTracking?.() + + await waitFor(() => { + expect(mockGetRedirection).toHaveBeenCalledWith(mockApp, mockPush, expect.any(Object)) + }) + }) + it('shows error toast when creation fails', async () => { mockCreateApp.mockRejectedValue(new Error('boom')) const { onClose } = renderModal() diff --git a/web/app/components/app/create-app-modal/index.tsx b/web/app/components/app/create-app-modal/index.tsx index 011e5292167..8ad3ae10757 100644 --- a/web/app/components/app/create-app-modal/index.tsx +++ b/web/app/components/app/create-app-modal/index.tsx @@ -95,7 +95,12 @@ function CreateApp({ onClose, onSuccess, onCreateFromTemplate, defaultAppMode }: mode: appMode, }) - trackCreateApp({ source: 'studio_blank', appMode: app.mode }) + try { + await trackCreateApp({ source: 'studio_blank', appMode: app.mode }) + } + catch { + // Analytics should not turn a successful app creation into a failed flow. + } toast.success(t('newApp.appCreated', { ns: 'app' })) onSuccess() diff --git a/web/app/components/base/amplitude/__tests__/utils.spec.ts b/web/app/components/base/amplitude/__tests__/utils.spec.ts index 87ac9159d06..859d4895c32 100644 --- a/web/app/components/base/amplitude/__tests__/utils.spec.ts +++ b/web/app/components/base/amplitude/__tests__/utils.spec.ts @@ -1,10 +1,11 @@ -import { resetUser, setUserId, setUserProperties, trackEvent } from '../utils' +import { flushEvents, resetUser, setUserId, setUserProperties, trackEvent } from '../utils' const mockState = vi.hoisted(() => ({ enabled: true, })) const mockTrack = vi.hoisted(() => vi.fn()) +const mockFlush = vi.hoisted(() => vi.fn()) const mockSetUserId = vi.hoisted(() => vi.fn()) const mockIdentify = vi.hoisted(() => vi.fn()) const mockReset = vi.hoisted(() => vi.fn()) @@ -28,6 +29,7 @@ vi.mock('@/config', () => ({ vi.mock('@amplitude/analytics-browser', () => ({ track: (...args: unknown[]) => mockTrack(...args), + flush: (...args: unknown[]) => mockFlush(...args), setUserId: (...args: unknown[]) => mockSetUserId(...args), identify: (...args: unknown[]) => mockIdentify(...args), reset: (...args: unknown[]) => mockReset(...args), @@ -41,11 +43,15 @@ describe('amplitude utils', () => { }) describe('trackEvent', () => { - it('should call amplitude.track when amplitude is enabled', () => { - trackEvent('dataset_created', { source: 'wizard' }) + it('should call amplitude.track and return its result when amplitude is enabled', () => { + const trackResult = { promise: Promise.resolve({}) } + mockTrack.mockReturnValue(trackResult) + + const result = trackEvent('dataset_created', { source: 'wizard' }) expect(mockTrack).toHaveBeenCalledTimes(1) expect(mockTrack).toHaveBeenCalledWith('dataset_created', { source: 'wizard' }) + expect(result).toBe(trackResult) }) it('should not call amplitude.track when amplitude is disabled', () => { @@ -57,6 +63,26 @@ describe('amplitude utils', () => { }) }) + describe('flushEvents', () => { + it('should call amplitude.flush and return its result when amplitude is enabled', () => { + const flushResult = { promise: Promise.resolve() } + mockFlush.mockReturnValue(flushResult) + + const result = flushEvents() + + expect(mockFlush).toHaveBeenCalledTimes(1) + expect(result).toBe(flushResult) + }) + + it('should not call amplitude.flush when amplitude is disabled', () => { + mockState.enabled = false + + flushEvents() + + expect(mockFlush).not.toHaveBeenCalled() + }) + }) + describe('setUserId', () => { it('should call amplitude.setUserId when amplitude is enabled', () => { setUserId('user-123') @@ -76,7 +102,7 @@ describe('amplitude utils', () => { describe('setUserProperties', () => { it('should build identify event and call amplitude.identify when amplitude is enabled', () => { - const properties: Record = { + const properties = { role: 'owner', seats: 3, verified: true, diff --git a/web/app/components/base/amplitude/utils.ts b/web/app/components/base/amplitude/utils.ts index 8faa8e852e7..7aecf8c3cad 100644 --- a/web/app/components/base/amplitude/utils.ts +++ b/web/app/components/base/amplitude/utils.ts @@ -6,10 +6,16 @@ import { isAmplitudeEnabled } from '@/config' * @param eventName Event name * @param eventProperties Event properties (optional) */ -export const trackEvent = (eventName: string, eventProperties?: Record) => { +export const trackEvent = (eventName: string, eventProperties?: Record) => { if (!isAmplitudeEnabled) return - amplitude.track(eventName, eventProperties) + return amplitude.track(eventName, eventProperties) +} + +export const flushEvents = () => { + if (!isAmplitudeEnabled) + return + return amplitude.flush() } /** @@ -26,7 +32,7 @@ export const setUserId = (userId: string) => { * Set user properties * @param properties User properties */ -export const setUserProperties = (properties: Record) => { +export const setUserProperties = (properties: Record) => { if (!isAmplitudeEnabled) return const identifyEvent = new amplitude.Identify() diff --git a/web/app/components/external-attribution-recorder.tsx b/web/app/components/external-attribution-recorder.tsx index 2a659dc8bbf..95418dd6a12 100644 --- a/web/app/components/external-attribution-recorder.tsx +++ b/web/app/components/external-attribution-recorder.tsx @@ -10,6 +10,44 @@ const UTM_INFO_COOKIE = 'utm_info' const UTM_INFO_COOKIE_EXPIRES_DAYS = 1 const UTM_INFO_QUERY_KEYS = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_content', 'utm_term', 'slug'] as const +type SearchParamReader = { + get: (name: string) => string | null +} + +const normalizeString = (value?: string | null) => { + const trimmed = value?.trim() + return trimmed || undefined +} + +const getSearchParamValue = (searchParams: SearchParamReader, key: string) => + normalizeString(searchParams.get(key)) + +const parseRedirectUrlSearchParams = (redirectUrl: string) => { + const baseUrl = window.location.origin + + try { + const url = new URL(redirectUrl, baseUrl) + if (url.origin !== baseUrl) + return null + + return url.searchParams + } + catch { + return null + } +} + +const resolveAttributionSearchParams = (searchParams: SearchParamReader): SearchParamReader | null => { + if (getSearchParamValue(searchParams, 'utm_source')) + return searchParams + + const redirectUrl = getSearchParamValue(searchParams, 'redirect_url') + if (!redirectUrl) + return null + + return parseRedirectUrlSearchParams(redirectUrl) +} + /** * Captures external-campaign params (utm_* + blog `slug`) from the landing URL. * @@ -32,12 +70,16 @@ const ExternalAttributionRecorder = () => { if (!IS_CLOUD_EDITION) return - const utmSource = searchParams.get('utm_source')?.trim() + const attributionSearchParams = resolveAttributionSearchParams(searchParams) + if (!attributionSearchParams) + return + + const utmSource = getSearchParamValue(attributionSearchParams, 'utm_source') if (!utmSource) return // create_app conversion attribution (utm_source + slug). - rememberCreateAppExternalAttribution({ searchParams }) + rememberCreateAppExternalAttribution({ searchParams: attributionSearchParams }) // Seed the utm_info cookie the registration trackers read. A campaign click always // overwrites any previous value, so the most recent blog link wins (last-touch) and @@ -45,7 +87,7 @@ const ExternalAttributionRecorder = () => { // mirrors the create_app attribution refreshed just above. const utmInfo: Record = {} UTM_INFO_QUERY_KEYS.forEach((key) => { - const value = searchParams.get(key)?.trim() + const value = getSearchParamValue(attributionSearchParams, key) if (value) utmInfo[key] = value }) diff --git a/web/app/signup/set-password/__tests__/page.spec.tsx b/web/app/signup/set-password/__tests__/page.spec.tsx index 5f8b7260e23..f7cb7fb3ba3 100644 --- a/web/app/signup/set-password/__tests__/page.spec.tsx +++ b/web/app/signup/set-password/__tests__/page.spec.tsx @@ -10,7 +10,8 @@ import { useMailRegister } from '@/service/use-common' import { getBrowserTimezone } from '@/utils/timezone' import ChangePasswordForm from '../page' -const { mockRememberRegistrationSuccess, mockSendGAEvent } = vi.hoisted(() => ({ +const { mockRememberCreateAppExternalAttribution, mockRememberRegistrationSuccess, mockSendGAEvent } = vi.hoisted(() => ({ + mockRememberCreateAppExternalAttribution: vi.fn(), mockRememberRegistrationSuccess: vi.fn(), mockSendGAEvent: vi.fn(), })) @@ -41,7 +42,7 @@ vi.mock('@/app/components/base/amplitude/registration-tracking', () => ({ })) vi.mock('@/utils/create-app-tracking', () => ({ - rememberCreateAppExternalAttribution: vi.fn(), + rememberCreateAppExternalAttribution: (...args: unknown[]) => mockRememberCreateAppExternalAttribution(...args), })) const mockRegister = vi.fn() @@ -137,8 +138,8 @@ describe('Signup Set Password Page', () => { expect(mockReplace).toHaveBeenCalledWith('/') }) - it('should remember the utm event and clear the utm cookie when a utm_info cookie is present', async () => { - Cookies.set('utm_info', JSON.stringify({ utm_source: 'twitter' })) + it('should remember the utm event with slug and clear the utm cookie when a utm_info cookie is present', async () => { + Cookies.set('utm_info', JSON.stringify({ utm_source: 'community', slug: 'partner-launch' })) mockRegister.mockResolvedValue({ result: 'success', data: {} }) renderWithQueryClient() @@ -147,12 +148,16 @@ describe('Signup Set Password Page', () => { await waitFor(() => { expect(mockRememberRegistrationSuccess).toHaveBeenCalledWith({ method: 'email', - utmInfo: { utm_source: 'twitter' }, + utmInfo: { utm_source: 'community', slug: 'partner-launch' }, }) }) + expect(mockRememberCreateAppExternalAttribution).toHaveBeenCalledWith({ + utmInfo: { utm_source: 'community', slug: 'partner-launch' }, + }) expect(mockSendGAEvent).toHaveBeenCalledWith('user_registration_success_with_utm', { method: 'email', - utm_source: 'twitter', + utm_source: 'community', + slug: 'partner-launch', }) expect(Cookies.get('utm_info')).toBeUndefined() }) diff --git a/web/utils/__tests__/create-app-tracking.spec.ts b/web/utils/__tests__/create-app-tracking.spec.ts index 4939b0dfd3c..acf3fd67927 100644 --- a/web/utils/__tests__/create-app-tracking.spec.ts +++ b/web/utils/__tests__/create-app-tracking.spec.ts @@ -1,6 +1,6 @@ import Cookies from 'js-cookie' import { beforeEach, describe, expect, it, vi } from 'vitest' -import * as amplitude from '@/app/components/base/amplitude' +import * as amplitude from '@/app/components/base/amplitude/utils' import { AppModeEnum } from '@/types/app' import { buildCreateAppEventPayload, @@ -29,7 +29,7 @@ describe('create-app-tracking', () => { }) }) - it('should gate on known external sources but keep raw values', () => { + it('should accept any non-empty utm_source and keep raw values', () => { expect(extractExternalCreateAppAttribution({ searchParams: new URLSearchParams('utm_source=newsletter'), })).toEqual({ utmSource: 'newsletter' }) @@ -41,14 +41,36 @@ describe('create-app-tracking', () => { slug: 'launch-week', }) - // Unknown sources are still rejected. expect(extractExternalCreateAppAttribution({ searchParams: new URLSearchParams('utm_source=random&slug=x'), - })).toBeNull() + })).toEqual({ + utmSource: 'random', + slug: 'x', + }) }) }) describe('rememberCreateAppExternalAttribution', () => { + it('should remember unknown utm_source values from the utm cookie', () => { + vi.spyOn(Cookies, 'get').mockImplementation(((key?: string) => { + return key + ? JSON.stringify({ + utm_source: 'community', + slug: 'partner-launch', + }) + : {} + }) as typeof Cookies.get) + + expect(rememberCreateAppExternalAttribution()).toEqual({ + utmSource: 'community', + slug: 'partner-launch', + }) + expect(window.sessionStorage.getItem('create_app_external_attribution')).toBe(JSON.stringify({ + utmSource: 'community', + slug: 'partner-launch', + })) + }) + it('should ignore malformed utm cookies', () => { vi.spyOn(Cookies, 'get').mockImplementation(((key?: string) => { return key ? 'not-json' : {} @@ -168,6 +190,24 @@ describe('create-app-tracking', () => { }) describe('trackCreateApp', () => { + it('should flush the create_app event immediately when tracking returns an SDK handle', async () => { + vi.spyOn(amplitude, 'trackEvent').mockReturnValue({ + promise: Promise.resolve({}), + } as ReturnType) + const flushEventsSpy = vi.spyOn(amplitude, 'flushEvents').mockReturnValue({ + promise: Promise.resolve(), + } as ReturnType) + + await expect(trackCreateApp({ source: 'studio_blank', appMode: AppModeEnum.ADVANCED_CHAT })).resolves.toBeUndefined() + + expect(amplitude.trackEvent).toHaveBeenCalledWith('create_app', { + source: 'studio_blank', + app_mode: 'chatflow', + time: expect.stringMatching(/^\d{2}-\d{2}-\d{2}:\d{2}:\d{2}$/), + }) + expect(flushEventsSpy).toHaveBeenCalledTimes(1) + }) + it('should track remembered external attribution once before falling back to internal source', () => { rememberCreateAppExternalAttribution({ searchParams: new URLSearchParams('utm_source=newsletter&slug=how-to-build-rag-agent'), @@ -242,7 +282,7 @@ describe('create-app-tracking', () => { it('should consume snake_case sessionStorage attribution and map legacy utm_campaign to slug', () => { window.sessionStorage.setItem('create_app_external_attribution', JSON.stringify({ - utm_source: 'twitter', + utm_source: 'community', utm_campaign: 'launch-week', })) @@ -252,7 +292,7 @@ describe('create-app-tracking', () => { source: 'external', app_mode: 'chatflow', time: expect.stringMatching(/^\d{2}-\d{2}-\d{2}:\d{2}:\d{2}$/), - utm_source: 'twitter', + utm_source: 'community', slug: 'launch-week', }) expect(window.sessionStorage.getItem('create_app_external_attribution')).toBeNull() diff --git a/web/utils/create-app-tracking.ts b/web/utils/create-app-tracking.ts index 0a7ed96928c..71e53ee8ff9 100644 --- a/web/utils/create-app-tracking.ts +++ b/web/utils/create-app-tracking.ts @@ -1,18 +1,18 @@ import Cookies from 'js-cookie' -import { trackEvent } from '@/app/components/base/amplitude' +import { flushEvents, trackEvent } from '@/app/components/base/amplitude/utils' import { AppModeEnum } from '@/types/app' const CREATE_APP_EXTERNAL_ATTRIBUTION_STORAGE_KEY = 'create_app_external_attribution' -const EXTERNAL_UTM_SOURCE_MAP = { - 'blog': 'blog', - 'dify_blog': 'blog', - 'linkedin': 'linkedin', - 'newsletter': 'blog', - 'twitter': 'twitter/x', - 'twitter/x': 'twitter/x', - 'x': 'twitter/x', -} as const +// const EXTERNAL_UTM_SOURCE_MAP = { +// 'blog': 'blog', +// 'dify_blog': 'blog', +// 'linkedin': 'linkedin', +// 'newsletter': 'blog', +// 'twitter': 'twitter/x', +// 'twitter/x': 'twitter/x', +// 'x': 'twitter/x', +// } as const type SearchParamReader = { get: (name: string) => string | null @@ -36,9 +36,6 @@ export type TrackCreateAppParams = { } type ExternalCreateAppAttribution = { - // Raw utm_source from the link (e.g. "dify_blog"), reported as-is to stay consistent - // with the registration event. EXTERNAL_UTM_SOURCE_MAP is only used to gate which - // sources count as external, not to rewrite the reported value. utmSource: string slug?: string } @@ -75,14 +72,6 @@ const getCookieUtmInfo = () => { return parseJSONRecord(Cookies.get('utm_info')) } -const mapExternalUtmSource = (value?: string) => { - if (!value) - return undefined - - const normalized = value.toLowerCase() - return EXTERNAL_UTM_SOURCE_MAP[normalized as keyof typeof EXTERNAL_UTM_SOURCE_MAP] -} - const padTimeValue = (value: number) => String(value).padStart(2, '0') const formatCreateAppTime = (date: Date) => { @@ -108,8 +97,7 @@ export const extractExternalCreateAppAttribution = ({ }) => { const rawSource = getSearchParamValue(searchParams, 'utm_source') ?? getObjectStringValue(utmInfo?.utm_source) - // Gate on known external sources, but keep the raw value for reporting. - if (!rawSource || !mapExternalUtmSource(rawSource)) + if (!rawSource) return null const slug = getSearchParamValue(searchParams, 'slug') @@ -127,8 +115,7 @@ const readRememberedExternalCreateAppAttribution = (): ExternalCreateAppAttribut const attribution = parseJSONRecord(window.sessionStorage.getItem(CREATE_APP_EXTERNAL_ATTRIBUTION_STORAGE_KEY)) const rawSource = getObjectStringValue(attribution?.utmSource) ?? getObjectStringValue(attribution?.utm_source) - // Gate on known external sources, but keep the raw value for reporting. - if (!rawSource || !mapExternalUtmSource(rawSource)) + if (!rawSource) return null const slug = getObjectStringValue(attribution?.slug) @@ -198,7 +185,7 @@ export const buildCreateAppEventPayload = ( } satisfies Record } -export const trackCreateApp = (params: TrackCreateAppParams) => { +export const trackCreateApp = (params: TrackCreateAppParams): Promise | undefined => { const externalAttribution = resolveCurrentExternalCreateAppAttribution() const payload = buildCreateAppEventPayload(params, externalAttribution) @@ -208,5 +195,16 @@ export const trackCreateApp = (params: TrackCreateAppParams) => { if (externalAttribution) clearRememberedExternalCreateAppAttribution() - trackEvent('create_app', payload) + const trackingResult = trackEvent('create_app', payload) + + if (!trackingResult) + return + + const flushResult = flushEvents() + + const completionPromise = flushResult + ? Promise.all([trackingResult.promise, flushResult.promise]).then(() => undefined) + : trackingResult.promise.then(() => undefined) + + return completionPromise.catch(() => undefined) }