Prevent (probably accidental) double posting of the same request.

5 files changed, 53 insertions, 11 deletions

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 72042138c..bc03fd539 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.26 2008-01-03 12:54:40 francis Exp $
+# $Id: request_controller.rb,v 1.27 2008-01-04 10:56:22 francis Exp $
 
 class RequestController < ApplicationController
     
@@ -32,6 +32,13 @@ class RequestController < ApplicationController
 
     # Page new form posts to
     def create
+        # See if the exact same request has already been submitted
+        # XXX this *should* also check outgoing message joined to is an initial request (rather than follow up)
+        # XXX this check could go in the model, except we really want to pass @existing_request to the view so it can link to it.
+        # XXX could have a date range here, so say only check last month's worth of new requests. If somebody is making
+        # repeated requests, say once a quarter for time information, then might need to do that.
+        @existing_request = InfoRequest.find(:first, :conditions => [ 'title = ? and public_body_id = ? and outgoing_messages.body = ?', params[:info_request][:title], params[:info_request][:public_body_id], params[:outgoing_message][:body] ], :include => [ :outgoing_messages ] )
+
         # Create both FOI request and the first request message
         @info_request = InfoRequest.new(params[:info_request])
         @outgoing_message = OutgoingMessage.new(params[:outgoing_message].merge({ 
@@ -41,8 +48,8 @@ class RequestController < ApplicationController
         @info_request.outgoing_messages << @outgoing_message
         @outgoing_message.info_request = @info_request
 
-        # This automatically saves dependent objects, such as @info_request, in the same transaction
-        if not @info_request.valid?
+        # See if values were valid or not
+        if !@existing_request.nil? || !@info_request.valid?
             render :action => 'new'
         elsif authenticated?(
                 :web => "To send your FOI request",
@@ -50,6 +57,7 @@ class RequestController < ApplicationController
                 :email_subject => "Confirm that you want to send an FOI request to " + @info_request.public_body.name
             )
             @info_request.user = authenticated_user
+            # This automatically saves dependent objects, such as @outgoing_message, in the same transaction
             @info_request.save!
             @outgoing_message.send_message
             flash[:notice] = "Your Freedom of Information request has been created and sent on its way."
diff --git a/app/helpers/link_to_helper.rb b/app/helpers/link_to_helper.rb
index 41a462280..b26c7d020 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.7 2008-01-02 20:18:06 francis Exp $
+# $Id: link_to_helper.rb,v 1.8 2008-01-04 10:56:22 francis Exp $
 
 module LinkToHelper
 
@@ -36,6 +36,22 @@ module LinkToHelper
     def user_link(user)
         link_to h(user.name), user_url(user)
     end
+    def user_or_you_link(user)
+        if @user && user == @user
+            link_to h("you"), user_url(user)
+        else
+            link_to h(user.name), user_url(user)
+        end
+    end
+    def user_or_you_capital_link(user)
+        if @user && user == @user
+            link_to h("You"), user_url(user)
+        else
+            link_to h(user.name), user_url(user)
+        end
+    end
+
+
 
     def info_request_link(info_request)
         link_to h(info_request.title), show_request_url(:id => info_request)
diff --git a/app/views/request/new.rhtml b/app/views/request/new.rhtml
index 8fcc43c90..8621fdf9e 100644
--- a/app/views/request/new.rhtml
+++ b/app/views/request/new.rhtml
@@ -1,5 +1,16 @@
 <% @title = "New FOI request" %>
 
+<% if @existing_request %>
+    <div class="errorExplanation" id="errorExplanation"><ul>
+    <li>
+    <%= user_or_you_capital_link(@existing_request.user) %> already
+    created the same request on <%=simple_date(@existing_request.created_at)%>.
+    You can either view the <a href="<%=request_url(@existing_request)%>">existing request</a>,
+    or edit the details below to make a new but similar request.
+    </li>
+    </ul></div>
+<% end %>
+
 <%= foi_error_messages_for :info_request, :outgoing_message %>
 
 <% form_for(:info_request, @info_request, :url => { :action => :create }, :html => { :id => 'writeForm' }  ) do |f| %>
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 412ad0d5b..7dde21ba5 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -87,7 +87,7 @@ end
 
 describe RequestController, "when creating a new request" do
     integrate_views
-    fixtures :info_requests, :public_bodies, :users
+    fixtures :info_requests, :outgoing_messages, :public_bodies, :users
 
     it "should render with 'new' template" do
         get :new
@@ -136,6 +136,13 @@ describe RequestController, "when creating a new request" do
         om.body.should == "This is a silly letter. It is too short to be interesting."
         response.should redirect_to(:controller => 'request', :action => 'show', :id => ir.id)
     end
+
+    it "should give an error if the same request is submitted twice" do
+        post :create, :info_request => { :public_body_id => info_requests(:fancy_dog_request).public_body_id, 
+            :title => info_requests(:fancy_dog_request).title},
+            :outgoing_message => { :body => info_requests(:fancy_dog_request).outgoing_messages[0].body}  
+        response.should render_template('new')
+    end
 end
 
 
diff --git a/todo.txt b/todo.txt
index 18d6fa23a..70a2e7952 100644
--- a/todo.txt
+++ b/todo.txt
@@ -25,8 +25,6 @@ Make response messages go to a mailbox as backup
 Make it so if the pipe fails, exim tries again rather than sending an error to the public body.
     Or so errors go to an admin somehow, at the very least.
 
-Either rotate log files, or merge with Apache ones
-
 Let requester send follow-ups - but to which email address???!! aargh
 Do something after 20 working days if you get no response
     display the working days correctly
@@ -35,13 +33,13 @@ Work out how to get it to tell you code coverage of .rhtml files
 Make it validate the HTML
     maybe with http://www.anodyne.ca/wp-content/uploads/2007/09/be_valid_xhtml.rb
 Test that it is actually sending the request outgoing mail, by using deliveries
+Add fixtures for info_request_event
+Test sending a message to bounce/envelope-from address
 
 Remove "Outgoing messages is invalid" error
 
 Make it so "mysociety test" can be done on the servers, so it checks any packages are installed
  
-Add fixtures for info_request_event
-Test sending a message to bounce/envelope-from address
 
 Tidying
 =======
@@ -56,8 +54,6 @@ http://localhost:3000/request/5
 
 Add SQL database indexes to token / email_token in post_redirects
 
-Prevent double posting of same request
-
 Set "null" and "default" options more in schema
 Add SQL foreign keys to database schema
     execute 'ALTER TABLE researchers ADD CONSTRAINT fk_researchers_departments FOREIGN KEY ( department_id ) REFERENCES departments( id ) '
@@ -72,6 +68,8 @@ Do pretty error messages, e.g. on invalid public body name page etc.
 404s on all invalid URL parameters
 Hook global error message also
 
+Check log rotation is working well
+
 Legal/privacy
 =============
 
@@ -114,6 +112,8 @@ http://www.mysociety.org/2006/04/04/freedom-of-information-archive/
 
 Check FOE site lots
 http://community.foe.co.uk/tools/right_to_know/request_generator.html
+It has email addresses of lots of bodies, e.g.
+http://community.foe.co.uk/app/tc
 
 Look at this basic US site
 http://www.rcfp.org/foi_letter/generate.php