Merge branch '296640-engineering-add-invite-option-to-the-comment-component-3' into 'master'

Remove gon.global experiment scoping and organize helpers

See merge request gitlab-org/gitlab!55421

app/assets/javascripts/experimentation/constants.js

+export const TRACKING_CONTEXT_SCHEMA = 'iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0';

app/assets/javascripts/experiment_tracking.js → app/assets/javascripts/experimentation/experiment_tracking.js

-import { get } from 'lodash';
 import Tracking from '~/tracking';
-
-const TRACKING_CONTEXT_SCHEMA = 'iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0';
+import { TRACKING_CONTEXT_SCHEMA } from './constants';
+import { getExperimentData } from './utils';
 
 export default class ExperimentTracking {
   constructor(experimentName, trackingArgs = {}) {
     this.trackingArgs = trackingArgs;
-    this.experimentData = get(window, ['gon', 'global', 'experiment', experimentName]);
+    this.data = getExperimentData(experimentName);
   }
 
   event(action) {
-    if (!this.experimentData) {
+    if (!this.data) {
       return false;
     }
 
@@ -18,7 +17,7 @@ export default class ExperimentTracking {
       ...this.trackingArgs,
       context: {
         schema: TRACKING_CONTEXT_SCHEMA,
-        data: this.experimentData,
+        data: this.data,
       },
     });
   }

app/assets/javascripts/experimentation/utils.js

+// This file only applies to use of experiments through https://gitlab.com/gitlab-org/gitlab-experiment
+import { get } from 'lodash';
+
+export function getExperimentData(experimentName) {
+  return get(window, ['gon', 'experiment', experimentName]);
+}
+
+export function isExperimentVariant(experimentName, variantName) {
+  return getExperimentData(experimentName)?.variant === variantName;
+}

app/assets/javascripts/lib/utils/experimentation.js

-export function isExperimentEnabled(experimentKey) {
-  return Boolean(window.gon?.experiments?.[experimentKey]);
-}

app/assets/javascripts/projects/upload_file_experiment.js

-import ExperimentTracking from '~/experiment_tracking';
+import ExperimentTracking from '~/experimentation/experiment_tracking';
 
 function trackEvent(eventName) {
   const isEmpty = Boolean(document.querySelector('.project-home-panel.empty-project'));

app/assets/javascripts/tracking.js

-import { omitBy, isUndefined, get } from 'lodash';
+import { omitBy, isUndefined } from 'lodash';
+import { TRACKING_CONTEXT_SCHEMA } from '~/experimentation/constants';
+import { getExperimentData } from '~/experimentation/utils';
 
 const standardContext = { ...window.gl?.snowplowStandardContext };
 
@@ -32,8 +34,8 @@ const createEventPayload = (el, { suffix = '' } = {}) => {
 
   let context = el.dataset.trackContext;
   if (el.dataset.trackExperiment) {
-    const data = get(window, ['gon', 'global', 'experiment', el.dataset.trackExperiment]);
-    if (data) context = { schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0', data };
+    const data = getExperimentData(el.dataset.trackExperiment);
+    if (data) context = { schema: TRACKING_CONTEXT_SCHEMA, data };
   }
 
   const data = {

app/experiments/application_experiment.rb

@@ -11,7 +11,9 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp
 
   def publish(_result)
     track(:assignment) # track that we've assigned a variant for this context
-    Gon.global.push({ experiment: { name => signature } }, true) # push the experiment data to the client
+
+    # push the experiment data to the client
+    Gon.push({ experiment: { name => signature } }, true) if in_request_cycle?
   end
 
   def track(action, **event_args)
@@ -47,6 +49,12 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp
     name.tr('/', '_')
   end
 
+  def in_request_cycle?
+    # Gon is only accessible when having a request. This will be fixed with
+    # https://gitlab.com/gitlab-org/gitlab/-/issues/323352
+    context.instance_variable_defined?(:@request)
+  end
+
   def resolve_variant_name
     case rollout_strategy
     when :round_robin

doc/development/experiment_guide/index.md

@@ -140,15 +140,8 @@ You find out how to conduct experiments using `gitlab-experiment` in the [README
 
      The above checks whether the experiment is enabled and pushes the result to the frontend.
 
-     You can check the state of the feature flag in JavaScript:
-
-     ```javascript
-     import { isExperimentEnabled } from '~/experimentation';
-
-     if ( isExperimentEnabled('signupFlow') ) {
-       // ...
-     }
-     ```
+     The Frontend helpers for this are no longer used in production.
+     [More details TBD](https://gitlab.com/gitlab-org/gitlab/-/issues/323934).
 
    - It is also possible to run an experiment outside of the controller scope, for example in a worker:
 
@@ -238,11 +231,10 @@ expect(Gon.tracking_data).to eq(
 Which can then be used for tracking as follows:
 
 ```javascript
-import { isExperimentEnabled } from '~/lib/utils/experimentation';
 import Tracking from '~/tracking';
 
 document.addEventListener('DOMContentLoaded', () => {
-  const signupFlowExperimentEnabled = isExperimentEnabled('signupFlow');
+  const signupFlowExperimentEnabled = gon.experiments['signupFlow'];
 
   if (signupFlowExperimentEnabled && gon.tracking_data) {
     const { category, action, ...data } = gon.tracking_data;

spec/experiments/application_experiment_spec.rb

@@ -64,21 +64,35 @@ RSpec.describe ApplicationExperiment, :experiment do
       subject.publish(nil)
     end
 
-    it "pushes the experiment knowledge into the client using Gon.global" do
-      expect(Gon.global).to receive(:push).with(
-        {
-          experiment: {
-            'namespaced/stub' => { # string key because it can be namespaced
-              experiment: 'namespaced/stub',
-              key: '86208ac54ca798e11f127e8b23ec396a',
-              variant: 'control'
+    context "when inside a request cycle" do
+      before do
+        subject.context.instance_variable_set(:@request, double('Request', headers: 'true'))
+      end
+
+      it "pushes the experiment knowledge into the client using Gon" do
+        expect(Gon).to receive(:push).with(
+          {
+            experiment: {
+              'namespaced/stub' => { # string key because it can be namespaced
+                                     experiment: 'namespaced/stub',
+                                     key: '86208ac54ca798e11f127e8b23ec396a',
+                                     variant: 'control'
+              }
             }
-          }
-        },
-        true
-      )
+          },
+          true
+        )
 
-      subject.publish(nil)
+        subject.publish(nil)
+      end
+    end
+
+    context "when outside a request cycle" do
+      it "does not push to gon when outside request cycle" do
+        expect(Gon).not_to receive(:push)
+
+        subject.publish(nil)
+      end
     end
   end
 

spec/frontend/experiment_tracking_spec.js → spec/frontend/experimentation/experiment_tracking_spec.js

-import ExperimentTracking from '~/experiment_tracking';
+import { TRACKING_CONTEXT_SCHEMA } from '~/experimentation/constants';
+import ExperimentTracking from '~/experimentation/experiment_tracking';
+import { getExperimentData } from '~/experimentation/utils';
 import Tracking from '~/tracking';
 
-jest.mock('~/tracking');
-
-const oldGon = window.gon;
-
-let newGon = {};
 let experimentTracking;
 let label;
 let property;
 
+jest.mock('~/tracking');
+jest.mock('~/experimentation/utils', () => ({ getExperimentData: jest.fn() }));
+
 const setup = () => {
-  window.gon = newGon;
   experimentTracking = new ExperimentTracking('sidebar_experiment', { label, property });
 };
 
@@ -20,16 +19,18 @@ beforeEach(() => {
 });
 
 afterEach(() => {
-  window.gon = oldGon;
-  Tracking.mockClear();
   label = undefined;
   property = undefined;
 });
 
 describe('event', () => {
+  beforeEach(() => {
+    getExperimentData.mockReturnValue(undefined);
+  });
+
   describe('when experiment data exists for experimentName', () => {
     beforeEach(() => {
-      newGon = { global: { experiment: { sidebar_experiment: 'experiment-data' } } };
+      getExperimentData.mockReturnValue('experiment-data');
       setup();
     });
 
@@ -45,7 +46,7 @@ describe('event', () => {
           label: 'sidebar-drawer',
           property: 'dark-mode',
           context: {
-            schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0',
+            schema: TRACKING_CONTEXT_SCHEMA,
             data: 'experiment-data',
           },
         });
@@ -58,7 +59,7 @@ describe('event', () => {
       expect(Tracking.event).toHaveBeenCalledTimes(1);
       expect(Tracking.event).toHaveBeenCalledWith('issues-page', 'click_sidebar_trigger', {
         context: {
-          schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0',
+          schema: TRACKING_CONTEXT_SCHEMA,
           data: 'experiment-data',
         },
       });
@@ -67,7 +68,6 @@ describe('event', () => {
 
   describe('when experiment data does NOT exists for the experimentName', () => {
     beforeEach(() => {
-      newGon = { global: { experiment: { unrelated_experiment: 'not happening' } } };
       setup();
     });
 

spec/frontend/experimentation/utils_spec.js

+import * as experimentUtils from '~/experimentation/utils';
+
+const TEST_KEY = 'abc';
+
+describe('experiment Utilities', () => {
+  const oldGon = window.gon;
+
+  afterEach(() => {
+    window.gon = oldGon;
+  });
+
+  describe('getExperimentData', () => {
+    it.each`
+      gon                                         | input         | output
+      ${{ experiment: { [TEST_KEY]: '_data_' } }} | ${[TEST_KEY]} | ${'_data_'}
+      ${{}}                                       | ${[TEST_KEY]} | ${undefined}
+    `('with input=$input and gon=$gon, returns $output', ({ gon, input, output }) => {
+      window.gon = gon;
+
+      expect(experimentUtils.getExperimentData(...input)).toEqual(output);
+    });
+  });
+
+  describe('isExperimentVariant', () => {
+    it.each`
+      gon                                                             | input                            | output
+      ${{ experiment: { [TEST_KEY]: { variant: 'control' } } }}       | ${[TEST_KEY, 'control']}         | ${true}
+      ${{ experiment: { [TEST_KEY]: { variant: '_variant_name' } } }} | ${[TEST_KEY, '_variant_name']}   | ${true}
+      ${{ experiment: { [TEST_KEY]: { variant: '_variant_name' } } }} | ${[TEST_KEY, '_bogus_name']}     | ${false}
+      ${{ experiment: { [TEST_KEY]: { variant: '_variant_name' } } }} | ${['boguskey', '_variant_name']} | ${false}
+      ${{}}                                                           | ${[TEST_KEY, '_variant_name']}   | ${false}
+    `('with input=$input and gon=$gon, returns $output', ({ gon, input, output }) => {
+      window.gon = gon;
+
+      expect(experimentUtils.isExperimentVariant(...input)).toEqual(output);
+    });
+  });
+});

spec/frontend/lib/utils/experimentation_spec.js

-import * as experimentUtils from '~/lib/utils/experimentation';
-
-const TEST_KEY = 'abc';
-
-describe('experiment Utilities', () => {
-  describe('isExperimentEnabled', () => {
-    it.each`
-      experiments              | value
-      ${{ [TEST_KEY]: true }}  | ${true}
-      ${{ [TEST_KEY]: false }} | ${false}
-      ${{ def: true }}         | ${false}
-      ${{}}                    | ${false}
-      ${null}                  | ${false}
-    `('returns correct value of $value for experiments=$experiments', ({ experiments, value }) => {
-      window.gon = { experiments };
-
-      expect(experimentUtils.isExperimentEnabled(TEST_KEY)).toEqual(value);
-    });
-  });
-});

spec/frontend/projects/upload_file_experiment_spec.js

-import ExperimentTracking from '~/experiment_tracking';
+import ExperimentTracking from '~/experimentation/experiment_tracking';
 import * as UploadFileExperiment from '~/projects/upload_file_experiment';
 
-const mockExperimentTrackingEvent = jest.fn();
-jest.mock('~/experiment_tracking', () =>
-  jest.fn().mockImplementation(() => ({
-    event: mockExperimentTrackingEvent,
-  })),
-);
+jest.mock('~/experimentation/experiment_tracking');
 
 const fixture = `<a class='js-upload-file-experiment-trigger' data-toggle='modal' data-target='#modal-upload-blob'></a><div id='modal-upload-blob'></div><div class='project-home-panel empty-project'></div>`;
 const findModal = () => document.querySelector('[aria-modal="true"]');
 const findTrigger = () => document.querySelector('.js-upload-file-experiment-trigger');
 
 beforeEach(() => {
-  ExperimentTracking.mockClear();
-  mockExperimentTrackingEvent.mockClear();
-
   document.body.innerHTML = fixture;
 });
 
@@ -31,7 +23,9 @@ describe('trackUploadFileFormSubmitted', () => {
       label: 'blob-upload-modal',
       property: 'empty',
     });
-    expect(mockExperimentTrackingEvent).toHaveBeenCalledWith('click_upload_modal_form_submit');
+    expect(ExperimentTracking.prototype.event).toHaveBeenCalledWith(
+      'click_upload_modal_form_submit',
+    );
   });
 
   it('initializes ExperimentTracking with the correct arguments when the project is not empty', () => {
@@ -53,6 +47,6 @@ describe('initUploadFileTrigger', () => {
     expect(findModal()).not.toExist();
     findTrigger().click();
     expect(findModal()).toExist();
-    expect(mockExperimentTrackingEvent).toHaveBeenCalledWith('click_upload_modal_trigger');
+    expect(ExperimentTracking.prototype.event).toHaveBeenCalledWith('click_upload_modal_trigger');
   });
 });

spec/frontend/tracking_spec.js

 import { setHTMLFixture } from 'helpers/fixtures';
+import { TRACKING_CONTEXT_SCHEMA } from '~/experimentation/constants';
+import { getExperimentData } from '~/experimentation/utils';
 import Tracking, { initUserTracking, initDefaultTrackers, STANDARD_CONTEXT } from '~/tracking';
 
+jest.mock('~/experimentation/utils', () => ({ getExperimentData: jest.fn() }));
+
 describe('Tracking', () => {
   let snowplowSpy;
   let bindDocumentSpy;
   let trackLoadEventsSpy;
 
   beforeEach(() => {
+    getExperimentData.mockReturnValue(undefined);
+
     window.snowplow = window.snowplow || (() => {});
     window.snowplowOptions = {
       namespace: '_namespace_',
@@ -245,18 +251,18 @@ describe('Tracking', () => {
     });
 
     it('brings in experiment data if linked to an experiment', () => {
-      const data = {
+      const mockExperimentData = {
         variant: 'candidate',
         experiment: 'repo_integrations_link',
         key: '2bff73f6bb8cc11156c50a8ba66b9b8b',
       };
+      getExperimentData.mockReturnValue(mockExperimentData);
 
-      window.gon.global = { experiment: { example: data } };
       document.querySelector('[data-track-event="click_input3"]').click();
 
       expect(eventSpy).toHaveBeenCalledWith('_category_', 'click_input3', {
         value: '_value_',
-        context: { schema: 'iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0', data },
+        context: { schema: TRACKING_CONTEXT_SCHEMA, data: mockExperimentData },
       });
     });
   });
@@ -301,21 +307,21 @@ describe('Tracking', () => {
 
   describe('tracking mixin', () => {
     describe('trackingOptions', () => {
-      it('return the options defined on initialisation', () => {
+      it('returns the options defined on initialisation', () => {
         const mixin = Tracking.mixin({ foo: 'bar' });
         expect(mixin.computed.trackingOptions()).toEqual({ foo: 'bar' });
       });
 
-      it('local tracking value override and extend options', () => {
+      it('lets local tracking value override and extend options', () => {
         const mixin = Tracking.mixin({ foo: 'bar' });
-        //  the value of this in the  vue lifecyle is different, but this serve the tests purposes
+        // The value of this in the Vue lifecyle is different, but this serves the test's purposes
         mixin.computed.tracking = { foo: 'baz', baz: 'bar' };
         expect(mixin.computed.trackingOptions()).toEqual({ foo: 'baz', baz: 'bar' });
       });
     });
 
     describe('trackingCategory', () => {
-      it('return the category set in the component properties first', () => {
+      it('returns the category set in the component properties first', () => {
         const mixin = Tracking.mixin({ category: 'foo' });
         mixin.computed.tracking = {
           category: 'bar',
@@ -323,12 +329,12 @@ describe('Tracking', () => {
         expect(mixin.computed.trackingCategory()).toBe('bar');
       });
 
-      it('return the category set in the options', () => {
+      it('returns the category set in the options', () => {
         const mixin = Tracking.mixin({ category: 'foo' });
         expect(mixin.computed.trackingCategory()).toBe('foo');
       });
 
-      it('if no category is selected returns undefined', () => {
+      it('returns undefined if no category is selected', () => {
         const mixin = Tracking.mixin();
         expect(mixin.computed.trackingCategory()).toBe(undefined);
       });
@@ -363,7 +369,7 @@ describe('Tracking', () => {
         expect(eventSpy).toHaveBeenCalledWith(undefined, 'foo', {});
       });
 
-      it('give precedence to data for category and options', () => {
+      it('gives precedence to data for category and options', () => {
         mixin.trackingCategory = mixin.trackingCategory();
         mixin.trackingOptions = mixin.trackingOptions();
         const data = { category: 'foo', label: 'baz' };