Lots more internal review stuff.

12 files changed, 195 insertions, 122 deletions

diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb
index 5fc039b3e..3cce00d1e 100644
--- a/app/controllers/help_controller.rb
+++ b/app/controllers/help_controller.rb
@@ -4,13 +4,20 @@
 # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: help_controller.rb,v 1.9 2008-10-07 22:05:06 francis Exp $
+# $Id: help_controller.rb,v 1.10 2008-11-07 00:01:49 francis Exp $
 
 class HelpController < ApplicationController
     
     def about
     end
 
+    def unhappy
+        @info_request = nil
+        if params[:url_title]
+            @info_request = InfoRequest.find_by_url_title(params[:url_title])
+        end
+    end
+
     def contact
         @contact_email = MySociety::Config.get("CONTACT_EMAIL", 'contact@localhost')
         @contact_email = @contact_email.gsub(/@/, "&#64;")
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 86cdb65b0..c0d788fc0 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_controller.rb
@@ -4,7 +4,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: request_controller.rb,v 1.134 2008-11-06 03:42:03 francis Exp $
+# $Id: request_controller.rb,v 1.135 2008-11-07 00:01:49 francis Exp $
 
 class RequestController < ApplicationController
     
@@ -92,6 +92,11 @@ class RequestController < ApplicationController
 
     # Page new form posts to
     def new
+        # All new requests are of normal_sort
+        if !params[:outgoing_message].nil?
+            params[:outgoing_message][:what_doing] = 'normal_sort'
+        end
+
         # If we've just got here (so no writing to lose), and we're already
         # logged in, force the user to describe any undescribed requests. Allow
         # margin of 1 undescribed so it isn't too annoying - the function
@@ -99,6 +104,10 @@ class RequestController < ApplicationController
         # arrived.
         if !@user.nil? && params[:submitted_new_request].nil?
             @undescribed_requests = @user.get_undescribed_requests 
+            if params[:public_body_id].nil?
+                redirect_to frontpage_url
+                return
+            end
             @public_body = PublicBody.find(params[:public_body_id])
             if @undescribed_requests.size > 1
                 render :action => 'new_please_describe'
@@ -264,24 +273,21 @@ class RequestController < ApplicationController
             # XXX offer fancier option to duplicate request?
             redirect_to request_url(@info_request)
         elsif @info_request.calculate_status == 'rejected'
-            # XXX explain how to complain
             flash[:notice] = "Oh no! Sorry to hear that your request was rejected. Here is what to do now."
-            redirect_to unhappy_url
+            redirect_to unhappy_url(@info_request)
         elsif @info_request.calculate_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 WhatDoTheyKnow useful, <a href=\"http://www.mysociety.org/donate/\">make a donation</a> to the charity which runs it.</p>"
-            # XXX quiz them here for a comment
             redirect_to request_url(@info_request)
         elsif @info_request.calculate_status == 'partially_successful'
-            flash[:notice] = "<p>We're glad you got some of the information that you wanted. We have details on what to do if you are <a href=\"/help/unhappy\">unhappy about the response you got</a>.</p><p>If you found WhatDoTheyKnow useful, <a href=\"http://www.mysociety.org/donate/\">make a donation</a> to the charity which runs it.</p>"
-            # XXX explain how to complain / quiz them for a comment
-            redirect_to request_url(@info_request)
+            flash[:notice] = "<p>We're glad you got some of the information that you wanted. If you found WhatDoTheyKnow useful, <a href=\"http://www.mysociety.org/donate/\">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>"
+            redirect_to unhappy_url(@info_request)
         elsif @info_request.calculate_status == 'waiting_clarification'
             flash[:notice] = "Please write your follow up message containing the necessary clarifications below."
             redirect_to show_response_url(:id => @info_request.id, :incoming_message_id => @events_needing_description[-1].params[:incoming_message_id])
         elsif @info_request.calculate_status == 'gone_postal'
             redirect_to respond_to_last_url(@info_request) + "?gone_postal=1"
         elsif @info_request.calculate_status == 'internal_review'
