Revert "Revert 'Add deeply read right ab test'"

This reverts commit cb69e63.
guardian · Jun 10, 2024 · 9c113a3 · 9c113a3
1 parent ce71b78
commit 9c113a3
Show file tree

Hide file tree

Showing 14 changed files with 277 additions and 113 deletions.
diff --git 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
@@ -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
@@ -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(
-				<MostViewedRightWrapper
-					componentDataAttribute={''}
-					maxHeightPx={0}
-					renderAds={false}
-				/>,
+				<ConfigProvider
+					value={{
+						renderingTarget: 'Web',
+						darkModeAvailable: false,
+						assetOrigin: '/',
+					}}
+				>
+					<MostViewedRightWithAd
+						format={{
+							theme: Pillar.News,
+							design: ArticleDesign.Standard,
+							display: ArticleDisplay.Standard,
+						}}
+						isPaidContent={false}
+						renderAds={false}
+						shouldHideReaderRevenue={false}
+					/>
+					,
+				</ConfigProvider>,
 			),
 		).not.toThrow();
 	});

diff --git 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
@@ -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
@@ -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<FETrailTabType>(endpointUrl);
+		'https://api.nextgen.guardianapps.co.uk/most-read-with-deeply-read.json';
+	const { data, error } = useApi<MostViewedRightPayloadType>(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 (
 			<div
 				css={[wrapperStyles, stickToTop && stickyStyles]}
 				data-component="geo-most-popular"
 			>
-				<StraightLines
-					cssOverrides={css`
-						display: block;
-					`}
-					count={4}
-					color={themePalette('--straight-lines')}
-				/>
-				<h3 css={headingStyles}>Most viewed</h3>
-				<ul data-link-name="Right hand most popular geo GB">
-					{trails.map((trail, index) => (
-						<MostViewedRightItem
-							key={trail.url}
-							trail={trail}
-							mostViewedItemIndex={index}
+				{testVariant !== 'deeply-read-only' && (
+					<>
+						<StraightLines
+							cssOverrides={css`
+								display: block;
+							`}
+							count={4}
+							color={themePalette('--straight-lines')}
+						/>
+						<h3 css={headingStyles}>Most viewed</h3>
+						<ul data-link-name="Right hand most popular geo GB">
+							{mostReadTrails?.map((trail, index) => (
+								<MostViewedRightItem
+									key={trail.url}
+									trail={trail}
+									mostViewedItemIndex={index}
+								/>
+							))}
+						</ul>
+					</>
+				)}
+				{(testVariant === 'deeply-read-only' ||
+					testVariant === 'deeply-read-and-most-viewed') && (
+					<>
+						<StraightLines
+							cssOverrides={css`
+								display: block;
+								margin-top: ${testVariant ===
+								'deeply-read-and-most-viewed'
+									? '30px'
+									: '0'};
+							`}
+							count={4}
+							color={themePalette('--straight-lines')}
 						/>
-					))}
-				</ul>
+						<h3 css={headingStyles}>Deeply read</h3>
+						<ul data-link-name="Right hand deeply read">
+							{deeplyReadTrails?.map((trail, index) => (
+								<MostViewedRightItem
+									key={trail.url}
+									trail={trail}
+									mostViewedItemIndex={index}
+								/>
+							))}
+						</ul>
+					</>
+				)}
 			</div>
 		);
 	}

diff --git a/.../src/components/MostViewedRightWithAd.tsx → ...ents/MostViewedRightWithAd.importable.tsx b/.../src/components/MostViewedRightWithAd.tsx → ...ents/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 (
 		<div
 			// This attribute is necessary so that most viewed wrapper
@@ -43,7 +67,13 @@ export const MostViewedRightWithAd = ({
 				 * On Web - we restrict the height to the maximum height, so that the top right ad can be sticky until the
 				 *          most viewed component is in view at MAX_HEIGHT_PX, or 100% of the article height if it is a short article
 				*/
-				height: ${isApps ? '100%' : `min(100%, ${MAX_HEIGHT_PX}px)`};
+				height: ${isApps
+					? '100%'
+					: `min(100%, ${
+							deeplyReadAndMostViewed
+								? MAX_HEIGHT_PX_DEEPLY_READ
+								: MAX_HEIGHT_PX
+					  }px)`};
 				display: flex;
 				flex-direction: column;
 			`}
@@ -59,22 +89,11 @@ export const MostViewedRightWithAd = ({
 			) : null}
 
 			{!isPaidContent ? (
-				<Island
-					priority="feature"
-					defer={{
-						until: 'visible',
-						// Provide a much higher value for the top margin for the intersection observer
-						// This is because the most viewed would otherwise only be lazy loaded when the
-						// bottom of the container intersects with the viewport
-						rootMargin: '700px 100px',
-					}}
-				>
-					<MostViewedRightWrapper
-						maxHeightPx={MAX_HEIGHT_PX}
-						componentDataAttribute={componentDataAttribute}
-						renderAds={renderAds}
-					/>
-				</Island>
+				<MostViewedRightWrapper
+					maxHeightPx={MAX_HEIGHT_PX}
+					componentDataAttribute={componentDataAttribute}
+					renderAds={renderAds}
+				/>
 			) : null}
 
 			{isApps && <RightAdsPlaceholder />}

diff --git a/...nts/MostViewedRightWrapper.importable.tsx → ...src/components/MostViewedRightWrapper.tsx b/...nts/MostViewedRightWrapper.importable.tsx → ...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
@@ -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,
 ];