Allow WWOrganisation to use FeaturedImageData

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]>

app/controllers/admin/worldwide_organisations_controller.rb

@@ -5,6 +5,7 @@ class Admin::WorldwideOrganisationsController < Admin::BaseController
 
   before_action :find_worldwide_organisation, except: %i[index new create]
   before_action :build_worldwide_organisation, only: %i[new create]
+
   layout "design_system"
 
   def index
@@ -69,13 +70,23 @@ def build_worldwide_organisation
   end
 
   def worldwide_organisation_params
-    params.require(:worldwide_organisation).permit(
+    @worldwide_organisation_params ||= params.require(:worldwide_organisation).permit(
       :name,
       :logo_formatted_name,
       world_location_ids: [],
       sponsoring_organisation_ids: [],
-      default_news_image_attributes: %i[file file_cache],
+      default_news_image_attributes: %i[file file_cache id],
     )
+
+    clear_file_cache(@worldwide_organisation_params)
+  end
+
+  def clear_file_cache(params)
+    if params.dig(:default_news_image_attributes, :file).present? && params.dig(:default_news_image_attributes, :file_cache).present?
+      params[:default_news_image_attributes].delete(:file_cache)
+    end
+
+    params
   end
 
   def main_office_params

app/models/worldwide_organisation.rb

@@ -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
 
@@ -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
@@ -151,4 +137,13 @@ def search_index
   def publishing_api_presenter
     PublishingApi::WorldwideOrganisationPresenter
   end
+
+  def republish_dependent_documents
+    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

app/views/admin/worldwide_organisations/_form.html.erb

@@ -62,20 +62,22 @@
     rows: 4,
   } %>
 