-            flash[:notice] = 'XXX message to give people entering internal review state to be done'
+            flash[:notice] = "Thank you! Hopefully your wait isn't too long."
             redirect_to request_url(@info_request)
         elsif @info_request.calculate_status == 'requires_admin'
             flash[:notice] = "Please use the form below if you would like to tell us what is unusual about the response."
@@ -344,6 +350,11 @@ class RequestController < ApplicationController
             :message_type => 'followup',
             :incoming_message_followup => @incoming_message
         })
+        @internal_review = false
+        if !params[:internal_review].nil?
+            params_outgoing_message[:what_doing] = 'internal_review'
+            @internal_review = true
+        end
         @outgoing_message = OutgoingMessage.new(params_outgoing_message)
         @outgoing_message.set_signature_name(@user.name) if !@user.nil?
 
@@ -491,6 +502,5 @@ class RequestController < ApplicationController
             return
         end
     end
-
 end
 
diff --git a/app/helpers/link_to_helper.rb b/app/helpers/link_to_helper.rb
index f3e705196..c036dfc92 100644
--- a/app/helpers/link_to_helper.rb
+++ b/app/helpers/link_to_helper.rb
@@ -5,7 +5,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: link_to_helper.rb,v 1.46 2008-10-20 10:20:57 francis Exp $
+# $Id: link_to_helper.rb,v 1.47 2008-11-07 00:01:50 francis Exp $
 
 module LinkToHelper
 
@@ -155,8 +155,12 @@ module LinkToHelper
     def about_url
         return help_general_url(:action => 'about')
     end
-    def unhappy_url
-        return help_general_url(:action => 'unhappy')
+    def unhappy_url(info_request = nil)
+        if info_request.nil?
+            return help_general_url(:action => 'unhappy')
+        else
+            return help_general_url(:action => 'unhappy', :url_title => info_request.url_title)
+        end
     end
 
 
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index 76a818c6e..ff00235ed 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -23,7 +23,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: info_request.rb,v 1.152 2008-11-06 14:53:07 francis Exp $
+# $Id: info_request.rb,v 1.153 2008-11-07 00:01:50 francis Exp $
 
 require 'digest/sha1'
 require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian')
@@ -491,6 +491,10 @@ public
         return response_required_by
     end
 
+    def days_overdue
+        return (Time.now - self.date_response_required_by) / 3600 / 24
+    end
+
     # Where the initial request is sent to
     def recipient_email
         return self.public_body.request_email
diff --git a/app/views/help/unhappy.rhtml b/app/views/help/unhappy.rhtml
index cfb5659aa..34ecc2929 100644
--- a/app/views/help/unhappy.rhtml
+++ b/app/views/help/unhappy.rhtml
@@ -1,6 +1,13 @@
 <% @title = "Unhappy about a Freedom of Information request?" %>
 
+
+<% if !@info_request.nil? %>
+<h1>Unhappy about the response you got 
+to your request '<%=request_link(@info_request) %>'?
+</h1>
+<% else %>
 <h1>Unhappy about the response you got?</h1>
+<% end %>
 
 <p>If ...</p>
 
@@ -20,10 +27,15 @@
 
 <h1 id="internal_review">1. Asking for an internal review</h1>
 
-<p>You can use WhatDoTheyKnow to request an internal review. Go to your
-request and choose "send follow up" message, then write a message asking
-for an internal review of your request. You may want to include a link to the request
-page, to make it clear which request you are talking about.
+<p>
+<% if !@info_request.nil? %>
+    Choose <%= link_to "request an internal review", show_response_no_followup_url(:id => @info_request.id, :incoming_message_id => nil) + "?internal_review=1#followup" %> and then write a message asking the authority to review your request.
+<% else %>
+    Go to your request on WhatDoTheyKnow and choose "request an internal review"
+    at the bottom of the page. Then write a message asking for an internal
+    review of your request. You may want to include a link to the request page,
+    to make it clear which request you are talking about.
+<% end %>
 </p>
 
 <p>The internal review should take 2-3 weeks for simple cases, and up to 6
