Render experiments on the server (#10665)

config/test.js

@@ -10,9 +10,4 @@ module.exports = {
   enableRequestID: false,
 
   mozillaUserId: 1337,
-  
-  // Because tests run on the server by default and experiments are client-side
-  // only, we don't want to have any active experiments. Otherwise tests try to
-  // send an enrollment event, which triggers an exception.
-  experiments: null,
 };

src/amo/components/AddonVersionCard/index.js

@@ -198,13 +198,11 @@ export const AddonVersionCardBase = (props: InternalProps): React.Node => {
         </>
       </div>
 
-      {addon && version && (
-        <InstallButtonWrapper
-          addon={addon}
-          version={version}
-          showLinkInsteadOfButton={!isCurrentVersion}
-        />
-      )}
+      <InstallButtonWrapper
+        addon={addon}
+        version={version}
+        showLinkInsteadOfButton={!isCurrentVersion}
+      />
 
       {addon && <InstallWarning addon={addon} />}
     </li>

src/amo/components/InstallButtonWrapper/index.js

@@ -39,7 +39,7 @@ import './styles.scss';
 export type Props = {|
   _getClientCompatibility?: typeof getClientCompatibility,
   _findInstallURL?: typeof findInstallURL,
-  addon: AddonType,
+  addon: AddonType | null,
   className?: string,
   defaultButtonText?: string,
   puffy?: boolean,

src/amo/pages/Addon/index.js

@@ -480,7 +480,7 @@ export class AddonBase extends React.Component {
                     <p className="Addon-summary" {...summaryProps} />
                   ) : null}
 
-                  {addon && <InstallButtonWrapper addon={addon} />}
+                  <InstallButtonWrapper addon={addon} />
                 </div>
 
                 <h2 className="visually-hidden">

src/amo/reducers/experiments.js

@@ -0,0 +1,45 @@
+/* @flow */
+export const STORE_EXPERIMENT_VARIANT: 'STORE_EXPERIMENT_VARIANT' =
+  'STORE_EXPERIMENT_VARIANT';
+
+export type ExperimentsState = {| [experimentId: string]: string |};
+
+export const initialState: ExperimentsState = {};
+
+type StoreExperimentVariantParams = {|
+  id: string,
+  variant: string,
+|};
+
+export type StoreExperimentVariantAction = {|
+  type: typeof STORE_EXPERIMENT_VARIANT,
+  payload: StoreExperimentVariantParams,
+|};
+
+export const storeExperimentVariant = ({
+  id,
+  variant,
+}: StoreExperimentVariantParams): StoreExperimentVariantAction => {
+  return {
+    type: STORE_EXPERIMENT_VARIANT,
+    payload: { id, variant },
+  };
+};
+
+type Action = StoreExperimentVariantAction;
+
+export default function experimentsReducer(
+  state: ExperimentsState = initialState,
+  action: Action,
+): ExperimentsState {
+  switch (action.type) {
+    case STORE_EXPERIMENT_VARIANT: {
+      const { id, variant } = action.payload;
+
+      return { ...state, [id]: variant };
+    }
+
+    default:
+      return state;
+  }
+}

src/amo/store.js

@@ -19,6 +19,7 @@ import { createLogger } from 'redux-logger';
 import addonsByAuthors from 'amo/reducers/addonsByAuthors';
 import collections from 'amo/reducers/collections';
 import blocks from 'amo/reducers/blocks';
+import experiments from 'amo/reducers/experiments';
 import home from 'amo/reducers/home';
 import landing from 'amo/reducers/landing';
 import recommendations from 'amo/reducers/recommendations';
@@ -46,6 +47,7 @@ import log from 'amo/logger';
 import type { AddonsByAuthorsState } from 'amo/reducers/addonsByAuthors';
 import type { BlocksState } from 'amo/reducers/blocks';
 import type { CollectionsState } from 'amo/reducers/collections';
+import type { ExperimentsState } from 'amo/reducers/experiments';
 import type { HomeState } from 'amo/reducers/home';
 import type { LandingState } from 'amo/reducers/landing';
 import type { RecommendationsState } from 'amo/reducers/recommendations';
@@ -143,6 +145,7 @@ type InternalAppState = {|
   collections: CollectionsState,
   errorPage: ErrorPageState,
   errors: Object,
+  experiments: ExperimentsState,
   formOverlay: FormOverlayState,
   home: HomeState,
   infoDialog: InfoDialogState,
@@ -199,6 +202,7 @@ export const reducers: AppReducersType = {
   collections,
   errors,
   errorPage,
+  experiments,
   formOverlay,
   home,
   infoDialog,

src/amo/withExperiment.js

@@ -3,9 +3,15 @@ import config from 'config';
 import invariant from 'invariant';
 import * as React from 'react';
 import { withCookies, Cookies } from 'react-cookie';
+import { connect } from 'react-redux';
+import { compose } from 'redux';
 
+import { storeExperimentVariant } from 'amo/reducers/experiments';
 import tracking from 'amo/tracking';
 import { getDisplayName } from 'amo/utils';
+import type { ExperimentsState } from 'amo/reducers/experiments';
+import type { AppState } from 'amo/store';
+import type { DispatchFunc } from 'amo/types/redux';
 
 /*  Usage
  *
@@ -26,13 +32,6 @@ import { getDisplayName } from 'amo/utils';
  *        in the experiment, assign a percentage to the special
  *        `NOT_IN_EXPERIMENT` variant.
  *
- *  Note: Experiments are only executed on the client. The component will
- *        initially be rendered with `variant === null`. Therefore the
- *        version of the component that will be the least disruptive should
- *        be generated when `variant === null`. Once a variant is determined
- *        on the client, the layout of the component will change to match that
- *        of the variant.
- *
  *  Example:
  *
  *     withExperiment({
@@ -75,10 +74,8 @@ type CookieConfig = {|
   secure?: boolean,
 |};
 
-type ExperimentVariant = {|
-  id: string,
-  percentage: number,
-|};
+type ExperimentVariant = {| id: string, percentage: number |};
+type RegisteredExpermients = {| [experimentId: string]: string |};
 
 type withExperimentProps = {|
   _config?: typeof config,
@@ -88,17 +85,13 @@ type withExperimentProps = {|
   variants: ExperimentVariant[],
 |};
 
-type ExpermientVariant = {| id: string, percentage: number |};
-
-type RegisteredExpermients = {| [experimentId: string]: string |};
-
 export const getVariant = ({
   randomizer = Math.random,
   variants,
 }: {|
   randomizer?: () => number,
   variants: ExperimentVariant[],
-|}): ExpermientVariant => {
+|}): string => {
   invariant(
     variants.reduce((total, variant) => total + variant.percentage, 0) === 1,
     'The sum of all percentages in `variants` must be 1',
@@ -110,7 +103,7 @@ export const getVariant = ({
   for (const variant of variants) {
     variantMax = variantMin + variant.percentage;
     if (randomNumber > variantMin && randomNumber <= variantMax) {
-      return variant;
+      return variant.id;
     }
     variantMin = variantMax;
   }
@@ -130,11 +123,17 @@ export const isExperimentEnabled = ({
   return experiments[id] === true;
 };
 
+type WithExperimentsPropsFromState = {|
+  storedVariants: ExperimentsState,
+|};
+
 type withExperimentInternalProps = {|
   ...withExperimentProps,
+  ...WithExperimentsPropsFromState,
   _getVariant: typeof getVariant,
   _isExperimentEnabled: typeof isExperimentEnabled,
   cookies: typeof Cookies,
+  dispatch: DispatchFunc,
   WrappedComponent: React.ComponentType<any>,
 |};
 
@@ -164,6 +163,8 @@ export const withExperiment =
     invariant(defaultVariants, 'variants is required');
 
     class WithExperiment extends React.Component<withExperimentInternalProps> {
+      variant: string | null;
+
       static defaultProps = {
         _getVariant: getVariant,
         _isExperimentEnabled: isExperimentEnabled,
@@ -175,15 +176,66 @@ export const withExperiment =
         WrappedComponent,
       )})`;
 
-      componentDidMount() {
-        const { _getVariant, _isExperimentEnabled, cookies, id, variants } =
-          this.props;
+      constructor(props: withExperimentInternalProps) {
+        super(props);
 
+        this.variant = this.experimentSetup(props).variant;
+      }
+
+      experimentSetup(props) {
+        const {
+          _getVariant,
+          _isExperimentEnabled,
+          dispatch,
+          id,
+          storedVariants,
+          variants,
+        } = props;
+
+        let { variant } = this;
         const isEnabled = _isExperimentEnabled({ _config, id });
         const registeredExperiments = this.getExperiments();
         const experimentInCookie = this.cookieIncludesExperiment(
           registeredExperiments,
         );
+        const addExperimentToCookie = !experimentInCookie;
+        const variantFromStore = storedVariants[id];
+
+        if (isEnabled && !variant) {
+          // Use the variant in the cookie if one exists, otherwise use the
+          // variant from the Redux store.
+          if (experimentInCookie) {
+            variant = registeredExperiments[id];
+          } else if (variantFromStore) {
+            variant = variantFromStore;
+          }
+
+          // Do we need to store the variant in the cookie?
+          if (addExperimentToCookie) {
+            // Determine the variant if we don't already have one.
+            variant = variant || _getVariant({ variants });
+
+            // Store the variant in the Redux store for use during
+            // componentDidMount.
+            dispatch(storeExperimentVariant({ id, variant }));
+          }
+        }
+
+        return {
+          // We only need to add the experiment to the cookie if we have a
+          // variant.
+          addExperimentToCookie: addExperimentToCookie && variant,
+          registeredExperiments,
+          variant,
+        };
+      }
+
+      componentDidMount() {
+        const { _isExperimentEnabled, cookies, id } = this.props;
+
+        const { addExperimentToCookie, registeredExperiments, variant } =
+          this.experimentSetup(this.props);
+
         const experimentsToStore = { ...registeredExperiments };
 
         // Clear any disabled experiments from the cookie.
@@ -195,18 +247,14 @@ export const withExperiment =
           }
         }
 
-        // Do we need to record this experiment in the cookie?
-        const addExperimentToCookie = isEnabled && !experimentInCookie;
-
         if (addExperimentToCookie) {
-          const variantToStore = _getVariant({ variants });
-          experimentsToStore[id] = variantToStore.id;
+          experimentsToStore[id] = variant;
 
-          if (variantToStore.id !== NOT_IN_EXPERIMENT) {
+          if (variant) {
             // Send an enrollment event.
             _tracking.sendEvent({
               _config,
-              action: variantToStore.id,
+              action: variant,
               category: [EXPERIMENT_ENROLLMENT_CATEGORY, id].join(' '),
             });
           }
@@ -233,23 +281,27 @@ export const withExperiment =
         const { _isExperimentEnabled, id, ...props } = this.props;
 
         const isEnabled = _isExperimentEnabled({ _config, id });
-        const registeredExperiments = this.getExperiments();
-
-        const variant =
-          isEnabled && this.cookieIncludesExperiment(registeredExperiments)
-            ? registeredExperiments[id]
-            : null;
 
         const exposedProps: WithExperimentInjectedProps = {
           experimentId: id,
           isExperimentEnabled: isEnabled,
-          isUserInExperiment: variant !== null && variant !== NOT_IN_EXPERIMENT,
-          variant,
+          isUserInExperiment: Boolean(
+            this.variant && this.variant !== NOT_IN_EXPERIMENT,
+          ),
+          variant: this.variant,
         };
 
         return <WrappedComponent {...exposedProps} {...props} />;
       }
     }
 
-    return withCookies(WithExperiment);
+    const mapStateToProps = (
+      state: AppState,
+    ): WithExperimentsPropsFromState => {
+      return {
+        storedVariants: state.experiments,
+      };
+    };
+
+    return compose(withCookies, connect(mapStateToProps))(WithExperiment);
   };

tests/unit/amo/components/TestAddonVersionCard.js

@@ -385,21 +385,6 @@ describe(__filename, () => {
     );
   });
 
-  it('does not render an InstallButtonWrapper if there is no add-on', () => {
-    const root = render({ addon: null });
-
-    expect(root.find(InstallButtonWrapper)).toHaveLength(0);
-  });
-
-  it('does not render an InstallButtonWrapper if there is no version', () => {
-    const root = render({
-      addon: createInternalAddonWithLang(fakeAddon),
-      version: null,
-    });
-
-    expect(root.find(InstallButtonWrapper)).toHaveLength(0);
-  });
-
   describe('InstallWarning', () => {
     it('renders the InstallWarning if an add-on exists', () => {
       const root = render({ addon: createInternalAddonWithLang(fakeAddon) });

tests/unit/amo/pages/TestAddon.js

@@ -279,7 +279,7 @@ describe(__filename, () => {
     });
 
     // These should be empty:
-    expect(root.find(InstallButtonWrapper)).toHaveLength(0);
+    expect(root.find(InstallButtonWrapper)).toHaveProp('addon', null);
     expect(root.find(AddonCompatibilityError)).toHaveProp('addon', null);
     expect(root.find(RatingManager)).toHaveLength(0);
 

tests/unit/amo/reducers/test_experiments.js

@@ -0,0 +1,37 @@
+import reducer, {
+  initialState,
+  storeExperimentVariant,
+} from 'amo/reducers/experiments';
+
+describe(__filename, () => {
+  describe('reducer', () => {
+    const id = 'some_experiment_id';
+    const variant = 'some-variant';
+
+    it('initializes properly', () => {
+      const state = reducer(undefined, { type: 'NONE' });
+
+      expect(state).toEqual(initialState);
+    });
+
+    it('stores an experiment variant', () => {
+      const state = reducer(undefined, storeExperimentVariant({ id, variant }));
+
+      expect(state[id]).toEqual(variant);
+    });
+
+    it('can store variants for multiple experiments', () => {
+      const anotherId = 'some_other_experiment_id';
+      const anotherVariant = 'another-variant';
+
+      let state = reducer(undefined, storeExperimentVariant({ id, variant }));
+      state = reducer(
+        state,
+        storeExperimentVariant({ id: anotherId, variant: anotherVariant }),
+      );
+
+      expect(state[id]).toEqual(variant);
+      expect(state[anotherId]).toEqual(anotherVariant);
+    });
+  });
+});

tests/unit/amo/test_withExperiment.js

@@ -1,6 +1,7 @@
 import { shallow } from 'enzyme';
 import * as React from 'react';
 
+import { storeExperimentVariant } from 'amo/reducers/experiments';
 import {
   EXPERIMENT_COOKIE_NAME,
   EXPERIMENT_ENROLLMENT_CATEGORY,
@@ -14,8 +15,8 @@ import {
 import * as defaultConfig from 'config/default';
 import {
   createFakeTracking,
+  dispatchClientMetadata,
   fakeCookies,
-  fakeVariant,
   getFakeConfig,
 } from 'tests/unit/helpers';
 
@@ -40,6 +41,7 @@ describe(__filename, () => {
     cookies = fakeCookies(),
     experimentProps,
     props,
+    store = dispatchClientMetadata().store,
   } = {}) => {
     const id = (experimentProps && experimentProps.id) || makeId('some-id');
     const allExperimentProps = {
@@ -68,7 +70,7 @@ describe(__filename, () => {
     // See: https://github.com/mozilla/addons-frontend/issues/6839
     //
     // 1. Render everything
-    const root = shallow(<SomeComponent {...props} />);
+    const root = shallow(<SomeComponent store={store} {...props} />);
     // 2. Get and render the withExperiment HOC (inside withCookies() HOC)
     return shallow(root.props().children(cookies));
   };
@@ -76,10 +78,12 @@ describe(__filename, () => {
   const render = (props = {}) => {
     const root = renderWithExperiment(props);
     // Return the wrapped instance (`SomeComponentBase`)
-    return root.dive();
+    // We have to dive twice because of the withCookies HOC and the connect
+    // wrapper.
+    return root.dive().dive();
   };
 
-  it('injects a variant prop', () => {
+  it('injects a variant prop from a cookie', () => {
     const id = makeId('test-id');
     const variantId = 'some-variant-id';
     const cookies = fakeCookies({
@@ -91,6 +95,59 @@ describe(__filename, () => {
     expect(root).toHaveProp('variant', variantId);
   });
 
+  it('injects a variant prop from the redux store', () => {
+    const { store } = dispatchClientMetadata();
+    const id = makeId('test-id');
+    const variantId = 'some-variant-id';
+    const cookies = fakeCookies({
+      get: sinon.stub().returns(undefined),
+    });
+
+    store.dispatch(storeExperimentVariant({ id, variant: variantId }));
+
+    const root = render({ cookies, experimentProps: { id }, store });
+
+    expect(root).toHaveProp('variant', variantId);
+  });
+
+  // If a cookie exists, we always want to use it, to maintain consistency
+  // for the user.
+  it('prefers a variant from a cookie to one from the the redux store', () => {
+    const { store } = dispatchClientMetadata();
+    const id = makeId('test-id');
+    const cookieVariant = 'cookie-variant';
+    const storeVariant = 'store-variant';
+    const cookies = fakeCookies({
+      get: sinon
+        .stub()
+        .returns(createExperimentData({ id, variantId: cookieVariant })),
+    });
+
+    store.dispatch(storeExperimentVariant({ id, variant: storeVariant }));
+
+    const root = render({ cookies, experimentProps: { id }, store });
+
+    expect(root).toHaveProp('variant', cookieVariant);
+  });
+
+  // Test for https://github.com/mozilla/addons-frontend/issues/10681
+  it('injects a newly created variant prop', () => {
+    const id = makeId('hero');
+    const variantId = 'some-variant-id';
+    const cookies = fakeCookies({
+      get: sinon.stub().returns(undefined),
+    });
+    const _getVariant = sinon.stub().returns(variantId);
+
+    const root = render({
+      cookies,
+      experimentProps: { id },
+      props: { _getVariant },
+    });
+
+    expect(root).toHaveProp('variant', variantId);
+  });
+
   it('injects an experimentId', () => {
     const id = makeId('injected-id');
 
@@ -161,7 +218,7 @@ describe(__filename, () => {
       { id: 'some-variant-a', percentage: 0.5 },
       { id: 'some-variant-b', percentage: 0.5 },
     ];
-    const _getVariant = sinon.stub().returns(variants[0]);
+    const _getVariant = sinon.stub().returns(variants[0].id);
 
     render({
       cookies,
@@ -179,6 +236,70 @@ describe(__filename, () => {
     );
   });
 
+  it('does not use a stored variant if it is not for this experiment', () => {
+    const otherExperimentVariant = 'variant-for-another-experiment';
+    const thisExperimentVariant = 'variant-for-this-experiment';
+    const id = makeId('hero');
+    const cookies = fakeCookies({
+      get: sinon.stub().returns(undefined),
+    });
+    const _getVariant = sinon.stub().returns(thisExperimentVariant);
+    const { store } = dispatchClientMetadata();
+
+    // Store a variant for a different experiment.
+    store.dispatch(
+      storeExperimentVariant({
+        id: `different_${id}`,
+        variant: otherExperimentVariant,
+      }),
+    );
+
+    const root = render({
+      cookies,
+      experimentProps: { id },
+      props: { _getVariant },
+      store,
+    });
+
+    // This expected variant comes from the mocked _getVariant.
+    expect(root).toHaveProp('variant', thisExperimentVariant);
+  });
+
+  it('uses the stored variant for this experiment, even when others are present', () => {
+    const otherExperimentVariant = 'variant-for-another-experiment';
+    const thisExperimentVariant = 'variant-for-this-experiment';
+    const id = makeId('hero');
+    const cookies = fakeCookies({
+      get: sinon.stub().returns(undefined),
+    });
+    const { store } = dispatchClientMetadata();
+
+    // Store a variant for this experiment.
+    store.dispatch(
+      storeExperimentVariant({
+        id,
+        variant: thisExperimentVariant,
+      }),
+    );
+
+    // Store a variant for a different experiment.
+    store.dispatch(
+      storeExperimentVariant({
+        id: `different_${id}`,
+        variant: otherExperimentVariant,
+      }),
+    );
+
+    const root = render({
+      cookies,
+      experimentProps: { id },
+      store,
+    });
+
+    // This expected variant comes from the store.
+    expect(root).toHaveProp('variant', thisExperimentVariant);
+  });
+
   it('creates a cookie upon render if the current experiment is not already in the cookie', () => {
     const experimentId = makeId('thisExperiment');
     const anotherExperimentId = makeId('anotherExperiment');
@@ -194,7 +315,7 @@ describe(__filename, () => {
         ...createExperimentData({ id: anotherExperimentId, variantId }),
       }),
     });
-    const _getVariant = sinon.stub().returns({ ...fakeVariant, id: variantId });
+    const _getVariant = sinon.stub().returns(variantId);
 
     render({
       configOverrides,
@@ -214,6 +335,29 @@ describe(__filename, () => {
     );
   });
 
+  it('dispatches storeExperimentVariant to store the variant', () => {
+    const id = makeId('hero');
+    const variantId = 'some-variant-id';
+    const cookies = fakeCookies({
+      get: sinon.stub().returns(undefined),
+    });
+    const _getVariant = sinon.stub().returns(variantId);
+    const { store } = dispatchClientMetadata();
+    const fakeDispatch = sinon.stub(store, 'dispatch');
+
+    render({
+      cookies,
+      experimentProps: { id },
+      props: { _getVariant },
+      store,
+    });
+
+    sinon.assert.calledWith(
+      fakeDispatch,
+      storeExperimentVariant({ id, variant: variantId }),
+    );
+  });
+
   it('adds an experiment to the cookie, and removes a disabled one, as expected', () => {
     const experimentId = makeId('thisExperiment');
     const anotherExperimentId = makeId('anotherExperiment');
@@ -229,7 +373,7 @@ describe(__filename, () => {
         ...createExperimentData({ id: anotherExperimentId, variantId }),
       }),
     });
-    const _getVariant = sinon.stub().returns({ ...fakeVariant, id: variantId });
+    const _getVariant = sinon.stub().returns(variantId);
 
     render({
       configOverrides,
@@ -269,7 +413,7 @@ describe(__filename, () => {
         .stub()
         .returns(createExperimentData({ id: anotherExperimentId })),
     });
-    const _getVariant = sinon.stub().returns({ ...fakeVariant, id: variantId });
+    const _getVariant = sinon.stub().returns(variantId);
     const _tracking = createFakeTracking();
 
     render({
@@ -287,24 +431,6 @@ describe(__filename, () => {
     );
   });
 
-  it('does not send an enrollment event if the user is not in the experiment', () => {
-    const cookies = fakeCookies({
-      get: sinon.stub().returns(undefined),
-    });
-    const _tracking = createFakeTracking();
-    const _getVariant = sinon
-      .stub()
-      .returns({ ...fakeVariant, id: NOT_IN_EXPERIMENT });
-
-    render({
-      cookies,
-      experimentProps: { _tracking },
-      props: { _getVariant },
-    });
-
-    sinon.assert.notCalled(_tracking.sendEvent);
-  });
-
   it('does not send an enrollment event if the user is in the experiment', () => {
     const id = makeId('hero');
     const cookies = fakeCookies({
@@ -346,7 +472,7 @@ describe(__filename, () => {
     const variantId = 'some-variant-id';
     const cookies = fakeCookies();
     const cookieConfig = { path: '/test' };
-    const _getVariant = sinon.stub().returns({ ...fakeVariant, id: variantId });
+    const _getVariant = sinon.stub().returns(variantId);
 
     render({
       cookies,
@@ -455,7 +581,9 @@ describe(__filename, () => {
       (expectedVariant, randomNumber) => {
         const randomizer = sinon.stub().returns(randomNumber);
 
-        expect(getVariant({ randomizer, variants })).toEqual(expectedVariant);
+        expect(getVariant({ randomizer, variants })).toEqual(
+          expectedVariant.id,
+        );
         sinon.assert.called(randomizer);
       },
     );

tests/unit/helpers.js

@@ -936,7 +936,7 @@ export function apiResponsePage({
 export function shallowUntilTarget(
   componentInstance,
   TargetComponent,
-  { maxTries = 10, shallowOptions, _shallow = shallow } = {},
+  { maxTries = 12, shallowOptions, _shallow = shallow } = {},
 ) {
   if (!componentInstance) {
     throw new Error('componentInstance parameter is required');
@@ -1474,11 +1474,6 @@ export const createInternalSuggestionWithLang = (
   return createInternalSuggestion(suggestion, lang);
 };
 
-export const fakeVariant = Object.freeze({
-  id: 'some-variant-id',
-  percentage: 0.5,
-});
-
 export const fakeTrackingEvent = Object.freeze({
   action: 'some-action',
   category: 'some-category',