Merge pull request #2974 from alphagov/content-modelling/647-move-ren…

…dering-logic-to-external-gem

(657) Move rendering logic to external gem

Gemfile

@@ -5,6 +5,7 @@ gem "rails", "7.2.2"
 gem "aws-sdk-s3"
 gem "bootsnap", require: false
 gem "bunny"
+gem "content_block_tools"
 gem "dalli"
 gem "fuzzy_match"
 gem "gds-api-adapters"

Gemfile.lock

@@ -119,6 +119,8 @@ GEM
     climate_control (1.2.0)
     concurrent-ruby (1.3.4)
     connection_pool (2.4.1)
+    content_block_tools (0.2.0)
+      actionview (= 7.2.2)
     crack (1.0.0)
       bigdecimal
       rexml
@@ -868,6 +870,7 @@ DEPENDENCIES
   bunny
   byebug
   climate_control
+  content_block_tools
   dalli
   database_cleaner
   factory_bot_rails

app/presenters/content_embed_presenter.rb

@@ -64,14 +64,13 @@ def render_embedded_editions(content)
       content
     end
 
-    # This is a temporary solution to get email address content blocks working
-    # while we agree on a long-term approach that works for everything.
     def get_content_for_edition(edition)
-      if edition.document_type == "content_block_email_address"
-        edition.details[:email_address]
-      else
-        edition.title
-      end
+      ContentBlockTools::ContentBlock.new(
+        document_type: edition.document_type,
+        content_id: edition.document.content_id,
+        title: edition.title,
+        details: edition.details,
+      ).render
     end
   end
 end

app/services/embedded_content_finder_service.rb

@@ -1,10 +1,4 @@
 class EmbeddedContentFinderService
