From 2f23f8327a3e4965d05d60d9344fa06df6edbbeb Mon Sep 17 00:00:00 2001 From: "James R. Griffin III" <1443986+jrgriffiniii@users.noreply.github.com> Date: Wed, 17 Jul 2024 08:10:16 -0400 Subject: [PATCH] Ensuring that at least one file upload is provided for draft Works (#1848) Co-authored-by: Carolyn Cole --- app/services/work_validator.rb | 11 +++++++++ spec/controllers/works_controller_spec.rb | 21 +++++++++++----- .../works_wizard_controller_spec.rb | 24 +++++++++++++------ spec/models/work_state_spec.rb | 2 +- spec/services/work_validator_spec.rb | 10 ++++++-- spec/system/external_ids_spec.rb | 2 +- spec/system/migrate_submission_spec.rb | 5 ++-- spec/system/work_create_spec.rb | 22 ++++++++++++++++- 8 files changed, 77 insertions(+), 20 deletions(-) diff --git a/app/services/work_validator.rb b/app/services/work_validator.rb index 938b76bf4..1fefbff58 100644 --- a/app/services/work_validator.rb +++ b/app/services/work_validator.rb @@ -162,6 +162,17 @@ def validate_metadata def validate_files return if @work.resource.migrated readme = Readme.new(work, nil) + + # when files are not uploaded errors.add(:base, "You must include a README. Please upload one") if readme.blank? + if !work.files_location_upload? + elsif work.uploads.length < 2 + + # files_location_upload? + # 1 readme and 1 file + # 2 readme files and 0 files + errors.add(:base, + "You must include one or more files if you are uploading files from your local environment. Please resubmit after uploading the file(s)") + end end end diff --git a/spec/controllers/works_controller_spec.rb b/spec/controllers/works_controller_spec.rb index 5ac8f14c4..a027ff053 100644 --- a/spec/controllers/works_controller_spec.rb +++ b/spec/controllers/works_controller_spec.rb @@ -725,6 +725,7 @@ end context "no files attached" do + let(:work1) { FactoryBot.create(:draft_work, doi: "10.34770/123-abc") } let(:data) do [ FactoryBot.build(:s3_readme), @@ -734,16 +735,23 @@ let(:fake_s3_service) { stub_s3(data:) } before do fake_s3_service - work.complete_submission!(user) + allow(Work).to receive(:find).with(work1.id.to_s).and_return(work1) + work1.complete_submission!(user) allow(fake_s3_service).to receive(:client_s3_files).and_return([]) sign_in curator end it "handles aproval errors" do - post :approve, params: { id: work.id } + post :approve, params: { id: work1.id } expect(response.status).to be 302 + errors = assigns[:errors] + expect(errors).not_to be_empty + expect(errors.length).to eq(1) + error = errors.first + expect(error).to include("We apologize, the following errors were encountered: You must include a README. Please upload one") # rubocop:disable Layout/LineLength - expect(assigns[:errors]).to eq(["We apologize, the following errors were encountered: You must include a README. Please upload one, You must include at least one file. Please upload one. Please contact the PDC Describe administrators for any assistance."]) + expect(error).to include("You must include one or more files if you are uploading files from your local environment. Please resubmit after uploading the file(s), You must include at least one file. Please upload one.") # rubocop:enable Layout/LineLength + expect(error).to include("Please contact the PDC Describe administrators for any assistance.") end end @@ -1267,18 +1275,19 @@ end describe "#validate" do - it "validates a work and errors when there is no readme" do + it "validates a work and errors when there is no readme and no files" do stub_s3 sign_in user post :validate, params: { id: work.id } expect(response).to redirect_to(edit_work_path(work)) # rubocop:disable Layout/LineLength - expect(controller.flash[:notice]).to eq("We apologize, the following errors were encountered: You must include a README. Please upload one. Please contact the PDC Describe administrators for any assistance.") + expect(controller.flash[:notice]).to include("We apologize, the following errors were encountered: You must include a README. Please upload one,") + expect(controller.flash[:notice]).to include("You must include one or more files if you are uploading files from your local environment. Please resubmit after uploading the file(s). Please contact the PDC Describe administrators for any assistance.") # rubocop:enable Layout/LineLength end it "validates a work completes it when there are no errors" do - stub_s3 data: [FactoryBot.build(:s3_readme)] + stub_s3 data: [FactoryBot.build(:s3_readme), FactoryBot.build(:s3_file)] sign_in user post :validate, params: { id: work.id } expect(response).to redirect_to(user_path(user)) diff --git a/spec/controllers/works_wizard_controller_spec.rb b/spec/controllers/works_wizard_controller_spec.rb index 8c63583e7..2c08c07ad 100644 --- a/spec/controllers/works_wizard_controller_spec.rb +++ b/spec/controllers/works_wizard_controller_spec.rb @@ -333,8 +333,9 @@ end describe "#validate" do + let(:s3_readme) { FactoryBot.build(:s3_readme) } before do - stub_s3 data: [FactoryBot.build(:s3_readme)] + stub_s3 data: [s3_readme, FactoryBot.build(:s3_file)] sign_in user end @@ -373,7 +374,8 @@ describe "#validate" do before do stub_s3(data: [ - FactoryBot.build(:s3_readme) + FactoryBot.build(:s3_readme), + FactoryBot.build(:s3_file) ]) sign_in(user) post :validate, params: { id: work.id } @@ -389,6 +391,8 @@ end context "a work with no README files" do + let(:work1) { FactoryBot.create(:draft_work, doi: "10.34770/123-abc") } + describe "#validate" do before do stub_s3(data: []) @@ -396,14 +400,20 @@ end it "renders the error message" do - work.save - post :validate, params: { id: work.id } - expect(response).to redirect_to(edit_work_wizard_path(work)) + work1.save + post :validate, params: { id: work1.id } + expect(response).to redirect_to(edit_work_wizard_path(work1)) expect(response.status).to be 302 - expect(work.reload).to be_draft + expect(work1.reload).to be_draft + errors = assigns[:errors] + expect(errors).not_to be_empty + expect(errors.length).to eq(1) + error = errors.first # rubocop:disable Layout/LineLength - expect(assigns[:errors]).to eq(["We apologize, the following errors were encountered: You must include a README. Please upload one. Please contact the PDC Describe administrators for any assistance."]) + expect(error).to include("We apologize, the following errors were encountered: You must include a README. Please upload one,") + expect(error).to include("You must include one or more files if you are uploading files from your local environment. Please resubmit after uploading the file(s).") # rubocop:enable Layout/LineLength + expect(error).to include("Please contact the PDC Describe administrators for any assistance.") end end end diff --git a/spec/models/work_state_spec.rb b/spec/models/work_state_spec.rb index 0e65389d4..961b6531c 100644 --- a/spec/models/work_state_spec.rb +++ b/spec/models/work_state_spec.rb @@ -4,7 +4,7 @@ RSpec.describe "Work state transions", type: :model do let(:curator_user) { FactoryBot.create :user, groups_to_admin: [work.group] } before do - stub_s3 data: [FactoryBot.build(:s3_readme)] + stub_s3 data: [FactoryBot.build(:s3_readme), FactoryBot.build(:s3_file)] end { diff --git a/spec/services/work_validator_spec.rb b/spec/services/work_validator_spec.rb index c0a6db179..142c2ce32 100644 --- a/spec/services/work_validator_spec.rb +++ b/spec/services/work_validator_spec.rb @@ -85,10 +85,16 @@ end end - context "a readme exisits" do + context "a readme exists" do + let(:s3_readme) { FactoryBot.build(:s3_readme) } + let(:s3_file) { FactoryBot.build(:s3_file) } + before do + stub_s3 data: [s3_readme, s3_file] + end + it "is valid" do - stub_s3 data: [FactoryBot.build(:s3_readme)] validator = WorkValidator.new(work) + expect(validator.valid?).to be_truthy expect(validator.valid_to_complete).to be_truthy expect(work.errors.full_messages).to eq([]) diff --git a/spec/system/external_ids_spec.rb b/spec/system/external_ids_spec.rb index 823d20c8a..f7fe8430e 100644 --- a/spec/system/external_ids_spec.rb +++ b/spec/system/external_ids_spec.rb @@ -7,7 +7,7 @@ before do stub_datacite(host: "api.datacite.org", body: datacite_register_body(prefix: "10.34770")) - stub_s3 data: [FactoryBot.build(:s3_readme)] + stub_s3 data: [FactoryBot.build(:s3_readme), FactoryBot.build(:s3_file)] end it "Mints a DOI, but does not mint an ark at any point in the wizard proccess" do diff --git a/spec/system/migrate_submission_spec.rb b/spec/system/migrate_submission_spec.rb index 407f2b267..65730dc16 100644 --- a/spec/system/migrate_submission_spec.rb +++ b/spec/system/migrate_submission_spec.rb @@ -49,7 +49,7 @@ context "happy path" do before do stub_request(:get, "https://handle.stage.datacite.org/10.34770/123-abc").to_return(status: 200, body: "", headers: {}) - stub_s3 data: [FactoryBot.build(:s3_readme)] + stub_s3 data: [FactoryBot.build(:s3_readme), FactoryBot.build(:s3_file)] end it "produces and saves a valid datacite record" do @@ -154,9 +154,10 @@ click_on "Create" click_on "Complete" expect(page).to have_content "You must include a README." + expect(page).to have_content "You must include one or more files if you are uploading files from your local environment" # fake that the user put a file up in globus - allow(fake_s3_service).to receive(:client_s3_files).and_return([FactoryBot.build(:s3_readme)]) + allow(fake_s3_service).to receive(:client_s3_files).and_return([FactoryBot.build(:s3_readme), FactoryBot.build(:s3_file)]) click_on "Save Work" click_on "Complete" expect(page).to have_content "awaiting_approval" diff --git a/spec/system/work_create_spec.rb b/spec/system/work_create_spec.rb index d2f88857f..15380ec13 100644 --- a/spec/system/work_create_spec.rb +++ b/spec/system/work_create_spec.rb @@ -280,6 +280,26 @@ expect(page).to have_content("Please upload the README") expect(page).to have_content("README.txt was previously uploaded. You will replace it if you select a different file.") end + + context "when a Work has no attached files" do + let(:work) do + FactoryBot.create(:draft_work, created_by_user_id: user.id) + end + let(:s3_readme) { FactoryBot.build(:s3_readme, work:) } + + before do + stub_s3 data: [s3_readme] + sign_in user + end + + it "redirects users to the edit Work view and does not advance the Work beyond the draft state" do + post work_validate_path(work) + + work.reload + expect(work.state).to eq("draft") + expect(response).to redirect_to(edit_work_path(work)) + end + end end end @@ -290,7 +310,7 @@ end before do - stub_s3 data: [FactoryBot.build(:s3_readme, work:)] + stub_s3 data: [FactoryBot.build(:s3_readme, work:), FactoryBot.build(:s3_file)] end it "allows users to upload files", js: true do