Task/fix grant redirect on slug change (#1056)

* Reload grant on edit to get current friendly url

* WIP: add friendly url spec, remove instance variables

* WIP: rubocop refactor, code and comment cleanup

* WIP: add draft banner specs

app/controllers/grants_controller.rb

@@ -20,9 +20,7 @@ def show
     flash.keep
     @grant = Grant.includes(:contacts).kept.friendly.find(params[:id])
 
-    if authorize @grant
-      draft_banner
-    end
+    draft_banner if authorize @grant
   end
 
   # GET /grants/new
@@ -62,7 +60,8 @@ def update
     authorize @grant
     if @grant.update(grant_params)
       flash[:notice] = 'Grant was successfully updated.'
-      redirect_back(fallback_location: grant_path(@grant))
+
+      redirect_to edit_grant_path(@grant.reload)
     else
       flash.now[:alert] = @grant.errors.full_messages
       render :edit
@@ -101,13 +100,14 @@ def grant_params
       :max_submissions_per_reviewer,
       :panel_date,
       :panel_location,
-      criteria_attributes: [
-        :id,
-        :name,
-        :description,
-        :is_mandatory,
-        :show_comment_field,
-        :_destroy]
+      criteria_attributes: %i[
+        id
+        name  
+        description
+        is_mandatory
+        show_comment_field
+        _destroy
+      ]
     )
   end
 

spec/system/grants/grants_spec.rb

@@ -31,43 +31,73 @@
     end
   end
 
+  describe 'Draft Grant' do
+    let(:draft_grant) { create(:draft_grant) }
+
+    context 'draft banner' do
+      before(:each) do
+        login_as draft_grant.admins.first
+      end
+
+      scenario 'it shows the draft banner warning' do
+        visit grant_path(draft_grant)
+        expect(page).to have_content 'You must publish this grant to make it available to the public.'
+      end
+
+      scenario 'it removes the draft banner warning on publish' do
+        visit edit_grant_path(draft_grant)
+        click_link 'Publish this Grant'
+        visit grant_path(draft_grant)
+        expect(page).not_to have_content 'You must publish this grant to make it available to the public.'
+      end
+    end
+  end
+
   describe 'Edit' do
     context 'admin' do
-      before(:each) do
-        @grant          = create(:grant_with_users)
-        @admin_user     = @grant.grant_permissions.role_admin.first.user
+      let(:grant) { create(:grant_with_users) }
+      let(:admin_user) { grant.admins.first }
 
-        login_user @admin_user
-        visit edit_grant_path(@grant)
+      before(:each) do
+        login_user admin_user
+        visit edit_grant_path(grant)
       end
 
       scenario 'date fields edited with datepicker are properly formatted' do
         tomorrow = (Date.current + 1.day)
-        expect(page).to have_field('grant_publish_date', with: I18n.l(@grant.publish_date, format: :mmddyyyy))
+        expect(page).to have_field('grant_publish_date', with: I18n.l(grant.publish_date, format: :mmddyyyy))
         page.execute_script("$('#grant_publish_date').fdatepicker('setDate',new Date('#{I18n.l(tomorrow)}'))")
         click_button 'Update'
-        expect(@grant.reload.publish_date).to eql(tomorrow)
+        expect(grant.reload.publish_date).to eql(tomorrow)
+      end
+
+      scenario 'changing slug redirects to correct path' do
+        page.fill_in 'Short Name', with: 'newslug'
+        click_button 'Update'
+        expect(page).to have_content 'Grant was successfully updated.'
+        expect(current_path).not_to eql edit_grant_path(grant)
+        expect(current_path).to eql '/grants/newslug/edit'
       end
 
       scenario 'versioning tracks whodunnit', versioning: true do
         expect(PaperTrail).to be_enabled
         fill_in 'grant_name', with: 'New_Name'
         click_button 'Update'
         expect(page).to have_content 'Grant was successfully updated.'
-        expect(@grant.versions.last.whodunnit).to eql(@admin_user.id)
+        expect(grant.versions.last.whodunnit).to eql(admin_user.id)
       end
 
       scenario 'invalid submission', versioning: true do
