From 874a516051bcd5ac595234afd8808f7ea7f3cdde Mon Sep 17 00:00:00 2001 From: Carolyn Cole Date: Fri, 22 Mar 2024 12:45:22 -0400 Subject: [PATCH] Separating wizard new and non wizard new refs #1684 Also adding documentation to the top of the file for which methods get called when The goal is to move the wizard to a new controller so that we can more easily update it --- app/controllers/works_controller.rb | 59 ++++++++++++++++++++--- app/views/users/show.html.erb | 2 +- app/views/works/new_submission.html.erb | 2 +- config/routes.rb | 3 +- spec/controllers/works_controller_spec.rb | 6 +-- spec/system/pppl_work_create_spec.rb | 2 +- spec/system/work_create_spec.rb | 10 ++-- spec/system/work_spec.rb | 12 ++++- 8 files changed, 75 insertions(+), 21 deletions(-) diff --git a/app/controllers/works_controller.rb b/app/controllers/works_controller.rb index 7baa8c64a..656afd02d 100644 --- a/app/controllers/works_controller.rb +++ b/app/controllers/works_controller.rb @@ -3,10 +3,33 @@ require "nokogiri" require "open-uri" +# Currently this controller supports Multiple ways to create a work, wizard mode, create dataset, and migrate +# The goal is to eventually break some of these workflows into separate contorllers. +# For the moment I'm documenting which methods get called by each workflow below. +# Note: new, edit and update get called by many of the workflows +# +# wizard mode +# new_sumission -> new_submission_save -> edit -> update -> readme_select -> readme_uploaded -> attachment_select -> +# attachment_selected -> file_other -> review -> validate -> show & file_list +# \> file_upload -> file_uploaded -^ +# +# Non wizard mode +# new & file_list -> create -> show & file_list +# +# Clicking Edit puts you in wizard mode for some reason :( +# +# migrate +# +# new & file_list -> create -> show & file_list +# +# Clicking edit +# edit & file_list -> update -> show & file_list +# + # rubocop:disable Metrics/ClassLength class WorksController < ApplicationController include ERB::Util - around_action :rescue_aasm_error, only: [:approve, :withdraw, :resubmit, :validate, :create, :new_submission] + around_action :rescue_aasm_error, only: [:approve, :withdraw, :resubmit, :validate, :create, :new_submission_save] skip_before_action :authenticate_user! before_action :authenticate_user!, unless: :public_request? @@ -19,17 +42,15 @@ def index end end - # Renders the "step 0" information page before creating a new dataset + # only non wizard mode def new group = Group.find_by(code: params[:group_code]) || current_user.default_group @work = Work.new(created_by_user_id: current_user.id, group:) @work_decorator = WorkDecorator.new(@work, current_user) @form_resource_decorator = FormResourceDecorator.new(@work, current_user) - if wizard_mode? - render "new_submission" - end end + # only non wizard mode def create @work = Work.new(created_by_user_id: current_user.id, group_id: params_group_id, user_entered_doi: params["doi"].present?) @work.resource = FormToResourceService.convert(params, @work) @@ -46,10 +67,22 @@ def create end end - # Creates the new dataset + # get Renders the "step 0" information page before creating a new dataset + # only wizard mode def new_submission - default_group_id = current_user.default_group.id - @work = Work.new(created_by_user_id: current_user.id, group_id: default_group_id) + group = Group.find_by(code: params[:group_code]) || current_user.default_group + group_id = group.id + @work = Work.new(created_by_user_id: current_user.id, group_id:) + @work_decorator = WorkDecorator.new(@work, current_user) + @form_resource_decorator = FormResourceDecorator.new(@work, current_user) + end + + # Creates the new dataset + # only wizard mode + def new_submission_save + group = Group.find_by(code: params[:group_code]) || current_user.default_group + group_id = group.id + @work = Work.new(created_by_user_id: current_user.id, group_id:) @work.resource = FormToResourceService.convert(params, @work) @work.draft!(current_user) redirect_to edit_work_path(@work, wizard: true) @@ -76,6 +109,7 @@ def show end end + # only non wizard mode def file_list if params[:id] == "NONE" # This is a special case when we render the file list for a work being created @@ -99,6 +133,7 @@ def resolve_ark # GET /works/1/edit # rubocop:disable Metrics/MethodLength + # Both wizard and not wizard mode def edit @work = Work.find(params[:id]) @work_decorator = WorkDecorator.new(@work, current_user) @@ -119,6 +154,7 @@ def edit end # rubocop:enable Metrics/MethodLength + # Both wizard and not wizard mode def update @work = Work.find(params[:id]) if current_user.blank? || !@work.editable_by?(current_user) @@ -133,12 +169,14 @@ def update end # Prompt to select how to submit their files + # only wizard mode def attachment_select @work = Work.find(params[:id]) @wizard_mode = true end # User selected a specific way to submit their files + # only wizard mode def attachment_selected @work = Work.find(params[:id]) @wizard_mode = true @@ -180,10 +218,12 @@ def file_uploaded end # Allow user to indicate where their files are located in the WWW + # only wizard mode def file_other @work = Work.find(params[:id]) end + # only wizard mode def review @work = Work.find(params[:id]) if request.method == "POST" @@ -192,6 +232,7 @@ def review end end + # only wizard mode def validate @work = Work.find(params[:id]) @work.submission_notes = params["submission_notes"] @@ -271,6 +312,7 @@ def datacite_validate end end + # only wizard mode def readme_select @work = Work.find(params[:id]) readme = Readme.new(@work, current_user) @@ -278,6 +320,7 @@ def readme_select @wizard = true end + # only wizard mode def readme_uploaded @work = Work.find(params[:id]) @wizard = true diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index de052b0fb..f94586963 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"), new_work_path(params: {wizard: true}), class: "btn btn-primary", :role => "button", title: "Start the process to deposit a new dataset" %> + <%= 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" %>
<% else %> <% if @can_edit %> diff --git a/app/views/works/new_submission.html.erb b/app/views/works/new_submission.html.erb index 054a7ad61..e377c3510 100644 --- a/app/views/works/new_submission.html.erb +++ b/app/views/works/new_submission.html.erb @@ -52,7 +52,7 @@ your submission, we may be reached at prds@princeton.edu

