diff --git a/app/controllers/wizard_policy_controller.rb b/app/controllers/wizard_policy_controller.rb new file mode 100644 index 000000000..283289900 --- /dev/null +++ b/app/controllers/wizard_policy_controller.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require "nokogiri" +require "open-uri" + +# Controller to handle the policy agreement acknowlegdement before the wizard is started +# +class WizardPolicyController < ApplicationController + # get /works/policy + def show; end + + # post /works/policy + def update + group = Group.find_by(code: params[:group_code]) || current_user.default_group + if params[:agreement] == "1" + work = Work.create!(created_by_user_id: current_user.id, group:) + work.add_provenance_note(DateTime.now, "User agreed to the Data Acceptance and Retention policy", current_user.id) + redirect_to work_create_new_submission_path(work) + else + redirect_to root_path, notice: "You must agree to the policy to deposit" + end + end +end diff --git a/app/controllers/works_wizard_controller.rb b/app/controllers/works_wizard_controller.rb index a5d095210..6b3680df4 100644 --- a/app/controllers/works_wizard_controller.rb +++ b/app/controllers/works_wizard_controller.rb @@ -208,7 +208,7 @@ def redirect_aasm_error(transition_error_message) if @work.persisted? redirect_to edit_work_wizard_path(id: @work.id), notice: transition_error_message, params: else - redirect_to work_create_new_submission_path, notice: transition_error_message, params: + redirect_to work_create_new_submission_path(@work), notice: transition_error_message, params: end end end diff --git a/app/services/work_metadata_service.rb b/app/services/work_metadata_service.rb index bd02e5e60..f1d0b3156 100644 --- a/app/services/work_metadata_service.rb +++ b/app/services/work_metadata_service.rb @@ -19,8 +19,7 @@ def work_for_new_submission if params[:id].present? Work.find(params[:id]) else - group = Group.find_by(code: params[:group_code]) || current_user.default_group - Work.new(created_by_user_id: current_user.id, group_id: group.id) + Work.new(created_by_user_id: current_user.id, group_id: group_code.id) end end @@ -28,8 +27,7 @@ def work_for_new_submission # # @returns the new work def new_submission - group_id = group_code.id - work = Work.new(created_by_user_id: current_user.id, group_id:) + work = work_for_new_submission work.resource = FormToResourceService.convert(params, work) if work.valid_to_draft work.draft!(current_user) diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index f94586963..2ae3ee166 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -44,7 +44,7 @@ <% if @my_dashboard %>
- <%= link_to t("users.form.create_work"), work_create_new_submission_path, class: "btn btn-primary", :role => "button", title: "Start the process to deposit a new dataset" %> + <%= link_to t("users.form.create_work"), work_policy_path, class: "btn btn-primary", :role => "button", title: "Start the process to deposit a new dataset" %>
<% else %> <% if @can_edit %> diff --git a/app/views/wizard_policy/show.html.erb b/app/views/wizard_policy/show.html.erb new file mode 100644 index 000000000..875f223f7 --- /dev/null +++ b/app/views/wizard_policy/show.html.erb @@ -0,0 +1,21 @@ +
+ +

New Submission Policy

+ <%= render "works_wizard/wizard_progress", wizard_step: 0 %> + +

Before you proceed, please review the PDC policies for Acceptance and Retention + and Distribution to be sure your intended submission is eligible for PDC, + that you are authorized to grant permission for redistribution, and that you are prepared to pay any applicable costs. If you are unsure about any of these points, please reach out to our team before submitting. +

