From 2eadcc3417b11237f9e40dd5eb5804b2fcc44c82 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Tue, 12 Feb 2013 12:53:30 +1100
Subject: Pass parameters to method rather explicitly

---
 app/controllers/request_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index dfa3a4834..184441e06 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -175,7 +175,7 @@ class RequestController < ApplicationController
         end
 
         params[:latest_status] = @view
-        query = make_query_from_params
+        query = make_query_from_params(params)
         @title = _("View and search requests")
         sortby = "newest"
         xapian_object = perform_search([InfoRequestEvent], query, sortby, 'request_collapse')
-- 
cgit v1.2.3


From 87beeb16db6e76bb28cc13e5382a81a8103c5cc8 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Tue, 12 Feb 2013 14:19:04 +1100
Subject: Don't pass latest_status param to locale switching links on view
 requests page

---
 app/controllers/request_controller.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 184441e06..6b8444f90 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -174,8 +174,7 @@ class RequestController < ApplicationController
             raise ActiveRecord::RecordNotFound.new("Sorry. No pages after #{MAX_RESULTS / PER_PAGE}.")
         end
 
-        params[:latest_status] = @view
-        query = make_query_from_params(params)
+        query = 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')
-- 
cgit v1.2.3


From 4b83519e02d5d42708dd1770f965b122a11c4440 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Wed, 13 Feb 2013 16:58:33 +1100
Subject: Rename helper method

---
 app/controllers/request_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index dfa3a4834..0dec1fb72 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -279,7 +279,7 @@ class RequestController < ApplicationController
                 else
                     # if not requestable because defunct or not_apply, redirect to main page
                     # (which doesn't link to the /new/ URL)
-                    redirect_to public_body_url(@info_request.public_body)
+                    redirect_to public_body_path(@info_request.public_body)
                 end
             end
             return
-- 
cgit v1.2.3


From 85223bb2b630c8576352816b47b3c93f5c0b593b Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Wed, 13 Feb 2013 17:05:39 +1100
Subject: Redirects should be done with absolute urls

---
 app/controllers/request_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 0dec1fb72..dfa3a4834 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -279,7 +279,7 @@ class RequestController < ApplicationController
                 else
                     # if not requestable because defunct or not_apply, redirect to main page
                     # (which doesn't link to the /new/ URL)
-                    redirect_to public_body_path(@info_request.public_body)
+                    redirect_to public_body_url(@info_request.public_body)
                 end
             end
             return
-- 
cgit v1.2.3


From ba15c9b78dca182f74281d7354e77942860259e7 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Wed, 13 Feb 2013 19:15:51 +1100
Subject: Rename helper method

---
 app/controllers/request_controller.rb | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index dfa3a4834..0dd54b503 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -57,7 +57,7 @@ class RequestController < ApplicationController
             # Look up by old style numeric identifiers
             if params[:url_title].match(/^[0-9]+$/)
                 @info_request = InfoRequest.find(params[:url_title].to_i)
-                redirect_to request_url(@info_request, :format => params[:format])
+                redirect_to request_path(@info_request, :format => params[:format])
                 return
             end
 
@@ -380,14 +380,14 @@ class RequestController < ApplicationController
 
         # If this isn't a form submit, go to the request page
         if params[:submitted_describe_state].nil?
-            redirect_to request_url(@info_request)
+            redirect_to request_path(@info_request)
             return
         end
 
         # If this is an external request, go to the request page - we don't allow
         # state change from the front end interface.
         if @info_request.is_external?
-            redirect_to request_url(@info_request)
+            redirect_to request_path(@info_request)
             return
         end
 
@@ -408,13 +408,13 @@ class RequestController < ApplicationController
 
         if !params[:incoming_message]
             flash[:error] = _("Please choose whether or not you got some of the information that you wanted.")
-            redirect_to request_url(@info_request)
+            redirect_to request_path(@info_request)
             return
         end
 
         if params[:last_info_request_event_id].to_i != @last_info_request_event_id
             flash[:error] = _("The request has been updated since you originally loaded this page. Please check for any new incoming messages below, and try again.")
-            redirect_to request_url(@info_request)
+            redirect_to request_path(@info_request)
             return
         end
 
@@ -439,11 +439,11 @@ class RequestController < ApplicationController
             # 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?
             if session[:request_game]
-                flash[:notice] = _('Thank you for updating the status of the request \'<a href="{{url}}">{{info_request_title}}</a>\'. There are some more requests below for you to classify.',:info_request_title=>CGI.escapeHTML(@info_request.title), :url=>CGI.escapeHTML(request_url(@info_request)))
+                flash[:notice] = _('Thank you for updating the status of the request \'<a href="{{url}}">{{info_request_title}}</a>\'. There are some more requests below for you to classify.',:info_request_title=>CGI.escapeHTML(@info_request.title), :url=>CGI.escapeHTML(request_path(@info_request)))
                 redirect_to play_url
             else
                 flash[:notice] = _('Thank you for updating this request!')
-                redirect_to request_url(@info_request)
+                redirect_to request_path(@info_request)
             end
             return
         end
@@ -453,10 +453,10 @@ class RequestController < ApplicationController
         if calculated_status == 'waiting_response'
             flash[:notice] = _("<p>Thank you! Hopefully your wait isn't too long.</p> <p>By law, you should get a response promptly, and normally before the end of <strong>
 {{date_response_required_by}}</strong>.</p>",:date_response_required_by=>simple_date(@info_request.date_response_required_by))
-            redirect_to request_url(@info_request)
+            redirect_to request_path(@info_request)
         elsif calculated_status == 'waiting_response_overdue'
             flash[:notice] = _("<p>Thank you! Hope you don't have to wait much longer.</p> <p>By law, you should have got a response promptly, and normally before the end of <strong>{{date_response_required_by}}</strong>.</p>",:date_response_required_by=>simple_date(@info_request.date_response_required_by))
-            redirect_to request_url(@info_request)
+            redirect_to request_path(@info_request)
         elsif calculated_status == 'waiting_response_very_overdue'
             flash[:notice] = _("<p>Thank you! Your request is long overdue, by more than {{very_late_number_of_days}} working days. Most requests should be answered within {{late_number_of_days}} working days. You might like to complain about this, see below.</p>", :very_late_number_of_days => Configuration::reply_very_late_after_days, :late_number_of_days => Configuration::reply_late_after_days)
             redirect_to unhappy_url(@info_request)
@@ -474,13 +474,13 @@ class RequestController < ApplicationController
             :find_authority_url => "/new",
             :complain_url => CGI.escapeHTML(unhappy_url(@info_request)),
             :other_means_url => CGI.escapeHTML(unhappy_url(@info_request)) + "#other_means")
-            redirect_to request_url(@info_request)
+            redirect_to request_path(@info_request)
         elsif calculated_status == 'rejected'
             flash[:notice] = _("Oh no! Sorry to hear that your request was refused. Here is what to do now.")
             redirect_to unhappy_url(@info_request)
         elsif calculated_status == 'successful'
             flash[:notice] = _("<p>We're glad you got all the information that you wanted. If you write about or make use of the information, please come back and add an annotation below saying what you did.</p><p>If you found {{site_name}} useful, <a href=\"{{donation_url}}\">make a donation</a> to the charity which runs it.</p>", :site_name=>site_name, :donation_url => "http://www.mysociety.org/donate/")
