diff options
| -rw-r--r-- | app/controllers/request_controller.rb | 13 | ||||
| -rw-r--r-- | app/views/request/list.rhtml | 2 | ||||
| -rw-r--r-- | spec/controllers/request_controller_spec.rb | 12 | ||||
| -rw-r--r-- | spec/views/request/list.rhtml_spec.rb | 2 |
4 files changed, 22 insertions, 7 deletions
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 7bc51bc28..0f980b43f 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -13,6 +13,9 @@ require 'open-uri' class RequestController < ApplicationController before_filter :check_read_only, :only => [ :new, :show_response, :describe_state, :upload_response ] protect_from_forgery :only => [ :new, :show_response, :describe_state, :upload_response ] # See ActionController::RequestForgeryProtection for details + + MAX_RESULTS = 500 + PER_PAGE = 25 @@custom_states_loaded = false begin @@ -155,11 +158,10 @@ class RequestController < ApplicationController if @view == "recent" return redirect_to request_list_all_path(:action => "list", :view => "all", :page => @page), :status => :moved_permanently end - - # Temporary patch - # Later pages are very expensive to load - if @page > 100 - raise "Sorry. No pages after 100 today." + + # Later pages are very expensive to load + if @page > MAX_RESULTS / PER_PAGE + raise ActiveRecord::RecordNotFound.new("Sorry. No pages after #{MAX_RESULTS / PER_PAGE}.") end params[:latest_status] = @view @@ -170,6 +172,7 @@ class RequestController < ApplicationController xapian_object = perform_search([InfoRequestEvent], query, sortby, 'request_collapse') @list_results = xapian_object.results.map { |r| r[:model] } @matches_estimated = xapian_object.matches_estimated + @show_no_more_than = (@matches_estimated > MAX_RESULTS) ? MAX_RESULTS : @matches_estimated end @title = @title + " (page " + @page.to_s + ")" if (@page > 1) diff --git a/app/views/request/list.rhtml b/app/views/request/list.rhtml index 862d03828..42cd41498 100644 --- a/app/views/request/list.rhtml +++ b/app/views/request/list.rhtml @@ -30,6 +30,6 @@ </div> <% end %> - <%= will_paginate WillPaginate::Collection.new(@page, @per_page, @matches_estimated) %> + <%= will_paginate WillPaginate::Collection.new(@page, @per_page, @show_no_more_than) %> <% end %> </div> diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 97688b3c0..6a09516fd 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -53,14 +53,24 @@ describe RequestController, "when listing recent requests" do it "should assign the first page of results" do xap_results = mock_model(ActsAsXapian::Search, :results => (1..25).to_a.map { |m| { :model => m } }, - :matches_estimated => 103) + :matches_estimated => 1000000) InfoRequest.should_receive(:full_search). with([InfoRequestEvent]," (variety:sent OR variety:followup_sent OR variety:response OR variety:comment)", "created_at", anything, anything, anything, anything). and_return(xap_results) get :list, :view => 'all' assigns[:list_results].size.should == 25 + assigns[:show_no_more_than].should == RequestController::MAX_RESULTS end + it "should return 404 for pages we don't want to serve up" do + xap_results = mock_model(ActsAsXapian::Search, + :results => (1..25).to_a.map { |m| { :model => m } }, + :matches_estimated => 1000000) + lambda { + get :list, :view => 'all', :page => 100 + }.should raise_error(ActiveRecord::RecordNotFound) + end + end describe RequestController, "when showing one request" do diff --git a/spec/views/request/list.rhtml_spec.rb b/spec/views/request/list.rhtml_spec.rb index 1f86ec641..c7067294f 100644 --- a/spec/views/request/list.rhtml_spec.rb +++ b/spec/views/request/list.rhtml_spec.rb @@ -33,6 +33,7 @@ describe "when listing recent requests" do it "should be successful" do assigns[:list_results] = [ make_mock_event, make_mock_event ] assigns[:matches_estimated] = 2 + assigns[:show_no_more_than] = 100 render "request/list" response.should have_tag("div.request_listing") response.should_not have_tag("p", /No requests of this sort yet/m) @@ -41,6 +42,7 @@ describe "when listing recent requests" do it "should cope with no results" do assigns[:list_results] = [ ] assigns[:matches_estimated] = 0 + assigns[:show_no_more_than] = 0 render "request/list" response.should have_tag("p", /No requests of this sort yet/m) response.should_not have_tag("div.request_listing") |