Move make_query_from_params to XapianQueries

This is involved with the construction of meaningful xapian queries with
respect to InfoRequestEvents. This commit also removes the
get_tags_from_params method, which presumably was targeted at
PublicBodies, but doesn't seem to actually be used anywhere.
XapianQueries is used to extend InfoRequestEvent in order to prevent
InfoRequestEvent becoming too unwieldy and to preserve the association
between these methods.

7 files changed, 93 insertions, 97 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 161a82b26..7c122917d 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -428,100 +428,6 @@ class ApplicationController < ActionController::Base
         end
     end
 
-    def get_request_variety_from_params(params)
-        query = ""
-        sortby = "newest"
-        varieties = []
-        if params[:request_variety] && !(query =~ /variety:/)
-            if params[:request_variety].include? "sent"
-                varieties -= ['variety:sent', 'variety:followup_sent', 'variety:response', 'variety:comment']
-                varieties << ['variety:sent', 'variety:followup_sent']
-            end
-            if params[:request_variety].include? "response"
-                varieties << ['variety:response']
-            end
-            if params[:request_variety].include? "comment"
-                varieties << ['variety:comment']
-            end
-        end
-        if !varieties.empty?
-            query = " (#{varieties.join(' OR ')})"
-        end
-        return query
-    end
-
-    def get_status_from_params(params)
-        query = ""
-        if params[:latest_status]
-            statuses = []
-            if params[:latest_status].class == String
-                params[:latest_status] = [params[:latest_status]]
-            end
-            if params[:latest_status].include?("recent") ||  params[:latest_status].include?("all")
-                query += " (variety:sent OR variety:followup_sent OR variety:response OR variety:comment)"
-            end
-            if params[:latest_status].include? "successful"
-                statuses << ['latest_status:successful', 'latest_status:partially_successful']
-            end
-            if params[:latest_status].include? "unsuccessful"
-                statuses << ['latest_status:rejected', 'latest_status:not_held']
-            end
-            if params[:latest_status].include? "awaiting"
-                statuses << ['latest_status:waiting_response', 'latest_status:waiting_clarification', 'waiting_classification:true', 'latest_status:internal_review','latest_status:gone_postal', 'latest_status:error_message', 'latest_status:requires_admin']
-            end
-            if params[:latest_status].include? "internal_review"
-                statuses << ['status:internal_review']
-            end
-            if params[:latest_status].include? "other"
-                statuses << ['latest_status:gone_postal', 'latest_status:error_message', 'latest_status:requires_admin', 'latest_status:user_withdrawn']
-            end
-            if params[:latest_status].include? "gone_postal"
-                statuses << ['latest_status:gone_postal']
-            end
-            if !statuses.empty?
-                query = " (#{statuses.join(' OR ')})"
-            end
-        end
-        return query
-    end
-
-    def get_date_range_from_params(params)
-        query = ""
-        if params.has_key?(:request_date_after) && !params.has_key?(:request_date_before)
-            params[:request_date_before] = Time.now.strftime("%d/%m/%Y")
-            query += " #{params[:request_date_after]}..#{params[:request_date_before]}"
-        elsif !params.has_key?(:request_date_after) && params.has_key?(:request_date_before)
-            params[:request_date_after] = "01/01/2001"
-        end
-        if params.has_key?(:request_date_after)
-            query = " #{params[:request_date_after]}..#{params[:request_date_before]}"
-        end
-        return query
-    end
-
-    def get_tags_from_params(params)
-        query = ""
-        tags = []
-        if params.has_key?(:tags)
-            params[:tags].split().each do |tag|
-                tags << "tag:#{tag}"
-            end
-        end
-        if !tags.empty?
-            query = " (#{tags.join(' OR ')})"
-        end
-        return query
-    end
-
-    def make_query_from_params(params)
-        query = params[:query] || "" if query.nil?
-        query += get_date_range_from_params(params)
-        query += get_request_variety_from_params(params)
-        query += get_status_from_params(params)
-        query += get_tags_from_params(params)
-        return query
-    end
-
     def country_from_ip
         country = ""
         if !AlaveteliConfiguration::gaze_url.empty?
diff --git a/app/controllers/general_controller.rb b/app/controllers/general_controller.rb
index b01a67027..6f0d29889 100644
--- a/app/controllers/general_controller.rb
+++ b/app/controllers/general_controller.rb
@@ -103,7 +103,7 @@ class GeneralController < ApplicationController
             params[:query] = @query
         end
         if @variety_postfix != "all" && @requests
-            @query, _ = make_query_from_params(params)
+            @query = InfoRequestEvent.make_query_from_params(params)
         end
         @inputted_sortby = @sortby
         if @sortby.nil?
diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb
index 308d38e4c..862f4b318 100644
--- a/app/controllers/public_body_controller.rb
+++ b/app/controllers/public_body_controller.rb
@@ -40,7 +40,7 @@ class PublicBodyController < ApplicationController
                 @searched_to_send_request = true
             end
             @view = params[:view]
-            query = make_query_from_params(params.merge(:latest_status => @view))
+            query = InfoRequestEvent.make_query_from_params(params.merge(:latest_status => @view))
             query += " requested_from:#{@public_body.url_name}"
             # Use search query for this so can collapse and paginate easily
             # XXX really should just use SQL query here rather than Xapian.
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index d982bd391..04d8f06b0 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -151,7 +151,7 @@ class RequestController < ApplicationController
             raise ActiveRecord::RecordNotFound.new("Sorry. No pages after #{MAX_RESULTS / PER_PAGE}.")
         end
 