-        page.fill_in 'Submission Close Date', with: (@grant.submission_open_date - 1.day).to_fs
+        page.fill_in 'Submission Close Date', with: (grant.submission_open_date - 1.day).to_fs
         click_button 'Update'
         expect(page).to have_content 'Submission Close Date must be after the opening date for submissions.'
       end
     end
 
     context 'editor' do
       before(:each) do
-        @grant          = create(:grant_with_users)
-        @editor_user     = @grant.grant_permissions.role_editor.first.user
+        @grant = create(:grant_with_users)
+        @editor_user = @grant.grant_permissions.role_editor.first.user
 
         login_user @editor_user
         visit edit_grant_path(@grant)
@@ -100,8 +130,8 @@
 
     context 'viewer' do
       before(:each) do
-        @grant          = create(:grant_with_users)
-        @viewer_user     = @grant.grant_permissions.role_viewer.first.user
+        @grant = create(:grant_with_users)
+        @viewer_user = @grant.grant_permissions.role_viewer.first.user
 
         login_user @viewer_user
         visit edit_grant_path(@grant)
@@ -208,7 +238,7 @@
 
           expect(page).to have_button REGISTERED_USER_LOGIN_BUTTON_TEXT
           click_button REGISTERED_USER_LOGIN_BUTTON_TEXT
-          expect(current_path).to eq("/registered_users/sign_in")
+          expect(current_path).to eq('/registered_users/sign_in')
 
           fill_in 'Email address', with: registered_submitter.email
           fill_in 'Password', with: registered_submitter.password
@@ -377,7 +407,7 @@
           visit grant_path(grant)
           within('div#grant-contacts') do
             expect(page).to have_text 'Contact'
-            expect(page).to have_content "#{full_name(contact.user)}"
+            expect(page).to have_content full_name(contact.user).to_s
             expect(page).to have_link contact.user.email
           end
         end
@@ -393,9 +423,9 @@
           visit grant_path(grant)
           within('div#grant-contacts') do
             expect(page).to have_text 'Contacts'
-            expect(page).to have_content "#{full_name(contact.user)}"
+            expect(page).to have_content full_name(contact.user).to_s
             expect(page).to have_link contact.user.email
-            expect(page).to have_content "#{full_name(contact2.user)}"
+            expect(page).to have_content full_name(contact2.user).to_s
             expect(page).to have_link contact2.user.email
           end
         end
@@ -429,10 +459,6 @@
       expect(grant.reload.discarded?).to be false
     end
 
-    pending 'delete button hidden when not discardable' do
-      fail '#TODO: logic to display the delete button?'
-    end
-
     scenario 'draft grant can be soft deleted' do
       grant.update!(state: 'draft')
       visit edit_grant_path(grant)
@@ -462,7 +488,7 @@
       @grant_reviewer = @grant.reviewers.first
       @system_admin = create(:system_admin_saml_user)
 
-      @grant_permission   = @grant.grant_permissions.role_editor.first
+      @grant_permission = @grant.grant_permissions.role_editor.first
     end
 
     context 'anoymous user' do
@@ -492,7 +518,6 @@
         visit grant_path(@grant)
         expect(page).to have_content authorization_error_text
       end
-
     end
 
     context 'system admin' do

spec/system/grants/state_spec.rb

@@ -92,15 +92,13 @@
 
       scenario 'current status and change status button are shown' do
         expect(page).to have_selector '#grant-state .current', text: 'Draft'
-        # expect(page).to have_content 'Current Publish Status: Draft'
         expect(page).to have_link 'Publish this Grant'
       end
 
       scenario 'cannot change status to published without questions' do
         @draft_grant.questions.each { |q| q.destroy! }
         click_link 'Publish this Grant'
         expect(page).to have_selector '#grant-state .current', text: 'Draft'
-        # expect(page).to have_content 'Current Publish Status: Draft'
         expect(page).to have_content 'Status change failed.'
       end