diff --git a/Gemfile b/Gemfile index 001e4d7a1..afa1e9a1b 100644 --- a/Gemfile +++ b/Gemfile @@ -25,7 +25,7 @@ gem 'font-awesome-rails', '~> 4.7.0.0' # Authentication gem gem "devise", ">= 4.7.1" # Shared libraries for workflow and explore -gem 'etda_utilities', '~> 0.0' +gem 'etda_utilities', "~> 0.22.0" # Ldap client gem 'net-ldap', '~> 0.16.1' # sftp for lionapth csv imports diff --git a/Gemfile.lock b/Gemfile.lock index 90465155e..11adfe0eb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -170,7 +170,7 @@ GEM rubocop (>= 1) smart_properties erubi (1.13.1) - etda_utilities (0.21.0) + etda_utilities (0.22.0) factory_bot (6.5.1) activesupport (>= 6.1.0) factory_bot_rails (6.4.4) @@ -566,7 +566,7 @@ DEPENDENCIES down ed25519 (~> 1.2.4) enumerize (~> 2.6.0) - etda_utilities (~> 0.0) + etda_utilities (~> 0.22.0) factory_bot_rails (~> 6.4.0) faker (~> 3.5.1) font-awesome-rails (~> 4.7.0.0) diff --git a/app/models/etda_file_paths.rb b/app/models/etda_file_paths.rb index 890abd657..90f2287d7 100644 --- a/app/models/etda_file_paths.rb +++ b/app/models/etda_file_paths.rb @@ -9,7 +9,8 @@ def explore_base_path EXPLORE_BASE_PATH end - def move_a_file(fid, original_file_location) + # file_class injection options: [FinalSubmissionFile, RemediatedFinalSubmissionFile] + def move_a_file(fid, original_file_location, file_class:) error_msg = '' unless File.exist?(original_file_location) @@ -18,7 +19,7 @@ def move_a_file(fid, original_file_location) return error_msg end - updated_file = FinalSubmissionFile.find(fid) + updated_file = file_class.find(fid) # this is calculating the new location based on updated submission and file attributes new_location = updated_file.full_file_path diff --git a/app/models/final_submission_file.rb b/app/models/final_submission_file.rb index 5722225a7..38aa9becd 100755 --- a/app/models/final_submission_file.rb +++ b/app/models/final_submission_file.rb @@ -53,7 +53,7 @@ def move_file path_builder = EtdaFilePaths.new original_file_location = "#{WORKFLOW_BASE_PATH}final_submission_files/#{path_builder.detailed_file_path(id)}#{asset_identifier}" - path_builder.move_a_file(id, original_file_location) + path_builder.move_a_file(id, original_file_location, file_class: self.class) end def delete_file diff --git a/app/models/remediated_final_submission_file.rb b/app/models/remediated_final_submission_file.rb index 8f2e1554e..a8742b35c 100644 --- a/app/models/remediated_final_submission_file.rb +++ b/app/models/remediated_final_submission_file.rb @@ -7,7 +7,47 @@ class RemediatedFinalSubmissionFile < ApplicationRecord validates :submission_id, :asset, presence: true validates :asset, virus_free: true + after_save :move_file + before_destroy :delete_file + def class_name self.class.to_s.underscore.dasherize end + + def current_location + # full file path including file name + full_file_path + asset_identifier + end + + def full_file_path + # file path w/o file name + main_file_path + file_detail_path + end + + def file_detail_path + # partial unique path built from file id -- ie('/01/01/') + # Note: remediated is true to differentiate paths from original files + EtdaFilePaths.new.detailed_file_path(id, remediated: true) + end + + def main_file_path + # base portion of path up to file_detail_path + SubmissionFilePath.new(submission).full_path_for_final_submissions.to_s + end + + private + + def move_file + return unless submission.status_behavior.released_for_publication? + + path_builder = EtdaFilePaths.new + original_file_location = "#{WORKFLOW_BASE_PATH}final_submission_files/#{path_builder.detailed_file_path(id, remediated: true)}#{asset_identifier}" + path_builder.move_a_file(id, original_file_location, file_class: self.class) + end + + def delete_file + return unless submission.status_behavior.released_for_publication? + + FileUtils.rm current_location + end end diff --git a/app/services/submission_release_service.rb b/app/services/submission_release_service.rb index 48174f671..562088b86 100644 --- a/app/services/submission_release_service.rb +++ b/app/services/submission_release_service.rb @@ -31,18 +31,28 @@ def unpublish(original_final_files) end def final_files_for_submission(submission) - # saves the file id and the original file path + # saves the file id, the original file path, and the class name location_array = [] submission.final_submission_files.each do |f| - location_array << [f.id, f.current_location] + location_array << [f.id, f.current_location, f.class.name] + + # also include remediated final submission files to be moved + next if f.remediated_final_submission_file.blank? + + location_array << [f.remediated_final_submission_file.id, + f.remediated_final_submission_file.current_location, + f.remediated_final_submission_file.class.name] end + location_array end def release_files(original_file_locations) etda_file_util = EtdaFilePaths.new - original_file_locations.each do |fid, original_file_location| - msg = etda_file_util.move_a_file(fid, original_file_location) + original_file_locations.each do |fid, original_file_location, class_name| + msg = etda_file_util.move_a_file(fid, + original_file_location, + file_class: class_name.constantize) next if msg.blank? record_error(msg) @@ -53,10 +63,10 @@ def release_files(original_file_locations) def file_verification(original_files_array) file_error_list = [] - original_files_array.each do |id, original_file| + original_files_array.each do |id, original_file, class_name| next if File.exist? original_file - err = "File Not Found for Final Submission File #{id}, #{original_file} " + err = "File Not Found for #{class_name.underscore.humanize} #{id}, #{original_file} " record_error(err) file_error_list << err end diff --git a/app/uploaders/submission_file_uploader.rb b/app/uploaders/submission_file_uploader.rb index 8f26d0a64..0c3d2456d 100644 --- a/app/uploaders/submission_file_uploader.rb +++ b/app/uploaders/submission_file_uploader.rb @@ -34,13 +34,11 @@ def cache_dir end def asset_prefix - case model.class_name - when 'final-submission-file' + case model_class_name + when 'final-submission-file', 'remediated-final-submission-file' Rails.root.join(WORKFLOW_BASE_PATH, 'final_submission_files') when 'admin-feedback-file' Rails.root.join(WORKFLOW_BASE_PATH, 'admin_feedback_files') - when 'remediated_final_submission_files' - Rails.root.join(WORKFLOW_BASE_PATH, 'remediated_final_submission_files') else Rails.root.join(WORKFLOW_BASE_PATH, 'format_review_files') end @@ -48,7 +46,8 @@ def asset_prefix def asset_hash path_builder = EtdaFilePaths.new - path_builder.detailed_file_path(model.id) + path_builder.detailed_file_path(model.id, + remediated: model_class_name == 'remediated-final-submission-file') end def identity_subdir @@ -65,4 +64,10 @@ def content_type_allowlist def extension_allowlist %w[pdf txt jpg jpeg png gif mp3 wav mov mp4 zip] end + + private + + def model_class_name + model.class_name + end end diff --git a/spec/models/access_level_spec.rb b/spec/models/access_level_spec.rb index e8d5a54da..40e567b67 100755 --- a/spec/models/access_level_spec.rb +++ b/spec/models/access_level_spec.rb @@ -9,12 +9,13 @@ describe '#ACCESS_LEVEL_KEYS' do it 'constant containing all access levels' do - expect(described_class::ACCESS_LEVEL_KEYS).to contain_exactly('open_access', 'restricted_to_institution', 'restricted', '') + expect(described_class::ACCESS_LEVEL_KEYS).to contain_exactly('open_access', 'restricted_to_institution', 'restricted_liberal_arts', 'restricted', '') expect(described_class::ACCESS_LEVEL_KEYS).to include('open_access') expect(described_class::ACCESS_LEVEL_KEYS).to include('restricted') expect(described_class::ACCESS_LEVEL_KEYS).to include('restricted_to_institution') + expect(described_class::ACCESS_LEVEL_KEYS).to include('restricted_liberal_arts') expect(described_class::ACCESS_LEVEL_KEYS).to include('') - expect(described_class::ACCESS_LEVEL_KEYS.length).to eq(4) + expect(described_class::ACCESS_LEVEL_KEYS.length).to eq(5) end end diff --git a/spec/models/etda_file_paths_spec.rb b/spec/models/etda_file_paths_spec.rb index f2456182a..9d6d243f2 100644 --- a/spec/models/etda_file_paths_spec.rb +++ b/spec/models/etda_file_paths_spec.rb @@ -41,4 +41,61 @@ expect(described_class.new.explore_open).to eql("#{explore_path}open_access/") end end + + describe '#move_a_file' do + let(:file_paths) { described_class.new } + let(:fid) { 123 } + let(:original_file_location) { '/tmp/original/file.pdf' } + let(:updated_file) do + instance_double('FinalSubmissionFile', + full_file_path: '/dest/path/', + asset_identifier: 'file.pdf') + end + + context 'when the original file does not exist' do + it 'returns an error message and does not look up the file' do + allow(File).to receive(:exist?).with(original_file_location).and_return(false) + allow(Rails.logger).to receive(:error) + file_class = class_double('FinalSubmissionFile') + allow(file_class).to receive(:find) + + result = file_paths.move_a_file(fid, original_file_location, file_class: file_class) + + expect(result).to eq("File not found: #{original_file_location}") + expect(file_class).not_to have_received(:find) + end + end + + context 'when the original file exists' do + it 'moves the file using the provided file_class' do + file_class = class_double('RemediatedFinalSubmissionFile').as_stubbed_const + + allow(File).to receive(:exist?).with(original_file_location).and_return(true) + allow(file_class).to receive(:find).with(fid).and_return(updated_file) + allow(FileUtils).to receive(:mkpath) + allow(FileUtils).to receive(:mv) + + result = file_paths.move_a_file(fid, original_file_location, file_class: file_class) + + expect(FileUtils).to have_received(:mkpath).with('/dest/path/') + expect(FileUtils).to have_received(:mv).with(original_file_location, '/dest/path/file.pdf') + expect(result).to eq('') + end + end + + context 'when using FinalSubmissionFile as the file_class' do + it 'uses FinalSubmissionFile to find the record' do + file_class = class_double('FinalSubmissionFile').as_stubbed_const + + allow(File).to receive(:exist?).with(original_file_location).and_return(true) + allow(file_class).to receive(:find).with(fid).and_return(updated_file) + allow(FileUtils).to receive(:mkpath) + allow(FileUtils).to receive(:mv) + + file_paths.move_a_file(fid, original_file_location, file_class: file_class) + + expect(file_class).to have_received(:find).with(fid) + end + end + end end diff --git a/spec/models/remediated_final_submission_file_spec.rb b/spec/models/remediated_final_submission_file_spec.rb index f83472632..5f2031176 100644 --- a/spec/models/remediated_final_submission_file_spec.rb +++ b/spec/models/remediated_final_submission_file_spec.rb @@ -22,6 +22,38 @@ expect(described_class.new.class_name).to eql('remediated-final-submission-file') end + describe '#current_location' do + context 'when submission is waiting for publication release' do + it 'returns full workflow file path w filename' do + submission = FactoryBot.create :submission, :waiting_for_publication_release + remediated_final_submission_file = FactoryBot.create(:remediated_final_submission_file, + submission_id: submission.id) + remediated_final_submission_file.id = 1234 + expect(remediated_final_submission_file.current_location) + .to eq( + "#{WORKFLOW_BASE_PATH}final_submission_files/" \ + "#{EtdaFilePaths.new.detailed_file_path(remediated_final_submission_file.id, remediated: true)}" \ + "#{remediated_final_submission_file.asset_identifier}" + ) + end + end + + context 'when submission has been released for publication' do + it 'returns full explore file path w filename' do + submission = FactoryBot.create :submission, :released_for_publication + remediated_final_submission_file = FactoryBot.create(:remediated_final_submission_file, + submission_id: submission.id) + remediated_final_submission_file.id = 1234 + expect(remediated_final_submission_file.current_location) + .to eq( + "#{EXPLORE_BASE_PATH + submission.access_level_key}" \ + "/#{EtdaFilePaths.new.detailed_file_path(remediated_final_submission_file.id, remediated: true)}" \ + "#{remediated_final_submission_file.asset_identifier}" + ) + end + end + end + describe 'virus scanning' do # We have these Virus scanner tests for both format review file and final_submission_file models # Not sure if it's valauble to have them here as well? @@ -60,4 +92,55 @@ end end end + + describe 'after_save :move_file' do + let(:submission) do + FactoryBot.create :submission, + :released_for_publication + end + + let(:remediated_file) do + FactoryBot.create :remediated_final_submission_file, + submission: submission + end + + it 'calls EtdaFilePaths.move_a_file with remediated_file: true' do + path_builder = instance_double(EtdaFilePaths) + allow(EtdaFilePaths).to receive(:new).and_return(path_builder) + allow(path_builder).to receive(:detailed_file_path).and_return('path/to/file/') + allow(path_builder).to receive(:move_a_file) + + original_file_location = "#{WORKFLOW_BASE_PATH}final_submission_files/" \ + "#{path_builder.detailed_file_path(remediated_file.id, remediated: true)}" \ + "#{remediated_file.asset_identifier}" + + remediated_file.save! + + # Expect twice: once during create, once during save! + expect(path_builder) + .to have_received(:move_a_file) + .with(remediated_file.id, original_file_location, file_class: remediated_file.class) + .twice + end + end + + describe 'before_destroy :delete_file' do + let(:submission) do + FactoryBot.create :submission, + :released_for_publication + end + + let(:remediated_file) do + FactoryBot.create :remediated_final_submission_file, + submission: submission + end + + it 'deletes the file from the filesystem' do + expect(File.exist?(remediated_file.current_location)).to be true + + remediated_file.destroy + + expect(File.exist?(remediated_file.current_location)).to be false + end + end end diff --git a/spec/services/submission_release_service_spec.rb b/spec/services/submission_release_service_spec.rb index b2309c269..efbd66094 100644 --- a/spec/services/submission_release_service_spec.rb +++ b/spec/services/submission_release_service_spec.rb @@ -70,4 +70,115 @@ service.publish([submission.id], DateTime.now, release_type) end end + + describe '#unpublish' do + it 'delegates to #release_files and returns its result' do + original_final_files = [[1, '/some/path/file.pdf', 'FinalSubmissionFile']] + allow(service).to receive(:release_files).and_return(true) + + result = service.unpublish(original_final_files) + expect(service).to have_received(:release_files).with(original_final_files) + expect(result).to be true + end + end + + describe '#final_files_for_submission' do + let(:submission) { FactoryBot.create(:submission, :waiting_for_publication_release) } + + context 'when submission has only final submission files' do + it 'returns id, current_location, and class name for each final file' do + final_file = FactoryBot.create(:final_submission_file, submission:) + + result = service.final_files_for_submission(submission) + + expect(result).to eq([[final_file.id, final_file.current_location, 'FinalSubmissionFile']]) + end + end + + context 'when submission has remediated final submission files' do + it 'includes both original and remediated files' do + final_file = FactoryBot.create(:final_submission_file, submission:) + remed_file = FactoryBot.create(:remediated_final_submission_file, submission:, final_submission_file: final_file) + + result = service.final_files_for_submission(submission) + + expect(result).to eq([ + [final_file.id, final_file.current_location, 'FinalSubmissionFile'], + [remed_file.id, remed_file.current_location, 'RemediatedFinalSubmissionFile'] + ]) + end + end + end + + describe '#release_files' do + let(:locations) do + [ + [1, '/workflow/path/1.pdf', 'FinalSubmissionFile'], + [2, '/workflow/path/2.pdf', 'RemediatedFinalSubmissionFile'] + ] + end + + it 'moves each file using EtdaFilePaths and returns true when all succeed' do + etda_double = instance_double(EtdaFilePaths) + allow(EtdaFilePaths).to receive(:new).and_return(etda_double) + + allow(etda_double).to receive(:move_a_file) + allow(etda_double).to receive(:move_a_file) + + result = service.release_files(locations) + expect(etda_double).to have_received(:move_a_file).with(1, '/workflow/path/1.pdf', file_class: FinalSubmissionFile) + expect(etda_double).to have_received(:move_a_file).with(2, '/workflow/path/2.pdf', file_class: RemediatedFinalSubmissionFile) + expect(result).to be true + end + + it 'records an error and returns false when move_a_file fails' do + etda_double = instance_double(EtdaFilePaths) + allow(EtdaFilePaths).to receive(:new).and_return(etda_double) + + # First iteration fails + allow(etda_double).to receive(:move_a_file).with(1, '/workflow/path/1.pdf', file_class: FinalSubmissionFile).and_return('Error moving file') + allow(etda_double).to receive(:move_a_file).with(2, '/workflow/path/2.pdf', file_class: RemediatedFinalSubmissionFile) + + allow(service).to receive(:record_error) + + result = service.release_files(locations) + expect(etda_double).to have_received(:move_a_file).with(1, '/workflow/path/1.pdf', file_class: FinalSubmissionFile) + # The second call should never be reached once an error occurs + expect(etda_double).not_to have_received(:move_a_file).with(2, '/workflow/path/2.pdf', file_class: RemediatedFinalSubmissionFile) + expect(service).to have_received(:record_error).with('Error moving file') + expect(result).to be false + end + end + + describe '#file_verification' do + let(:files_array) do + [ + [1, '/workflow/path/1.pdf', 'FinalSubmissionFile'], + [2, '/workflow/path/2.pdf', 'RemediatedFinalSubmissionFile'] + ] + end + + it 'returns valid: true when all files exist' do + allow(File).to receive(:exist?).and_return(true) + + result = service.file_verification(files_array) + + expect(result[:valid]).to be true + expect(result[:file_error_list]).to be_empty + end + + it 'records an error and returns valid: false when a file is missing' do + allow(File).to receive(:exist?).with('/workflow/path/1.pdf').and_return(true) + allow(File).to receive(:exist?).with('/workflow/path/2.pdf').and_return(false) + + expected_error = 'File Not Found for Remediated final submission file 2, /workflow/path/2.pdf ' + allow(service).to receive(:record_error) + + result = service.file_verification(files_array) + expect(service).to have_received(:record_error).with(expected_error) + + expect(result[:valid]).to be false + expect(result[:file_error_list]).to eq([expected_error]) + end + end end diff --git a/spec/uploaders/submission_file_uploader_spec.rb b/spec/uploaders/submission_file_uploader_spec.rb index 0446d1845..a5a0c6a8b 100644 --- a/spec/uploaders/submission_file_uploader_spec.rb +++ b/spec/uploaders/submission_file_uploader_spec.rb @@ -18,4 +18,64 @@ it "does not allow word docs to be uploaded" do expect { File.open('spec/fixtures/files/format_review_file_03.docx') { |f| uploader.store!(f) } }.to raise_error(CarrierWave::IntegrityError) end + + describe '#asset_prefix' do + subject(:asset_prefix) { described_class.new(model).asset_prefix } + + context "when model.class_name is 'final-submission-file'" do + let(:model) { create :final_submission_file } + + it 'returns the final submission files path' do + expect(asset_prefix).to eq(Rails.root.join(WORKFLOW_BASE_PATH, 'final_submission_files')) + end + end + + context "when model.class_name is 'admin-feedback-file'" do + let(:model) { create :admin_feedback_file, feedback_type: 'final-submission' } + + it 'returns the admin feedback files path' do + expect(asset_prefix).to eq(Rails.root.join(WORKFLOW_BASE_PATH, 'admin_feedback_files')) + end + end + + context "when model.class_name is 'remediated-final-submission-file'" do + let(:model) { create :remediated_final_submission_file } + + it 'returns the final submission files path' do + expect(asset_prefix).to eq(Rails.root.join(WORKFLOW_BASE_PATH, 'final_submission_files')) + end + end + + context 'when model.class_name is anything else' do + let(:model) { create :format_review_file } + + it 'returns the format review files path' do + expect(asset_prefix).to eq(Rails.root.join(WORKFLOW_BASE_PATH, 'format_review_files')) + end + end + end + + describe '#asset_hash' do + subject(:asset_hash) { described_class.new(model).asset_hash } + + context "when model.class_name is 'remediated-final-submission-file'" do + let(:model) { create :remediated_final_submission_file } + + it 'builds the path with remediated: true' do + path_builder = EtdaFilePaths.new + expected_hash = path_builder.detailed_file_path(model.id, remediated: true) + expect(asset_hash).to eq(expected_hash) + end + end + + context 'when model.class_name is anything else' do + let(:model) { create :final_submission_file } + + it 'builds the path with remediated: false' do + path_builder = EtdaFilePaths.new + expected_hash = path_builder.detailed_file_path(model.id, remediated: false) + expect(asset_hash).to eq(expected_hash) + end + end + end end