Merge pull request #1773 from bgeuken/bsrequest_show

LGTM 👍

src/api/app/controllers/webui/request_controller.rb

@@ -84,7 +84,6 @@ def modify_review
   end
 
   def show
-    redirect_back_or_to user_show_path(User.current) and return if !params[:number]
     @bsreq = BsRequest.find_by_number(params[:number])
     unless @bsreq
       flash[:error] = "Can't find request #{params[:number]}"
@@ -111,17 +110,15 @@ def show
     @history = History.find_by_request(@bsreq, {withreviews: 1})
     @actions = @req['actions']
 
-    request_list = session[:requests]
     @request_before = nil
     @request_after = nil
-    index = request_list.index(@id) if request_list
-    if index and index > 0
-      @request_before = request_list[index-1]
-    end
+    index = session[:request_numbers].try(:index, @number)
     if index
-      # will be nul for after end
-      @request_after = request_list[index+1]
+      @request_before = session[:request_numbers][index-1] if index > 0
+      # will be nil for after end
+      @request_after = session[:request_numbers][index+1]
     end
+
     @comments = @bsreq.comments
   end
 
@@ -244,10 +241,9 @@ def diff
   def list
     redirect_to user_show_path(User.current) and return unless request.xhr? # non ajax request
     requests = BsRequest.list_ids(params)
-    elide_len = 44
-    elide_len = params[:elide_len].to_i if params[:elide_len]
-    session[:requests] = requests
-    requests = BsRequest.collection(ids: session[:requests])
+    elide_len = (params[:elide_len] || 44).to_i
+    session[:request_numbers] = requests.map { |id| BsRequest.find(id).number }.uniq
+    requests = BsRequest.collection(ids: requests)
     render :partial => 'shared/requests', :locals => {:requests => requests, :elide_len => elide_len, :no_target => params[:no_target]}
   end
 

src/api/app/views/webui/request/show.html.erb

@@ -8,10 +8,10 @@
 
 <div id="request_navigation_box" style="position: absolute; right: 0.3em;">
   <% if @request_before -%>
-      <%= link_to '<<', :controller => :request, :action => :show, id: @request_before %>
+      <%= link_to '<<', request_show_path(number: @request_before) %>
   <% end -%>
   <% if @request_after -%>
-      <%= link_to '>>', :controller => :request, :action => :show, id: @request_after %>
+      <%= link_to '>>', request_show_path(number: @request_after) %>
   <% end -%>
 </div>
 

src/api/test/functional/webui/request_controller_test.rb

@@ -303,6 +303,34 @@ def test_requests
     assert_equal [], result["aaData"]
   end
 
+  def test_going_through_request_list
+    use_js
+    login_king
+
+    visit project_requests_path(project: "Apache")
+    page.must_have_text "Requests for Apache"
+    all("a.request_link").count.must_equal 4
+
+    first("a.request_link").click
+    assert_equal "/request/show/1000", page.current_path
+    # start of list
+    page.wont_have_link("<<")
+    click_link(">>")
+    assert_equal "/request/show/10", page.current_path
+    click_link(">>")
+    assert_equal "/request/show/5", page.current_path
+    click_link(">>")
+    assert_equal "/request/show/4", page.current_path
+    page.wont_have_link(">>")
+    # end of list
+    click_link("<<")
+    assert_equal "/request/show/5", page.current_path
+    click_link("<<")
+    assert_equal "/request/show/10", page.current_path
+    click_link(">>")
+    assert_equal "/request/show/5", page.current_path
+  end
+
   def test_succesful_comment_creation
     login_Iggy to: request_show_path(1)
     fill_in 'body', with: 'Comment Body'