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