diff options
| -rw-r--r-- | app/controllers/public_body_controller.rb | 10 | ||||
| -rw-r--r-- | app/helpers/public_body_helper.rb | 7 | ||||
| -rw-r--r-- | spec/controllers/public_body_controller_spec.rb | 7 | ||||
| -rw-r--r-- | spec/helpers/public_body_helper_spec.rb | 2 |
4 files changed, 21 insertions, 5 deletions
diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb index cc3d0b64a..854e79a19 100644 --- a/app/controllers/public_body_controller.rb +++ b/app/controllers/public_body_controller.rb @@ -9,9 +9,17 @@ require 'confidence_intervals' require 'tempfile' class PublicBodyController < ApplicationController + + MAX_RESULTS = 500 # TODO: tidy this up with better error messages, and a more standard infrastructure for the redirect to canonical URL def show long_cache + @page = get_search_page_from_params + requests_per_page = 25 + # Later pages are very expensive to load + if @page > MAX_RESULTS / requests_per_page + raise ActiveRecord::RecordNotFound.new("Sorry. No pages after #{MAX_RESULTS / requests_per_page}.") + end if MySociety::Format.simplify_url_part(params[:url_name], 'body') != params[:url_name] redirect_to :url_name => MySociety::Format.simplify_url_part(params[:url_name], 'body'), :status => :moved_permanently return @@ -45,7 +53,7 @@ class PublicBodyController < ApplicationController # TODO: really should just use SQL query here rather than Xapian. sortby = "described" begin - @xapian_requests = perform_search([InfoRequestEvent], query, sortby, 'request_collapse') + @xapian_requests = perform_search([InfoRequestEvent], query, sortby, 'request_collapse', requests_per_page) if (@page > 1) @page_desc = " (page " + @page.to_s + ")" else diff --git a/app/helpers/public_body_helper.rb b/app/helpers/public_body_helper.rb index 332e93284..57c90a9ba 100644 --- a/app/helpers/public_body_helper.rb +++ b/app/helpers/public_body_helper.rb @@ -38,12 +38,13 @@ module PublicBodyHelper # # Returns a string def type_of_authority(public_body) - types = public_body.tags.each_with_index.map do |tag, index| + first = true + types = public_body.tags.each.map do |tag| if PublicBodyCategory.get().by_tag().include?(tag.name) desc = PublicBodyCategory.get().singular_by_tag()[tag.name] - - if index.zero? + if first desc = desc.sub(/\S/) { |m| Unicode.upcase(m) } + first = false end link_to(desc, list_public_bodies_path(tag.name)) end diff --git a/spec/controllers/public_body_controller_spec.rb b/spec/controllers/public_body_controller_spec.rb index 130631ef6..ff0a70a6f 100644 --- a/spec/controllers/public_body_controller_spec.rb +++ b/spec/controllers/public_body_controller_spec.rb @@ -82,6 +82,13 @@ describe PublicBodyController, "when showing a body" do expect(flash[:search_params]).to eq(search_params) end + + it 'should not show high page offsets as these are extremely slow to generate' do + lambda { + get :show, { :url_name => 'dfh', :view => 'all', :page => 25 } + }.should raise_error(ActiveRecord::RecordNotFound) + end + end describe PublicBodyController, "when listing bodies" do diff --git a/spec/helpers/public_body_helper_spec.rb b/spec/helpers/public_body_helper_spec.rb index 0bf55abb4..d4f3acf78 100644 --- a/spec/helpers/public_body_helper_spec.rb +++ b/spec/helpers/public_body_helper_spec.rb @@ -74,7 +74,7 @@ describe PublicBodyHelper do :description => "spec category #{i}") heading.add_category(category) end - public_body = FactoryGirl.create(:public_body, :tag_string => 'spec_0 spec_2 unknown') + public_body = FactoryGirl.create(:public_body, :tag_string => 'unknown spec_0 spec_2') expected = '<a href="/body/list/spec_0">Spec category 0</a> and <a href="/body/list/spec_2">spec category 2</a>' expect(type_of_authority(public_body)).to eq(expected) end |