-  ContentReference = Data.define(:document_type, :content_id, :embed_code)
-
-  SUPPORTED_DOCUMENT_TYPES = %w[contact content_block_email_address].freeze
-  UUID_REGEX = /([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})/
-  EMBED_REGEX = /({{embed:(#{SUPPORTED_DOCUMENT_TYPES.join('|')}):#{UUID_REGEX}}})/
-
   def fetch_linked_content_ids(details, locale)
     content_references = details.values.map { |value|
       find_content_references(value)
@@ -21,7 +15,7 @@ def find_content_references(value)
     when Hash
       value.map { |_, v| find_content_references(v) }.flatten.uniq
     when String
-      value.scan(EMBED_REGEX).map { |match| ContentReference.new(document_type: match[1], content_id: match[2], embed_code: match[0]) }
+      ContentBlockTools::ContentBlockReference.find_all_in_document(value)
     else
       []
     end

docs/arch/adr-007-move-content-block-rendering-logic-to-external-gem.md

@@ -0,0 +1,92 @@
+# Decision Record: Move Content Block rendering logic to external gem
+
+## Context
+
+Content blocks within Publishing API are a relatively new concept, that mainly sits with the Content
+Modelling Team. Whilst still in development, the basic concept is that a publishing user will be able
+to embed a "block" of reusable content within a document, and it will then be rendered within the 
+document on publication. When the content of the block changes, all pages that use the block will 
+change to show the latest information.
+
+When a piece of content that uses a content block (or content blocks) is sent to the publishing API,
+we first scan over each field within the content to find embed codes, which currently look like this:
+
+```markdown
+{{embed:$embed_type:$uuid}}
+```
+
+Where `$embed_type` is the type of content to be embedded and `$uuid` is the content ID of the content
+block.
+
+If there are any embed codes found, these are added as "links" to the content within Publishing API
+([See here for more information about links and link expansion](https://github.com/alphagov/publishing-api/blob/main/docs/link-expansion.md)).
+
+When the piece of content is sent to the Content Store (either draft or live), we scan the content
+again for embed codes, and replace the embed code with the actual content block content.
+
+At the moment, this is fairly simple, contacts are replaced with the Title (as contacts work is
+still a work in progress) and email addresses are replaced with the email. However, as this work
+progresses, it is envisioned that embedded content rendering will get significantly more complex.
+
+Additionally, there are places where we need access to the embed logic outside of Publishing API:
+
+### Whitehall Live Preview
+
+At the moment, when a user clicks on "Preview" within Whitehall, it takes the content from the 
+body field, and  uses the Govspeak gem (and some additional Whitehall-specific logic) to 
+render the Govspeak.
+
+Currently, Whitehall or Govspeak does not understand the embed code, so embed codes are left
+untouched:
+
+![Whitehall editor body field showing link to preview button](./images/live-preview-1.png)
+
+![Live preview showing embed code left inplace](./images/live-preview-2.png)
+
+We therefore need a way to handle the embed codes within live preview
+
+### "In-app" preview within Content Block Manager
+
+When editors are making changes to content blocks within the Content Block Manager, it's important
+for users to be able to see the changes they make within the context of the pages that use the
+blocks.
+
+We can't use the existing draft stack because:
+
+a) We want to show the content block changes in the context of the live documents; and
+b) Publishing content block changes to the draft stack could cause confusion when a user
+is previewing the changes to their own content and seeing unpublished changes to a content block
+
+With this in mind, we are proposing that we fetch the HTML of live content and then run a 
+find/replace against the HTML of a content block with the HTML of the current draft, then rendering
+this HTML within our app.
+
+Both of these solutions require each application having knowledge of how a content block will be
+rendered.
+
+Initially we considered having an API endpoint within Publishing API to "preview" a content block.
+However, this was discounted because:
+
+a) We run the risk of overloading the Publishing API with extra API endpoints
+b) There is not much rendering logic in Publishing API itself, and logic will potentially get more
+complex as content blocks evolve
+c) If there are a number of content blocks within a page, this may cause a performance issues when
+making multiple calls to an API
+
+## Decisions
+
+We are proposing moving the Content Block rendering and parsing logic to a new [content_block_tools gem](https://github.com/alphagov/content_block_tools)
+
+This will give a single source of truth for how content blocks are rendered, which can be easily pulled
+into other applications.
+
+## Consequences
+
+Once the work to move the logic into the gem is approved, we will then need to add content_block_tools as
+a dependency in Publishing API to ensure the gem is the only source of truth. 
+
+We will then be able to carry out the two pieces of work as detailed above. They are both contained within
+the Whitehall application, which already has govuk_publishing_components as a dependency.
+
+One possible downside is we will need to release a new version of a gem and ensure it is used in each application
+when a change occurs, but as we don't expect to be making changes regularly, we believe this risk is minimal.

spec/integration/put_content/content_with_embedded_content_spec.rb

@@ -22,7 +22,7 @@
     it "should send transformed content to the content store" do
       put "/v2/content/#{content_id}", params: payload.to_json
 
-      expect_content_store_to_have_received_details_including({ "body" => "#{first_contact.title} #{second_contact.title}" })
+      expect_content_store_to_have_received_details_including({ "body" => "#{presented_details_for(first_contact)} #{presented_details_for(second_contact)}" })
     end
   end
 
@@ -52,7 +52,7 @@
     it "should send transformed content to the content store" do
       put "/v2/content/#{content_id}", params: payload_for_multiple_field_embeds.to_json
 
-      expect_content_store_to_have_received_details_including({ "downtime_message" => first_contact.title.to_s })
+      expect_content_store_to_have_received_details_including({ "downtime_message" => presented_details_for(first_contact) })
     end
   end
 
@@ -128,11 +128,11 @@
             "body" => [
               {
                 "content_type" => "text/govspeak",
-                "content" => first_contact.title,
+                "content" => presented_details_for(first_contact),
               },
               {
                 "content_type" => "text/html",
-                "content" => "<p>#{first_contact.title}</p>",
+                "content" => "<p>#{presented_details_for(first_contact)}</p>",
               },
             ],
           },
@@ -142,11 +142,11 @@
             "body" => [
               {
                 "content_type" => "text/govspeak",
-                "content" => second_contact.title,
+                "content" => presented_details_for(second_contact),
               },
               {
                 "content_type" => "text/html",
-                "content" => "<p>#{second_contact.title}</p>",
+                "content" => "<p>#{presented_details_for(second_contact)}</p>",
               },
             ],
           },
@@ -176,32 +176,7 @@
     it "should send transformed content to the content store" do
       put "/v2/content/#{content_id}", params: payload.to_json
 
-      expect_content_store_to_have_received_details_including({ "body" => array_including({ "content_type" => "text/govspeak", "content" => "#{first_contact.title} #{second_contact.title}" }) })
-    end
-  end
-
-  context "with an embedded email address" do
-    let(:first_email_address) { create(:edition, state: "published", content_store: "live", document_type: "content_block_email_address", details: { email_address: "[email protected]" }) }
-    let(:second_email_address) { create(:edition, state: "published", content_store: "live", document_type: "content_block_email_address", details: { email_address: "[email protected]" }) }
-    let(:document) { create(:document, content_id:) }
-
-    before do
-      payload.merge!(document_type: "press_release", schema_name: "news_article", details: { body: "{{embed:content_block_email_address:#{first_email_address.document.content_id}}} {{embed:content_block_email_address:#{second_email_address.document.content_id}}}" })
-    end
-
-    it "should create links" do
-      expect {
-        put "/v2/content/#{content_id}", params: payload.to_json
-      }.to change(Link, :count).by(2)
-
-      expect(Link.find_by(target_content_id: first_email_address.content_id)).not_to be_nil
-      expect(Link.find_by(target_content_id: second_email_address.content_id)).not_to be_nil
-    end
-
-    it "should send transformed content to the content store" do
-      put "/v2/content/#{content_id}", params: payload.to_json
-
-      expect_content_store_to_have_received_details_including({ "body" => "#{first_email_address.details[:email_address]} #{second_email_address.details[:email_address]}" })
+      expect_content_store_to_have_received_details_including({ "body" => array_including({ "content_type" => "text/govspeak", "content" => "#{presented_details_for(first_contact)} #{presented_details_for(second_contact)}" }) })
     end
   end
 
@@ -226,7 +201,7 @@
     it "should send transformed content to the content store" do
       put "/v2/content/#{content_id}", params: payload.to_json
 
-      expect_content_store_to_have_received_details_including({ "body" => "#{email_address.details[:email_address]} #{contact.title}" })
+      expect_content_store_to_have_received_details_including({ "body" => "#{presented_details_for(email_address)} #{presented_details_for(contact)}" })
     end
   end