Skip to content

Commit

Permalink
Adding a new policy page to the front of the wizard
Browse files Browse the repository at this point in the history
fixes #1688
  • Loading branch information
carolyncole authored and jrgriffiniii committed Apr 9, 2024
1 parent c9397eb commit 1feff9c
Show file tree
Hide file tree
Showing 16 changed files with 191 additions and 25 deletions.
23 changes: 23 additions & 0 deletions app/controllers/wizard_policy_controller.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion app/controllers/works_wizard_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions app/services/work_metadata_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,15 @@ 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

# generates the work for a 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)
Expand Down
2 changes: 1 addition & 1 deletion app/views/users/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@

<% if @my_dashboard %>
<div class="text-left dashboard-options">
<%= 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" %>
</div>
<% else %>
<% if @can_edit %>
Expand Down
21 changes: 21 additions & 0 deletions app/views/wizard_policy/show.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<div class="wizard-area">

<h1>New Submission Policy</h1>
<%= render "works_wizard/wizard_progress", wizard_step: 0 %>

<p> Before you proceed, please review the PDC policies for <a href="https://drive.google.com/file/d/1GECvKoOjwqvKTKYvyNThCyzTWjD6cPxs/view?usp=sharing" target="_blank">Acceptance and Retention</a>
and <a href="https://drive.google.com/file/d/1E8EgfyL2yB2rH0xCIIqYrTFE0QfY8Sk_/view?usp=sharing" target="_blank">Distribution</a> 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 <a href="mailto:[email protected]" target="_blank">reach out to our team</a> before submitting.
</p>

<%= form_tag work_policy_path, method: :post %>
<%= check_box_tag(:agreement) %>
<%= label_tag(:agreement, "I understand this policy and wish to continue.") %>
<hr />
<div class="actions">
<%= link_to "Cancel", root_path, class: "btn btn-secondary" %>
<%= submit_tag "Confirm", class: "btn btn-primary wizard-next-button" %>
</div>
</form>

</div>
8 changes: 6 additions & 2 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
66 changes: 66 additions & 0 deletions spec/controllers/wizard_policy_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -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
25 changes: 18 additions & 7 deletions spec/controllers/works_wizard_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions spec/factories/work.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
4 changes: 4 additions & 0 deletions spec/system/authz_submitter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions spec/system/authz_super_admin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions spec/system/external_ids_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 5 additions & 1 deletion spec/system/pppl_work_create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
14 changes: 9 additions & 5 deletions spec/system/work_create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

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

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

Expand Down Expand Up @@ -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"
Expand Down
12 changes: 11 additions & 1 deletion spec/system/work_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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"
Expand Down
Loading

0 comments on commit 1feff9c

Please sign in to comment.