Refactor helper functions

bbc · Oct 15, 2024 · 68d5932 · 68d5932
1 parent cf7a1f1
commit 68d5932
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 66 deletions.
diff --git a/src/app/pages/ArticlePage/ArticlePage.tsx b/src/app/pages/ArticlePage/ArticlePage.tsx
@@ -65,8 +65,7 @@ import { ComponentToRenderProps, TimeStampProps } from './types';
 import AmpExperiment from '../../components/AmpExperiment';
 import {
   experimentTopStoriesConfig,
-  enableExperimentTopStories,
-  insertExperimentTopStories,
+  getExperimentTopStoriesBlocks,
   ExperimentTopStories,
 } from './experimentTopStories/helpers';
 
@@ -139,18 +138,14 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => {
   };
 
   const topStoriesContent = pageData?.secondaryColumn?.topStories;
-  const shouldEnableExperimentTopStories =
-    enableExperimentTopStories({
+  const { shouldEnableExperimentTopStories, transformedBlocks } =
+    getExperimentTopStoriesBlocks({
+      blocks,
+      topStoriesContent,
       isAmp,
       service,
       id,
-    }) && topStoriesContent;
-  const blocksWithExperimentTopStories = shouldEnableExperimentTopStories
-    ? insertExperimentTopStories({
-        blocks,
-        topStoriesContent,
-      })
-    : blocks;
+    });
 
   const componentsToRender = {
     visuallyHiddenHeadline,
@@ -196,7 +191,7 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => {
     ),
     podcastPromo: () => (podcastPromoEnabled ? <InlinePodcastPromo /> : null),
     experimentTopStories: () =>
-      shouldEnableExperimentTopStories ? (
+      topStoriesContent ? (
         <ExperimentTopStories topStoriesContent={topStoriesContent} />
       ) : null,
   };
@@ -208,8 +203,8 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => {
   };
 
   const articleBlocks = startsWithHeading
-    ? blocksWithExperimentTopStories
-    : [visuallyHiddenBlock, ...blocksWithExperimentTopStories];
+    ? transformedBlocks
+    : [visuallyHiddenBlock, ...transformedBlocks];
 
   const promoImageBlocks =
     pageData?.promo?.images?.defaultPromoImage?.blocks ?? [];

diff --git a/src/app/pages/ArticlePage/experimentTopStories/helpers.test.tsx b/src/app/pages/ArticlePage/experimentTopStories/helpers.test.tsx
@@ -1,57 +1,60 @@
-import {
-  enableExperimentTopStories,
-  insertExperimentTopStories,
-} from './helpers';
+import { getExperimentTopStoriesBlocks } 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({
+  const mockTextBlock = {
+    type: 'text',
+    model: {
+      blocks: [],
+    },
+  };
+  const expectedExperimentTopStoriesBlock = {
+    type: 'experimentTopStories',
+    model: topStoriesList,
+    id: 'experimentTopStories-1',
+  };
+
+  const blocksEvenLength = [mockTextBlock, mockTextBlock];
+  const blocksOddLength = [mockTextBlock, mockTextBlock, mockTextBlock];
+
+  describe('getExperimentTopStoriesBlocks()', () => {
+    it('returns shouldEnableExperimentTopStories as true if props match conditions.', () => {
+      const { shouldEnableExperimentTopStories } =
+        getExperimentTopStoriesBlocks({
+          blocks: blocksEvenLength,
+          topStoriesContent: topStoriesList,
           isAmp: true,
           id: 'c6v11qzyv8po',
           service: 'news',
-        }),
-      ).toBe(true);
+        });
+      expect(shouldEnableExperimentTopStories).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, id, service }) => {
-      expect(
-        enableExperimentTopStories({
-          isAmp,
-          id,
-          service,
-        }),
-      ).toBe(false);
-    });
-  });
+      testDescription                                | isAmp    | id                | 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} | ${'c6v11qzyv8po'} | ${undefined}
+      ${'all props defined but pathname is invalid'} | ${false} | ${'c1231qzyv8po'} | ${undefined}
+      ${'only service is undefined'}                 | ${true}  | ${'c6v11qzyv8po'} | ${undefined}
+      ${'only service is defined and valid'}         | ${false} | ${undefined}      | ${'news'}
+      ${'all props defined but service is invalid'}  | ${true}  | ${'c6v11qzyv8po'} | ${'igbo'}
+    `(
+      'returns shouldEnableExperimentTopStories as false because $testDescription.',
+      ({ isAmp, id, service }) => {
+        const { shouldEnableExperimentTopStories } =
+          getExperimentTopStoriesBlocks({
+            blocks: blocksEvenLength,
+            topStoriesContent: topStoriesList,
+            isAmp,
+            id,
+            service,
+          });
 
-  describe('insertExperimentTopStories()', () => {
-    const mockTextBlock = {
-      type: 'text',
-      model: {
-        blocks: [],
+        expect(shouldEnableExperimentTopStories).toBe(false);
       },
-    };
-    const expectedExperimentTopStoriesBlock = {
-      type: 'experimentTopStories',
-      model: topStoriesList,
-      id: 'experimentTopStories-1',
-    };
-
-    const blocksEvenLength = [mockTextBlock, mockTextBlock];
-    const blocksOddLength = [mockTextBlock, mockTextBlock, mockTextBlock];
+    );
 
     const expectedBlocksEvenLength = [
       mockTextBlock,
@@ -72,12 +75,14 @@ describe('AMP top stories experiment', () => {
     `(
       '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);
+        const { transformedBlocks } = getExperimentTopStoriesBlocks({
+          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
@@ -14,16 +14,18 @@ export const experimentTopStoriesConfig = {
   },
 };
 
-export const enableExperimentTopStories = ({
+const enableExperimentTopStories = ({
+  topStoriesContent,
   isAmp,
   service,
   id,
 }: {
+  topStoriesContent: TopStoryItem[] | undefined;
   isAmp: boolean;
   service: string;
   id: string | null;
 }) => {
-  if (!isAmp || !service || !id) return false;
+  if (!topStoriesContent || !isAmp || !service || !id) return false;
   const newsTestAsset = 'c6v11qzyv8po';
   const newsAsset = 'cz7xywn940ro';
   const sportAsset = 'cpgw0xjmpd3o';
@@ -38,7 +40,7 @@ export const enableExperimentTopStories = ({
   );
 };
 
-export const insertExperimentTopStories = ({
+const insertExperimentTopStories = ({
   blocks,
   topStoriesContent,
 }: {
@@ -57,6 +59,43 @@ export const insertExperimentTopStories = ({
   return blocksClone;
 };
 
+export const getExperimentTopStoriesBlocks = ({
+  blocks,
+  topStoriesContent,
+  isAmp,
+  service,
+  id,
+}: {
+  blocks: OptimoBlock[];
+  topStoriesContent: TopStoryItem[] | undefined;
+  isAmp: boolean;
+  service: string;
+  id: string | null;
+}) => {
+  if (!topStoriesContent)
+    return {
+      transformedBlocks: blocks,
+      shouldEnableExperimentTopStories: false,
+    };
+
+  const shouldEnableExperimentTopStories = enableExperimentTopStories({
+    topStoriesContent,
+    isAmp,
+    service,
+    id,
+  });
+
+  const transformedBlocks = insertExperimentTopStories({
+    blocks,
+    topStoriesContent,
+  });
+
+  return {
+    transformedBlocks,
+    shouldEnableExperimentTopStories,
+  };
+};
+
 export const ExperimentTopStories = ({
   topStoriesContent,
 }: {