Fix various things to do with event statuses. Oops.

6 files changed, 39 insertions, 18 deletions

diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb
index be40e17f4..f94861b5e 100644
--- a/app/controllers/admin_request_controller.rb
+++ b/app/controllers/admin_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: admin_request_controller.rb,v 1.11 2008-05-05 22:38:04 francis Exp $
+# $Id: admin_request_controller.rb,v 1.12 2008-05-12 00:56:21 francis Exp $
 
 class AdminRequestController < ApplicationController
     layout "admin"
@@ -47,7 +47,7 @@ class AdminRequestController < ApplicationController
         @info_request.title = params[:info_request][:title]
         @info_request.prominence = params[:info_request][:prominence]
         if @info_request.described_state != params[:info_request][:described_state]
-            @info_request.set_described_state(params[:info_request][:described_state], @info_request.get_last_event.id.to_i)
+            @info_request.set_described_state(params[:info_request][:described_state])
         end
         @info_request.awaiting_description = params[:info_request][:awaiting_description] == "true" ? true : false
 
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index db53631c4..1879482a3 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.78 2008-05-05 22:48:43 francis Exp $
+# $Id: request_controller.rb,v 1.79 2008-05-12 00:56:21 francis Exp $
 
 class RequestController < ApplicationController
     
@@ -191,7 +191,7 @@ class RequestController < ApplicationController
             end
 
             # Make the state change
-            @info_request.set_described_state(params[:incoming_message][:described_state], @last_info_request_event_id)
+            @info_request.set_described_state(params[:incoming_message][:described_state])
 
             # Display appropriate next page (e.g. help for complaint etc.)
             if @info_request.calculate_status == 'waiting_response'
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index d3285116c..e439ee656 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -21,7 +21,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.106 2008-05-09 01:02:32 francis Exp $
+# $Id: info_request.rb,v 1.107 2008-05-12 00:56:22 francis Exp $
 
 require 'digest/sha1'
 require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian')
@@ -178,12 +178,11 @@ public
         RequestMailer.deliver_new_response(self, incoming_message)
     end
 
-    # Change status - event id is of the most recent event at the change
-    # XXX should probably check event id is last event here
-    def set_described_state(new_state, event_id)
+    # Change status, including for last event for later historical purposes
+    def set_described_state(new_state)
         ActiveRecord::Base.transaction do
             self.awaiting_description = false
-            last_event = InfoRequestEvent.find(event_id)
+            last_event = self.get_last_event
             last_event.described_state = new_state
             self.described_state = new_state
             last_event.save!
@@ -358,6 +357,16 @@ public
     end
 
     # The last response is the default one people might want to reply to
+    def get_last_response_event
+        info_request_event_id = get_last_response_event_id
+        if info_request_event_id.nil?
+            return nil
+        else
+            return InfoRequestEvent.find(info_request_event_id)
+        end
+    end
+
+    # The last response is the default one people might want to reply to
     def get_last_response
         event_id = self.get_last_response_event_id
         if event_id.nil?
diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb
index 189ff77d2..b13f266d5 100644
--- a/app/models/outgoing_message.rb
+++ b/app/models/outgoing_message.rb
@@ -21,7 +21,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: outgoing_message.rb,v 1.44 2008-04-24 22:50:03 angie Exp $
+# $Id: outgoing_message.rb,v 1.45 2008-05-12 00:56:22 francis Exp $
 
 class OutgoingMessage < ActiveRecord::Base
     belongs_to :info_request
@@ -97,7 +97,7 @@ class OutgoingMessage < ActiveRecord::Base
                 self.save!
                 self.info_request.log_event('followup_' + log_event_type, { :email => self.info_request.recipient_email, :outgoing_message_id => self.id })
                 if self.info_request.described_state == 'waiting_clarification'
-                    self.info_request.set_described_state('waiting_response', self.info_request.events_needing_description[-1].id)
+                    self.info_request.set_described_state('waiting_response')
                 end
             else
                 raise "Message id #{self.id} has type '#{self.message_type}' which send_message can't handle"
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 4855a88c2..980094e7a 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -225,6 +225,8 @@ describe RequestController, "when classifying an individual response" do
         response.should redirect_to(:controller => 'help', :action => 'unhappy')
         info_requests(:fancy_dog_request).reload
         info_requests(:fancy_dog_request).awaiting_description.should == false
+        info_requests(:fancy_dog_request).described_state.should == 'rejected'
+        info_requests(:fancy_dog_request).get_last_response_event.calculated_state.should == 'rejected'
     end
 
         #response.should redirect_to(:action => 'show', :id => info_requests(:fancy_dog_request))
@@ -257,9 +259,16 @@ describe RequestController, "when sending a followup message" do
 
 
     it "should send the follow up message if you are the right user" do
+        # fake that this is a clarification
+        info_requests(:fancy_dog_request).set_described_state('waiting_clarification')
+        info_requests(:fancy_dog_request).described_state.should == 'waiting_clarification'
+        info_requests(:fancy_dog_request).get_last_response_event.calculated_state.should == 'waiting_clarification'
+
+        # 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
 
+        # check it worked
         deliveries = ActionMailer::Base.deliveries
         deliveries.size.should == 1
         mail = deliveries[0]
@@ -267,6 +276,11 @@ describe RequestController, "when sending a followup message" do
         mail.to_addrs.to_s.should == "FOI Person <foiperson@localhost>"
 
         response.should redirect_to(:action => 'show', :url_title => info_requests(:fancy_dog_request).url_title)
+
+        # and that the status changed
+        info_requests(:fancy_dog_request).reload
+        info_requests(:fancy_dog_request).described_state.should == 'waiting_response'
+        info_requests(:fancy_dog_request).get_last_response_event.calculated_state.should == 'waiting_response'
     end
 
 
diff --git a/todo.txt b/todo.txt
index 15858b5fd..292dc9b10 100644
--- a/todo.txt
+++ b/todo.txt
@@ -39,23 +39,19 @@ Things to track - don't ever return users / public bodies for now!
 Editing status in admin interface isn't putting it in info request events
     Do search for cases where current value doens't corespond to last event value
 
-Museum aliases
-
 Later
 =====
 
-Make the "needs admin attention" have a text box asking why
+Museum aliases
 
 Highlight text search finds in word docs
 Edits to outgoing/incoming/title won't be reindexed in Xapian
 
+Make the "needs admin attention" have a text box asking why
+
 Internet explorer bug with HTML for Elena
 http://www.whatdotheyknow.com/request/_the_infection_rates_of_orthopae
 
-CSV export of lists of emails for admins
-
-Enter > 255 char comment when editing public body gives unhelpful error
-
 Put name of admin user rather than import_csv
 
 Link more clearly to full request and next/previous correspondence (ask Matthew)
@@ -116,6 +112,8 @@ Synthesise these tips into our handful of snappy snappy bullet points
 
 Show public body email address on their public page, with a link to say "this isn't right!"
 
+CSV export of lists of emails for admins
+
 Requests with related content 
     - via a compare document search somehow?
     - using the tracking requests network