From cfbd8a9673821eac307fdec0f54deded8a83d93c Mon Sep 17 00:00:00 2001
From: Max Duval <max.duval@theguardian.com>
Date: Tue, 16 Jan 2024 10:04:40 +0000
Subject: [PATCH] refactor(YoutubeAtom): no reliance on elementId

the uniqueId only needs to be truly unique on the client-side
which can be simply achieved with an increasing integer

update tests to use regular expression matching
---
 dotcom-rendering/src/components/Card/Card.tsx |  3 --
 .../YoutubeAtom/YoutubeAtom.stories.tsx       | 28 -------------------
 .../YoutubeAtom/YoutubeAtom.test.tsx          |  9 ------
 .../components/YoutubeAtom/YoutubeAtom.tsx    | 23 +++++++++------
 .../YoutubeBlockComponent.importable.tsx      |  3 --
 .../YoutubeBlockComponent.stories.tsx         |  7 -----
 dotcom-rendering/src/lib/renderElement.tsx    |  1 -
 7 files changed, 15 insertions(+), 59 deletions(-)

diff --git a/dotcom-rendering/src/components/Card/Card.tsx b/dotcom-rendering/src/components/Card/Card.tsx
index af4e2f53b65..2d3cb2dfc66 100644
--- a/dotcom-rendering/src/components/Card/Card.tsx
+++ b/dotcom-rendering/src/components/Card/Card.tsx
@@ -474,9 +474,6 @@ export const Card = ({
 										>
 											<YoutubeBlockComponent
 												id={media.mainMedia.elementId}
-												elementId={
-													media.mainMedia.elementId
-												}
 												assetId={
 													media.mainMedia.videoId
 												}
diff --git a/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.stories.tsx b/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.stories.tsx
index 2907c77e948..2ca1b0d716a 100644
--- a/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.stories.tsx
+++ b/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.stories.tsx
@@ -55,7 +55,6 @@ export const NoConsent = (): JSX.Element => {
 	return (
 		<div style={containerStyle}>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -83,7 +82,6 @@ export const NoOverlay = (): JSX.Element => {
 	return (
 		<div style={containerStyle}>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -118,7 +116,6 @@ export const WithOverrideImage = (): JSX.Element => {
 		<div style={containerStyle}>
 			<OverlayAutoplayExplainer />
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="3jpXAMwRSu4"
 				alt="Microscopic image of COVID"
 				eventEmitters={[
@@ -150,7 +147,6 @@ export const WithPosterImage = (): JSX.Element => {
 		<div style={containerStyle}>
 			<OverlayAutoplayExplainer />
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="N9Cgy-ke5-s"
 				alt=""
 				eventEmitters={[
@@ -185,7 +181,6 @@ export const WithOverlayAndPosterImage = (): JSX.Element => {
 		<div style={containerStyle}>
 			<OverlayAutoplayExplainer />
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="N9Cgy-ke5-s"
 				alt=""
 				eventEmitters={[
@@ -224,7 +219,6 @@ export const GiveConsent = (): JSX.Element => {
 			<button onClick={() => setConsented(true)}>Give consent</button>
 			<div style={containerStyle}>
 				<YoutubeAtom
-					elementId="xyz"
 					videoId="3jpXAMwRSu4"
 					alt="Microscopic image of COVID"
 					eventEmitters={[
@@ -258,7 +252,6 @@ export const Sticky = (): JSX.Element => {
 			<div>Scroll down...</div>
 			<div style={{ height: '1000px' }}></div>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -292,7 +285,6 @@ export const StickyMainMedia = (): JSX.Element => {
 			<div>Scroll down...</div>
 			<div style={{ height: '1000px' }}></div>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -328,7 +320,6 @@ export const DuplicateVideos = (): JSX.Element => {
 	return (
 		<div style={containerStyleSmall}>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -350,7 +341,6 @@ export const DuplicateVideos = (): JSX.Element => {
 			/>
 			<br />
 			<YoutubeAtom
-				elementId="xyz2"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -388,7 +378,6 @@ export const MultipleStickyVideos = (): JSX.Element => {
 	return (
 		<div style={{ width: '500px', height: '5000px' }}>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -411,7 +400,6 @@ export const MultipleStickyVideos = (): JSX.Element => {
 				adTargeting={adTargeting}
 			/>
 			<YoutubeAtom
-				elementId="xyz-2"
 				videoId="pcMiS6PW8aQ"
 				alt=""
 				eventEmitters={[
@@ -434,7 +422,6 @@ export const MultipleStickyVideos = (): JSX.Element => {
 				adTargeting={adTargeting}
 			/>
 			<YoutubeAtom
-				elementId="xyu"
 				videoId="3jpXAMwRSu4"
 				alt=""
 				eventEmitters={[
@@ -469,7 +456,6 @@ export const PausesOffscreen = (): JSX.Element => {
 		<div>
 			<div>Scroll down...</div>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -506,7 +492,6 @@ export const NoConsentWithIma = (): JSX.Element => {
 	return (
 		<div style={containerStyle}>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -534,7 +519,6 @@ export const AdFreeWithIma = (): JSX.Element => {
 	return (
 		<div style={containerStyle}>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -563,7 +547,6 @@ export const NoOverlayWithIma = (): JSX.Element => {
 	return (
 		<div style={containerStyle}>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -593,7 +576,6 @@ export const WithOverrideImageWithIma = (): JSX.Element => {
 	return (
 		<div style={containerStyle}>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="3jpXAMwRSu4"
 				alt="Microscopic image of COVID"
 				eventEmitters={[
@@ -622,7 +604,6 @@ export const WithPosterImageWithIma = (): JSX.Element => {
 	return (
 		<div style={containerStyle}>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="N9Cgy-ke5-s"
 				alt=""
 				eventEmitters={[
@@ -653,7 +634,6 @@ export const WithOverlayAndPosterImageWithIma = (): JSX.Element => {
 	return (
 		<div style={containerStyle}>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="N9Cgy-ke5-s"
 				alt=""
 				eventEmitters={[
@@ -688,7 +668,6 @@ export const GiveConsentWithIma = (): JSX.Element => {
 			<button onClick={() => setConsented(true)}>Give consent</button>
 			<div style={containerStyle}>
 				<YoutubeAtom
-					elementId="xyz"
 					videoId="3jpXAMwRSu4"
 					alt="Microscopic image of COVID"
 					eventEmitters={[
@@ -722,7 +701,6 @@ export const StickyWithIma = (): JSX.Element => {
 			<div style={{ fontSize: '36px' }}>⬇️</div>
 			<div style={{ height: '1000px' }}></div>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -755,7 +733,6 @@ export const StickyMainMediaWithIma = (): JSX.Element => {
 			<div style={{ fontSize: '36px' }}>⬇️</div>
 			<div style={{ height: '1000px' }}></div>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -786,7 +763,6 @@ export const DuplicateVideosWithIma = (): JSX.Element => {
 	return (
 		<div style={containerStyleSmall}>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -808,7 +784,6 @@ export const DuplicateVideosWithIma = (): JSX.Element => {
 			/>
 			<br />
 			<YoutubeAtom
-				elementId="xyz2"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -840,7 +815,6 @@ export const MultipleStickyVideosWithIma = (): JSX.Element => {
 	return (
 		<div style={{ width: '500px', height: '5000px' }}>
 			<YoutubeAtom
-				elementId="xyz"
 				videoId="-ZCvZmYlQD8"
 				alt=""
 				eventEmitters={[
@@ -863,7 +837,6 @@ export const MultipleStickyVideosWithIma = (): JSX.Element => {
 				abTestParticipations={{}}
 			/>
 			<YoutubeAtom
-				elementId="xyz-2"
 				videoId="pcMiS6PW8aQ"
 				alt=""
 				eventEmitters={[
@@ -886,7 +859,6 @@ export const MultipleStickyVideosWithIma = (): JSX.Element => {
 				abTestParticipations={{}}
 			/>
 			<YoutubeAtom
-				elementId="xyu"
 				videoId="3jpXAMwRSu4"
 				alt=""
 				eventEmitters={[
diff --git a/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.test.tsx b/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.test.tsx
index 21c2857dbdf..91dfbd8253b 100644
--- a/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.test.tsx
+++ b/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.test.tsx
@@ -28,7 +28,6 @@ describe('YoutubeAtom', () => {
 				value={{ renderingTarget: 'Web', darkModeAvailable: false }}
 			>
 				<YoutubeAtom
-					elementId="123"
 					title="My Youtube video!"
 					videoId="ZCvZmYlQD8"
 					alt=""
@@ -58,7 +57,6 @@ describe('YoutubeAtom', () => {
 				value={{ renderingTarget: 'Web', darkModeAvailable: false }}
 			>
 				<YoutubeAtom
-					elementId="123"
 					title="My Youtube video!"
 					videoId="ZCvZmYlQD8"
 					alt=""
@@ -97,7 +95,6 @@ describe('YoutubeAtom', () => {
 				value={{ renderingTarget: 'Web', darkModeAvailable: false }}
 			>
 				<YoutubeAtom
-					elementId="123"
 					title="My Youtube video!"
 					videoId="ZCvZmYlQD8"
 					alt=""
@@ -128,7 +125,6 @@ describe('YoutubeAtom', () => {
 				value={{ renderingTarget: 'Web', darkModeAvailable: false }}
 			>
 				<YoutubeAtom
-					elementId="123"
 					title="My Youtube video!"
 					videoId="ZCvZmYlQD8"
 					alt=""
@@ -161,7 +157,6 @@ describe('YoutubeAtom', () => {
 				value={{ renderingTarget: 'Web', darkModeAvailable: false }}
 			>
 				<YoutubeAtom
-					elementId="123"
 					title="My Youtube video!"
 					videoId="ZCvZmYlQD8"
 					alt=""
@@ -192,7 +187,6 @@ describe('YoutubeAtom', () => {
 				value={{ renderingTarget: 'Web', darkModeAvailable: false }}
 			>
 				<YoutubeAtom
-					elementId="123"
 					title="My Youtube video!"
 					videoId="ZCvZmYlQD8"
 					alt=""
@@ -222,7 +216,6 @@ describe('YoutubeAtom', () => {
 				value={{ renderingTarget: 'Web', darkModeAvailable: false }}
 			>
 				<YoutubeAtom
-					elementId="123"
 					title="My Youtube video!"
 					videoId="ZCvZmYlQD8"
 					alt=""
@@ -256,7 +249,6 @@ describe('YoutubeAtom', () => {
 					value={{ renderingTarget: 'Web', darkModeAvailable: false }}
 				>
 					<YoutubeAtom
-						elementId="123"
 						title="My Youtube video!"
 						videoId="ZCvZmYlQD8"
 						alt=""
@@ -274,7 +266,6 @@ describe('YoutubeAtom', () => {
 						abTestParticipations={{}}
 					/>
 					<YoutubeAtom
-						elementId="123"
 						title="My Youtube video 2!"
 						videoId="ZCvZmYlQD8"
 						alt=""
diff --git a/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.tsx b/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.tsx
index b8484508e84..a7e7c475efa 100644
--- a/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.tsx
+++ b/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.tsx
@@ -1,7 +1,7 @@
 import type { Participations } from '@guardian/ab-core';
 import type { ConsentState } from '@guardian/consent-management-platform/dist/types';
 import type { ArticleFormat } from '@guardian/libs';
-import { useCallback, useState } from 'react';
+import { useCallback, useEffect, useState } from 'react';
 import { MaintainAspectRatio } from '../MaintainAspectRatio';
 import type { VideoCategory } from './YoutubeAtomOverlay';
 import { YoutubeAtomOverlay } from './YoutubeAtomOverlay';
@@ -21,7 +21,6 @@ export type VideoEventKey =
 	| 'pause';
 
 type Props = {
-	elementId: string;
 	videoId: string;
 	overrideImage?: string | undefined;
 	posterImage?: string | undefined;
@@ -45,8 +44,10 @@ type Props = {
 	showTextOverlay?: boolean;
 };
 
+/** always undefined on the server */
+let index: number | undefined = undefined;
+
 export const YoutubeAtom = ({
-	elementId,
 	videoId,
 	overrideImage,
 	posterImage,
@@ -74,8 +75,14 @@ export const YoutubeAtom = ({
 	const [isActive, setIsActive] = useState<boolean>(false);
 	const [isClosed, setIsClosed] = useState<boolean>(false);
 	const [pauseVideo, setPauseVideo] = useState<boolean>(false);
+	const [uniqueId, setUniqueId] = useState<string>();
+
+	useEffect(() => {
+		index ??= 0;
+		index++;
+		setUniqueId(`${videoId}-${index}`);
+	}, [videoId]);
 
-	const uniqueId = `${videoId}-${elementId}`;
 	const enableIma =
 		imaEnabled &&
 		!!adTargeting &&
@@ -152,7 +159,7 @@ export const YoutubeAtom = ({
 
 	return (
 		<YoutubeAtomSticky
-			uniqueId={uniqueId}
+			uniqueId={uniqueId ?? videoId}
 			videoId={videoId}
 			shouldStick={shouldStick}
 			isActive={isActive}
@@ -164,7 +171,7 @@ export const YoutubeAtom = ({
 			shouldPauseOutOfView={shouldPauseOutOfView}
 		>
 			<MaintainAspectRatio height={height} width={width}>
-				{loadPlayer && consentState && adTargeting && (
+				{loadPlayer && consentState && adTargeting && !!uniqueId && (
 					<YoutubeAtomPlayer
 						videoId={videoId}
 						uniqueId={uniqueId}
@@ -191,7 +198,7 @@ export const YoutubeAtom = ({
 				)}
 				{showOverlay && (
 					<YoutubeAtomOverlay
-						uniqueId={uniqueId}
+						uniqueId={uniqueId ?? videoId}
 						overrideImage={overrideImage}
 						posterImage={posterImage}
 						height={height}
@@ -207,7 +214,7 @@ export const YoutubeAtom = ({
 					/>
 				)}
 				{showPlaceholder && (
-					<YoutubeAtomPlaceholder uniqueId={uniqueId} />
+					<YoutubeAtomPlaceholder uniqueId={uniqueId ?? videoId} />
 				)}
 			</MaintainAspectRatio>
 		</YoutubeAtomSticky>
diff --git a/dotcom-rendering/src/components/YoutubeBlockComponent.importable.tsx b/dotcom-rendering/src/components/YoutubeBlockComponent.importable.tsx
index c3527a6f2b8..954a099d4c2 100644
--- a/dotcom-rendering/src/components/YoutubeBlockComponent.importable.tsx
+++ b/dotcom-rendering/src/components/YoutubeBlockComponent.importable.tsx
@@ -13,7 +13,6 @@ import { YoutubeAtom } from './YoutubeAtom/YoutubeAtom';
 
 type Props = {
 	id: string;
-	elementId: string;
 	mediaTitle?: string;
 	altText?: string;
 	assetId: string;
@@ -89,7 +88,6 @@ const getLargestImageSize = (
 
 export const YoutubeBlockComponent = ({
 	id,
-	elementId,
 	assetId,
 	mediaTitle,
 	altText,
@@ -200,7 +198,6 @@ export const YoutubeBlockComponent = ({
 	return (
 		<div data-chromatic="ignore" data-component="youtube-atom">
 			<YoutubeAtom
-				elementId={elementId}
 				videoId={assetId}
 				overrideImage={overrideImage}
 				posterImage={getLargestImageSize(posterImage)?.url}
diff --git a/dotcom-rendering/src/components/YoutubeBlockComponent.stories.tsx b/dotcom-rendering/src/components/YoutubeBlockComponent.stories.tsx
index 8ec8de567bd..169731b59e7 100644
--- a/dotcom-rendering/src/components/YoutubeBlockComponent.stories.tsx
+++ b/dotcom-rendering/src/components/YoutubeBlockComponent.stories.tsx
@@ -50,7 +50,6 @@ export const Default = () => {
 				assetId="d2Q5bXvEgMg"
 				mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video"
 				id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28"
-				elementId="5ab531a2-f6ea-499d-b274-191114c8628c"
 				expired={false}
 				stickyVideos={false}
 			/>
@@ -83,7 +82,6 @@ export const Vertical = () => {
 				assetId="d2Q5bXvEgMg"
 				mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video"
 				id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28"
-				elementId="5ab531a2-f6ea-499d-b274-191114c8628c"
 				expired={false}
 				height={259}
 				width={460}
@@ -118,7 +116,6 @@ export const Expired = () => {
 				assetId="d2Q5bXvEgMg"
 				mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video"
 				id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28"
-				elementId="5ab531a2-f6ea-499d-b274-191114c8628c"
 				expired={true}
 				overrideImage="https://i.guim.co.uk/img/media/49565a29c6586fe6b748926e0be96c5e9c90473c/0_0_4981_2989/500.jpg?quality=85&auto=format&fit=max&s=17c70ec70002ea34886fd6c2605cd81e"
 				height={259}
@@ -154,7 +151,6 @@ export const WithOverlayImage = () => {
 				assetId="d2Q5bXvEgMg"
 				mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video"
 				id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28"
-				elementId="5ab531a2-f6ea-499d-b274-191114c8628c"
 				expired={false}
 				duration={333}
 				overrideImage="https://i.guim.co.uk/img/media/49565a29c6586fe6b748926e0be96c5e9c90473c/0_0_4981_2989/500.jpg?quality=85&auto=format&fit=max&s=17c70ec70002ea34886fd6c2605cd81e"
@@ -191,7 +187,6 @@ export const WithPosterImage = () => {
 				assetId="d2Q5bXvEgMg"
 				mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video"
 				id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28"
-				elementId="5ab531a2-f6ea-499d-b274-191114c8628c"
 				expired={false}
 				duration={333}
 				posterImage={[
@@ -249,7 +244,6 @@ export const WithPosterAndOverlayImage = () => {
 				assetId="d2Q5bXvEgMg"
 				mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video"
 				id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28"
-				elementId="5ab531a2-f6ea-499d-b274-191114c8628c"
 				expired={false}
 				overrideImage="https://i.guim.co.uk/img/media/49565a29c6586fe6b748926e0be96c5e9c90473c/0_0_4981_2989/500.jpg?quality=85&auto=format&fit=max&s=17c70ec70002ea34886fd6c2605cd81e"
 				duration={333}
@@ -308,7 +302,6 @@ export const WithShowMainVideoFlagOff = () => {
 				assetId="d2Q5bXvEgMg"
 				mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video"
 				id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28"
-				elementId="5ab531a2-f6ea-499d-b274-191114c8628c"
 				expired={false}
 				overrideImage="https://i.guim.co.uk/img/media/49565a29c6586fe6b748926e0be96c5e9c90473c/0_0_4981_2989/500.jpg?quality=85&auto=format&fit=max&s=17c70ec70002ea34886fd6c2605cd81e"
 				duration={333}
diff --git a/dotcom-rendering/src/lib/renderElement.tsx b/dotcom-rendering/src/lib/renderElement.tsx
index e9c328d3d59..2b14b99afcc 100644
--- a/dotcom-rendering/src/lib/renderElement.tsx
+++ b/dotcom-rendering/src/lib/renderElement.tsx
@@ -743,7 +743,6 @@ export const renderElement = ({
 						hideCaption={hideCaption}
 						isMainMedia={isMainMedia}
 						id={element.id}
-						elementId={element.elementId}
 						assetId={element.assetId}
 						expired={element.expired}
 						overrideImage={element.overrideImage}