@@ -32,52 +44,55 @@ you originally requested, or you will be told that the review upholds the
 original decision.
 </p>
 
-<!-- XXX need decent link to more detailed instructions -->
-
 <h1 id="complaining">2. Complaining to the Information Commissioner</h1>
 
-<!-- XXX should include brief instructions inline, and link to more detail -->
-
 <p>If you are still unhappy after the public authority has done their internal review,
 then you can complain to the Information Commisioner. To do this read
 <a href="http://www.ico.gov.uk/complaints/freedom_of_information.aspx">Complaints about Freedom of Information</a>
-on the Information Commisioner's website.
-
-<p>Again, you can include a link to your request on WhatDoTheyKnow, or print
-out the whole page of your request, to make it easy to send the relevant
-information to the Information Commissioner.
+on the Information Commisioner's website.</p>
+
+<p>To make it easier to send the relevant information to the 
+Information Commisioner, either
+<% if !@info_request.nil? %>
+    include a link to your request 
+    <strong><%=h main_url(request_url(@info_request)) %></strong>
+<% else %>
+    include a link to your request on WhatDoTheyKnow
+<% end %>
+in your complaint or print out the whole page of your request.
+</p>
 
-<p>There is a backlog of work at the Information Commissioner, and
+<p>A warning. There is a backlog of work at the Information Commissioner, and
 it can take literally years to get resolution from them. If you reach this point,
 you should accept that you won't get the information quickly by this means. Maybe
 you want to help the fight to improve Freedom of Information, or maybe
 getting the information slowly is still worthwhile. You can also try and
-get the information by other means...</p>
+get the information by <strong>other means...</strong></p>
 
 <h1 id="other_means">3. Using other means to answer your question</h1>
 
 <p>You can try persuing your problem or your research in other ways.
 
 <ul>
-<li>Make an FOI request for summary information, or for documentation relating
-indirectly to matters in your rejected request.  
+<li>Make a <strong>new FOI request</strong> for summary information, or for
+documentation relating indirectly to matters in your rejected request.  
 <a href="/help/contact">Ask us for ideas</a> if you're stuck.</li>
-<li>If any other public authorities or publicly owned companies are involved,
+<li>If any <strong>other public authorities</strong> or publicly owned companies are involved,
 then make FOI requests to them.</li>
-<li><a href="http://www.writetothem.com">Write to your MP or other politician</a>
-and ask for their help. Often MPs can find out information which individuals can't,
-by directly writing to ministers or departments, or by asking a written question in the
-House of Commons. Likewise, councillors in local authorities can go and talk directly
-to officers to try and find things out.</li>
-<li>Ask others researching the same or similar questions for ideas. Perhaps you
-can find them by browsing this site; you can contact any registered user from
-their page. Perhaps there is an Internet forum or group where there are people
-who know about the area you are researching.  If it is a local matter, use <a
+<li>Write to <strong>your MP</strong> or other politician using <a
+href="http://www.writetothem.com">WriteToThem</a> and ask for their help
+finding the answer. MPs can write directly to ministers or departments, and
+can ask written questions in the House of Commons. Councillors in local authorities
+can talk directly to council officers.</li>
+<li>Ask <strong>other researchers</strong> who are interested in a similar 
+issue to yours for ideas. You can sometimes find them by browsing this site;
+contact any registered user from their page. There may be an Internet
+forum or group that they hang out in. If it is a local matter, use <a
 href="http://www.groupsnearyou.com">GroupsNearYou</a> to find such a 
 forum.</li>
-<li><a href="http://www.pledgebank.com">Start a pledge</a> to get others 
-to act together with you. For example, you could arrange a meeting with staff
-from the authority. Or you could form a small local campaigns group.
+<li><strong>Start a pledge</strong> on <a href="http://www.pledgebank.com">PledgeBank</a> to get 
+others to act together with you. For example, you could arrange a meeting with
+staff from the authority. Or you could form a small local campaigns group.
 </ul>
 
 
