Updating cancel on new submission wizard page to remove empty work (#…

…1752)

* Updating cancel on new submission wizard page to remove empty work
fixes #1749

* Moving new submission actions into thier own controller

app/controllers/wizard_new_submission_controller.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+require "nokogiri"
+require "open-uri"
+
+# Controller to handle the policy agreement acknowlegdement before the wizard is started
+#
+class WizardNewSubmissionController < ApplicationController
+  before_action :load_work
+  before_action :can_edit
+
+  # get Renders the "step 0" information page before creating a new dataset
+  # GET /works/1/new_submission
+  def new_submission
+    @work = WorkMetadataService.new(params:, current_user:).work_for_new_submission
+    prepare_decorators_for_work_form(@work)
+  end
+
+  # Creates the new dataset or update the dataset is save only was done previously
+  # POST /works/new_submission or POST /works/1/new_submission
+  def new_submission_save
+    @work = WorkMetadataService.new(params:, current_user:).new_submission
+    @errors = @work.errors.to_a
+    if @errors.count.positive?
+      prepare_decorators_for_work_form(@work)
+      render :new_submission
+    else
+      redirect_to edit_work_wizard_path(@work)
+    end
+  end
+
+  # GET /works/1/new-submission-delete
+  def new_submission_delete
+    if @work.editable_by?(current_user) && @work.none?
+      @work.destroy
+    end
+    redirect_to user_path(current_user)
+  end
+
+  private
+
+    def load_work
+      @work = Work.find(params[:id])
+    end
+
+    def can_edit
+      return if @work.editable_by?(current_user)
+
+      redirect_to user_path(current_user), notice: "You do not have permission to modify the work."
+    end
+end

app/controllers/works_wizard_controller.rb

@@ -18,26 +18,6 @@ class WorksWizardController < ApplicationController
                                    :file_upload, :file_uploaded, :file_other, :review, :validate,
                                    :readme_select, :readme_uploaded]
 
-  # get Renders the "step 0" information page before creating a new dataset
-  # GET /works/new_submission
-  def new_submission
-    @work = WorkMetadataService.new(params:, current_user:).work_for_new_submission
-    prepare_decorators_for_work_form(@work)
-  end
-
-  # Creates the new dataset or update the dataset is save only was done previously
-  # POST /works/new_submission or POST /works/1/new_submission
-  def new_submission_save
-    @work = WorkMetadataService.new(params:, current_user:).new_submission
-    @errors = @work.errors.to_a
-    if @errors.count.positive?
-      prepare_decorators_for_work_form(@work)
-      render :new_submission
-    else
-      redirect_to edit_work_wizard_path(@work)
-    end
-  end
-
   # GET /works/1/edit-wizard
   def edit_wizard
     @wizard_mode = true

app/views/works_wizard/new_submission.html.erb → app/views/wizard_new_submission/new_submission.html.erb

@@ -36,7 +36,7 @@
   <%= render '/works/form_errors' %>
 
   <h1>New Submission</h1>
-  <%= render "wizard_progress", wizard_step: 0 %>
+  <%= render "works_wizard/wizard_progress", wizard_step: 0 %>
 
   <p>
     You are starting a new submission.
@@ -65,7 +65,7 @@
     <%= render '/works/form_hidden_fields' %>
     <hr />
     <div class="actions">
-      <%= link_to "Cancel", user_path(current_user), class: "btn btn-secondary" %>
+      <%= link_to "Cancel", work_delete_new_submission_path(@work), class: "btn btn-secondary" %>
       <%= submit_tag "Create New", class: "btn btn-primary wizard-next-button", id: "btn-create-new" %>
     </div>
   <% end %>

config/routes.rb

@@ -28,9 +28,12 @@
   get "license", to: "welcome#license", as: :welcome_license
   get "how-to-submit", to: "welcome#how_to_submit", as: :welcome_how_to_submit
 
+  # The wizard new submission controller (work wizard step 0)
+  get "works/:id/new-submission", to: "wizard_new_submission#new_submission", as: :work_create_new_submission
+  patch "works/:id/new-submission", to: "wizard_new_submission#new_submission_save", as: :work_new_submission
+  get "works/:id/new-submission-delete", to: "wizard_new_submission#new_submission_delete", as: :work_delete_new_submission
+
   # The work wizard
-  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

spec/controllers/wizard_new_submission_controller_spec.rb

