Merge pull request #8590 from alphagov/refactor-factories-to-use-file…

…name-from-file

Refactor factories to use filename method instead of hardcoded file name

app/models/image_data.rb

@@ -22,7 +22,7 @@ class ImageData < ApplicationRecord
   delegate :content_type, to: :file
 
   def filename
-    file.file.filename
+    file&.file&.filename
   end
 
   def auth_bypass_ids

test/factories/attachment_data.rb

@@ -6,24 +6,24 @@
       content_type { AttachmentUploader::PDF_CONTENT_TYPE }
 
       after(:build) do |attachment_data|
-        attachment_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: "greenpaper.pdf")
-        attachment_data.assets << build(:asset, asset_manager_id: "asset_manager_id_thumbnail", variant: Asset.variants[:thumbnail], filename: "thumbnail_greenpaper.pdf.png")
+        attachment_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: attachment_data.filename)
+        attachment_data.assets << build(:asset, asset_manager_id: "asset_manager_id_thumbnail", variant: Asset.variants[:thumbnail], filename: "thumbnail_#{attachment_data.filename}.png")
       end
     end
 
     trait(:doc) do
       file { File.open(Rails.root.join("test/fixtures/sample.docx")) }
 
       after(:build) do |attachment_data|
-        attachment_data.assets << build(:asset, asset_manager_id: "asset_manager_id", variant: Asset.variants[:original], filename: "sample.docx")
+        attachment_data.assets << build(:asset, asset_manager_id: "asset_manager_id", variant: Asset.variants[:original], filename: attachment_data.filename)
       end
     end
 
     trait(:csv) do
       file { File.open(Rails.root.join("test/fixtures/sample.csv")) }
 
       after(:build) do |attachment_data|
-        attachment_data.assets << build(:asset, asset_manager_id: "asset_manager_id", variant: Asset.variants[:original], filename: "sample.csv")
+        attachment_data.assets << build(:asset, asset_manager_id: "asset_manager_id", variant: Asset.variants[:original], filename: attachment_data.filename)
       end
     end
   end

test/factories/call_for_evidence_response_form_data.rb

@@ -3,7 +3,7 @@
     file { File.open(Rails.root.join("test/fixtures/two-pages.pdf")) }
 
     after(:build) do |call_for_evidence_response_form_data|
-      call_for_evidence_response_form_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: "two-pages.pdf")
+      call_for_evidence_response_form_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: call_for_evidence_response_form_data.filename)
     end
   end
 end

test/factories/consultation_response_form_data.rb

@@ -3,7 +3,7 @@
     file { File.open(Rails.root.join("test/fixtures/two-pages.pdf")) }
 
     after(:build) do |consultation_response_form_data|
-      consultation_response_form_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: "two-pages.pdf")
+      consultation_response_form_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: consultation_response_form_data.filename)
     end
   end
 end

test/factories/image_data.rb

@@ -4,21 +4,21 @@
 
     trait(:jpg) do
       after(:build) do |image_data|
-        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: "minister-of-funk.960x640.jpg")
-        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s960", variant: Asset.variants[:s960], filename: "s960_minister-of-funk.960x640.jpg")
-        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s712", variant: Asset.variants[:s712], filename: "s712_minister-of-funk.960x640.jpg")
-        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s630", variant: Asset.variants[:s630], filename: "s630_minister-of-funk.960x640.jpg")
-        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s465", variant: Asset.variants[:s465], filename: "s465_minister-of-funk.960x640.jpg")
-        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s300", variant: Asset.variants[:s300], filename: "s300_minister-of-funk.960x640.jpg")
-        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s216", variant: Asset.variants[:s216], filename: "s216_minister-of-funk.960x640.jpg")
+        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: image_data.filename)
+        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s960", variant: Asset.variants[:s960], filename: "s960_#{image_data.filename}")
+        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s712", variant: Asset.variants[:s712], filename: "s712_#{image_data.filename}")
+        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s630", variant: Asset.variants[:s630], filename: "s630_#{image_data.filename}")
+        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s465", variant: Asset.variants[:s465], filename: "s465_#{image_data.filename}")
+        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s300", variant: Asset.variants[:s300], filename: "s300_#{image_data.filename}")
+        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s216", variant: Asset.variants[:s216], filename: "s216_#{image_data.filename}")
       end
     end
 
     trait(:svg) do
       file { File.open(Rails.root.join("test/fixtures/images/test-svg.svg")) }
 
       after(:build) do |image_data|
-        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id", variant: Asset.variants[:original], filename: "test-svg.svg")
+        image_data.assets << build(:asset, asset_manager_id: "asset_manager_id", variant: Asset.variants[:original], filename: image_data.filename)
       end
     end
   end

test/factories/organisations.rb

@@ -16,7 +16,7 @@
       logo { image_fixture_file }
 
       after :build do |organisation|