+ + <%= form_tag work_policy_path, method: :post %> + <%= check_box_tag(:agreement) %> + <%= label_tag(:agreement, "I understand this policy and wish to continue.") %> +
+
+ <%= link_to "Cancel", root_path, class: "btn btn-secondary" %> + <%= submit_tag "Confirm", class: "btn btn-primary wizard-next-button" %> +
+ + +
diff --git a/config/routes.rb b/config/routes.rb index ba0d3d715..001575cb1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -29,8 +29,8 @@ get "how-to-submit", to: "welcome#how_to_submit", as: :welcome_how_to_submit # The work wizard - get "works/new-submission", to: "works_wizard#new_submission", as: :work_create_new_submission - post "works/new-submission/", to: "works_wizard#new_submission_save", as: :work_new_submission + get "works/:id/new-submission", to: "works_wizard#new_submission", as: :work_create_new_submission + patch "works/:id/new-submission", to: "works_wizard#new_submission_save", as: :work_new_submission get "works/:id/readme-select", to: "works_wizard#readme_select", as: :work_readme_select patch "works/:id/readme-uploaded", to: "works_wizard#readme_uploaded", as: :work_readme_uploaded patch "works/:id/file-upload", to: "works_wizard#file_uploaded", as: :work_file_uploaded @@ -50,6 +50,10 @@ get "works/:id/update-additional", to: "works_update_additional#update_additional", as: :work_update_additional patch "works/:id/update-additional", to: "works_update_additional#update_additional_save" + # policy agreement + get "works/policy", to: "wizard_policy#show", as: :work_policy + post "works/policy", to: "wizard_policy#update" + get "works/:id/file-list", to: "works#file_list", as: :work_file_list post "work/:id/approve", to: "works#approve", as: :approve_work post "work/:id/withdraw", to: "works#withdraw", as: :withdraw_work diff --git a/spec/controllers/wizard_policy_controller_spec.rb b/spec/controllers/wizard_policy_controller_spec.rb new file mode 100644 index 000000000..bf75e1685 --- /dev/null +++ b/spec/controllers/wizard_policy_controller_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe WizardPolicyController do + include ActiveJob::TestHelper + + let(:user) { FactoryBot.create :princeton_submitter } + + context "valid user login" do + before do + sign_in user + end + + describe "#show" do + it "show the user the policy agreement for" do + get(:show) + expect(response.status).to be 200 + expect(response).to render_template(:show) + end + end + + describe "#update" do + let(:params) { { "agreement" => "1" } } + + it "creates a work with an activity" do + sign_in user + expect { post(:update, params:) }.to change { Work.count }.by(1) + expect(response.status).to be 302 + work = Work.last + expect(response).to redirect_to(work_create_new_submission_path(work)) + + expect(work.state).to eq("none") + expect(work.work_activity.count).to eq(1) + end + + context "agreement is missing" do + let(:params) { {} } + + it "redirects to the dashboard" do + sign_in user + post(:update, params:) + expect(response.status).to be 302 + end + end + end + end + + context "invalid user" do + describe "#show" do + it "redirects the user" do + get(:show) + expect(response.status).to be 302 + end + end + + describe "#update" do + let(:params) { { "agreement" => "1" } } + + it "redirects the user" do + post(:update, params:) + expect(response.status).to be 302 + end + end + end +end diff --git a/spec/controllers/works_wizard_controller_spec.rb b/spec/controllers/works_wizard_controller_spec.rb index bc1e45c1f..3ecaf167c 100644 --- a/spec/controllers/works_wizard_controller_spec.rb +++ b/spec/controllers/works_wizard_controller_spec.rb @@ -24,24 +24,34 @@ let(:uploaded_file) { fixture_file_upload("us_covid_2019.csv", "text/csv") } context "valid user login" do - it "renders the new submission wizard' step 0" do - sign_in user - get :new_submission - expect(response).to render_template("new_submission") + describe "#new_submission" do + let(:work) { FactoryBot.create :policy_work } + + it "renders the new submission wizard' step 0" do + sign_in user + get :new_submission, params: { id: work.id } + expect(response).to render_template("new_submission") + work_on_page = assigns[:work] + expect(work_on_page.id).to eq(work.id) + expect(work_on_page.work_activity.count).to eq(1) + end end describe "#new_submission_save" do + let(:work) { FactoryBot.create :policy_work } let(:params) do { + id: work.id, "title_main" => "test dataset updated", "group_id" => work.group.id, "creators" => [{ "orcid" => "", "given_name" => "Jane", "family_name" => "Smith" }] } end - it "renders creates the work and renders the edit wizard page when creating a new submission" do + it "updates the work and renders the edit wizard page when creating a new submission" do sign_in user - expect { post(:new_submission_save, params:) }.to change { Work.count }.by 1 + expect { patch(:new_submission_save, params:) }.to change { WorkActivity.count }.by 2 + expect(Work.last.work_activity.count).to eq(3) # keeps the policay activity expect(response.status).to be 302 expect(response.location.start_with?("http://test.host/works/")).to be true end @@ -51,13 +61,14 @@ context "no title is present" do let(:params_no_title) do { + id: work.id, "group_id" => work.group.id, "creators" => [{ "orcid" => "", "given_name" => "Jane", "family_name" => "Smith" }] } end it "renders the edit page when creating a new dataset without a title" do sign_in user - post(:new_submission_save, params: params_no_title) + patch(:new_submission_save, params: params_no_title) expect(response.status).to be 200 expect(assigns[:errors]).to eq(["Must provide a title"]) expect(response).to render_template(:new_submission) diff --git a/spec/factories/work.rb b/spec/factories/work.rb index b6e6cc3a0..6f9d007cc 100644 --- a/spec/factories/work.rb +++ b/spec/factories/work.rb @@ -7,6 +7,15 @@ ark { nil } end + factory :policy_work do + group { Group.research_data } + state { "none" } + created_by_user_id { FactoryBot.create(:user).id } + after(:create) do |work, _evaluator| + work.add_provenance_note(DateTime.now, "User agreed to the Data Acceptance and Retention policy", work.created_by_user.id) + end + end + factory :new_work do group { Group.research_data } state { "none" } diff --git a/spec/system/authz_submitter_spec.rb b/spec/system/authz_submitter_spec.rb index dcd0e3f9b..7658774ac 100644 --- a/spec/system/authz_submitter_spec.rb +++ b/spec/system/authz_submitter_spec.rb @@ -23,6 +23,10 @@ visit user_path(submitter1) expect(page).to have_content submitter1.given_name click_on "Submit New" + + check "agreement" + click_on "Confirm" + fill_in "title_main", with: title1 fill_in "creators[][given_name]", with: FFaker::Name.first_name diff --git a/spec/system/authz_super_admin_spec.rb b/spec/system/authz_super_admin_spec.rb index e1c1f700a..a0967406f 100644 --- a/spec/system/authz_super_admin_spec.rb +++ b/spec/system/authz_super_admin_spec.rb @@ -21,6 +21,10 @@ visit user_path(submitter2) expect(page).to have_content submitter2.given_name click_on "Submit New" + + check "agreement" + click_on "Confirm" + fill_in "title_main", with: title1 fill_in "creators[][given_name]", with: FFaker::Name.first_name diff --git a/spec/system/external_ids_spec.rb b/spec/system/external_ids_spec.rb index bbbfdaa1a..ac74e7106 100644 --- a/spec/system/external_ids_spec.rb +++ b/spec/system/external_ids_spec.rb @@ -14,6 +14,10 @@ sign_in user visit user_path(user) click_on "Submit New" + + check "agreement" + click_on "Confirm" + fill_in "title_main", with: "test title" fill_in "creators[][given_name]", with: "Sally" diff --git a/spec/system/pppl_work_create_spec.rb b/spec/system/pppl_work_create_spec.rb index 2b57280aa..92d3ced56 100644 --- a/spec/system/pppl_work_create_spec.rb +++ b/spec/system/pppl_work_create_spec.rb @@ -23,7 +23,11 @@ context "happy path" do it "produces and saves a valid datacite record", js: true do sign_in user - visit work_create_new_submission_path + visit work_policy_path + + check "agreement" + click_on "Confirm" + fill_in "title_main", with: title find("tr:last-child input[name='creators[][given_name]']").set "Samantha" find("tr:last-child input[name='creators[][family_name]']").set "Abrams" diff --git a/spec/system/work_create_spec.rb b/spec/system/work_create_spec.rb index 1127ff90a..b1795bd82 100644 --- a/spec/system/work_create_spec.rb +++ b/spec/system/work_create_spec.rb @@ -40,9 +40,11 @@ end context "when using the wizard mode and creating a new work" do + let(:work) { FactoryBot.create :policy_work, created_by_user_id: user.id } + it "persists the required metadata and saves a valid work", js: true do sign_in user - visit work_create_new_submission_path + visit work_create_new_submission_path(work) fill_in "title_main", with: title @@ -93,7 +95,7 @@ context "when failing to provide the title" do it "it renders a warning in response to form submissions", js: true do sign_in user - visit work_create_new_submission_path + visit work_create_new_submission_path(work) fill_in "title_main", with: "" click_on "Create New" @@ -105,7 +107,7 @@ context "when failing to provide the given name for the creator" do it "renders a warning in response to form submissions", js: true do sign_in user - visit work_create_new_submission_path + visit work_create_new_submission_path(work) fill_in "title_main", with: title @@ -119,7 +121,7 @@ context "when failing to provide the family name for the creator" do it "renders a warning in response to form submissions", js: true do sign_in user - visit work_create_new_submission_path + visit work_create_new_submission_path(work) fill_in "title_main", with: title @@ -387,9 +389,11 @@ end context "invalid readme" do + let(:work) { FactoryBot.create :policy_work, created_by_user_id: user.id } + it "prevents the user from continuing when the readme file is not valid", js: true do sign_in user - visit work_create_new_submission_path + visit work_create_new_submission_path(work) fill_in "title_main", with: title find("tr:last-child input[name='creators[][given_name]']").set "Samantha" diff --git a/spec/system/work_spec.rb b/spec/system/work_spec.rb index 350ce4e8c..8ce1886bf 100644 --- a/spec/system/work_spec.rb +++ b/spec/system/work_spec.rb @@ -13,6 +13,10 @@ sign_in user visit user_path(user) click_on "Submit New" + + check "agreement" + click_on "Confirm" + fill_in "title_main", with: "Supreme" fill_in "creators[][given_name]", with: "Sonia" fill_in "creators[][family_name]", with: "Sotomayor" @@ -26,6 +30,10 @@ sign_in user visit user_path(user) click_on "Submit New" + + check "agreement" + click_on "Confirm" + fill_in "title_main", with: "" click_on "Create New" expect(page).to have_content "Must provide a title" @@ -34,7 +42,9 @@ # this test depends of the fake ORCID server defined in spec/support/orcid_specs.rb it "Fills in the creator based on an ORCID ID for the wizard", js: true do sign_in user - visit work_create_new_submission_path + work = FactoryBot.create :policy_work + + visit work_create_new_submission_path(work) click_on "Add Another Creator" find("tr:last-child input[name='creators[][orcid]']").set "0000-0000-1111-2222" expect(find("tr:last-child input[name='creators[][given_name]']").value).to eq "Sally" diff --git a/spec/system/work_wizard_spec.rb b/spec/system/work_wizard_spec.rb index 4f3c056b7..3872ffbb0 100644 --- a/spec/system/work_wizard_spec.rb +++ b/spec/system/work_wizard_spec.rb @@ -7,15 +7,19 @@ sign_in user stub_datacite - visit work_create_new_submission_path - expect(page).to have_css("form[action='/works/new-submission']") + visit work_policy_path + expect(page).to have_css("form[action='/works/policy']") + check "agreement" + expect { click_on "Confirm" }.to change { Work.count }.by(1) + work = Work.last + + expect(page).to have_css("form[action='/works/#{work.id}/new-submission']") fill_in "title_main", with: "title" click_on "Add me as a Creator" expect(find("tr:last-child input[name='creators[][given_name]']").value).to eq(user.given_name) expect(find("tr:last-child input[name='creators[][family_name]']").value).to eq(user.family_name) click_on "Create New" - work = Work.last edit_form_css = "form[action='/works/#{work.id}/update-wizard']" additional_form_css = "form[action='/works/#{work.id}/update-additional']" readme_form_css = "form[action='/works/#{work.id}/readme-uploaded']"