From 8eea6f585a31ccd436785275af5ffb9919ea71c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sat, 18 Sep 2021 17:37:54 +0200
Subject: [PATCH] Remove hack to allow IO files in Active Storage

We were using this hack in order to allow `File.new` attachments in
tests files. However, we can use the `fixture_file_upload` helper
instead.

Just like it happened with `file_fixture`, this helper method doesn't
work in fixtures, so in this case we're using `Rack::Test::UploadedFile`
instead.
---
 app/models/concerns/has_attachment.rb              |  4 +---
 db/dev_seeds/budgets.rb                            | 14 ++++++--------
 db/dev_seeds/proposals.rb                          | 14 ++++++--------
 db/dev_seeds/widgets.rb                            |  2 +-
 spec/factories/administration.rb                   |  2 +-
 spec/factories/files.rb                            |  8 ++++----
 spec/lib/tasks/active_storage_spec.rb              |  2 +-
 spec/models/direct_upload_spec.rb                  |  8 ++++----
 spec/models/document_spec.rb                       |  2 +-
 spec/models/image_spec.rb                          |  2 +-
 spec/models/local_census_records/import_spec.rb    |  8 ++++----
 spec/models/site_customization/image_spec.rb       |  8 ++++----
 spec/shared/models/acts_as_imageable.rb            |  8 ++++----
 spec/shared/models/document_validations.rb         |  2 +-
 spec/shared/models/image_validations.rb            |  4 ++--
 spec/spec_helper.rb                                |  1 +
 .../system/admin/site_customization/images_spec.rb |  2 +-
 17 files changed, 43 insertions(+), 48 deletions(-)

diff --git a/app/models/concerns/has_attachment.rb b/app/models/concerns/has_attachment.rb
index accca53c2..2ffbd931b 100644
--- a/app/models/concerns/has_attachment.rb
+++ b/app/models/concerns/has_attachment.rb
@@ -6,9 +6,7 @@ module HasAttachment
       has_one_attached attribute
 
       define_method :"#{attribute}=" do |file|
-        if file.is_a?(IO)
-          send(attribute).attach(io: file, filename: File.basename(file.path))
-        elsif file.nil?
+        if file.nil?
           send(attribute).detach
         else
           send(attribute).attach(file)
diff --git a/db/dev_seeds/budgets.rb b/db/dev_seeds/budgets.rb
index c686a5c9c..e3dc861c2 100644
--- a/db/dev_seeds/budgets.rb
+++ b/db/dev_seeds/budgets.rb
@@ -15,14 +15,12 @@ end
 
 def add_image_to(imageable)
   # imageable should respond to #title & #author
-  File.open(INVESTMENT_IMAGE_FILES.sample) do |file|
-    imageable.image = Image.create!({
-      imageable: imageable,
-      title: imageable.title,
-      attachment: file,
-      user: imageable.author
-    })
-  end
+  imageable.image = Image.create!({
+    imageable: imageable,
+    title: imageable.title,
+    attachment: Rack::Test::UploadedFile.new(INVESTMENT_IMAGE_FILES.sample),
+    user: imageable.author
+  })
   imageable.save!
 end
 
diff --git a/db/dev_seeds/proposals.rb b/db/dev_seeds/proposals.rb
index 756dd00b3..a2ccbfb08 100644
--- a/db/dev_seeds/proposals.rb
+++ b/db/dev_seeds/proposals.rb
@@ -12,14 +12,12 @@ end
 
 def add_image_to(imageable)
   # imageable should respond to #title & #author
-  File.open(IMAGE_FILES.sample) do |file|
-    imageable.image = Image.create!({
-      imageable: imageable,
-      title: imageable.title,
-      attachment: file,
-      user: imageable.author
-    })
-  end
+  imageable.image = Image.create!({
+    imageable: imageable,
+    title: imageable.title,
+    attachment: Rack::Test::UploadedFile.new(IMAGE_FILES.sample),
+    user: imageable.author
+  })
   imageable.save!
 end
 
diff --git a/db/dev_seeds/widgets.rb b/db/dev_seeds/widgets.rb
index 41711c937..d82d655fe 100644
--- a/db/dev_seeds/widgets.rb
+++ b/db/dev_seeds/widgets.rb
@@ -1,7 +1,7 @@
 section "Creating header and cards for the homepage" do
   def create_image_attachment(type)
     {
-      attachment: File.new(Rails.root.join("db/dev_seeds/images/#{type}_background.jpg")),
+      attachment: Rack::Test::UploadedFile.new("db/dev_seeds/images/#{type}_background.jpg"),
       title: "#{type}_background.jpg",
       user: User.first
     }
