Merge branch 'wdtk' of git.mysociety.org:/data/git/public/alaveteli into wdtk

22 files changed, 278 insertions, 98 deletions

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 6e983a014..268ecc73a 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -422,19 +422,19 @@ class RequestController < ApplicationController
         old_described_state = @info_request.described_state
         @info_request.set_described_state(params[:incoming_message][:described_state])
 
-        # If you're not the *actual* requester owner. e.g. you are playing the
+        # If you're not the *actual* requester. e.g. you are playing the
         # classification game, or you're doing this just because you are an
         # admin user (not because you also own the request).
         if !@info_request.is_actual_owning_user?(authenticated_user)
-            # Log what you did, for classification game score purposes. We
-            # don't log if you were the requester XXX This is presumably so you
-            # don't score for classifying your own requests. Could instead
-            # always log and filter at display time.
-            @info_request.log_event("status_update",
+            # Log the status change by someone other than the requester
+            event = @info_request.log_event("status_update",
                 { :user_id => authenticated_user.id,
                   :old_described_state => old_described_state,
                   :described_state => @info_request.described_state,
                 })
+            # Create a classification event for league tables
+            RequestClassification.create!(:user_id => authenticated_user.id,
+                                          :info_request_event_id => event.id)
 
             # Don't give advice on what to do next, as it isn't their request
             RequestMailer.deliver_old_unclassified_updated(@info_request) if !@info_request.is_external?
diff --git a/app/controllers/request_game_controller.rb b/app/controllers/request_game_controller.rb
index 904c44759..f22652dd1 100644
--- a/app/controllers/request_game_controller.rb
+++ b/app/controllers/request_game_controller.rb
@@ -11,13 +11,12 @@ class RequestGameController < ApplicationController
     def play
         session[:request_game] = Time.now
 
-        old = InfoRequest.find_old_unclassified(:conditions => ["prominence = 'normal'"])
-        @missing = old.size
+        @missing = InfoRequest.count_old_unclassified(:conditions => ["prominence = 'normal'"])
         @total = InfoRequest.count
         @done = @total - @missing
         @percentage = (@done.to_f / @total.to_f * 10000).round / 100.0
 
-        @requests = old.sort_by{ rand }.slice(0..2)
+        @requests = InfoRequest.get_random_old_unclassified(3)
 
         if @missing == 0
             flash[:notice] = _('<p>All done! Thank you very much for your help.</p><p>There are <a href="{{helpus_url}}">more things you can do</a> to help {{site_name}}.</p>',
@@ -25,12 +24,8 @@ class RequestGameController < ApplicationController
                 :site_name => site_name)
         end
 
-        @league_table_28_days = InfoRequestEvent.make_league_table(
-            [ "event_type = 'status_update' and created_at >= ?", Time.now() - 28.days ]
-        )[0..10]
-        @league_table_all_time = InfoRequestEvent.make_league_table(
-            [ "event_type = 'status_update'"]
-        )[0..10]
+        @league_table_28_days = RequestClassification.league_table(10, [ "created_at >= ?", Time.now() - 28.days ])
+        @league_table_all_time = RequestClassification.league_table(10)
         @play_urls = true
     end
 
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index 6f472c290..f2d8929bc 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -223,7 +223,7 @@ class InfoRequest < ActiveRecord::Base
             incoming_message.clear_in_database_caches!
         end
     end
-    
+
     # For debugging
     def InfoRequest.profile_search(query)
         t = Time.now.usec
@@ -939,17 +939,44 @@ public
     # Used to find when event last changed
     def InfoRequest.last_event_time_clause(event_type=nil)
         event_type_clause = ''
-        event_type_clause = " and info_request_events.event_type = '#{event_type}'" if event_type
-        "(select created_at from info_request_events where info_request_events.info_request_id = info_requests.id#{event_type_clause} order by created_at desc limit 1)"
+        event_type_clause = " AND info_request_events.event_type = '#{event_type}'" if event_type
+        "(SELECT created_at
+          FROM info_request_events
+          WHERE info_request_events.info_request_id = info_requests.id
+          #{event_type_clause}
+          ORDER BY created_at desc
+          LIMIT 1)"
     end
 
