Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implemented product recommendations experiment #174

Merged
7 changes: 6 additions & 1 deletion src/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
import { reduxHooks } from 'hooks';
import Dashboard from 'containers/Dashboard';
import ZendeskFab from 'components/ZendeskFab';
import { ExperimentProvider } from 'experimentContext';

import track from 'tracking';

Expand Down Expand Up @@ -84,7 +85,11 @@ export const App = () => {
<Alert variant="danger">
<ErrorPage message={formatMessage(messages.errorMessage, { supportEmail })} />
</Alert>
) : (<Dashboard />)}
) : (
<ExperimentProvider>
<Dashboard />
</ExperimentProvider>
)}
</main>
<Footer logo={process.env.LOGO_POWERED_BY_OPEN_EDX_URL_SVG} />
<ZendeskFab />
Expand Down
6 changes: 5 additions & 1 deletion src/App.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { RequestKeys } from 'data/constants/requests';
import { reduxHooks } from 'hooks';
import Dashboard from 'containers/Dashboard';
import LearnerDashboardHeader from 'containers/LearnerDashboardHeader';
import { ExperimentProvider } from 'experimentContext';
import { App } from './App';
import messages from './messages';

Expand All @@ -21,6 +22,9 @@ jest.mock('@edx/frontend-component-footer', () => 'Footer');
jest.mock('containers/Dashboard', () => 'Dashboard');
jest.mock('containers/LearnerDashboardHeader', () => 'LearnerDashboardHeader');
jest.mock('components/ZendeskFab', () => 'ZendeskFab');
jest.mock('experimentContext', () => ({
ExperimentProvider: 'ExperimentProvider',
}));
jest.mock('data/redux', () => ({
selectors: 'redux.selectors',
actions: 'redux.actions',
Expand Down Expand Up @@ -71,7 +75,7 @@ describe('App router component', () => {
runBasicTests();
it('loads dashboard', () => {
expect(el.find('main')).toMatchObject(shallow(
<main><Dashboard /></main>,
<main><ExperimentProvider><Dashboard /></ExperimentProvider></main>,
));
});
});
Expand Down
4 changes: 3 additions & 1 deletion src/__snapshots__/App.test.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ exports[`App router component component no network failure snapshot 1`] = `
<div>
<LearnerDashboardHeader />
<main>
<Dashboard />
<ExperimentProvider>
<Dashboard />
</ExperimentProvider>
</main>
<Footer
logo="fakeLogo.png"
Expand Down
4 changes: 2 additions & 2 deletions src/containers/WidgetContainers/LoadedSidebar/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import RecommendationsPanel from 'widgets/RecommendationsPanel';
import hooks from 'widgets/ProductRecommendations/hooks';

export const WidgetSidebar = ({ setSidebarShowing }) => {
const { shouldShowFooter } = hooks.useShowRecommendationsFooter();
const { inRecommendationsVariant, isExperimentActive } = hooks.useShowRecommendationsFooter();

if (!shouldShowFooter) {
if (!inRecommendationsVariant && isExperimentActive) {
setSidebarShowing(true);

return (
Expand Down
18 changes: 15 additions & 3 deletions src/containers/WidgetContainers/LoadedSidebar/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,31 @@ describe('WidgetSidebar', () => {
describe('snapshots', () => {
test('default', () => {
hooks.useShowRecommendationsFooter.mockReturnValueOnce(
mockFooterRecommendationsHook.dontShowOrLoad,
mockFooterRecommendationsHook.activeControl,
);
const wrapper = shallow(<WidgetSidebar {...props} />);

expect(props.setSidebarShowing).toHaveBeenCalledWith(true);
expect(wrapper).toMatchSnapshot();
});
});

test('is hidden if footer is shown', () => {
test('is hidden when the has the default values', () => {
hooks.useShowRecommendationsFooter.mockReturnValueOnce(
mockFooterRecommendationsHook.default,
);
const wrapper = shallow(<WidgetSidebar {...props} />);

expect(props.setSidebarShowing).not.toHaveBeenCalled();
expect(wrapper.type()).toBeNull();
});

test('is hidden when the has the treatment values', () => {
hooks.useShowRecommendationsFooter.mockReturnValueOnce(
mockFooterRecommendationsHook.showDontLoad,
mockFooterRecommendationsHook.activeTreatment,
);
const wrapper = shallow(<WidgetSidebar {...props} />);

expect(props.setSidebarShowing).not.toHaveBeenCalled();
expect(wrapper.type()).toBeNull();
});
Expand Down
4 changes: 2 additions & 2 deletions src/containers/WidgetContainers/NoCoursesSidebar/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import RecommendationsPanel from 'widgets/RecommendationsPanel';
import hooks from 'widgets/ProductRecommendations/hooks';

export const WidgetSidebar = ({ setSidebarShowing }) => {
const { shouldShowFooter } = hooks.useShowRecommendationsFooter();
const { inRecommendationsVariant, isExperimentActive } = hooks.useShowRecommendationsFooter();

if (!shouldShowFooter) {
if (!inRecommendationsVariant && isExperimentActive) {
setSidebarShowing(true);

return (
Expand Down
18 changes: 15 additions & 3 deletions src/containers/WidgetContainers/NoCoursesSidebar/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,31 @@ describe('WidgetSidebar', () => {
describe('snapshots', () => {
test('default', () => {
hooks.useShowRecommendationsFooter.mockReturnValueOnce(
mockFooterRecommendationsHook.dontShowOrLoad,
mockFooterRecommendationsHook.activeControl,
);
const wrapper = shallow(<WidgetSidebar {...props} />);

expect(props.setSidebarShowing).toHaveBeenCalledWith(true);
expect(wrapper).toMatchSnapshot();
});
});

test('is hidden if footer is shown', () => {
test('is hidden when the has the default values', () => {
hooks.useShowRecommendationsFooter.mockReturnValueOnce(
mockFooterRecommendationsHook.default,
);
const wrapper = shallow(<WidgetSidebar {...props} />);

expect(props.setSidebarShowing).not.toHaveBeenCalled();
expect(wrapper.type()).toBeNull();
});

test('is hidden when the has the treatment values', () => {
hooks.useShowRecommendationsFooter.mockReturnValueOnce(
mockFooterRecommendationsHook.showDontLoad,
mockFooterRecommendationsHook.activeTreatment,
);
const wrapper = shallow(<WidgetSidebar {...props} />);

expect(props.setSidebarShowing).not.toHaveBeenCalled();
expect(wrapper.type()).toBeNull();
});
Expand Down
5 changes: 3 additions & 2 deletions src/containers/WidgetContainers/WidgetFooter/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ import ProductRecommendations from 'widgets/ProductRecommendations';
import hooks from 'widgets/ProductRecommendations/hooks';

export const WidgetFooter = () => {
const { shouldShowFooter, shouldLoadFooter } = hooks.useShowRecommendationsFooter();
hooks.useActivateRecommendationsExperiment();
const { inRecommendationsVariant, isExperimentActive } = hooks.useShowRecommendationsFooter();

if (shouldShowFooter && shouldLoadFooter) {
if (inRecommendationsVariant && isExperimentActive) {
return (
<div className="widget-footer">
<ProductRecommendations />
Expand Down
17 changes: 12 additions & 5 deletions src/containers/WidgetContainers/WidgetFooter/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,40 @@ import WidgetFooter from '.';

jest.mock('widgets/LookingForChallengeWidget', () => 'LookingForChallengeWidget');
jest.mock('widgets/ProductRecommendations/hooks', () => ({
useActivateRecommendationsExperiment: jest.fn(),
useShowRecommendationsFooter: jest.fn(),
}));

describe('WidgetFooter', () => {
describe('snapshots', () => {
test('default', () => {
hooks.useShowRecommendationsFooter.mockReturnValueOnce(
mockFooterRecommendationsHook.showAndLoad,
mockFooterRecommendationsHook.activeTreatment,
);
const wrapper = shallow(<WidgetFooter />);

expect(hooks.useActivateRecommendationsExperiment).toHaveBeenCalled();
expect(wrapper).toMatchSnapshot();
});
});

test('is hidden when shouldShowFooter is false but shouldLoadFooter is true', () => {
test('is hidden when the experiment has the default values', () => {
hooks.useShowRecommendationsFooter.mockReturnValueOnce(
mockFooterRecommendationsHook.loadDontShow,
mockFooterRecommendationsHook.default,
);
const wrapper = shallow(<WidgetFooter />);

expect(hooks.useActivateRecommendationsExperiment).toHaveBeenCalled();
expect(wrapper.type()).toBeNull();
});

test('is hidden when shouldLoadFooter is false but shouldShowFooter is true', () => {
test('is hidden when the experiment has the control values', () => {
hooks.useShowRecommendationsFooter.mockReturnValueOnce(
mockFooterRecommendationsHook.showDontLoad,
mockFooterRecommendationsHook.activeControl,
);
const wrapper = shallow(<WidgetFooter />);

expect(hooks.useActivateRecommendationsExperiment).toHaveBeenCalled();
expect(wrapper.type()).toBeNull();
});
});
74 changes: 74 additions & 0 deletions src/experimentContext.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My one request here is that this file get renamed to ExperimentContext, as is exporting a component provider.

import PropTypes from 'prop-types';
import { useWindowSize, breakpoints } from '@edx/paragon';
import { StrictDict } from 'utils';
import api from 'widgets/ProductRecommendations/api';
import * as module from './experimentContext';

export const state = StrictDict({
experiment: (val) => React.useState(val), // eslint-disable-line
countryCode: (val) => React.useState(val), // eslint-disable-line

Check warning on line 10 in src/experimentContext.jsx

View check run for this annotation

Codecov / codecov/patch

src/experimentContext.jsx#L9-L10

Added lines #L9 - L10 were not covered by tests
});

export const useCountryCode = (setCountryCode) => {
React.useEffect(() => {
let isMounted = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this important?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React.useEffect should be part of the component. useCountryCode should just do the fetching in this case. Then useEffect(fn, []) should run only once. There won't be need for isMounted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, I can't really think of a scenario where the component will be unmounted, I'll adjust it


api
.fetchRecommendationsContext()
.then((response) => {
if (isMounted) {
setCountryCode(response.data.countryCode);
}
})
.catch(() => {
if (isMounted) {
setCountryCode('');
}
});

return () => {
isMounted = false;
};
/* eslint-disable */
}, []);
};

export const ExperimentContext = React.createContext();

export const ExperimentProvider = ({ children }) => {
const [countryCode, setCountryCode] = module.state.countryCode(null);
const [experiment, setExperiment] = module.state.experiment({
isExperimentActive: false,
inRecommendationsVariant: true,
});

module.useCountryCode(setCountryCode);
const { width } = useWindowSize();
const isMobile = width < breakpoints.small.minWidth;

const contextValue = React.useMemo(
() => ({
experiment,
countryCode,
setExperiment,
setCountryCode,
isMobile,
}),
[experiment, countryCode, setExperiment, setCountryCode, isMobile]
);

return (
<ExperimentContext.Provider value={contextValue}>
{children}
</ExperimentContext.Provider>
);
};

export const useExperimentContext = () => React.useContext(ExperimentContext);

ExperimentProvider.propTypes = {
children: PropTypes.node.isRequired,
};

export default { useCountryCode, useExperimentContext };
Loading