From 98c51421d546e4268bac48b0474e8816dce99484 Mon Sep 17 00:00:00 2001 From: Jonathan Roebuck Date: Thu, 2 Dec 2021 15:48:31 +0000 Subject: [PATCH 1/6] use id if available for promo id --- src/app/containers/StoryPromo/index.jsx | 4 +-- src/app/containers/StoryPromo/index.test.jsx | 6 ++--- .../containers/StoryPromo/utilities/index.js | 15 ++++++----- .../StoryPromo/utilities/index.test.js | 27 ++++++++++--------- 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/app/containers/StoryPromo/index.jsx b/src/app/containers/StoryPromo/index.jsx index 46be841fe2e..4e63c36c819 100644 --- a/src/app/containers/StoryPromo/index.jsx +++ b/src/app/containers/StoryPromo/index.jsx @@ -24,7 +24,7 @@ import MediaIndicatorContainer from './MediaIndicator'; import IndexAlsosContainer from './IndexAlsos'; import loggerNode from '#lib/logger.node'; import { MEDIA_MISSING } from '#lib/logger.const'; -import { getHeadingTagOverride, getUniqueLinkId } from './utilities'; +import { getHeadingTagOverride, buildUniquePromoId } from './utilities'; import { MEDIA_ASSET_PAGE } from '#app/routes/utils/pageTypes'; import useCombinedClickTrackerHandler from './useCombinedClickTrackerHandler'; import PromoTimestamp from './Timestamp'; @@ -105,7 +105,7 @@ const StoryPromoContainer = ({ const { pageType } = useContext(RequestContext); const handleClickTracking = useCombinedClickTrackerHandler(eventTrackingData); - const linkId = getUniqueLinkId(item, labelId); + const linkId = buildUniquePromoId(item, labelId); const liveLabel = pathOr('LIVE', ['media', 'liveLabel'], translations); diff --git a/src/app/containers/StoryPromo/index.test.jsx b/src/app/containers/StoryPromo/index.test.jsx index 2e9d44b6eba..99773416f88 100644 --- a/src/app/containers/StoryPromo/index.test.jsx +++ b/src/app/containers/StoryPromo/index.test.jsx @@ -28,7 +28,7 @@ import { } from './helpers/fixtureData'; import StoryPromoContainer from '.'; import { ARTICLE_PAGE } from '#app/routes/utils/pageTypes'; -import { getUniqueLinkId } from './utilities'; +import { buildUniquePromoId } from './utilities'; const onlyOneRelatedItem = { ...indexAlsosItem, @@ -157,8 +157,8 @@ describe('StoryPromo Container', () => { it('should render h3, a, p, time', () => { const labelId = `unlabelled`; - const uriLabelId = getUniqueLinkId(assetTypeItem, labelId); - const assetUriId = getUniqueLinkId(cpsItem, labelId); + const uriLabelId = buildUniquePromoId(assetTypeItem, labelId); + const assetUriId = buildUniquePromoId(cpsItem, labelId); expect(cpsContainer.querySelectorAll('h3 a')[0].innerHTML).toEqual( `${cpsItem.headlines.headline}`, diff --git a/src/app/containers/StoryPromo/utilities/index.js b/src/app/containers/StoryPromo/utilities/index.js index 126c38c961b..7fa2277afb9 100644 --- a/src/app/containers/StoryPromo/utilities/index.js +++ b/src/app/containers/StoryPromo/utilities/index.js @@ -25,12 +25,15 @@ export const getHeadingTagOverride = ({ pageType, isContentTypeGuide }) => { export const isPgl = item => pathOr(null, ['cpsType'], item) === 'PGL'; -export const getUniqueLinkId = (item, labelId) => { +export const buildUniquePromoId = (item, labelId) => { + const id = pathOr('', ['id'], item); const assetUri = pathOr('', ['locators', 'assetUri'], item); - const contentType = pathOr('', ['contentType'], item); const uri = pathOr('', ['uri'], item); - const uniqueId = assetUri || uri; - const assetId = uniqueId.split('/').pop(); - const sanitisedId = assetId.replace(/\W/g, ''); - return `promo-link-${labelId}${sanitisedId || ''}${contentType}`; + const assetId = (id || assetUri || uri).replace(/\W/g, '').split('/').pop(); + const contentType = pathOr('', ['contentType'], item); + + return ['promo', labelId, assetId, contentType] + .filter(Boolean) + .join('-') + .toLowerCase(); }; diff --git a/src/app/containers/StoryPromo/utilities/index.test.js b/src/app/containers/StoryPromo/utilities/index.test.js index a7c54096553..88d3074ce88 100644 --- a/src/app/containers/StoryPromo/utilities/index.test.js +++ b/src/app/containers/StoryPromo/utilities/index.test.js @@ -1,4 +1,4 @@ -import { isMap, isPgl, getHeadingTagOverride, getUniqueLinkId } from '.'; +import { isMap, isPgl, getHeadingTagOverride, buildUniquePromoId } from '.'; import { MOST_WATCHED_PAGE, PHOTO_GALLERY_PAGE, @@ -77,35 +77,38 @@ describe('getHeadingTagOverride', () => { }); }); -describe('getUniqueLinkId', () => { +describe('buildUniquePromoId', () => { const labelId = 'unlabelled'; it('should return id of promo-link with contentType and URI if contentType exists', () => { - expect(getUniqueLinkId(secondaryColumnNoAssetURI, labelId)).toEqual( - 'promo-link-unlabellednewsRadioBulletin', + expect(buildUniquePromoId(secondaryColumnNoAssetURI, labelId)).toEqual( + 'promo-unlabelled-httpswwwbbccouknews-radiobulletin', ); }); it('should return id using URI if assetURI does not exist', () => { - expect(getUniqueLinkId(standardLinkItem, labelId)).toEqual( - 'promo-link-unlabelledazeriText', + expect(buildUniquePromoId(standardLinkItem, labelId)).toEqual( + 'promo-unlabelled-httpwwwbbccomazeri-text', ); }); it('should return id using assetURI does not exist and contentType does not exist', () => { - expect(getUniqueLinkId(completeItem, labelId)).toEqual( - 'promo-link-unlabelledwwwbbccouk', + expect(buildUniquePromoId(completeItem, labelId)).toEqual( + 'promo-unlabelled-httpswwwbbccouk', ); }); it('should return id with contentType only if assetURI and URI do not exist', () => { - expect(getUniqueLinkId(secondaryColumnContentType, labelId)).toEqual( - 'promo-link-unlabelledRadioBulletin', + expect(buildUniquePromoId(secondaryColumnContentType, labelId)).toEqual( + 'promo-unlabelled-radiobulletin', ); }); it('should sanitise link from item and split from last forward slash', () => { expect( - getUniqueLinkId({ locators: { assetUri: 'a/a/ab.b.b@c@c@c' } }, labelId), - ).toEqual('promo-link-unlabelledabbbccc'); + buildUniquePromoId( + { locators: { assetUri: 'a/a/ab.b.b@c@c@c' } }, + labelId, + ), + ).toEqual('promo-unlabelled-aaabbbccc'); }); }); From a7d614ebf2bcc9753353ddcf72df0a74104ec4ab Mon Sep 17 00:00:00 2001 From: Jonathan Roebuck Date: Thu, 2 Dec 2021 16:06:52 +0000 Subject: [PATCH 2/6] remove ID and use promo iindex --- src/app/containers/StoryPromo/index.jsx | 2 +- src/app/containers/StoryPromo/utilities/index.js | 15 +++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/app/containers/StoryPromo/index.jsx b/src/app/containers/StoryPromo/index.jsx index 4e63c36c819..65862eb1044 100644 --- a/src/app/containers/StoryPromo/index.jsx +++ b/src/app/containers/StoryPromo/index.jsx @@ -105,7 +105,7 @@ const StoryPromoContainer = ({ const { pageType } = useContext(RequestContext); const handleClickTracking = useCombinedClickTrackerHandler(eventTrackingData); - const linkId = buildUniquePromoId(item, labelId); + const linkId = buildUniquePromoId(labelId, item, index); // TODO add promo index here const liveLabel = pathOr('LIVE', ['media', 'liveLabel'], translations); diff --git a/src/app/containers/StoryPromo/utilities/index.js b/src/app/containers/StoryPromo/utilities/index.js index 7fa2277afb9..b4def5384fa 100644 --- a/src/app/containers/StoryPromo/utilities/index.js +++ b/src/app/containers/StoryPromo/utilities/index.js @@ -25,14 +25,13 @@ export const getHeadingTagOverride = ({ pageType, isContentTypeGuide }) => { export const isPgl = item => pathOr(null, ['cpsType'], item) === 'PGL'; -export const buildUniquePromoId = (item, labelId) => { - const id = pathOr('', ['id'], item); - const assetUri = pathOr('', ['locators', 'assetUri'], item); - const uri = pathOr('', ['uri'], item); - const assetId = (id || assetUri || uri).replace(/\W/g, '').split('/').pop(); - const contentType = pathOr('', ['contentType'], item); - - return ['promo', labelId, assetId, contentType] +export const buildUniquePromoId = (promoGroupId, promoItem, promoIndex) => { + const assetUri = pathOr('', ['locators', 'assetUri'], promoItem); + const uri = pathOr('', ['uri'], promoItem); + const assetId = (assetUri || uri).replace(/\W/g, '').split('/').pop(); + const contentType = pathOr('', ['contentType'], promoItem); + + return ['promo', promoGroupId, assetId, contentType, promoIndex] .filter(Boolean) .join('-') .toLowerCase(); From c8110240863a589fe4b143d276580ad0fd316b18 Mon Sep 17 00:00:00 2001 From: Jonathan Roebuck Date: Thu, 2 Dec 2021 16:13:58 +0000 Subject: [PATCH 3/6] fix unit tests --- .../containers/StoryPromo/utilities/index.js | 4 ++-- .../StoryPromo/utilities/index.test.js | 21 ++++++++++--------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/app/containers/StoryPromo/utilities/index.js b/src/app/containers/StoryPromo/utilities/index.js index b4def5384fa..bd63b3b8c27 100644 --- a/src/app/containers/StoryPromo/utilities/index.js +++ b/src/app/containers/StoryPromo/utilities/index.js @@ -25,13 +25,13 @@ export const getHeadingTagOverride = ({ pageType, isContentTypeGuide }) => { export const isPgl = item => pathOr(null, ['cpsType'], item) === 'PGL'; -export const buildUniquePromoId = (promoGroupId, promoItem, promoIndex) => { +export const buildUniquePromoId = (promoGroupId, promoItem, promoIndex = 0) => { const assetUri = pathOr('', ['locators', 'assetUri'], promoItem); const uri = pathOr('', ['uri'], promoItem); const assetId = (assetUri || uri).replace(/\W/g, '').split('/').pop(); const contentType = pathOr('', ['contentType'], promoItem); - return ['promo', promoGroupId, assetId, contentType, promoIndex] + return ['promo', promoGroupId, assetId, contentType, promoIndex + 1] .filter(Boolean) .join('-') .toLowerCase(); diff --git a/src/app/containers/StoryPromo/utilities/index.test.js b/src/app/containers/StoryPromo/utilities/index.test.js index 88d3074ce88..4a886b91aa2 100644 --- a/src/app/containers/StoryPromo/utilities/index.test.js +++ b/src/app/containers/StoryPromo/utilities/index.test.js @@ -80,35 +80,36 @@ describe('getHeadingTagOverride', () => { describe('buildUniquePromoId', () => { const labelId = 'unlabelled'; it('should return id of promo-link with contentType and URI if contentType exists', () => { - expect(buildUniquePromoId(secondaryColumnNoAssetURI, labelId)).toEqual( - 'promo-unlabelled-httpswwwbbccouknews-radiobulletin', + expect(buildUniquePromoId(labelId, secondaryColumnNoAssetURI, 0)).toEqual( + 'promo-unlabelled-httpswwwbbccouknews-radiobulletin-1', ); }); it('should return id using URI if assetURI does not exist', () => { - expect(buildUniquePromoId(standardLinkItem, labelId)).toEqual( - 'promo-unlabelled-httpwwwbbccomazeri-text', + expect(buildUniquePromoId(labelId, standardLinkItem, 1)).toEqual( + 'promo-unlabelled-httpwwwbbccomazeri-text-2', ); }); it('should return id using assetURI does not exist and contentType does not exist', () => { - expect(buildUniquePromoId(completeItem, labelId)).toEqual( - 'promo-unlabelled-httpswwwbbccouk', + expect(buildUniquePromoId(labelId, completeItem, 2)).toEqual( + 'promo-unlabelled-httpswwwbbccouk-3', ); }); it('should return id with contentType only if assetURI and URI do not exist', () => { - expect(buildUniquePromoId(secondaryColumnContentType, labelId)).toEqual( - 'promo-unlabelled-radiobulletin', + expect(buildUniquePromoId(labelId, secondaryColumnContentType, 3)).toEqual( + 'promo-unlabelled-radiobulletin-4', ); }); it('should sanitise link from item and split from last forward slash', () => { expect( buildUniquePromoId( - { locators: { assetUri: 'a/a/ab.b.b@c@c@c' } }, labelId, + { locators: { assetUri: 'a/a/ab.b.b@c@c@c' } }, + 4, ), - ).toEqual('promo-unlabelled-aaabbbccc'); + ).toEqual('promo-unlabelled-aaabbbccc-5'); }); }); From b30a9e8a877a27a6a1d5c8549f37bd708f459ec4 Mon Sep 17 00:00:00 2001 From: Jonathan Roebuck Date: Fri, 3 Dec 2021 11:38:15 +0000 Subject: [PATCH 4/6] drill index down to promo to create id attrs --- src/app/containers/CpsFeaturesAnalysis/index.jsx | 3 ++- .../RelatedContentPromoList/index.jsx | 3 ++- src/app/containers/FrontPageStoryRows/index.jsx | 3 +++ src/app/containers/StoryPromo/index.jsx | 7 +++++-- src/app/containers/StoryPromo/index.test.jsx | 15 ++++++++------- src/app/containers/StoryPromo/utilities/index.js | 4 +++- .../containers/StoryPromo/utilities/index.test.js | 12 ++++++------ 7 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/app/containers/CpsFeaturesAnalysis/index.jsx b/src/app/containers/CpsFeaturesAnalysis/index.jsx index ad84a2c503f..df8643b71b8 100644 --- a/src/app/containers/CpsFeaturesAnalysis/index.jsx +++ b/src/app/containers/CpsFeaturesAnalysis/index.jsx @@ -27,10 +27,11 @@ const PromoListComponent = ({ promoItems, dir }) => { return ( - {promoItems.map(item => ( + {promoItems.map((item, index) => ( - {promoItems.map(item => ( + {promoItems.map((item, index) => ( const renderPromo = ({ item, + index, promoType = 'regular', isFirstSection = false, dir, @@ -30,6 +31,7 @@ const renderPromo = ({ {renderPromo({ + index: i, item: story, dir, displayImage: displayImages, diff --git a/src/app/containers/StoryPromo/index.jsx b/src/app/containers/StoryPromo/index.jsx index 65862eb1044..1cd165c9c31 100644 --- a/src/app/containers/StoryPromo/index.jsx +++ b/src/app/containers/StoryPromo/index.jsx @@ -1,5 +1,5 @@ import React, { useContext } from 'react'; -import { shape, bool, oneOf, oneOfType, string } from 'prop-types'; +import { shape, bool, oneOf, oneOfType, string, number } from 'prop-types'; import styled from '@emotion/styled'; import StoryPromo, { Headline, Summary, Link } from '@bbc/psammead-story-promo'; import { GEL_GROUP_4_SCREEN_WIDTH_MIN } from '@bbc/gel-foundations/breakpoints'; @@ -91,6 +91,7 @@ StoryPromoImage.defaultProps = { const StoryPromoContainer = ({ item, + index, promoType, lazyLoadImage, dir, @@ -290,6 +291,7 @@ StoryPromoContainer.propTypes = { }), }), labelId: string, + index: number, }; StoryPromoContainer.defaultProps = { @@ -301,7 +303,8 @@ StoryPromoContainer.defaultProps = { isSingleColumnLayout: false, serviceDatetimeLocale: null, eventTrackingData: null, - labelId: 'unlabelled', + labelId: '', + index: 0, }; export default StoryPromoContainer; diff --git a/src/app/containers/StoryPromo/index.test.jsx b/src/app/containers/StoryPromo/index.test.jsx index 99773416f88..89ab538c428 100644 --- a/src/app/containers/StoryPromo/index.test.jsx +++ b/src/app/containers/StoryPromo/index.test.jsx @@ -136,29 +136,30 @@ describe('StoryPromo Container', () => { let cpsContainer; let overtypedSummaryContainer; let assetTypeContainer; + const labelId = `test-group-id`; beforeEach(() => { cpsItem = deepClone(completeItem); - cpsContainer = render().container; + cpsContainer = render( + , + ).container; overtypedSummaryItem = deepClone(itemWithOvertypedSummary); overtypedSummaryContainer = render( - , + , ).container; assetTypeItem = deepClone(standardLinkItem); assetTypeContainer = render( - , + , ).container; }); afterEach(cleanup); it('should render h3, a, p, time', () => { - const labelId = `unlabelled`; - - const uriLabelId = buildUniquePromoId(assetTypeItem, labelId); - const assetUriId = buildUniquePromoId(cpsItem, labelId); + const uriLabelId = buildUniquePromoId(labelId, assetTypeItem); + const assetUriId = buildUniquePromoId(labelId, cpsItem); expect(cpsContainer.querySelectorAll('h3 a')[0].innerHTML).toEqual( `${cpsItem.headlines.headline}`, diff --git a/src/app/containers/StoryPromo/utilities/index.js b/src/app/containers/StoryPromo/utilities/index.js index bd63b3b8c27..67dfe2fdef0 100644 --- a/src/app/containers/StoryPromo/utilities/index.js +++ b/src/app/containers/StoryPromo/utilities/index.js @@ -28,7 +28,9 @@ export const isPgl = item => pathOr(null, ['cpsType'], item) === 'PGL'; export const buildUniquePromoId = (promoGroupId, promoItem, promoIndex = 0) => { const assetUri = pathOr('', ['locators', 'assetUri'], promoItem); const uri = pathOr('', ['uri'], promoItem); - const assetId = (assetUri || uri).replace(/\W/g, '').split('/').pop(); + const asset = assetUri || uri; + const assetParts = asset.split(/www\.bbc\.(co\.uk|com)/); + const assetId = assetParts[assetParts.length - 1].replace(/\W/g, ''); const contentType = pathOr('', ['contentType'], promoItem); return ['promo', promoGroupId, assetId, contentType, promoIndex + 1] diff --git a/src/app/containers/StoryPromo/utilities/index.test.js b/src/app/containers/StoryPromo/utilities/index.test.js index 4a886b91aa2..62fe8b50767 100644 --- a/src/app/containers/StoryPromo/utilities/index.test.js +++ b/src/app/containers/StoryPromo/utilities/index.test.js @@ -78,28 +78,28 @@ describe('getHeadingTagOverride', () => { }); describe('buildUniquePromoId', () => { - const labelId = 'unlabelled'; + const labelId = 'test-group-id'; it('should return id of promo-link with contentType and URI if contentType exists', () => { expect(buildUniquePromoId(labelId, secondaryColumnNoAssetURI, 0)).toEqual( - 'promo-unlabelled-httpswwwbbccouknews-radiobulletin-1', + 'promo-test-group-id-news-radiobulletin-1', ); }); it('should return id using URI if assetURI does not exist', () => { expect(buildUniquePromoId(labelId, standardLinkItem, 1)).toEqual( - 'promo-unlabelled-httpwwwbbccomazeri-text-2', + 'promo-test-group-id-azeri-text-2', ); }); it('should return id using assetURI does not exist and contentType does not exist', () => { expect(buildUniquePromoId(labelId, completeItem, 2)).toEqual( - 'promo-unlabelled-httpswwwbbccouk-3', + 'promo-test-group-id-3', ); }); it('should return id with contentType only if assetURI and URI do not exist', () => { expect(buildUniquePromoId(labelId, secondaryColumnContentType, 3)).toEqual( - 'promo-unlabelled-radiobulletin-4', + 'promo-test-group-id-radiobulletin-4', ); }); @@ -110,6 +110,6 @@ describe('buildUniquePromoId', () => { { locators: { assetUri: 'a/a/ab.b.b@c@c@c' } }, 4, ), - ).toEqual('promo-unlabelled-aaabbbccc-5'); + ).toEqual('promo-test-group-id-aaabbbccc-5'); }); }); From b0606db86a66cda3500fe7984bf3241154a9e91a Mon Sep 17 00:00:00 2001 From: Jonathan Roebuck Date: Fri, 3 Dec 2021 11:38:21 +0000 Subject: [PATCH 5/6] update snapshots --- .../__snapshots__/index.test.jsx.snap | 12 +- .../__snapshots__/index.test.jsx.snap | 4 +- .../__snapshots__/index.test.jsx.snap | 24 +-- .../__snapshots__/index.test.jsx.snap | 16 +- .../__snapshots__/index.test.jsx.snap | 12 +- .../__snapshots__/index.test.jsx.snap | 72 ++++---- .../__snapshots__/index.test.jsx.snap | 72 ++++---- .../__snapshots__/index.test.jsx.snap | 172 +++++++++--------- .../IdxPage/__snapshots__/index.test.jsx.snap | 140 +++++++------- .../__snapshots__/index.test.jsx.snap | 16 +- .../__snapshots__/index.test.jsx.snap | 12 +- .../__snapshots__/index.test.jsx.snap | 44 ++--- .../__snapshots__/index.test.jsx.snap | 24 +-- 13 files changed, 310 insertions(+), 310 deletions(-) diff --git a/src/app/containers/CpsFeaturesAnalysis/__snapshots__/index.test.jsx.snap b/src/app/containers/CpsFeaturesAnalysis/__snapshots__/index.test.jsx.snap index 04cd4f8cfaf..23286b7dff2 100644 --- a/src/app/containers/CpsFeaturesAnalysis/__snapshots__/index.test.jsx.snap +++ b/src/app/containers/CpsFeaturesAnalysis/__snapshots__/index.test.jsx.snap @@ -696,12 +696,12 @@ exports[`CpsRelatedContent should render Story Feature components when given app class="emotion-37 emotion-38" > STY - Ọrụ bekee na ịrụrụ onwe gị ọrụ, kedụ nk? @@ -756,12 +756,12 @@ exports[`CpsRelatedContent should render Story Feature components when given app class="emotion-37 emotion-38" > Robert Mugabe: Zimbabwe eferela nwa amadi a aka @@ -1480,12 +1480,12 @@ exports[`CpsRelatedContent should render Story Promo components without