Merge pull request #23562 from guardian/ph-20210222-1143-renderId

Introduce ElementsEnhancer and elementId

common/app/model/dotcomrendering/DotcomRenderingDataModel.scala

@@ -66,13 +66,41 @@ case class DotcomRenderingDataModel(
     isSpecialReport: Boolean, // Indicates whether the page is a special report.
 )
 
+object ElementsEnhancer {
+
+  // Note:
+  //     In the file PageElement-Identifiers.md you will find a discussion of identifiers used by PageElements
+  //     Also look for "03feb394-a17d-4430-8384-edd1891e0d01"
+
+  def enhanceElement(element: JsValue): JsValue = {
+    element.as[JsObject] ++ Json.obj("elementId" -> java.util.UUID.randomUUID.toString)
+  }
+
+  def enhanceElements(elements: JsValue): IndexedSeq[JsValue] = {
+    elements.as[JsArray].value.map(element => enhanceElement(element))
+  }
+
+  def enhanceBlock(block: JsValue): JsValue = {
+    val elements = block.as[JsObject].value("elements")
+    block.as[JsObject] ++ Json.obj("elements" -> enhanceElements(elements))
+  }
+
+  def enhanceBlocks(blocks: JsValue): IndexedSeq[JsValue] = {
+    blocks.as[JsArray].value.map(block => enhanceBlock(block))
+  }
+
+  def enhanceDcrObject(obj: JsObject): JsObject = {
+    obj ++ Json.obj("blocks" -> enhanceBlocks(obj.value("blocks")))
+  }
+}
+
 object DotcomRenderingDataModel {
 
   implicit val pageElementWrites = PageElement.pageElementWrites
 
   implicit val writes = new Writes[DotcomRenderingDataModel] {
-    def writes(model: DotcomRenderingDataModel) =
-      Json.obj(
+    def writes(model: DotcomRenderingDataModel) = {
+      val obj = Json.obj(
         "version" -> model.version,
         "headline" -> model.headline,
         "standfirst" -> model.standfirst,
@@ -126,6 +154,13 @@ object DotcomRenderingDataModel {
         "matchUrl" -> model.matchUrl,
         "isSpecialReport" -> model.isSpecialReport,
       )
+
+      // The following line essentially performs the "update" of the `elements` objects inside the `blocks` objects
+      // using functions of the ElementsEnhancer object.
+      // See comments in ElementsEnhancer for a full context of why this happens.
+      ElementsEnhancer.enhanceDcrObject(obj)
+
+    }
   }
 
   def toJson(model: DotcomRenderingDataModel): String = {

common/app/model/dotcomrendering/PageElement-Identifiers.md

@@ -0,0 +1,56 @@
+( document id: 03feb394-a17d-4430-8384-edd1891e0d01 )
+
+This document present the conventions around the use of IDs for `PageElements` and DCR's `BlockElements`
+
+### Introduction
+
+`trait PageElement` and its members model the objects the backend send to DCR for rendering. In the DCR data object, they are referred to as `BlockElement`s, for instance 
+
+```
+model.dotcomrendering.pageElements.TextBlockElement
+```
+
+### The State of PageElement Identifiers
+
+`BlockElement` do not per se have a neturally defined identifier. This is because although some of them correspond to CAPI elements who have a natural identifer, for instance atoms, PageElements are more general than content held in CAPI. 
+
+Due to historical reasons a few `PageElement`s have a field called `id`, for instance
+
+```
+case class MediaAtomBlockElement(
+    id: String,
+    title: String,
+    ...
+```
+
+but this `id` should be seen as metadata about the original data the PageElement represents. 
+
+In Feb 2021, the DCR team requested that PageElements be given an identifier that would help with rendering, notably to be used for DOM reconciliation during React rendering. Although DCR itself could be responsible to provide them (ultil this point it actually did using integer indices), the backend accepted to provide one, called `elementId`. 
+
+We then moved to serving, say, `TextBlockElement` like this:   
+
+```
+{
+  "_type": "model.dotcomrendering.pageElements.TextBlockElement",
+  "html": "<p>Something</p>"
+}
+```
+
+to this:
+
+```
+{
+  "elementId": "ae4e4580-dd0b-4aed-a958-f9ba6aa79ca2",
+  "_type": "model.dotcomrendering.pageElements.TextBlockElement",
+  "html": "<p>Something</p>"
+}
+```
+
+*NB*:
+
+1. The backend type `trait PageElement`, does not itself enforces the presence of `elementId`, this attribute is added (at the time these lines are written), as per this [https://github.com/guardian/frontend/pull/23562](https://github.com/guardian/frontend/pull/23562) , using a data enhancer applied to the JSON representation of the DCR data object. This choice was made on the basis that the `elementId` is a courtesy provided to DCR by the backend and not a real property of the data as seen by the backend
+
+
+2. The initial implementation uses UUIDs, but the contract is that any reasonably unique string can do 
+
+3. The value of `elementId` for each element is, as per original implementation, randomly chosen at each generation. In any case, there is no 1-2-1 mapping between `PageElement`s / `BlockElement`s and those values. 

common/app/model/dotcomrendering/pageElements/PageElement.scala

@@ -96,6 +96,10 @@ trait ThirdPartyEmbeddedContent {
 
 sealed trait PageElement
 
+// Note:
+//     In the file PageElement-Identifiers.md you will find a discussion of identifiers used by PageElements
+//     Also look for "03feb394-a17d-4430-8384-edd1891e0d01"
+
 case class AudioAtomBlockElement(
     id: String,
     kicker: String,