-  <%= render "components/single-image-upload", {
-    title: "Default news image",
-    name: "worldwide_organisation[default_news_image_attributes]",
-    id: "worldwide_organisation_default_news_image",
-    image_id: "worldwide_organisation_default_news_image_file",
-    image_name: "worldwide_organisation[default_news_image_attributes][file]",
-    remove_alt_text_field: true,
-    filename: worldwide_organisation.default_news_image.file.identifier,
-    page_errors: worldwide_organisation.errors.any?,
-    error_items: errors_for(worldwide_organisation.default_news_image.errors, :default_news_image),
-    image_src: worldwide_organisation.default_news_image.file.url,
-    image_cache_name: "worldwide_organisation[default_news_image_attributes][file_cache]",
-    image_cache: (worldwide_organisation.default_news_image.file_cache.presence),
-  } %>
+  <%= form.fields_for :default_news_image do |_image_fields| %>
+    <%= render "components/single-image-upload", {
+      title: "Default news image",
+      name: "worldwide_organisation[default_news_image_attributes]",
+      id: "worldwide_organisation_default_news_image",
+      image_id: "worldwide_organisation_default_news_image_file",
+      image_name: "worldwide_organisation[default_news_image_attributes][file]",
+      remove_alt_text_field: true,
+      filename: worldwide_organisation.default_news_image.file.identifier,
+      page_errors: worldwide_organisation.errors.any?,
+      error_items: errors_for(worldwide_organisation.errors, :"default_news_image.file"),
+      image_src: worldwide_organisation.default_news_image.file.url,
+      image_cache_name: "worldwide_organisation[default_news_image_attributes][file_cache]",
+      image_cache: (worldwide_organisation.default_news_image.file_cache.presence),
+    } %>
+  <% end %>
 
   <div class="govuk-button-group govuk-!-margin-top-8">
     <%= render "govuk_publishing_components/components/button", {

test/components/admin/worldwide_organisations/show/summary_list_component_test.rb

@@ -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",

test/factories/worldwide_organisations.rb

@@ -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

test/functional/admin/worldwide_organisations_controller_test.rb

@@ -102,31 +102,25 @@ class Admin::WorldwideOrganisationsControllerTest < ActionController::TestCase
   test "setting the main office" do
     offices = [create(:worldwide_office), create(:worldwide_office)]
     worldwide_organisation = create(:worldwide_organisation, offices:)
+
     put :set_main_office, params: { id: worldwide_organisation.id, worldwide_organisation: { main_office_id: offices.last.id } }
 
     assert_equal offices.last, worldwide_organisation.reload.main_office
     assert_equal "Main office updated successfully", flash[:notice]
     assert_redirected_to admin_worldwide_organisation_worldwide_offices_path(worldwide_organisation)
   end
 
-  test "destroys an existing object" do
-    organisation = create(:worldwide_organisation)
-
-    page = create(
-      :published_worldwide_organisation_corporate_information_page,
-      worldwide_organisation: organisation,
-    )
+  test "DELETE :destroys the WorldwideOrganisation and does not destroy dependent classes" do
+    worldwide_organisation = create(:worldwide_organisation, :with_default_news_image)
+    default_news_image_id = worldwide_organisation.default_news_image.id
+    count = WorldwideOrganisation.count
 
-    create(
-      :published_worldwide_organisation_corporate_information_page,
-      worldwide_organisation: organisation,
-      document: page.document,
-    )
+    delete :destroy, params: { id: worldwide_organisation.id }
 
-    count = WorldwideOrganisation.count
-    delete :destroy, params: { id: organisation.id }
     assert_equal "Organisation deleted successfully", flash[:notice]
     assert_equal count - 1, WorldwideOrganisation.count
+    assert_nil WorldwideOrganisation.find_by(id: worldwide_organisation.id)
+    assert FeaturedImageData.find_by(id: default_news_image_id)
   end
 
   test "GET :confirm_destroy calls correctly" do
@@ -145,4 +139,88 @@ class Admin::WorldwideOrganisationsControllerTest < ActionController::TestCase
     assert_response :success
     assert_equal organisation, assigns(:worldwide_organisation)
   end
+
+  test "PUT :update - discards default new organisation image cache if file is present " do
+    worldwide_organisation = create(:worldwide_organisation, :with_default_news_image)
+    replacement_filename = "example_fatality_notice_image.jpg"
+    cached_filename = "big-cheese.960x640.jpg"
+    cached_default_news_image = build(:featured_image_data, file: upload_fixture(cached_filename))
+
+    Services.asset_manager.stubs(:create_asset).returns("id" => "http://asset-manager/assets/asset_manager_id", "name" => replacement_filename)
+    AssetManagerCreateAssetWorker.expects(:perform_async).with(regexp_matches(/#{replacement_filename}/), anything, anything, anything, anything, anything).times(7)
+    AssetManagerCreateAssetWorker.expects(:perform_async).with(regexp_matches(/#{cached_filename}/), anything, anything, anything, anything, anything).never
+
+    put :update,
+        params: {
+          id: worldwide_organisation.id,
+          worldwide_organisation: {
+            name: "New name",
+            default_news_image_attributes: {
+              file: upload_fixture(replacement_filename),
+              file_cache: cached_default_news_image.file_cache,
+              id: worldwide_organisation.default_news_image.id,
+            },
+          },
+        }
+
+    assert_equal replacement_filename, WorldwideOrganisation.last.default_news_image.filename
+  end
+
+  test "POST :create uses the file cache if present" do
+    cached_filename = "big-cheese.960x640.jpg"
+    cached_default_news_image = build(:featured_image_data, file: upload_fixture(cached_filename))
+
+    post :create, params: {
+      worldwide_organisation: {
+        name: "name",
+        default_news_image_attributes: {
+          file_cache: cached_default_news_image.file_cache,
+        },
+      },
+    }
+
+    assert_equal cached_filename, WorldwideOrganisation.last.default_news_image.filename
+  end
+
+  test "POST :create discards the file cache if file is present" do
+    cached_filename = "big-cheese.960x640.jpg"
+    cached_default_news_image = build(:featured_image_data, file: upload_fixture(cached_filename))
+    replacement_filename = "example_fatality_notice_image.jpg"
+
+    AssetManagerCreateAssetWorker.expects(:perform_async).with(regexp_matches(/#{cached_filename}/), anything, anything, anything, anything, anything).never
+    AssetManagerCreateAssetWorker.expects(:perform_async).with(regexp_matches(/#{replacement_filename}/), anything, anything, anything, anything, anything).times(7)
+
+    post :create, params: {
+      worldwide_organisation: {
+        name: "name",
+        default_news_image_attributes: {
+          file: upload_fixture(replacement_filename),
+          file_cache: cached_default_news_image.file_cache,
+        },
+      },
+    }
+
+    assert_equal replacement_filename, WorldwideOrganisation.last.default_news_image.filename
+  end
+
+  test "PUT :update - updates existing default new image when image file is replaced" do
+    worldwide_organisation = create(:worldwide_organisation, :with_default_news_image)
+    default_news_image_id = worldwide_organisation.default_news_image.id
+
+    put :update,
+        params: {
+          id: worldwide_organisation.id,
+          worldwide_organisation: {
+            name: "New name",
+            default_news_image_attributes: {
+              id: default_news_image_id,
+              file: upload_fixture("minister-of-funk.960x640.jpg"),
+            },
+          },
+        }
+
+    worldwide_organisation = WorldwideOrganisation.last
+    assert_equal "minister-of-funk.960x640.jpg", worldwide_organisation.default_news_image.file.file.filename
+    assert_equal default_news_image_id, worldwide_organisation.default_news_image.id
+  end
 end

test/unit/app/models/worldwide_organisation_test.rb

@@ -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
@@ -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
 

test/unit/app/presenters/publishing_api/news_article_presenter_test.rb

@@ -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 = []
+
+      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

test/unit/app/workers/asset_manager_create_asset_worker_test.rb

@@ -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