Skip to content

Commit

Permalink
Allow WWOrganisation to use FeaturedImageData
Browse files Browse the repository at this point in the history
This Change makes WWOrganisation use Asset manager ids instead of legacy url path for WWOrganisation default news images. This should make things more consistent across these classes.

Co-authored-by: Iris Lau <[email protected]>
  • Loading branch information
syed-ali-tw and yuetylauiris committed Nov 2, 2023
1 parent e50b4ed commit fb98fd0
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 30 deletions.
16 changes: 1 addition & 15 deletions app/models/worldwide_organisation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class WorldwideOrganisation < ApplicationRecord

has_many :editions, through: :edition_worldwide_organisations

belongs_to :default_news_image, class_name: "DefaultNewsOrganisationImageData", foreign_key: :default_news_organisation_image_data_id
has_one :default_news_image, class_name: "FeaturedImageData", as: :featured_imageable, inverse_of: :featured_imageable

accepts_nested_attributes_for :default_news_image, reject_if: :all_blank

Expand All @@ -43,20 +43,6 @@ class WorldwideOrganisation < ApplicationRecord
extend FriendlyId
friendly_id

after_save do
# If the default news organisation image changes we need to republish all
# news articles belonging to the worldwide organisation
if saved_change_to_default_news_organisation_image_data_id?
documents = NewsArticle
.in_worldwide_organisation(self)
.includes(:images)
.where(images: { id: nil })
.map(&:document)

documents.each { |d| Whitehall::PublishingApi.republish_document_async(d) }
end
end

after_commit :republish_embassies_index_page_to_publishing_api, :republish_worldwide_offices

# I'm trying to use a domain centric design rather than a persistence
Expand Down
11 changes: 8 additions & 3 deletions app/workers/asset_manager_create_asset_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ def enqueue_downstream_service_updates(assetable_id, assetable_type, attachable_
if assetable_type == FeaturedImageData.to_s
assetable = FeaturedImageData.find(assetable_id)
featured_imageable = assetable.featured_imageable
if assetable.featured_imageable_type.to_s == Organisation.to_s

if %w[Organisation WorldwideOrganisation].include?(assetable.featured_imageable_type)
# If the default news organisation image changes we need to republish all
# news articles belonging to the organisation
republish_news_articles_with_default_news_image(featured_imageable)
Expand All @@ -61,8 +62,12 @@ def enqueue_downstream_service_updates(assetable_id, assetable_type, attachable_
end

def republish_news_articles_with_default_news_image(featured_imageable)
documents = NewsArticle
.in_organisation(featured_imageable)
query = if featured_imageable.instance_of?(Organisation)
NewsArticle.in_organisation(featured_imageable)
else
NewsArticle.in_worldwide_organisation(featured_imageable)
end
documents = query
.includes(:images)
.where(images: { id: nil })
.map(&:document)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Admin::WorldwideOrganisations::Show::SummaryListComponentTest < ViewCompon
test "renders the correct items when all fields are completed and only 1 world location & sponsoring organisation" do
world_location = build_stubbed(:world_location)
sponsoring_organisation = build_stubbed(:organisation)
news_image = build_stubbed(:default_news_organisation_image_data)
news_image = build_stubbed(:featured_image_data)
worldwide_organisation = build_stubbed(
:worldwide_organisation,
logo_formatted_name: "Optional log formatted name",
Expand Down
5 changes: 5 additions & 0 deletions test/factories/worldwide_organisations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,10 @@
worldwide_organisation.world_locations << FactoryBot.create(:world_location)
end
end
trait(:with_default_news_image) do
after :build do |organisation|
organisation.default_news_image = build(:featured_image_data)
end
end
end
end
9 changes: 0 additions & 9 deletions test/unit/app/models/edition/lead_image_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +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
# e.g. DefaultNewsOrganisationImageData doesn't yet implement all_asset_variants_uploaded?
image = build(:featured_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
4 changes: 2 additions & 2 deletions test/unit/app/models/worldwide_organisation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class WorldwideOrganisationTest < ActiveSupport::TestCase
end

test "can have a default news article image" do
image = build(:default_news_organisation_image_data)
image = build(:featured_image_data)
worldwide_organisation = build(:worldwide_organisation, default_news_image: image)
assert_equal image, worldwide_organisation.default_news_image
end
Expand Down Expand Up @@ -334,7 +334,7 @@ class WorldwideOrganisationTest < ActiveSupport::TestCase
end

world_organisation.update!(
default_news_image: create(:default_news_organisation_image_data),
default_news_image: create(:featured_image_data),
)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,23 @@ class NewsArticleWithImage < TestCase

assert_details_attribute :image, expected_image
end

test "should return lead image from worldwide_organisations when lead_image_has_all_assets?" do
image = build(:featured_image_data)
worldwide_organisation = build(:worldwide_organisation, default_news_image: image)
self.news_article = create(:news_article_world_news_story, worldwide_organisations: [worldwide_organisation])

assert presented_news_article.content[:details][:image][:url].include?("s300_minister-of-funk.960x640.jpg")
end

test "should not return lead image from worldwide_organisations when lead_image dont have assets?" do
image = build(:featured_image_data)
worldwide_organisation = build(:worldwide_organisation, default_news_image: image)
worldwide_organisation.default_news_image.assets.delete_all

self.news_article = create(:news_article_world_news_story, worldwide_organisations: [worldwide_organisation])
assert presented_news_article.content[:details][:image].nil?
end
end

class NewsArticleWithMinisterialRoleAppointments < TestCase
Expand Down
18 changes: 18 additions & 0 deletions test/unit/app/workers/asset_manager_create_asset_worker_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -246,4 +246,22 @@ class AssetManagerCreateAssetWorkerTest < ActiveSupport::TestCase

@worker.perform(@file.path, asset_params, true, nil, nil)
end

test "should republish all related news article for worldwide-organisations if featured image changes" do
worldwide_organisation = create(:worldwide_organisation, :with_default_news_image)
last_asset = worldwide_organisation.default_news_image.assets.last
asset_params = {
assetable_id: worldwide_organisation.default_news_image.id,
asset_variant: last_asset.variant,
assetable_type: FeaturedImageData.to_s,
}.deep_stringify_keys

asset_manager_response_with_new_id = { "id" => "http://asset-manager/assets/some_asset_manager_id", "name" => File.basename(@file) }
Services.asset_manager.stubs(:create_asset).returns(asset_manager_response_with_new_id)
news_article = create(:news_article_world_news_story, worldwide_organisations: [worldwide_organisation])

Whitehall::PublishingApi.expects(:republish_document_async).with(news_article.document)

@worker.perform(@file.path, asset_params, true, nil, nil)
end
end

0 comments on commit fb98fd0

Please sign in to comment.