Skip to content

Commit

Permalink
Ensuring that at least one file upload is provided for draft Works (#…
Browse files Browse the repository at this point in the history
…1848)

Co-authored-by: Carolyn Cole <cac9@princeton.edu>
  • Loading branch information
jrgriffiniii and carolyncole authored Jul 17, 2024
1 parent 22d3ee3 commit 2f23f83
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 20 deletions.
11 changes: 11 additions & 0 deletions app/services/work_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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. <a href='#{work_readme_select_path(work)}'>Please upload one</a>") 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. <a href='#{work_file_upload_path(work)}'>Please resubmit after uploading the file(s)</a>")
end
end
end
21 changes: 15 additions & 6 deletions spec/controllers/works_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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. <a href='#{work_readme_select_path(work1)}'>Please upload one</a>")
# rubocop:disable Layout/LineLength
expect(assigns[:errors]).to eq(["We apologize, the following errors were encountered: You must include a README. <a href='#{work_readme_select_path(work)}'>Please upload one</a>, You must include at least one file. <a href='#{work_file_upload_path(work)}'>Please upload one</a>. 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. <a href='#{work_file_upload_path(work1)}'>Please resubmit after uploading the file(s)</a>, You must include at least one file. <a href='#{work_file_upload_path(work1)}'>Please upload one</a>.")
# rubocop:enable Layout/LineLength
expect(error).to include("Please contact the PDC Describe administrators for any assistance.")
end
end

Expand Down Expand Up @@ -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. <a href='#{work_readme_select_path(work)}'>Please upload one</a>. 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. <a href='#{work_readme_select_path(work)}'>Please upload one</a>,")
expect(controller.flash[:notice]).to include("You must include one or more files if you are uploading files from your local environment. <a href='#{work_file_upload_path(work)}'>Please resubmit after uploading the file(s)</a>. 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))
Expand Down
24 changes: 17 additions & 7 deletions spec/controllers/works_wizard_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 }
Expand All @@ -389,21 +391,29 @@
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: [])
sign_in(user)
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. <a href='#{work_readme_select_path(work)}'>Please upload one</a>. 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. <a href='#{work_readme_select_path(work1)}'>Please upload one</a>,")
expect(error).to include("You must include one or more files if you are uploading files from your local environment. <a href='#{work_file_upload_path(work1)}'>Please resubmit after uploading the file(s)</a>.")
# rubocop:enable Layout/LineLength
expect(error).to include("Please contact the PDC Describe administrators for any assistance.")
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/models/work_state_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

{
Expand Down
10 changes: 8 additions & 2 deletions spec/services/work_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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([])
Expand Down
2 changes: 1 addition & 1 deletion spec/system/external_ids_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions spec/system/migrate_submission_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down
22 changes: 21 additions & 1 deletion spec/system/work_create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down

0 comments on commit 2f23f83

Please sign in to comment.