diff --git a/spec/factories/administration.rb b/spec/factories/administration.rb
index a59f30776..07a110ca5 100644
--- a/spec/factories/administration.rb
+++ b/spec/factories/administration.rb
@@ -58,7 +58,7 @@ FactoryBot.define do
   end
 
   factory :site_customization_image, class: "SiteCustomization::Image" do
-    image { File.new("spec/fixtures/files/logo_header.png") }
+    image { Rack::Test::UploadedFile.new("spec/fixtures/files/logo_header.png") }
     name { "logo_header" }
   end
 
diff --git a/spec/factories/files.rb b/spec/factories/files.rb
index 0926ee576..5876c90c8 100644
--- a/spec/factories/files.rb
+++ b/spec/factories/files.rb
@@ -1,6 +1,6 @@
 FactoryBot.define do
   factory :image do
-    attachment { File.new("spec/fixtures/files/clippy.jpg") }
+    attachment { Rack::Test::UploadedFile.new("spec/fixtures/files/clippy.jpg") }
     title { "Lorem ipsum dolor sit amet" }
     association :user, factory: :user
 
@@ -16,7 +16,7 @@ FactoryBot.define do
   factory :document do
     sequence(:title) { |n| "Document title #{n}" }
     association :user, factory: :user
-    attachment { File.new("spec/fixtures/files/empty.pdf") }
+    attachment { Rack::Test::UploadedFile.new("spec/fixtures/files/empty.pdf") }
 
     trait :proposal_document do
       association :documentable, factory: :proposal
@@ -47,11 +47,11 @@ FactoryBot.define do
 
     trait :documents do
       resource_relation { "documents" }
-      attachment { File.new("spec/fixtures/files/empty.pdf") }
+      attachment { Rack::Test::UploadedFile.new("spec/fixtures/files/empty.pdf") }
     end
     trait :image do
       resource_relation { "image" }
-      attachment { File.new("spec/fixtures/files/clippy.jpg") }
+      attachment { Rack::Test::UploadedFile.new("spec/fixtures/files/clippy.jpg") }
     end
     initialize_with { new(attributes) }
   end
diff --git a/spec/lib/tasks/active_storage_spec.rb b/spec/lib/tasks/active_storage_spec.rb
index 0f43b39cd..bf394d84d 100644
--- a/spec/lib/tasks/active_storage_spec.rb
+++ b/spec/lib/tasks/active_storage_spec.rb
@@ -35,7 +35,7 @@ describe "active storage tasks" do
     end
 
     it "does not modify old ckeditor attachments" do
-      image = Ckeditor::Picture.create!(data: Rack::Test::UploadedFile.new(file_fixture("clippy.png")))
+      image = Ckeditor::Picture.create!(data: fixture_file_upload("clippy.png"))
 
       expect(image.storage_data.attachment.name).to eq "storage_data"
 
diff --git a/spec/models/direct_upload_spec.rb b/spec/models/direct_upload_spec.rb
index bdf079680..ec342f0de 100644
--- a/spec/models/direct_upload_spec.rb
+++ b/spec/models/direct_upload_spec.rb
@@ -10,10 +10,10 @@ describe DirectUpload do
     end
 
     it "is not valid for different kind of combinations with invalid atttachment content types" do
-      expect(build(:direct_upload, :proposal, :documents, attachment: File.new(file_fixture("clippy.png")))).not_to be_valid
-      expect(build(:direct_upload, :proposal, :image, attachment: File.new(file_fixture("empty.pdf")))).not_to be_valid
-      expect(build(:direct_upload, :budget_investment, :documents, attachment: File.new(file_fixture("clippy.png")))).not_to be_valid
-      expect(build(:direct_upload, :budget_investment, :image, attachment: File.new(file_fixture("empty.pdf")))).not_to be_valid
+      expect(build(:direct_upload, :proposal, :documents, attachment: fixture_file_upload("clippy.png"))).not_to be_valid
+      expect(build(:direct_upload, :proposal, :image, attachment: fixture_file_upload("empty.pdf"))).not_to be_valid
+      expect(build(:direct_upload, :budget_investment, :documents, attachment: fixture_file_upload("clippy.png"))).not_to be_valid
+      expect(build(:direct_upload, :budget_investment, :image, attachment: fixture_file_upload("empty.pdf"))).not_to be_valid
     end
 
     it "is not valid without resource_type" do