-        query = make_query_from_params(params.merge(:latest_status => @view))
+        query = InfoRequestEvent.make_query_from_params(params.merge(:latest_status => @view))
         @title = _("View and search requests")
         sortby = "newest"
         xapian_object = perform_search([InfoRequestEvent], query, sortby, 'request_collapse')
diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb
index e268b28ca..5eed5ba83 100644
--- a/app/models/info_request_event.rb
+++ b/app/models/info_request_event.rb
@@ -21,6 +21,9 @@
 # Email: hello@mysociety.org; WWW: http://www.mysociety.org/
 
 class InfoRequestEvent < ActiveRecord::Base
+
+    extend XapianQueries
+
     belongs_to :info_request
     validates_presence_of :info_request
 
@@ -416,4 +419,5 @@ class InfoRequestEvent < ActiveRecord::Base
       yield(column.human_name, self.send(column.name), column.type.to_s, column.name)
     end
   end
+
 end
diff --git a/config/initializers/alaveteli.rb b/config/initializers/alaveteli.rb
index 631251b87..4b6cf8e8c 100644
--- a/config/initializers/alaveteli.rb
+++ b/config/initializers/alaveteli.rb
@@ -52,6 +52,7 @@ require 'alaveteli_localization'
 require 'message_prominence'
 require 'actionmailer_patches'
 require 'theme'
+require 'xapian_queries'
 
 AlaveteliLocalization.set_locales(AlaveteliConfiguration::available_locales,
                                   AlaveteliConfiguration::default_locale)
diff --git a/lib/xapian_queries.rb b/lib/xapian_queries.rb
new file mode 100644
index 000000000..b3599740a
--- /dev/null
+++ b/lib/xapian_queries.rb
@@ -0,0 +1,85 @@
+module XapianQueries
+
+    # These methods take some filter criteria expressed in a hash and convert them
+    # into a xapian query referencing the terms and values stored by InfoRequestEvent.
+    # Note that the params are request params and may contain irrelevant keys
+
+    def get_request_variety_from_params(params)
+        query = ""
+        sortby = "newest"
+        varieties = []
+        if params[:request_variety] && !(query =~ /variety:/)
+            if params[:request_variety].include? "sent"
+                varieties -= ['variety:sent', 'variety:followup_sent', 'variety:response', 'variety:comment']
+                varieties << ['variety:sent', 'variety:followup_sent']
+            end
+            if params[:request_variety].include? "response"
+                varieties << ['variety:response']
+            end
+            if params[:request_variety].include? "comment"
+                varieties << ['variety:comment']
+            end
+        end
+        if !varieties.empty?
+            query = " (#{varieties.join(' OR ')})"
+        end
+        return query
+    end
+
+    def get_status_from_params(params)
+        query = ""
+        if params[:latest_status]
+            statuses = []
+            if params[:latest_status].class == String
+                params[:latest_status] = [params[:latest_status]]
+            end
+            if params[:latest_status].include?("recent") ||  params[:latest_status].include?("all")
+                query += " (variety:sent OR variety:followup_sent OR variety:response OR variety:comment)"
+            end
+            if params[:latest_status].include? "successful"
+                statuses << ['latest_status:successful', 'latest_status:partially_successful']
+            end
+            if params[:latest_status].include? "unsuccessful"
+                statuses << ['latest_status:rejected', 'latest_status:not_held']
+            end
+            if params[:latest_status].include? "awaiting"
+                statuses << ['latest_status:waiting_response', 'latest_status:waiting_clarification', 'waiting_classification:true', 'latest_status:internal_review','latest_status:gone_postal', 'latest_status:error_message', 'latest_status:requires_admin']
+            end
+            if params[:latest_status].include? "internal_review"
+                statuses << ['status:internal_review']
+            end
+            if params[:latest_status].include? "other"
+                statuses << ['latest_status:gone_postal', 'latest_status:error_message', 'latest_status:requires_admin', 'latest_status:user_withdrawn']
+            end
+            if params[:latest_status].include? "gone_postal"
+                statuses << ['latest_status:gone_postal']
+            end
+            if !statuses.empty?
+                query = " (#{statuses.join(' OR ')})"
+            end
+        end
+        return query
+    end
+
+    def get_date_range_from_params(params)
+        query = ""
+        if params.has_key?(:request_date_after) && !params.has_key?(:request_date_before)
+            params[:request_date_before] = Time.now.strftime("%d/%m/%Y")
+            query += " #{params[:request_date_after]}..#{params[:request_date_before]}"
+        elsif !params.has_key?(:request_date_after) && params.has_key?(:request_date_before)
+            params[:request_date_after] = "01/01/2001"
+        end
+        if params.has_key?(:request_date_after)
+            query = " #{params[:request_date_after]}..#{params[:request_date_before]}"
+        end
+        return query
+    end
+
+    def make_query_from_params(params)
+        query = params[:query] || "" if query.nil?
+        query += get_date_range_from_params(params)
+        query += get_request_variety_from_params(params)
+        query += get_status_from_params(params)
+        return query
+    end
+end