- <%= form_tag(action: "new_submission") do %> + <%= form_tag(action: "new_submission_save") do %> <%= render(partial: 'works/required_title', locals: {allow_many: false}) %> <%= render(partial: 'works/required_creators_table') %> <%= render 'form_hidden_fields' %> diff --git a/config/routes.rb b/config/routes.rb index c4c0b765b..eb88493f8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -29,7 +29,8 @@ get "how-to-submit", to: "welcome#how_to_submit", as: :welcome_how_to_submit get "works/:id/file-list", to: "works#file_list", as: :work_file_list - post "works/new-submission", to: "works#new_submission", as: :work_new_submission + get "works/new-submission", to: "works#new_submission", as: :work_create_new_submission + post "works/new-submission", to: "works#new_submission_save", as: :work_new_submission get "works/:id/readme-select", to: "works#readme_select", as: :work_readme_select patch "works/:id/readme-uploaded", to: "works#readme_uploaded", as: :work_readme_uploaded get "works/:id/attachment-select", to: "works#attachment_select", as: :work_attachment_select diff --git a/spec/controllers/works_controller_spec.rb b/spec/controllers/works_controller_spec.rb index f5434311c..231908a29 100644 --- a/spec/controllers/works_controller_spec.rb +++ b/spec/controllers/works_controller_spec.rb @@ -49,7 +49,7 @@ it "renders the new submission wizard' step 0" do sign_in user - get :new, params: { wizard: true } + get :new_submission expect(response).to render_template("new_submission") end @@ -66,7 +66,7 @@ "creators" => [{ "orcid" => "", "given_name" => "Jane", "family_name" => "Smith" }] } sign_in user - post(:new_submission, params:) + post(:new_submission_save, params:) expect(response.status).to be 302 expect(response.location.start_with?("http://test.host/works/")).to be true end @@ -79,7 +79,7 @@ "creators" => [{ "orcid" => "", "given_name" => "Jane", "family_name" => "Smith" }] } sign_in user - post(:new_submission, params:) + post(:new_submission_save, params:) expect(response.status).to be 302 # rubocop:disable Layout/LineLength expect(assigns[:errors]).to eq(["We apologize, the following errors were encountered: Must provide a title. Please contact the PDC Describe administrators for any assistance."]) diff --git a/spec/system/pppl_work_create_spec.rb b/spec/system/pppl_work_create_spec.rb index 603a77d00..16c26b06d 100644 --- a/spec/system/pppl_work_create_spec.rb +++ b/spec/system/pppl_work_create_spec.rb @@ -23,7 +23,7 @@ context "happy path" do it "produces and saves a valid datacite record", js: true do sign_in user - visit new_work_path(params: { wizard: true }) + visit work_create_new_submission_path 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 bc4464df3..e6228403a 100644 --- a/spec/system/work_create_spec.rb +++ b/spec/system/work_create_spec.rb @@ -42,7 +42,7 @@ context "when using the wizard mode and creating a new work" do it "persists the required metadata and saves a valid work", js: true do sign_in user - visit new_work_path(params: { wizard: true }) + visit work_create_new_submission_path fill_in "title_main", with: title @@ -93,7 +93,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 new_work_path(params: { wizard: true }) + visit work_create_new_submission_path fill_in "title_main", with: "" click_on "Create New" @@ -105,7 +105,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 new_work_path(params: { wizard: true }) + visit work_create_new_submission_path fill_in "title_main", with: title @@ -119,7 +119,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 new_work_path(params: { wizard: true }) + visit work_create_new_submission_path fill_in "title_main", with: title @@ -387,7 +387,7 @@ context "invalid readme" do it "prevents the user from continuing when the readme file is not valid", js: true do sign_in user - visit new_work_path(params: { wizard: true }) + visit work_create_new_submission_path click_on "Create New" fill_in "title_main", with: title diff --git a/spec/system/work_spec.rb b/spec/system/work_spec.rb index 22adfaf8a..350ce4e8c 100644 --- a/spec/system/work_spec.rb +++ b/spec/system/work_spec.rb @@ -31,10 +31,20 @@ expect(page).to have_content "Must provide a title" end + # 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 + 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" + expect(find("tr:last-child input[name='creators[][family_name]']").value).to eq "Smith" + end + # 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", js: true do sign_in user - visit new_work_path(params: { wizard: true }) + visit new_work_path 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"