From 9c113a3eb1818337b65a9998a640bc4cf85b9bbe Mon Sep 17 00:00:00 2001 From: Dominik Lander Date: Wed, 22 May 2024 10:18:38 +0100 Subject: [PATCH] Revert "Revert 'Add deeply read right ab test'" This reverts commit cb69e63604155a83b60a4e318adf282ce1e5f116. --- .../playwright/tests/article.e2e.spec.ts | 10 +- .../tests/article.interactivity.e2e.spec.ts | 6 +- .../src/components/Island.test.tsx | 28 ++++-- .../components/MostViewedRight.stories.tsx | 8 +- .../src/components/MostViewedRight.test.tsx | 4 +- .../src/components/MostViewedRight.tsx | 91 ++++++++++++++----- ...x => MostViewedRightWithAd.importable.tsx} | 57 ++++++++---- ...ortable.tsx => MostViewedRightWrapper.tsx} | 13 --- dotcom-rendering/src/experiments/ab-tests.ts | 2 + .../tests/deeply-read-right-column.ts | 35 +++++++ .../src/layouts/CommentLayout.tsx | 35 ++++--- .../src/layouts/ShowcaseLayout.tsx | 35 ++++--- .../src/layouts/StandardLayout.tsx | 35 ++++--- .../src/lib/useDeeplyReadTestVariant.tsx | 31 +++++++ 14 files changed, 277 insertions(+), 113 deletions(-) rename dotcom-rendering/src/components/{MostViewedRightWithAd.tsx => MostViewedRightWithAd.importable.tsx} (62%) rename dotcom-rendering/src/components/{MostViewedRightWrapper.importable.tsx => MostViewedRightWrapper.tsx} (78%) create mode 100644 dotcom-rendering/src/experiments/tests/deeply-read-right-column.ts create mode 100644 dotcom-rendering/src/lib/useDeeplyReadTestVariant.tsx diff --git a/dotcom-rendering/playwright/tests/article.e2e.spec.ts b/dotcom-rendering/playwright/tests/article.e2e.spec.ts index ab4fa14679d..a2ef500c8db 100644 --- a/dotcom-rendering/playwright/tests/article.e2e.spec.ts +++ b/dotcom-rendering/playwright/tests/article.e2e.spec.ts @@ -34,13 +34,13 @@ test.describe('E2E Page rendering', () => { ); // most viewed right hand column, response promise - // https://api.nextgen.guardianapps.co.uk/most-read-geo.json?dcr=true + // https://api.nextgen.guardianapps.co.uk/most-read-with-deeply-read.json const mostReadRightResponsePromise = page.waitForResponse( (response) => responseHasJsonProperty( response, - /most-read-geo\.json/, - 'heading', + /most-read-with-deeply-read\.json/, + 'tabs', ), ); @@ -77,11 +77,11 @@ test.describe('E2E Page rendering', () => { ).toBeVisible(); // expect most read right to be loaded, its data response and its text to be visible - await waitForIsland(page, 'MostViewedRightWrapper', {}); + await waitForIsland(page, 'MostViewedRightWithAd', {}); await mostReadRightResponsePromise; await expect( page - .locator(`gu-island[name="MostViewedRightWrapper"]`) + .locator(`gu-island[name="MostViewedRightWithAd"]`) .getByText('Most Viewed'), ).toBeVisible(); diff --git a/dotcom-rendering/playwright/tests/article.interactivity.e2e.spec.ts b/dotcom-rendering/playwright/tests/article.interactivity.e2e.spec.ts index 13755d9f2ab..ff013990b5c 100644 --- a/dotcom-rendering/playwright/tests/article.interactivity.e2e.spec.ts +++ b/dotcom-rendering/playwright/tests/article.interactivity.e2e.spec.ts @@ -87,7 +87,7 @@ test.describe('Interactivity', () => { await disableCMP(context); await loadPage(page, `/Article/${articleUrl}`); await expectToNotExist(page, '[data-component=geo-most-popular]'); - await waitForIsland(page, 'MostViewedRightWrapper', {}); + await waitForIsland(page, 'MostViewedRightWithAd', {}); await expectToExist(page, '[data-component=geo-most-popular]'); }); @@ -141,10 +141,10 @@ test.describe('Interactivity', () => { ).toHaveCount(0); // Wait for hydration - await waitForIsland(page, 'MostViewedRightWrapper', {}); + await waitForIsland(page, 'MostViewedRightWithAd', {}); await expect( page - .locator(`gu-island[name="MostViewedRightWrapper"]`) + .locator(`gu-island[name="MostViewedRightWithAd"]`) .filter({ hasText: 'Most Viewed' }), ).toBeVisible(); diff --git a/dotcom-rendering/src/components/Island.test.tsx b/dotcom-rendering/src/components/Island.test.tsx index d6fc2d4134d..3da41c19e22 100644 --- a/dotcom-rendering/src/components/Island.test.tsx +++ b/dotcom-rendering/src/components/Island.test.tsx @@ -23,7 +23,7 @@ import { LiveBlogEpic } from './LiveBlogEpic.importable'; import { Liveness } from './Liveness.importable'; import { Metrics } from './Metrics.importable'; import { MostViewedFooterData } from './MostViewedFooterData.importable'; -import { MostViewedRightWrapper } from './MostViewedRightWrapper.importable'; +import { MostViewedRightWithAd } from './MostViewedRightWithAd.importable'; import { OnwardsUpper } from './OnwardsUpper.importable'; import { ReaderRevenueDev } from './ReaderRevenueDev.importable'; import { ReaderRevenueLinks } from './ReaderRevenueLinks.importable'; @@ -321,14 +321,28 @@ describe('Island: server-side rendering', () => { ).not.toThrow(); }); - test('MostViewedRightWrapper', () => { + test('MostViewedRightWithAd', () => { expect(() => renderToString( - , + + + , + , ), ).not.toThrow(); }); diff --git a/dotcom-rendering/src/components/MostViewedRight.stories.tsx b/dotcom-rendering/src/components/MostViewedRight.stories.tsx index 53b2e43e6dc..c0cf0893c0e 100644 --- a/dotcom-rendering/src/components/MostViewedRight.stories.tsx +++ b/dotcom-rendering/src/components/MostViewedRight.stories.tsx @@ -8,7 +8,7 @@ import { import { ArticleContainer } from './ArticleContainer'; import { Flex } from './Flex'; import { LeftColumn } from './LeftColumn'; -import { mockTab1 } from './MostViewed.mocks'; +import { responseWithTwoTabs } from './MostViewed.mocks'; import { MostViewedRight } from './MostViewedRight'; import { RightColumn } from './RightColumn'; import { Section } from './Section'; @@ -33,7 +33,7 @@ export const defaultStory: StoryObj = ({ format }: StoryProps) => { .restore() .getOnce('*', { status: 200, - body: mockTab1, + body: responseWithTwoTabs, }) .spy('end:.hot-update.json'); @@ -67,7 +67,7 @@ export const limitItemsStory: StoryObj = ({ format }: StoryProps) => { .restore() .getOnce('*', { status: 200, - body: mockTab1, + body: responseWithTwoTabs, }) .spy('end:.hot-update.json'); @@ -101,7 +101,7 @@ export const outsideContextStory: StoryObj = () => { .restore() .getOnce('*', { status: 200, - body: mockTab1, + body: responseWithTwoTabs, }) .spy('end:.hot-update.json'); diff --git a/dotcom-rendering/src/components/MostViewedRight.test.tsx b/dotcom-rendering/src/components/MostViewedRight.test.tsx index 26c3aaa9651..3741f963a68 100644 --- a/dotcom-rendering/src/components/MostViewedRight.test.tsx +++ b/dotcom-rendering/src/components/MostViewedRight.test.tsx @@ -1,10 +1,10 @@ import { render } from '@testing-library/react'; import { useApi as useApi_ } from '../lib/useApi'; import { ConfigProvider } from './ConfigContext'; -import { mockTab1 } from './MostViewed.mocks'; +import { responseWithTwoTabs } from './MostViewed.mocks'; import { MostViewedRight } from './MostViewedRight'; -const response = { data: mockTab1 }; +const response = { data: responseWithTwoTabs }; const useApi: { [key: string]: any } = useApi_; jest.mock('../lib/useApi', () => ({ diff --git a/dotcom-rendering/src/components/MostViewedRight.tsx b/dotcom-rendering/src/components/MostViewedRight.tsx index 460e19144dc..7ef24a77b07 100755 --- a/dotcom-rendering/src/components/MostViewedRight.tsx +++ b/dotcom-rendering/src/components/MostViewedRight.tsx @@ -3,8 +3,9 @@ import { headlineBold17 } from '@guardian/source/foundations'; import { StraightLines } from '@guardian/source-development-kitchen/react-components'; import { decideTrail } from '../lib/decideTrail'; import { useApi } from '../lib/useApi'; +import { useDeeplyReadTestVariant } from '../lib/useDeeplyReadTestVariant'; import { palette as themePalette } from '../palette'; -import type { FETrailTabType, TrailType } from '../types/trails'; +import type { FETrailTabType } from '../types/trails'; import { MostViewedRightItem } from './MostViewedRightItem'; const wrapperStyles = css` @@ -29,47 +30,89 @@ interface Props { stickToTop?: boolean; } +interface MostViewedRightPayloadType { + tabs: FETrailTabType[]; +} + export const MostViewedRight = ({ limitItems = 5, stickToTop = false, }: Props) => { + const testVariant = useDeeplyReadTestVariant(); const endpointUrl = - 'https://api.nextgen.guardianapps.co.uk/most-read-geo.json?dcr=true'; - const { data, error } = useApi(endpointUrl); + 'https://api.nextgen.guardianapps.co.uk/most-read-with-deeply-read.json'; + const { data, error } = useApi(endpointUrl); if (error) { - window.guardian.modules.sentry.reportError(error, 'most-viewed-right'); + window.guardian.modules.sentry.reportError( + error, + 'most-viewed-deeply-read-right', + ); return null; } if (data) { - const trails: TrailType[] = data.trails - .slice(0, limitItems) - .map(decideTrail); + const mostReadTrails = data.tabs[0]?.trails + ?.slice(0, limitItems) + ?.map(decideTrail); + const deeplyReadTrails = + testVariant === 'deeply-read-and-most-viewed' || + testVariant === 'deeply-read-only' + ? data.tabs[1]?.trails?.slice(0, limitItems)?.map(decideTrail) + : undefined; - // Look I don't know why data-component is geo-most-popular either, but it is, ok? Ok. return (
- -

Most viewed

-
    - {trails.map((trail, index) => ( - + +

    Most viewed

    +
      + {mostReadTrails?.map((trail, index) => ( + + ))} +
    + + )} + {(testVariant === 'deeply-read-only' || + testVariant === 'deeply-read-and-most-viewed') && ( + <> + - ))} -
+

Deeply read

+
    + {deeplyReadTrails?.map((trail, index) => ( + + ))} +
+ + )}
); } diff --git a/dotcom-rendering/src/components/MostViewedRightWithAd.tsx b/dotcom-rendering/src/components/MostViewedRightWithAd.importable.tsx similarity index 62% rename from dotcom-rendering/src/components/MostViewedRightWithAd.tsx rename to dotcom-rendering/src/components/MostViewedRightWithAd.importable.tsx index b21bed3232c..f0c92a1ef82 100644 --- a/dotcom-rendering/src/components/MostViewedRightWithAd.tsx +++ b/dotcom-rendering/src/components/MostViewedRightWithAd.importable.tsx @@ -1,10 +1,10 @@ import { css } from '@emotion/react'; import { ArticleDesign, ArticleSpecial } from '@guardian/libs'; +import { useDeeplyReadTestVariant } from '../lib/useDeeplyReadTestVariant'; import { RightAdsPlaceholder } from './AdPlaceholder.apps'; import { AdSlot } from './AdSlot.web'; import { useConfig } from './ConfigContext'; -import { Island } from './Island'; -import { MostViewedRightWrapper } from './MostViewedRightWrapper.importable'; +import { MostViewedRightWrapper } from './MostViewedRightWrapper'; type Props = { format: ArticleFormat; @@ -19,6 +19,26 @@ type Props = { */ const MAX_HEIGHT_PX = 1600; +/** + * When in the deeply-read-and-most-viewed variant of the deeplyReadRightColumn + * AB test, there is an extra container of five article links. + */ +const MAX_HEIGHT_PX_DEEPLY_READ = 2250; + +/** + * Wrapping `MostViewedRight` so we can determine whether or not + * there's enough vertical space in the container to render it. + * + * ## Why does this need to be an Island? + * + * We may show the most viewed component depending on the length of the article, + * based on the computed height of the container and the height of this component is + * changed dynamically. + * + * --- + * + * (No visual story exists) + */ export const MostViewedRightWithAd = ({ format, isPaidContent, @@ -33,6 +53,10 @@ export const MostViewedRightWithAd = ({ format.design === ArticleDesign.Audio) && format.theme !== ArticleSpecial.Labs; + const deeplyReadTestVariant = useDeeplyReadTestVariant(); + const deeplyReadAndMostViewed = + deeplyReadTestVariant === 'deeply-read-and-most-viewed'; + return (
- - + ) : null} {isApps && } diff --git a/dotcom-rendering/src/components/MostViewedRightWrapper.importable.tsx b/dotcom-rendering/src/components/MostViewedRightWrapper.tsx similarity index 78% rename from dotcom-rendering/src/components/MostViewedRightWrapper.importable.tsx rename to dotcom-rendering/src/components/MostViewedRightWrapper.tsx index 2927f1b900c..e0e3ac31762 100644 --- a/dotcom-rendering/src/components/MostViewedRightWrapper.importable.tsx +++ b/dotcom-rendering/src/components/MostViewedRightWrapper.tsx @@ -10,19 +10,6 @@ type Props = { renderAds: boolean; }; -/** - * Wrapping `MostViewedRight` so we can determine whether or not - * there's enough vertical space in the container to render it. - * - * ## Why does this need to be an Island? - * - * We may show the most viewed component depending on the length of the article, - * based on the computed height of the container. - * - * --- - * - * (No visual story exists) - */ export const MostViewedRightWrapper = ({ componentDataAttribute, maxHeightPx, diff --git a/dotcom-rendering/src/experiments/ab-tests.ts b/dotcom-rendering/src/experiments/ab-tests.ts index 9430fa84e27..2d239159965 100644 --- a/dotcom-rendering/src/experiments/ab-tests.ts +++ b/dotcom-rendering/src/experiments/ab-tests.ts @@ -2,6 +2,7 @@ import type { ABTest } from '@guardian/ab-core'; import { abTestTest } from './tests/ab-test-test'; import { adBlockAsk } from './tests/ad-block-ask'; import { consentlessAds } from './tests/consentless-ads'; +import { deeplyReadRightColumn } from './tests/deeply-read-right-column'; import { integrateIma } from './tests/integrate-ima'; import { mpuWhenNoEpic } from './tests/mpu-when-no-epic'; import { signInGateAlternativeWording } from './tests/sign-in-gate-alternative-wording'; @@ -19,4 +20,5 @@ export const tests: ABTest[] = [ integrateIma, mpuWhenNoEpic, adBlockAsk, + deeplyReadRightColumn, ]; diff --git a/dotcom-rendering/src/experiments/tests/deeply-read-right-column.ts b/dotcom-rendering/src/experiments/tests/deeply-read-right-column.ts new file mode 100644 index 00000000000..564c673807e --- /dev/null +++ b/dotcom-rendering/src/experiments/tests/deeply-read-right-column.ts @@ -0,0 +1,35 @@ +import type { ABTest } from '@guardian/ab-core'; + +export const deeplyReadRightColumn: ABTest = { + id: 'DeeplyReadRightColumn', + author: '@dotcom-platform', + start: '2024-04-30', + expiry: '2024-07-31', + audience: 0 / 100, + audienceOffset: 0 / 100, + audienceCriteria: '', + successMeasure: 'Improved click though rate', + description: + 'Test the impact of adding deeply read component to the right column.', + variants: [ + { + id: 'most-viewed-only', + test: (): void => { + /* no-op */ + }, + }, + { + id: 'deeply-read-and-most-viewed', + test: (): void => { + /* no-op */ + }, + }, + { + id: 'deeply-read-only', + test: (): void => { + /* no-op */ + }, + }, + ], + canRun: () => true, +}; diff --git a/dotcom-rendering/src/layouts/CommentLayout.tsx b/dotcom-rendering/src/layouts/CommentLayout.tsx index 651a8e29b85..482df1adadd 100644 --- a/dotcom-rendering/src/layouts/CommentLayout.tsx +++ b/dotcom-rendering/src/layouts/CommentLayout.tsx @@ -31,7 +31,7 @@ import { MainMedia } from '../components/MainMedia'; import { Masthead } from '../components/Masthead'; import { MostViewedFooterData } from '../components/MostViewedFooterData.importable'; import { MostViewedFooterLayout } from '../components/MostViewedFooterLayout'; -import { MostViewedRightWithAd } from '../components/MostViewedRightWithAd'; +import { MostViewedRightWithAd } from '../components/MostViewedRightWithAd.importable'; import { Nav } from '../components/Nav/Nav'; import { OnwardsUpper } from '../components/OnwardsUpper.importable'; import { RightColumn } from '../components/RightColumn'; @@ -799,17 +799,28 @@ export const CommentLayout = (props: WebProps | AppsProps) => { `} > - + + +
diff --git a/dotcom-rendering/src/layouts/ShowcaseLayout.tsx b/dotcom-rendering/src/layouts/ShowcaseLayout.tsx index 42ac2ac1cb7..cbba33df1bb 100644 --- a/dotcom-rendering/src/layouts/ShowcaseLayout.tsx +++ b/dotcom-rendering/src/layouts/ShowcaseLayout.tsx @@ -33,7 +33,7 @@ import { MainMedia } from '../components/MainMedia'; import { Masthead } from '../components/Masthead'; import { MostViewedFooterData } from '../components/MostViewedFooterData.importable'; import { MostViewedFooterLayout } from '../components/MostViewedFooterLayout'; -import { MostViewedRightWithAd } from '../components/MostViewedRightWithAd'; +import { MostViewedRightWithAd } from '../components/MostViewedRightWithAd.importable'; import { Nav } from '../components/Nav/Nav'; import { OnwardsUpper } from '../components/OnwardsUpper.importable'; import { RightColumn } from '../components/RightColumn'; @@ -820,17 +820,28 @@ export const ShowcaseLayout = (props: WebProps | AppsProps) => { `} > - + + + diff --git a/dotcom-rendering/src/layouts/StandardLayout.tsx b/dotcom-rendering/src/layouts/StandardLayout.tsx index d10007904c1..932615eab7a 100644 --- a/dotcom-rendering/src/layouts/StandardLayout.tsx +++ b/dotcom-rendering/src/layouts/StandardLayout.tsx @@ -38,7 +38,7 @@ import { MainMedia } from '../components/MainMedia'; import { Masthead } from '../components/Masthead'; import { MostViewedFooterData } from '../components/MostViewedFooterData.importable'; import { MostViewedFooterLayout } from '../components/MostViewedFooterLayout'; -import { MostViewedRightWithAd } from '../components/MostViewedRightWithAd'; +import { MostViewedRightWithAd } from '../components/MostViewedRightWithAd.importable'; import { Nav } from '../components/Nav/Nav'; import { OnwardsUpper } from '../components/OnwardsUpper.importable'; import { RightColumn } from '../components/RightColumn'; @@ -964,17 +964,28 @@ export const StandardLayout = (props: WebProps | AppProps) => { `} > - + + + diff --git a/dotcom-rendering/src/lib/useDeeplyReadTestVariant.tsx b/dotcom-rendering/src/lib/useDeeplyReadTestVariant.tsx new file mode 100644 index 00000000000..cb24cc5c143 --- /dev/null +++ b/dotcom-rendering/src/lib/useDeeplyReadTestVariant.tsx @@ -0,0 +1,31 @@ +import { isOneOf } from '@guardian/libs'; +import { useEffect, useState } from 'react'; +import { deeplyReadRightColumn } from '../experiments/tests/deeply-read-right-column'; +import { useAB } from './useAB'; + +const variants = [ + 'deeply-read-only', + 'deeply-read-and-most-viewed', + 'most-viewed-only', + 'none', +] as const; +type Variant = (typeof variants)[number]; + +const isVariant = isOneOf(variants); + +export const useDeeplyReadTestVariant = (): Variant => { + const [variant, setVariant] = useState('none'); + const ABTestAPI = useAB()?.api; + + useEffect(() => { + const variantId = + ABTestAPI?.runnableTest(deeplyReadRightColumn)?.variantToRun.id ?? + 'none'; + + if (isVariant(variantId)) { + setVariant(variantId); + } + }, [ABTestAPI]); + + return variant; +};