diff --git a/README.md b/README.md index cb332712401..1fb01f94b7b 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ Each render is passed through a set of HOC's (Higher Order Components) to enhanc - withData - withHashChangeHandler -With a selection of page types passed through withOptimizelyProvider, currently [Article](https://github.com/bbc/simorgh/blob/latest/src/app/pages/ArticlePage/index.jsx) and [Story](https://github.com/bbc/simorgh/blob/latest/src/app/pages/StoryPage/index.jsx) pages. +With a selection of page types passed through withOptimizelyProvider, that enables usage of Optimizely in the selected page types. #### withVariant @@ -97,6 +97,20 @@ The withHashChangeHandler HOC is a wrapper applied to all pages that checks for The withOptimizelyProvider HOC returns components that have been enhanced with access to an Optimizely client, that is used to run our A/B testing. This is done to limit bundle sizes, as we seperate some of our bundles by page type, that means if we're only running A/B testing on certain page types, we can prevent polluting page type bundles with the weight of the SDK library we use for Optimizely. +withOptimizelyProvider should be added as the value of the `handlerBeforeContexts` object key within [applyBasicPageHandlers.js](https://github.com/bbc/simorgh/tree/latest/src/app/pages/utils/applyBasicPageHandlers.js#L8), as the `ckns_mvt` is [set within the UserContext](https://github.com/bbc/simorgh/tree/latest/src/app/contexts/UserContext/index.tsx#L33), so the `withOptimizelyProvider` HOC needs to be applied in the correct order alongside the [withContexts](https://github.com/bbc/simorgh/tree/latest/src/app/pages/utils/applyBasicPageHandlers.js#L13) HOC. This makes the `ckns_mvt` available on first time visits to pass into the `OptimizelyProvider`, along with attributes such as `service`, which is used for determining when Optimizely should enable an experiment. + +Example for Article page: +```jsx +import withOptimizelyProvider from '#app/legacy/containers/PageHandlers/withOptimizelyProvider'; +import ArticlePage from './ArticlePage'; +import applyBasicPageHandlers from '../utils/applyBasicPageHandlers'; + +export default applyBasicPageHandlers(ArticlePage, { + handlerBeforeContexts: withOptimizelyProvider, +}); + +``` + ### Adding a new Page type When adding a new page type there are several parts required. diff --git a/src/app/components/ATIAnalytics/canonical/index.test.tsx b/src/app/components/ATIAnalytics/canonical/index.test.tsx index 0e39adbd742..33b404b057d 100644 --- a/src/app/components/ATIAnalytics/canonical/index.test.tsx +++ b/src/app/components/ATIAnalytics/canonical/index.test.tsx @@ -31,7 +31,7 @@ describe('Canonical ATI Analytics', () => { }); it('should not send beacon when browser is Opera Mini', () => { - jest.spyOn(isOperaProxy, 'default').mockImplementationOnce(() => true); + jest.spyOn(isOperaProxy, 'default').mockImplementation(() => true); act(() => { render(); diff --git a/src/app/contexts/UserContext/index.test.tsx b/src/app/contexts/UserContext/index.test.tsx index 7f99b9018a3..50adc967449 100644 --- a/src/app/contexts/UserContext/index.test.tsx +++ b/src/app/contexts/UserContext/index.test.tsx @@ -1,5 +1,8 @@ import React, { useContext } from 'react'; import { render } from '@testing-library/react'; +import Cookie from 'js-cookie'; +import * as onClient from '#app/lib/utilities/onClient'; +import * as isOperaProxy from '#app/lib/utilities/isOperaProxy'; import { UserContext, UserContextProvider } from '.'; import { getCookiePolicy, personalisationEnabled } from './cookies'; import * as chartbeat from './Chartbeat'; @@ -61,4 +64,73 @@ describe('UserContext', () => { {}, ); }); + describe('ckns_mvt cookie', () => { + const cookieSetterSpy = jest.spyOn(Cookie, 'set'); + const cookieGetterSpy = jest.spyOn(Cookie, 'get'); + const isOperaProxySpy = jest.spyOn(isOperaProxy, 'default'); + const onClientSpy = jest.spyOn(onClient, 'default'); + + beforeEach(() => { + jest.clearAllMocks(); + Cookie.remove('ckns_mvt'); + }); + + it('should call cookie logic when not opera mini and is on client', () => { + onClientSpy.mockImplementationOnce(() => true as unknown as Location); + isOperaProxySpy.mockImplementationOnce(() => false); + + render(); + + expect(cookieGetterSpy).toHaveBeenCalled(); + }); + + it('should not call cookie logic when on opera mini and is on client', () => { + onClientSpy.mockImplementationOnce(() => true as unknown as Location); + isOperaProxySpy.mockImplementationOnce(() => true); + + render(); + + expect(cookieGetterSpy).not.toHaveBeenCalled(); + }); + + it('should not call cookie logic when not on opera mini and is not on client', () => { + onClientSpy.mockImplementationOnce(() => false); + isOperaProxySpy.mockImplementationOnce(() => false); + + render(); + + expect(cookieGetterSpy).not.toHaveBeenCalled(); + }); + + it('should not set cookie when ckns_mvt cookie already exists', () => { + onClientSpy.mockImplementationOnce(() => true as unknown as Location); + isOperaProxySpy.mockImplementationOnce(() => false); + Cookie.set('ckns_mvt', 'foo'); + cookieSetterSpy.mockClear(); + + render(); + + expect(cookieGetterSpy).toHaveReturnedWith('foo'); + expect(cookieSetterSpy).not.toHaveBeenCalled(); + }); + + it('should set cookie when no ckns_mvt cookie exists', () => { + const uuidRegex = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-5][0-9a-f]{3}-[089ab][0-9a-f]{3}-[0-9a-f]{12}$/i; + onClientSpy.mockImplementationOnce(() => true as unknown as Location); + isOperaProxySpy.mockImplementationOnce(() => false); + // @ts-expect-error This should be able to be mocked as a string or undefined + cookieGetterSpy.mockImplementationOnce(() => undefined); + + render(); + + const [[cookieName, cookieValue, cookieOptions]] = + cookieSetterSpy.mock.calls; + + expect(cookieValue).toMatch(uuidRegex); + expect(cookieName).toEqual('ckns_mvt'); + expect(cookieOptions).toEqual({ expires: 365, path: '/', secure: true }); + expect(cookieSetterSpy).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/src/app/contexts/UserContext/index.tsx b/src/app/contexts/UserContext/index.tsx index 7febc7431b0..fbe7ee61260 100644 --- a/src/app/contexts/UserContext/index.tsx +++ b/src/app/contexts/UserContext/index.tsx @@ -5,6 +5,10 @@ import React, { SetStateAction, useMemo, } from 'react'; +import { v4 as uuid } from 'uuid'; +import Cookie from 'js-cookie'; +import onClient from '#app/lib/utilities/onClient'; +import isOperaProxy from '#app/lib/utilities/isOperaProxy'; import { getCookiePolicy, personalisationEnabled } from './cookies'; import Chartbeat from './Chartbeat'; @@ -19,10 +23,25 @@ export const UserContext = React.createContext( {} as UserContextProps, ); +const cknsMvtCookie = () => { + const cookieName = 'ckns_mvt'; + const cookieValue = Cookie.get(cookieName); + + if (!cookieValue) { + const cookieUuid = uuid(); + const expires = 365; + Cookie.set(cookieName, cookieUuid, { expires, path: '/', secure: true }); + } +}; + export const UserContextProvider = ({ children }: PropsWithChildren) => { const [cookiePolicy, setCookiePolicy] = useState(getCookiePolicy()); const [chartbeatConfig, sendCanonicalChartbeatBeacon] = useState(null); + if (onClient() && !isOperaProxy()) { + cknsMvtCookie(); + } + const value = useMemo( () => ({ cookiePolicy, diff --git a/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/getOptimizelyUserId/index.js b/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/getOptimizelyUserId/index.js deleted file mode 100644 index 8dbd0809c1f..00000000000 --- a/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/getOptimizelyUserId/index.js +++ /dev/null @@ -1,24 +0,0 @@ -import onClient from '#lib/utilities/onClient'; -import isOperaProxy from '#lib/utilities/isOperaProxy'; -import { v4 as uuid } from 'uuid'; -import Cookie from 'js-cookie'; - -const getOptimizelyUserId = () => { - // Users accessing the site on opera "extreme data saving mode" have the pages rendered by an intermediate service - // Attempting to track these users is just tracking that proxy, causing all opera mini visitors to have the same id - if (!onClient() || isOperaProxy()) return null; - - const cookieName = 'ckns_mvt'; - const cookieValue = Cookie.get(cookieName); - const expires = 365; // expires in 12 Months - - if (!cookieValue) { - const cookieUuid = uuid(); - Cookie.set(cookieName, cookieUuid, { expires, path: '/', secure: true }); - return cookieUuid; - } - - return cookieValue; -}; - -export default getOptimizelyUserId; diff --git a/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/getOptimizelyUserId/index.test.js b/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/getOptimizelyUserId/index.test.js deleted file mode 100644 index 5d4e499c883..00000000000 --- a/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/getOptimizelyUserId/index.test.js +++ /dev/null @@ -1,33 +0,0 @@ -import Cookie from 'js-cookie'; -import getOptimizelyUserId from './index'; - -describe('getOptimizelyUserId', () => { - const cookieSetterSpy = jest.spyOn(Cookie, 'set'); - - afterEach(() => { - jest.clearAllMocks(); - Cookie.remove('ckns_mvt'); - }); - - it('should return the optimizely user id if the cookie exists', () => { - Cookie.set('ckns_mvt', 'random-uuid'); - - const optimizelyUserId = getOptimizelyUserId(); - - expect(optimizelyUserId).toEqual('random-uuid'); - }); - - it('should create and return optimizely user id if it does not exist', () => { - const uuidRegex = - /^[0-9a-f]{8}-[0-9a-f]{4}-[0-5][0-9a-f]{3}-[089ab][0-9a-f]{3}-[0-9a-f]{12}$/i; - const optimizelyUserId = getOptimizelyUserId(); - const [[cookieName, cookieValue, cookieOptions]] = - cookieSetterSpy.mock.calls; - - expect(optimizelyUserId).toMatch(uuidRegex); - expect(cookieName).toEqual('ckns_mvt'); - expect(cookieValue).toEqual(optimizelyUserId); - expect(cookieOptions).toEqual({ expires: 365, path: '/', secure: true }); - expect(cookieSetterSpy).toHaveBeenCalledTimes(1); - }); -}); diff --git a/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/index.jsx b/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/index.jsx index fc7c142e29b..c7ae3f7d17f 100644 --- a/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/index.jsx +++ b/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/index.jsx @@ -8,8 +8,9 @@ import isLive from '#lib/utilities/isLive'; import onClient from '#lib/utilities/onClient'; import { GEL_GROUP_3_SCREEN_WIDTH_MAX } from '#psammead/gel-foundations/src/breakpoints'; import { getEnvConfig } from '#app/lib/utilities/getEnvConfig'; +import Cookie from 'js-cookie'; +import isOperaProxy from '#app/lib/utilities/isOperaProxy'; import { ServiceContext } from '../../../../contexts/ServiceContext'; -import getOptimizelyUserId from './getOptimizelyUserId'; // 004_brasil_recommendations_experiment const isCypress = onClient() && window.Cypress; @@ -32,10 +33,10 @@ const withOptimizelyProvider = Component => { let mobile; const getUserId = () => { - if (disableOptimizely) { + if (disableOptimizely || !onClient() || isOperaProxy()) { return null; } - return getOptimizelyUserId(); + return Cookie.get('ckns_mvt') ?? null; }; if (onClient()) { diff --git a/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/index.test.jsx b/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/index.test.jsx index 5b0f45b5d92..e0d4a640f52 100644 --- a/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/index.test.jsx +++ b/src/app/legacy/containers/PageHandlers/withOptimizelyProvider/index.test.jsx @@ -1,15 +1,11 @@ import React, { useMemo } from 'react'; import * as optimizelyReactSdk from '@optimizely/react-sdk'; import { render } from '@testing-library/react'; +import Cookie from 'js-cookie'; import latin from '../../../../components/ThemeProvider/fontScripts/latin'; import { ServiceContext } from '../../../../contexts/ServiceContext'; import withOptimizelyProvider from '.'; -const optimizelyProviderSpy = jest.spyOn( - optimizelyReactSdk.OptimizelyProvider.prototype, - 'render', -); - const props = { bbcOrigin: 'https://www.bbc.com', id: 'c0000000000o', @@ -44,8 +40,32 @@ const TestComponent = () => { describe('withOptimizelyProvider HOC', () => { it('should enrich the component with the Optimizely API', () => { + const optimizelyProviderRenderSpy = jest.spyOn( + optimizelyReactSdk.OptimizelyProvider.prototype, + 'render', + ); + + render(); + + expect(optimizelyProviderRenderSpy).toHaveBeenCalledTimes(1); + }); + + it('should return undefined when ckns_mvt is fetched with Cookie.get', () => { + const cookieGetterSpy = jest.spyOn(Cookie, 'get'); + + render(); + + expect(cookieGetterSpy).toHaveBeenCalledWith('ckns_mvt'); + expect(cookieGetterSpy).toHaveReturnedWith(undefined); + }); + + it('should return the correct ckns_mvt cookie value from Cookie.get', () => { + const cookieGetterSpy = jest.spyOn(Cookie, 'get'); + Cookie.set('ckns_mvt', 'random-uuid'); + render(); - expect(optimizelyProviderSpy).toHaveBeenCalledTimes(1); + expect(cookieGetterSpy).toHaveBeenCalledWith('ckns_mvt'); + expect(cookieGetterSpy).toHaveReturnedWith('random-uuid'); }); }); diff --git a/src/app/pages/utils/applyBasicPageHandlers.js b/src/app/pages/utils/applyBasicPageHandlers.js index 63a9075195c..c9a1f81bf67 100644 --- a/src/app/pages/utils/applyBasicPageHandlers.js +++ b/src/app/pages/utils/applyBasicPageHandlers.js @@ -5,11 +5,15 @@ import withError from '#containers/PageHandlers/withError'; import withData from '#containers/PageHandlers/withData'; import withHashChangeHandler from '#containers/PageHandlers/withHashChangeHandler'; -export default component => +export default ( + component, + { handlerBeforeContexts = Component => Component } = {}, +) => pipe( withData, withError, withPageWrapper, + handlerBeforeContexts, withContexts, withHashChangeHandler, )(component); diff --git a/src/app/pages/utils/applyBasicPageHandlers.test.js b/src/app/pages/utils/applyBasicPageHandlers.test.js new file mode 100644 index 00000000000..2c2d1210aae --- /dev/null +++ b/src/app/pages/utils/applyBasicPageHandlers.test.js @@ -0,0 +1,38 @@ +import * as pipe from 'ramda/src/pipe'; +import WithContexts from '#app/legacy/containers/PageHandlers/withContexts'; +import applyBasicPageHandlers from './applyBasicPageHandlers'; + +jest.mock('ramda/src/pipe', () => { + const originalModule = jest.requireActual('ramda/src/pipe'); + + return { + __esModule: true, + ...originalModule, + default: originalModule, + }; +}); + +describe('applyBasicPageHandlers', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.resetModules(); + }); + + it('should call pipe with function as the argument before the withContexts argument when passed via handlerBeforeContexts', () => { + const component = jest.fn(); + const mockBeforeContextsFunction = jest.fn(); + const pipeMock = jest.spyOn(pipe, 'default'); + + applyBasicPageHandlers(component, { + handlerBeforeContexts: mockBeforeContextsFunction, + }); + + const args = pipeMock.mock.calls[0]; + const beforeContextsFunctionArg = args[3]; + const WithContextsFunctionArg = args[4]; + + expect(beforeContextsFunctionArg).toEqual(mockBeforeContextsFunction); + expect(WithContextsFunctionArg).toEqual(WithContexts); + expect(mockBeforeContextsFunction).toHaveBeenCalledTimes(1); + }); +});