fix: initializing surveys twice and calling survey shown/dismissed more than once per survey

src/__tests__/posthog-surveys.test.ts

@@ -0,0 +1,261 @@
+/* eslint-disable compat/compat */
+jest.mock('../utils/logger', () => ({
+    createLogger: jest.fn().mockReturnValue({
+        info: jest.fn(),
+        warn: jest.fn(),
+        error: jest.fn(),
+    }),
+}))
+jest.useFakeTimers()
+
+import { SURVEYS } from '../constants'
+import { PostHog } from '../posthog-core'
+import { PostHogSurveys } from '../posthog-surveys'
+import { assignableWindow } from '../utils/globals'
+
+describe('PostHogSurveys', () => {
+    let mockPostHog: PostHog & {
+        get_property: jest.Mock
+        _send_request: jest.Mock
+    }
+    let surveys: PostHogSurveys
+    let mockGenerateSurveys: jest.Mock
+    let mockLoadExternalDependency: jest.Mock
+
+    beforeEach(() => {
+        // Reset mocks
+        jest.clearAllMocks()
+
+        // Mock PostHog instance
+        mockPostHog = {
+            config: {
+                disable_surveys: false,
+                token: 'test-token',
+            },
+            persistence: {
+                register: jest.fn(),
+                props: {},
+            },
+            requestRouter: {
+                endpointFor: jest.fn().mockReturnValue('https://test.com/api/surveys'),
+            },
+            _send_request: jest.fn(),
+            get_property: jest.fn(),
+        } as unknown as PostHog & {
+            get_property: jest.Mock
+            _send_request: jest.Mock
+        }
+
+        // Create surveys instance
+        surveys = new PostHogSurveys(mockPostHog as PostHog)
+
+        // Mock window.__PosthogExtensions__
+        mockGenerateSurveys = jest.fn()
+        mockLoadExternalDependency = jest.fn()
+        assignableWindow.__PosthogExtensions__ = {
+            generateSurveys: mockGenerateSurveys,
+            loadExternalDependency: mockLoadExternalDependency,
+        }
+    })
+
+    afterEach(() => {
+        // Clean up
+        delete assignableWindow.__PosthogExtensions__
+    })
+
+    describe('loadIfEnabled', () => {
+        it('should not initialize if surveys are already loaded', () => {
+            // Set surveyManager to simulate already loaded state
+            surveys['_surveyManager'] = {}
+            surveys.loadIfEnabled()
+
+            expect(mockGenerateSurveys).not.toHaveBeenCalled()
+            expect(mockLoadExternalDependency).not.toHaveBeenCalled()
+        })
+
+        it('should not initialize if already initializing', () => {
+            // Set isInitializingSurveys to true
+            surveys['_isInitializingSurveys'] = true
+            surveys.loadIfEnabled()
+
+            expect(mockGenerateSurveys).not.toHaveBeenCalled()
+            expect(mockLoadExternalDependency).not.toHaveBeenCalled()
+        })
+
+        it('should not initialize if surveys are disabled', () => {
+            mockPostHog.config.disable_surveys = true
+            surveys.loadIfEnabled()
+
+            expect(mockGenerateSurveys).not.toHaveBeenCalled()
+            expect(mockLoadExternalDependency).not.toHaveBeenCalled()
+        })
+
+        it('should not initialize if PostHog Extensions are not found', () => {
+            delete assignableWindow.__PosthogExtensions__
+            surveys.loadIfEnabled()
+
+            expect(mockGenerateSurveys).not.toHaveBeenCalled()
+            expect(mockLoadExternalDependency).not.toHaveBeenCalled()
+        })
+
+        it('should not initialize if decide server response is not ready', () => {
+            surveys.loadIfEnabled()
+
+            expect(mockGenerateSurveys).not.toHaveBeenCalled()
+            expect(mockLoadExternalDependency).not.toHaveBeenCalled()
+        })
+
+        it('should set isInitializingSurveys to false after successful initialization', () => {
+            // Set decide server response
+            surveys['_decideServerResponse'] = true
+            mockGenerateSurveys.mockReturnValue({})
+
+            surveys.loadIfEnabled()
+
+            expect(surveys['_isInitializingSurveys']).toBe(false)
+        })
+
+        it('should set isInitializingSurveys to false after failed initialization', () => {
+            // Set decide server response
+            surveys['_decideServerResponse'] = true
+            mockGenerateSurveys.mockImplementation(() => {
+                throw new Error('Test error')
+            })
+
+            expect(() => surveys.loadIfEnabled()).toThrow('Test error')
+            expect(surveys['_isInitializingSurveys']).toBe(false)
+        })
+
+        it('should set isInitializingSurveys to false when loadExternalDependency fails', () => {
+            // Set decide server response but no generateSurveys
+            surveys['_decideServerResponse'] = true
+            mockGenerateSurveys = undefined
+            assignableWindow.__PosthogExtensions__.generateSurveys = undefined
+
+            mockLoadExternalDependency.mockImplementation((_instance, _type, callback) => {
+                callback(new Error('Failed to load'))
+            })
+
+            surveys.loadIfEnabled()
+
+            expect(surveys['_isInitializingSurveys']).toBe(false)
+        })
+    })
+
+    describe('getSurveys', () => {
+        const mockCallback = jest.fn()
+        const mockSurveys = [{ id: 'test-survey' }]
+
+        beforeEach(() => {
+            mockCallback.mockClear()
+        })
+
+        it('should return cached surveys and not fetch if they exist', () => {
+            mockPostHog.get_property.mockReturnValue(mockSurveys)
+
+            surveys.getSurveys(mockCallback)
+
+            expect(mockPostHog._send_request).not.toHaveBeenCalled()
+            expect(mockCallback).toHaveBeenCalledWith(mockSurveys)
+            expect(surveys['_isFetchingSurveys']).toBe(false)
+        })
+
+        it('should not make concurrent API calls', () => {
+            surveys['_isFetchingSurveys'] = true
+
+            surveys.getSurveys(mockCallback)
+
+            expect(mockPostHog._send_request).not.toHaveBeenCalled()
+            expect(mockCallback).toHaveBeenCalledWith([])
+        })
+
+        it('should reset _isFetchingSurveys after successful API call', () => {
+            mockPostHog._send_request.mockImplementation(({ callback }) => {
+                callback({ statusCode: 200, json: { surveys: mockSurveys } })
+            })
+
+            surveys.getSurveys(mockCallback)
+
+            expect(surveys['_isFetchingSurveys']).toBe(false)
+            expect(mockCallback).toHaveBeenCalledWith(mockSurveys)
+            expect(mockPostHog.persistence?.register).toHaveBeenCalledWith({ [SURVEYS]: mockSurveys })
+        })
+
+        it('should reset _isFetchingSurveys after failed API call (non-200 status)', () => {
+            mockPostHog._send_request.mockImplementation(({ callback }) => {
+                callback({ statusCode: 500 })
+            })
+
+            surveys.getSurveys(mockCallback)
+
+            expect(surveys['_isFetchingSurveys']).toBe(false)
+            expect(mockCallback).toHaveBeenCalledWith([])
+        })
+
+        it('should reset _isFetchingSurveys when API call throws error', () => {
+            mockPostHog._send_request.mockImplementation(() => {
+                throw new Error('Network error')
+            })
+
+            expect(() => surveys.getSurveys(mockCallback)).toThrow('Network error')
+            expect(surveys['_isFetchingSurveys']).toBe(false)
+        })
+
+        it('should reset _isFetchingSurveys when request times out', () => {
+            // Mock a request that will timeout
+            mockPostHog._send_request.mockImplementation(({ callback }) => {
+                // Simulate a timeout by calling callback with status 0
+                callback({ statusCode: 0, text: 'timeout' })
+            })
+
+            surveys.getSurveys(mockCallback)
+
+            expect(surveys['_isFetchingSurveys']).toBe(false)
+            expect(mockCallback).toHaveBeenCalledWith([])
+        })
+
+        it('should handle delayed successful responses correctly', () => {
+            const delayedSurveys = [{ id: 'delayed-survey' }]
+
+            // Mock a request that takes some time to respond
+            mockPostHog._send_request.mockImplementation(({ callback }) => {
+                setTimeout(() => {
+                    callback({
+                        statusCode: 200,
+                        json: { surveys: delayedSurveys },
+                    })
+                }, 100)
+            })
+
+            surveys.getSurveys(mockCallback)
+
+            // Initially the flag should be true
+            expect(surveys['_isFetchingSurveys']).toBe(true)
+
+            // After the response comes in
+            jest.advanceTimersByTime(100)
+
+            expect(surveys['_isFetchingSurveys']).toBe(false)
+            expect(mockCallback).toHaveBeenCalledWith(delayedSurveys)
+            expect(mockPostHog.persistence?.register).toHaveBeenCalledWith({ [SURVEYS]: delayedSurveys })
+        })
+
+        it('should set correct timeout value in request', () => {
+            surveys.getSurveys(mockCallback)
+
+            expect(mockPostHog._send_request).toHaveBeenCalledWith(
+                expect.objectContaining({
+                    timeout: 10000,
+                })
+            )
+        })
+
+        it('should force reload surveys when forceReload is true', () => {
+            mockPostHog.get_property.mockReturnValue(mockSurveys)
+
+            surveys.getSurveys(mockCallback, true)
+
+            expect(mockPostHog._send_request).toHaveBeenCalled()
+        })
+    })
+})

