From 741d8ee4cb0b068147c4e427858f6d5144c9ca7a Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Mon, 22 May 2023 18:49:51 +0100 Subject: [PATCH 01/17] Move getRemoteData script into separate function --- .../Amp/getRemoteDataScript/index.test.ts | 0 .../MostRead/Amp/getRemoteDataScript/index.ts | 44 +++++++++++++++++++ src/app/components/MostRead/Amp/index.tsx | 41 ++--------------- 3 files changed, 47 insertions(+), 38 deletions(-) create mode 100644 src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts create mode 100644 src/app/components/MostRead/Amp/getRemoteDataScript/index.ts diff --git a/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts b/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts b/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts new file mode 100644 index 00000000000..7eae6d9ddfb --- /dev/null +++ b/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts @@ -0,0 +1,44 @@ +import { Services } from '../../../../models/types/global'; +import { serviceNumerals } from '../../Canonical/Rank'; + +export default ({ + endpoint, + service, +}: { + endpoint: string; + service: Services; +}) => { + const translation = serviceNumerals(service); + + return ` + const translations = ${JSON.stringify(translation)} + const getRemoteData = async () => { + try{ + const response = await fetch("${endpoint}"); + const data = await response.json(); + + if(data.records.length === 0){ + throw new Error("Empty records from mostread endpoint"); + } + + data.records.forEach((item, index) => { + item.rankTranslation = translations[index+1]; + + if (!item.promo.headlines.shortHeadline) { + item.promo.headlines.shortHeadline = item.promo.headlines.seoHeadline; + } + + if(!item.promo.locators.assetUri) { + item.promo.locators.assetUri = item.promo.locators.canonicalUrl; + } + + }); + + return data; + } catch(error){ + console.warn(error); + return []; + } + } + exportFunction('getRemoteData', getRemoteData);`; +}; diff --git a/src/app/components/MostRead/Amp/index.tsx b/src/app/components/MostRead/Amp/index.tsx index 8d9a035d018..78cc6ab0f7e 100644 --- a/src/app/components/MostRead/Amp/index.tsx +++ b/src/app/components/MostRead/Amp/index.tsx @@ -10,46 +10,11 @@ import { import pathOr from 'ramda/src/pathOr'; import { ServiceContext } from '../../../contexts/ServiceContext'; import { MostReadItemWrapper, MostReadLink } from '../Canonical/Item'; -import MostReadRank, { serviceNumerals } from '../Canonical/Rank'; +import MostReadRank from '../Canonical/Rank'; import generateCSPHash from '../utilities/generateCSPHash'; -import { Services } from '../../../models/types/global'; import { Size, Direction } from '../types'; import styles from './index.styles'; - -const rankTranslationScript = (endpoint: string, service: Services) => { - const translation = serviceNumerals(service); - return ` - const translations = ${JSON.stringify(translation)} - const getRemoteData = async () => { - try{ - const response = await fetch("${endpoint}"); - const data = await response.json(); - - if(data.records.length === 0){ - throw new Error("Empty records from mostread endpoint"); - } - - data.records.forEach((item, index) => { - item.rankTranslation = translations[index+1]; - - if (!item.promo.headlines.shortHeadline) { - item.promo.headlines.shortHeadline = item.promo.headlines.seoHeadline; - } - - if(!item.promo.locators.assetUri) { - item.promo.locators.assetUri = item.promo.locators.canonicalUrl; - } - - }); - - return data; - } catch(error){ - console.warn(error); - return []; - } - } - exportFunction('getRemoteData', getRemoteData);`; -}; +import getRemoteDataScript from './getRemoteDataScript'; interface AmpMostReadProps { endpoint: string; @@ -64,7 +29,7 @@ const AmpMostRead = ({ endpoint, size = 'default' }: AmpMostReadProps) => { translations, } = useContext(ServiceContext); - const onlyinnerscript = rankTranslationScript(endpoint, service); + const onlyinnerscript = getRemoteDataScript({ endpoint, service }); const fallbackText = pathOr( 'Content is not available', From 03a0b2239e0ba1925e359dc579f8131049f13174 Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Mon, 22 May 2023 19:58:11 +0100 Subject: [PATCH 02/17] Implement innerscript as a testable function --- .../Amp/getRemoteDataScript/index.test.ts | 19 ++++++ .../MostRead/Amp/getRemoteDataScript/index.ts | 60 ++++++++++--------- 2 files changed, 51 insertions(+), 28 deletions(-) diff --git a/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts b/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts index e69de29bb2d..42ad0df3805 100644 --- a/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts +++ b/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts @@ -0,0 +1,19 @@ +import fetchMock from 'fetch-mock'; +import getRemoteDataScript, { getInnerScript } from '.'; +import { WesternArabic } from '../../../../legacy/psammead/psammead-locales/src/numerals'; +import pidginMostRead from '../../../../../../data/pidgin/mostRead/index.json'; + +describe('getRemoteDataScript', () => { + beforeEach(() => { + jest.clearAllMocks(); + fetchMock.restore(); + }); + + it('getScriptContents should do stuff', async () => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const data = JSON.parse(JSON.stringify(pidginMostRead)); + + // eslint-disable-next-line no-eval + console.log(eval(getInnerScript(data))); + }); +}); diff --git a/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts b/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts index 7eae6d9ddfb..e07e11f7887 100644 --- a/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts +++ b/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts @@ -1,6 +1,22 @@ import { Services } from '../../../../models/types/global'; import { serviceNumerals } from '../../Canonical/Rank'; +// eslint-disable-next-line @typescript-eslint/no-unused-vars +export const getInnerScript = (_data?: object) => ` + if (data.items.length === 0) { + throw new Error('Empty records from mostread endpoint'); + } + + data.items.map((item, index) => { + return { + ...item, + rankTranslation: translations[index + 1], + }; + }); + + return data; + `; + export default ({ endpoint, service, @@ -8,37 +24,25 @@ export default ({ endpoint: string; service: Services; }) => { - const translation = serviceNumerals(service); - - return ` - const translations = ${JSON.stringify(translation)} - const getRemoteData = async () => { - try{ - const response = await fetch("${endpoint}"); - const data = await response.json(); + // @ts-expect-error only a subset of services need to override the default service numerals + const translations = serviceNumerals[service]; - if(data.records.length === 0){ - throw new Error("Empty records from mostread endpoint"); - } + const innerscript = getInnerScript(); - data.records.forEach((item, index) => { - item.rankTranslation = translations[index+1]; - - if (!item.promo.headlines.shortHeadline) { - item.promo.headlines.shortHeadline = item.promo.headlines.seoHeadline; - } - - if(!item.promo.locators.assetUri) { - item.promo.locators.assetUri = item.promo.locators.canonicalUrl; - } + return ` + const translations = JSON.stringify(${translations}); + try { + const response = await fetch("${endpoint}"); + const data = await response.json(); - }); + ${innerscript} - return data; - } catch(error){ - console.warn(error); - return []; - } + return data; + } catch (error) { + console.warn(error); + return []; } - exportFunction('getRemoteData', getRemoteData);`; + + exportFunction('getRemoteData', getRemoteData); + `; }; From 674b15f2a814dea2065b6489cd68e2fdde58ae4b Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Mon, 22 May 2023 20:31:36 +0100 Subject: [PATCH 03/17] Updating amp inner script functionality --- .../Amp/getRemoteDataScript/index.test.ts | 36 +++++++++++++------ .../MostRead/Amp/getRemoteDataScript/index.ts | 10 +++--- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts b/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts index 42ad0df3805..ad4af69a64c 100644 --- a/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts +++ b/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts @@ -1,19 +1,35 @@ -import fetchMock from 'fetch-mock'; -import getRemoteDataScript, { getInnerScript } from '.'; +/* eslint-disable no-eval */ +import { getInnerScript } from '.'; import { WesternArabic } from '../../../../legacy/psammead/psammead-locales/src/numerals'; import pidginMostRead from '../../../../../../data/pidgin/mostRead/index.json'; describe('getRemoteDataScript', () => { - beforeEach(() => { - jest.clearAllMocks(); - fetchMock.restore(); + it('getInnerScript should append rankTranslation to each item', async () => { + const data = { + ...pidginMostRead, + items: pidginMostRead.records.slice(0, 10), + }; + expect(data).toHaveProperty('items'); + + const translations = WesternArabic; + + const response = eval(getInnerScript()); + + response.forEach((item: { rankTranslation: number }, index: number) => { + expect(item).toHaveProperty('rankTranslation'); + expect(item.rankTranslation).toBe(translations[index + 1]); + }); }); - it('getScriptContents should do stuff', async () => { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const data = JSON.parse(JSON.stringify(pidginMostRead)); + it('getInnerScript should throw an error if there are no items in the data response', async () => { + const data = pidginMostRead; + expect(data).not.toHaveProperty('items'); + + const translations = WesternArabic; + expect(translations).toBeDefined(); - // eslint-disable-next-line no-eval - console.log(eval(getInnerScript(data))); + expect(() => eval(getInnerScript())).toThrowError( + 'Empty records from mostread endpoint', + ); }); }); diff --git a/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts b/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts index e07e11f7887..0e3b48bd02d 100644 --- a/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts +++ b/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts @@ -1,9 +1,9 @@ import { Services } from '../../../../models/types/global'; import { serviceNumerals } from '../../Canonical/Rank'; -// eslint-disable-next-line @typescript-eslint/no-unused-vars -export const getInnerScript = (_data?: object) => ` - if (data.items.length === 0) { +export const getInnerScript = () => { + return ` + if (!data.items || (data.items && data.items.length === 0)) { throw new Error('Empty records from mostread endpoint'); } @@ -13,9 +13,8 @@ export const getInnerScript = (_data?: object) => ` rankTranslation: translations[index + 1], }; }); - - return data; `; +}; export default ({ endpoint, @@ -31,6 +30,7 @@ export default ({ return ` const translations = JSON.stringify(${translations}); + try { const response = await fetch("${endpoint}"); const data = await response.json(); From 2c66d9d7774265f2f6988d594d30d6c13f77b542 Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Mon, 22 May 2023 21:07:19 +0100 Subject: [PATCH 04/17] Update tests & implementation of getRemoteDataScript to cater for current most read response --- .../Amp/getRemoteDataScript/index.test.ts | 16 +++---- .../MostRead/Amp/getRemoteDataScript/index.ts | 42 +++++++++---------- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts b/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts index ad4af69a64c..a166e5520bf 100644 --- a/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts +++ b/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts @@ -1,10 +1,11 @@ /* eslint-disable no-eval */ -import { getInnerScript } from '.'; +import { omit } from 'ramda'; +import { transformData } from '.'; import { WesternArabic } from '../../../../legacy/psammead/psammead-locales/src/numerals'; import pidginMostRead from '../../../../../../data/pidgin/mostRead/index.json'; describe('getRemoteDataScript', () => { - it('getInnerScript should append rankTranslation to each item', async () => { + it('transformData should append rankTranslation to each item', async () => { const data = { ...pidginMostRead, items: pidginMostRead.records.slice(0, 10), @@ -13,22 +14,23 @@ describe('getRemoteDataScript', () => { const translations = WesternArabic; - const response = eval(getInnerScript()); + eval(transformData()); - response.forEach((item: { rankTranslation: number }, index: number) => { + data.items.forEach((item: object, index: number) => { expect(item).toHaveProperty('rankTranslation'); + // @ts-expect-error required for testing purposes expect(item.rankTranslation).toBe(translations[index + 1]); }); }); - it('getInnerScript should throw an error if there are no items in the data response', async () => { - const data = pidginMostRead; + it('transformData should throw an error if there are no items in the data response', async () => { + const data = omit(['records'], pidginMostRead); expect(data).not.toHaveProperty('items'); const translations = WesternArabic; expect(translations).toBeDefined(); - expect(() => eval(getInnerScript())).toThrowError( + expect(() => eval(transformData())).toThrowError( 'Empty records from mostread endpoint', ); }); diff --git a/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts b/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts index 0e3b48bd02d..31207f4ace7 100644 --- a/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts +++ b/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts @@ -1,17 +1,16 @@ import { Services } from '../../../../models/types/global'; import { serviceNumerals } from '../../Canonical/Rank'; -export const getInnerScript = () => { +export const transformData = () => { return ` + data.items = data.records; + if (!data.items || (data.items && data.items.length === 0)) { throw new Error('Empty records from mostread endpoint'); } - data.items.map((item, index) => { - return { - ...item, - rankTranslation: translations[index + 1], - }; + data.items.forEach((item, index) => { + item.rankTranslation = translations[index + 1]; }); `; }; @@ -23,24 +22,21 @@ export default ({ endpoint: string; service: Services; }) => { - // @ts-expect-error only a subset of services need to override the default service numerals - const translations = serviceNumerals[service]; - - const innerscript = getInnerScript(); - return ` - const translations = JSON.stringify(${translations}); - - try { - const response = await fetch("${endpoint}"); - const data = await response.json(); - - ${innerscript} - - return data; - } catch (error) { - console.warn(error); - return []; + const translations = ${JSON.stringify(serviceNumerals(service))} + + const getRemoteData = async () => { + try { + const response = await fetch("${endpoint}"); + const data = await response.json(); + + ${transformData()} + + return data; + } catch (error) { + console.warn(error); + return []; + } } exportFunction('getRemoteData', getRemoteData); From 4cd80374ec3182ab52541915dc0547746a24047c Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Mon, 22 May 2023 21:07:36 +0100 Subject: [PATCH 05/17] Use items instead of records --- .../components/MostRead/Amp/__snapshots__/index.test.tsx.snap | 2 +- src/app/components/MostRead/Amp/index.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/components/MostRead/Amp/__snapshots__/index.test.tsx.snap b/src/app/components/MostRead/Amp/__snapshots__/index.test.tsx.snap index 0ff2f50732b..f695a4db661 100644 --- a/src/app/components/MostRead/Amp/__snapshots__/index.test.tsx.snap +++ b/src/app/components/MostRead/Amp/__snapshots__/index.test.tsx.snap @@ -227,7 +227,7 @@ exports[`AmpMostRead should render as expected 1`] = ` > { Date: Tue, 23 May 2023 09:04:57 +0100 Subject: [PATCH 06/17] Tighten logic --- src/app/components/MostRead/Amp/getRemoteDataScript/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts b/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts index 31207f4ace7..8c20803455c 100644 --- a/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts +++ b/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts @@ -3,9 +3,9 @@ import { serviceNumerals } from '../../Canonical/Rank'; export const transformData = () => { return ` - data.items = data.records; + data.items = data.records || []; - if (!data.items || (data.items && data.items.length === 0)) { + if (data.items.length === 0) { throw new Error('Empty records from mostread endpoint'); } From 97070a4274c956f75fa34338bf9fcab246473407 Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Tue, 23 May 2023 10:30:45 +0100 Subject: [PATCH 07/17] Add capability to call the BFF for most read data --- .../getUrlHelpers/getMostReadUrls/index.js | 17 ++++++++++---- .../getMostReadUrls/index.test.js | 23 +++++++++++-------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/app/lib/utilities/getUrlHelpers/getMostReadUrls/index.js b/src/app/lib/utilities/getUrlHelpers/getMostReadUrls/index.js index 9e249de79aa..08907961264 100644 --- a/src/app/lib/utilities/getUrlHelpers/getMostReadUrls/index.js +++ b/src/app/lib/utilities/getUrlHelpers/getMostReadUrls/index.js @@ -1,9 +1,16 @@ -export const getMostReadEndpoint = ({ service, variant }) => - variant - ? `/${service}/mostread/${variant}.json` - : `/${service}/mostread.json`; - export const getLocalMostReadEndpoint = ({ service, variant = 'default' }) => `./data/${service}/mostRead/${ variant === 'default' ? 'index' : variant }.json`; + +export const getMostReadEndpoint = ({ service, variant, isBff = false }) => { + if (isBff) { + return variant + ? `/fd/simorgh-bff?pageType=mostRead&service=${service}&variant=${variant}` + : `/fd/simorgh-bff?pageType=mostRead&service=${service}`; + } + + return variant + ? `/${service}/mostread/${variant}.json` + : `/${service}/mostread.json`; +}; diff --git a/src/app/lib/utilities/getUrlHelpers/getMostReadUrls/index.test.js b/src/app/lib/utilities/getUrlHelpers/getMostReadUrls/index.test.js index 15764a79e57..234f52312c8 100644 --- a/src/app/lib/utilities/getUrlHelpers/getMostReadUrls/index.test.js +++ b/src/app/lib/utilities/getUrlHelpers/getMostReadUrls/index.test.js @@ -1,17 +1,20 @@ import { getMostReadEndpoint, getLocalMostReadEndpoint } from '.'; describe('getMostReadEndpoint', () => { - it('should return endpoint when passed service', () => { - expect(getMostReadEndpoint({ service: 'hausa' })).toBe( - '/hausa/mostread.json', - ); - }); - it('should return endpoint when passed service and variant', () => { - expect(getMostReadEndpoint({ service: 'serbian', variant: 'lat' })).toBe( - '/serbian/mostread/lat.json', - ); - }); + it.each` + service | variant | isBff | expected + ${'hausa'} | ${null} | ${false} | ${'/hausa/mostread.json'} + ${'hausa'} | ${null} | ${true} | ${'/fd/simorgh-bff?pageType=mostRead&service=hausa'} + ${'serbian'} | ${'lat'} | ${false} | ${'/serbian/mostread/lat.json'} + ${'serbian'} | ${'lat'} | ${true} | ${'/fd/simorgh-bff?pageType=mostRead&service=serbian&variant=lat'} + `( + 'should return the correct endpoint when service is $service, variant is $variant and isBff is $isBff', + ({ isBff, service, variant, expected }) => { + expect(getMostReadEndpoint({ service, variant, isBff })).toBe(expected); + }, + ); }); + describe('getLocalMostReadEndpoint', () => { it('should return endpoint when passed service', () => { expect(getLocalMostReadEndpoint({ service: 'hausa' })).toBe( From ca224fb7e01c6f4cafe8ad172eeb0c0eca0fb0e2 Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Tue, 23 May 2023 10:31:11 +0100 Subject: [PATCH 08/17] Ensure that the correct endpoint is returned based on environment --- src/app/components/MostRead/index.test.tsx | 38 ++++++++++++++++++++++ src/app/components/MostRead/index.tsx | 5 +++ 2 files changed, 43 insertions(+) diff --git a/src/app/components/MostRead/index.test.tsx b/src/app/components/MostRead/index.test.tsx index 5001a68fa84..d8d48a46714 100644 --- a/src/app/components/MostRead/index.test.tsx +++ b/src/app/components/MostRead/index.test.tsx @@ -17,9 +17,13 @@ import { PageTypes, Services, Variants } from '../../models/types/global'; import Canonical from './Canonical'; import Amp from './Amp'; import { MostReadData } from './types'; +import isLocal from '../../lib/utilities/isLocal'; jest.mock('./Canonical'); jest.mock('./Amp'); +jest.mock('../../lib/utilities/isLocal', () => + jest.fn().mockImplementation(() => true), +); interface MostReadProps { isAmp: boolean; @@ -244,5 +248,39 @@ describe('MostRead', () => { }); }, ); + + describe('endpoint', () => { + it.each` + service | variant | isLocalEnv | endpoint + ${'pidgin'} | ${null} | ${true} | ${'/pidgin/mostread.json'} + ${'pidgin'} | ${null} | ${false} | ${'/fd/simorgh-bff?pageType=mostRead&service=pidgin'} + ${'serbian'} | ${'cyr'} | ${true} | ${'/serbian/mostread/cyr.json'} + ${'serbian'} | ${'cyr'} | ${false} | ${'/fd/simorgh-bff?pageType=mostRead&service=serbian&variant=cyr'} + `( + 'should be $endpoint when service is $service, variant is $variant and isLocalEnv is $isLocalEnv', + async ({ service, variant, isLocalEnv, endpoint }) => { + // @ts-expect-error need to mock isLocal function for testing purposes + isLocal.mockImplementationOnce(() => isLocalEnv); + + render( + , + ); + + expect(Amp).toHaveBeenCalledWith( + expect.objectContaining({ + endpoint: expect.stringContaining(endpoint), + }), + {}, + ); + }, + ); + }); }); }); diff --git a/src/app/components/MostRead/index.tsx b/src/app/components/MostRead/index.tsx index ed835685748..182613d2867 100644 --- a/src/app/components/MostRead/index.tsx +++ b/src/app/components/MostRead/index.tsx @@ -9,6 +9,7 @@ import { ColumnLayout, Size, MostReadData } from './types'; import MostReadSection from './Section'; import MostReadSectionLabel from './Label'; import { WHITE } from '../ThemeProvider/palette'; +import isLocal from '../../lib/utilities/isLocal'; const blockLevelEventTrackingData = { componentName: 'most-read', @@ -46,9 +47,13 @@ const MostRead = ({ return null; } + // If not in local environment, use the BFF, otherwise use fixture data + const isBff = !isLocal(); + const endpoint = getMostReadEndpoint({ service, variant, + isBff, }); // We render amp on ONLY STY, CSP and ARTICLE pages using amp-list. From 97a97271de448444eb07ac8426cdbb8dd74418f6 Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Thu, 15 Jun 2023 15:03:37 +0100 Subject: [PATCH 09/17] Mostread.json endpoint should temorarily point to the simplified most read BFF response --- src/server/local/constructDataFilePath/index.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/server/local/constructDataFilePath/index.js b/src/server/local/constructDataFilePath/index.js index 1aaff010eb4..5d9c978155e 100644 --- a/src/server/local/constructDataFilePath/index.js +++ b/src/server/local/constructDataFilePath/index.js @@ -12,9 +12,11 @@ export default ({ let dataPath; switch (pageType) { + case 'mostRead': + dataPath = `${variant || service}.json`; + break; case 'frontpage': case 'homePage': - case 'mostRead': case 'mostWatched': case 'secondaryColumn': case 'recommendations': From 11951cabec602bd63a2fbb651625898708a2efe0 Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Thu, 15 Jun 2023 15:09:11 +0100 Subject: [PATCH 10/17] Fix getRemoteData logic - no longer reference records --- .../Amp/getRemoteDataScript/index.test.ts | 15 +++++---------- .../MostRead/Amp/getRemoteDataScript/index.ts | 4 +--- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts b/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts index a166e5520bf..19984c8a588 100644 --- a/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts +++ b/src/app/components/MostRead/Amp/getRemoteDataScript/index.test.ts @@ -1,16 +1,11 @@ /* eslint-disable no-eval */ -import { omit } from 'ramda'; import { transformData } from '.'; import { WesternArabic } from '../../../../legacy/psammead/psammead-locales/src/numerals'; -import pidginMostRead from '../../../../../../data/pidgin/mostRead/index.json'; +import { data as pidginMostRead } from '../../../../../../data/pidgin/mostRead/pidgin.json'; describe('getRemoteDataScript', () => { it('transformData should append rankTranslation to each item', async () => { - const data = { - ...pidginMostRead, - items: pidginMostRead.records.slice(0, 10), - }; - expect(data).toHaveProperty('items'); + const data = pidginMostRead; const translations = WesternArabic; @@ -23,9 +18,9 @@ describe('getRemoteDataScript', () => { }); }); - it('transformData should throw an error if there are no items in the data response', async () => { - const data = omit(['records'], pidginMostRead); - expect(data).not.toHaveProperty('items'); + it('transformData should throw an error if there are empty items in the data response', async () => { + const data = { ...pidginMostRead, items: [] }; + expect(data.items).toHaveLength(0); const translations = WesternArabic; expect(translations).toBeDefined(); diff --git a/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts b/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts index 8c20803455c..87a272cf10e 100644 --- a/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts +++ b/src/app/components/MostRead/Amp/getRemoteDataScript/index.ts @@ -3,8 +3,6 @@ import { serviceNumerals } from '../../Canonical/Rank'; export const transformData = () => { return ` - data.items = data.records || []; - if (data.items.length === 0) { throw new Error('Empty records from mostread endpoint'); } @@ -28,7 +26,7 @@ export default ({ const getRemoteData = async () => { try { const response = await fetch("${endpoint}"); - const data = await response.json(); + const { data } = await response.json(); ${transformData()} From 5271509e6a02eeb1acf68ad5c5cd185755c20644 Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Thu, 15 Jun 2023 15:09:33 +0100 Subject: [PATCH 11/17] Replace title and href placeholders --- src/app/components/MostRead/Amp/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/components/MostRead/Amp/index.tsx b/src/app/components/MostRead/Amp/index.tsx index 715bd58f588..23ca0e10402 100644 --- a/src/app/components/MostRead/Amp/index.tsx +++ b/src/app/components/MostRead/Amp/index.tsx @@ -92,8 +92,8 @@ const AmpMostRead = ({ endpoint, size = 'default' }: AmpMostReadProps) => { From ea0962b560890869d598c43b19ba38cd400d6962 Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Thu, 15 Jun 2023 15:16:52 +0100 Subject: [PATCH 12/17] Fix type error --- src/app/components/MostRead/index.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/components/MostRead/index.test.tsx b/src/app/components/MostRead/index.test.tsx index e7949d36ac9..78f2be4ac31 100644 --- a/src/app/components/MostRead/index.test.tsx +++ b/src/app/components/MostRead/index.test.tsx @@ -2,7 +2,7 @@ import React from 'react'; import isLive from '#app/lib/utilities/isLive'; import { RequestContextProvider } from '../../contexts/RequestContext'; import { ToggleContextProvider } from '../../contexts/ToggleContext'; -import pidginMostReadData from '../../../../data/pidgin/mostRead/pidgin.json'; +import { data as pidginMostReadData } from '../../../../data/pidgin/mostRead/pidgin.json'; import serbianLatMostReadData from '../../../../data/serbian/mostRead/lat.json'; import { FRONT_PAGE, @@ -301,7 +301,7 @@ describe('MostRead', () => { // if isLive is true, DO NOT show most read component const { container } = render( - , + , { toggles }, ); @@ -314,7 +314,7 @@ describe('MostRead', () => { // if isLive is false, show most read component const { container } = render( - , + , { toggles }, ); expect(container).not.toBeEmptyDOMElement(); From a5f5acbc4b044351eaa7f6eeaea41ff936d1b368 Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Thu, 15 Jun 2023 15:21:10 +0100 Subject: [PATCH 13/17] Reinstate order of functions --- .../utilities/getUrlHelpers/getMostReadUrls/index.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/app/lib/utilities/getUrlHelpers/getMostReadUrls/index.js b/src/app/lib/utilities/getUrlHelpers/getMostReadUrls/index.js index 08907961264..4dcd577f9d6 100644 --- a/src/app/lib/utilities/getUrlHelpers/getMostReadUrls/index.js +++ b/src/app/lib/utilities/getUrlHelpers/getMostReadUrls/index.js @@ -1,8 +1,3 @@ -export const getLocalMostReadEndpoint = ({ service, variant = 'default' }) => - `./data/${service}/mostRead/${ - variant === 'default' ? 'index' : variant - }.json`; - export const getMostReadEndpoint = ({ service, variant, isBff = false }) => { if (isBff) { return variant @@ -14,3 +9,8 @@ export const getMostReadEndpoint = ({ service, variant, isBff = false }) => { ? `/${service}/mostread/${variant}.json` : `/${service}/mostread.json`; }; + +export const getLocalMostReadEndpoint = ({ service, variant = 'default' }) => + `./data/${service}/mostRead/${ + variant === 'default' ? 'index' : variant + }.json`; From 1a11d09cd2476fc2c0eb24c2ce089c921aec4497 Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Thu, 15 Jun 2023 15:25:24 +0100 Subject: [PATCH 14/17] Revert "Mostread.json endpoint should temorarily point to the simplified most read BFF response" This reverts commit 97a97271de448444eb07ac8426cdbb8dd74418f6. --- src/server/local/constructDataFilePath/index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/server/local/constructDataFilePath/index.js b/src/server/local/constructDataFilePath/index.js index 5d9c978155e..1aaff010eb4 100644 --- a/src/server/local/constructDataFilePath/index.js +++ b/src/server/local/constructDataFilePath/index.js @@ -12,11 +12,9 @@ export default ({ let dataPath; switch (pageType) { - case 'mostRead': - dataPath = `${variant || service}.json`; - break; case 'frontpage': case 'homePage': + case 'mostRead': case 'mostWatched': case 'secondaryColumn': case 'recommendations': From e888d9a8692cdc383629ef99f5415853515efdda Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Thu, 15 Jun 2023 16:13:54 +0100 Subject: [PATCH 15/17] Adding mundo most read data --- data/mundo/mostRead/mundo.json | 150 +++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 data/mundo/mostRead/mundo.json diff --git a/data/mundo/mostRead/mundo.json b/data/mundo/mostRead/mundo.json new file mode 100644 index 00000000000..f4d64fd9d23 --- /dev/null +++ b/data/mundo/mostRead/mundo.json @@ -0,0 +1,150 @@ +{ + "data": { + "generated": "2023-06-15T14:59:17.801Z", + "lastRecordTimeStamp": "2023-06-15T14:57:00Z", + "firstRecordTimeStamp": "2023-06-15T14:42:00Z", + "items": [ + { + "id": "4adf2180-74e0-4ea9-836b-a00797fa0f48", + "rank": 1, + "title": "Quién es Valerii Zaluzhnyi, el popular \"general de hierro\" que está al mando de la contraofensiva ucraniana", + "href": "/mundo/noticias-internacional-65909204", + "timestamp": 1686821815000 + }, + { + "id": "7cd5b8ff-6587-4c92-917d-107aba6aa2d6", + "rank": 2, + "title": "El director de la morgue de la Universidad de Harvard es acusado de robar y vender partes de cadáveres humanos", + "href": "/mundo/noticias-65917580", + "timestamp": 1686832214000 + }, + { + "id": "b2c8768e-b7e9-448d-b163-e82f38d41f22", + "rank": 3, + "title": "Qué busca Bukele con la reducción de 262 a 44 municipios en El Salvador (y por qué causa polémica)", + "href": "/mundo/noticias-america-latina-65911008", + "timestamp": 1686826839000 + }, + { + "id": "781266b2-f28e-48f4-81a1-93fa29db1499", + "rank": 4, + "title": "Los estudios que muestran cómo la taurina puede alargar la vida", + "href": "/mundo/noticias-65863611", + "timestamp": 1686823420000 + }, + { + "id": "30ccf622-8542-40ed-b19d-81a77f38f0a1", + "rank": 5, + "title": "El inesperado peligro medioambiental que pueden generar los paneles solares", + "href": "/mundo/noticias-65815486", + "timestamp": 1686782133000 + }, + { + "id": "21331ae7-9b0b-4e93-a0ad-23b5ad254d32", + "rank": 6, + "title": "Qué es y qué queda del \"Tanomoshi\", el sistema que permitió prosperar a los japoneses que llegaron a Perú", + "href": "/mundo/noticias-america-latina-65851680", + "timestamp": 1686834397000 + }, + { + "id": "05a53fcc-a62e-4c25-95ad-0634e9969807", + "rank": 7, + "title": "Los contrastes de Colombia que revela la historia de los niños indígenas que estuvieron perdidos en la selva", + "href": "/mundo/noticias-america-latina-65899580", + "timestamp": 1686740548000 + }, + { + "id": "9bc341bf-308a-42d5-bb07-50c6cb7b90bf", + "rank": 8, + "title": "Muere la legendaria actriz inglesa Glenda Jackson, que inspiró al escritor argentino Julio Cortázar", + "href": "/mundo/noticias-65920533", + "timestamp": 1686839483000 + }, + { + "id": "0aa9876c-a4a4-4e88-9a5b-13ccea229a32", + "rank": 9, + "title": "Al menos 79 muertos y cientos de desaparecidos en el naufragio de un barco de migrantes frente a las costas de Grecia", + "href": "/mundo/noticias-internacional-65908697", + "timestamp": 1686801120000 + }, + { + "id": "1b2c3053-2fb3-4ffb-b0d0-ce1f868e47d2", + "rank": 10, + "title": "\"No se entiende que, con todo lo que recorrimos, encontráramos a los niños por donde ya habíamos pasado\"", + "href": "/mundo/noticias-america-latina-65886379", + "timestamp": 1686613016000 + }, + { + "id": "edbec380-d45e-491d-9d65-a7daa0eb28cd", + "rank": 11, + "title": "Quién es María Asunción Aramburuzabala, la mujer más rica de México", + "href": "/mundo/noticias-62897874", + "timestamp": 1664284329000 + }, + { + "id": "2a684580-b743-4180-8943-ca3f665e0f21", + "rank": 12, + "title": "\"Nos golpean día y noche sin parar\": las torturas y secuestros que sufren los migrantes que huyen del Talibán", + "href": "/mundo/noticias-internacional-65903588", + "timestamp": 1686797230000 + }, + { + "id": "0bd1529c-1339-48c1-9c06-c3d4a741e648", + "rank": 13, + "title": "Las incógnitas sobre quién heredará el imperio de US$6.500 millones de Silvio Berlusconi", + "href": "/mundo/noticias-internacional-65904977", + "timestamp": 1686748823000 + }, + { + "id": "30fa30ca-e382-4f92-b85d-9ffa56df4cc5", + "rank": 14, + "title": "3 áreas en las que la inteligencia artificial ya está mejorando nuestras vidas", + "href": "/mundo/noticias-65839988", + "timestamp": 1686735365000 + }, + { + "id": "e9301754-82ea-46e6-b8cc-e1cdaea0029f", + "rank": 15, + "title": "Investigación BBC | Los hombres que venden videos toqueteando a chicas en el metro de Japón (y cómo los están persiguiendo)", + "href": "/mundo/noticias-internacional-65861189", + "timestamp": 1686535346000 + }, + { + "id": "256c9c44-624a-47ac-8d48-e03908500b89", + "rank": 16, + "title": "\"La selva no era la amenaza, la selva los salvó\": ¿cómo pudieron sobrevivir los 4 niños que pasaron 40 días en la Amazonía colombiana?", + "href": "/mundo/noticias-america-latina-65869230", + "timestamp": 1686438275000 + }, + { + "id": "2534c64f-f53e-4af8-adcb-19d3eeeec700", + "rank": 17, + "title": "Françoise Gilot, la artista que amó y abandonó a Picasso (y que reveló el lado oscuro del genio)", + "href": "/mundo/noticias-65851130", + "timestamp": 1686482061000 + }, + { + "id": "fd1f78e8-9800-4905-b532-2dc5a6ad0b3b", + "rank": 18, + "title": "4 asombrosas historias de supervivencia y rescate que conmovieron al mundo", + "href": "/mundo/noticias-internacional-65885939", + "timestamp": 1686614917000 + }, + { + "id": "e1c294e0-90ba-425a-8d8e-2a89a1af5efa", + "rank": 19, + "title": "Por qué salir del clóset puede durar “toda la vida” para las personas LGBT+ (y cómo el apoyo familiar puede marcar la diferencia)", + "href": "/mundo/vert-fut-65891479", + "timestamp": 1686738366000 + }, + { + "id": "8edfba76-2d51-4261-aba1-fdb9d1800270", + "rank": 20, + "title": "Quién es el heredero de George Soros que tomará el control de su imperio de US$25.000 millones", + "href": "/mundo/noticias-internacional-65883449", + "timestamp": 1686588543000 + } + ] + }, + "contentType": "application/json; charset=utf-8" +} From 561414ae7c1e6636372381a8e0deddf48f926c1c Mon Sep 17 00:00:00 2001 From: karinathomasbbc Date: Thu, 15 Jun 2023 16:14:04 +0100 Subject: [PATCH 16/17] Fix amp most read tests --- .../Amp/__snapshots__/index.test.tsx.snap | 4 ++-- .../components/MostRead/Amp/index.test.tsx | 19 +++++++++++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/app/components/MostRead/Amp/__snapshots__/index.test.tsx.snap b/src/app/components/MostRead/Amp/__snapshots__/index.test.tsx.snap index f695a4db661..4f905dfb623 100644 --- a/src/app/components/MostRead/Amp/__snapshots__/index.test.tsx.snap +++ b/src/app/components/MostRead/Amp/__snapshots__/index.test.tsx.snap @@ -266,9 +266,9 @@ exports[`AmpMostRead should render as expected 1`] = ` > - {{promo.headlines.shortHeadline}} + {{title}} diff --git a/src/app/components/MostRead/Amp/index.test.tsx b/src/app/components/MostRead/Amp/index.test.tsx index 6f49b27c38f..ad161b79b3f 100644 --- a/src/app/components/MostRead/Amp/index.test.tsx +++ b/src/app/components/MostRead/Amp/index.test.tsx @@ -4,7 +4,7 @@ import { render, act } from '../../react-testing-library-with-providers'; import { ServiceContextProvider } from '../../../contexts/ServiceContext'; import AmpMostRead from '.'; import { Services } from '../../../models/types/global'; -import mundoMostReadResponse from '../../../../../data/mundo/mostRead/index.json'; +import { data as mundoMostReadResponse } from '../../../../../data/mundo/mostRead/mundo.json'; interface MostReadAmpWithContextProps { service: Services; @@ -45,6 +45,9 @@ describe('AmpMostRead', () => { const { container, getByText } = render( , + { + service: 'mundo', + }, ); await act(async () => { @@ -58,17 +61,19 @@ describe('AmpMostRead', () => { }); }); - it('should render fallback when fetch records are empty', async () => { + it('should render fallback when items are empty', async () => { fetchMock.mock('localhost:7080/mundo/mostread.json', { generated: '2022-05-03T14:44:35.496Z', lastRecordTimeStamp: '2022-05-03T14:42:00Z', firstRecordTimeStamp: '2022-05-03T14:27:00Z', - totalRecords: 20, - records: [], + items: [], }); const { container, getByText } = render( , + { + service: 'mundo', + }, ); await act(async () => { @@ -82,16 +87,18 @@ describe('AmpMostRead', () => { }); }); - it('should render fallback when fetch records is undefined', async () => { + it('should render fallback when items are undefined', async () => { fetchMock.mock('localhost:7080/mundo/mostread.json', { generated: '2022-05-03T14:44:35.496Z', lastRecordTimeStamp: '2022-05-03T14:42:00Z', firstRecordTimeStamp: '2022-05-03T14:27:00Z', - totalRecords: 20, }); const { container, getByText } = render( , + { + service: 'mundo', + }, ); await act(async () => { From 7d4107743b032e1e9cdcc03fdc5ba67fbdb68c4e Mon Sep 17 00:00:00 2001 From: Karina Thomas <58214768+karinathomasbbc@users.noreply.github.com> Date: Tue, 20 Jun 2023 09:28:34 +0100 Subject: [PATCH 17/17] Cast function to a jest.Mock --- src/app/components/MostRead/index.test.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/app/components/MostRead/index.test.tsx b/src/app/components/MostRead/index.test.tsx index 78f2be4ac31..b8a980f35f0 100644 --- a/src/app/components/MostRead/index.test.tsx +++ b/src/app/components/MostRead/index.test.tsx @@ -260,8 +260,7 @@ describe('MostRead', () => { `( 'should be $endpoint when service is $service, variant is $variant and isLocalEnv is $isLocalEnv', async ({ service, variant, isLocalEnv, endpoint }) => { - // @ts-expect-error need to mock isLocal function for testing purposes - isLocal.mockImplementationOnce(() => isLocalEnv); + (isLocal as jest.Mock).mockImplementationOnce(() => isLocalEnv); render(