diff --git a/app/controllers/admin/features_controller.rb b/app/controllers/admin/features_controller.rb index 49acae4b4ce..e52755174f8 100644 --- a/app/controllers/admin/features_controller.rb +++ b/app/controllers/admin/features_controller.rb @@ -45,7 +45,11 @@ def build_feature def feature_params params.fetch(:feature, {}).permit( - :image, :image_cache, :alt_text, :document_id, :topical_event_id, :offsite_link_id + :alt_text, + :document_id, + :topical_event_id, + :offsite_link_id, + image_attributes: %i[file], ) end diff --git a/app/models/edition/lead_image.rb b/app/models/edition/lead_image.rb index f374bc9156c..103e949d005 100644 --- a/app/models/edition/lead_image.rb +++ b/app/models/edition/lead_image.rb @@ -34,8 +34,6 @@ def lead_image_caption 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 diff --git a/app/models/feature.rb b/app/models/feature.rb index 3474533090c..d5d0e483e6f 100644 --- a/app/models/feature.rb +++ b/app/models/feature.rb @@ -4,14 +4,13 @@ class Feature < ApplicationRecord belongs_to :offsite_link belongs_to :feature_list - has_one :image_new, class_name: "FeaturedImageData", as: :featured_imageable, inverse_of: :featured_imageable + has_one :image, class_name: "FeaturedImageData", as: :featured_imageable, inverse_of: :featured_imageable + accepts_nested_attributes_for :image, reject_if: :all_blank - mount_uploader :image, FeaturedImageUploader, mount_on: :carrierwave_image validates :document, presence: true, unless: ->(feature) { feature.topical_event_id.present? || feature.offsite_link_id.present? } validates :started_at, presence: true - validates :image, presence: true, on: :create + validate :image_is_present validates :alt_text, length: { maximum: 255 } - validates_with ImageValidator, method: :image, size: [960, 640], if: :image_changed? before_validation :set_started_at!, on: :create @@ -50,13 +49,20 @@ def locale feature_list ? feature_list.locale : :en end + def republish_to_publishing_api_async + republish_featurable_to_publishing_api + if document&.editions&.select { |edition| edition.is_a?(Speech) }.present? + Whitehall::PublishingApi.republish_document_async(document) + end + end + private def set_started_at! self.started_at = Time.zone.now end - def image_changed? - changes["carrierwave_image"].present? + def image_is_present + errors.add(:"image.file", "can't be blank") if image.blank? end end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 3b039132a36..c157883cdbe 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -242,6 +242,7 @@ def republish_dependent_documents .includes(:images) .where(images: { id: nil }) .map(&:document) + .uniq(&:id) documents.each { |d| Whitehall::PublishingApi.republish_document_async(d) } end diff --git a/app/models/person.rb b/app/models/person.rb index c6fdcfdf220..27e7e0e5bbd 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -122,7 +122,7 @@ def publishing_api_presenter end def republish_dependent_documents - speeches.map { |speech| Whitehall::PublishingApi.republish_document_async(speech.document) } + speeches.uniq { |speech| speech.document.id }.map { |speech| Whitehall::PublishingApi.republish_document_async(speech.document) } historical_account&.republish_to_publishing_api_async end diff --git a/app/models/worldwide_organisation.rb b/app/models/worldwide_organisation.rb index d75e2ef4815..00c9478bf45 100644 --- a/app/models/worldwide_organisation.rb +++ b/app/models/worldwide_organisation.rb @@ -144,6 +144,7 @@ def republish_dependent_documents .includes(:images) .where(images: { id: nil }) .map(&:document) + .uniq(&:id) documents.each { |d| Whitehall::PublishingApi.republish_document_async(d) } end end diff --git a/app/presenters/publishing_api/featured_documents_presenter.rb b/app/presenters/publishing_api/featured_documents_presenter.rb index e6c6e251868..712aa63c86d 100644 --- a/app/presenters/publishing_api/featured_documents_presenter.rb +++ b/app/presenters/publishing_api/featured_documents_presenter.rb @@ -1,15 +1,18 @@ module PublishingApi module FeaturedDocumentsPresenter def featured_documents(featurable_item, document_limit) - featurable_item.feature_list_for_locale(I18n.locale).current.limit(document_limit).map do |feature| - if feature.document - featured_documents_editioned(feature) - elsif feature.topical_event - featured_documents_topical_event(feature) - elsif feature.offsite_link - featured_documents_offsite_link(feature) + featurable_item + .feature_list_for_locale(I18n.locale).current.limit(document_limit) + .select { |feature| feature.image.all_asset_variants_uploaded? } + .map do |feature| + if feature.document + featured_documents_editioned(feature) + elsif feature.topical_event + featured_documents_topical_event(feature) + elsif feature.offsite_link + featured_documents_offsite_link(feature) + end end - end end private @@ -20,10 +23,7 @@ def featured_documents_editioned(feature) { title: edition.title, href: edition.public_path(locale: feature.feature_list.locale), - image: { - url: feature.image.url, - alt_text: feature.alt_text.presence || "", - }, + image: get_image(feature), summary: Whitehall::GovspeakRenderer.new.govspeak_to_html(edition.summary), public_updated_at: edition.public_timestamp, document_type: edition.display_type, @@ -36,10 +36,7 @@ def featured_documents_topical_event(feature) { title: topical_event.name, href: topical_event.public_path(locale: feature.feature_list.locale), - image: { - url: feature.image.url, - alt_text: feature.alt_text, - }, + image: get_image(feature), summary: Whitehall::GovspeakRenderer.new.govspeak_to_html(topical_event.summary), public_updated_at: topical_event.start_date, document_type: nil, # We don't want a type for topical events @@ -52,14 +49,20 @@ def featured_documents_offsite_link(feature) { title: offsite_link.title, href: offsite_link.url, - image: { - url: feature.image.url, - alt_text: feature.alt_text, - }, + image: get_image(feature), summary: Whitehall::GovspeakRenderer.new.govspeak_to_html(offsite_link.summary), public_updated_at: offsite_link.date, document_type: offsite_link.display_type, } end + + def get_image(feature) + { + url: feature.image.url, + medium_resolution_url: feature.image.url(:s465), + high_resolution_url: feature.image.url(:s712), + alt_text: feature.alt_text.presence || "", + } + end end end diff --git a/app/presenters/publishing_api/speech_presenter.rb b/app/presenters/publishing_api/speech_presenter.rb index 01c507d8ef3..468a726cbf6 100644 --- a/app/presenters/publishing_api/speech_presenter.rb +++ b/app/presenters/publishing_api/speech_presenter.rb @@ -139,8 +139,6 @@ def has_image? end def image_has_all_assets? - return true unless image.instance_of?(FeaturedImageData) - image.all_asset_variants_uploaded? end diff --git a/app/views/admin/features/new.html.erb b/app/views/admin/features/new.html.erb index 5a8d5a685f1..1da1ffb0bc9 100644 --- a/app/views/admin/features/new.html.erb +++ b/app/views/admin/features/new.html.erb @@ -19,9 +19,9 @@ heading_size: "l", }, hint: "Select a 960px wide and 640px tall image to be shown when featuring", - name: "feature[image]", - id: "feature_image", - error_items: errors_for(@feature.errors, :image), + name: "feature[image_attributes][file]", + id: "feature_image_file", + error_items: errors_for(@feature.errors, :"image.file"), } %> <%= render "govuk_publishing_components/components/input", { diff --git a/test/factories/featured_image_data.rb b/test/factories/featured_image_data.rb index cc7aea8e72c..223b77f68c0 100644 --- a/test/factories/featured_image_data.rb +++ b/test/factories/featured_image_data.rb @@ -4,13 +4,13 @@ featured_imageable { build(:topical_event) } after(:build) do |featured_image_data| - featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: "minister-of-funk.960x640.jpg") - featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s960", variant: Asset.variants[:s960], filename: "s960_minister-of-funk.960x640.jpg") - featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s712", variant: Asset.variants[:s712], filename: "s712_minister-of-funk.960x640.jpg") - featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s630", variant: Asset.variants[:s630], filename: "s630_minister-of-funk.960x640.jpg") - featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s465", variant: Asset.variants[:s465], filename: "s465_minister-of-funk.960x640.jpg") - featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s300", variant: Asset.variants[:s300], filename: "s300_minister-of-funk.960x640.jpg") - featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s216", variant: Asset.variants[:s216], filename: "s216_minister-of-funk.960x640.jpg") + featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: featured_image_data.filename) + featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s960", variant: Asset.variants[:s960], filename: "s960_#{featured_image_data.filename}") + featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s712", variant: Asset.variants[:s712], filename: "s712_#{featured_image_data.filename}") + featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s630", variant: Asset.variants[:s630], filename: "s630_#{featured_image_data.filename}") + featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s465", variant: Asset.variants[:s465], filename: "s465_#{featured_image_data.filename}") + featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s300", variant: Asset.variants[:s300], filename: "s300_#{featured_image_data.filename}") + featured_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s216", variant: Asset.variants[:s216], filename: "s216_#{featured_image_data.filename}") end end end diff --git a/test/factories/features.rb b/test/factories/features.rb index 04b4b0564a5..5f19123f9ca 100644 --- a/test/factories/features.rb +++ b/test/factories/features.rb @@ -1,7 +1,7 @@ FactoryBot.define do factory :feature do document - image { image_fixture_file } + image { build(:featured_image_data) } trait :with_topical_event_association do topical_event diff --git a/test/functional/admin/features_controller_test.rb b/test/functional/admin/features_controller_test.rb index 61bed4390ea..4688bcdb0e4 100644 --- a/test/functional/admin/features_controller_test.rb +++ b/test/functional/admin/features_controller_test.rb @@ -58,7 +58,9 @@ class Admin::FeaturesControllerTest < ActionController::TestCase params = { document_id: edition.document_id, - image: upload_fixture("images/960x640_gif.gif"), + image_attributes: { + file: upload_fixture("images/960x640_gif.gif"), + }, alt_text: "some text", } @@ -68,6 +70,7 @@ class Admin::FeaturesControllerTest < ActionController::TestCase post :create, params: { feature_list_id: feature_list.id, feature: params } assert_equal edition.document_id, Feature.last.document_id + assert_equal "960x640_gif.gif", Feature.last.image.filename end test "post :feature creates a feature and republishes the document & world location news when the featurable is an world location news" do @@ -78,7 +81,9 @@ class Admin::FeaturesControllerTest < ActionController::TestCase params = { document_id: edition.document_id, - image: upload_fixture("images/960x640_gif.gif"), + image_attributes: { + file: upload_fixture("images/960x640_gif.gif"), + }, alt_text: "some text", } @@ -88,5 +93,6 @@ class Admin::FeaturesControllerTest < ActionController::TestCase post :create, params: { feature_list_id: feature_list.id, feature: params } assert_equal edition.document_id, Feature.last.document_id + assert_equal "960x640_gif.gif", Feature.last.image.filename end end diff --git a/test/functional/admin/organisations_controller_test.rb b/test/functional/admin/organisations_controller_test.rb index 2716cf88d3b..3478ec19c47 100644 --- a/test/functional/admin/organisations_controller_test.rb +++ b/test/functional/admin/organisations_controller_test.rb @@ -356,7 +356,9 @@ def example_organisation_attributes feature_list = organisation.load_or_create_feature_list("en") feature_list.features.create!( topical_event:, - image: image_fixture_file, + image_attributes: { + file: image_fixture_file, + }, alt_text: "Image alternative text", ) diff --git a/test/unit/app/models/edition/lead_image_test.rb b/test/unit/app/models/edition/lead_image_test.rb index 79296d904dd..1dfe27e5565 100644 --- a/test/unit/app/models/edition/lead_image_test.rb +++ b/test/unit/app/models/edition/lead_image_test.rb @@ -57,12 +57,4 @@ class Edition::LeadImageTest < ActiveSupport::TestCase 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 - image = build(:featured_image_data) - organisation = build(:organisation, default_news_image: image) - model = stub("Target", { lead_image: nil, lead_organisations: [], organisations: [organisation] }).extend(Edition::LeadImage) - - assert model.lead_image_has_all_assets? - end end diff --git a/test/unit/app/models/feature_test.rb b/test/unit/app/models/feature_test.rb index 3ce4c530ed6..df7d5d15d5c 100644 --- a/test/unit/app/models/feature_test.rb +++ b/test/unit/app/models/feature_test.rb @@ -9,18 +9,9 @@ class FeatureTest < ActiveSupport::TestCase assert_not build(:feature, image: nil).valid? end - test "valid without image on update" do - # This is to work around the fact that carrierwave will consider the image invalid - # because the file will have been moved to the 'clean' folder by the virus scanner - # and therefore not in the location that it expects - feature = create(:feature) - feature.image = nil - assert feature.valid? - end - test "started_at set by default on creation" do feature = Feature.create!( - image: image_fixture_file, + image: build(:featured_image_data), feature_list: create(:feature_list), document: create(:document), alt_text: "foo", @@ -42,28 +33,4 @@ class FeatureTest < ActiveSupport::TestCase assert feature.end! assert_equal Time.zone.now, feature.reload.ended_at end - - test "rejects SVG logo uploads" do - svg_image = File.open(Rails.root.join("test/fixtures/images/test-svg.svg")) - feature = build(:feature, image: svg_image) - - assert_not feature.valid? - assert_includes feature.errors.map(&:full_message), "Image You are not allowed to upload \"svg\" files, allowed types: jpg, jpeg, gif, png" - end - - test "rejects non-image file uploads" do - non_image_file = File.open(Rails.root.join("test/fixtures/folders.zip")) - feature = build(:feature, image: non_image_file) - - assert_not feature.valid? - assert_includes feature.errors.map(&:full_message), "Image You are not allowed to upload \"zip\" files, allowed types: jpg, jpeg, gif, png" - end - - test "accepts valid image uploads" do - jpg_image = File.open(Rails.root.join("test/fixtures/big-cheese.960x640.jpg")) - feature = build(:feature, image: jpg_image) - - assert feature - assert_empty feature.errors - end end diff --git a/test/unit/app/models/featured_image_data_test.rb b/test/unit/app/models/featured_image_data_test.rb index bdec3ee9b10..77693909df7 100644 --- a/test/unit/app/models/featured_image_data_test.rb +++ b/test/unit/app/models/featured_image_data_test.rb @@ -70,7 +70,8 @@ class FeaturedImageDataTest < ActiveSupport::TestCase test "#republish_on_assets_ready should republish organisation and associations if assets are ready" do organisation = create(:organisation, :with_default_news_image) - news_article = create(:news_article, organisations: [organisation]) + news_article = create(:news_article, :published, organisations: [organisation]) + create(:news_article, :draft, organisations: [organisation], document: news_article.document) Whitehall::PublishingApi.expects(:republish_async).with(organisation).once Whitehall::PublishingApi.expects(:republish_document_async).with(news_article.document).once @@ -80,10 +81,11 @@ class FeaturedImageDataTest < ActiveSupport::TestCase test "#republish_on_assets_ready should republish worldwide organisation and associations if assets are ready" do worldwide_organisation = create(:worldwide_organisation, :with_default_news_image) - news_article = create(:news_article_world_news_story, worldwide_organisations: [worldwide_organisation]) + news_article = create(:news_article_world_news_story, :published, worldwide_organisations: [worldwide_organisation]) + create(:news_article_world_news_story, :draft, worldwide_organisations: [worldwide_organisation], document: news_article.document) Whitehall::PublishingApi.expects(:republish_async).with(worldwide_organisation).once - Whitehall::PublishingApi.expects(:republish_document_async).with(news_article.document) + Whitehall::PublishingApi.expects(:republish_document_async).with(news_article.document).once worldwide_organisation.default_news_image.republish_on_assets_ready end @@ -98,12 +100,13 @@ class FeaturedImageDataTest < ActiveSupport::TestCase test "#republish_on_assets_ready should republish person and associations if assets are ready" do person = create(:person, :with_image) - speech = create(:speech, role_appointment: create(:role_appointment, role: create(:ministerial_role), person:)) + speech = create(:published_speech, role_appointment: create(:role_appointment, role: create(:ministerial_role), person:)) + create(:draft_speech, role_appointment: create(:role_appointment, role: create(:ministerial_role), person:), document: speech.document) create(:historical_account, person:) Whitehall::PublishingApi.expects(:republish_async).with(person).once Whitehall::PublishingApi.expects(:republish_async).with(person.historical_account).once - Whitehall::PublishingApi.expects(:republish_document_async).with(speech.document) + Whitehall::PublishingApi.expects(:republish_document_async).with(speech.document).once person.image.republish_on_assets_ready end @@ -116,6 +119,39 @@ class FeaturedImageDataTest < ActiveSupport::TestCase take_part_page.image.republish_on_assets_ready end + test "#republish_on_assets_ready should republish feature via organisation if assets are ready" do + speech1 = create(:speech) + speech2 = create(:speech) + document = create(:document, editions: [speech1, speech2]) + + organisation = create(:organisation) + feature_list = create(:feature_list, featurable: organisation, locale: :en) + feature = create(:feature, document:, feature_list:) + featurable = feature.feature_list.featurable + + Whitehall::PublishingApi.expects(:republish_async).with(featurable).once + Whitehall::PublishingApi.expects(:republish_document_async).with(document).once + + feature.image.republish_on_assets_ready + end + + test "#republish_on_assets_ready should republish feature via world location news if assets are ready" do + speech1 = create(:speech) + speech2 = create(:speech) + document = create(:document, editions: [speech1, speech2]) + + world_location = create(:world_location) + world_location_news = world_location.world_location_news + feature_list = create(:feature_list, featurable: world_location_news, locale: :en) + feature = create(:feature, document:, feature_list:) + featurable = feature.feature_list.featurable + + Whitehall::PublishingApi.expects(:republish_async).with(featurable).once + Whitehall::PublishingApi.expects(:republish_document_async).with(document).once + + feature.image.republish_on_assets_ready + end + test "#republish_on_assets_ready should not run any republishing action if assets are not ready" do person = create(:person, :with_image) person.image.assets.destroy_all diff --git a/test/unit/app/presenters/publishing_api/featured_documents_presenter_test.rb b/test/unit/app/presenters/publishing_api/featured_documents_presenter_test.rb index d38e9fb742c..d2c8df4841f 100644 --- a/test/unit/app/presenters/publishing_api/featured_documents_presenter_test.rb +++ b/test/unit/app/presenters/publishing_api/featured_documents_presenter_test.rb @@ -24,14 +24,18 @@ class PublishingApi::FeaturedDocumentsPresenterTest < ActiveSupport::TestCase expected_ordered_featured_documents = [ { title: case_study.title, href: "/government/case-studies/case-study-title#{locale[:suffix]}", - image: { url: first_feature.image.url, + image: { url: "#{Plek.asset_root}/media/asset_manager_id_original/minister-of-funk.960x640.jpg", + medium_resolution_url: "#{Plek.asset_root}/media/asset_manager_id_s465/s465_minister-of-funk.960x640.jpg", + high_resolution_url: "#{Plek.asset_root}/media/asset_manager_id_s712/s712_minister-of-funk.960x640.jpg", alt_text: "" }, summary: Whitehall::GovspeakRenderer.new.govspeak_to_html(case_study.summary), public_updated_at: case_study.public_timestamp, document_type: I18n.t("document.type.case_study.one") }, { title: news_article.title, href: "/government/news/news-title#{locale[:suffix]}", - image: { url: second_feature.image.url, + image: { url: "#{Plek.asset_root}/media/asset_manager_id_original/minister-of-funk.960x640.jpg", + medium_resolution_url: "#{Plek.asset_root}/media/asset_manager_id_s465/s465_minister-of-funk.960x640.jpg", + high_resolution_url: "#{Plek.asset_root}/media/asset_manager_id_s712/s712_minister-of-funk.960x640.jpg", alt_text: "" }, summary: Whitehall::GovspeakRenderer.new.govspeak_to_html(news_article.summary), public_updated_at: news_article.public_timestamp, @@ -62,8 +66,10 @@ class PublishingApi::FeaturedDocumentsPresenterTest < ActiveSupport::TestCase expected_ordered_featured_documents = [ { title: topical_event.name, href: "/government/topical-events/topical_event_1#{locale[:suffix]}", - image: { url: feature.image.url, - alt_text: feature.alt_text }, + image: { url: "#{Plek.asset_root}/media/asset_manager_id_original/minister-of-funk.960x640.jpg", + medium_resolution_url: "#{Plek.asset_root}/media/asset_manager_id_s465/s465_minister-of-funk.960x640.jpg", + high_resolution_url: "#{Plek.asset_root}/media/asset_manager_id_s712/s712_minister-of-funk.960x640.jpg", + alt_text: "" }, summary: Whitehall::GovspeakRenderer.new.govspeak_to_html(topical_event.summary), public_updated_at: topical_event.start_date, document_type: nil }, @@ -90,8 +96,10 @@ class PublishingApi::FeaturedDocumentsPresenterTest < ActiveSupport::TestCase expected_ordered_featured_documents = [ { title: offsite_link.title, href: offsite_link.url, - image: { url: feature.image.url, - alt_text: feature.alt_text }, + image: { url: "#{Plek.asset_root}/media/asset_manager_id_original/minister-of-funk.960x640.jpg", + medium_resolution_url: "#{Plek.asset_root}/media/asset_manager_id_s465/s465_minister-of-funk.960x640.jpg", + high_resolution_url: "#{Plek.asset_root}/media/asset_manager_id_s712/s712_minister-of-funk.960x640.jpg", + alt_text: "" }, summary: Whitehall::GovspeakRenderer.new.govspeak_to_html(offsite_link.summary), public_updated_at: offsite_link.date, document_type: offsite_link.display_type }, @@ -115,4 +123,33 @@ class PublishingApi::FeaturedDocumentsPresenterTest < ActiveSupport::TestCase assert_equal [create(:published_case_study).title], (presented_locations.map { |presented_location| presented_location[:title] }) end + + test("filters out featured documents if feature image assets are missing") do + case_study = create(:published_case_study) + first_feature = build(:feature, document: case_study.document, ordering: 1) + news_article = create(:published_news_article) + second_feature = build(:feature, document: news_article.document, ordering: 2) + second_feature.image.assets = [] + featured_documents_display_limit = 5 + + world_location = create(:world_location) + + create(:feature_list, featurable: world_location.world_location_news, features: [second_feature, first_feature]) + + expected_ordered_featured_documents = [ + { + title: case_study.title, + href: "/government/case-studies/case-study-title", + image: { url: "#{Plek.asset_root}/media/asset_manager_id_original/minister-of-funk.960x640.jpg", + medium_resolution_url: "#{Plek.asset_root}/media/asset_manager_id_s465/s465_minister-of-funk.960x640.jpg", + high_resolution_url: "#{Plek.asset_root}/media/asset_manager_id_s712/s712_minister-of-funk.960x640.jpg", + alt_text: "" }, + summary: Whitehall::GovspeakRenderer.new.govspeak_to_html(case_study.summary), + public_updated_at: case_study.public_timestamp, + document_type: I18n.t("document.type.case_study.one"), + }, + ] + + assert_equal expected_ordered_featured_documents, featured_documents(world_location.world_location_news, featured_documents_display_limit) + 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 1c7e89dd023..dce7702be7a 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 @@ -238,6 +238,7 @@ class NewsArticleWithImage < TestCase lead_image_alt_text: "Bar", lead_image_caption: "Baz", images: [build(:image)], + lead_image: build(:image), ) end diff --git a/test/unit/app/presenters/publishing_api/speech_presenter_test.rb b/test/unit/app/presenters/publishing_api/speech_presenter_test.rb index 7d1906d7955..368e3f7a948 100644 --- a/test/unit/app/presenters/publishing_api/speech_presenter_test.rb +++ b/test/unit/app/presenters/publishing_api/speech_presenter_test.rb @@ -209,9 +209,7 @@ def iso8601_regex create( :feature, document: speech.document, - image: File.open( - Rails.root.join("test/fixtures/images/960x640_gif.gif"), - ), + image: build(:featured_image_data), alt_text: "featured image", ) end @@ -220,9 +218,7 @@ def iso8601_regex create( :feature, document: speech.document, - image: File.open( - Rails.root.join("test/fixtures/images/960x640_gif.gif"), - ), + image: build(:featured_image_data, file: upload_fixture("big-cheese.960x640.jpg", "image/jpg")), alt_text: "featured image two", ) end @@ -230,7 +226,15 @@ def iso8601_regex it "presents the most recent featured image" do details = presented.content[:details] assert_equal("featured image two", details[:image][:alt_text]) - assert_match(/960x640_gif.gif$/, details[:image][:url]) + assert_match(/big-cheese.960x640.jpg$/, details[:image][:url]) + end + + it "does not present the featured image if any assets are missing" do + feature_two.image.assets.destroy_all + + details = presented.content[:details] + + assert_nil details[:image] end end @@ -241,7 +245,7 @@ def iso8601_regex assert_match(/minister-of-funk.960x640.jpg$/, details[:image][:url]) end - test "it filters out the person image if it has missing assets" do + test "does not present the person image if it has missing assets" do person.image.assets.destroy_all details = presented.content[:details]