Don't allow duplicate followup messages to the same request.

Prevent requests, followups and annotations which have all capitals or all lowercase letters.

6 files changed, 41 insertions, 7 deletions

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index c91f2fad8..ec5115f99 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.168 2009-07-14 23:30:37 francis Exp $
+# $Id: request_controller.rb,v 1.169 2009-08-18 20:51:25 francis Exp $
 
 class RequestController < ApplicationController
     
@@ -469,6 +469,12 @@ class RequestController < ApplicationController
             if @info_request.allow_new_responses_from == 'nobody'
                 flash[:error] = 'Your follow up has not been sent because this request has been stopped to prevent spam. Please <a href="/help/contact">contact us</a> if you really want to send a follow up message.'
             else
+                if @info_request.find_existing_outgoing_message(params[:outgoing_message][:body])
+                    flash[:error] = 'You previously submitted that exact follow up message for this request.'
+                    render :action => 'show_response'
+                    return
+                end
+
                 # See if values were valid or not
                 @outgoing_message.info_request = @info_request
                 if !@outgoing_message.valid?
diff --git a/app/models/comment.rb b/app/models/comment.rb
index 250b94c20..af6a4f9ef 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -19,7 +19,7 @@
 # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: comment.rb,v 1.16 2009-06-26 14:28:37 francis Exp $
+# $Id: comment.rb,v 1.17 2009-08-18 20:51:26 francis Exp $
 
 class Comment < ActiveRecord::Base
     strip_attributes!
@@ -58,6 +58,9 @@ class Comment < ActiveRecord::Base
         if self.body.empty? || self.body =~ /^\s+$/
             errors.add(:body, "^Please enter your annotation")
         end
+        if !MySociety::Validate.uses_mixed_capitals(self.body)
+            errors.add(:body, '^Please write your annotation using a mixture of capital and lower case letters. This makes it easier for others to read.')
+        end
     end
 
     # Return body for display as HTML
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index dfa66102e..13d5c8a2e 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -24,7 +24,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.198 2009-07-03 11:43:37 francis Exp $
+# $Id: info_request.rb,v 1.199 2009-08-18 20:51:26 francis Exp $
 
 require 'digest/sha1'
 require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian')
@@ -279,6 +279,17 @@ public
         end
     end
 
+    def find_existing_outgoing_message(body)
+        # XXX can add other databases here which have regexp_replace
+        if ActiveRecord::Base.connection.adapter_name == "PostgreSQL"
+            # Exclude spaces from the body comparison using regexp_replace
+            return self.outgoing_messages.find(:first, :conditions => [ "regexp_replace(outgoing_messages.body, '[[:space:]]', '', 'g') = regexp_replace(?, '[[:space:]]', '', 'g')", body ])
+        else
+            # For other databases (e.g. SQLite) not the end of the world being space-sensitive for this check
+            return self.outgoing_messages.find(:first, :conditions => [ "outgoing_messages.body = ?", body ])
+        end
+    end
+
     # A new incoming email to this request
     def receive(email, raw_email_data, override_stop_new_responses = false)
         if !override_stop_new_responses
diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb
index 6d1900d64..28701185a 100644
--- a/app/models/outgoing_message.rb
+++ b/app/models/outgoing_message.rb
@@ -22,7 +22,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.87 2009-07-21 12:09:30 francis Exp $
+# $Id: outgoing_message.rb,v 1.88 2009-08-18 20:51:26 francis Exp $
 
 class OutgoingMessage < ActiveRecord::Base
     strip_attributes!
@@ -129,8 +129,10 @@ class OutgoingMessage < ActiveRecord::Base
         if self.body =~ /#{get_signoff}\s*\Z/ms
             errors.add(:body, '^Please sign at the bottom with your name, or alter the "' + get_signoff + '" signature')
         end
+        if !MySociety::Validate.uses_mixed_capitals(self.body)
+            errors.add(:body, '^Please write your message using a mixture of capital and lower case letters. This makes it easier for others to read.')
+        end
         if self.what_doing.nil? || !['new_information', 'internal_review', 'normal_sort'].include?(self.what_doing)
-
             errors.add(:what_doing_dummy, '^Please choose what sort of reply you are making.')
         end
     end
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 939061ce8..e8bf5c1fc 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -681,6 +681,18 @@ describe RequestController, "when sending a followup message" do
         info_requests(:fancy_dog_request).get_last_response_event.calculated_state.should == 'waiting_clarification'
     end
 
+    it "should give an error if the same followup is submitted twice" do
+        session[:user_id] = users(:bob_smith_user).id
+
+        # make the followup once
+        post :show_response, :outgoing_message => { :body => "Stop repeating yourself!", :what_doing => 'normal_sort' }, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message), :submitted_followup => 1
+        response.should redirect_to(:action => 'show', :url_title => info_requests(:fancy_dog_request).url_title)
+        
+        # second time should give an error
+        post :show_response, :outgoing_message => { :body => "Stop repeating yourself!", :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')
+    end
 
 end
 
diff --git a/todo.txt b/todo.txt
index 28d3afeba..c60e2ade1 100644
--- a/todo.txt
+++ b/todo.txt
@@ -88,8 +88,6 @@ their email address (perhaps just have admins validate / approve it)
 Let requesters view the uncensored versions of their correspondence (e.g. with
 emails in it). Let other people do so with a CAPTCHA?
 
-Test for duplicate followups, as we do for requests.
-
 For followups, have radio button to say is it a new request or followup
 Do by uncommenting the "new information" option when writing a followup, so
 that it makes a new request
@@ -208,6 +206,8 @@ all listing should just link to top of page, rather than # links for outgoing
 incoming, or perhaps just some of them.
 
 Help page improvements:
+    Needs to say somewhere in flow that you can make a request privately (I think quite a 
+       few people get to us via Google and don't realise that they can)
     Add "Who should I make my request to?" - make flow better after first section, to abrupt now
     I think the advice in this annotation could go into a nice comment:
       http://www.whatdotheyknow.com/request/berr_response_to_eu_on_phorm_bt#comment-356