| <%= document.title %> |
- <%= document.storage_attachment.content_type %> |
- <%= number_to_human_size(document.storage_attachment.byte_size) %> |
- <%= link_to document.title, document.storage_attachment, target: :blank %> |
+ <%= document.attachment.content_type %> |
+ <%= number_to_human_size(document.attachment.byte_size) %> |
+ <%= link_to document.title, document.attachment, target: :blank %> |
<%= render Admin::TableActionsComponent.new(
diff --git a/app/views/admin/site_customization/images/index.html.erb b/app/views/admin/site_customization/images/index.html.erb
index 43da53e9e..354c3626c 100644
--- a/app/views/admin/site_customization/images/index.html.erb
+++ b/app/views/admin/site_customization/images/index.html.erb
@@ -16,7 +16,7 @@
<%= form_for([:admin, image], html: { id: "edit_#{dom_id(image)}" }) do |f| %>
- <%= image_tag image.storage_image if image.persisted_attachment? %>
+ <%= image_tag image.image if image.persisted_attachment? %>
<%= f.file_field :image, label: false %>
diff --git a/app/views/dashboard/_document.html.erb b/app/views/dashboard/_document.html.erb
index e91efb98b..d59177a30 100644
--- a/app/views/dashboard/_document.html.erb
+++ b/app/views/dashboard/_document.html.erb
@@ -1,5 +1,5 @@
- <%= link_to document.storage_attachment, target: "_blank" do %>
+ <%= link_to document.attachment, target: "_blank" do %>
<%= document.title %>
(<%= document.humanized_content_type %>
|
diff --git a/app/views/documents/_document.html.erb b/app/views/documents/_document.html.erb
index 67c61216b..977e3acc4 100644
--- a/app/views/documents/_document.html.erb
+++ b/app/views/documents/_document.html.erb
@@ -1,6 +1,6 @@
<%= link_to t("documents.buttons.download_document"),
- document.storage_attachment, target: "_blank",
+ document.attachment, target: "_blank",
rel: "nofollow", class: "button hollow medium float-right" %>
<%= document.title %>
diff --git a/app/views/milestones/_milestone.html.erb b/app/views/milestones/_milestone.html.erb
index ce8ea12bc..52f8e6956 100644
--- a/app/views/milestones/_milestone.html.erb
+++ b/app/views/milestones/_milestone.html.erb
@@ -37,7 +37,7 @@
<% milestone.documents.each do |document| %>
<%= link_to document.title,
- document.storage_attachment,
+ document.attachment,
target: "_blank",
rel: "nofollow" %>
diff --git a/app/views/polls/_gallery.html.erb b/app/views/polls/_gallery.html.erb
index e62827908..43ac86cf6 100644
--- a/app/views/polls/_gallery.html.erb
+++ b/app/views/polls/_gallery.html.erb
@@ -18,8 +18,8 @@
<% answer.images.reverse.each_with_index do |image, index| %>
- <%= link_to image.storage_attachment, target: "_blank" do %>
- <%= image_tag image.storage_attachment,
+ <%= link_to image.attachment, target: "_blank" do %>
+ <%= image_tag image.attachment,
class: "orbit-image",
alt: image.title.unicode_normalize %>
<% end %>
diff --git a/app/views/polls/show.html.erb b/app/views/polls/show.html.erb
index 69272441a..1aa0e89ec 100644
--- a/app/views/polls/show.html.erb
+++ b/app/views/polls/show.html.erb
@@ -86,7 +86,7 @@
<% answer.documents.each do |document| %>
<%= link_to document.title,
- document.storage_attachment,
+ document.attachment,
target: "_blank",
rel: "nofollow" %>
<% end %>
diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb
deleted file mode 100644
index 97a4079d9..000000000
--- a/config/initializers/paperclip.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-class Paperclip::UrlGenerator
- private
-
- # Code copied from the paperclip gem, only replacing
- # `URI.escape` with `URI::DEFAULT_PARSER.escape`
- def escape_url(url)
- if url.respond_to?(:escape)
- url.escape
- else
- URI::DEFAULT_PARSER.escape(url).gsub(escape_regex) { |m| "%#{m.ord.to_s(16).upcase}" }
- end
- end
-end
diff --git a/lib/tasks/active_storage.rake b/lib/tasks/active_storage.rake
index 548d5cfc9..2f65e829b 100644
--- a/lib/tasks/active_storage.rake
+++ b/lib/tasks/active_storage.rake
@@ -1,98 +1,9 @@
-# This code is based on Thoughtbot's guide to migrating from Paperclip
-# to Active Storage:
-# https://github.com/thoughtbot/paperclip/blob/master/MIGRATING.md
namespace :active_storage do
- desc "Copy paperclip's attachment database columns to active storage"
- task migrate_from_paperclip: :environment do
- logger = ApplicationLogger.new
- connection = ActiveRecord::Base.connection.raw_connection
- statement_name = "active_storage_statement"
-
- unless connection.exec("SELECT COUNT(*) FROM pg_prepared_statements WHERE name='#{statement_name}'").each_row.to_a[0][0] > 0
- connection.prepare(statement_name, <<-SQL)
- with rows as(
- INSERT INTO active_storage_blobs (
- key, filename, content_type, metadata, byte_size, checksum, created_at
- ) VALUES ($1, $2, $3, '{}', $4, $5, $6) RETURNING id
- )
- INSERT INTO active_storage_attachments (
- name, record_type, record_id, blob_id, created_at
- ) VALUES ($7, $8, $9, (SELECT id FROM rows), $10)
- SQL
- end
-
- Rails.application.eager_load!
- models = ActiveRecord::Base.descendants.reject(&:abstract_class?)
- paperclip_storage = Paperclip::Attachment.default_options[:storage]
-
- ActiveRecord::Base.transaction do
- models.each do |model|
- next if model.name == "OldPassword" && !model.table_exists?
-
- attachments = model.column_names.map do |c|
- if c =~ /(.+)_file_name$/
- $1
- end
- end.compact
-
- if attachments.blank?
- next
- end
-
- model.find_each.each do |instance|
- attachments.each do |attachment|
- next if instance.send(:"storage_#{attachment}").attached?
-
- source = if paperclip_storage == :filesystem
- instance.send(attachment).path
- else
- instance.send(attachment).url
- end
-
- next if source.blank? || source == "/images/original/missing.png"
-
- file = if paperclip_storage == :filesystem
- File.read(source) if File.exist?(source)
- else
- Net::HTTP.get(URI(source))
- end
-
- connection.exec_prepared(
- statement_name, [
- SecureRandom.uuid, # Alternatively instance.send("#{attachment}_file_name"),
- instance.send("#{attachment}_file_name"),
- instance.send("#{attachment}_content_type"),
- instance.send("#{attachment}_file_size"),
- file && Digest::MD5.base64digest(file) || SecureRandom.hex(32),
- instance.updated_at.iso8601,
- "storage_#{attachment}",
- model.name,
- instance.id,
- instance.updated_at.iso8601
- ])
- end
- end
- end
- end
-
- ActiveStorage::Attachment.find_each do |attachment|
- blob = attachment.blob
-
- next if blob.service.exist?(blob.key) || !attachment.record
-
- name = attachment.name.delete_prefix("storage_")
- paperclip_attachment = attachment.record.send(name)
-
- next unless paperclip_attachment.exists?
-
- source_file = if paperclip_storage == :filesystem
- paperclip_attachment.path
- else
- URI.parse(paperclip_attachment.url).open.read
- end
-
- logger.info "Copying #{paperclip_attachment.url} to active storage"
- blob.service.upload(blob.key, source_file)
- end
+ desc "Updates ActiveStorage::Attachment old records created when we were also using paperclip"
+ task remove_paperclip_compatibility_in_existing_attachments: :environment do
+ ApplicationLogger.new.info "Updating Active Storage attachment records"
+ ActiveStorage::Attachment.where("name ILIKE 'storage_%'")
+ .where.not(record_type: "Ckeditor::Asset")
+ .update_all("name = replace(name, 'storage_', '')")
end
end
diff --git a/lib/tasks/consul.rake b/lib/tasks/consul.rake
index a45012ab5..6bd19134b 100644
--- a/lib/tasks/consul.rake
+++ b/lib/tasks/consul.rake
@@ -2,10 +2,10 @@ namespace :consul do
desc "Runs tasks needed to upgrade to the latest version"
task execute_release_tasks: ["settings:rename_setting_keys",
"settings:add_new_settings",
- "execute_release_1.4.0_tasks"]
+ "execute_release_1.5.0_tasks"]
- desc "Runs tasks needed to upgrade from 1.3.0 to 1.4.0"
- task "execute_release_1.4.0_tasks": [
- "active_storage:migrate_from_paperclip"
+ desc "Runs tasks needed to upgrade from 1.4.0 to 1.5.0"
+ task "execute_release_1.5.0_tasks": [
+ "active_storage:remove_paperclip_compatibility_in_existing_attachments"
]
end
diff --git a/spec/controllers/proposals_controller_spec.rb b/spec/controllers/proposals_controller_spec.rb
index ae586123b..05e1cc279 100644
--- a/spec/controllers/proposals_controller_spec.rb
+++ b/spec/controllers/proposals_controller_spec.rb
@@ -8,38 +8,4 @@ describe ProposalsController do
expect { get :index }.to raise_exception(FeatureFlags::FeatureDisabled)
end
end
-
- describe "PUT update" do
- let(:file) { Rails.root.join("spec/hacked") }
-
- before do
- File.delete(file) if File.exist?(file)
- InvisibleCaptcha.timestamp_enabled = false
- end
-
- after { InvisibleCaptcha.timestamp_enabled = true }
-
- it "ignores malicious cached attachments with remote storages" do
- allow_any_instance_of(Image).to receive(:filesystem_storage?).and_return(false)
- user = create(:user)
- proposal = create(:proposal, author: user)
- sign_in user
-
- begin
- put :update, params: {
- id: proposal,
- proposal: {
- image_attributes: {
- title: "Hacked!",
- user_id: user.id,
- cached_attachment: "| touch #{file}"
- }
- }
- }
- rescue StandardError
- ensure
- expect(file).not_to exist
- end
- end
- end
end
diff --git a/spec/lib/tasks/active_storage_spec.rb b/spec/lib/tasks/active_storage_spec.rb
index 2a8043556..9b2edc68d 100644
--- a/spec/lib/tasks/active_storage_spec.rb
+++ b/spec/lib/tasks/active_storage_spec.rb
@@ -1,90 +1,67 @@
require "rails_helper"
describe "active storage tasks" do
- describe "migrate_from_paperclip" do
+ describe "remove_paperclip_compatibility_in_existing_attachments" do
let(:run_rake_task) do
- Rake::Task["active_storage:migrate_from_paperclip"].reenable
- Rake.application.invoke_task("active_storage:migrate_from_paperclip")
+ Rake::Task["active_storage:remove_paperclip_compatibility_in_existing_attachments"].reenable
+ Rake.application.invoke_task("active_storage:remove_paperclip_compatibility_in_existing_attachments")
end
- let(:storage_root) { ActiveStorage::Blob.service.root }
- before { FileUtils.rm_rf storage_root }
+ it "updates old regular attachments" do
+ document = create(:document)
+ image = create(:image)
+ site_customization_image = create(:site_customization_image)
- it "migrates records and attachments" do
- document = build(:document,
- attachment: nil,
- paperclip_attachment: File.new("spec/fixtures/files/clippy.pdf"))
- document.save(validate: false)
+ document.attachment.attachment.update_column(:name, "storage_attachment")
+ image.attachment.attachment.update_column(:name, "storage_attachment")
+ site_customization_image.image.attachment.update_column(:name, "storage_image")
- expect(ActiveStorage::Attachment.count).to eq 0
- expect(ActiveStorage::Blob.count).to eq 0
- expect(test_storage_file_paths.count).to eq 0
+ document.reload
+ image.reload
+ site_customization_image.reload
+
+ expect(document.attachment.attachment).to be nil
+ expect(image.attachment.attachment).to be nil
+ expect(site_customization_image.image.attachment).to be nil
run_rake_task
document.reload
+ image.reload
+ site_customization_image.reload
- expect(ActiveStorage::Attachment.count).to eq 1
- expect(ActiveStorage::Blob.count).to eq 1
- expect(document.storage_attachment.filename).to eq "clippy.pdf"
- expect(test_storage_file_paths.count).to eq 1
- expect(storage_file_path(document)).to eq test_storage_file_paths.first
+ expect(document.attachment.attachment.name).to eq "attachment"
+ expect(image.attachment.attachment.name).to eq "attachment"
+ expect(site_customization_image.reload.image.attachment.name).to eq "image"
end
- it "migrates records with deleted files ignoring the files" do
- document = build(:document,
- attachment: nil,
- paperclip_attachment: File.new("spec/fixtures/files/clippy.pdf"))
- document.save(validate: false)
- FileUtils.rm(document.attachment.path)
+ it "does not modify old ckeditor attachments" do
+ image = Ckeditor::Picture.create!(data: Rack::Test::UploadedFile.new("spec/fixtures/files/clippy.png"))
+
+ expect(image.storage_data.attachment.name).to eq "storage_data"
+
+ run_rake_task
+ image.reload
+
+ expect(image.storage_data.attachment.name).to eq "storage_data"
+ end
+
+ it "does not modify new regular attachments" do
+ document = create(:document)
+ image = create(:image)
+ site_customization_image = create(:site_customization_image)
+
+ expect(document.attachment.attachment.name).to eq "attachment"
+ expect(image.attachment.attachment.name).to eq "attachment"
+ expect(site_customization_image.image.attachment.name).to eq "image"
run_rake_task
document.reload
+ image.reload
+ site_customization_image.reload
- expect(ActiveStorage::Attachment.count).to eq 1
- expect(ActiveStorage::Blob.count).to eq 1
- expect(document.storage_attachment.filename).to eq "clippy.pdf"
- expect(test_storage_file_paths.count).to eq 0
- end
-
- it "does not migrate already migrated records" do
- document = create(:document, attachment: File.new("spec/fixtures/files/clippy.pdf"))
-
- migrated_file = test_storage_file_paths.first
- attachment_id = document.storage_attachment.attachment.id
- blob_id = document.storage_attachment.blob.id
-
- run_rake_task
- document.reload
-
- expect(ActiveStorage::Attachment.count).to eq 1
- expect(ActiveStorage::Blob.count).to eq 1
- expect(document.storage_attachment.attachment.id).to eq attachment_id
- expect(document.storage_attachment.blob.id).to eq blob_id
-
- expect(test_storage_file_paths.count).to eq 1
- expect(storage_file_path(document)).to eq migrated_file
- expect(test_storage_file_paths.first).to eq migrated_file
- end
-
- it "does not migrate files for deleted records" do
- document = create(:document, attachment: File.new("spec/fixtures/files/clippy.pdf"))
- FileUtils.rm storage_file_path(document)
- Document.delete_all
-
- run_rake_task
-
- expect(ActiveStorage::Attachment.count).to eq 1
- expect(ActiveStorage::Blob.count).to eq 1
- expect(document.storage_attachment.filename).to eq "clippy.pdf"
- expect(test_storage_file_paths.count).to eq 0
- end
-
- def test_storage_file_paths
- Dir.glob("#{storage_root}/**/*").select { |file_or_folder| File.file?(file_or_folder) }
- end
-
- def storage_file_path(record)
- ActiveStorage::Blob.service.path_for(record.storage_attachment.blob.key)
+ expect(document.attachment.attachment.name).to eq "attachment"
+ expect(image.attachment.attachment.name).to eq "attachment"
+ expect(site_customization_image.reload.image.attachment.name).to eq "image"
end
end
end
diff --git a/spec/lib/tasks/files_spec.rb b/spec/lib/tasks/files_spec.rb
index 454abe7be..4d55334bb 100644
--- a/spec/lib/tasks/files_spec.rb
+++ b/spec/lib/tasks/files_spec.rb
@@ -11,8 +11,8 @@ describe "files tasks" do
image = build(:image)
document = build(:document)
- image.storage_attachment.blob.save!
- document.storage_attachment.blob.save!
+ image.attachment.blob.save!
+ document.attachment.blob.save!
travel_to(2.days.from_now) { run_rake_task }
@@ -24,8 +24,8 @@ describe "files tasks" do
image = build(:image)
document = build(:document)
- image.storage_attachment.blob.save!
- document.storage_attachment.blob.save!
+ image.attachment.blob.save!
+ document.attachment.blob.save!
travel_to(2.minutes.from_now) { run_rake_task }
diff --git a/spec/models/direct_upload_spec.rb b/spec/models/direct_upload_spec.rb
index 7f281cc22..815421180 100644
--- a/spec/models/direct_upload_spec.rb
+++ b/spec/models/direct_upload_spec.rb
@@ -40,8 +40,8 @@ describe DirectUpload do
direct_upload.save_attachment
expect(File.exist?(direct_upload.relation.file_path)).to be true
- expect(direct_upload.relation.storage_attachment.blob).to be_persisted
- expect(direct_upload.relation.storage_attachment.attachment).not_to be_persisted
+ expect(direct_upload.relation.attachment.blob).to be_persisted
+ expect(direct_upload.relation.attachment.attachment).not_to be_persisted
end
end
end
diff --git a/spec/models/document_spec.rb b/spec/models/document_spec.rb
index 2f8cf65fe..af3294e9f 100644
--- a/spec/models/document_spec.rb
+++ b/spec/models/document_spec.rb
@@ -4,14 +4,11 @@ describe Document do
it_behaves_like "document validations", "budget_investment_document"
it_behaves_like "document validations", "proposal_document"
- it "stores attachments with both Paperclip and Active Storage" do
+ it "stores attachments with Active Storage" do
document = create(:document, attachment: File.new("spec/fixtures/files/clippy.pdf"))
- expect(document.attachment).to exist
- expect(document.attachment_file_name).to eq "clippy.pdf"
-
- expect(document.storage_attachment).to be_attached
- expect(document.storage_attachment.filename).to eq "clippy.pdf"
+ expect(document.attachment).to be_attached
+ expect(document.attachment.filename).to eq "clippy.pdf"
end
context "scopes" do
diff --git a/spec/models/image_spec.rb b/spec/models/image_spec.rb
index d119c8138..da5b70d46 100644
--- a/spec/models/image_spec.rb
+++ b/spec/models/image_spec.rb
@@ -4,13 +4,10 @@ describe Image do
it_behaves_like "image validations", "budget_investment_image"
it_behaves_like "image validations", "proposal_image"
- it "stores attachments with both Paperclip and Active Storage" do
+ it "stores attachments with Active Storage" do
image = create(:image, attachment: File.new("spec/fixtures/files/clippy.jpg"))
- expect(image.attachment).to exist
- expect(image.attachment_file_name).to eq "clippy.jpg"
-
- expect(image.storage_attachment).to be_attached
- expect(image.storage_attachment.filename).to eq "clippy.jpg"
+ expect(image.attachment).to be_attached
+ expect(image.attachment.filename).to eq "clippy.jpg"
end
end
diff --git a/spec/models/site_customization/image_spec.rb b/spec/models/site_customization/image_spec.rb
index ddd266d0e..c1bb77880 100644
--- a/spec/models/site_customization/image_spec.rb
+++ b/spec/models/site_customization/image_spec.rb
@@ -1,15 +1,12 @@
require "rails_helper"
describe SiteCustomization::Image do
- it "stores images with both Paperclip and Active Storage" do
+ it "stores images with Active Storage" do
image = create(:site_customization_image, name: "map",
image: File.new("spec/fixtures/files/custom_map.jpg"))
- expect(image.image).to exist
- expect(image.image_file_name).to eq "custom_map.jpg"
-
- expect(image.storage_image).to be_attached
- expect(image.storage_image.filename).to eq "custom_map.jpg"
+ expect(image.image).to be_attached
+ expect(image.image.filename).to eq "custom_map.jpg"
end
describe "logo" do
diff --git a/spec/system/admin/site_customization/documents_spec.rb b/spec/system/admin/site_customization/documents_spec.rb
index fe2fe332a..6a2063299 100644
--- a/spec/system/admin/site_customization/documents_spec.rb
+++ b/spec/system/admin/site_customization/documents_spec.rb
@@ -18,7 +18,7 @@ describe "Documents", :admin do
1.times { create(:document) }
document = Document.first
- url = polymorphic_path(document.storage_attachment)
+ url = polymorphic_path(document.attachment)
visit admin_site_customization_documents_path
| |