diff --git a/app/views/request/_describe_state.rhtml b/app/views/request/_describe_state.rhtml
index 6ed2fd39a..ed22a37b1 100644
--- a/app/views/request/_describe_state.rhtml
+++ b/app/views/request/_describe_state.rhtml
@@ -1,20 +1,40 @@
 <% if @is_owning_user || @requires_admin_describe %>
     <% form_for(:incoming_message, @info_request, :url => describe_state_url(:id => @info_request.id)) do |f| %>
         <h2>What is the status of this request now?</h2>
-        <hr>
-        <div>
-            <%= radio_button "incoming_message", "described_state", "waiting_response", :id => 'waiting_response' + id_suffix %>
-            <label for="waiting_response<%=id_suffix%>">I'm still <strong>waiting</strong> for my information</label>
-        </div>
-        <div>
-            <%= radio_button "incoming_message", "described_state", "waiting_clarification", :id => 'waiting_clarification' + id_suffix %>
-            <label for="waiting_clarification<%=id_suffix%>">I'm about to <strong>clarify</strong> my request</label>
-        </div>
+
+        <hr> <!------------------------------------------------>
+
+        <% if @info_request.described_state != 'internal_review' %>
+            <div>
+                <%= radio_button "incoming_message", "described_state", "waiting_response", :id => 'waiting_response' + id_suffix %>
+                <label for="waiting_response<%=id_suffix%>">I'm still <strong>waiting</strong> for my information</label>
+            </div>
+        <% end %>
+        <% if @info_request.described_state == 'internal_review' %>
+            <div>
+                <%= radio_button "incoming_message", "described_state", "internal_review", :id => 'internal_review' + id_suffix %>
+                <label for="internal_review<%=id_suffix%>">I'm still <strong>waiting</strong> for the internal review</label>
+            </div>
+        <% end %>
+
+        <% if @info_request.described_state != 'internal_review' %>
+            <div>
+                <%= radio_button "incoming_message", "described_state", "waiting_clarification", :id => 'waiting_clarification' + id_suffix %>
+                <label for="waiting_clarification<%=id_suffix%>">I'm about to <strong>clarify</strong> my request</label>
+            </div>
+        <% end %>
+
         <div>
             <%= radio_button "incoming_message", "described_state", "gone_postal", :id => 'gone_postal' + id_suffix %>
             <label for="gone_postal<%=id_suffix%>">They are going to reply <strong>by post</strong></label>
         </div>
-        <hr>
+
+        <hr> <!------------------------------------------------>
+
+        <% if @info_request.described_state == 'internal_review' %>
+            <p>The <strong>review has finished</strong> and overall:</p>
+        <% end %>
+
         <div>
             <%= radio_button "incoming_message", "described_state", "not_held", :id => 'not_held' + id_suffix %>
             <label for="not_held<%=id_suffix%>">They do <strong>not have</strong> the information <small>(maybe they say who does)</small></label>
@@ -31,7 +51,9 @@
             <%= radio_button "incoming_message", "described_state", "rejected", :id => 'rejected' + id_suffix %>
             <label for="rejected<%=id_suffix%>">My request has been <strong>rejected</strong></label>
         </div>
-        <hr>
+
+        <hr> <!------------------------------------------------>
+
         <div>
            <%= radio_button "incoming_message", "described_state", "requires_admin", :id => 'requires_admin' + id_suffix %>
             <label for="requires_admin<%=id_suffix%>"><strong>None</strong> of the above</label>
diff --git a/app/views/request/_followup.rhtml b/app/views/request/_followup.rhtml
index fe16ff33b..138503a30 100644
--- a/app/views/request/_followup.rhtml
+++ b/app/views/request/_followup.rhtml
@@ -9,6 +9,12 @@
         </h2>
     <% end %>
 
