diff --git a/src/__tests__/extensions/surveys.test.ts b/src/__tests__/extensions/surveys.test.ts index dabefd11a..51c2470c3 100644 --- a/src/__tests__/extensions/surveys.test.ts +++ b/src/__tests__/extensions/surveys.test.ts @@ -1,13 +1,16 @@ +/* eslint-disable compat/compat */ import { act, fireEvent, render, renderHook } from '@testing-library/preact' import { generateSurveys, renderFeedbackWidgetPreview, renderSurveysPreview, SurveyManager, + SurveyPopup, usePopupVisibility, } from '../../extensions/surveys' import { createShadow } from '../../extensions/surveys/surveys-utils' import { Survey, SurveyQuestionType, SurveyType } from '../../posthog-surveys-types' +import { CaptureResult } from '../../types' import { beforeEach } from '@jest/globals' import '@testing-library/jest-dom' @@ -15,6 +18,7 @@ import { h } from 'preact' import { useEffect, useRef, useState } from 'preact/hooks' import { PostHog } from '../../posthog-core' import { DecideResponse } from '../../types' +import { isArray } from '../../utils/type-utils' declare const global: any @@ -112,6 +116,7 @@ describe('usePopupVisibility', () => { end_date: null, current_iteration: null, current_iteration_start_date: null, + feature_flag_keys: null, } const mockPostHog = { getActiveMatchingSurveys: jest.fn().mockImplementation((callback) => callback([mockSurvey])), @@ -585,6 +590,268 @@ describe('SurveyManager', () => { }) }) +describe('SurveyPopup URL change handling', () => { + let posthog: PostHog + let mockRemoveSurveyFromFocus: jest.Mock + let originalLocationHref: string + + const createTestEvent = (event: string): CaptureResult => ({ + event, + properties: {}, + timestamp: new Date(), + uuid: 'test-uuid', + }) + + const createTestSurvey = (urlCondition?: { url: string; urlMatchType?: string }): Survey => + ({ + id: 'test-survey', + name: 'Test Survey', + description: 'Test Survey Description', + type: SurveyType.Popover, + questions: [ + { + type: SurveyQuestionType.Open, + question: 'What do you think?', + }, + ], + conditions: urlCondition ? { url: urlCondition.url, urlMatchType: urlCondition.urlMatchType } : undefined, + start_date: new Date().toISOString(), + end_date: null, + feature_flag_keys: null, + linked_flag_key: null, + targeting_flag_key: null, + appearance: {}, + }) as Survey + + beforeEach(() => { + // Mock PostHog instance + posthog = { + config: { + before_send: undefined, + }, + capture: jest.fn(), + } as unknown as PostHog + + mockRemoveSurveyFromFocus = jest.fn() + + // Store original location and set initial location + originalLocationHref = window.location.href + Object.defineProperty(window, 'location', { + value: new URL('https://example.com'), + writable: true, + }) + }) + + afterEach(() => { + // Restore original location + Object.defineProperty(window, 'location', { + value: new URL(originalLocationHref), + writable: true, + }) + }) + + it('should not hide survey when URL matches - exact match', () => { + const survey = createTestSurvey({ url: 'https://example.com/', urlMatchType: 'exact' }) + + render( + h(SurveyPopup, { + survey, + posthog, + removeSurveyFromFocus: mockRemoveSurveyFromFocus, + isPopup: true, + }) + ) + + // Simulate pageview event - should not hide survey since URLs match + act(() => { + const beforeSend = posthog.config.before_send + if (isArray(beforeSend)) { + beforeSend.forEach((fn) => fn(createTestEvent('$pageview'))) + } else if (beforeSend) { + beforeSend(createTestEvent('$pageview')) + } + }) + + expect(mockRemoveSurveyFromFocus).not.toHaveBeenCalled() + }) + + it('should hide survey when URL changes to non-matching - exact match', () => { + const survey = createTestSurvey({ url: 'https://example.com/', urlMatchType: 'exact' }) + + render( + h(SurveyPopup, { + survey, + posthog, + removeSurveyFromFocus: mockRemoveSurveyFromFocus, + isPopup: true, + }) + ) + + // Change URL to non-matching + Object.defineProperty(window, 'location', { + value: new URL('https://example.com/other'), + writable: true, + }) + + // Simulate URL change through before_send + act(() => { + const beforeSend = posthog.config.before_send + if (isArray(beforeSend)) { + beforeSend.forEach((fn) => fn(createTestEvent('$pageview'))) + } else if (beforeSend) { + beforeSend(createTestEvent('$pageview')) + } + }) + + expect(mockRemoveSurveyFromFocus).toHaveBeenCalledTimes(1) + expect(mockRemoveSurveyFromFocus).toHaveBeenCalledWith('test-survey') + }) + + it('should not hide survey when URL matches - contains', () => { + const survey = createTestSurvey({ url: 'example', urlMatchType: 'icontains' }) + + render( + h(SurveyPopup, { + survey, + posthog, + removeSurveyFromFocus: mockRemoveSurveyFromFocus, + isPopup: true, + }) + ) + + // Simulate pageview event - should not hide survey since URLs match + act(() => { + const beforeSend = posthog.config.before_send + if (isArray(beforeSend)) { + beforeSend.forEach((fn) => fn(createTestEvent('$pageview'))) + } else if (beforeSend) { + beforeSend(createTestEvent('$pageview')) + } + }) + + expect(mockRemoveSurveyFromFocus).not.toHaveBeenCalled() + }) + + it('should hide survey when URL changes to non-matching - contains', () => { + const survey = createTestSurvey({ url: 'example', urlMatchType: 'icontains' }) + + render( + h(SurveyPopup, { + survey, + posthog, + removeSurveyFromFocus: mockRemoveSurveyFromFocus, + isPopup: true, + }) + ) + + // Change URL to non-matching + Object.defineProperty(window, 'location', { + value: new URL('https://othersite.com'), + writable: true, + }) + + // Simulate URL change through before_send + act(() => { + const beforeSend = posthog.config.before_send + if (isArray(beforeSend)) { + beforeSend.forEach((fn) => fn(createTestEvent('$pageview'))) + } else if (beforeSend) { + beforeSend(createTestEvent('$pageview')) + } + }) + + expect(mockRemoveSurveyFromFocus).toHaveBeenCalledTimes(1) + expect(mockRemoveSurveyFromFocus).toHaveBeenCalledWith('test-survey') + }) + + it('should preserve customer before_send handlers', () => { + const customerBeforeSend = jest.fn((data) => data) + posthog.config.before_send = customerBeforeSend + + const survey = createTestSurvey({ url: 'example', urlMatchType: 'icontains' }) + + render( + h(SurveyPopup, { + survey, + posthog, + removeSurveyFromFocus: mockRemoveSurveyFromFocus, + isPopup: true, + }) + ) + + // Simulate event + const eventData = createTestEvent('$pageview') + + act(() => { + const beforeSend = posthog.config.before_send + if (isArray(beforeSend)) { + beforeSend.forEach((fn) => fn(eventData)) + } else if (beforeSend) { + beforeSend(eventData) + } + }) + + expect(customerBeforeSend).toHaveBeenCalledWith(eventData) + expect(mockRemoveSurveyFromFocus).not.toHaveBeenCalled() + }) + + it('should handle multiple before_send handlers correctly', () => { + const customerBeforeSend1 = jest.fn((data) => ({ ...data, modified1: true }) as CaptureResult) + const customerBeforeSend2 = jest.fn((data) => ({ ...data, modified2: true }) as CaptureResult) + posthog.config.before_send = [customerBeforeSend1, customerBeforeSend2] + + const survey = createTestSurvey({ url: 'example', urlMatchType: 'icontains' }) + + render( + h(SurveyPopup, { + survey, + posthog, + removeSurveyFromFocus: mockRemoveSurveyFromFocus, + isPopup: true, + }) + ) + + // Simulate event + const eventData = createTestEvent('$pageview') + + let result + act(() => { + const beforeSend = posthog.config.before_send + if (isArray(beforeSend)) { + result = beforeSend.reduce((acc, fn) => (acc ? fn(acc) : null), eventData) + } else if (beforeSend) { + result = beforeSend(eventData) + } + }) + + expect(customerBeforeSend1).toHaveBeenCalled() + expect(customerBeforeSend2).toHaveBeenCalled() + expect(result).toHaveProperty('modified1', true) + expect(result).toHaveProperty('modified2', true) + expect(mockRemoveSurveyFromFocus).not.toHaveBeenCalled() + }) + + it('should clean up before_send handlers on unmount', () => { + const customerBeforeSend = jest.fn((data) => data) + posthog.config.before_send = customerBeforeSend + + const survey = createTestSurvey({ url: 'example', urlMatchType: 'icontains' }) + + const { unmount } = render( + h(SurveyPopup, { + survey, + posthog, + removeSurveyFromFocus: mockRemoveSurveyFromFocus, + isPopup: true, + }) + ) + + unmount() + + expect(posthog.config.before_send).toBe(customerBeforeSend) + }) +}) + describe('preview renders', () => { beforeEach(() => { // we have to manually reset the DOM before each test diff --git a/src/extensions/surveys.tsx b/src/extensions/surveys.tsx index fc27a94d7..a090e840c 100644 --- a/src/extensions/surveys.tsx +++ b/src/extensions/surveys.tsx @@ -1,4 +1,5 @@ import { PostHog } from '../posthog-core' +import { surveyUrlValidationMap } from '../posthog-surveys' import { Survey, SurveyAppearance, @@ -8,12 +9,13 @@ import { SurveyRenderReason, SurveyType, } from '../posthog-surveys-types' +import { CaptureResult } from '../types' import * as Preact from 'preact' import { useContext, useEffect, useMemo, useRef, useState } from 'preact/hooks' import { document as _document, window as _window } from '../utils/globals' import { createLogger } from '../utils/logger' -import { isNull, isNumber } from '../utils/type-utils' +import { isArray, isNull, isNumber } from '../utils/type-utils' import { createWidgetShadow, createWidgetStyle } from './surveys-widget' import { ConfirmationMessage } from './surveys/components/ConfirmationMessage' import { Cancel } from './surveys/components/QuestionHeader' @@ -483,6 +485,70 @@ export function SurveyPopup({ const shouldShowConfirmation = isSurveySent || previewPageIndex === survey.questions.length const confirmationBoxLeftStyle = style?.left && isNumber(style?.left) ? { left: style.left - 40 } : {} + // Add URL change listener to hide survey when URL no longer matches + useEffect(() => { + if (isPreviewMode || !survey.conditions?.url || !posthog) { + return + } + + const checkUrlMatch = () => { + const urlMatchType = survey.conditions?.urlMatchType ?? 'icontains' + const url = survey.conditions?.url + if (!url) { + return + } + const urlCheck = surveyUrlValidationMap[urlMatchType](url) + if (!urlCheck) { + setIsPopupVisible(false) + removeSurveyFromFocus(survey.id) + } + } + + // Listen for navigation events + const handleUrlChange = () => { + checkUrlMatch() + } + + // Listen for browser back/forward + window.addEventListener('popstate', handleUrlChange) + + // Create our before_send handler + const surveyBeforeSend = (eventData: CaptureResult | null) => { + if (eventData?.event === '$pageview' || eventData?.event === '$pageleave') { + handleUrlChange() + } + return eventData + } + + // Add our handler to the existing before_send configuration + const originalBeforeSend = posthog.config.before_send + if (isArray(originalBeforeSend)) { + posthog.config.before_send = [...originalBeforeSend, surveyBeforeSend] + } else if (originalBeforeSend) { + posthog.config.before_send = [originalBeforeSend, surveyBeforeSend] + } else { + posthog.config.before_send = surveyBeforeSend + } + + return () => { + window.removeEventListener('popstate', handleUrlChange) + // Remove our handler from the before_send configuration + if (isArray(posthog.config.before_send)) { + posthog.config.before_send = posthog.config.before_send.filter((fn) => fn !== surveyBeforeSend) + // If there's only one handler left, convert back to a single function + if (posthog.config.before_send.length === 1) { + posthog.config.before_send = posthog.config.before_send[0] + } + // If there are no handlers left, restore to original state + else if (posthog.config.before_send.length === 0) { + posthog.config.before_send = undefined + } + } else if (posthog.config.before_send === surveyBeforeSend) { + posthog.config.before_send = undefined + } + } + }, [isPreviewMode, survey, removeSurveyFromFocus, posthog]) + if (isPreviewMode) { style = style || {} style.left = 'unset'