Merge branch 'NEWSWORLDSERVICE-1477-no-import-assign' of github.com:b…

…bc/simorgh into NEWSWORLDSERVICE-1477-no-import-assign
bbc · Nov 25, 2024 · 60436da · 60436da
2 parents 2554656 + ebe5faa
commit 60436da
Show file tree

Hide file tree

Showing 10 changed files with 71 additions and 74 deletions.
diff --git a/src/app/components/Embeds/EmbedHtml/index.test.tsx b/src/app/components/Embeds/EmbedHtml/index.test.tsx
@@ -148,6 +148,18 @@ describe('EmbedHtml', () => {
         },
       );
 
+      expect(container).toBeEmptyDOMElement();
+    });
+  });
+  describe('Lite', () => {
+    it('Should return null if isLite is true', () => {
+      const { container } = render(
+        <EmbedHtml embeddableContent={electionHtml} />,
+        {
+          isLite: true,
+        },
+      );
+
       expect(container).toBeEmptyDOMElement();
     });
   });

diff --git a/src/app/components/Embeds/EmbedHtml/index.tsx b/src/app/components/Embeds/EmbedHtml/index.tsx
@@ -1,19 +1,23 @@
 /** @jsx jsx */
 import { jsx } from '@emotion/react';
-import { PropsWithChildren } from 'react';
+import { PropsWithChildren, useContext } from 'react';
 import useToggle from '#app/hooks/useToggle';
+import { RequestContext } from '../../../contexts/RequestContext';
 import styles from './index.styles';
 
 type Props = {
   embeddableContent: string;
 };
 
 const EmbedHtml = ({ embeddableContent }: PropsWithChildren<Props>) => {
+  const { isLite } = useContext(RequestContext);
   const { enabled: electionBannerEnabled }: { enabled: boolean | null } =
     useToggle('electionBanner');
 
   if (!embeddableContent) return null;
 
+  if (isLite) return null;
+
   // TODO: Remove this logic after the US Elections
   const isUSElectionBanner = embeddableContent.includes(
     '2024-us-presidential-election-banner',

diff --git a/src/app/components/Embeds/EmbedImages/index.test.tsx b/src/app/components/Embeds/EmbedImages/index.test.tsx
@@ -75,4 +75,16 @@ describe('EmbedImages', () => {
       );
     });
   });
+
+  describe('Lite', () => {
+    it('Should return null if isLite is true', () => {
+      const { container } = render(
+        <EmbedImages blocks={chartEmbedImages.blocks} />,
+        {
+          isLite: true,
+        },
+      );
+      expect(container).toBeEmptyDOMElement();
+    });
+  });
 });
diff --git a/src/app/components/Embeds/EmbedImages/index.tsx b/src/app/components/Embeds/EmbedImages/index.tsx
@@ -12,13 +12,15 @@ type Props = {
 };
 
 const EmbedImages = ({ blocks: embedImages }: PropsWithChildren<Props>) => {
-  const { isAmp, env } = useContext(RequestContext);
+  const { isAmp, env, isLite } = useContext(RequestContext);
   const ampImage = embedImages?.[1]?.model?.blocks;
   const canonicalImage = embedImages?.[2]?.model?.blocks;
   const image = isAmp ? ampImage : canonicalImage;
   const rawImage = image?.[1]?.model;
   if (!rawImage) return null;
 
+  if (isLite) return null;
+
   const idt2EnvUrlSubPath = env === 'live' ? 'idt2' : 'idt2-test';
   const { width, height, locator } = rawImage;
 

diff --git a/src/app/hooks/useOptimizelyVariation/index.jsx b/src/app/hooks/useOptimizelyVariation/index.jsx
@@ -29,9 +29,6 @@ const useOptimizelyVariation = (
       }
     }, [isClientReady, decision.variationKey, didTimeout]);
 
-    // Optimizely sets 'off' as the default variation if a flag is not enabled.
-    if (variation === 'off') return null;
-
     return variation;
   }
 