+    <% if @internal_review %>
+        <p>Write a message asking the authority to conduct an internal review
+        of their handling of your request.
+        </p>
+    <% end %>
+
     <% if @info_request.stop_new_responses %>
         <p>Follow ups and new responses to this request have been stopped to prevent spam. Please
         <a href="/help/contact">contact us</a> if you are <%= user_link(@info_request.user) %>
diff --git a/app/views/request/show.rhtml b/app/views/request/show.rhtml
index eb09bfa43..ef5c7557d 100644
--- a/app/views/request/show.rhtml
+++ b/app/views/request/show.rhtml
@@ -73,6 +73,13 @@
         the response had to be
         <%= link_to "no later", about_url + "#quickly_response" %>
         than <strong><%= simple_date(@info_request.date_response_required_by) %></strong>.
+
+        <% if @info_request.days_overdue > 14 %>
+            They are <%= @info_request.days_overdue.to_i %> days late, you
+            can <strong>complain</strong> by
+            <%= link_to "requesting an internal review", show_response_no_followup_url(:id => @info_request.id, :incoming_message_id => nil) + "?internal_review=1#followup" %>.
+        <% end %>
+
     <% elsif @status == 'not_held' %>
         <%= public_body_link(@info_request.public_body) %> <strong>did not have</strong> the information requested.
     <% elsif @status == 'rejected' %>
@@ -117,21 +124,27 @@
 
     <div id="after_actions">
         <h2>Things to do with this request</h2>
-        <%= link_to "Add an annotation", new_comment_url(:url_title => @info_request.url_title) %> (to help
-        the requester or others)
-        <br>
-        <% if @last_response.nil? %>
-            <%= link_to "Send follow up to " + RequestMailer.name_for_followup(@info_request, @last_response), show_response_no_followup_url(:id => @info_request.id, :incoming_message_id => nil) + "#followup" %> 
-        <% else %>
-            <%= link_to "Reply to " + RequestMailer.name_for_followup(@info_request, @last_response), show_response_url(:id => @info_request.id, :incoming_message_id => @last_response.id) + "#followup" %> 
-        <% end %>
-        (<%=h @info_request.user.name %> only)
-        <br>
-        <!-- <%= link_to "Request an internal review", show_response_no_followup_url(:id => @info_request.id, :incoming_message_id => nil) + "#followup" %> 
-        (<%=h @info_request.user.name %> only)
-        <br> -->
-        <%= link_to "Respond to request", upload_response_url(:url_title => @info_request.url_title) %> 
-        (<%=h @info_request.public_body.name %> only)
+
+        <p>
+            <%= link_to "Add an annotation", new_comment_url(:url_title => @info_request.url_title) %> (to help
+            the requester or others)
+        </p>
+
+        <p>
+            <strong><%=h @info_request.user.name %> only:</strong>
+            <% if @last_response.nil? %>
+                <%= link_to "Send follow up to " + RequestMailer.name_for_followup(@info_request, @last_response), show_response_no_followup_url(:id => @info_request.id, :incoming_message_id => nil) + "#followup" %> 
+            <% else %>
+                <%= link_to "Reply to " + RequestMailer.name_for_followup(@info_request, @last_response), show_response_url(:id => @info_request.id, :incoming_message_id => @last_response.id) + "#followup" %> 
+            <% end %>
+            |
+            <%= link_to "Request an internal review", show_response_no_followup_url(:id => @info_request.id, :incoming_message_id => nil) + "?internal_review=1#followup" %> 
+
+            <br>
+
+            <strong><%=h @info_request.public_body.name %> only:</strong>
+            <%= link_to "Respond to request", upload_response_url(:url_title => @info_request.url_title) %> 
+        </p>
     </div>
 </div>
 
diff --git a/app/views/request/show_response.rhtml b/app/views/request/show_response.rhtml
index 25b9e6470..7f43aae7a 100644
--- a/app/views/request/show_response.rhtml
+++ b/app/views/request/show_response.rhtml
@@ -47,29 +47,27 @@
 <% end %>
 
 <div id="show_response_view">
