fix(flags): Make sure we don't override flags when decide is disabled

PostHog · Nov 23, 2023 · 47fb9ad · 47fb9ad
1 parent ca8fad7
commit 47fb9ad
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 13 deletions.
diff --git a/src/__tests__/decide.js b/src/__tests__/decide.js
@@ -170,7 +170,7 @@ describe('Decide', () => {
             expect(console.error).toHaveBeenCalledWith('[PostHog.js]', 'Failed to fetch feature flags from PostHog.')
         })
 
-        it('Make sure receivedFeatureFlags is called with empty if advanced_disable_feature_flags_on_first_load is set', () => {
+        it('Make sure receivedFeatureFlags is not called if advanced_disable_feature_flags_on_first_load is set', () => {
             given('decideResponse', () => ({
                 enable_collect_everything: true,
                 featureFlags: { 'test-flag': true },
@@ -191,6 +191,27 @@ describe('Decide', () => {
             expect(given.posthog.featureFlags.receivedFeatureFlags).not.toHaveBeenCalled()
         })
 
+        it('Make sure receivedFeatureFlags is not called if advanced_disable_feature_flags is set', () => {
+            given('decideResponse', () => ({
+                enable_collect_everything: true,
+                featureFlags: { 'test-flag': true },
+            }))
+            given('config', () => ({
+                api_host: 'https://test.com',
+                token: 'testtoken',
+                persistence: 'memory',
+                advanced_disable_feature_flags: true,
+            }))
+
+            given.subject()
+
+            expect(autocapture.afterDecideResponse).toHaveBeenCalledWith(given.decideResponse, given.posthog)
+            expect(given.posthog.sessionRecording.afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)
+            expect(given.posthog.toolbar.afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)
+
+            expect(given.posthog.featureFlags.receivedFeatureFlags).not.toHaveBeenCalled()
+        })
+
         it('runs site apps if opted in', () => {
             given('config', () => ({ api_host: 'https://test.com', opt_in_site_apps: true, persistence: 'memory' }))
             given('decideResponse', () => ({ siteApps: [{ id: 1, url: '/site_app/1/tokentoken/hash/' }] }))

diff --git a/src/__tests__/featureflags.js b/src/__tests__/featureflags.js
@@ -559,25 +559,37 @@ describe('featureflags', () => {
                 ...given.instance.config,
                 advanced_disable_feature_flags: true,
             }
+            given.instance.persistence.register({
+                $enabled_feature_flags: {
+                    'beta-feature': true,
+                    'random-feature': 'xatu',
+                },
+            })
 
             given.featureFlags.reloadFeatureFlags()
-
             jest.runAllTimers()
 
             expect(given.featureFlags.getFlagVariants()).toEqual({
-                first: 'variant-1',
-                second: true,
+                'beta-feature': true,
+                'random-feature': 'xatu',
             })
 
-            // check the request sent disable_flags
-            expect(
-                JSON.parse(Buffer.from(given.instance._send_request.mock.calls[0][1].data, 'base64').toString())
-            ).toEqual({
-                token: 'random fake token',
-                distinct_id: 'blah id',
-                person_properties: { a: 'b', c: 'd' },
-                disable_flags: true,
+            // check reload request was not sent
+            expect(given.instance._send_request).not.toHaveBeenCalled()
+
+            // check the same for other ways to call reload flags
+
+            given.featureFlags.setPersonPropertiesForFlags({ a: 'b', c: 'd' })
+
+            jest.runAllTimers()
+
+            expect(given.featureFlags.getFlagVariants()).toEqual({
+                'beta-feature': true,
+                'random-feature': 'xatu',
             })
+
+            // check reload request was not sent
+            expect(given.instance._send_request).not.toHaveBeenCalled()
         })
     })
 

diff --git a/src/decide.ts b/src/decide.ts
@@ -64,7 +64,10 @@ export class Decide {
         autocapture.afterDecideResponse(response, this.instance)
         this.instance._afterDecideResponse(response)
 
-        if (!this.instance.config.advanced_disable_feature_flags_on_first_load) {
+        if (
+            !this.instance.config.advanced_disable_feature_flags_on_first_load &&
+            !this.instance.config.advanced_disable_feature_flags
+        ) {
             this.instance.featureFlags.receivedFeatureFlags(response)
         }
 

diff --git a/src/posthog-featureflags.ts b/src/posthog-featureflags.ts
@@ -170,6 +170,10 @@ export class PostHogFeatureFlags {
     }
 
     _reloadFeatureFlagsRequest(): void {
+        if (this.instance.config.advanced_disable_feature_flags) {
+            return
+        }
+
         this.setReloadingPaused(true)
         const token = this.instance.config.token
         const personProperties = this.instance.get_property(STORED_PERSON_PROPERTIES_KEY)