diff --git a/src/app/pages/ArticlePage/ArticlePage.tsx b/src/app/pages/ArticlePage/ArticlePage.tsx
@@ -77,8 +77,7 @@ import {
 } from './experimentTopStories/helpers';
 
 const ArticlePage = ({ pageData }: { pageData: Article }) => {
-  const { isApp, pageType, service, isAmp, id, env } =
-    useContext(RequestContext);
+  const { isApp, pageType, service, isAmp, env } = useContext(RequestContext);
 
   const {
     articleAuthor,
@@ -159,7 +158,6 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => {
       topStoriesContent,
       isAmp,
       service,
-      id,
     });
 
   const showRelatedContent = blocks.some(

diff --git a/src/app/pages/ArticlePage/experimentTopStories/helpers.test.tsx b/src/app/pages/ArticlePage/experimentTopStories/helpers.test.tsx
@@ -26,42 +26,37 @@ describe('AMP top stories experiment', () => {
     };
   };
 
-  const blocksEvenLength = Array(10).fill(mockTextBlock);
-  const blocksOddLength = Array(11).fill(mockTextBlock);
+  const blocksEvenLength = Array(14).fill(mockTextBlock);
+  const blocksOddLength = Array(15).fill(mockTextBlock);
 
   describe('getExperimentTopStories()', () => {
     it('returns shouldEnableExperimentTopStories as true if props match conditions.', () => {
       const { shouldEnableExperimentTopStories } = getExperimentTopStories({
         blocks: blocksEvenLength,
         topStoriesContent: topStoriesList,
         isAmp: true,
-        id: 'c6v11qzyv8po',
         service: 'news',
       });
       expect(shouldEnableExperimentTopStories).toBe(true);
     });
 
     it.each`
-      testDescription                                     | isAmp    | id                | service      | blocksLength
-      ${'all props are undefined'}                        | ${false} | ${undefined}      | ${undefined} | ${undefined}
-      ${'only isAmp is true'}                             | ${true}  | ${undefined}      | ${undefined} | ${undefined}
-      ${'only id is undefined'}                           | ${true}  | ${undefined}      | ${'news'}    | ${11}
-      ${'only id is defined and valid'}                   | ${false} | ${'c6v11qzyv8po'} | ${undefined} | ${undefined}
-      ${'all props defined but id is invalid'}            | ${true}  | ${'c1231qzyv8po'} | ${'news'}    | ${11}
-      ${'only service is undefined'}                      | ${true}  | ${'c6v11qzyv8po'} | ${undefined} | ${11}
-      ${'only service is defined and valid'}              | ${false} | ${undefined}      | ${'news'}    | ${undefined}
-      ${'all props defined but service is invalid'}       | ${true}  | ${'c6v11qzyv8po'} | ${'igbo'}    | ${11}
-      ${'only blocks length is defined and valid'}        | ${false} | ${undefined}      | ${undefined} | ${11}
-      ${'all props defined but blocks length is invalid'} | ${true}  | ${'c6v11qzyv8po'} | ${'news'}    | ${7}
+      testDescription                                     | isAmp    | service      | blocksLength
+      ${'all props are undefined'}                        | ${false} | ${undefined} | ${undefined}
+      ${'only isAmp is true'}                             | ${true}  | ${undefined} | ${undefined}
+      ${'only service is undefined'}                      | ${true}  | ${undefined} | ${14}
+      ${'only service is defined and valid'}              | ${false} | ${'news'}    | ${undefined}
+      ${'all props defined but service is invalid'}       | ${true}  | ${'igbo'}    | ${14}
+      ${'only blocks length is defined and valid'}        | ${false} | ${undefined} | ${14}
+      ${'all props defined but blocks length is invalid'} | ${true}  | ${'news'}    | ${7}
     `(
       'returns shouldEnableExperimentTopStories as false because $testDescription.',
-      ({ isAmp, id, service, blocksLength }) => {
+      ({ isAmp, service, blocksLength }) => {
         const blocks = Array(blocksLength).fill(mockTextBlock, 0);
         const { shouldEnableExperimentTopStories } = getExperimentTopStories({
           blocks,
           topStoriesContent: topStoriesList,
           isAmp,
-          id,
           service,
         });
 
@@ -75,30 +70,39 @@ describe('AMP top stories experiment', () => {
       mockTextBlock,
       expectedExperimentTopStoriesBlock('Quarter'),
       mockTextBlock,
+      mockTextBlock,
+      mockTextBlock,
       expectedExperimentTopStoriesBlock('Half'),
       mockTextBlock,
       mockTextBlock,
+      mockTextBlock,
       expectedExperimentTopStoriesBlock('ThreeQuarters'),
       mockTextBlock,
       mockTextBlock,
       mockTextBlock,
       mockTextBlock,
+      mockTextBlock,
     ];
+
     const expectedBlocksOddLength = [
       mockTextBlock,
       mockTextBlock,
       mockTextBlock,
       expectedExperimentTopStoriesBlock('Quarter'),
       mockTextBlock,
+      mockTextBlock,
+      mockTextBlock,
       expectedExperimentTopStoriesBlock('Half'),
       mockTextBlock,
       mockTextBlock,
       mockTextBlock,
+      mockTextBlock,
       expectedExperimentTopStoriesBlock('ThreeQuarters'),
       mockTextBlock,
       mockTextBlock,
       mockTextBlock,
       mockTextBlock,
+      mockTextBlock,
     ];
 
     it.each`
@@ -112,7 +116,6 @@ describe('AMP top stories experiment', () => {
           blocks: inputBlocks,
           topStoriesContent: topStoriesList,
           isAmp: true,
-          id: 'c6v11qzyv8po',
           service: 'news',
         });
         expect(transformedBlocks).toEqual(expectedOutput);

diff --git a/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx b/src/app/pages/ArticlePage/experimentTopStories/helpers.tsx
@@ -11,10 +11,10 @@ export const experimentName = 'topStoriesExperiment';
 export const experimentTopStoriesConfig = {
   [experimentName]: {
     variants: {
-      control: 25,
-      show_at_quarter: 25,
-      show_at_half: 25,
-      show_at_three_quarters: 25,
+      control: 0.25,
+      show_at_quarter: 0.25,
+      show_at_half: 0.25,
+      show_at_three_quarters: 0.25,
     },
   },
 };
@@ -23,53 +23,21 @@ type VariantNames = 'Quarter' | 'Half' | 'ThreeQuarters';
 type Positions = 'articleBody' | 'secondaryColumn';
 type TrackingEventType = 'view' | 'click';
 
-const ARTICLE_LENGTH_THRESHOLD = 10;
+const ARTICLE_LENGTH_THRESHOLD = 14;
 const enableExperimentTopStories = ({
   isAmp,
   service,
-  id,
   blocksLength,
 }: {
   isAmp: boolean;
   service: string;
-  id: string | null;
   blocksLength: number;
 }) => {
-  if (!isAmp || !service || !id || blocksLength < ARTICLE_LENGTH_THRESHOLD) {
+  if (!isAmp || !service || blocksLength < ARTICLE_LENGTH_THRESHOLD) {
     return false;
   }
-
-  const newsAsset = 'cz7xywn940ro';
-  const newsCPSAsset = 'news/world-europe-60506682';
-  const newsShortAsset = 'cd4117egk3go';
-  const newsOneColumnAsset = 'c99vz4kz5vzo';
-  const newsTestAsset = 'c6v11qzyv8po';
-  const newsTestBreakingNewsAsset = 'cgx1znpjjx7o';
-  const sportAsset = 'c2vwq901e93o';
-  const sportShortAsset = 'cpgw0xjmpd3o';
-  const sportOneColumnAsset = 'c4ngy9xjpzro';
-  const cymrufywAsset = 'ckg080e0d1eo';
-
-  const experimentAssets = [
-    newsAsset,
-    newsCPSAsset,
-    newsShortAsset,
-    newsOneColumnAsset,
-    newsTestAsset,
-    newsTestBreakingNewsAsset,
-    sportAsset,
-    sportShortAsset,
-    sportOneColumnAsset,
-    cymrufywAsset,
-  ];
   const experimentServices = ['news', 'sport'];
-
-  return (
-    isAmp &&
-    id &&
-    experimentServices.includes(service) &&
-    experimentAssets.includes(id)
-  );
+  return isAmp && experimentServices.includes(service);
 };
 
 const insertBlockAtPosition = (
@@ -123,18 +91,15 @@ export const getExperimentTopStories = ({
   topStoriesContent,
   isAmp,
   service,
-  id,
 }: {
   blocks: OptimoBlock[];
   topStoriesContent: TopStoryItem[] | undefined;
   isAmp: boolean;
   service: string;
-  id: string | null;
 }) => {
   const shouldEnableExperimentTopStories = enableExperimentTopStories({
     isAmp,
     service,
-    id,
     blocksLength: blocks.length,
   });
 

diff --git a/src/app/pages/ArticlePage/fixtureData.ts b/src/app/pages/ArticlePage/fixtureData.ts
@@ -277,6 +277,11 @@ export const articleDataNewsLongLength = articleDataBuilder(
     'A paragraph.',
     'A paragraph.',
     'A paragraph.',
+    'A paragraph.',
+    'A paragraph.',
+    'A paragraph.',
+    'A paragraph.',
+    'A paragraph.',
   ],
   'Article Headline for SEO',
   'Article Headline for Promo',

diff --git a/src/app/pages/ArticlePage/index.test.tsx b/src/app/pages/ArticlePage/index.test.tsx
@@ -927,7 +927,7 @@ describe('Article Page', () => {
       ${'news'}  | ${validNewsAsset}
       ${'sport'} | ${validSportAsset}
     `(
-      'should render page with experiment-top-stories blocks only on specific $service assets that are long enough',
+      'should render page with experiment-top-stories blocks on $service assets that are long enough',
       ({ service, id }) => {
         const { queryByTestId } = renderAmpPage({
           service,
@@ -947,11 +947,10 @@ describe('Article Page', () => {
     );
 
     it.each`
-      service     | id                | isShortArticle | testDescription
-      ${'news'}   | ${'c1231qzyv8po'} | ${false}       | ${'news assets not specified'}
-      ${'sport'}  | ${'c1231qzyv8po'} | ${false}       | ${'sport assets not specified'}
-      ${'pidgin'} | ${'c6v11qzyv8po'} | ${false}       | ${`services which are not 'news' or 'sport'`}
-      ${'news'}   | ${'c6v11qzyv8po'} | ${true}        | ${'valid asset is too short'}
+      service     | id                 | isShortArticle | testDescription
+      ${'pidgin'} | ${validNewsAsset}  | ${false}       | ${`services which are not 'news' or 'sport'`}
+      ${'news'}   | ${validNewsAsset}  | ${true}        | ${`'news' assets that are too short`}
+      ${'sport'}  | ${validSportAsset} | ${true}        | ${`'sport' assets that are too short`}
     `(
       'should render page without experiment-top-stories blocks on $testDescription',
       ({ service, id, isShortArticle }) => {
@@ -973,7 +972,7 @@ describe('Article Page', () => {
       },
     );
 
-    it('should add ampExperimentName to atiAnalytics on valid services and assets', async () => {
+    it('should add ampExperimentName to ATIAnalytics on valid services', async () => {
       (ATIAnalytics as jest.Mock).mockImplementation(() => <div />);
       const service = 'news';
       const id = validNewsAsset;