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

Ab test rammeverk #353

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ yarn-error.log
/htdocs/
.vscode
.env
secrets.json
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"lint-es": "eslint --cache --cache-location '.eslintcache/' --ext .js --ext .jsx --max-warnings=0 src e2e",
"format": "node prettier.js write",
"format-check": "node prettier.js lint",
"start": "razzle start --inspect --inspect-port 9230",
"start": "cross-env GOOGLE_APPLICATION_CREDENTIALS=./secrets.json razzle start --inspect --inspect-port 9230",
Copy link
Author

Choose a reason for hiding this comment

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

Denne env variabelen bør sikkert settes på en annen måte

Copy link
Contributor

Choose a reason for hiding this comment

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

Hvor brukes denne?

Copy link
Author

Choose a reason for hiding this comment

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

Denne brukes til å sette client og server secrets for autentisering mot Google analytics management API. Optimalt bør disse verdiene settes som env variabler i de forskjellige miljøene slik at vi slipper secrets.json

"start-without-ssr": "cross-env RAZZLE_DISABLE_SSR=true razzle start",
"start-with-local-graphql": "cross-env LOCAL_GRAPHQL_API=true yarn start",
"start-with-local-graphql-and-article-converter": "cross-env LOCAL_GRAPHQL_API=true RAZZLE_LOCAL_ARTICLE_CONVERTER=true razzle start",
Expand Down Expand Up @@ -80,6 +80,7 @@
"defined": "1.0.0",
"express": "^4.16.3",
"express-http-proxy": "^1.4.0",
"googleapis": "^39.2.0",
"graphql": "^14.0.2",
"graphql-tag": "^2.10.0",
"helmet": "^3.15.1",
Expand Down Expand Up @@ -116,6 +117,7 @@
"sass-loader": "^7.1.0",
"serialize-javascript": "^1.5.0",
"source-map-support": "^0.5.9",
"universal-analytics": "^0.4.20",
Copy link
Contributor

Choose a reason for hiding this comment

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

Ser ikke at denne er brukt noe sted?

Copy link
Author

Choose a reason for hiding this comment

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

Den skal jeg fjerne. Gammelt rusk som vi glemte å ta ut.

"warning": "^4.0.3",
"webpack-bundle-analyzer": "^3.0.4"
},
Expand Down
7 changes: 7 additions & 0 deletions src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import Masthead from './containers/Masthead';
import { routes } from './routes';
import handleError from './util/handleError';
import ErrorPage from './containers/ErrorPage/ErrorPage';
import { NDLA_ABTEST_COOKIE_KEY } from './util/getABTests';
import { isValidCookie, deleteCookie, setCookie } from './util/cookieHandler';

