fix: survey URL targeting should re-evaluate after the URL changes

src/__tests__/extensions/surveys.test.ts

@@ -1,20 +1,24 @@
+/* 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'
 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

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'