From 9a2290e6c7df38324851427ca24a1eb8c036fe70 Mon Sep 17 00:00:00 2001 From: hotinglok Date: Wed, 9 Oct 2024 11:33:12 +0100 Subject: [PATCH 01/24] Create AmpExperiment component --- .../__snapshots__/index.test.tsx.snap | 25 +++++++ .../components/AmpExperiment/index.test.tsx | 66 +++++++++++++++++++ src/app/components/AmpExperiment/index.tsx | 46 +++++++++++++ src/app/components/AmpExperiment/types.d.ts | 12 ++++ 4 files changed, 149 insertions(+) create mode 100644 src/app/components/AmpExperiment/__snapshots__/index.test.tsx.snap create mode 100644 src/app/components/AmpExperiment/index.test.tsx create mode 100644 src/app/components/AmpExperiment/index.tsx create mode 100644 src/app/components/AmpExperiment/types.d.ts diff --git a/src/app/components/AmpExperiment/__snapshots__/index.test.tsx.snap b/src/app/components/AmpExperiment/__snapshots__/index.test.tsx.snap new file mode 100644 index 00000000000..44e12803c08 --- /dev/null +++ b/src/app/components/AmpExperiment/__snapshots__/index.test.tsx.snap @@ -0,0 +1,25 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Amp experiment container on Amp pages should render an amp-experiment with the expected config 1`] = ` +
+ + + +
+`; + +exports[`Amp experiment container on Amp pages should render an amp-experiment with the expected config when multiple experiments are running at the same time 1`] = ` +
+ + + +
+`; diff --git a/src/app/components/AmpExperiment/index.test.tsx b/src/app/components/AmpExperiment/index.test.tsx new file mode 100644 index 00000000000..169d20f65ed --- /dev/null +++ b/src/app/components/AmpExperiment/index.test.tsx @@ -0,0 +1,66 @@ +import React from 'react'; +import { render, waitFor } from '../react-testing-library-with-providers'; +import AmpExperiment from './index'; + +const experimentConfig = { + someExperiment: { + variants: { + control: 33, + variant_1: 33, + variant_2: 33, + }, + }, +}; + +const multipleExperimentConfig = { + aExperiment: { + variants: { + control: 33, + variant_1: 33, + variant_2: 33, + }, + }, + bExperiment: { + variants: { + control: 33, + variant_1: 33, + variant_2: 33, + }, + }, +}; + +describe('Amp experiment container on Amp pages', () => { + it('should render an amp-experiment with the expected config', async () => { + const { container } = render( + , + ); + expect(container.querySelector('amp-experiment')).toBeInTheDocument(); + expect(container).toMatchSnapshot(); + }); + + it('should render an amp-experiment with the expected config when multiple experiments are running at the same time', async () => { + const { container } = render( + , + ); + expect(container.querySelector('amp-experiment')).toBeInTheDocument(); + expect(container).toMatchSnapshot(); + }); + + it(`should add amp-experiment extension script to page head`, async () => { + render(); + + await waitFor(() => { + const scripts = Array.from(document.querySelectorAll('head script')); + + expect(scripts).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + src: `https://cdn.ampproject.org/v0/amp-experiment-0.1.js`, + }), + ]), + ); + + expect(scripts).toHaveLength(1); + }); + }); +}); diff --git a/src/app/components/AmpExperiment/index.tsx b/src/app/components/AmpExperiment/index.tsx new file mode 100644 index 00000000000..215979f17d3 --- /dev/null +++ b/src/app/components/AmpExperiment/index.tsx @@ -0,0 +1,46 @@ +/** @jsx jsx */ +/* @jsxFrag React.Fragment */ +import { jsx } from '@emotion/react'; +import React from 'react'; +import { Helmet } from 'react-helmet'; + +type AmpExperimentConfig = { + [key: string]: { + sticky?: boolean; + consentNotificationId?: string; + variants: { + [key: string]: number; // floating point number + }; + }; +}; + +type AmpExperiment = { + [key: string]: AmpExperimentConfig; +}; + +const AmpHead = () => ( + + - - -`; - -exports[`Amp experiment container on Amp pages should render an amp-experiment with the expected config when multiple experiments are running at the same time 1`] = ` -
- - - -
-`; diff --git a/src/app/components/AmpExperiment/index.test.tsx b/src/app/components/AmpExperiment/index.test.tsx index 169d20f65ed..753eb4abde9 100644 --- a/src/app/components/AmpExperiment/index.test.tsx +++ b/src/app/components/AmpExperiment/index.test.tsx @@ -35,7 +35,17 @@ describe('Amp experiment container on Amp pages', () => { , ); expect(container.querySelector('amp-experiment')).toBeInTheDocument(); - expect(container).toMatchSnapshot(); + expect(container).toMatchInlineSnapshot(` +
+ + + +
+ `); }); it('should render an amp-experiment with the expected config when multiple experiments are running at the same time', async () => { @@ -43,7 +53,17 @@ describe('Amp experiment container on Amp pages', () => { , ); expect(container.querySelector('amp-experiment')).toBeInTheDocument(); - expect(container).toMatchSnapshot(); + expect(container).toMatchInlineSnapshot(` +
+ + + +
+ `); }); it(`should add amp-experiment extension script to page head`, async () => { From bdd3edd1d6abde514f7e90ff6af4fda0d8426026 Mon Sep 17 00:00:00 2001 From: hotinglok Date: Wed, 9 Oct 2024 18:01:53 +0100 Subject: [PATCH 03/24] Implement front-end changes --- .../pages/ArticlePage/ArticlePage.styles.ts | 37 +++++++++++++- src/app/pages/ArticlePage/ArticlePage.tsx | 41 ++++++++++++++-- src/app/pages/ArticlePage/SecondaryColumn.tsx | 7 +-- .../ArticlePage/experimentTopStoriesUtils.tsx | 48 +++++++++++++++++++ 4 files changed, 123 insertions(+), 10 deletions(-) create mode 100644 src/app/pages/ArticlePage/experimentTopStoriesUtils.tsx diff --git a/src/app/pages/ArticlePage/ArticlePage.styles.ts b/src/app/pages/ArticlePage/ArticlePage.styles.ts index d1ef396298b..860d8006157 100644 --- a/src/app/pages/ArticlePage/ArticlePage.styles.ts +++ b/src/app/pages/ArticlePage/ArticlePage.styles.ts @@ -82,7 +82,7 @@ export default { }, }), - topStoriesAndFeaturesSection: ({ spacings, mq }: Theme) => + featuresSection: ({ spacings, mq }: Theme) => css({ marginBottom: `${spacings.TRIPLE}rem`, @@ -91,4 +91,39 @@ export default { padding: `${spacings.DOUBLE}rem`, }, }), + + topStoriesSection: ({ spacings, mq }: Theme) => + css({ + marginBottom: `${spacings.TRIPLE}rem`, + + [mq.GROUP_4_MIN_WIDTH]: { + display: 'block', + marginBottom: `${spacings.FULL}rem`, + padding: `${spacings.DOUBLE}rem`, + }, + + '[amp-x-topStoriesExperiment="show_at_halfway"] &': { + display: 'none', + + [mq.GROUP_4_MIN_WIDTH]: { + display: 'block', + marginBottom: `${spacings.FULL}rem`, + padding: `${spacings.DOUBLE}rem`, + }, + }, + }), + + experimentTopStoriesAndFeaturesSection: ({ spacings, mq }: Theme) => + css({ + display: 'none', + marginBottom: `${spacings.TRIPLE}rem`, + + '[amp-x-topStoriesExperiment="show_at_halfway"] &': { + display: 'block', + + [mq.GROUP_4_MIN_WIDTH]: { + display: 'none', + }, + }, + }), }; diff --git a/src/app/pages/ArticlePage/ArticlePage.tsx b/src/app/pages/ArticlePage/ArticlePage.tsx index 7e1b6f51308..fa72c4d4b07 100644 --- a/src/app/pages/ArticlePage/ArticlePage.tsx +++ b/src/app/pages/ArticlePage/ArticlePage.tsx @@ -56,6 +56,7 @@ import { categoryName, getAuthorTwitterHandle, } from '../../components/Byline/utilities'; +import AmpExperiment from '../../components/AmpExperiment'; import { ServiceContext } from '../../contexts/ServiceContext'; import RelatedContentSection from '../../components/RelatedContentSection'; import Disclaimer from '../../components/Disclaimer'; @@ -64,15 +65,20 @@ import SecondaryColumn from './SecondaryColumn'; import styles from './ArticlePage.styles'; import { ComponentToRenderProps, TimeStampProps } from './types'; +import { + ExperimentTopStories, + insertExperimentTopStories, +} from './experimentTopStoriesUtils'; const ArticlePage = ({ pageData }: { pageData: Article }) => { - const { isApp, pageType, service } = useContext(RequestContext); + const { isApp, pageType, service, isAmp } = useContext(RequestContext); const { articleAuthor, isTrustProjectParticipant, showRelatedTopics, brandName, } = useContext(ServiceContext); + const { enabled: preloadLeadImageToggle } = useToggle('preloadLeadImage'); const { @@ -131,6 +137,27 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { ...(isCPS && { pageTitle: `${atiAnalytics.pageTitle} - ${brandName}` }), }; + const enableExperimentTopStories = + isAmp && (service === 'news' || service === 'sport'); + + const topStoriesContent = pageData?.secondaryColumn?.topStories; + let blocksWithExperimentTopStories = blocks; + if (enableExperimentTopStories) { + blocksWithExperimentTopStories = insertExperimentTopStories({ + blocks, + topStoriesContent, + }); + } + + const experimentData = { + topStoriesExperiment: { + variants: { + control: 50, + show_at_halfway: 50, + }, + }, + }; + const componentsToRender = { visuallyHiddenHeadline, headline: headings, @@ -173,7 +200,10 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { disclaimer: (props: ComponentToRenderProps) => ( ), - podcastPromo: () => (podcastPromoEnabled ? : null), + experimentTopStories: () => + topStoriesContent ? ( + + ) : null, }; const visuallyHiddenBlock = { @@ -183,8 +213,8 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { }; const articleBlocks = startsWithHeading - ? blocks - : [visuallyHiddenBlock, ...blocks]; + ? blocksWithExperimentTopStories + : [visuallyHiddenBlock, ...blocksWithExperimentTopStories]; const promoImageBlocks = pageData?.promo?.images?.defaultPromoImage?.blocks ?? []; @@ -206,6 +236,9 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { return (
+ {enableExperimentTopStories && ( + + )} { return (
{topStoriesContent && ( -
+
)} {featuresContent && ( -
+
{ + return ( +
+ +
+ ); +}; + +export const insertExperimentTopStories = ({ + blocks, + topStoriesContent, +}: { + blocks: OptimoBlock[]; + topStoriesContent: TopStoryItem[] | undefined; +}) => { + if (!topStoriesContent) return blocks; + + const experimentTopStoriesBlock = { + type: 'experimentTopStories', + model: topStoriesContent, + id: 'experimentTopStories', + }; + + const halfway = (blocks.length / 2, 10) + 1; + const blocksBeforeInsertIndex = blocks.slice(0, halfway); + const blocksAfterInsertIndex = blocks.slice(halfway, blocks.length); + + return [ + ...blocksBeforeInsertIndex, + experimentTopStoriesBlock, + ...blocksAfterInsertIndex, + ]; +}; From a21578030b416d845e8fa7fb388a1d6bf677b8d0 Mon Sep 17 00:00:00 2001 From: hotinglok Date: Thu, 10 Oct 2024 14:57:29 +0100 Subject: [PATCH 04/24] Refactor various logic --- .../pages/ArticlePage/ArticlePage.styles.ts | 8 ----- src/app/pages/ArticlePage/ArticlePage.tsx | 36 ++++++++++--------- .../experimentTopStories.tsx} | 24 +++++++------ 3 files changed, 32 insertions(+), 36 deletions(-) rename src/app/pages/ArticlePage/{experimentTopStoriesUtils.tsx => experimentTopStories/experimentTopStories.tsx} (69%) diff --git a/src/app/pages/ArticlePage/ArticlePage.styles.ts b/src/app/pages/ArticlePage/ArticlePage.styles.ts index 860d8006157..68009770819 100644 --- a/src/app/pages/ArticlePage/ArticlePage.styles.ts +++ b/src/app/pages/ArticlePage/ArticlePage.styles.ts @@ -81,7 +81,6 @@ export default { paddingBottom: `${spacings.QUADRUPLE}rem`, }, }), - featuresSection: ({ spacings, mq }: Theme) => css({ marginBottom: `${spacings.TRIPLE}rem`, @@ -91,20 +90,16 @@ export default { padding: `${spacings.DOUBLE}rem`, }, }), - topStoriesSection: ({ spacings, mq }: Theme) => css({ marginBottom: `${spacings.TRIPLE}rem`, - [mq.GROUP_4_MIN_WIDTH]: { display: 'block', marginBottom: `${spacings.FULL}rem`, padding: `${spacings.DOUBLE}rem`, }, - '[amp-x-topStoriesExperiment="show_at_halfway"] &': { display: 'none', - [mq.GROUP_4_MIN_WIDTH]: { display: 'block', marginBottom: `${spacings.FULL}rem`, @@ -112,15 +107,12 @@ export default { }, }, }), - experimentTopStoriesAndFeaturesSection: ({ spacings, mq }: Theme) => css({ display: 'none', marginBottom: `${spacings.TRIPLE}rem`, - '[amp-x-topStoriesExperiment="show_at_halfway"] &': { display: 'block', - [mq.GROUP_4_MIN_WIDTH]: { display: 'none', }, diff --git a/src/app/pages/ArticlePage/ArticlePage.tsx b/src/app/pages/ArticlePage/ArticlePage.tsx index fa72c4d4b07..ada12f05e84 100644 --- a/src/app/pages/ArticlePage/ArticlePage.tsx +++ b/src/app/pages/ArticlePage/ArticlePage.tsx @@ -56,22 +56,23 @@ import { categoryName, getAuthorTwitterHandle, } from '../../components/Byline/utilities'; -import AmpExperiment from '../../components/AmpExperiment'; import { ServiceContext } from '../../contexts/ServiceContext'; import RelatedContentSection from '../../components/RelatedContentSection'; import Disclaimer from '../../components/Disclaimer'; - import SecondaryColumn from './SecondaryColumn'; - import styles from './ArticlePage.styles'; import { ComponentToRenderProps, TimeStampProps } from './types'; +import AmpExperiment from '../../components/AmpExperiment'; import { + experimentTopStoriesConfig, ExperimentTopStories, insertExperimentTopStories, -} from './experimentTopStoriesUtils'; +} from './experimentTopStories/experimentTopStories'; const ArticlePage = ({ pageData }: { pageData: Article }) => { - const { isApp, pageType, service, isAmp } = useContext(RequestContext); + const { isApp, pageType, service, isAmp, pathname } = + useContext(RequestContext); + const { articleAuthor, isTrustProjectParticipant, @@ -137,9 +138,18 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { ...(isCPS && { pageTitle: `${atiAnalytics.pageTitle} - ${brandName}` }), }; - const enableExperimentTopStories = - isAmp && (service === 'news' || service === 'sport'); + const urn = pathname.split('/').pop(); + const newsTestAsset = 'c6v11qzyv8po'; + const newsAsset = 'cz7xywn940ro'; + const sportAsset = 'cpgw0xjmpd3o'; + const experimentAssets = [newsAsset, sportAsset, newsTestAsset]; + const experimentServices = ['news', 'sport']; + const enableExperimentTopStories = + isAmp && + urn && + experimentServices.includes(service) && + experimentAssets.includes(urn); const topStoriesContent = pageData?.secondaryColumn?.topStories; let blocksWithExperimentTopStories = blocks; if (enableExperimentTopStories) { @@ -149,15 +159,6 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { }); } - const experimentData = { - topStoriesExperiment: { - variants: { - control: 50, - show_at_halfway: 50, - }, - }, - }; - const componentsToRender = { visuallyHiddenHeadline, headline: headings, @@ -200,6 +201,7 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { disclaimer: (props: ComponentToRenderProps) => ( ), + podcastPromo: () => (podcastPromoEnabled ? : null), experimentTopStories: () => topStoriesContent ? ( @@ -237,7 +239,7 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { return (
{enableExperimentTopStories && ( - + )} Date: Mon, 14 Oct 2024 11:47:19 +0100 Subject: [PATCH 05/24] Fix urn bug --- src/app/pages/ArticlePage/ArticlePage.tsx | 30 +++++++---------- .../{experimentTopStories.tsx => helpers.tsx} | 33 ++++++++++++++++++- 2 files changed, 44 insertions(+), 19 deletions(-) rename src/app/pages/ArticlePage/experimentTopStories/{experimentTopStories.tsx => helpers.tsx} (59%) diff --git a/src/app/pages/ArticlePage/ArticlePage.tsx b/src/app/pages/ArticlePage/ArticlePage.tsx index ada12f05e84..5f2eba8cc24 100644 --- a/src/app/pages/ArticlePage/ArticlePage.tsx +++ b/src/app/pages/ArticlePage/ArticlePage.tsx @@ -65,9 +65,10 @@ import { ComponentToRenderProps, TimeStampProps } from './types'; import AmpExperiment from '../../components/AmpExperiment'; import { experimentTopStoriesConfig, - ExperimentTopStories, + enableExperimentTopStories, insertExperimentTopStories, -} from './experimentTopStories/experimentTopStories'; + ExperimentTopStories, +} from './experimentTopStories/helpers'; const ArticlePage = ({ pageData }: { pageData: Article }) => { const { isApp, pageType, service, isAmp, pathname } = @@ -138,21 +139,14 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { ...(isCPS && { pageTitle: `${atiAnalytics.pageTitle} - ${brandName}` }), }; - const urn = pathname.split('/').pop(); - const newsTestAsset = 'c6v11qzyv8po'; - const newsAsset = 'cz7xywn940ro'; - const sportAsset = 'cpgw0xjmpd3o'; - const experimentAssets = [newsAsset, sportAsset, newsTestAsset]; - const experimentServices = ['news', 'sport']; - - const enableExperimentTopStories = - isAmp && - urn && - experimentServices.includes(service) && - experimentAssets.includes(urn); - const topStoriesContent = pageData?.secondaryColumn?.topStories; let blocksWithExperimentTopStories = blocks; - if (enableExperimentTopStories) { + const topStoriesContent = pageData?.secondaryColumn?.topStories; + const shouldEnableExperimentTopStories = enableExperimentTopStories({ + isAmp, + pathname, + service, + }); + if (shouldEnableExperimentTopStories) { blocksWithExperimentTopStories = insertExperimentTopStories({ blocks, topStoriesContent, @@ -203,7 +197,7 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { ), podcastPromo: () => (podcastPromoEnabled ? : null), experimentTopStories: () => - topStoriesContent ? ( + shouldEnableExperimentTopStories && topStoriesContent ? ( ) : null, }; @@ -238,7 +232,7 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { return (
- {enableExperimentTopStories && ( + {shouldEnableExperimentTopStories && ( )} diff --git a/src/app/pages/ArticlePage/experimentTopStories/experimentTopStories.tsx b/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx similarity index 59% rename from src/app/pages/ArticlePage/experimentTopStories/experimentTopStories.tsx rename to src/app/pages/ArticlePage/experimentTopStories/helpers.tsx index 6a4b1bcc13b..02c154fa843 100644 --- a/src/app/pages/ArticlePage/experimentTopStories/experimentTopStories.tsx +++ b/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx @@ -14,6 +14,30 @@ export const experimentTopStoriesConfig = { }, }; +export const enableExperimentTopStories = ({ + isAmp, + pathname, + service, +}: { + isAmp: boolean; + pathname: string; + service: string; +}) => { + const urn = pathname.split('/')[3].slice(0, -4); // .slice() to remove '.amp' at the end of pathname + const newsTestAsset = 'c6v11qzyv8po'; + const newsAsset = 'cz7xywn940ro'; + const sportAsset = 'cpgw0xjmpd3o'; + const experimentAssets = [newsAsset, newsTestAsset, sportAsset]; + const experimentServices = ['news', 'sport']; + + return ( + isAmp && + urn && + experimentServices.includes(service) && + experimentAssets.includes(urn) + ); +}; + export const ExperimentTopStories = ({ topStoriesContent, }: { @@ -46,5 +70,12 @@ export const insertExperimentTopStories = ({ }; const halfway = Math.floor(blocks.length * 0.5); - return blocks.splice(halfway, 0, experimentTopStoriesBlock); + const blocksBeforeInsertIndex = blocks.slice(0, halfway); + const blocksAfterInsertIndex = blocks.slice(halfway, blocks.length); + + return [ + ...blocksBeforeInsertIndex, + experimentTopStoriesBlock, + ...blocksAfterInsertIndex, + ]; }; From b11f47a79f79a58e7e2168f7012fbea72fb5b583 Mon Sep 17 00:00:00 2001 From: hotinglok Date: Mon, 14 Oct 2024 15:41:51 +0100 Subject: [PATCH 06/24] Refactor various logic, add tests --- .../pages/ArticlePage/ArticlePage.styles.ts | 2 +- src/app/pages/ArticlePage/ArticlePage.tsx | 14 +-- .../experimentTopStories/helpers.test.tsx | 85 +++++++++++++++++++ .../experimentTopStories/helpers.tsx | 32 +++---- 4 files changed, 110 insertions(+), 23 deletions(-) create mode 100644 src/app/pages/ArticlePage/experimentTopStories/helpers.test.tsx diff --git a/src/app/pages/ArticlePage/ArticlePage.styles.ts b/src/app/pages/ArticlePage/ArticlePage.styles.ts index 68009770819..016016aac21 100644 --- a/src/app/pages/ArticlePage/ArticlePage.styles.ts +++ b/src/app/pages/ArticlePage/ArticlePage.styles.ts @@ -107,7 +107,7 @@ export default { }, }, }), - experimentTopStoriesAndFeaturesSection: ({ spacings, mq }: Theme) => + experimentTopStoriesSection: ({ spacings, mq }: Theme) => css({ display: 'none', marginBottom: `${spacings.TRIPLE}rem`, diff --git a/src/app/pages/ArticlePage/ArticlePage.tsx b/src/app/pages/ArticlePage/ArticlePage.tsx index 5f2eba8cc24..02a5a9bae8d 100644 --- a/src/app/pages/ArticlePage/ArticlePage.tsx +++ b/src/app/pages/ArticlePage/ArticlePage.tsx @@ -141,11 +141,13 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { let blocksWithExperimentTopStories = blocks; const topStoriesContent = pageData?.secondaryColumn?.topStories; - const shouldEnableExperimentTopStories = enableExperimentTopStories({ - isAmp, - pathname, - service, - }); + const shouldEnableExperimentTopStories = + enableExperimentTopStories({ + isAmp, + service, + pathname, + }) && topStoriesContent; + if (shouldEnableExperimentTopStories) { blocksWithExperimentTopStories = insertExperimentTopStories({ blocks, @@ -197,7 +199,7 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { ), podcastPromo: () => (podcastPromoEnabled ? : null), experimentTopStories: () => - shouldEnableExperimentTopStories && topStoriesContent ? ( + shouldEnableExperimentTopStories ? ( ) : null, }; diff --git a/src/app/pages/ArticlePage/experimentTopStories/helpers.test.tsx b/src/app/pages/ArticlePage/experimentTopStories/helpers.test.tsx new file mode 100644 index 00000000000..5d60c3397d7 --- /dev/null +++ b/src/app/pages/ArticlePage/experimentTopStories/helpers.test.tsx @@ -0,0 +1,85 @@ +import { + enableExperimentTopStories, + insertExperimentTopStories, +} from './helpers'; +import { topStoriesList } from '../PagePromoSections/TopStoriesSection/fixture/index'; + +describe('AMP top stories experiment', () => { + describe('enableExperimentTopStories()', () => { + it('should return true if props match conditions.', () => { + expect( + enableExperimentTopStories({ + isAmp: true, + pathname: '/news/articles/c6v11qzyv8po.amp', + service: 'news', + }), + ).toBe(true); + }); + it.each` + testDescription | isAmp | pathname | service + ${'all props are undefined'} | ${false} | ${undefined} | ${undefined} + ${'only isAmp is true'} | ${true} | ${undefined} | ${undefined} + ${'only pathname is undefined'} | ${true} | ${undefined} | ${'news'} + ${'only pathname is defined and valid'} | ${false} | ${'/news/articles/c6v11qzyv8po.amp'} | ${undefined} + ${'all props defined but pathname is invalid'} | ${false} | ${'/news/articles/c1231qzyv8po.amp'} | ${undefined} + ${'only service is undefined'} | ${true} | ${'/news/articles/c6v11qzyv8po.amp'} | ${undefined} + ${'only service is defined and valid'} | ${false} | ${undefined} | ${'news'} + ${'all props defined but service is invalid'} | ${true} | ${'/news/articles/c6v11qzyv8po.amp'} | ${'igbo'} + `( + 'should return false if $testDescription.', + ({ isAmp, pathname, service }) => { + expect( + enableExperimentTopStories({ + isAmp, + pathname, + service, + }), + ).toBe(false); + }, + ); + }); + + describe('insertExperimentTopStories()', () => { + const mockTextBlock = { + type: 'text', + model: { + blocks: [], + }, + }; + const expectedExperimentTopStoriesBlock = { + type: 'experimentTopStories', + model: topStoriesList, + id: 'experimentTopStories', + }; + + const blocksEvenLength = [mockTextBlock, mockTextBlock]; + const blocksOddLength = [mockTextBlock, mockTextBlock, mockTextBlock]; + const expectedBlocksEvenLength = [ + mockTextBlock, + expectedExperimentTopStoriesBlock, + mockTextBlock, + ]; + const expectedBlocksOddLength = [ + mockTextBlock, + expectedExperimentTopStoriesBlock, + mockTextBlock, + mockTextBlock, + ]; + + it.each` + testType | inputBlocks | expectedOutput + ${'even'} | ${blocksEvenLength} | ${expectedBlocksEvenLength} + ${'odd'} | ${blocksOddLength} | ${expectedBlocksOddLength} + `( + 'should insert experimentTopStories block into blocks array in the correct position when blocks.length is $testType', + ({ inputBlocks, expectedOutput }) => { + expect( + insertExperimentTopStories({ + blocks: inputBlocks, + topStoriesContent: topStoriesList, + }), + ).toEqual(expectedOutput); + }, + ); + }); +}); diff --git a/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx b/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx index 02c154fa843..28445845f25 100644 --- a/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx +++ b/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx @@ -16,13 +16,14 @@ export const experimentTopStoriesConfig = { export const enableExperimentTopStories = ({ isAmp, - pathname, service, + pathname, }: { isAmp: boolean; - pathname: string; service: string; + pathname: string; }) => { + if (!isAmp || !service || !pathname) return false; const urn = pathname.split('/')[3].slice(0, -4); // .slice() to remove '.amp' at the end of pathname const newsTestAsset = 'c6v11qzyv8po'; const newsAsset = 'cz7xywn940ro'; @@ -30,6 +31,13 @@ export const enableExperimentTopStories = ({ const experimentAssets = [newsAsset, newsTestAsset, sportAsset]; const experimentServices = ['news', 'sport']; + console.log( + isAmp, + urn, + experimentServices.includes(service), + experimentAssets.includes(urn), + ); + return ( isAmp && urn && @@ -45,7 +53,7 @@ export const ExperimentTopStories = ({ }) => { return (
@@ -59,23 +67,15 @@ export const insertExperimentTopStories = ({ topStoriesContent, }: { blocks: OptimoBlock[]; - topStoriesContent: TopStoryItem[] | undefined; + topStoriesContent: TopStoryItem[]; }) => { - if (!topStoriesContent) return blocks; - + const insertIndex = Math.floor(blocks.length * 0.5); // halfway index of blocks array const experimentTopStoriesBlock = { type: 'experimentTopStories', model: topStoriesContent, - id: 'experimentTopStories', + id: `experimentTopStories-${insertIndex}`, }; - const halfway = Math.floor(blocks.length * 0.5); - const blocksBeforeInsertIndex = blocks.slice(0, halfway); - const blocksAfterInsertIndex = blocks.slice(halfway, blocks.length); - - return [ - ...blocksBeforeInsertIndex, - experimentTopStoriesBlock, - ...blocksAfterInsertIndex, - ]; + blocks.splice(insertIndex, 0, experimentTopStoriesBlock); + return blocks; }; From 739314b356ff9e60249fbb675cbfd38f48371410 Mon Sep 17 00:00:00 2001 From: hotinglok Date: Mon, 14 Oct 2024 17:40:24 +0100 Subject: [PATCH 07/24] Refactor block insertion logic, add article page tests --- .../experimentTopStories/helpers.tsx | 13 ++-- src/app/pages/ArticlePage/index.test.tsx | 72 +++++++++++++++++++ 2 files changed, 76 insertions(+), 9 deletions(-) diff --git a/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx b/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx index 28445845f25..11e2c515658 100644 --- a/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx +++ b/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx @@ -31,13 +31,6 @@ export const enableExperimentTopStories = ({ const experimentAssets = [newsAsset, newsTestAsset, sportAsset]; const experimentServices = ['news', 'sport']; - console.log( - isAmp, - urn, - experimentServices.includes(service), - experimentAssets.includes(urn), - ); - return ( isAmp && urn && @@ -76,6 +69,8 @@ export const insertExperimentTopStories = ({ id: `experimentTopStories-${insertIndex}`, }; - blocks.splice(insertIndex, 0, experimentTopStoriesBlock); - return blocks; + const blocksClone = [...blocks]; + + blocksClone.splice(insertIndex, 0, experimentTopStoriesBlock); + return blocksClone; }; diff --git a/src/app/pages/ArticlePage/index.test.tsx b/src/app/pages/ArticlePage/index.test.tsx index f7acc2bf37c..588a1ca76d9 100644 --- a/src/app/pages/ArticlePage/index.test.tsx +++ b/src/app/pages/ArticlePage/index.test.tsx @@ -39,6 +39,7 @@ import { ServiceContextProvider } from '../../contexts/ServiceContext'; import ArticlePage from './ArticlePage'; import ThemeProvider from '../../components/ThemeProvider'; import ATIAnalytics from '../../components/ATIAnalytics'; +import { topStoriesList } from './PagePromoSections/TopStoriesSection/fixture/index'; jest.mock('../../components/ThemeProvider'); @@ -64,6 +65,8 @@ type Props = { showAdsBasedOnLocation?: boolean; isApp?: boolean; promo?: boolean | null; + isAmp?: boolean; + pathname?: string; }; const Context = ({ @@ -74,12 +77,16 @@ const Context = ({ showAdsBasedOnLocation = false, isApp = false, promo = null, + isAmp = false, + pathname = '/pathname', }: PropsWithChildren = {}) => { const appInput = { ...input, service, showAdsBasedOnLocation, isApp, + isAmp, + pathname, }; return ( @@ -869,4 +876,69 @@ describe('Article Page', () => { ); }); }); + + describe('when rendering an AMP page', () => { + const pageDataWithSecondaryColumn = { + ...articleDataNews, + secondaryColumn: { + topStories: topStoriesList, + features: [], + }, + }; + + const renderAmpPage = ({ + service, + pathname, + }: { + service: Services; + pathname: string; + }) => { + return render( + + + , + { + isAmp: true, + service, + pathname, + }, + ); + }; + + const validNewsAsset = '/news/articles/c6v11qzyv8po.amp'; + const validSportAsset = '/sport/articles/cpgw0xjmpd3o.amp'; + + it.each` + service | pathname + ${'news'} | ${validNewsAsset} + ${'sport'} | ${validSportAsset} + `( + 'should render page with experiment-top-stories blocks only on specific $service assets', + ({ service, pathname }) => { + const { queryByTestId } = renderAmpPage({ + service, + pathname, + }); + + expect(queryByTestId('experiment-top-stories')).toBeInTheDocument(); + }, + ); + + it.each` + service | pathname | testDescription + ${'news'} | ${'/news/articles/c1231qzyv8po.amp'} | ${'news assets not specified'} + ${'sport'} | ${'/sport/articles/c1231qzyv8po.amp'} | ${'sport assets not specified'} + ${'pidgin'} | ${'/pidgin/articles/c6v11qzyv8po.amp'} | ${`services which are not 'news' or 'sport'`} + `( + 'should render page without experiment-top-stories blocks on $testDescription', + ({ service, pathname }) => { + const { queryByTestId } = renderAmpPage({ + service, + pathname, + }); + + expect(queryByTestId('experiment-top-stories')).not.toBeInTheDocument(); + }, + ); + }); }); From db2b3ffb2ef8509d8f82756a42cf4315cf449121 Mon Sep 17 00:00:00 2001 From: hotinglok Date: Mon, 14 Oct 2024 17:57:47 +0100 Subject: [PATCH 08/24] Update block insertion tests --- .../pages/ArticlePage/experimentTopStories/helpers.test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/pages/ArticlePage/experimentTopStories/helpers.test.tsx b/src/app/pages/ArticlePage/experimentTopStories/helpers.test.tsx index 5d60c3397d7..4565b18f06d 100644 --- a/src/app/pages/ArticlePage/experimentTopStories/helpers.test.tsx +++ b/src/app/pages/ArticlePage/experimentTopStories/helpers.test.tsx @@ -49,11 +49,12 @@ describe('AMP top stories experiment', () => { const expectedExperimentTopStoriesBlock = { type: 'experimentTopStories', model: topStoriesList, - id: 'experimentTopStories', + id: 'experimentTopStories-1', }; const blocksEvenLength = [mockTextBlock, mockTextBlock]; const blocksOddLength = [mockTextBlock, mockTextBlock, mockTextBlock]; + const expectedBlocksEvenLength = [ mockTextBlock, expectedExperimentTopStoriesBlock, From bcc34e5f0f11c8015191f1f413f65240ec598dce Mon Sep 17 00:00:00 2001 From: hotinglok Date: Tue, 15 Oct 2024 11:04:27 +0100 Subject: [PATCH 09/24] Fix urn retrieval function --- src/app/pages/ArticlePage/experimentTopStories/helpers.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx b/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx index 11e2c515658..6c802435b26 100644 --- a/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx +++ b/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx @@ -24,7 +24,7 @@ export const enableExperimentTopStories = ({ pathname: string; }) => { if (!isAmp || !service || !pathname) return false; - const urn = pathname.split('/')[3].slice(0, -4); // .slice() to remove '.amp' at the end of pathname + const urn = pathname.split('/').pop()?.slice(0, -4); // .slice() to remove '.amp' at the end of pathname const newsTestAsset = 'c6v11qzyv8po'; const newsAsset = 'cz7xywn940ro'; const sportAsset = 'cpgw0xjmpd3o'; From 6aae3125111b8ac36c2e38babfb2da464c24afa0 Mon Sep 17 00:00:00 2001 From: hotinglok Date: Tue, 15 Oct 2024 12:13:22 +0100 Subject: [PATCH 10/24] Minor cleanup --- src/app/components/AmpExperiment/types.d.ts | 5 --- .../experimentTopStories/helpers.tsx | 32 +++++++++---------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/app/components/AmpExperiment/types.d.ts b/src/app/components/AmpExperiment/types.d.ts index 6550b6219fd..53aa9b97cb2 100644 --- a/src/app/components/AmpExperiment/types.d.ts +++ b/src/app/components/AmpExperiment/types.d.ts @@ -1,9 +1,4 @@ declare namespace JSX { - /* - * AMP currently doesn't have built-in types for TypeScript, but it's in their roadmap (https://github.com/ampproject/amphtml/issues/13791). - * As a workaround you can manually create custom types (https://stackoverflow.com/a/50601125). - */ - interface IntrinsicElements { 'amp-experiment': React.PropsWithChildren< ScriptHTMLAttributes diff --git a/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx b/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx index 6c802435b26..ce9a40e2c50 100644 --- a/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx +++ b/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx @@ -39,22 +39,6 @@ export const enableExperimentTopStories = ({ ); }; -export const ExperimentTopStories = ({ - topStoriesContent, -}: { - topStoriesContent: TopStoryItem[]; -}) => { - return ( -
- -
- ); -}; - export const insertExperimentTopStories = ({ blocks, topStoriesContent, @@ -74,3 +58,19 @@ export const insertExperimentTopStories = ({ blocksClone.splice(insertIndex, 0, experimentTopStoriesBlock); return blocksClone; }; + +export const ExperimentTopStories = ({ + topStoriesContent, +}: { + topStoriesContent: TopStoryItem[]; +}) => { + return ( +
+ +
+ ); +}; From 3be6ae5cb46456dfec32ab661a705ae21ced829f Mon Sep 17 00:00:00 2001 From: hotinglok Date: Tue, 15 Oct 2024 12:18:47 +0100 Subject: [PATCH 11/24] Minor cleanup --- src/app/components/AmpExperiment/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/components/AmpExperiment/index.tsx b/src/app/components/AmpExperiment/index.tsx index 215979f17d3..03d27e510c2 100644 --- a/src/app/components/AmpExperiment/index.tsx +++ b/src/app/components/AmpExperiment/index.tsx @@ -9,7 +9,7 @@ type AmpExperimentConfig = { sticky?: boolean; consentNotificationId?: string; variants: { - [key: string]: number; // floating point number + [key: string]: number; // variantName: traffic allocation % }; }; }; From cfe5254d939a515bc79f00142205897fc2e5478b Mon Sep 17 00:00:00 2001 From: hotinglok Date: Tue, 15 Oct 2024 15:29:45 +0100 Subject: [PATCH 12/24] Rename, move, update various files and variables --- src/app/components/AmpExperiment/index.tsx | 17 +++++++++++------ src/app/pages/ArticlePage/ArticlePage.styles.ts | 13 +------------ src/app/pages/ArticlePage/ArticlePage.tsx | 13 ++++++------- .../experimentTopStories/helpers.tsx | 2 +- .../experimentTopStories/index.styles.ts | 15 +++++++++++++++ 5 files changed, 34 insertions(+), 26 deletions(-) create mode 100644 src/app/pages/ArticlePage/experimentTopStories/index.styles.ts diff --git a/src/app/components/AmpExperiment/index.tsx b/src/app/components/AmpExperiment/index.tsx index 03d27e510c2..e445f88bef5 100644 --- a/src/app/components/AmpExperiment/index.tsx +++ b/src/app/components/AmpExperiment/index.tsx @@ -4,18 +4,23 @@ import { jsx } from '@emotion/react'; import React from 'react'; import { Helmet } from 'react-helmet'; + +type Variant = string; +type Experiment = string; +type trafficAllocationPercentage = number; + type AmpExperimentConfig = { - [key: string]: { + [key: Experiment]: { sticky?: boolean; consentNotificationId?: string; variants: { - [key: string]: number; // variantName: traffic allocation % + [key: Variant]: trafficAllocationPercentage; }; }; }; -type AmpExperiment = { - [key: string]: AmpExperimentConfig; +type AmpExperimentProps = { + [key: Experiment]: AmpExperimentConfig; }; const AmpHead = () => ( @@ -28,7 +33,7 @@ const AmpHead = () => ( ); -const AmpExperiment = ({ experimentData }: AmpExperiment) => { +const AmpExperiment = ({ experimentConfig }: AmpExperimentProps) => { return ( <> @@ -36,7 +41,7 @@ const AmpExperiment = ({ experimentData }: AmpExperiment) => {