-    def InfoRequest.find_old_unclassified(extra_params={})
+    def InfoRequest.old_unclassified_params(extra_params, include_last_response_time=false)
         last_response_created_at = last_event_time_clause('response')
         age = extra_params[:age_in_days] ? extra_params[:age_in_days].days : OLD_AGE_IN_DAYS
-        params = {:select => "*, #{last_response_created_at} as last_response_time",
-                  :conditions => ["awaiting_description = ? and #{last_response_created_at} < ? and url_title != 'holding_pen' and user_id is not null",
-                                 true, Time.now() - age],
-                                 :order => "last_response_time"}
+        params = { :conditions => ["awaiting_description = ?
+                                    AND #{last_response_created_at} < ?
+                                    AND url_title != 'holding_pen'
+                                    AND user_id IS NOT NULL",
+                                    true, Time.now() - age] }
+        if include_last_response_time
+            params[:select] = "*, #{last_response_created_at} AS last_response_time"
+            params[:order] = 'last_response_time'
+        end
+        return params
+    end
+
+    def InfoRequest.count_old_unclassified(extra_params={})
+        params = old_unclassified_params(extra_params)
+        count(:all, params)
+    end
+
+    def InfoRequest.get_random_old_unclassified(limit)
+        params = old_unclassified_params({})
+        params[:limit] = limit
+        params[:order] = "random()"
+        find(:all, params)
+    end
+
+    def InfoRequest.find_old_unclassified(extra_params={})
+        params = old_unclassified_params(extra_params, include_last_response_time=true)
         params[:limit] = extra_params[:limit] if extra_params[:limit]
         params[:include] = extra_params[:include] if extra_params[:include]
         if extra_params[:order]
@@ -958,7 +985,7 @@ public
         end
         if extra_params[:conditions]
             condition_string = extra_params[:conditions].shift
-            params[:conditions][0] += " and #{condition_string}"
+            params[:conditions][0] += " AND #{condition_string}"
             params[:conditions] += extra_params[:conditions]
         end
         find(:all, params)
diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb
index a827d19a4..54d2f5ef7 100644
--- a/app/models/info_request_event.rb
+++ b/app/models/info_request_event.rb
@@ -43,7 +43,7 @@ class InfoRequestEvent < ActiveRecord::Base
          'resent',
          'followup_sent',
          'followup_resent',
-         
+
          'edit', # title etc. edited (in admin interface)
          'edit_outgoing', # outgoing message edited (in admin interface)
          'edit_comment', # comment edited (in admin interface)
@@ -53,7 +53,7 @@ class InfoRequestEvent < ActiveRecord::Base
          'move_request', # changed user or public body (in admin interface)
          'hide', # hid a request (in admin interface)
          'manual', # you did something in the db by hand
-         
+
          'response',
          'comment',
          'status_update'
@@ -389,24 +389,6 @@ class InfoRequestEvent < ActiveRecord::Base
         return TMail::Address.parse(prev_addr).address == TMail::Address.parse(curr_addr).address
     end
 
