From 9635b5e686242265940fab329a94d567d064b78d Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Tue, 16 Jul 2024 13:11:17 -0400 Subject: [PATCH 1/3] Delete supplemental files when parent object is deleted --- app/controllers/master_files_controller.rb | 2 ++ app/jobs/bulk_action_jobs.rb | 16 ++++++++++++++++ .../controllers/master_files_controller_spec.rb | 17 +++++++++++++++++ .../media_objects_controller_spec.rb | 8 ++++++++ 4 files changed, 43 insertions(+) diff --git a/app/controllers/master_files_controller.rb b/app/controllers/master_files_controller.rb index 0c0735e9ec..cc386b31b6 100644 --- a/app/controllers/master_files_controller.rb +++ b/app/controllers/master_files_controller.rb @@ -204,6 +204,8 @@ def destroy filename = File.basename(@master_file.file_location) if @master_file.file_location.present? filename ||= @master_file.id media_object_id = @master_file.media_object_id + supplemental_files = @master_file.supplemental_files + BulkActionJobs::DeleteChildFiles.perform_later(supplemental_files, nil) @master_file.destroy flash[:notice] = "#{filename} has been deleted from the system" redirect_to edit_media_object_path(media_object_id, step: "file-upload") diff --git a/app/jobs/bulk_action_jobs.rb b/app/jobs/bulk_action_jobs.rb index a1ca94efe3..18778ed308 100644 --- a/app/jobs/bulk_action_jobs.rb +++ b/app/jobs/bulk_action_jobs.rb @@ -134,6 +134,9 @@ def perform(documents, _params) successes = [] documents.each do |id| media_object = MediaObject.find(id) + supplemental_files = media_object.supplemental_files + DeleteChildFiles.perform_later(supplemental_files, nil) + if media_object.destroy successes += [media_object] else @@ -144,6 +147,19 @@ def perform(documents, _params) end end + class DeleteChildFiles < ActiveJob::Base + def perform(documents, _params) + documents.each do |doc| + begin + doc.destroy + rescue + logger.error("Failed to delete supplemental file #{doc.id}") + next + end + end + end + end + class Move < ActiveJob::Base def perform(documents, params) collection = Admin::Collection.find( params[:target_collection_id] ) diff --git a/spec/controllers/master_files_controller_spec.rb b/spec/controllers/master_files_controller_spec.rb index 21f0d77277..a4e2eba4e2 100644 --- a/spec/controllers/master_files_controller_spec.rb +++ b/spec/controllers/master_files_controller_spec.rb @@ -228,6 +228,23 @@ class << file expect(controller.current_ability.can?(:destroy, master_file)).to be_falsey expect(post(:destroy, params: { id: master_file.id })).to render_template('errors/restricted_pid') end + + context "master file with child supplemental files" do + let!(:master_file) { FactoryBot.create(:master_file, :with_media_object, :cancelled_processing, supplemental_files: [supplemental_file]) } + let(:supplemental_file) { FactoryBot.create(:supplemental_file) } + + around(:example) do |example| + # In Rails 5.1+ this can be restricted to whitelist jobs allowed to be performed + perform_enqueued_jobs { example.run } + end + + it "deletes child files" do + login_as :administrator + expect(SupplementalFile.exists?(supplemental_file.id)).to be_truthy + post :destroy, params: { id: master_file.id } + expect(SupplementalFile.exists?(supplemental_file.id)).to be_falsey + end + end end describe "#show" do diff --git a/spec/controllers/media_objects_controller_spec.rb b/spec/controllers/media_objects_controller_spec.rb index d9ecc4bdfa..1aeab53427 100644 --- a/spec/controllers/media_objects_controller_spec.rb +++ b/spec/controllers/media_objects_controller_spec.rb @@ -1374,6 +1374,14 @@ expect(flash[:notice]).to include('3 media objects') media_objects.each {|mo| expect(MediaObject.exists?(mo.id)).to be_falsey } end + + it "should remove child supplemental files" do + supplemental_file = FactoryBot.create(:supplemental_file) + media_object = FactoryBot.create(:media_object, collection: collection, supplemental_files: [supplemental_file] ) + expect(SupplementalFile.exists?(supplemental_file.id)).to be_truthy + delete :destroy, params: { id: media_object.id } + expect(SupplementalFile.exists?(supplemental_file.id)).to be_falsey + end end describe "#confirm_remove" do From fe40545531808b8a84a9b2da8bf27c61410fb2e1 Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Fri, 23 Aug 2024 13:25:56 -0400 Subject: [PATCH 2/3] Delete section supplemental files when parent media object is deleted --- app/controllers/master_files_controller.rb | 2 -- app/jobs/bulk_action_jobs.rb | 2 +- app/models/master_file.rb | 11 +++++++++++ spec/controllers/media_objects_controller_spec.rb | 9 +++++++++ 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/app/controllers/master_files_controller.rb b/app/controllers/master_files_controller.rb index cc386b31b6..0c0735e9ec 100644 --- a/app/controllers/master_files_controller.rb +++ b/app/controllers/master_files_controller.rb @@ -204,8 +204,6 @@ def destroy filename = File.basename(@master_file.file_location) if @master_file.file_location.present? filename ||= @master_file.id media_object_id = @master_file.media_object_id - supplemental_files = @master_file.supplemental_files - BulkActionJobs::DeleteChildFiles.perform_later(supplemental_files, nil) @master_file.destroy flash[:notice] = "#{filename} has been deleted from the system" redirect_to edit_media_object_path(media_object_id, step: "file-upload") diff --git a/app/jobs/bulk_action_jobs.rb b/app/jobs/bulk_action_jobs.rb index 18778ed308..6e068b999a 100644 --- a/app/jobs/bulk_action_jobs.rb +++ b/app/jobs/bulk_action_jobs.rb @@ -135,7 +135,7 @@ def perform(documents, _params) documents.each do |id| media_object = MediaObject.find(id) supplemental_files = media_object.supplemental_files - DeleteChildFiles.perform_later(supplemental_files, nil) + DeleteChildFiles.perform_now(supplemental_files, nil) if media_object.destroy successes += [media_object] diff --git a/app/models/master_file.rb b/app/models/master_file.rb index 789471d64c..22a5f5b85a 100644 --- a/app/models/master_file.rb +++ b/app/models/master_file.rb @@ -164,6 +164,7 @@ def error after_save :update_stills_from_offset!, if: Proc.new { |mf| mf.previous_changes.include?("poster_offset") || mf.previous_changes.include?("thumbnail_offset") } before_destroy :stop_processing! before_destroy :update_parent! + before_destroy :remove_child_files define_hooks :after_transcoding, :after_processing after_update_index { |mf| mf.media_object&.enqueue_long_indexing } @@ -562,6 +563,12 @@ def stop_processing! ActiveEncodeJobs::CancelEncodeJob.perform_later(workflow_id, id) if workflow_id.present? && !finished_processing? end + # Delete does not trigger callbacks so override method to ensure deletion of child supplemental files + def delete + remove_child_files + super + end + protected def mediainfo @@ -792,6 +799,10 @@ def update_parent! end end + def remove_child_files + BulkActionJobs::DeleteChildFiles.perform_later(supplemental_files, nil) + end + private def generate_waveform diff --git a/spec/controllers/media_objects_controller_spec.rb b/spec/controllers/media_objects_controller_spec.rb index 1aeab53427..c59ab6ed63 100644 --- a/spec/controllers/media_objects_controller_spec.rb +++ b/spec/controllers/media_objects_controller_spec.rb @@ -1382,6 +1382,15 @@ delete :destroy, params: { id: media_object.id } expect(SupplementalFile.exists?(supplemental_file.id)).to be_falsey end + + it "should remove section supplemental files" do + supplemental_file = FactoryBot.create(:supplemental_file) + media_object = FactoryBot.create(:media_object, collection: collection ) + master_file = FactoryBot.create(:master_file, media_object: media_object, supplemental_files: [supplemental_file]) + expect(SupplementalFile.exists?(supplemental_file.id)).to be_truthy + delete :destroy, params: { id: media_object.id } + expect(SupplementalFile.exists?(supplemental_file.id)).to be_falsey + end end describe "#confirm_remove" do From 68581f31de2aaef44b2d23d3a4d89731a8b99bcb Mon Sep 17 00:00:00 2001 From: Mason Ballengee <68433277+masaball@users.noreply.github.com> Date: Mon, 26 Aug 2024 12:37:19 -0400 Subject: [PATCH 3/3] Update spec/controllers/master_files_controller_spec.rb Co-authored-by: Chris Colvard --- spec/controllers/master_files_controller_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/controllers/master_files_controller_spec.rb b/spec/controllers/master_files_controller_spec.rb index a4e2eba4e2..bc268418e7 100644 --- a/spec/controllers/master_files_controller_spec.rb +++ b/spec/controllers/master_files_controller_spec.rb @@ -234,7 +234,6 @@ class << file let(:supplemental_file) { FactoryBot.create(:supplemental_file) } around(:example) do |example| - # In Rails 5.1+ this can be restricted to whitelist jobs allowed to be performed perform_enqueued_jobs { example.run } end