Skip to content

Commit

Permalink
Filter out unhealthy lead images from presenters payload
Browse files Browse the repository at this point in the history
  • Loading branch information
lauraghiorghisor-tw committed Oct 4, 2023
1 parent 80e5ea8 commit c19ccda
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 2 deletions.
6 changes: 6 additions & 0 deletions app/models/edition/lead_image.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ def lead_image_caption
end
end

def lead_image_has_all_assets?
return true unless image_data.respond_to?(:all_asset_variants_uploaded?)

image_data.all_asset_variants_uploaded?
end

private

def placeholder_image_url
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/publishing_api/case_study_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def details
first_public_at:,
format_display_type: item.display_type_key,
}
details_hash[:image] = if image_available? && image_required?
details_hash[:image] = if image_available? && image_required? && item.lead_image_has_all_assets?
image_details
else
{ url: "", caption: nil, alt_text: "" }
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/publishing_api/news_article_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def initialize(news_article)
end

def call
return {} unless news_article.has_lead_image?
return {} unless news_article.has_lead_image? && news_article.lead_image_has_all_assets?

{
image: {
Expand Down
27 changes: 27 additions & 0 deletions test/unit/app/models/edition/lead_image_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,31 @@ class Edition::LeadImageTest < ActiveSupport::TestCase
assert_equal "url", model.lead_image_url
assert_equal "alt_text", model.lead_image_alt_text
end

test "#lead_image_has_all_assets? returns false if the lead image (ImageData) has missing assets" do
image_with_missing_assets = build(:image)
image_with_missing_assets.image_data.use_non_legacy_endpoints = true
image_with_missing_assets.image_data.assets << [build(:asset)]

model = stub("Target", { images: [image_with_missing_assets] }).extend(Edition::LeadImage)

assert_not model.lead_image_has_all_assets?
end

test "#lead_image_has_all_assets? returns true if the lead image (ImageData) has all assets" do
image_with_missing_assets = build(:image_with_assets)

model = stub("Target", { images: [image_with_missing_assets] }).extend(Edition::LeadImage)

assert model.lead_image_has_all_assets?
end

test "#lead_image_has_all_assets? returns true if the lead image data doesn't implement all_asset_variants_uploaded?" do
# e.g. DefaultNewsOrganisationImageData doesn't yet implement all_asset_variants_uploaded?
image = build(:default_news_organisation_image_data)
organisation = build(:organisation, default_news_image: image)
model = stub("Target", { images: [], lead_organisations: [], organisations: [organisation] }).extend(Edition::LeadImage)

assert model.lead_image_has_all_assets?
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ class NewsArticleWithImage < TestCase
high_resolution_lead_image_url: "/foo-large",
lead_image_alt_text: "Bar",
lead_image_caption: "Baz",
images: [build(:image)],
)
end

Expand Down

0 comments on commit c19ccda

Please sign in to comment.