diff options
-rw-r--r-- | app/controllers/application_controller.rb | 9 | ||||
-rw-r--r-- | app/controllers/request_controller.rb | 3 | ||||
-rw-r--r-- | app/models/track_thing.rb | 15 | ||||
-rw-r--r-- | app/views/public_body/_search_ahead.rhtml | 5 | ||||
-rw-r--r-- | app/views/request/_search_ahead.rhtml | 2 | ||||
-rw-r--r-- | spec/controllers/application_controller_spec.rb | 15 | ||||
-rw-r--r-- | spec/controllers/public_body_controller_spec.rb | 3 | ||||
-rw-r--r-- | spec/controllers/request_controller_spec.rb | 12 | ||||
-rw-r--r-- | spec/models/track_thing_spec.rb | 7 | ||||
-rw-r--r-- | spec/spec_helper.rb | 10 |
10 files changed, 64 insertions, 17 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index eae27c667..73ba74f30 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -365,7 +365,10 @@ class ApplicationController < ActionController::Base def get_search_page_from_params return (params[:page] || "1").to_i end + def perform_search_typeahead(query, model) + @page = get_search_page_from_params + @per_page = 10 query_words = query.split(/ +(?![-+]+)/) if query_words.last.nil? || query_words.last.strip.length < 3 xapian_requests = nil @@ -376,8 +379,8 @@ class ApplicationController < ActionController::Base collapse = 'request_collapse' end options = { - :offset => 0, - :limit => 5, + :offset => (@page - 1) * @per_page, + :limit => @per_page, :sort_by_prefix => nil, :sort_by_ascending => true, :collapse_by_prefix => collapse, @@ -534,7 +537,7 @@ class ApplicationController < ActionController::Base def quietly_try_to_open(url) begin result = open(url).read.strip - rescue OpenURI::HTTPError, SocketError + rescue OpenURI::HTTPError, SocketError, Errno::ETIMEDOUT, Errno::ECONNREFUSED, Errno::EHOSTUNREACH logger.warn("Unable to open third-party URL #{url}") result = "" end diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 90c1f416d..aeb6d31fe 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -689,10 +689,11 @@ class RequestController < ApplicationController # Internal function def get_attachment_internal(html_conversion) @incoming_message = IncomingMessage.find(params[:incoming_message_id]) + @requested_request = InfoRequest.find(params[:id]) @incoming_message.parse_raw_email! @info_request = @incoming_message.info_request if @incoming_message.info_request_id != params[:id].to_i - raise sprintf("Incoming message %d does not belong to request %d", @incoming_message.info_request_id, params[:id]) + raise ActiveRecord::RecordNotFound.new("Incoming message %d does not belong to request %d", @incoming_message.info_request_id, params[:id]) end @part_number = params[:part].to_i @filename = params[:file_name].join("/") diff --git a/app/models/track_thing.rb b/app/models/track_thing.rb index 6938fade9..58d70ed86 100644 --- a/app/models/track_thing.rb +++ b/app/models/track_thing.rb @@ -71,14 +71,13 @@ class TrackThing < ActiveRecord::Base def track_query_description # XXX this is very brittle... we should probably ask users # simply to name their tracks when they make them? - self.track_query = self.track_query.gsub(/([()]|OR)/, "") - filters = self.track_query.scan /\b\S+:\S+\b/ - text = self.track_query + original_text = parsed_text = self.track_query.gsub(/([()]|OR)/, "") + filters = parsed_text.scan /\b\S+:\S+\b/ varieties = Set.new date = "" statuses = Set.new for filter in filters - text = text.sub(filter, "") + parsed_text = parsed_text.sub(filter, "") if filter =~ /variety:user/ varieties << _("users") end @@ -105,7 +104,7 @@ class TrackThing < ActiveRecord::Base end end if filters.empty? - text = self.track_query + parsed_text = original_text end descriptions = [] if varieties.include? _("requests") @@ -116,10 +115,10 @@ class TrackThing < ActiveRecord::Base varieties << _("anything") end descriptions += Array(varieties) - text = text.strip + parsed_text = parsed_text.strip descriptions = descriptions.join(_(" or ")) - if !text.empty? - descriptions += _("{{list_of_things}} matching text '{{search_query}}'", :list_of_things => "", :search_query => text) + if !parsed_text.empty? + descriptions += _("{{list_of_things}} matching text '{{search_query}}'", :list_of_things => "", :search_query => parsed_text) end return descriptions end diff --git a/app/views/public_body/_search_ahead.rhtml b/app/views/public_body/_search_ahead.rhtml index 436471544..7ade89b8e 100644 --- a/app/views/public_body/_search_ahead.rhtml +++ b/app/views/public_body/_search_ahead.rhtml @@ -1,4 +1,4 @@ -<p> +<div> <% if !@xapian_requests.nil? %> <% if @xapian_requests.results.size > 0 %> <h3><%= _('Top search results:') %></h3> @@ -13,8 +13,9 @@ <%= render :partial => 'body_listing_single', :locals => { :public_body => result[:model] } %> <% end %> </div> + <%= will_paginate WillPaginate::Collection.new(@page, @per_page, @xapian_requests.matches_estimated) %> <% end %> -</p> +</div> diff --git a/app/views/request/_search_ahead.rhtml b/app/views/request/_search_ahead.rhtml index d0b19de7d..1e65a5458 100644 --- a/app/views/request/_search_ahead.rhtml +++ b/app/views/request/_search_ahead.rhtml @@ -8,7 +8,7 @@ <% end %> <p> - <a id="body-site-search-link" target="_blank"><%= _("Or search in their website for this information.") %></a> + <a id="body-site-search-link"><%= _("Or search in their website for this information.") %></a> </p> <% end %> </div> diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 1d6802940..2e0e99200 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -2,6 +2,19 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') require 'fakeweb' describe ApplicationController, "when accessing third party services" do + before (:each) do + FakeWeb.clean_registry + end + after (:each) do + FakeWeb.clean_registry + end + it "should succeed if the service responds OK" do + config = MySociety::Config.load_default() + config['GAZE_URL'] = 'http://denmark.com' + FakeWeb.register_uri(:get, %r|denmark.com|, :body => "DK") + country = self.controller.send :country_from_ip + country.should == "DK" + end it "should fail silently if the country_from_ip domain doesn't exist" do config = MySociety::Config.load_default() config['GAZE_URL'] = 'http://12123sdf14qsd.com' @@ -15,7 +28,7 @@ describe ApplicationController, "when accessing third party services" do country.should == config['ISO_COUNTRY_CODE'] end it "should fail silently if the country_from_ip service returns an error" do - FakeWeb.register_uri(:get, %r|.*|, :body => "Error", :status => ["500", "Error"]) + FakeWeb.register_uri(:get, %r|500.com|, :body => "Error", :status => ["500", "Error"]) config = MySociety::Config.load_default() config['GAZE_URL'] = 'http://500.com' country = self.controller.send :country_from_ip diff --git a/spec/controllers/public_body_controller_spec.rb b/spec/controllers/public_body_controller_spec.rb index a563b92ad..72fbe965c 100644 --- a/spec/controllers/public_body_controller_spec.rb +++ b/spec/controllers/public_body_controller_spec.rb @@ -181,6 +181,8 @@ end describe PublicBodyController, "when doing type ahead searches" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things + integrate_views + it "should return nothing for the empty query string" do get :search_typeahead, :query => "" response.should render_template('public_body/_search_ahead') @@ -190,6 +192,7 @@ describe PublicBodyController, "when doing type ahead searches" do it "should return a body matching the given keyword, but not users with a matching description" do get :search_typeahead, :query => "Geraldine" response.should render_template('public_body/_search_ahead') + response.body.should include('search_ahead') assigns[:xapian_requests].results.size.should == 1 assigns[:xapian_requests].results[0][:model].name.should == public_bodies(:geraldine_public_body).name end diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 86665a793..97688b3c0 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -171,6 +171,16 @@ describe RequestController, "when showing one request" do response.should have_text(/Second hello/) end + it "should return 404 for ugly URLs contain a request id that isn't an integer " do + ir = info_requests(:fancy_dog_request) + receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) + ir.reload + ugly_id = "55195" + lambda { + get :get_attachment_as_html, :incoming_message_id => ir.incoming_messages[1].id, :id => ugly_id, :part => 2, :file_name => ['hello.txt.html'], :skip_cache => 1 + }.should raise_error(ActiveRecord::RecordNotFound) + end + it "should generate valid HTML verson of PDF attachments " do ir = info_requests(:fancy_dog_request) receive_incoming_mail('incoming-request-pdf-attachment.email', ir.incoming_email) @@ -1493,6 +1503,8 @@ end describe RequestController, "when doing type ahead searches" do fixtures :public_bodies, :public_body_translations, :public_body_versions, :users, :info_requests, :raw_emails, :incoming_messages, :outgoing_messages, :comments, :info_request_events, :track_things + integrate_views + it "should return nothing for the empty query string" do get :search_typeahead, :q => "" response.should render_template('request/_search_ahead.rhtml') diff --git a/spec/models/track_thing_spec.rb b/spec/models/track_thing_spec.rb index 4922a96c7..7891bd229 100644 --- a/spec/models/track_thing_spec.rb +++ b/spec/models/track_thing_spec.rb @@ -28,6 +28,13 @@ describe TrackThing, "when tracking changes" do found_track.should == @track_thing end + it "can display the description of a deleted track_thing" do + track_thing = TrackThing.create_track_for_search_query('fancy dog') + description = track_thing.track_query_description + track_thing.destroy + track_thing.track_query_description.should == description + end + it "will make some sane descriptions of search-based tracks" do tests = [['bob variety:user', "users matching text 'bob'"], ['bob (variety:sent OR variety:followup_sent OR variety:response OR variety:comment) (latest_status:successful OR latest_status:partially_successful OR latest_status:rejected OR latest_status:not_held)', "requests which are successful or unsuccessful or comments matching text 'bob'"], diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e58c3890a..33a28449e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -143,7 +143,10 @@ if $tempfilecount.nil? module TestProcess # Hook into the process function, so can automatically get HTML after each request alias :original_process :process - + def is_fragment + # XXX there must be a better way of doing this! + return @request.query_parameters["action"] == "search_typeahead" + end def process(action, parameters = nil, session = nil, flash = nil, http_method = 'GET') self.original_process(action, parameters, session, flash, http_method) # don't validate auto-generated HTML @@ -152,7 +155,12 @@ if $tempfilecount.nil? return unless @response.template.controller.instance_eval { integrate_views? } # And then if HTML, not a redirect (302, 301) if @response.content_type == "text/html" && ! [301,302,401].include?(@response.response_code) + if !is_fragment validate_html(@response.body) + else + # it's a partial + validate_as_body(@response.body) + end end end end |