From e757324b24ba7f288b4951071e6d9ddb1bfb8684 Mon Sep 17 00:00:00 2001
From: davidgisbey <david.gisbey@digital.cabinet-office.gov.uk>
Date: Tue, 31 Oct 2023 09:20:26 +0000
Subject: [PATCH] Fix bug with update_lead_image method

The 'and return if' isn't returning when the image_display_option is
no_image or organisation_image.

This means that when an image is created and the lead_image isn't set
the assoication is created.

It still wouldn't be pushed downstream to Publishing API, but we
shouldn't have lead images sitting in the db when there isn't a
lead image.

This updates the method to explicitly call return within the conditional
which resolves the issue.
---
 app/models/edition/custom_lead_image.rb       |  5 +++-
 .../models/edition/custom_lead_image_test.rb  | 24 +++++++++++++++++--
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/app/models/edition/custom_lead_image.rb b/app/models/edition/custom_lead_image.rb
index 46b9a063188..0acffc8ad10 100644
--- a/app/models/edition/custom_lead_image.rb
+++ b/app/models/edition/custom_lead_image.rb
@@ -15,7 +15,10 @@ def image_disallowed_in_body_text?(index)
   end
 
   def update_lead_image
-    remove_lead_image and return if %w[no_image organisation_image].include?(image_display_option)
+    if %w[no_image organisation_image].include?(image_display_option)
+      remove_lead_image
+      return
+    end
 
     return if lead_image.present? || images.blank?
 
diff --git a/test/unit/app/models/edition/custom_lead_image_test.rb b/test/unit/app/models/edition/custom_lead_image_test.rb
index 4ff036d2b8b..7dfb3d61f4a 100644
--- a/test/unit/app/models/edition/custom_lead_image_test.rb
+++ b/test/unit/app/models/edition/custom_lead_image_test.rb
@@ -55,34 +55,54 @@ def body_text_valid(body)
     edition_lead_image = build(:edition_lead_image)
     edition = build(:news_article, image_display_option: "no_image", edition_lead_image:)
 
+    edition.stubs(:images).returns([edition_lead_image.image])
+
     edition_lead_image
     .expects(:destroy!)
     .once
 
+    edition
+    .expects(:build_edition_lead_image)
+    .never
+
     edition.update_lead_image
   end
 
   test "#update_lead_image deletes the associated edition_lead_image if image_display_option is 'organisation_image'" do
     edition_lead_image = build(:edition_lead_image)
     edition = build(:news_article, image_display_option: "organisation_image", edition_lead_image:)
+    edition.stubs(:images).returns([edition_lead_image.image])
 
     edition_lead_image
     .expects(:destroy!)
     .once
 
+    edition
+    .expects(:build_edition_lead_image)
+    .never
+
     edition.update_lead_image
   end
 
   test "#update_lead_image returns nil if lead_image is present" do
     edition = build(:news_article)
-    build(:image)
-    edition.stubs(:lead_image).returns(edition)
+    image = build(:image)
+    edition.stubs(:lead_image).returns(image)
+
+    edition
+    .expects(:build_edition_lead_image)
+    .never
 
     assert_nil edition.update_lead_image
   end
 
   test "#update_lead_image returns nil if no images are present" do
     edition = build(:news_article)
+
+    edition
+    .expects(:build_edition_lead_image)
+    .never
+
     assert_nil edition.update_lead_image
   end
 end