-            redirect_to request_url(@info_request)
+            redirect_to request_path(@info_request)
         elsif calculated_status == 'partially_successful'
             flash[:notice] = _("<p>We're glad you got some of the information that you wanted. If you found {{site_name}} useful, <a href=\"{{donation_url}}\">make a donation</a> to the charity which runs it.</p><p>If you want to try and get the rest of the information, here's what to do now.</p>", :site_name=>site_name, :donation_url=>"http://www.mysociety.org/donate/")
             redirect_to unhappy_url(@info_request)
@@ -491,7 +491,7 @@ class RequestController < ApplicationController
             redirect_to respond_to_last_url(@info_request) + "?gone_postal=1"
         elsif calculated_status == 'internal_review'
             flash[:notice] = _("<p>Thank you! Hopefully your wait isn't too long.</p><p>You should get a response within {{late_number_of_days}} days, or be told if it will take longer (<a href=\"{{review_url}}\">details</a>).</p>",:late_number_of_days => Configuration.reply_late_after_days, :review_url => unhappy_url(@info_request) + "#internal_review")
-            redirect_to request_url(@info_request)
+            redirect_to request_path(@info_request)
         elsif calculated_status == 'error_message'
             flash[:notice] = _("<p>Thank you! We'll look into what happened and try and fix it up.</p><p>If the error was a delivery failure, and you can find an up to date FOI email address for the authority, please tell us using the form below.</p>")
             redirect_to help_general_url(:action => 'contact')
@@ -520,7 +520,7 @@ class RequestController < ApplicationController
             redirect_to outgoing_message_url(@info_request_event.outgoing_message), :status => :moved_permanently
         else
             # XXX maybe there are better URLs for some events than this
-            redirect_to request_url(@info_request_event.info_request), :status => :moved_permanently
+            redirect_to request_path(@info_request_event.info_request), :status => :moved_permanently
         end
     end
 
@@ -655,7 +655,7 @@ class RequestController < ApplicationController
                 else
                     flash[:notice] = _("Your follow up message has been sent on its way.")
                 end
-                redirect_to request_url(@info_request)
+                redirect_to request_path(@info_request)
             end
         else
             # render default show_response template
@@ -696,7 +696,7 @@ class RequestController < ApplicationController
         else
             flash[:notice] = _("This request has already been reported for administrator attention")
         end
-        redirect_to request_url(info_request)
+        redirect_to request_path(info_request)
     end
 
     # special caching code so mime types are handled right
@@ -853,7 +853,7 @@ class RequestController < ApplicationController
             mail = RequestMailer.create_fake_response(@info_request, @user, body, file_name, file_content)
             @info_request.receive(mail, mail.encoded, true)
             flash[:notice] = _("Thank you for responding to this FOI request! Your response has been published below, and a link to your response has been emailed to ") + CGI.escapeHTML(@info_request.user.name) + "."
-            redirect_to request_url(@info_request)
+            redirect_to request_path(@info_request)
             return
         end
     end
@@ -895,7 +895,7 @@ class RequestController < ApplicationController
                         convert_command = Configuration::html_to_pdf_command
                         done = false
                         if !convert_command.blank? && File.exists?(convert_command)
-                            url = "http://#{Configuration::domain}#{request_url(@info_request)}?print_stylesheet=1"
+                            url = "http://#{Configuration::domain}#{request_path(@info_request)}?print_stylesheet=1"
                             tempfile = Tempfile.new('foihtml2pdf')
                             output = AlaveteliExternalCommand.run(convert_command, url, tempfile.path)
                             if !output.nil?
-- 
cgit v1.2.3


From 5dce6d1465efa82a33a07706363221b39e8ca029 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Wed, 13 Feb 2013 19:32:50 +1100
Subject: Redirects should be done with absolute urls

---
 app/controllers/request_controller.rb | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 0dd54b503..163c64bc4 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -57,7 +57,7 @@ class RequestController < ApplicationController
             # Look up by old style numeric identifiers
             if params[:url_title].match(/^[0-9]+$/)
                 @info_request = InfoRequest.find(params[:url_title].to_i)
-                redirect_to request_path(@info_request, :format => params[:format])
+                redirect_to request_url(@info_request, :format => params[:format])
                 return
             end
 
@@ -380,14 +380,14 @@ class RequestController < ApplicationController
 
         # If this isn't a form submit, go to the request page
         if params[:submitted_describe_state].nil?
-            redirect_to request_path(@info_request)
+            redirect_to request_url(@info_request)
             return
         end
 
         # If this is an external request, go to the request page - we don't allow
         # state change from the front end interface.
         if @info_request.is_external?
-            redirect_to request_path(@info_request)
+            redirect_to request_url(@info_request)
             return
         end
 
@@ -408,13 +408,13 @@ class RequestController < ApplicationController
 
         if !params[:incoming_message]
             flash[:error] = _("Please choose whether or not you got some of the information that you wanted.")
-            redirect_to request_path(@info_request)
+            redirect_to request_url(@info_request)
             return
         end
 
         if params[:last_info_request_event_id].to_i != @last_info_request_event_id
             flash[:error] = _("The request has been updated since you originally loaded this page. Please check for any new incoming messages below, and try again.")
-            redirect_to request_path(@info_request)
+            redirect_to request_url(@info_request)
             return
         end
 
@@ -443,7 +443,7 @@ class RequestController < ApplicationController
                 redirect_to play_url
             else
                 flash[:notice] = _('Thank you for updating this request!')
-                redirect_to request_path(@info_request)
+                redirect_to request_url(@info_request)
             end
             return
         end
@@ -453,10 +453,10 @@ class RequestController < ApplicationController
         if calculated_status == 'waiting_response'
             flash[:notice] = _("<p>Thank you! Hopefully your wait isn't too long.</p> <p>By law, you should get a response promptly, and normally before the end of <strong>
 {{date_response_required_by}}</strong>.</p>",:date_response_required_by=>simple_date(@info_request.date_response_required_by))
-            redirect_to request_path(@info_request)
+            redirect_to request_url(@info_request)
         elsif calculated_status == 'waiting_response_overdue'
             flash[:notice] = _("<p>Thank you! Hope you don't have to wait much longer.</p> <p>By law, you should have got a response promptly, and normally before the end of <strong>{{date_response_required_by}}</strong>.</p>",:date_response_required_by=>simple_date(@info_request.date_response_required_by))
-            redirect_to request_path(@info_request)
+            redirect_to request_url(@info_request)
         elsif calculated_status == 'waiting_response_very_overdue'
             flash[:notice] = _("<p>Thank you! Your request is long overdue, by more than {{very_late_number_of_days}} working days. Most requests should be answered within {{late_number_of_days}} working days. You might like to complain about this, see below.</p>", :very_late_number_of_days => Configuration::reply_very_late_after_days, :late_number_of_days => Configuration::reply_late_after_days)
             redirect_to unhappy_url(@info_request)
@@ -474,13 +474,13 @@ class RequestController < ApplicationController
             :find_authority_url => "/new",
             :complain_url => CGI.escapeHTML(unhappy_url(@info_request)),
             :other_means_url => CGI.escapeHTML(unhappy_url(@info_request)) + "#other_means")
-            redirect_to request_path(@info_request)
+            redirect_to request_url(@info_request)
         elsif calculated_status == 'rejected'
             flash[:notice] = _("Oh no! Sorry to hear that your request was refused. Here is what to do now.")
             redirect_to unhappy_url(@info_request)
         elsif calculated_status == 'successful'
             flash[:notice] = _("<p>We're glad you got all the information that you wanted. If you write about or make use of the information, please come back and add an annotation below saying what you did.</p><p>If you found {{site_name}} useful, <a href=\"{{donation_url}}\">make a donation</a> to the charity which runs it.</p>", :site_name=>site_name, :donation_url => "http://www.mysociety.org/donate/")