diff --git a/spec/models/document_spec.rb b/spec/models/document_spec.rb
index 1d7a087a4..0180c4010 100644
--- a/spec/models/document_spec.rb
+++ b/spec/models/document_spec.rb
@@ -5,7 +5,7 @@ describe Document do
   it_behaves_like "document validations", "proposal_document"
 
   it "stores attachments with Active Storage" do
-    document = create(:document, attachment: File.new(file_fixture("clippy.pdf")))
+    document = create(:document, attachment: fixture_file_upload("clippy.pdf"))
 
     expect(document.attachment).to be_attached
     expect(document.attachment.filename).to eq "clippy.pdf"
diff --git a/spec/models/image_spec.rb b/spec/models/image_spec.rb
index 7022e0b40..6b4603923 100644
--- a/spec/models/image_spec.rb
+++ b/spec/models/image_spec.rb
@@ -5,7 +5,7 @@ describe Image do
   it_behaves_like "image validations", "proposal_image"
 
   it "stores attachments with Active Storage" do
-    image = create(:image, attachment: File.new(file_fixture("clippy.jpg")))
+    image = create(:image, attachment: fixture_file_upload("clippy.jpg"))
 
     expect(image.attachment).to be_attached
     expect(image.attachment.filename).to eq "clippy.jpg"
diff --git a/spec/models/local_census_records/import_spec.rb b/spec/models/local_census_records/import_spec.rb
index 70ec5038b..7a97f0df5 100644
--- a/spec/models/local_census_records/import_spec.rb
+++ b/spec/models/local_census_records/import_spec.rb
@@ -17,7 +17,7 @@ describe LocalCensusRecords::Import do
 
     context "When file extension" do
       it "is wrong it should not be valid" do
-        file = Rack::Test::UploadedFile.new(file_fixture("clippy.gif"))
+        file = fixture_file_upload("clippy.gif")
         import = build(:local_census_records_import, file: file)
 
         expect(import).not_to be_valid
@@ -25,7 +25,7 @@ describe LocalCensusRecords::Import do
 
       it "is csv it should be valid" do
         path = base_files_path << "valid.csv"
-        file = Rack::Test::UploadedFile.new(file_fixture(path))
+        file = fixture_file_upload(path)
         import = build(:local_census_records_import, file: file)
 
         expect(import).to be_valid
@@ -35,7 +35,7 @@ describe LocalCensusRecords::Import do
     context "When file headers" do
       it "are all missing it should not be valid" do
         path = base_files_path << "valid-without-headers.csv"
-        file = Rack::Test::UploadedFile.new(file_fixture(path))
+        file = fixture_file_upload(path)
         import = build(:local_census_records_import, file: file)
 
         expect(import).not_to be_valid
@@ -64,7 +64,7 @@ describe LocalCensusRecords::Import do
 
     it "Add invalid local census records to invalid_records array" do
       path = base_files_path << "invalid.csv"
-      file = Rack::Test::UploadedFile.new(file_fixture(path))
+      file = fixture_file_upload(path)
       import.file = file
 
       import.save!
diff --git a/spec/models/site_customization/image_spec.rb b/spec/models/site_customization/image_spec.rb
index eef988bd1..076fc01d5 100644
--- a/spec/models/site_customization/image_spec.rb
+++ b/spec/models/site_customization/image_spec.rb
@@ -3,7 +3,7 @@ require "rails_helper"
 describe SiteCustomization::Image do
   it "stores images with Active Storage" do
     image = create(:site_customization_image, name: "map",
-                   image: File.new(file_fixture("custom_map.jpg")))
+                   image: fixture_file_upload("custom_map.jpg"))
 
     expect(image.image).to be_attached
     expect(image.image.filename).to eq "custom_map.jpg"
@@ -13,7 +13,7 @@ describe SiteCustomization::Image do
     it "is valid with a 260x80 image" do
       image = build(:site_customization_image,
                     name: "logo_header",
-                    image: File.new(file_fixture("logo_header-260x80.png")))
+                    image: fixture_file_upload("logo_header-260x80.png"))
 
       expect(image).to be_valid
     end
@@ -21,7 +21,7 @@ describe SiteCustomization::Image do
     it "is valid with a 223x80 image" do
       image = build(:site_customization_image,
                     name: "logo_header",
-                    image: File.new(file_fixture("logo_header.png")))
+                    image: fixture_file_upload("logo_header.png"))
 
       expect(image).to be_valid
     end
