Merge branch 'release/0.5'

1 files changed, 61 insertions, 25 deletions

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index dad5e81cd..2295d6718 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -13,6 +13,9 @@ require 'open-uri'
 class RequestController < ApplicationController
     before_filter :check_read_only, :only => [ :new, :show_response, :describe_state, :upload_response ]
     protect_from_forgery :only => [ :new, :show_response, :describe_state, :upload_response ] # See ActionController::RequestForgeryProtection for details
+
+    MAX_RESULTS = 500
+    PER_PAGE = 25
     
     @@custom_states_loaded = false
     begin
@@ -35,11 +38,9 @@ class RequestController < ApplicationController
             # do nothing - as "authenticated?" has done the redirect to signin page for us
             return
         end
-        
         if !params[:query].nil?
-            query = params[:query] + '*'
-            query = query.split(' ').join(' OR ')       # XXX: HACK for OR instead of default AND!
-            @xapian_requests = perform_search([PublicBody], query, 'relevant', nil, 5)
+            query = params[:query]
+            @xapian_requests = perform_search_typeahead(query, PublicBody)
         end
         medium_cache
     end
@@ -73,8 +74,9 @@ class RequestController < ApplicationController
             @info_request_events = @info_request.info_request_events
             @status = @info_request.calculate_status
             @collapse_quotes = params[:unfold] ? false : true
-            @update_status = params[:update_status] ? true : false    
+            @update_status = params[:update_status] ? true : false
             @old_unclassified = @info_request.is_old_unclassified? && !authenticated_user.nil?