-            redirect_to request_path(@info_request)
+            redirect_to request_url(@info_request)
         elsif calculated_status == 'partially_successful'
             flash[:notice] = _("<p>We're glad you got some of the information that you wanted. If you found {{site_name}} useful, <a href=\"{{donation_url}}\">make a donation</a> to the charity which runs it.</p><p>If you want to try and get the rest of the information, here's what to do now.</p>", :site_name=>site_name, :donation_url=>"http://www.mysociety.org/donate/")
             redirect_to unhappy_url(@info_request)
@@ -491,7 +491,7 @@ class RequestController < ApplicationController
             redirect_to respond_to_last_url(@info_request) + "?gone_postal=1"
         elsif calculated_status == 'internal_review'
             flash[:notice] = _("<p>Thank you! Hopefully your wait isn't too long.</p><p>You should get a response within {{late_number_of_days}} days, or be told if it will take longer (<a href=\"{{review_url}}\">details</a>).</p>",:late_number_of_days => Configuration.reply_late_after_days, :review_url => unhappy_url(@info_request) + "#internal_review")
-            redirect_to request_path(@info_request)
+            redirect_to request_url(@info_request)
         elsif calculated_status == 'error_message'
             flash[:notice] = _("<p>Thank you! We'll look into what happened and try and fix it up.</p><p>If the error was a delivery failure, and you can find an up to date FOI email address for the authority, please tell us using the form below.</p>")
             redirect_to help_general_url(:action => 'contact')
@@ -520,7 +520,7 @@ class RequestController < ApplicationController
             redirect_to outgoing_message_url(@info_request_event.outgoing_message), :status => :moved_permanently
         else
             # XXX maybe there are better URLs for some events than this
-            redirect_to request_path(@info_request_event.info_request), :status => :moved_permanently
+            redirect_to request_url(@info_request_event.info_request), :status => :moved_permanently
         end
     end
 
@@ -655,7 +655,7 @@ class RequestController < ApplicationController
                 else
                     flash[:notice] = _("Your follow up message has been sent on its way.")
                 end
-                redirect_to request_path(@info_request)
+                redirect_to request_url(@info_request)
             end
         else
             # render default show_response template
@@ -696,7 +696,7 @@ class RequestController < ApplicationController
         else
             flash[:notice] = _("This request has already been reported for administrator attention")
         end
-        redirect_to request_path(info_request)
+        redirect_to request_url(info_request)
     end
 
     # special caching code so mime types are handled right
@@ -853,7 +853,7 @@ class RequestController < ApplicationController
             mail = RequestMailer.create_fake_response(@info_request, @user, body, file_name, file_content)
             @info_request.receive(mail, mail.encoded, true)
             flash[:notice] = _("Thank you for responding to this FOI request! Your response has been published below, and a link to your response has been emailed to ") + CGI.escapeHTML(@info_request.user.name) + "."
-            redirect_to request_path(@info_request)
+            redirect_to request_url(@info_request)
             return
         end
     end
-- 
cgit v1.2.3


From 64fe099163e8b786e6c05fe05fe9cbb8c068e380 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Thu, 14 Feb 2013 04:32:11 +1100
Subject: Rename named routes for categorisation game as they clash with
 request_url & request_path helpers

---
 app/controllers/request_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 163c64bc4..f8e865523 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -440,7 +440,7 @@ class RequestController < ApplicationController
             RequestMailer.deliver_old_unclassified_updated(@info_request) if !@info_request.is_external?
             if session[:request_game]
                 flash[:notice] = _('Thank you for updating the status of the request \'<a href="{{url}}">{{info_request_title}}</a>\'. There are some more requests below for you to classify.',:info_request_title=>CGI.escapeHTML(@info_request.title), :url=>CGI.escapeHTML(request_path(@info_request)))
-                redirect_to play_url
+                redirect_to categorise_play_url
             else
                 flash[:notice] = _('Thank you for updating this request!')
                 redirect_to request_url(@info_request)
-- 
cgit v1.2.3


From 4dd61c8235bb685b1d2b3041c4ab2cf912c04006 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Fri, 15 Feb 2013 14:50:02 +1100
Subject: Use absolute urls when redirecting

---
 app/controllers/request_controller.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index f8e865523..3e18acd82 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -166,7 +166,7 @@ class RequestController < ApplicationController
         @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
+            return redirect_to request_list_all_url(:action => "list", :view => "all", :page => @page), :status => :moved_permanently
         end
 
         # Later pages are very expensive to load
@@ -370,7 +370,7 @@ class RequestController < ApplicationController
             <p>If you write about this request (for example in a forum or a blog) please link to this page, and add an
             annotation below telling people about your writing.</p>",:law_used_full=>@info_request.law_used_full,
             :late_number_of_days => Configuration::reply_late_after_days)
-        redirect_to show_new_request_path(:url_title => @info_request.url_title)
+        redirect_to show_new_request_url(:url_title => @info_request.url_title)
     end
 
     # Submitted to the describing state of messages form
-- 
cgit v1.2.3


From fa624fc7f17b6b4b6710817ed63291386e525f9c Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Sat, 2 Mar 2013 14:27:20 +1100
Subject: Use routes to only allow post to RequestController#describe_state

---
 app/controllers/request_controller.rb | 6 ------
 1 file changed, 6 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 6b8444f90..80df9b04d 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -377,12 +377,6 @@ class RequestController < ApplicationController
         @info_request = InfoRequest.find(params[:id].to_i)
         set_last_request(@info_request)
 
-        # If this isn't a form submit, go to the request page
-        if params[:submitted_describe_state].nil?
-            redirect_to request_url(@info_request)
-            return
-        end
-
         # If this is an external request, go to the request page - we don't allow
         # state change from the front end interface.
         if @info_request.is_external?
-- 
cgit v1.2.3


From 1f39f9f562d33c03bdeedbb19c5582585f242e6f Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Wed, 27 Feb 2013 16:06:03 +1100
Subject: Refactor state logic

---
 app/controllers/request_controller.rb | 71 ++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 35 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 80df9b04d..23e01be54 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -441,20 +441,17 @@ class RequestController < ApplicationController
             return
         end
 
-        calculated_status = @info_request.calculate_status
         # Display advice for requester on what to do next, as appropriate