-    # Given a find condition clause, creates a league table of users who made those events.
-    # XXX this isn't very generic yet, it is just used for the categorisation game tables.
-    def InfoRequestEvent.make_league_table(conditions)
-        status_update_events = InfoRequestEvent.find(:all, :conditions => conditions)
-        table = Hash.new { |h,k| h[k] = 0 }
-        for event in status_update_events
-            user_id = event.params[:user_id]
-            table[user_id] += 1
-        end
-        league_table = []
-        for user_id, count in table
-            user = User.find(user_id)
-            league_table.push([user, count])
-        end
-        league_table.sort! { |a,b| b[1] <=> a[1] }
-        return league_table
-    end
-
     def json_for_api(deep, snippet_highlight_proc = nil)
         ret = {
             :id => self.id,
diff --git a/app/models/public_body.rb b/app/models/public_body.rb
index 60ecb2781..fb30da234 100644
--- a/app/models/public_body.rb
+++ b/app/models/public_body.rb
@@ -104,7 +104,7 @@ class PublicBody < ActiveRecord::Base
         locale = self.locale || I18n.locale
         PublicBody.with_locale(locale) do
             found = PublicBody.find(:all,
-                                    :conditions => ["public_body_translations.url_name='#{name}'"],
+                                    :conditions => ["public_body_translations.url_name=?", name],
                                     :joins => :translations,
                                     :readonly => false)
             # If many bodies are found (usually because the url_name is the same across
diff --git a/app/models/request_classification.rb b/app/models/request_classification.rb
new file mode 100644
index 000000000..678b6cd16
--- /dev/null
+++ b/app/models/request_classification.rb
@@ -0,0 +1,16 @@
+class RequestClassification < ActiveRecord::Base
+    belongs_to :user
+
+    # return classification instances representing the top n
+    # users, with a 'cnt' attribute representing the number
+    # of classifications the user has made.
+    def RequestClassification.league_table(size, conditions=[])
+        find(:all, :select => 'user_id, count(*) as cnt',
+                                         :conditions => conditions,
+                                         :group => 'user_id',
+                                         :order => 'cnt desc',
+                                         :limit => size,
+                                         :include => :user)
+    end
+
+end
\ No newline at end of file
diff --git a/app/views/request_game/play.rhtml b/app/views/request_game/play.rhtml
index acf6cce87..eedf19ca2 100644
--- a/app/views/request_game/play.rhtml
+++ b/app/views/request_game/play.rhtml
@@ -7,22 +7,22 @@
     </p>
     <h2>Top recent players</h2>
     <table>
-    <% c = 0; for user, count in @league_table_28_days %>
+    <% c = 0; for classifications in @league_table_28_days %>
         <tr>
             <td> <%= c += 1 %>. <td>
-            <td> <%= user_link(user) %> </td>
-            <td> <%=pluralize(count, 'request').gsub(" ", "&nbsp;")%> </td>
+            <td> <%= user_link(classifications.user) %> </td>
+            <td> <%=pluralize(classifications.cnt, 'request').gsub(" ", "&nbsp;")%> </td>
         </tr>
     <% end %>
     </table>
 
     <h2>All time best players</h2>
     <table>
-    <% c = 0; for user, count in @league_table_all_time %>
+    <% c = 0; for classifications in @league_table_all_time %>
         <tr>
             <td> <%= c += 1 %>. <td>
-            <td> <%= user_link(user) %> </td>
-            <td> <%=pluralize(count, 'request').gsub(" ", "&nbsp;")%> </td>
+            <td> <%= user_link(classifications.user) %> </td>
+            <td> <%= pluralize(classifications.cnt, 'request').gsub(" ", "&nbsp;")%> </td>
         </tr>
     <% end %>
     </table>
diff --git a/config/packages b/config/packages
index d059d2906..3cffcb0f9 100644
--- a/config/packages
+++ b/config/packages
@@ -9,7 +9,7 @@ irb
 wv
 poppler-utils
 pdftk (>> 1.41+dfsg-1) | pdftk (<< 1.41+dfsg-1) # that version has a non-functionining uncompress option
-gs-gpl
+ghostscript
 catdoc
 links
 elinks
diff --git a/db/migrate/20120910153022_create_request_classifications.rb b/db/migrate/20120910153022_create_request_classifications.rb
new file mode 100644
index 000000000..7c6270c9e
--- /dev/null
+++ b/db/migrate/20120910153022_create_request_classifications.rb
@@ -0,0 +1,14 @@
+class CreateRequestClassifications < ActiveRecord::Migration
+  def self.up
+    create_table :request_classifications do |t|
+      t.integer :user_id
+      t.integer :info_request_event_id
+      t.timestamps
+    end
+    add_index :request_classifications, :user_id
+  end
+
+  def self.down
+    drop_table :request_classifications
+  end
+end
diff --git a/db/migrate/20120912111713_add_raw_email_index_to_incoming_messages.rb b/db/migrate/20120912111713_add_raw_email_index_to_incoming_messages.rb
new file mode 100644
index 000000000..14174935e
--- /dev/null
+++ b/db/migrate/20120912111713_add_raw_email_index_to_incoming_messages.rb
@@ -0,0 +1,9 @@
+class AddRawEmailIndexToIncomingMessages < ActiveRecord::Migration
+  def self.up
+      add_index :incoming_messages, :raw_email_id
+  end
+
+  def self.down
+      remove_index :incoming_messages, :raw_email_id
+  end
+end
diff --git a/db/migrate/20120912112036_add_info_request_id_index_to_exim_logs.rb b/db/migrate/20120912112036_add_info_request_id_index_to_exim_logs.rb
new file mode 100644
index 000000000..81e2a7946
--- /dev/null
+++ b/db/migrate/20120912112036_add_info_request_id_index_to_exim_logs.rb
@@ -0,0 +1,9 @@
+class AddInfoRequestIdIndexToEximLogs < ActiveRecord::Migration
+  def self.up
+      add_index :exim_logs, :info_request_id
+  end
+
+  def self.down
+      remove_index :exim_logs, :info_request_id
+  end
+end
diff --git a/db/migrate/20120912112312_add_info_request_id_index_to_incoming_and_outgoing_messages.rb b/db/migrate/20120912112312_add_info_request_id_index_to_incoming_and_outgoing_messages.rb
new file mode 100644
index 000000000..814fa7540
--- /dev/null
+++ b/db/migrate/20120912112312_add_info_request_id_index_to_incoming_and_outgoing_messages.rb
@@ -0,0 +1,11 @@
+class AddInfoRequestIdIndexToIncomingAndOutgoingMessages < ActiveRecord::Migration
+  def self.up
+      add_index :incoming_messages, :info_request_id
+      add_index :outgoing_messages, :info_request_id
+  end
+
+  def self.down
+      remove_index :incoming_messages, :info_request_id
+      remove_index :outgoing_messages, :info_request_id
+  end
+end
diff --git a/db/migrate/20120912112655_add_incoming_message_id_index_to_foi_attachments.rb b/db/migrate/20120912112655_add_incoming_message_id_index_to_foi_attachments.rb
new file mode 100644
index 000000000..be0bf76c3
--- /dev/null
+++ b/db/migrate/20120912112655_add_incoming_message_id_index_to_foi_attachments.rb
@@ -0,0 +1,9 @@
+class AddIncomingMessageIdIndexToFoiAttachments < ActiveRecord::Migration
+  def self.up
+      add_index :foi_attachments, :incoming_message_id
+  end
+
+  def self.down
+      remove_index :foi_attachments, :incoming_message_id
+  end
+end
diff --git a/db/migrate/20120912113004_add_indexes_to_info_request_events.rb b/db/migrate/20120912113004_add_indexes_to_info_request_events.rb
new file mode 100644
index 000000000..b3780322f
--- /dev/null
+++ b/db/migrate/20120912113004_add_indexes_to_info_request_events.rb
@@ -0,0 +1,13 @@
+class AddIndexesToInfoRequestEvents < ActiveRecord::Migration
+  def self.up
+      add_index :info_request_events, :incoming_message_id
+      add_index :info_request_events, :outgoing_message_id
+      add_index :info_request_events, :comment_id
+  end
+
+  def self.down
+      remove_index :info_request_events, :incoming_message_id
+      remove_index :info_request_events, :outgoing_message_id
+      remove_index :info_request_events, :comment_id
+  end
+end
diff --git a/db/migrate/20120912113720_add_public_body_index_to_info_requests.rb b/db/migrate/20120912113720_add_public_body_index_to_info_requests.rb
new file mode 100644
index 000000000..a56cad1f9
--- /dev/null
+++ b/db/migrate/20120912113720_add_public_body_index_to_info_requests.rb
@@ -0,0 +1,9 @@
+class AddPublicBodyIndexToInfoRequests < ActiveRecord::Migration
+  def self.up
+      add_index :info_requests, :public_body_id
+  end
+
+  def self.down
+      remove_index :info_requests, :public_body_id
+  end
+end
diff --git a/db/migrate/20120912114022_add_user_index_to_info_requests.rb b/db/migrate/20120912114022_add_user_index_to_info_requests.rb
new file mode 100644
index 000000000..8be51c0c8
--- /dev/null
+++ b/db/migrate/20120912114022_add_user_index_to_info_requests.rb
@@ -0,0 +1,9 @@
+class AddUserIndexToInfoRequests < ActiveRecord::Migration
+  def self.up
+      add_index :info_requests, :user_id
+  end
+
+  def self.down
+      remove_index :info_requests, :user_id
+  end
+end
diff --git a/lib/tasks/temp.rake b/lib/tasks/temp.rake
index 669cdf989..e50ee2cb8 100644
--- a/lib/tasks/temp.rake
+++ b/lib/tasks/temp.rake
@@ -1,5 +1,14 @@
 namespace :temp do
 
+    desc 'Populate the request_classifications table from info_request_events'
+    task :populate_request_classifications => :environment do
+        InfoRequestEvent.find_each(:conditions => ["event_type = 'status_update'"]) do |classification|
+            RequestClassification.create!(:created_at => classification.created_at,
+                                          :user_id => classification.params[:user_id],
+                                          :info_request_event_id => classification.id)
+        end
+    end
+
     desc "Remove plaintext passwords from post_redirect params"
     task :remove_post_redirect_passwords => :environment do
         PostRedirect.find_each(:conditions => ['post_params_yaml is not null']) do |post_redirect|
diff --git a/lib/tasks/themes.rake b/lib/tasks/themes.rake
index 2b1dbb3a9..6eb64b4b0 100644
--- a/lib/tasks/themes.rake
+++ b/lib/tasks/themes.rake
@@ -9,33 +9,43 @@ namespace :themes do
         File.join(plugin_dir, theme_name)
     end
 
+    def checkout_tag(version)
+        checkout_command = "git checkout #{usage_tag(version)}"
+        success = system(checkout_command)
+        puts "Using tag #{usage_tag(version)}" if verbose && success
+        success
+    end
+
+    def usage_tag(version)
+        "use-with-alaveteli-#{version}"
+    end
+
     def install_theme_using_git(name, uri, verbose=false, options={})
-        mkdir_p(install_path = theme_dir(name))
-        Dir.chdir install_path do
-            init_cmd = "git init"
-            init_cmd += " -q" if options[:quiet] and not verbose
-            puts init_cmd if verbose
-            system(init_cmd)
-            base_cmd = "git pull --depth 1 #{uri}"
-            # Is there a tag for this version of Alaveteli?
-            usage_tag = "use-with-alaveteli-#{ALAVETELI_VERSION}"
-            # Query the remote repository passing flags for tags
-            version_tag = `git ls-remote --tags #{uri} #{usage_tag}`
-            if /^[a-z0-9]+\s+refs\/tags\/#{Regexp.escape(usage_tag)}$/.match(version_tag)
-                # If we got a tag, pull that instead of HEAD
-                puts "Using tag #{usage_tag}" if verbose
-                base_cmd += " refs/tags/#{usage_tag}"
-            else
-                puts "No specific tag for this version: using HEAD" if verbose
-            end
-            base_cmd += " -q" if options[:quiet] and not verbose
-            puts base_cmd if verbose
-            if system(base_cmd)
-                puts "removing: .git .gitignore" if verbose
-                rm_rf %w(.git .gitignore)
+        install_path = theme_dir(name)
+        Dir.chdir(plugin_dir) do
+            clone_command = "git clone #{uri} #{name}"
+            if system(clone_command)
+                Dir.chdir install_path do
+                    # try to checkout a tag exactly matching ALAVETELI VERSION
+                    tag_checked_out = checkout_tag(ALAVETELI_VERSION)
+                    if ! tag_checked_out
+                        # if we're on a hotfix release (four sequence elements or more),
+                        # look for a usage tag matching the minor release (three sequence elements)
+                        # and check that out if found
+                        if hotfix_version = /^(\d+\.\d+\.\d+)(\.\d+)+/.match(ALAVETELI_VERSION)
+                            base_version = hotfix_version[1]
+                            tag_checked_out = checkout_tag(base_version)
+                        end
+                    end
+                    if ! tag_checked_out
+                        puts "No specific tag for this version: using HEAD" if verbose
+                    end
+                    puts "removing: .git .gitignore" if verbose
+                    rm_rf %w(.git .gitignore)
+                end
             else
                 rm_rf install_path
-                raise "#{base_cmd} failed! Stopping."
+                raise "#{clone_command} failed! Stopping."
             end
         end
     end
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index b8425613c..95737a250 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -1275,7 +1275,8 @@ describe RequestController, "when classifying an information request" do
                     expected_params = {:user_id => users(:silly_name_user).id,
                                        :old_described_state => 'waiting_response',
                                        :described_state => 'rejected'}
-                    @dog_request.should_receive(:log_event).with("status_update", expected_params)
+                    event = mock_model(InfoRequestEvent)
+                    @dog_request.should_receive(:log_event).with("status_update", expected_params).and_return(event)
                     post_status('rejected')
                 end
 
@@ -1314,10 +1315,19 @@ describe RequestController, "when classifying an information request" do
             end
 
             it 'should log a status update event' do
+                event = mock_model(InfoRequestEvent)
                 expected_params = {:user_id => @admin_user.id,
                                    :old_described_state => 'waiting_response',
                                    :described_state => 'rejected'}
-                @dog_request.should_receive(:log_event).with("status_update", expected_params)
+                @dog_request.should_receive(:log_event).with("status_update", expected_params).and_return(event)
+                post_status('rejected')
+            end
+
+            it 'should record a classification' do
+                event = mock_model(InfoRequestEvent)
+                @dog_request.stub!(:log_event).with("status_update", anything()).and_return(event)
+                RequestClassification.should_receive(:create!).with(:user_id => @admin_user.id,
+                                                                    :info_request_event_id => event.id)
                 post_status('rejected')
             end
 
diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb
index 1ca97f9d8..e931624c1 100644
--- a/spec/models/info_request_spec.rb
+++ b/spec/models/info_request_spec.rb
@@ -350,20 +350,49 @@ describe InfoRequest do
         end
 
         it 'should add extra conditions if supplied' do
-            InfoRequest.should_receive(:find).with(:all,
-                  {:select=> anything,
-                   :order=> anything,
-                   :conditions=>["awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen' and user_id is not null and prominence != 'backpage'",
-                    true, Time.now - 21.days]})
+            expected_conditions = ["awaiting_description = ?
+                                    AND (SELECT created_at
+                                         FROM info_request_events
+                                         WHERE info_request_events.info_request_id = info_requests.id
+                                         AND info_request_events.event_type = 'response'
+                                         ORDER BY created_at desc LIMIT 1) < ?
+                                    AND url_title != 'holding_pen'
+                                    AND user_id IS NOT NULL
+                                    AND prominence != 'backpage'".split(' ').join(' '),
+                                    true, Time.now - 21.days]
+            # compare conditions ignoring whitespace differences
+            InfoRequest.should_receive(:find) do |all, query_params|
+                query_string = query_params[:conditions][0]
+                query_params[:conditions][0] = query_string.split(' ').join(' ')
+                query_params[:conditions].should == expected_conditions
+            end
             InfoRequest.find_old_unclassified({:conditions => ["prominence != 'backpage'"]})
         end
 
-        it 'should ask the database for requests that are awaiting description, have a last response older than 21 days old, are not the holding pen and are not backpaged' do
-            InfoRequest.should_receive(:find).with(:all,
-                  {:select=>"*, (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) as last_response_time",
-                   :order=>"last_response_time",
-                   :conditions=>["awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen' and user_id is not null",
-                    true, Time.now - 21.days]})
+        it 'should ask the database for requests that are awaiting description, have a last response older
+        than 21 days old, have a user, are not the holding pen and are not backpaged' do
+            expected_conditions = ["awaiting_description = ?
+                                    AND (SELECT created_at
+                                         FROM info_request_events
+                                         WHERE info_request_events.info_request_id = info_requests.id
+                                         AND info_request_events.event_type = 'response'
+                                         ORDER BY created_at desc LIMIT 1) < ?
+                                    AND url_title != 'holding_pen'
+                                    AND user_id IS NOT NULL".split(' ').join(' '),
+                                    true, Time.now - 21.days]
+            expected_select = "*, (SELECT created_at
+                                   FROM info_request_events
+                                   WHERE info_request_events.info_request_id = info_requests.id
+                                   AND info_request_events.event_type = 'response'
+                                   ORDER BY created_at desc LIMIT 1)
+                                   AS last_response_time".split(' ').join(' ')
+            InfoRequest.should_receive(:find) do |all, query_params|
+                query_string = query_params[:conditions][0]
+                query_params[:conditions][0] = query_string.split(' ').join(' ')
+                query_params[:conditions].should == expected_conditions
+                query_params[:select].split(' ').join(' ').should == expected_select
+                query_params[:order].should == "last_response_time"
+            end
             InfoRequest.find_old_unclassified
         end
 
diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb
index 8ff6afde3..9e22a9c43 100644
--- a/spec/models/public_body_spec.rb
+++ b/spec/models/public_body_spec.rb
@@ -222,6 +222,10 @@ describe PublicBody, "when searching" do
             body.name.should == "El A Geraldine Quango"
         end
     end
+
+    it 'should not raise an error on a name with a single quote in it' do
+        body = PublicBody.find_by_url_name_with_historic("belfast city council'")
+    end
 end
 
 describe PublicBody, " when dealing public body locales" do
diff --git a/spec/models/request_mailer_spec.rb b/spec/models/request_mailer_spec.rb
index ea75ec765..98681a9e9 100644
--- a/spec/models/request_mailer_spec.rb
+++ b/spec/models/request_mailer_spec.rb
@@ -29,7 +29,7 @@ describe RequestMailer, " when receiving incoming mail" do
         InfoRequest.holding_pen_request.incoming_messages.size.should == 1
         last_event = InfoRequest.holding_pen_request.incoming_messages[0].info_request.get_last_event
         last_event.params[:rejected_reason].should == "Could not identify the request from the email address"
-        
+
         deliveries = ActionMailer::Base.deliveries
         deliveries.size.should == 1
         mail = deliveries[0]
@@ -49,7 +49,7 @@ describe RequestMailer, " when receiving incoming mail" do
         InfoRequest.holding_pen_request.incoming_messages.size.should == 1
         last_event = InfoRequest.holding_pen_request.incoming_messages[0].info_request.get_last_event
         last_event.params[:rejected_reason].should =~ /there is no "From" address/
