Merge branch 'release/0.5' into wdtk

Conflicts:
	app/views/public_body/_search_ahead.rhtml

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 = "55195"
+            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