fixup: part two of "remove attachment_data_with_asset" factory

This removes the temorarily renamed factory method, and also
tidied up some test file `let` declarations that were unused
(following the approach of "if it's only used once, define it
inline rather than as a `let`").

Note that I've deliberately used `csv_attachment` rather than
`file_attachment` as the resulting path for the latter is quite
different to the resulting path for the former. `csv_attachment`
generates a path that is most similar to the docx path that these
tests had relied on until now.

test/factories/attachment_data.rb

@@ -10,14 +10,6 @@
       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: attachment_data.filename)
-      end
-    end
-
     trait(:csv) do
       file { File.open(Rails.root.join("test/fixtures/sample.csv")) }
 
@@ -28,7 +20,6 @@
   end
 
   factory :attachment_data, parent: :generic_attachment_data, traits: [:pdf]
-  factory :attachment_data_for_doc, parent: :generic_attachment_data, traits: [:doc]
   factory :attachment_data_for_csv, parent: :generic_attachment_data, traits: [:csv]
   factory :attachment_data_with_no_assets, parent: :generic_attachment_data
 end

test/factories/attachments.rb

@@ -19,16 +19,6 @@
       end
     end
 
-    trait(:doc) do
-      transient do
-        file { File.open(Rails.root.join("test/fixtures/sample.docx")) }
-      end
-
-      after(:build) do |attachment, evaluator|
-        attachment.attachment_data ||= build(:attachment_data_for_doc, file: evaluator.file, attachable: attachment.attachable)
-      end
-    end
-
     trait(:csv) do
       transient do
         file { File.open(Rails.root.join("test/fixtures/sample.csv")) }
@@ -47,7 +37,6 @@
   end
 
   factory :file_attachment, parent: :attachment, traits: [:pdf]
-  factory :file_attachment_with_asset, parent: :attachment, traits: [:doc]
   factory :csv_attachment, parent: :attachment, traits: [:csv]
   factory :file_attachment_with_no_assets, parent: :attachment, traits: [:with_no_assets]
 

test/integration/asset_access_options_integration_test.rb

@@ -256,7 +256,7 @@ def setup_publishing_api_for(edition)
 
     def add_file_attachment_with_asset(filename, to:)
       to.attachments << FactoryBot.build(
-        :file_attachment_with_asset,
+        :csv_attachment,
         title: filename,
         attachable: to,
       )
@@ -266,10 +266,10 @@ def path_to_attachment(filename)
       fixture_path.join(filename)
     end
 
-    def stub_asset(asset_manger_id, attributes = {})
-      url_id = "http://asset-manager/assets/#{asset_manger_id}"
+    def stub_asset(asset_manager_id, attributes = {})
+      url_id = "http://asset-manager/assets/#{asset_manager_id}"
       Services.asset_manager.stubs(:asset)
-              .with(asset_manger_id)
+              .with(asset_manager_id)
               .returns(attributes.merge(id: url_id).stringify_keys)
     end
   end

test/integration/attachment_deletion_integration_test.rb

@@ -10,7 +10,7 @@ class AttachmentDeletionIntegrationTest < ActionDispatch::IntegrationTest
   describe "attachment deletion" do
     context "given a draft document with multiple file attachments" do
       let(:managing_editor) { create(:managing_editor) }
-      let(:first_attachment) { build(:file_attachment_with_asset, attachable: edition, title: "first attachment") }
+      let(:first_attachment) { build(:csv_attachment, attachable: edition, title: "first attachment") }
       let(:first_asset_id) { "asset_manager_id" }
       let(:second_attachment) { build(:file_attachment, attachable: edition) }
       let(:second_asset_id_original) { "asset_manager_id_original" }

test/integration/attachment_draft_status_integration_test.rb

@@ -11,7 +11,6 @@ class AttachmentDraftStatusIntegrationTest < ActionDispatch::IntegrationTest
     let(:filename) { "sample.docx" }
     let(:asset_manager_id) { "asset_manager_id" }
     let(:topic_taxon) { build(:taxon_hash) }
-    let(:variant) { Asset.variants[:original] }
 
     before do
       login_as create(:managing_editor)
@@ -20,12 +19,11 @@ class AttachmentDraftStatusIntegrationTest < ActionDispatch::IntegrationTest
     end
 
     context "given a file attachment" do
-      let(:attachment) { build(:file_attachment_with_asset, attachable:) }
       let(:attachable) { edition }
 
       before do
         setup_publishing_api_for(edition)
-        attachable.attachments << attachment
+        attachable.attachments << build(:csv_attachment, attachable:)
         attachable.save!
       end
 

test/integration/attachment_link_header_test.rb

@@ -8,9 +8,7 @@ class AttachmentLinkHeaderIntegrationTest < ActionDispatch::IntegrationTest
   include TaxonomyHelper
 
   describe "attachment link header" do
-    let(:filename) { "sample.docx" }
     let(:asset_manager_id) { "asset_manager_id" }
-    let(:variant) { Asset.variants[:original] }
 
     before do
       login_as create(:managing_editor)
