Skip to content

Commit

Permalink
Fix bug with update_lead_image method
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
davidgisbey committed Oct 31, 2023
1 parent 9374970 commit e757324
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 3 deletions.
5 changes: 4 additions & 1 deletion app/models/edition/custom_lead_image.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand Down
24 changes: 22 additions & 2 deletions test/unit/app/models/edition/custom_lead_image_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit e757324

Please sign in to comment.