const Route = ({
component: Component,
Expand Down Expand Up @@ -81,6 +83,11 @@ class App extends React.Component {
) {
this.handleLoadInitialProps(this.props);
}
// Update/Set cookies
if (isValidCookie(NDLA_ABTEST_COOKIE_KEY, document.cookie)) {
deleteCookie(NDLA_ABTEST_COOKIE_KEY)
}
setCookie(NDLA_ABTEST_COOKIE_KEY, JSON.stringify(this.props.initialProps.experiments));
}

componentDidUpdate() {
Expand Down
2 changes: 1 addition & 1 deletion src/client.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,4 @@ renderOrHydrate(

if (module.hot) {
module.hot.accept();
}
}
24 changes: 24 additions & 0 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,29 @@ const gaTrackingId = () => {
}
};

const gaExperiments = () => {
switch (ndlaEnvironment) {
case 'local':
return {};
case 'dev':
// test IDs
return {
GOOGLE_ACCOUNT_ID: "74405776",
Copy link
Author

Choose a reason for hiding this comment

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

Dette er en test konto for å verifisere under utvikling. Dette må konfigureres opp for de forskjellige miljøene.

GOOGLE_WEB_PROPERTY_ID: "UA-74405776-4",
GOOGLE_PROFILE_ID: "140877480",
};
case 'prod':
//replace with correct IDs
return {
GOOGLE_ACCOUNT_ID: "",
GOOGLE_WEB_PROPERTY_ID: "",
GOOGLE_PROFILE_ID: "",
};
default:
return {};
}
}

const logglyApiKey = () => {
if (process.env.NODE_ENV === 'unittest') {
return '';
Expand All @@ -93,6 +116,7 @@ const config = {
learningPathDomain: learningPathDomain(),
googleTagManagerId: getEnvironmentVariabel('NDLA_GOOGLE_TAG_MANAGER_ID'),
gaTrackingId: gaTrackingId(),
gaExperiments: gaExperiments(),
zendeskWidgetKey: getEnvironmentVariabel('NDLA_ZENDESK_WIDGET_KEY'),
localGraphQLApi: getEnvironmentVariabel('LOCAL_GRAPHQL_API', false),
showAllFrontpageSubjects: true,
Expand Down
1 change: 1 addition & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,3 +258,4 @@ export const SEARCH_PATH = '/search(.*)';
export const TOPIC_PATH = '/subjects/:subjectId/:topicPath(.*)?/:topicId';
export const SUBJECT_PAGE_PATH = '/subjects/:subjectId';
export const SUBJECTS = '/subjects';
export const COLLECT_EXPERIMENTS = '/experiments';
2 changes: 1 addition & 1 deletion src/containers/Masthead/components/MastheadMenu.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class MastheadMenu extends Component {

return (
<Fragment>
<MastheadMenuModal onMenuExit={this.onMenuExit}>
<MastheadMenuModal>
{onClose => (
<MastheadTopics
onClose={onClose}
Expand Down
94 changes: 75 additions & 19 deletions src/containers/Masthead/components/MastheadMenuModal.jsx
Original file line number Diff line number Diff line change
@@ -1,28 +1,84 @@
import React from 'react';
import PropTypes from 'prop-types';
import Modal from '@ndla/modal';
import { injectT } from '@ndla/i18n';
import { TopicMenuButton } from '@ndla/ui';
import { ExperimentsContext, Experiment, Variant, isValidExperiment } from '@ndla/abtest';
import { Hamburger, Menu } from '@ndla/icons/common';

const MastheadMenuModal = ({ children, onMenuExit, t }) => (
<Modal
size="fullscreen"
activateButton={
<TopicMenuButton data-testid="masthead-menu-button">
{t('masthead.menu.title')}
</TopicMenuButton>
}
animation="subtle"
animationDuration={150}
backgroundColor="grey"
noBackdrop
onClose={onMenuExit}>
{children}
</Modal>
const experimentId = 'OtejUgVLRHmGTAGv7jbFyA';

const MastheadMenuModal = ({t, children}) => (
<ExperimentsContext.Consumer>
Copy link
Contributor

Choose a reason for hiding this comment

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

Bruk useContext i stedet

{({ experiments }) => (
<Modal
size="fullscreen"
onOpen={() => {
if(isValidExperiment({ experiments, id: experimentId })) { // check if this experimentId exists in data from server
// this event can be tracked by optimize to evaluate variant effectiveness
window.ga('send', {
hitType: 'event',
eventCategory: 'User interactions',
eventAction: 'MenuButtonOpen',
});
}
}}
activateButton={
<TopicMenuButton Icon={null} data-testid="masthead-menu-button">
<Experiment
id={experimentId}
experiments={experiments}
onVariantMount={({expId, expVar, isActiveExperiment}) => {
if(isActiveExperiment) {
window.ga('set', { expId, expVar }); // Perhaps get GA from ndla/tracker?
Copy link
Author

Choose a reason for hiding this comment

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

Tracking via GA er kanskje skilt ut i en pakke allerede?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ja, og abtest-pakken burde vel egentlig vært en del av ndla/tracker og ikke en egen pakke? Så burde man vel kanskje gjøre window.ga-funksjonene i Experiment / Variant komponenten?

Copy link
Author

Choose a reason for hiding this comment

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

Bra innspill.

window.ga('send', { // report display of experiment variant to analytics/optimize
hitType: 'event',
eventCategory: 'AB test',
eventAction: 'Display variant',
fieldsObject: {
nonInteraction: true,
}
});
}}
}
>
<Variant variantIndex={0} original>
<Menu /> {t('masthead.menu.title')} orginal
</Variant>
<Variant variantIndex={1}>
<Menu /> {t('abTests.masthead.menu.subjectOverview')}
</Variant>
<Variant variantIndex={2}>
<Menu /> {t('abTests.masthead.menu.overview')}
</Variant>
<Variant variantIndex={3}>
<Menu /> {t('abTests.masthead.menu.topics')}
</Variant>
<Variant variantIndex={4}>
<Hamburger /> {t('masthead.menu.title')}
</Variant>
<Variant variantIndex={5}>
<Hamburger /> {t('abTests.masthead.menu.subjectOverview')}
</Variant>
<Variant variantIndex={6}>
<Hamburger /> {t('abTests.masthead.menu.overview')}
</Variant>
<Variant variantIndex={7}>
<Hamburger /> {t('abTests.masthead.menu.topics')}
</Variant>
</Experiment>
</TopicMenuButton>
}
animation="subtle"
animationDuration={150}
backgroundColor="grey"
noBackdrop
>
{children}
</Modal>
)}
</ExperimentsContext.Consumer>
);

MastheadMenuModal.propTypes = {
onMenuExit: PropTypes.func,
};
MastheadMenuModal.contentType = ExperimentsContext;

export default injectT(MastheadMenuModal);
9 changes: 8 additions & 1 deletion src/routes.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import React from 'react';
import { ExperimentsContext } from '@ndla/abtest';
import WelcomePage from './containers/WelcomePage/WelcomePage';
import ArticlePage from './containers/ArticlePage/ArticlePage';
import PlainArticlePage from './containers/PlainArticlePage/PlainArticlePage';
Expand Down Expand Up @@ -69,5 +70,11 @@ export const routes = [
];

export default function(initialProps = {}, locale) {
return <App initialProps={initialProps} locale={locale} />;
return (
<ExperimentsContext.Provider value={{
experiments: initialProps.experiments,
}}>
<App initialProps={initialProps} locale={locale} />
</ExperimentsContext.Provider>
);
}
13 changes: 11 additions & 2 deletions src/server/helpers/Document.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,17 @@ const Document = ({ helmet, assets, data }) => {
rel="stylesheet"
href="https://fonts.googleapis.com/css?family=Source+Sans+Pro:300,400,400i,600,700|Source+Serif+Pro:400,700"
/>
{config.gaTrackingId && (
<script async src="https://www.google-analytics.com/analytics.js" />
{!config.gaTrackingId && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Skal det være '!'?

<script
dangerouslySetInnerHTML={{
// GA bør sikkert håndteres på en annen måte, via helper eller lib etc
__html: `(function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){
(i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o),
m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m)
})(window,document,'script','https://www.google-analytics.com/analytics.js','ga');
window.ga('create', 'UA-74405776-4', 'auto');`
Copy link
Author

Choose a reason for hiding this comment

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

Oppretting av tracker er muligens håndtert annerledes i dag?

}}
/>
)}
<GoogleTagMangerScript />
{helmet.title.toComponent()}
Expand Down
53 changes: 32 additions & 21 deletions src/server/routes/defaultRoute.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { createApolloClient } from '../../util/apiHelpers';
import handleError from '../../util/handleError';
import { getLocaleInfoFromPath } from '../../i18n';
import { renderHtml, renderPage } from '../helpers/render';
import fetchExperiments from '../../util/getABTests';

const assets = require(process.env.RAZZLE_ASSETS_MANIFEST); //eslint-disable-line

Expand Down Expand Up @@ -53,6 +54,8 @@ const disableSSR = req => {
async function doRender(req) {
global.assets = assets; // used for including mathjax js in pages with math
let initialProps = { loading: true };
let googleOptimizeExperiments;

const {
abbreviation: locale,
messages,
Expand All @@ -65,29 +68,37 @@ async function doRender(req) {
if (!disableSSR(req)) {
const route = serverRoutes.find(r => matchPath(basepath, r));
const match = matchPath(basepath, route);
initialProps = await loadGetInitialProps(route.component, {
isServer: true,
locale,
match,
client,
location: {
search: `?${queryString.stringify(req.query)}`,
},
});
[ googleOptimizeExperiments, initialProps ] = await Promise.all([
await fetchExperiments(req),
await loadGetInitialProps(route.component, {
isServer: true,
locale,
match,
client,
location: {
search: `?${queryString.stringify(req.query)}`,
},
}),
]);
} else {
googleOptimizeExperiments = await fetchExperiments(req);
}

initialProps.experiments = googleOptimizeExperiments;
const context = {};
const Page = !disableSSR(req) ? (
<ApolloProvider client={client}>
<IntlProvider locale={locale} messages={messages}>
<StaticRouter basename={basename} location={req.url} context={context}>
{routes(initialProps, locale)}
</StaticRouter>
</IntlProvider>
</ApolloProvider>
) : (
''
);
let Page;
if (!disableSSR(req)) {
Page = (
<ApolloProvider client={client}>
<IntlProvider locale={locale} messages={messages}>
<StaticRouter basename={basename} location={req.url} context={context}>
{routes(initialProps, locale)}
</StaticRouter>
</IntlProvider>
</ApolloProvider>
);
} else {
Page = '';
}

const apolloState = client.extract();
const { html, ...docProps } = renderPage(Page, getAssets(), {
Expand Down
64 changes: 64 additions & 0 deletions src/server/routes/experimentsRoute.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* Copyright (c) 2017-present, NDLA.
*
* This source code is licensed under the GPLv3 license found in the
* LICENSE file in the root directory of this source tree.
*
*/

import { google } from 'googleapis';
import serverConfig from '../../config';

const config = serverConfig.gaExperiments;

const accountId = config.GOOGLE_ACCOUNT_ID;
const webPropertyId = config.GOOGLE_WEB_PROPERTY_ID;
const profileId = config.GOOGLE_PROFILE_ID;
const scopes = ['https://www.googleapis.com/auth/analytics.readonly'];

const analytics = google.analytics({
version: 'v3'
});

const getAuth = async () => {
const auth = await google.auth.getClient({
scopes
});

return auth;
};

const filterAndMapExperiments = experiments =>
experiments
.filter(({ status }) => status === 'RUNNING')
.map(({ id, variations }) => ({
id,
variations: variations.map(({ name, weight }) => ({
name,
weight
}))
}));

const getExperimentList = async () => {
const auth = await getAuth();
const result = await analytics.management.experiments.list({
accountId,
webPropertyId,
profileId,
auth
});

if (!(result.data && result.data.items)) {
throw new Error('No experiments or invalid data');
}

return filterAndMapExperiments(result.data.items);
};

export const experimentsRoute = async (req) => {
const experiments = await getExperimentList();
return {
data: experiments,
status: 200
}
}
Loading