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

@@ -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,13 +312,15 @@ def datacite_validate
     end
   end
 
+  # only wizard mode
   def readme_select
     @work = Work.find(params[:id])
     readme = Readme.new(@work, current_user)
     @readme = readme.file_name
     @wizard = true
   end
 
+  # only wizard mode
   def readme_uploaded
     @work = Work.find(params[:id])
     @wizard = true

app/views/users/show.html.erb

@@ -44,7 +44,7 @@
 
 <% if @my_dashboard %>
   <div class="text-left dashboard-options">
-    <%= 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" %>
   </div>
 <% else %>
   <% if @can_edit %>

app/views/works/new_submission.html.erb

@@ -52,7 +52,7 @@
     your submission, we may be reached at <a href="mailto:prds@princeton.edu">prds@princeton.edu</a>
   </p>
 
-  <%= 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' %>

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

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."])

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"

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
 

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"