-        
+
         deliveries = ActionMailer::Base.deliveries
         deliveries.size.should == 1
         mail = deliveries[0]
@@ -69,7 +69,7 @@ describe RequestMailer, " when receiving incoming mail" do
         InfoRequest.holding_pen_request.incoming_messages.size.should == 1
         last_event = InfoRequest.holding_pen_request.incoming_messages[0].info_request.get_last_event
         last_event.params[:rejected_reason].should =~ /Only the authority can reply/
-        
+
         deliveries = ActionMailer::Base.deliveries
         deliveries.size.should == 1
         mail = deliveries[0]
@@ -222,12 +222,27 @@ describe RequestMailer, "when sending reminders to requesters to classify a resp
         RequestMailer.alert_new_response_reminders_internal(7, 'new_response_reminder_1')
     end
 
-    it 'should ask for all requests that are awaiting description and whose latest response is older than the number of days given and that are not the holding pen' do
-        expected_params = {:conditions => [ "awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen' and user_id is not null",
-                           true, Time.now() - 7.days ],
-                           :include => [ :user ],
-                           :order => "info_requests.id"}
-        InfoRequest.should_receive(:find).with(:all, expected_params).and_return([])
+    it 'should ask for all requests that are awaiting description and whose latest response is older
+        than the number of days given and that are not the holding pen' do
+        expected_conditions = [ "awaiting_description = ?
+                                 AND (SELECT created_at
+                                      FROM info_request_events
+                                      WHERE info_request_events.info_request_id = info_requests.id
+                                      AND info_request_events.event_type = 'response'
+                                      ORDER BY created_at desc LIMIT 1) < ?
+                                 AND url_title != 'holding_pen'
+                                 AND user_id IS NOT NULL".split(' ').join(' '),
+                                 true, Time.now() - 7.days ]
+
+        # compare the query string ignoring any spacing differences
+        InfoRequest.should_receive(:find) do |all, query_params|
+            query_string = query_params[:conditions][0]
+            query_params[:conditions][0] = query_string.split(' ').join(' ')
+            query_params[:conditions].should == expected_conditions
+            query_params[:include].should == [ :user ]
+            query_params[:order].should == 'info_requests.id'
+        end
+
         send_alerts
     end