-        if calculated_status == 'waiting_response'
-            flash[:notice] = _("<p>Thank you! Hopefully your wait isn't too long.</p> <p>By law, you should get a response promptly, and normally before the end of <strong>
+        flash[:notice] = case @info_request.calculate_status
+        when 'waiting_response'
+            _("<p>Thank you! Hopefully your wait isn't too long.</p> <p>By law, you should get a response promptly, and normally before the end of <strong>
 {{date_response_required_by}}</strong>.</p>",:date_response_required_by=>simple_date(@info_request.date_response_required_by))
-            redirect_to request_url(@info_request)
-        elsif calculated_status == 'waiting_response_overdue'
-            flash[:notice] = _("<p>Thank you! Hope you don't have to wait much longer.</p> <p>By law, you should have got a response promptly, and normally before the end of <strong>{{date_response_required_by}}</strong>.</p>",:date_response_required_by=>simple_date(@info_request.date_response_required_by))
-            redirect_to request_url(@info_request)
-        elsif calculated_status == 'waiting_response_very_overdue'
-            flash[:notice] = _("<p>Thank you! Your request is long overdue, by more than {{very_late_number_of_days}} working days. Most requests should be answered within {{late_number_of_days}} working days. You might like to complain about this, see below.</p>", :very_late_number_of_days => Configuration::reply_very_late_after_days, :late_number_of_days => Configuration::reply_late_after_days)
-            redirect_to unhappy_url(@info_request)
-        elsif calculated_status == 'not_held'
-            flash[:notice] = _("<p>Thank you! Here are some ideas on what to do next:</p>
+        when 'waiting_response_overdue'
+            _("<p>Thank you! Hope you don't have to wait much longer.</p> <p>By law, you should have got a response promptly, and normally before the end of <strong>{{date_response_required_by}}</strong>.</p>",:date_response_required_by=>simple_date(@info_request.date_response_required_by))
+        when 'waiting_response_very_overdue'
+            _("<p>Thank you! Your request is long overdue, by more than {{very_late_number_of_days}} working days. Most requests should be answered within {{late_number_of_days}} working days. You might like to complain about this, see below.</p>", :very_late_number_of_days => Configuration::reply_very_late_after_days, :late_number_of_days => Configuration::reply_late_after_days)
+        when 'not_held'
+            _("<p>Thank you! Here are some ideas on what to do next:</p>
             <ul>
             <li>To send your request to another authority, first copy the text of your request below, then <a href=\"{{find_authority_url}}\">find the other authority</a>.</li>
             <li>If you would like to contest the authority's claim that they do not hold the information, here is
@@ -467,38 +464,42 @@ class RequestController < ApplicationController
             :find_authority_url => "/new",
             :complain_url => CGI.escapeHTML(unhappy_url(@info_request)),
             :other_means_url => CGI.escapeHTML(unhappy_url(@info_request)) + "#other_means")
+        when 'rejected'
+            _("Oh no! Sorry to hear that your request was refused. Here is what to do now.")
+        when 'successful'
+            _("<p>We're glad you got all the information that you wanted. If you write about or make use of the information, please come back and add an annotation below saying what you did.</p><p>If you found {{site_name}} useful, <a href=\"{{donation_url}}\">make a donation</a> to the charity which runs it.</p>", :site_name=>site_name, :donation_url => "http://www.mysociety.org/donate/")
+        when 'partially_successful'
+            _("<p>We're glad you got some of the information that you wanted. If you found {{site_name}} useful, <a href=\"{{donation_url}}\">make a donation</a> to the charity which runs it.</p><p>If you want to try and get the rest of the information, here's what to do now.</p>", :site_name=>site_name, :donation_url=>"http://www.mysociety.org/donate/")
+        when 'waiting_clarification'
+            _("Please write your follow up message containing the necessary clarifications below.")
+        when 'gone_postal'
+            nil
+        when 'internal_review'
+            _("<p>Thank you! Hopefully your wait isn't too long.</p><p>You should get a response within {{late_number_of_days}} days, or be told if it will take longer (<a href=\"{{review_url}}\">details</a>).</p>",:late_number_of_days => Configuration.reply_late_after_days, :review_url => unhappy_url(@info_request) + "#internal_review")
+        when 'error_message'
+            _("<p>Thank you! We'll look into what happened and try and fix it up.</p><p>If the error was a delivery failure, and you can find an up to date FOI email address for the authority, please tell us using the form below.</p>")
+        when 'requires_admin'
+            _("Please use the form below to tell us more.")
+        when 'user_withdrawn'
+            _("If you have not done so already, please write a message below telling the authority that you have withdrawn your request. Otherwise they will not know it has been withdrawn.")
+        end
+
+        case @info_request.calculate_status
+        when 'waiting_response', 'waiting_response_overdue', 'not_held', 'successful', 'internal_review'
             redirect_to request_url(@info_request)
-        elsif calculated_status == 'rejected'
-            flash[:notice] = _("Oh no! Sorry to hear that your request was refused. Here is what to do now.")
-            redirect_to unhappy_url(@info_request)
-        elsif calculated_status == 'successful'
-            flash[:notice] = _("<p>We're glad you got all the information that you wanted. If you write about or make use of the information, please come back and add an annotation below saying what you did.</p><p>If you found {{site_name}} useful, <a href=\"{{donation_url}}\">make a donation</a> to the charity which runs it.</p>", :site_name=>site_name, :donation_url => "http://www.mysociety.org/donate/")
-            redirect_to request_url(@info_request)
-        elsif calculated_status == 'partially_successful'
-            flash[:notice] = _("<p>We're glad you got some of the information that you wanted. If you found {{site_name}} useful, <a href=\"{{donation_url}}\">make a donation</a> to the charity which runs it.</p><p>If you want to try and get the rest of the information, here's what to do now.</p>", :site_name=>site_name, :donation_url=>"http://www.mysociety.org/donate/")
+        when 'waiting_response_very_overdue', 'rejected', 'partially_successful'
             redirect_to unhappy_url(@info_request)
-        elsif calculated_status == 'waiting_clarification'
-            flash[:notice] = _("Please write your follow up message containing the necessary clarifications below.")
+        when 'waiting_clarification', 'user_withdrawn'
             redirect_to respond_to_last_url(@info_request)
-        elsif calculated_status == 'gone_postal'
+        when 'gone_postal'
             redirect_to respond_to_last_url(@info_request) + "?gone_postal=1"
-        elsif calculated_status == 'internal_review'
-            flash[:notice] = _("<p>Thank you! Hopefully your wait isn't too long.</p><p>You should get a response within {{late_number_of_days}} days, or be told if it will take longer (<a href=\"{{review_url}}\">details</a>).</p>",:late_number_of_days => Configuration.reply_late_after_days, :review_url => unhappy_url(@info_request) + "#internal_review")
-            redirect_to request_url(@info_request)
-        elsif calculated_status == 'error_message'
-            flash[:notice] = _("<p>Thank you! We'll look into what happened and try and fix it up.</p><p>If the error was a delivery failure, and you can find an up to date FOI email address for the authority, please tell us using the form below.</p>")
+        when 'error_message', 'requires_admin'
             redirect_to help_general_url(:action => 'contact')
-        elsif calculated_status == 'requires_admin'
-            flash[:notice] = _("Please use the form below to tell us more.")
-            redirect_to help_general_url(:action => 'contact')
-        elsif calculated_status == 'user_withdrawn'
-            flash[:notice] = _("If you have not done so already, please write a message below telling the authority that you have withdrawn your request. Otherwise they will not know it has been withdrawn.")
-            redirect_to respond_to_last_url(@info_request)
         else
             if @@custom_states_loaded
                 return self.theme_describe_state(@info_request)
             else
-                raise "unknown calculate_status " + calculated_status
+                raise "unknown calculate_status #{@info_request.calculate_status}"
             end
         end
     end
-- 
cgit v1.2.3


From d88b79d1aa24a78ed95255a37a0403d7e3c0117e Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Fri, 1 Mar 2013 13:04:02 +1100
Subject: Inline method

---
 app/controllers/request_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 23e01be54..3d9559c8b 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -877,7 +877,7 @@ class RequestController < ApplicationController
                               :email_subject => _("Log in to download a zip file of {{info_request_title}}",
                                            :info_request_title=>@info_request.title)
                               )
-                updated = Digest::SHA1.hexdigest(@info_request.get_last_event.created_at.to_i.to_s + @info_request.updated_at.to_i.to_s)
+                updated = Digest::SHA1.hexdigest(@info_request.info_request_events.last.created_at.to_i.to_s + @info_request.updated_at.to_i.to_s)
                 @url_path = File.join("/download",
                                        request_dirs(@info_request),
                                        updated,
-- 
cgit v1.2.3


From 43230ddb09284bce0ef8635f805acc2a1190809a Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Thu, 28 Feb 2013 16:04:30 +1100
Subject: New request controller action for requires_admin state with message

---
 app/controllers/request_controller.rb | 5 +++++
 1 file changed, 5 insertions(+)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 3d9559c8b..c382a55f1 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -504,6 +504,11 @@ class RequestController < ApplicationController
         end
     end
 
+    def describe_state_requires_admin
+        @info_request = InfoRequest.find(params[:id])
+        @info_request.set_described_state("requires_admin", nil, params[:message])
+    end
+
     # Used for links from polymorphic URLs e.g. in Atom feeds - just redirect to
     # proper URL for the message the event refers to
     def show_request_event
-- 
cgit v1.2.3


From ab3e3e7decda222ded7e6d83e0440bd5660a4bf0 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Thu, 28 Feb 2013 17:07:33 +1100
Subject: Add authentication

---
 app/controllers/request_controller.rb | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index c382a55f1..24dbbeba0 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -9,8 +9,8 @@ require 'zip/zip'
 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
+    before_filter :check_read_only, :only => [ :new, :show_response, :describe_state, :describe_state_requires_admin, :upload_response ]
+    protect_from_forgery :only => [ :new, :show_response, :describe_state, :describe_state_requires_admin, :upload_response ] # See ActionController::RequestForgeryProtection for details
 
     MAX_RESULTS = 500
     PER_PAGE = 25
@@ -506,6 +506,19 @@ class RequestController < ApplicationController
 
     def describe_state_requires_admin
         @info_request = InfoRequest.find(params[:id])
+
+        # Check authenticated. We check is_owning_user
+        # to get admin overrides (see is_owning_user? above)
+        if !@info_request.is_owning_user?(authenticated_user) &&
+            !authenticated_as_user?(@info_request.user,
+                :web => _("To classify the response to this FOI request"),
+                :email => _("Then you can classify the FOI response you have got from ") + @info_request.public_body.name + ".",
+                :email_subject => _("Classify an FOI response from ") + @info_request.public_body.name
+            )
+            # do nothing - as "authenticated?" has done the redirect to signin page for us
+            return
+        end
+
         @info_request.set_described_state("requires_admin", nil, params[:message])
     end
 
-- 
cgit v1.2.3


From 260bd5bf263683919bac17ce8dfebd153a45e8aa Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Sat, 2 Mar 2013 15:18:25 +1100
Subject: Switch to pretty urls

---
 app/controllers/request_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 24dbbeba0..749cf2d54 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -505,7 +505,7 @@ class RequestController < ApplicationController
     end
 
     def describe_state_requires_admin
-        @info_request = InfoRequest.find(params[:id])
+        @info_request = InfoRequest.find_by_url_title!(params[:url_title])
 
         # Check authenticated. We check is_owning_user
         # to get admin overrides (see is_owning_user? above)
-- 
cgit v1.2.3


From 00c188d129696353c48a2598ec7c747b8a90d658 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Sun, 3 Mar 2013 09:40:21 +1100
Subject: Add basic authentication to new action

---
 app/controllers/request_controller.rb | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 749cf2d54..74a310712 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -507,15 +507,10 @@ class RequestController < ApplicationController
     def describe_state_requires_admin
         @info_request = InfoRequest.find_by_url_title!(params[:url_title])
 
-        # Check authenticated. We check is_owning_user
-        # to get admin overrides (see is_owning_user? above)
-        if !@info_request.is_owning_user?(authenticated_user) &&
-            !authenticated_as_user?(@info_request.user,
-                :web => _("To classify the response to this FOI request"),
-                :email => _("Then you can classify the FOI response you have got from ") + @info_request.public_body.name + ".",
-                :email_subject => _("Classify an FOI response from ") + @info_request.public_body.name
-            )
-            # do nothing - as "authenticated?" has done the redirect to signin page for us
+        if !authenticated?(
+            :web => _("To classify the response to this FOI request"),
+            :email => _("Then you can classify the FOI response you have got from ") + @info_request.public_body.name + ".",
+            :email_subject => _("Classify an FOI response from ") + @info_request.public_body.name)
             return
         end
 
-- 
cgit v1.2.3


From faa70e9445a0a31fe0a49217ff2135b31ccce4ac Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Sun, 3 Mar 2013 10:12:49 +1100
Subject: only can make the change as the owner of a request

---
 app/controllers/request_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 74a310712..1698635e8 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -507,7 +507,7 @@ class RequestController < ApplicationController
     def describe_state_requires_admin
         @info_request = InfoRequest.find_by_url_title!(params[:url_title])
 
-        if !authenticated?(
+        if !authenticated_as_user?(@info_request.user,
             :web => _("To classify the response to this FOI request"),
             :email => _("Then you can classify the FOI response you have got from ") + @info_request.public_body.name + ".",
             :email_subject => _("Classify an FOI response from ") + @info_request.public_body.name)
-- 
cgit v1.2.3


From d189203172c931d255c685cecd3a10d6f8555fbc Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Sun, 3 Mar 2013 11:05:53 +1100
Subject: admin user can change status

---
 app/controllers/request_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 1698635e8..bba614851 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -507,7 +507,7 @@ class RequestController < ApplicationController
     def describe_state_requires_admin
         @info_request = InfoRequest.find_by_url_title!(params[:url_title])
 
-        if !authenticated_as_user?(@info_request.user,
+        unless @info_request.is_owning_user?(authenticated_user) || authenticated_as_user?(@info_request.user,
             :web => _("To classify the response to this FOI request"),
             :email => _("Then you can classify the FOI response you have got from ") + @info_request.public_body.name + ".",
             :email_subject => _("Classify an FOI response from ") + @info_request.public_body.name)
-- 
cgit v1.2.3


From d785bf7e0ad702116706672d964e228612d0b797 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Sun, 3 Mar 2013 11:56:46 +1100
Subject: Can update status when request is old and unclassified

---
 app/controllers/request_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index bba614851..659537e80 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -507,7 +507,7 @@ class RequestController < ApplicationController
     def describe_state_requires_admin
         @info_request = InfoRequest.find_by_url_title!(params[:url_title])
 
-        unless @info_request.is_owning_user?(authenticated_user) || authenticated_as_user?(@info_request.user,
+        unless (authenticated_user && @info_request.is_old_unclassified?) || @info_request.is_owning_user?(authenticated_user) || authenticated_as_user?(@info_request.user,
             :web => _("To classify the response to this FOI request"),
             :email => _("Then you can classify the FOI response you have got from ") + @info_request.public_body.name + ".",
             :email_subject => _("Classify an FOI response from ") + @info_request.public_body.name)
-- 
cgit v1.2.3


From f8c895f8eb93b2e818485085f232fe30a179805e Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Sun, 3 Mar 2013 12:37:08 +1100
Subject: Extract method that knows who can update the state of a request

---
 app/controllers/request_controller.rb | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 659537e80..984689ce1 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -507,10 +507,12 @@ class RequestController < ApplicationController
     def describe_state_requires_admin
         @info_request = InfoRequest.find_by_url_title!(params[:url_title])
 
-        unless (authenticated_user && @info_request.is_old_unclassified?) || @info_request.is_owning_user?(authenticated_user) || authenticated_as_user?(@info_request.user,
-            :web => _("To classify the response to this FOI request"),
-            :email => _("Then you can classify the FOI response you have got from ") + @info_request.public_body.name + ".",
-            :email_subject => _("Classify an FOI response from ") + @info_request.public_body.name)
+        unless Ability::can_update_request_state?(authenticated_user, @info_request)
+            # If we got here this is always going to be false
+            authenticated_as_user?(@info_request.user,
+                :web => _("To classify the response to this FOI request"),
+                :email => _("Then you can classify the FOI response you have got from ") + @info_request.public_body.name + ".",
+                :email_subject => _("Classify an FOI response from ") + @info_request.public_body.name)
             return
         end
 
-- 
cgit v1.2.3


From 2e9b6d80aee6bfe15149566c9c9a0a62fd1ab39e Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Mon, 4 Mar 2013 08:31:10 +1100
Subject: No need for instance variables in action without view

---
 app/controllers/request_controller.rb | 68 +++++++++++++++++------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 984689ce1..9a7b6d1c8 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -374,26 +374,26 @@ class RequestController < ApplicationController
 
     # Submitted to the describing state of messages form
     def describe_state
-        @info_request = InfoRequest.find(params[:id].to_i)
-        set_last_request(@info_request)
+        info_request = InfoRequest.find(params[:id].to_i)
+        set_last_request(info_request)
 
         # If this is an external request, go to the request page - we don't allow
         # state change from the front end interface.
-        if @info_request.is_external?
-            redirect_to request_url(@info_request)
+        if info_request.is_external?
+            redirect_to request_url(info_request)
             return
         end
 
-        @is_owning_user = @info_request.is_owning_user?(authenticated_user)
-        @last_info_request_event_id = @info_request.last_event_id_needing_description
-        @old_unclassified = @info_request.is_old_unclassified? && !authenticated_user.nil?
+        is_owning_user = info_request.is_owning_user?(authenticated_user)
+        last_info_request_event_id = info_request.last_event_id_needing_description
+        old_unclassified = info_request.is_old_unclassified? && !authenticated_user.nil?
 
         # Check authenticated, and parameters set. We check is_owning_user
         # to get admin overrides (see is_owning_user? above)
-        if !@old_unclassified && !@is_owning_user && !authenticated_as_user?(@info_request.user,
+        if !old_unclassified && !is_owning_user && !authenticated_as_user?(info_request.user,
                 :web => _("To classify the response to this FOI request"),
-                :email => _("Then you can classify the FOI response you have got from ") + @info_request.public_body.name + ".",
-                :email_subject => _("Classify an FOI response from ") + @info_request.public_body.name
+                :email => _("Then you can classify the FOI response you have got from ") + info_request.public_body.name + ".",
+                :email_subject => _("Classify an FOI response from ") + info_request.public_body.name
             )
             # do nothing - as "authenticated?" has done the redirect to signin page for us
             return
@@ -401,53 +401,53 @@ class RequestController < ApplicationController
 
         if !params[:incoming_message]
             flash[:error] = _("Please choose whether or not you got some of the information that you wanted.")
-            redirect_to request_url(@info_request)
+            redirect_to request_url(info_request)
             return
         end
 
-        if params[:last_info_request_event_id].to_i != @last_info_request_event_id
+        if params[:last_info_request_event_id].to_i != last_info_request_event_id
             flash[:error] = _("The request has been updated since you originally loaded this page. Please check for any new incoming messages below, and try again.")
-            redirect_to request_url(@info_request)
+            redirect_to request_url(info_request)
             return
         end
 
         # Make the state change
-        old_described_state = @info_request.described_state
-        @info_request.set_described_state(params[:incoming_message][:described_state])
+        old_described_state = info_request.described_state
+        info_request.set_described_state(params[:incoming_message][:described_state])
 
         # 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)
+        if !info_request.is_actual_owning_user?(authenticated_user)
             # Log the status change by someone other than the requester
-            event = @info_request.log_event("status_update",
+            event = info_request.log_event("status_update",
                 { :user_id => authenticated_user.id,
                   :old_described_state => old_described_state,
-                  :described_state => @info_request.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?
+            RequestMailer.deliver_old_unclassified_updated(info_request) if !info_request.is_external?
             if session[:request_game]
-                flash[:notice] = _('Thank you for updating the status of the request \'<a href="{{url}}">{{info_request_title}}</a>\'. There are some more requests below for you to classify.',:info_request_title=>CGI.escapeHTML(@info_request.title), :url=>CGI.escapeHTML(request_url(@info_request)))
+                flash[:notice] = _('Thank you for updating the status of the request \'<a href="{{url}}">{{info_request_title}}</a>\'. There are some more requests below for you to classify.',:info_request_title=>CGI.escapeHTML(info_request.title), :url=>CGI.escapeHTML(request_url(info_request)))
                 redirect_to play_url
             else
                 flash[:notice] = _('Thank you for updating this request!')
-                redirect_to request_url(@info_request)
+                redirect_to request_url(info_request)
             end
             return
         end
 
         # Display advice for requester on what to do next, as appropriate
-        flash[:notice] = case @info_request.calculate_status
+        flash[:notice] = case info_request.calculate_status
         when 'waiting_response'
             _("<p>Thank you! Hopefully your wait isn't too long.</p> <p>By law, you should get a response promptly, and normally before the end of <strong>
-{{date_response_required_by}}</strong>.</p>",:date_response_required_by=>simple_date(@info_request.date_response_required_by))
+{{date_response_required_by}}</strong>.</p>",:date_response_required_by=>simple_date(info_request.date_response_required_by))
         when 'waiting_response_overdue'
-            _("<p>Thank you! Hope you don't have to wait much longer.</p> <p>By law, you should have got a response promptly, and normally before the end of <strong>{{date_response_required_by}}</strong>.</p>",:date_response_required_by=>simple_date(@info_request.date_response_required_by))
+            _("<p>Thank you! Hope you don't have to wait much longer.</p> <p>By law, you should have got a response promptly, and normally before the end of <strong>{{date_response_required_by}}</strong>.</p>",:date_response_required_by=>simple_date(info_request.date_response_required_by))
         when 'waiting_response_very_overdue'
             _("<p>Thank you! Your request is long overdue, by more than {{very_late_number_of_days}} working days. Most requests should be answered within {{late_number_of_days}} working days. You might like to complain about this, see below.</p>", :very_late_number_of_days => Configuration::reply_very_late_after_days, :late_number_of_days => Configuration::reply_late_after_days)
         when 'not_held'
@@ -462,8 +462,8 @@ class RequestController < ApplicationController
             </li>
             </ul>",
             :find_authority_url => "/new",
-            :complain_url => CGI.escapeHTML(unhappy_url(@info_request)),
-            :other_means_url => CGI.escapeHTML(unhappy_url(@info_request)) + "#other_means")
+            :complain_url => CGI.escapeHTML(unhappy_url(info_request)),
+            :other_means_url => CGI.escapeHTML(unhappy_url(info_request)) + "#other_means")
         when 'rejected'
             _("Oh no! Sorry to hear that your request was refused. Here is what to do now.")
         when 'successful'
@@ -475,7 +475,7 @@ class RequestController < ApplicationController
         when 'gone_postal'
             nil
         when 'internal_review'
-            _("<p>Thank you! Hopefully your wait isn't too long.</p><p>You should get a response within {{late_number_of_days}} days, or be told if it will take longer (<a href=\"{{review_url}}\">details</a>).</p>",:late_number_of_days => Configuration.reply_late_after_days, :review_url => unhappy_url(@info_request) + "#internal_review")
+            _("<p>Thank you! Hopefully your wait isn't too long.</p><p>You should get a response within {{late_number_of_days}} days, or be told if it will take longer (<a href=\"{{review_url}}\">details</a>).</p>",:late_number_of_days => Configuration.reply_late_after_days, :review_url => unhappy_url(info_request) + "#internal_review")
         when 'error_message'
             _("<p>Thank you! We'll look into what happened and try and fix it up.</p><p>If the error was a delivery failure, and you can find an up to date FOI email address for the authority, please tell us using the form below.</p>")
         when 'requires_admin'
@@ -484,22 +484,22 @@ class RequestController < ApplicationController
             _("If you have not done so already, please write a message below telling the authority that you have withdrawn your request. Otherwise they will not know it has been withdrawn.")
         end
 
-        case @info_request.calculate_status
+        case info_request.calculate_status
         when 'waiting_response', 'waiting_response_overdue', 'not_held', 'successful', 'internal_review'
-            redirect_to request_url(@info_request)
+            redirect_to request_url(info_request)
         when 'waiting_response_very_overdue', 'rejected', 'partially_successful'
-            redirect_to unhappy_url(@info_request)
+            redirect_to unhappy_url(info_request)
         when 'waiting_clarification', 'user_withdrawn'
-            redirect_to respond_to_last_url(@info_request)
+            redirect_to respond_to_last_url(info_request)
         when 'gone_postal'
-            redirect_to respond_to_last_url(@info_request) + "?gone_postal=1"
+            redirect_to respond_to_last_url(info_request) + "?gone_postal=1"
         when 'error_message', 'requires_admin'
             redirect_to help_general_url(:action => 'contact')
         else
             if @@custom_states_loaded
-                return self.theme_describe_state(@info_request)
+                return self.theme_describe_state(info_request)
             else
-                raise "unknown calculate_status #{@info_request.calculate_status}"
+                raise "unknown calculate_status #{info_request.calculate_status}"
             end
         end
     end
-- 
cgit v1.2.3


From ab8f4379a6b43cd5578a6a7d6268cb904e69b2ff Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Mon, 4 Mar 2013 08:54:57 +1100
Subject: Inline temporary variables

---
 app/controllers/request_controller.rb | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 9a7b6d1c8..f958a4746 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -384,13 +384,9 @@ class RequestController < ApplicationController
             return
         end
 
-        is_owning_user = info_request.is_owning_user?(authenticated_user)
-        last_info_request_event_id = info_request.last_event_id_needing_description
-        old_unclassified = info_request.is_old_unclassified? && !authenticated_user.nil?
-
         # Check authenticated, and parameters set. We check is_owning_user
         # to get admin overrides (see is_owning_user? above)
-        if !old_unclassified && !is_owning_user && !authenticated_as_user?(info_request.user,
+        if !(authenticated_user && info_request.is_old_unclassified?) && !info_request.is_owning_user?(authenticated_user) && !authenticated_as_user?(info_request.user,
                 :web => _("To classify the response to this FOI request"),
                 :email => _("Then you can classify the FOI response you have got from ") + info_request.public_body.name + ".",
                 :email_subject => _("Classify an FOI response from ") + info_request.public_body.name
@@ -405,7 +401,7 @@ class RequestController < ApplicationController
             return
         end
 
-        if params[:last_info_request_event_id].to_i != last_info_request_event_id
+        if params[:last_info_request_event_id].to_i != info_request.last_event_id_needing_description
             flash[:error] = _("The request has been updated since you originally loaded this page. Please check for any new incoming messages below, and try again.")
             redirect_to request_url(info_request)
             return
-- 
cgit v1.2.3


From d16f5b29e106d6f5f8191b7f881386f281f98691 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Mon, 4 Mar 2013 09:06:04 +1100
Subject: Use Ability::can_update_request_state? in
 RequestController#describe_state

---
 app/controllers/request_controller.rb | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index f958a4746..36ec7ee13 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -384,13 +384,12 @@ class RequestController < ApplicationController
             return
         end
 
-        # Check authenticated, and parameters set. We check is_owning_user
-        # to get admin overrides (see is_owning_user? above)
-        if !(authenticated_user && info_request.is_old_unclassified?) && !info_request.is_owning_user?(authenticated_user) && !authenticated_as_user?(info_request.user,
+        # Check authenticated, and parameters set.
+        unless Ability::can_update_request_state?(authenticated_user, info_request)
+            authenticated_as_user?(info_request.user,
                 :web => _("To classify the response to this FOI request"),
                 :email => _("Then you can classify the FOI response you have got from ") + info_request.public_body.name + ".",
-                :email_subject => _("Classify an FOI response from ") + info_request.public_body.name
-            )
+                :email_subject => _("Classify an FOI response from ") + info_request.public_body.name)
             # do nothing - as "authenticated?" has done the redirect to signin page for us
             return
         end
-- 
cgit v1.2.3


From 28e65f615c24b96225d24f3f1e0c03f229f8fe4e Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Mon, 4 Mar 2013 09:38:07 +1100
Subject: Fix bug when user not owner of request changing state to
 requires_admin email should come from user making the change

---
 app/controllers/request_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 36ec7ee13..2cabb3cf0 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -408,7 +408,7 @@ class RequestController < ApplicationController
 
         # Make the state change
         old_described_state = info_request.described_state
-        info_request.set_described_state(params[:incoming_message][:described_state])
+        info_request.set_described_state(params[:incoming_message][:described_state], authenticated_user)
 
         # 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
-- 
cgit v1.2.3


From f60ada47d4e7aabe0dce152109cb0d91865929da Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Mon, 4 Mar 2013 14:16:37 +1100
Subject: Refactor functionality from controller to model

---
 app/controllers/request_controller.rb | 12 ------------
 1 file changed, 12 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 2cabb3cf0..fb78eaac0 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -407,25 +407,13 @@ class RequestController < ApplicationController
         end
 
         # Make the state change
-        old_described_state = info_request.described_state
         info_request.set_described_state(params[:incoming_message][:described_state], authenticated_user)
 
         # 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 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?
             if session[:request_game]
                 flash[:notice] = _('Thank you for updating the status of the request \'<a href="{{url}}">{{info_request_title}}</a>\'. There are some more requests below for you to classify.',:info_request_title=>CGI.escapeHTML(info_request.title), :url=>CGI.escapeHTML(request_url(info_request)))
                 redirect_to play_url
-- 
cgit v1.2.3


From 65deec2c6a9388d87a66e40a7b3a38adf16af6a4 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Mon, 4 Mar 2013 15:57:59 +1100
Subject: Record who changes the state

---
 app/controllers/request_controller.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index fb78eaac0..f36381c51 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -499,7 +499,7 @@ class RequestController < ApplicationController
             return
         end
 
-        @info_request.set_described_state("requires_admin", nil, params[:message])
+        @info_request.set_described_state("requires_admin", authenticated_user, params[:message])
     end
 
     # Used for links from polymorphic URLs e.g. in Atom feeds - just redirect to
-- 
cgit v1.2.3


From 25aad2807e04e2f0bc5dc339140915d6ca8ef3c7 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Mon, 4 Mar 2013 16:10:23 +1100
Subject: Don't allow external requests to have their state changed

---
 app/controllers/request_controller.rb | 7 +++++++
 1 file changed, 7 insertions(+)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index f36381c51..8f5eac85c 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -490,6 +490,13 @@ class RequestController < ApplicationController
     def describe_state_requires_admin
         @info_request = InfoRequest.find_by_url_title!(params[:url_title])
 
+        # If this is an external request, go to the request page - we don't allow
+        # state change from the front end interface.
+        if @info_request.is_external?
+            redirect_to request_url(@info_request)
+            return
+        end
+
         unless Ability::can_update_request_state?(authenticated_user, @info_request)
             # If we got here this is always going to be false
             authenticated_as_user?(@info_request.user,
-- 
cgit v1.2.3


From 78f2cb7a5d5ffabaf186e70e6fa02e60cbd724bc Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Tue, 5 Mar 2013 16:13:28 +1100
Subject: Remove code added previously because on the wrong track

---
 app/controllers/request_controller.rb | 22 ----------------------
 1 file changed, 22 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 8f5eac85c..f2b9150e0 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -487,28 +487,6 @@ class RequestController < ApplicationController
         end
     end
 
-    def describe_state_requires_admin
-        @info_request = InfoRequest.find_by_url_title!(params[:url_title])
-
-        # If this is an external request, go to the request page - we don't allow
-        # state change from the front end interface.
-        if @info_request.is_external?
-            redirect_to request_url(@info_request)
-            return
-        end
-
-        unless Ability::can_update_request_state?(authenticated_user, @info_request)
-            # If we got here this is always going to be false
-            authenticated_as_user?(@info_request.user,
-                :web => _("To classify the response to this FOI request"),
-                :email => _("Then you can classify the FOI response you have got from ") + @info_request.public_body.name + ".",
-                :email_subject => _("Classify an FOI response from ") + @info_request.public_body.name)
-            return
-        end
-
-        @info_request.set_described_state("requires_admin", authenticated_user, params[:message])
-    end
-
     # Used for links from polymorphic URLs e.g. in Atom feeds - just redirect to
     # proper URL for the message the event refers to
     def show_request_event
-- 
cgit v1.2.3


From 760ad672373427f139eaa37701d607bcd338cb7d Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Tue, 5 Mar 2013 17:29:32 +1100
Subject: Can also record a message when posting to
 RequestController#describe_state

---
 app/controllers/request_controller.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index f2b9150e0..c5788d131 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -407,7 +407,8 @@ class RequestController < ApplicationController
         end
 
         # Make the state change
-        info_request.set_described_state(params[:incoming_message][:described_state], authenticated_user)
+        info_request.set_described_state(params[:incoming_message][:described_state], authenticated_user,
+            params[:incoming_message][:message])
 
         # 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
-- 
cgit v1.2.3


From 32a4d1295ce9772b5a482be7afbb5fa29c2b2ba1 Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Wed, 6 Mar 2013 13:48:29 +1100
Subject: Now direct changes of state to error_message and requires_admin to a
 new page asking for more info

---
 app/controllers/request_controller.rb | 39 +++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 9 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index c5788d131..dbfb12fe1 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -406,9 +406,18 @@ class RequestController < ApplicationController
             return
         end
 
+        described_state = params[:incoming_message][:described_state]
+        message = params[:incoming_message][:message]
+        # For requires_admin and error_message states we ask for an extra message to send to
+        # the administrators.
+        # If this message hasn't been included then ask for it
+        if ["error_message", "requires_admin"].include?(described_state) && message.nil?
+            redirect_to describe_state_message_url(:url_title => info_request.url_title, :described_state => described_state)
+            return
+        end
+
         # Make the state change
-        info_request.set_described_state(params[:incoming_message][:described_state], authenticated_user,
-            params[:incoming_message][:message])
+        info_request.set_described_state(described_state, authenticated_user, message)
 
         # 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
@@ -460,16 +469,15 @@ class RequestController < ApplicationController
             nil
         when 'internal_review'
             _("<p>Thank you! Hopefully your wait isn't too long.</p><p>You should get a response within {{late_number_of_days}} days, or be told if it will take longer (<a href=\"{{review_url}}\">details</a>).</p>",:late_number_of_days => Configuration.reply_late_after_days, :review_url => unhappy_url(info_request) + "#internal_review")
-        when 'error_message'
-            _("<p>Thank you! We'll look into what happened and try and fix it up.</p><p>If the error was a delivery failure, and you can find an up to date FOI email address for the authority, please tell us using the form below.</p>")
-        when 'requires_admin'
-            _("Please use the form below to tell us more.")
+        when 'error_message', 'requires_admin'
+            _("Thank you! We'll look into what happened and try and fix it up.")
         when 'user_withdrawn'
             _("If you have not done so already, please write a message below telling the authority that you have withdrawn your request. Otherwise they will not know it has been withdrawn.")
         end
 
         case info_request.calculate_status
-        when 'waiting_response', 'waiting_response_overdue', 'not_held', 'successful', 'internal_review'
+        when 'waiting_response', 'waiting_response_overdue', 'not_held', 'successful',
+            'internal_review', 'error_message', 'requires_admin'
             redirect_to request_url(info_request)
         when 'waiting_response_very_overdue', 'rejected', 'partially_successful'
             redirect_to unhappy_url(info_request)
@@ -477,8 +485,6 @@ class RequestController < ApplicationController
             redirect_to respond_to_last_url(info_request)
         when 'gone_postal'
             redirect_to respond_to_last_url(info_request) + "?gone_postal=1"
-        when 'error_message', 'requires_admin'
-            redirect_to help_general_url(:action => 'contact')
         else
             if @@custom_states_loaded
                 return self.theme_describe_state(info_request)
@@ -488,6 +494,21 @@ class RequestController < ApplicationController
         end
     end
 
+    # Collect a message to include with the change of state
+    def describe_state_message
+        @info_request = InfoRequest.find_by_url_title!(params[:url_title])
+        @described_state = params[:described_state]
+        @last_info_request_event_id = @info_request.last_event_id_needing_description
+        @title = case @described_state
+        when "error_message"
+            _("I've received an error message")
+        when "requires_admin"
+            _("This request requires administrator attention")
+        else 
+            raise "Unsupported state"
+        end
+    end
+
     # Used for links from polymorphic URLs e.g. in Atom feeds - just redirect to
     # proper URL for the message the event refers to
     def show_request_event
-- 
cgit v1.2.3


From 20ec9b49f3407223b7cbcb25a636ba44a4b6967c Mon Sep 17 00:00:00 2001
From: Matthew Landauer <matthew@openaustralia.org>
Date: Wed, 6 Mar 2013 13:51:32 +1100
Subject: Remove deleted action from list of actions

---
 app/controllers/request_controller.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'app/controllers/request_controller.rb')

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index dbfb12fe1..aa646197f 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -9,8 +9,8 @@ require 'zip/zip'
 require 'open-uri'
 
 class RequestController < ApplicationController
-    before_filter :check_read_only, :only => [ :new, :show_response, :describe_state, :describe_state_requires_admin, :upload_response ]
-    protect_from_forgery :only => [ :new, :show_response, :describe_state, :describe_state_requires_admin, :upload_response ] # See ActionController::RequestForgeryProtection for details
+    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
-- 
cgit v1.2.3