diff --git a/app/assets/javascripts/documentable.js b/app/assets/javascripts/documentable.js index 7aa9e1b29..3a2880da5 100644 --- a/app/assets/javascripts/documentable.js +++ b/app/assets/javascripts/documentable.js @@ -115,7 +115,8 @@ $("#max-documents-notice").removeClass("hide"); }, initializeRemoveCachedDocumentLinks: function() { - $("#nested-documents").on("ajax:complete", "a.remove-cached-attachment", function() { + $("#nested-documents").on("click", "a.remove-cached-attachment", function(event) { + event.preventDefault(); App.Documentable.unlockUploads(); $(this).closest(".direct-upload").remove(); }); diff --git a/app/assets/javascripts/imageable.js b/app/assets/javascripts/imageable.js index a6e658796..84ad8ce8f 100644 --- a/app/assets/javascripts/imageable.js +++ b/app/assets/javascripts/imageable.js @@ -119,7 +119,8 @@ } }, initializeRemoveCachedImageLinks: function() { - $("#nested-image").on("ajax:complete", "a.remove-cached-attachment", function() { + $("#nested-image").on("click", "a.remove-cached-attachment", function(event) { + event.preventDefault(); $("#new_image_link").removeClass("hide"); $(this).closest(".direct-upload").remove(); }); diff --git a/app/components/documents/fields_component.rb b/app/components/documents/fields_component.rb index 3aa0126bf..1ef9febea 100644 --- a/app/components/documents/fields_component.rb +++ b/app/components/documents/fields_component.rb @@ -18,16 +18,7 @@ class Documents::FieldsComponent < ApplicationComponent def destroy_link if !document.persisted? && document.cached_attachment.present? - link_to t("documents.form.delete_button"), - direct_upload_destroy_path( - "direct_upload[resource_type]": document.documentable_type, - "direct_upload[resource_id]": document.documentable_id, - "direct_upload[resource_relation]": "documents", - "direct_upload[cached_attachment]": document.cached_attachment - ), - method: :delete, - remote: true, - class: "delete remove-cached-attachment" + link_to t("documents.form.delete_button"), "#", class: "delete remove-cached-attachment" else link_to_remove_association document.new_record? ? t("documents.form.cancel_button") : t("documents.form.delete_button"), f, class: "delete remove-document" end diff --git a/app/components/images/fields_component.rb b/app/components/images/fields_component.rb index 9b6ceb80d..ddf60fa45 100644 --- a/app/components/images/fields_component.rb +++ b/app/components/images/fields_component.rb @@ -19,16 +19,7 @@ class Images::FieldsComponent < ApplicationComponent def destroy_link if !image.persisted? && image.cached_attachment.present? - link_to t("images.form.delete_button"), - direct_upload_destroy_path( - "direct_upload[resource_type]": image.imageable_type, - "direct_upload[resource_id]": image.imageable_id, - "direct_upload[resource_relation]": "image", - "direct_upload[cached_attachment]": image.cached_attachment - ), - method: :delete, - remote: true, - class: "delete remove-cached-attachment" + link_to t("images.form.delete_button"), "#", class: "delete remove-cached-attachment" else link_to_remove_association t("images.form.delete_button"), f, class: "delete remove-image" end diff --git a/app/controllers/direct_uploads_controller.rb b/app/controllers/direct_uploads_controller.rb index a127967c7..b46fdda29 100644 --- a/app/controllers/direct_uploads_controller.rb +++ b/app/controllers/direct_uploads_controller.rb @@ -20,23 +20,11 @@ class DirectUploadsController < ApplicationController destroy_link: render_destroy_upload_link(@direct_upload), attachment_url: @direct_upload.relation.attachment.url } else - @direct_upload.destroy_attachment render json: { errors: @direct_upload.errors[:attachment].join(", ") }, status: 422 end end - def destroy - @direct_upload = DirectUpload.new(direct_upload_params.merge(user: current_user)) - @direct_upload.relation.set_attachment_from_cached_attachment - - if @direct_upload.destroy_attachment - render json: :ok - else - render json: :error - end - end - private def direct_upload_params diff --git a/app/helpers/direct_uploads_helper.rb b/app/helpers/direct_uploads_helper.rb index eabb0425e..bc89bb285 100644 --- a/app/helpers/direct_uploads_helper.rb +++ b/app/helpers/direct_uploads_helper.rb @@ -1,16 +1,6 @@ module DirectUploadsHelper def render_destroy_upload_link(direct_upload) label = direct_upload.resource_relation == "image" ? "images" : "documents" - link_to t("#{label}.form.delete_button"), - direct_upload_destroy_path( - "direct_upload[resource_type]": direct_upload.resource_type, - "direct_upload[resource_id]": direct_upload.resource_id, - "direct_upload[resource_relation]": direct_upload.resource_relation, - "direct_upload[cached_attachment]": direct_upload.relation.cached_attachment, - format: :json - ), - method: :delete, - remote: true, - class: "delete remove-cached-attachment" + link_to t("#{label}.form.delete_button"), "#", class: "delete remove-cached-attachment" end end diff --git a/app/models/direct_upload.rb b/app/models/direct_upload.rb index 2d2e9c14b..2b2760011 100644 --- a/app/models/direct_upload.rb +++ b/app/models/direct_upload.rb @@ -37,10 +37,6 @@ class DirectUpload @relation.attachment.save end - def destroy_attachment - @relation.attachment.destroy - end - def persisted? false end diff --git a/app/models/document.rb b/app/models/document.rb index 045370663..e6e21b526 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -5,7 +5,7 @@ class Document < ApplicationRecord hash_data: ":class/:style/:custom_hash_data", use_timestamp: false, hash_secret: Rails.application.secrets.secret_key_base - attr_accessor :cached_attachment, :remove, :original_filename + attr_accessor :cached_attachment belongs_to :user belongs_to :documentable, polymorphic: true @@ -22,7 +22,6 @@ class Document < ApplicationRecord validates :documentable_type, presence: true, if: -> { persisted? } before_save :set_attachment_from_cached_attachment, if: -> { cached_attachment.present? } - after_save :remove_cached_attachment, if: -> { cached_attachment.present? } scope :admin, -> { where(admin: true) } @@ -59,9 +58,7 @@ class Document < ApplicationRecord end def custom_hash_data(attachment) - original_filename = if !attachment.instance.persisted? && attachment.instance.remove - attachment.instance.original_filename - elsif !attachment.instance.persisted? + original_filename = if !attachment.instance.persisted? attachment.instance.attachment_file_name else attachment.instance.title @@ -104,14 +101,4 @@ class Document < ApplicationRecord errors.add(:attachment, I18n.t("errors.messages.blank")) end end - - def remove_cached_attachment - document = Document.new(documentable: documentable, - cached_attachment: cached_attachment, - user: user, - remove: true, - original_filename: title) - document.set_attachment_from_cached_attachment - document.attachment.destroy - end end diff --git a/app/models/image.rb b/app/models/image.rb index 9268fbfcc..7bcac83f1 100644 --- a/app/models/image.rb +++ b/app/models/image.rb @@ -30,7 +30,6 @@ class Image < ApplicationRecord validate :validate_image_dimensions, if: -> { attachment.present? && attachment.dirty? } before_save :set_attachment_from_cached_attachment, if: -> { cached_attachment.present? } - after_save :remove_cached_attachment, if: -> { cached_attachment.present? } def set_cached_attachment_from_attachment self.cached_attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem @@ -120,12 +119,4 @@ class Image < ApplicationRecord def attachment_of_valid_content_type? attachment.present? && imageable_accepted_content_types.include?(attachment_content_type) end - - def remove_cached_attachment - image = Image.new(imageable: imageable, - cached_attachment: cached_attachment, - user: user) - image.set_attachment_from_cached_attachment - image.attachment.destroy - end end diff --git a/config/routes/direct_upload.rb b/config/routes/direct_upload.rb index dbcf9a9d0..434926f00 100644 --- a/config/routes/direct_upload.rb +++ b/config/routes/direct_upload.rb @@ -1,2 +1 @@ resources :direct_uploads, only: [:create] -delete "direct_uploads/destroy", to: "direct_uploads#destroy", as: :direct_upload_destroy diff --git a/config/schedule.rb b/config/schedule.rb index 0c70a3ec7..9acb31c19 100644 --- a/config/schedule.rb +++ b/config/schedule.rb @@ -36,6 +36,10 @@ end # rake "dashboards:send_notifications" # end +every 1.day, at: "1:00 am", roles: [:cron] do + rake "files:remove_old_cached_attachments" +end + every 1.day, at: "3:00 am", roles: [:cron] do rake "votes:reset_hot_score" end diff --git a/lib/tasks/files.rake b/lib/tasks/files.rake new file mode 100644 index 000000000..ef46097cc --- /dev/null +++ b/lib/tasks/files.rake @@ -0,0 +1,18 @@ +namespace :files do + desc "Removes cached attachments which weren't deleted for some reason" + task remove_old_cached_attachments: :environment do + paths = Dir.glob(Rails.root.join("public/system/*/cached_attachments/**/*")) + logger = ApplicationLogger.new + + paths.each do |path| + if File.file?(path) + if File.mtime(path) < 1.day.ago + File.delete(path) + logger.info "The file #{path} has been removed" + end + else + Dir.delete(path) if Dir.empty?(path) + end + end + end +end diff --git a/spec/lib/tasks/files_spec.rb b/spec/lib/tasks/files_spec.rb new file mode 100644 index 000000000..2c464bc82 --- /dev/null +++ b/spec/lib/tasks/files_spec.rb @@ -0,0 +1,46 @@ +require "rails_helper" + +describe "files tasks" do + describe "remove_old_cached_attachments" do + let(:run_rake_task) do + Rake::Task["files:remove_old_cached_attachments"].reenable + Rake.application.invoke_task("files:remove_old_cached_attachments") + end + + it "deletes old cached attachments" do + image = build(:image) + document = build(:document) + + image.attachment.save + document.attachment.save + + travel_to(2.days.from_now) { run_rake_task } + + expect(File.exists?(image.attachment.path)).to be false + expect(File.exists?(document.attachment.path)).to be false + end + + it "does not delete recent cached attachments" do + image = build(:image) + document = build(:document) + + image.attachment.save + document.attachment.save + + travel_to(2.minutes.from_now) { run_rake_task } + + expect(File.exists?(image.attachment.path)).to be true + expect(File.exists?(document.attachment.path)).to be true + end + + it "does not delete old regular attachments" do + image = create(:image) + document = create(:document) + + travel_to(2.days.from_now) { run_rake_task } + + expect(File.exists?(image.attachment.path)).to be true + expect(File.exists?(document.attachment.path)).to be true + end + end +end diff --git a/spec/models/direct_upload_spec.rb b/spec/models/direct_upload_spec.rb index c4bc491aa..9c5657d55 100644 --- a/spec/models/direct_upload_spec.rb +++ b/spec/models/direct_upload_spec.rb @@ -43,16 +43,4 @@ describe DirectUpload do expect(proposal_document_direct_upload.relation.attachment.path).to include("cached_attachments") end end - - context "destroy_attachment" do - it "removes uploaded file" do - proposal_document_direct_upload = build(:direct_upload, :proposal, :documents) - - proposal_document_direct_upload.save_attachment - uploaded_path = proposal_document_direct_upload.relation.attachment.path - proposal_document_direct_upload.destroy_attachment - - expect(File.exist?(uploaded_path)).to eq(false) - end - end end