-        organisation.assets.build(asset_manager_id: "logo_asset_manager_id", variant: Asset.variants[:original], filename: "960x640_jpeg.jpg")
+        organisation.assets.build(asset_manager_id: "logo_asset_manager_id", variant: Asset.variants[:original], filename: organisation.logo.file.filename)
       end
     end
 

test/factories/promotional_feature_items.rb

@@ -8,13 +8,16 @@
       image_alt_text { "Image alt text" }
 
       after :build do |item|
-        item.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: "minister-of-funk.960x640.jpg")
-        item.assets << build(:asset, asset_manager_id: "asset_manager_id_s960", variant: Asset.variants[:s960], filename: "s960_minister-of-funk.960x640.jpg")
-        item.assets << build(:asset, asset_manager_id: "asset_manager_id_s712", variant: Asset.variants[:s712], filename: "s712_minister-of-funk.960x640.jpg")
-        item.assets << build(:asset, asset_manager_id: "asset_manager_id_s630", variant: Asset.variants[:s630], filename: "s630_minister-of-funk.960x640.jpg")
-        item.assets << build(:asset, asset_manager_id: "asset_manager_id_s465", variant: Asset.variants[:s465], filename: "s465_minister-of-funk.960x640.jpg")
-        item.assets << build(:asset, asset_manager_id: "asset_manager_id_s300", variant: Asset.variants[:s300], filename: "s300_minister-of-funk.960x640.jpg")
-        item.assets << build(:asset, asset_manager_id: "asset_manager_id_s216", variant: Asset.variants[:s216], filename: "s216_minister-of-funk.960x640.jpg")
+        next if item.image.blank?
+
+        filename = item.image.file.filename
+        item.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename:)
+        item.assets << build(:asset, asset_manager_id: "asset_manager_id_s960", variant: Asset.variants[:s960], filename: "s960_#{filename}")
+        item.assets << build(:asset, asset_manager_id: "asset_manager_id_s712", variant: Asset.variants[:s712], filename: "s712_#{filename}")
+        item.assets << build(:asset, asset_manager_id: "asset_manager_id_s630", variant: Asset.variants[:s630], filename: "s630_#{filename}")
+        item.assets << build(:asset, asset_manager_id: "asset_manager_id_s465", variant: Asset.variants[:s465], filename: "s465_#{filename}")
+        item.assets << build(:asset, asset_manager_id: "asset_manager_id_s300", variant: Asset.variants[:s300], filename: "s300_#{filename}")
+        item.assets << build(:asset, asset_manager_id: "asset_manager_id_s216", variant: Asset.variants[:s216], filename: "s216_#{filename}")
       end
     end
 

test/factories/topical_event_featuring_image_data.rb

@@ -3,13 +3,13 @@
     file { image_fixture_file }
 
     after(:build) do |topical_event_featuring_image_data|
-      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: "minister-of-funk.960x640.jpg")
-      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s960", variant: Asset.variants[:s960], filename: "s960_minister-of-funk.960x640.jpg")
-      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s712", variant: Asset.variants[:s712], filename: "s712_minister-of-funk.960x640.jpg")
-      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s630", variant: Asset.variants[:s630], filename: "s630_minister-of-funk.960x640.jpg")
-      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s465", variant: Asset.variants[:s465], filename: "s465_minister-of-funk.960x640.jpg")
-      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s300", variant: Asset.variants[:s300], filename: "s300_minister-of-funk.960x640.jpg")
-      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s216", variant: Asset.variants[:s216], filename: "s216_minister-of-funk.960x640.jpg")
+      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: topical_event_featuring_image_data.filename)
+      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s960", variant: Asset.variants[:s960], filename: "s960_#{topical_event_featuring_image_data.filename}")
+      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s712", variant: Asset.variants[:s712], filename: "s712_#{topical_event_featuring_image_data.filename}")
+      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s630", variant: Asset.variants[:s630], filename: "s630_#{topical_event_featuring_image_data.filename}")
+      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s465", variant: Asset.variants[:s465], filename: "s465_#{topical_event_featuring_image_data.filename}")
+      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s300", variant: Asset.variants[:s300], filename: "s300_#{topical_event_featuring_image_data.filename}")
+      topical_event_featuring_image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_s216", variant: Asset.variants[:s216], filename: "s216_#{topical_event_featuring_image_data.filename}")
     end
   end
 end

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

@@ -134,7 +134,7 @@ def govspeak_to_html(govspeak)
     )
     presented_item = present(organisation)
 
-    expected_image_url = "#{Plek.asset_root}/media/logo_asset_manager_id/960x640_jpeg.jpg"
+    expected_image_url = "#{Plek.asset_root}/media/logo_asset_manager_id/minister-of-funk.960x640.jpg"
 
     assert_equal(
       {