@@ -0,0 +1,157 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe WizardNewSubmissionController do
+  include ActiveJob::TestHelper
+
+  let(:user) { FactoryBot.create :princeton_submitter }
+  let(:work) { FactoryBot.create :policy_work, created_by_user_id: user.id }
+
+  before do
+    stub_datacite(host: "api.datacite.org", body: datacite_register_body(prefix: "10.34770"))
+    work # makes sure the work is initialized before the tests
+  end
+
+  context "valid user login" do
+    before do
+      sign_in user
+    end
+
+    describe "#new_submission" do
+      it "renders the new submission wizard' step 0" do
+        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_cancel" do
+      it "Removes the work and redirects to the user's dashboard" do
+        expect { get :new_submission_delete, params: { id: work.id } }.to change { Work.count }.by(-1)
+        expect(response.status).to be 302
+        expect(response.location).to eq "http://test.host/users/#{user.uid}"
+      end
+    end
+
+    describe "#new_submission_save" do
+      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 "updates the work and renders the edit wizard page when creating a new submission" do
+        sign_in user
+        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
+
+      # In theory we should never get to the new submission without a title, because the javascript should prevent it
+      # In reality we are occasionally having issues with the javascript failing and the button submitting anyway.
+      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
+          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)
+        end
+      end
+    end
+  end
+
+  context "other user login" do
+    let(:other_user) { FactoryBot.create :princeton_submitter }
+
+    before do
+      sign_in other_user
+    end
+
+    describe "#new_submission" do
+      it "redirects to the user dashboard" do
+        get :new_submission, params: { id: work.id }
+        expect(response.status).to be 302
+        expect(response.location).to eq "http://test.host/users/#{other_user.uid}"
+        expect(flash[:notice]).to eq("You do not have permission to modify the work.")
+      end
+    end
+
+    describe "#new_submission_cancel" do
+      it "redirects to the user dashboard" do
+        expect { get :new_submission_delete, params: { id: work.id } }.to change { Work.count }.by(0)
+        expect(response.status).to be 302
+        expect(response.location).to eq "http://test.host/users/#{other_user.uid}"
+        expect(flash[:notice]).to eq("You do not have permission to modify the work.")
+      end
+    end
+
+    describe "#new_submission_save" do
+      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 "redirects to the user dashboard" do
+        expect { patch(:new_submission_save, params:) }.to change { WorkActivity.count }.by 0
+        expect(response.status).to be 302
+        expect(response.location).to eq "http://test.host/users/#{other_user.uid}"
+        expect(flash[:notice]).to eq("You do not have permission to modify the work.")
+      end
+    end
+  end
+
+  context "invalid user" do
+    describe "#new_submission" do
+      it "redirects the user" do
+        get :new_submission, params: { id: work.id }
+        expect(response.status).to be 302
+        expect(response.location).to eq "http://test.host/sign_in"
+      end
+    end
+
+    describe "#new_submission_cancel" do
+      let(:work) { FactoryBot.create :policy_work }
+
+      it "redirects the user" do
+        expect { get :new_submission_delete, params: { id: work.id } }.to change { Work.count }.by(0)
+        expect(response.status).to be 302
+        expect(response.location).to eq "http://test.host/sign_in"
+      end
+    end
+
+    describe "#new_submission_save" do
+      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 "redirects the user" do
+        expect { patch(:new_submission_save, params:) }.to change { WorkActivity.count }.by 0
+        expect(response.status).to be 302
+        expect(response.location).to eq "http://test.host/sign_in"
+      end
+    end
+  end
+end

spec/controllers/works_wizard_controller_spec.rb

@@ -24,58 +24,6 @@
   let(:uploaded_file) { fixture_file_upload("us_covid_2019.csv", "text/csv") }
 
   context "valid user login" do
-    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 "updates the work and renders the edit wizard page when creating a new submission" do
-        sign_in user
-        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
-
-      # In theory we should never get to the new submission without a title, because the javascript should prevent it
-      # In reality we are occasionally having issues with the javascript failing and the button submitting anyway.
-      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
-          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)
-        end
-      end
-    end
-
     describe "#update_wizard" do
       let(:params) do
         {

spec/routing/works_wizard_routing_spec.rb

@@ -18,5 +18,9 @@
     it "routes to #attachment_selected" do
       expect(post: "/works/1/attachment-select").to route_to("works_wizard#attachment_selected", id: "1")
     end
+
+    it "routes to #new_submission" do
+      expect(get: "/works/1/new-submission-delete").to route_to("wizard_new_submission#new_submission_delete", id: "1")
+    end
   end
 end

spec/system/work_spec.rb

@@ -42,7 +42,7 @@
   # 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
-    work = FactoryBot.create :policy_work
+    work = FactoryBot.create :policy_work, created_by_user_id: user.id
 
     visit work_create_new_submission_path(work)
     click_on "Add Another Creator"