-    <% if @is_owning_user %>
-        <% if @incoming_message.nil? %>
-            <h2>Your last message sent for request '<%= request_link @info_request %>'</h2>
-        <% elsif @incoming_message.recently_arrived %>
-            <h2>New response to your request '<%= request_link @info_request %>'</h2>
+    <% if !@incoming_message.nil? %>
+        <% if @is_owning_user %>
+            <% if @incoming_message.recently_arrived %>
+                <h2>New response to your request '<%= request_link @info_request %>'</h2>
+            <% else %>
+                <h2>Response to your request '<%= request_link @info_request %>'</h2>
+            <% end %>
         <% else %>
-            <h2>Response to your request '<%= request_link @info_request %>'</h2>
+            <% if @incoming_message.recently_arrived %>
+                <h2>New response to <%=h(@info_request.law_used_short)%> request '<%= request_link @info_request %>'</h2>
+            <% else %>
+                <h2>Response to <%=h(@info_request.law_used_short)%> request '<%= request_link @info_request %>'</h2>
+            <% end %>
         <% end %>
-    <% else %>
+        
         <% if @incoming_message.nil? %>
-            <h2>Last message sent for <%=h(@info_request.law_used_short)%> request '<%= request_link @info_request %>'</h2>
-        <% elsif @incoming_message.recently_arrived %>
-            <h2>New response to <%=h(@info_request.law_used_short)%> request '<%= request_link @info_request %>'</h2>
+            <%= render :partial => 'correspondence', :locals => { :info_request_event => @info_request.get_last_outgoing_event, :incoming_message => nil } %>
         <% else %>
-            <h2>Response to <%=h(@info_request.law_used_short)%> request '<%= request_link @info_request %>'</h2>
+            <%= render :partial => 'correspondence', :locals => { :info_request_event => nil, :incoming_message => @incoming_message } %>
         <% end %>
     <% end %>
-    
-    <% if @incoming_message.nil? %>
-        <%= render :partial => 'correspondence', :locals => { :info_request_event => @info_request.get_last_outgoing_event, :incoming_message => nil } %>
-    <% else %>
-        <%= render :partial => 'correspondence', :locals => { :info_request_event => nil, :incoming_message => @incoming_message } %>
-    <% end %>
 
     <%= render :partial => 'followup', :locals => { :incoming_message => @incoming_message } %>
 </div>
diff --git a/config/routes.rb b/config/routes.rb
index b4f3e3a62..a7e5984fa 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -4,7 +4,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: routes.rb,v 1.78 2008-10-28 17:58:48 francis Exp $
+# $Id: routes.rb,v 1.79 2008-11-07 00:01:51 francis Exp $
 
 ActionController::Routing::Routes.draw do |map|
 
@@ -83,7 +83,8 @@ ActionController::Routing::Routes.draw do |map|
     end
 
     map.with_options :controller => 'help' do |help|
-      help.help_general '/help/:action',            :action => :action
+      help.help_general '/help/unhappy/:url_title', :action => 'unhappy'
+      help.help_general '/help/:action', :action => :action
     end
 
     # NB: We don't use routes to *construct* admin URLs, as they need to be relative
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index e3e032d01..18e686a07 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -102,8 +102,14 @@ describe RequestController, "when creating a new request" do
         response.should redirect_to(:controller => 'general', :action => 'frontpage')
     end
 
-    it "should accept a public body parameter posted from the front page" do
-        post :new, :info_request => { :public_body_id => public_bodies(:geraldine_public_body).id } 
+    it "should redirect to front page if no public body specified, when logged in" do
+        session[:user_id] = users(:bob_smith_user).id
+        get :new
+        response.should redirect_to(:controller => 'general', :action => 'frontpage')
+    end
+
+    it "should accept a public body parameter" do
+        get :new, :info_request => { :public_body_id => public_bodies(:geraldine_public_body).id } 
         assigns[:info_request].public_body.should == public_bodies(:geraldine_public_body)    
         response.should render_template('new')
     end