+            @is_owning_user = @info_request.is_owning_user?(authenticated_user)
             
             if @update_status
                 return if !@is_owning_user && !authenticated_as_user?(@info_request.user,
@@ -107,7 +109,6 @@ class RequestController < ApplicationController
 
             # For send followup link at bottom
             @last_response = @info_request.get_last_response
-            @is_owning_user = @info_request.is_owning_user?(authenticated_user)
             respond_to do |format|
                 format.html { @has_json = true; render :template => 'request/show'}
                 format.json { render :json => @info_request.json_for_api(true) }
@@ -119,11 +120,14 @@ class RequestController < ApplicationController
     def details
         long_cache
         @info_request = InfoRequest.find_by_url_title(params[:url_title])
-        if !@info_request.user_can_view?(authenticated_user)
-            render :template => 'request/hidden', :status => 410 # gone
-            return
+        if @info_request.nil?
+            raise ActiveRecord::RecordNotFound.new("Request not found")
+        else            
+            if !@info_request.user_can_view?(authenticated_user)
+                render :template => 'request/hidden', :status => 410 # gone
+                return
+            end
         end
-
         @columns = ['id', 'event_type', 'created_at', 'described_state', 'last_described_at', 'calculated_state' ]
     end
 
@@ -150,15 +154,26 @@ class RequestController < ApplicationController
     def list
         medium_cache
         @view = params[:view]
+        @page = get_search_page_from_params if !@page # used in cache case, as perform_search sets @page as side effect
+        if @view == "recent"
+            return redirect_to request_list_all_path(:action => "list", :view => "all", :page => @page), :status => :moved_permanently
+        end
+        
+        # Later pages are very expensive to load
+        if @page > MAX_RESULTS / PER_PAGE
+            raise ActiveRecord::RecordNotFound.new("Sorry. No pages after #{MAX_RESULTS / PER_PAGE}.")
+        end
+
         params[:latest_status] = @view
         query = make_query_from_params
         @title = _("View and search requests")
         sortby = "newest"
-        @page = get_search_page_from_params if !@page # used in cache case, as perform_search sets @page as side effect
-        behavior_cache :tag => [@view, @page] do
+        @cache_tag = Digest::MD5.hexdigest(query + @page.to_s) 
+        behavior_cache :tag => [@cache_tag] do
             xapian_object = perform_search([InfoRequestEvent], query, sortby, 'request_collapse')
             @list_results = xapian_object.results.map { |r| r[:model] }
             @matches_estimated = xapian_object.matches_estimated
+            @show_no_more_than = (@matches_estimated > MAX_RESULTS) ? MAX_RESULTS : @matches_estimated
         end
         
         @title = @title + " (page " + @page.to_s + ")" if (@page > 1)
@@ -192,14 +207,28 @@ class RequestController < ApplicationController
         end
 
         # Banned from making new requests?
+        user_exceeded_limit = false
         if !authenticated_user.nil? && !authenticated_user.can_file_requests?
-            @details = authenticated_user.can_fail_html
-            render :template => 'user/banned'
-            return
+            # If the reason the user cannot make new requests is that they are
+            # rate-limited, it’s possible they composed a request before they
+            # logged in and we want to include the text of the request so they
+            # can squirrel it away for tomorrow, so we detect this later after
+            # we have constructed the InfoRequest.
+            user_exceeded_limit = authenticated_user.exceeded_limit?
+            if !user_exceeded_limit
+                @details = authenticated_user.can_fail_html
+                render :template => 'user/banned'
+                return
+            end
         end
 
         # First time we get to the page, just display it
         if params[:submitted_new_request].nil? || params[:reedit]
+            if user_exceeded_limit
+                render :template => 'user/rate_limited'
+                return
+            end
+
             params[:info_request] = { } if !params[:info_request]
 
             # Read parameters in - first the public body (by URL name or id)
@@ -299,6 +328,11 @@ class RequestController < ApplicationController
             return
         end
 
+        if user_exceeded_limit
+            render :template => 'user/rate_limited'
+            return
+        end
+
         if !authenticated?(
                 :web => _("To send your FOI request"),
                 :email => _("Then your FOI request to {{public_body_name}} will be sent.",:public_body_name=>@info_request.public_body.name),
@@ -602,7 +636,9 @@ class RequestController < ApplicationController
     def authenticate_attachment
         # Test for hidden
         incoming_message = IncomingMessage.find(params[:incoming_message_id])
+        raise ActiveRecord::RecordNotFound.new("Message not found") if incoming_message.nil?
         if !incoming_message.info_request.user_can_view?(authenticated_user)
+            @info_request = incoming_message.info_request # used by view
             render :template => 'request/hidden', :status => 410 # gone
         end
     end
@@ -615,8 +651,8 @@ class RequestController < ApplicationController
         else
             key = params.merge(:only_path => true)
             key_path = foi_fragment_cache_path(key)
-
             if foi_fragment_cache_exists?(key_path)
+                raise PermissionDenied.new("Directory listing not allowed") if File.directory?(key_path)
                 cached = foi_fragment_cache_read(key_path)
                 response.content_type = AlaveteliFileTypes.filename_to_mimetype(params[:file_name].join("/")) || 'application/octet-stream'
                 render_for_text(cached)
@@ -676,10 +712,15 @@ 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])
+            # Note that params[:id] might not be an integer, though
+            # if we’ve got this far then it must begin with an integer
+            # and that integer must be the id number of an actual request.
+            message = "Incoming message %d does not belong to request '%s'" % [@incoming_message.info_request_id, params[:id]]
+            raise ActiveRecord::RecordNotFound.new(message)
         end
         @part_number = params[:part].to_i
         @filename = params[:file_name].join("/")
@@ -756,13 +797,7 @@ class RequestController < ApplicationController
         # Since acts_as_xapian doesn't support the Partial match flag, we work around it
         # by making the last work a wildcard, which is quite the same
         query = params[:q]
-        query = query.split(' ')
-        if query.last.nil? || query.last.strip.length < 3
-            @xapian_requests = nil
-        else
-            query = query.join(' OR ')       # XXX: HACK for OR instead of default AND!
-            @xapian_requests = perform_search([InfoRequestEvent], query, 'relevant', 'request_collapse', 5)
-        end
+        @xapian_requests = perform_search_typeahead(query, InfoRequestEvent)
         render :partial => "request/search_ahead.rhtml"
     end
 
@@ -815,7 +850,8 @@ class RequestController < ApplicationController
                         for message in info_request.incoming_messages                
                             attachments = message.get_attachments_for_display
                             for attachment in attachments
-                                zipfile.get_output_stream(attachment.display_filename) { |f|
+                                filename = "#{attachment.url_part_number}_#{attachment.display_filename}"
+                                zipfile.get_output_stream(filename) { |f|
                                     f.puts(attachment.body)
                                 }
                             end