@@ -29,7 +29,7 @@ describe SiteCustomization::Image do
     it "is not valid with a 400x80 image" do
       image = build(:site_customization_image,
                     name: "logo_header",
-                    image: File.new(file_fixture("logo_email_custom.png")))
+                    image: fixture_file_upload("logo_email_custom.png"))
 
       expect(image).not_to be_valid
     end
diff --git a/spec/shared/models/acts_as_imageable.rb b/spec/shared/models/acts_as_imageable.rb
index ed0d2df14..6e9810538 100644
--- a/spec/shared/models/acts_as_imageable.rb
+++ b/spec/shared/models/acts_as_imageable.rb
@@ -7,21 +7,21 @@ shared_examples "acts as imageable" do |imageable_factory|
 
   describe "file extension" do
     it "is not valid with '.png' extension" do
-      image.attachment = File.new(file_fixture("clippy.png"))
+      image.attachment = fixture_file_upload("clippy.png")
 
       expect(image).not_to be_valid
       expect(image.errors[:attachment].size).to eq(1)
     end
 
     it "is not valid with '.gif' extension" do
-      image.attachment = File.new(file_fixture("clippy.gif"))
+      image.attachment = fixture_file_upload("clippy.gif")
 
       expect(image).not_to be_valid
       expect(image.errors[:attachment].size).to eq(1)
     end
 
     it "is valid with '.jpg' extension" do
-      image.attachment = File.new(file_fixture("clippy.jpg"))
+      image.attachment = fixture_file_upload("clippy.jpg")
 
       expect(image).to be_valid
     end
@@ -33,7 +33,7 @@ shared_examples "acts as imageable" do |imageable_factory|
     end
 
     it "is not valid when image dimmensions are smaller than 475X475" do
-      image.attachment = File.new(file_fixture("logo_header.jpg"))
+      image.attachment = fixture_file_upload("logo_header.jpg")
 
       expect(image).not_to be_valid
     end
diff --git a/spec/shared/models/document_validations.rb b/spec/shared/models/document_validations.rb
index 01d13de23..11235f771 100644
--- a/spec/shared/models/document_validations.rb
+++ b/spec/shared/models/document_validations.rb
@@ -22,7 +22,7 @@ shared_examples "document validations" do |documentable_factory|
   it "is valid for all accepted content types" do
     acceptedcontenttypes.each do |content_type|
       extension = content_type.split("/").last
-      document.attachment = File.new(file_fixture("empty.#{extension}"))
+      document.attachment = fixture_file_upload("empty.#{extension}")
 
       expect(document).to be_valid
     end
diff --git a/spec/shared/models/image_validations.rb b/spec/shared/models/image_validations.rb
index 230fe4089..5d1245904 100644
--- a/spec/shared/models/image_validations.rb
+++ b/spec/shared/models/image_validations.rb
@@ -21,7 +21,7 @@ shared_examples "image validations" do |imageable_factory|
   it "is valid for all accepted content types" do
     acceptedcontenttypes.each do |content_type|
       extension = content_type.split("/").last
-      image.attachment = File.new(file_fixture("clippy.#{extension}"))
+      image.attachment = fixture_file_upload("clippy.#{extension}")
 
       expect(image).to be_valid
     end
@@ -30,7 +30,7 @@ shared_examples "image validations" do |imageable_factory|
   it "is not valid for png and gif image content types" do
     ["gif", "png"].each do |content_type|
       extension = content_type.split("/").last
-      image.attachment = File.new(file_fixture("clippy.#{extension}"))
+      image.attachment = fixture_file_upload("clippy.#{extension}")
 
       expect(image).not_to be_valid
     end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index b89e55699..b8b155795 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -9,6 +9,7 @@ Dir["./spec/shared/**/*.rb"].sort.each  { |f| require f }
 
 RSpec.configure do |config|
   config.use_transactional_fixtures = true
+  config.fixture_path = "spec/fixtures/files"
 
   config.filter_run_when_matching :focus
   config.include RequestSpecHelper, type: :request
diff --git a/spec/system/admin/site_customization/images_spec.rb b/spec/system/admin/site_customization/images_spec.rb
index c20b33e7e..85f3736ee 100644
--- a/spec/system/admin/site_customization/images_spec.rb
+++ b/spec/system/admin/site_customization/images_spec.rb
@@ -63,7 +63,7 @@ describe "Admin custom images", :admin do
   scenario "Custom apple touch icon is replaced on front views" do
     create(:site_customization_image,
            name: "apple-touch-icon-200",
-           image: File.new(file_fixture("apple-touch-icon-custom-200.png")))
+           image: fixture_file_upload("apple-touch-icon-custom-200.png"))
 
     visit root_path