Remove navigation parameters from issue detail URLs after updates (#42073).

Patch by Katsuya HIDAKA (user:hidakatsuya).


git-svn-id: https://svn.redmine.org/redmine/trunk@23433 e93f8b46-1217-0410-a6f0-8f06a7374b81

3 files changed, 59 insertions, 41 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 111c85bc5..a3eaec4bb 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -482,15 +482,17 @@ class ApplicationController < ActionController::Base
   end
   helper_method :back_url
 
-  def redirect_back_or_default(default, options={})
+  def redirect_back_or_default(default, options = {})
+    referer = options.delete(:referer)
+
     if back_url = validate_back_url(params[:back_url].to_s)
       redirect_to(back_url)
       return
-    elsif options[:referer]
+    elsif referer
       redirect_to_referer_or default
       return
     end
-    redirect_to default
+    redirect_to default, options
     false
   end
 
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index c5af8658f..d348ff631 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -224,9 +224,8 @@ class IssuesController < ApplicationController
       end
       respond_to do |format|
         format.html do
-          redirect_back_or_default(
-            issue_path(@issue, previous_and_next_issue_ids_params)
-          )
+          redirect_back_or_default issue_path(@issue),
+            flash: { previous_and_next_issue_ids: previous_and_next_issue_ids_params }
         end
         format.api  {render_api_ok}
       end
@@ -512,11 +511,14 @@ class IssuesController < ApplicationController
   end
 
   def retrieve_previous_and_next_issue_ids
-    if params[:prev_issue_id].present? || params[:next_issue_id].present?
-      @prev_issue_id = params[:prev_issue_id].presence.try(:to_i)
-      @next_issue_id = params[:next_issue_id].presence.try(:to_i)
-      @issue_position = params[:issue_position].presence.try(:to_i)
-      @issue_count = params[:issue_count].presence.try(:to_i)
+    if flash.key?(:previous_and_next_issue_ids)
+      flash[:previous_and_next_issue_ids].then do |info|
+        @prev_issue_id = info[:prev_issue_id].presence.try(:to_i)
+        @next_issue_id = info[:next_issue_id].presence.try(:to_i)
+        @issue_position = info[:issue_position].presence.try(:to_i)
+        @issue_count = info[:issue_count].presence.try(:to_i)
+      end
+      flash.delete(:previous_and_next_issue_ids)
     else
       retrieve_query_from_session
       if @query
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 2ed9ac9ea..1eff9b5d1 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -2642,18 +2642,47 @@ class IssuesControllerTest < Redmine::ControllerTest
   end
 
   def test_show_should_not_display_prev_next_links_for_issue_not_in_query_results
-    @request.session[:issue_query] =
-      {
-        :filters => {
-          'status_id' => {:values => [''], :operator => 'c'}
-        },
-        :project_id => 1,
-        :sort => [['id', 'asc']]
+    @request.session[:issue_query] = {
+      filters: {
+        'status_id' => {operator: 'o', values: ['']}
+      },
+      project_id: 1,
+      sort: [['id', 'asc']]
+    }
+    get(:show, params: {id: 8})
+
+    assert_response :success
+    assert_select 'a', text: /Previous/, count: 0
+    assert_select 'a', text: /Next/, count: 0
+  end
+
+  def test_show_should_display_prev_next_links_for_issue_not_in_query_when_flash_contains_previous_and_next_issue_ids
+    @request.session[:issue_query] = {
+      filters: {
+        'status_id' => {operator: 'o', values: ['']}
+      },
+      project_id: 1,
+      sort: [['id', 'asc']]
+    }
+    get(
+      :show,
+      params: { id: 8 }, # The issue#8 is closed
+      flash: {
+        previous_and_next_issue_ids: {
+          prev_issue_id: 7,
+          next_issue_id: 9,
+          issue_position: 7,
+          issue_count: 10
+        }
       }
-    get(:show, :params => {:id => 1})
+    )
+
     assert_response :success
-    assert_select 'a', :text => /Previous/, :count => 0
-    assert_select 'a', :text => /Next/, :count => 0
+    assert_select 'div.next-prev-links' do
+      assert_select 'a[href="/issues/7"]', text: /Previous/
+      assert_select 'a[href="/issues/9"]', text: /Next/
+      assert_select 'span.position', text: "7 of 10"
+    end
   end
 
   def test_show_show_should_display_prev_next_links_with_query_sort_by_user_custom_field
@@ -2684,25 +2713,6 @@ class IssuesControllerTest < Redmine::ControllerTest
     end
   end
 
-  def test_show_should_display_prev_next_links_when_request_has_previous_and_next_issue_ids_params
-    get(
-      :show,
-      :params => {
-        :id => 1,
-        :prev_issue_id => 1,
-        :next_issue_id => 3,
-        :issue_position => 2,
-        :issue_count => 4
-      }
-    )
-    assert_response :success
-    assert_select 'div.next-prev-links' do
-      assert_select 'a[href="/issues/1"]', :text => /Previous/
-      assert_select 'a[href="/issues/3"]', :text => /Next/
-      assert_select 'span.position', :text => "2 of 4"
-    end
-  end
-
   def test_show_should_display_category_field_if_categories_are_defined
     Issue.update_all :category_id => nil
     get(:show, :params => {:id => 1})
@@ -6903,7 +6913,11 @@ class IssuesControllerTest < Redmine::ControllerTest
         :issue_count => 3
       }
     )
-    assert_redirected_to '/issues/11?issue_count=3&issue_position=2&next_issue_id=12&prev_issue_id=8'
+    assert_redirected_to '/issues/11'
+    assert_equal(
+      { issue_count: '3', issue_position: '2', next_issue_id: '12', prev_issue_id: '8' },
+      flash[:previous_and_next_issue_ids]
+    )
   end
 
   def test_update_with_permission_on_tracker_should_be_allowed