From c19ccda9f165877d9453860a6dff7340f59d66b3 Mon Sep 17 00:00:00 2001 From: Laura Ghiorghisor Date: Thu, 28 Sep 2023 10:58:48 +0100 Subject: [PATCH] Filter out unhealthy lead images from presenters payload --- app/models/edition/lead_image.rb | 6 +++++ .../publishing_api/case_study_presenter.rb | 2 +- .../publishing_api/news_article_presenter.rb | 2 +- .../app/models/edition/lead_image_test.rb | 27 +++++++++++++++++++ .../news_article_presenter_test.rb | 1 + 5 files changed, 36 insertions(+), 2 deletions(-) diff --git a/app/models/edition/lead_image.rb b/app/models/edition/lead_image.rb index f223336cb9d..a8799a1d114 100644 --- a/app/models/edition/lead_image.rb +++ b/app/models/edition/lead_image.rb @@ -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 diff --git a/app/presenters/publishing_api/case_study_presenter.rb b/app/presenters/publishing_api/case_study_presenter.rb index 601b1de74d7..7f13976e8d7 100644 --- a/app/presenters/publishing_api/case_study_presenter.rb +++ b/app/presenters/publishing_api/case_study_presenter.rb @@ -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: "" } diff --git a/app/presenters/publishing_api/news_article_presenter.rb b/app/presenters/publishing_api/news_article_presenter.rb index e1b845846df..1fb8a964608 100644 --- a/app/presenters/publishing_api/news_article_presenter.rb +++ b/app/presenters/publishing_api/news_article_presenter.rb @@ -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: { diff --git a/test/unit/app/models/edition/lead_image_test.rb b/test/unit/app/models/edition/lead_image_test.rb index cfd78b7f2a2..7dd51ad6a04 100644 --- a/test/unit/app/models/edition/lead_image_test.rb +++ b/test/unit/app/models/edition/lead_image_test.rb @@ -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 diff --git a/test/unit/app/presenters/publishing_api/news_article_presenter_test.rb b/test/unit/app/presenters/publishing_api/news_article_presenter_test.rb index cad2f504276..c0b55b21ff2 100644 --- a/test/unit/app/presenters/publishing_api/news_article_presenter_test.rb +++ b/test/unit/app/presenters/publishing_api/news_article_presenter_test.rb @@ -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