@@ -278,20 +284,20 @@ describe RequestController, "when sending a followup message" do
     fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :raw_emails, :outgoing_messages # all needed as integrating views
   
     it "should require login" do
-        post :show_response, :outgoing_message => { :body => "What a useless response! You suck." }, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1
+        post :show_response, :outgoing_message => { :body => "What a useless response! You suck.", :what_doing => 'normal_sort' }, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1
         post_redirect = PostRedirect.get_last_post_redirect
         response.should redirect_to(:controller => 'user', :action => 'signin', :token => post_redirect.token)
     end
 
     it "should not let you if you are logged in as the wrong user" do
         session[:user_id] = users(:silly_name_user).id
-        post :show_response, :outgoing_message => { :body => "What a useless response! You suck." }, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1
+        post :show_response, :outgoing_message => { :body => "What a useless response! You suck.", :what_doing => 'normal_sort' }, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1
         response.should render_template('user/wrong_user')
     end
 
     it "should give an error and render 'show_response' template when a body isn't given" do
         session[:user_id] = users(:bob_smith_user).id
-        post :show_response, :outgoing_message => { :body => "" }, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1
+        post :show_response, :outgoing_message => { :body => "", :what_doing => 'normal_sort'}, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1
 
         # XXX how do I check the error message here?
         response.should render_template('show_response')
@@ -306,7 +312,7 @@ describe RequestController, "when sending a followup message" do
 
         # make the followup
         session[:user_id] = users(:bob_smith_user).id
-        post :show_response, :outgoing_message => { :body => "What a useless response! You suck." }, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1
+        post :show_response, :outgoing_message => { :body => "What a useless response! You suck.", :what_doing => 'normal_sort' }, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1
 
         # check it worked
         deliveries = ActionMailer::Base.deliveries
diff --git a/todo.txt b/todo.txt
index df6b46f74..fa5fa1f64 100644
--- a/todo.txt
+++ b/todo.txt
@@ -3,49 +3,36 @@ Test data for Tony
 Internal review
 ===============
 
-Change wording of the status change dialog when coming out of internal
-review
-
-Fix up controllers/request_controller internal_review state redirect
-in that case.
-
-When you select "waiting_response" might want to convert to 
-internal_review in case where current state is already internal_review.
+Add the "passing on" text from Julian's requests as default for internal review
+Perhaps log that whole chain of ?internal_review=1 things to only be reviews
+and change heading etc.
 
-Clarification of internal review in outgoing message send_message function
-might want to go back to internal review, not to general awaiting response?
-    models/outgoing_message.rb line 144
+Improve message on entering not_held - offer option of internal review.
 
-Send internal reviews to original main request address
+If you've already conducted an internal review, at all places offer
+link through to ICO submitting help page instead.
 
-Where to offer button to enter internal review state:
-- always offer option on request to have internal review, except when in waiting_response
-earlier than 20 days, and except when in waiting_clarification (call button "make clarification"!)
-- offer optin more forcefully when in waiting_response and lots of days have gone by
-- when entering not_held, rejected, partially_successful offer internal review button clearly
+Change preview when you've chosen the internal review radio button
 
-Make the internal review option default the radio button on the followup form
 
+Clock for internal review
+"The internal review should take 2-3 weeks for simple cases, and up to 6 weeks
+even for complex reviews."
 
 Search for text "internal review" in followups and add warning if they aren't
 using the internal review mode.
 
-Change preview when asking for internal review
-
-If you've already conducted an internal review, at all those places above offer
-link through to ICO submitting help page instead.
-
-Clock for internal review
-
-Add the "passing on" text from Julian's requests as default for internal review
-
 
 Next
 ====
 
+Rename show_response action to send followup?
+
 Finish "new information" option when writing followup, so makes new request
 Let people change status at any point
 
+Let admin users set things to internal review easily
+
 Make green box more visible when click reminder email link
 Check new wording in reminder email looks good
 Make the second and third reminder email for new responses go to the green bit