@@ -20,12 +18,11 @@ class AttachmentLinkHeaderIntegrationTest < ActionDispatch::IntegrationTest
     end
 
     context "given a file attachment" do
-      let(:attachment) { build(:file_attachment_with_asset, attachable:) }
       let(:attachable) { edition }
       let(:topic_taxon) { build(:taxon_hash) }
 
       before do
-        attachable.attachments << attachment
+        attachable.attachments << build(:csv_attachment, attachable:)
         attachable.save!
       end
 
@@ -55,10 +52,6 @@ def setup_publishing_api_for(edition)
       stub_publishing_api_links_with_taxons(edition.content_id, [topic_taxon["content_id"]])
     end
 
-    def path_to_attachment(filename)
-      fixture_path.join(filename)
-    end
-
     def stub_asset(asset_manger_id, attributes = {})
       url_id = "http://asset-manager/assets/#{asset_manger_id}"
       Services.asset_manager.stubs(:asset)

test/integration/attachment_redirect_due_to_unpublishing_integration_test.rb

@@ -8,14 +8,10 @@ class AttachmentRedirectDueToUnpublishingIntegrationTest < ActionDispatch::Integ
   include TaxonomyHelper
 
   describe "attachment redirect due to unpublishing" do
-    let(:filename) { "sample.docx" }
     let(:attachable) { edition }
     let(:asset_manager_id) { "asset_manager_id" }
     let(:redirect_path) { edition.public_path }
     let(:redirect_url) { edition.public_url }
-    let(:topic_taxon) { build(:taxon_hash) }
-    let(:variant) { Asset.variants[:original] }
-    let(:attachment) { build(:file_attachment_with_asset, attachable:) }
 
     before do
       login_as create(:managing_editor)
@@ -25,7 +21,7 @@ class AttachmentRedirectDueToUnpublishingIntegrationTest < ActionDispatch::Integ
       stub_publishing_api_expanded_links_with_taxons(edition.content_id, [])
       stub_asset(asset_manager_id)
 
-      attachable.attachments << attachment
+      attachable.attachments << build(:csv_attachment, attachable:)
       attachable.save!
     end
 

test/integration/attachment_replacement_integration_test.rb

@@ -14,7 +14,7 @@ class AttachmentReplacementIntegrationTest < ActionDispatch::IntegrationTest
     let(:replacement_filename) { "sample.rtf" }
     let(:replacement_asset_manager_id) { "replacement-asset-id" }
     let(:variant) { Asset.variants[:original] }
-    let(:attachment) { build(:file_attachment_with_asset, title: "attachment-title", attachable: edition) }
+    let(:attachment) { build(:csv_attachment, title: "attachment-title", attachable: edition) }
 
     before do
       login_as(managing_editor)

test/unit/lib/tasks/asset_manager_test.rb

@@ -15,7 +15,7 @@ class AssetManagerTest < ActiveSupport::TestCase
   %w[unpublished superseded].each do |state|
     context "for an asset attached only to a #{state} edition" do
       let(:edition) { create(:"#{state}_publication") }
-      let(:attachment) { create(:file_attachment_with_asset, attachable: edition) }
+      let(:attachment) { create(:csv_attachment, attachable: edition) }
 
       context "when asset is not deleted from asset manager" do
         before do
@@ -57,7 +57,7 @@ class AssetManagerTest < ActiveSupport::TestCase
 
   context "for an asset attached only to a draft edition" do
     let(:edition) { create(:draft_publication) }
-    let(:attachment) { create(:file_attachment_with_asset, attachable: edition) }
+    let(:attachment) { create(:csv_attachment, attachable: edition) }
 
     context "when asset is not deleted from asset manager" do
       before do
@@ -86,7 +86,7 @@ class AssetManagerTest < ActiveSupport::TestCase
 
   context "for an asset attached only to a published edition" do
     let(:edition) { create(:published_publication) }
-    let(:attachment) { create(:file_attachment_with_asset, attachable: edition) }
+    let(:attachment) { create(:file_attachment, attachable: edition) }
 
     context "when asset is not deleted from asset manager" do
       before do
@@ -114,7 +114,7 @@ class AssetManagerTest < ActiveSupport::TestCase
   end
 
   context "for an asset attached to both published and superseded editions" do
-    let(:attachment) { create(:file_attachment_with_asset) }
+    let(:attachment) { create(:csv_attachment) }
     let(:edition_1) { create(:superseded_publication, :with_alternative_format_provider, attachments: [attachment]) }
     let(:edition_2) { create(:published_publication, :with_alternative_format_provider, attachments: [attachment]) }
 
@@ -144,7 +144,7 @@ class AssetManagerTest < ActiveSupport::TestCase
   end
 
   context "for an asset attached only to something that is not an edition" do
-    let(:attachment) { create(:file_attachment_with_asset, attachable: create(:double)) }
+    let(:attachment) { create(:csv_attachment, attachable: create(:double)) }
 
     test "it should include no output in the report" do
       assert_output("") { task.invoke }