src/__tests__/surveys.test.ts

@@ -1,28 +1,27 @@
 /// <reference lib="dom" />
 
-import { PostHogSurveys } from '../posthog-surveys'
-import {
-    SurveyType,
-    SurveyQuestionType,
-    Survey,
-    MultipleSurveyQuestion,
-    SurveyQuestionBranchingType,
-    SurveyQuestion,
-    RatingSurveyQuestion,
-} from '../posthog-surveys-types'
+import { generateSurveys } from '../extensions/surveys'
 import {
     canActivateRepeatedly,
     getDisplayOrderChoices,
     getDisplayOrderQuestions,
 } from '../extensions/surveys/surveys-utils'
-import { PostHogPersistence } from '../posthog-persistence'
 import { PostHog } from '../posthog-core'
+import { PostHogPersistence } from '../posthog-persistence'
+import { PostHogSurveys } from '../posthog-surveys'
+import {
+    MultipleSurveyQuestion,
+    RatingSurveyQuestion,
+    Survey,
+    SurveyQuestion,
+    SurveyQuestionBranchingType,
+    SurveyQuestionType,
+    SurveyType,
+} from '../posthog-surveys-types'
 import { DecideResponse, PostHogConfig, Properties } from '../types'
-import { window } from '../utils/globals'
-import { RequestRouter } from '../utils/request-router'
-import { assignableWindow } from '../utils/globals'
-import { generateSurveys } from '../extensions/surveys'
 import * as globals from '../utils/globals'
+import { assignableWindow, window } from '../utils/globals'
+import { RequestRouter } from '../utils/request-router'
 
 describe('surveys', () => {
     let config: PostHogConfig
@@ -236,6 +235,7 @@ describe('surveys', () => {
         })
         expect(instance._send_request).toHaveBeenCalledWith({
             url: 'https://us.i.posthog.com/api/surveys/?token=testtoken',
+            timeout: 10000,
             method: 'GET',
             callback: expect.any(Function),
         })
@@ -280,6 +280,7 @@ describe('surveys', () => {
         })
         expect(instance._send_request).toHaveBeenCalledWith({
             url: 'https://us.i.posthog.com/api/surveys/?token=testtoken',
+            timeout: 10000,
             method: 'GET',
             callback: expect.any(Function),
         })