Refactor body_does_not_contain_lead_image validator

This commit refactors the `body_does_not_contain_lead_image` custom
validation method to reduce its coupling to Govspeak.

Right now it uses regular expressions to check if the lead image has
been embedded into the document body. This relies upon the validator
knowing the specific Govspeak syntax that would be used to embed the
image (of which there are two different variants).

This refactor takes a more 'black box' approach by rendering the
Govspeak as HTML, and then inspecting the output to see if it contains
the lead image. This way the validator doesn't need to parse the
Govspeak syntax directly – making it more robust against future changes
to the syntax.

I imagine this new approach is slightly less performant. It now renders
and inspects the resultant HTML, rather than simply matching a regex
against the raw Govspeak. However I expect this will have negligible
overall impact on performance, so would be a net benefit given the
improvements to code readability and cleanliness.

app/helpers/govspeak_helper.rb

@@ -52,7 +52,7 @@ def prepare_images(images)
       .select { |image| image.image_data&.all_asset_variants_uploaded? }
       .map do |image|
       {
-        id: image.image_data.carrierwave_image,
+        id: image.filename,
         image_data_id: image.image_data_id,
         edition_id: image.edition_id,
         alt_text: image.alt_text,

app/models/edition/custom_lead_image.rb

@@ -2,7 +2,7 @@ module Edition::CustomLeadImage
   extend ActiveSupport::Concern
 
   included do
-    validate :body_does_not_contain_lead_image_markdown
+    validate :body_does_not_contain_lead_image
   end
 
   def update_lead_image
@@ -33,21 +33,14 @@ def remove_lead_image
     edition_lead_image.destroy!
   end
 
-  def body_does_not_contain_lead_image_markdown
-    return if lead_image.blank?
+  def body_does_not_contain_lead_image
+    return if lead_image.blank? || images.none?
 
-    if body_contains_lead_image_filename_markdown? || body_contains_lead_image_index_markdown?
+    html = Whitehall::GovspeakRenderer.new.govspeak_edition_to_html(self)
+    doc = Nokogiri::HTML::DocumentFragment.parse(html)
+
+    if doc.css("img").any? { |img| img[:src] == lead_image.url }
       errors.add(:body, "cannot have a reference to the lead image in the text")
     end
   end
-
-  def body_contains_lead_image_filename_markdown?
-    body.match?(/\[Image:\s*#{Regexp.escape(lead_image.filename)}\s*\]/)
-  end
-
-  def body_contains_lead_image_index_markdown?
-    lead_image_index = images.find_index(lead_image)
-
-    body.match?(/^!!#{lead_image_index + 1}([^\d]|$)/)
-  end
 end

test/unit/app/helpers/govspeak_helper_test.rb

@@ -268,15 +268,15 @@ def edition_with_attachment(body:)
   test "embeds images using !!number syntax" do
     edition = build(:published_news_article, body: "!!1")
     image_data = create(:image_data, id: 1)
-    edition.stubs(:images).returns([OpenStruct.new(alt_text: "My Alt", url: "https://some.cdn.com/image.jpg", image_data: ImageData.find(image_data.id))])
+    edition.stubs(:images).returns([OpenStruct.new(alt_text: "My Alt", url: "https://some.cdn.com/image.jpg", filename: "image.jpg", image_data: ImageData.find(image_data.id))])
     html = govspeak_edition_to_html(edition)
     assert_select_within_html html, ".govspeak figure.image.embedded img[src='https://some.cdn.com/image.jpg']"
   end
 
   test "embeds images using [Image:] syntax" do
-    edition = build(:published_news_article, body: "[Image: minister-of-funk.960x640.jpg]")
+    edition = build(:published_news_article, body: "[Image: image.jpg]")
     image_data = create(:image_data, id: 1)
-    edition.stubs(:images).returns([OpenStruct.new(alt_text: "My Alt", url: "https://some.cdn.com/image.jpg", image_data: ImageData.find(image_data.id))])
+    edition.stubs(:images).returns([OpenStruct.new(alt_text: "My Alt", url: "https://some.cdn.com/image.jpg", filename: "image.jpg", image_data: ImageData.find(image_data.id))])
     html = govspeak_edition_to_html(edition)
     assert_select_within_html html, ".govspeak figure.image.embedded img[src='https://some.cdn.com/image.jpg']"
   end

test/unit/app/models/edition/custom_lead_image_test.rb

@@ -3,11 +3,18 @@
 class Edition::CustomLeadImageTest < ActiveSupport::TestCase
   include ActionDispatch::TestProcess
 
+  def stubbed_image(name)
+    build(:image).tap do |image|
+      image.stubs(:filename).returns(name)
+      image.stubs(:url).returns("http://example.com/#{name}")
+    end
+  end
+
   def edition_with_custom_lead_image(options = {})
-    image1 = build(:image)
-    image2 = build(:image)
+    non_lead_image = stubbed_image("non_lead_image.jpg")
+    lead_image = stubbed_image("lead_image.jpg")
 
-    build(:draft_news_article, options.merge(images: [image1, image2], lead_image: image2))
+    build(:draft_news_article, options.merge(images: [non_lead_image, lead_image], lead_image:))
   end
 
   def body_text_valid(body)
@@ -19,13 +26,13 @@ def body_text_valid(body)
     assert_not body_text_valid("foo\n!!2 \nbar")
     assert_not body_text_valid("foo\n!!2")
     assert_not body_text_valid("foo\n!!2s\nbar")
-    assert_not body_text_valid("foo\[Image: minister-of-funk.960x640.jpg]\nbar")
-    assert_not body_text_valid("foo\[Image:minister-of-funk.960x640.jpg]\nbar")
-    assert_not body_text_valid("foo\[Image:     minister-of-funk.960x640.jpg]\nbar")
-    assert_not body_text_valid("foo\[Image:  \n  minister-of-funk.960x640.jpg]\nbar")
+    assert_not body_text_valid("foo\n[Image: lead_image.jpg]\nbar")
+    assert_not body_text_valid("foo\n[Image:lead_image.jpg]\nbar")
+    assert_not body_text_valid("foo\n[Image:     lead_image.jpg]\nbar")
+    assert_not body_text_valid("foo\n[Image:  \n  lead_image.jpg]\nbar")
     assert body_text_valid("foo\n!!20\nbar")
     assert body_text_valid("foo\nfoo bar !!2\nbar")
-    assert body_text_valid("foo\[Image: anotherfilename.jpg]\nbar")
+    assert body_text_valid("foo\[Image: non_lead_image.jpg]\nbar")
   end
 
   test "#update_lead_image updates the lead_image association to the oldest image" do