Use the other model itself, rather than its id, when saving two models at once,

so that you don't get spurious error. This means if you forget to save the one
before the other you are screwed, but hey.
Instead of a transaction, just call valid? on both models first.

3 files changed, 15 insertions, 16 deletions

diff --git a/app/controllers/file_request_controller.rb b/app/controllers/file_request_controller.rb
index d9f22d804..ab9390a4f 100644
--- a/app/controllers/file_request_controller.rb
+++ b/app/controllers/file_request_controller.rb
@@ -4,31 +4,32 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: file_request_controller.rb,v 1.7 2007-09-11 15:21:30 francis Exp $
+# $Id: file_request_controller.rb,v 1.8 2007-09-12 15:56:18 francis Exp $
 
 class FileRequestController < ApplicationController
     def index
+        # render index.rhtml template
     end
 
+#    before_filter :check_authentication, :only => [:create]
     def create
+    
         # Create both FOI request and the first request message
         @info_request = InfoRequest.new(params[:info_request])
         @outgoing_message = OutgoingMessage.new(params[:outgoing_message].merge({ 
             :status => 'ready', 
             :message_type => 'initial_request'
         }))
+        @outgoing_message.info_request = @info_request
 
         # Save both models
-        ActiveRecord::Base.transaction do
-            begin
-                @info_request.save!
-                @outgoing_message.info_request_id = @info_request.id
-                @outgoing_message.save!
-                # render create action
-            rescue ActiveRecord::RecordInvalid => e
-                @outgoing_message.valid? # force cecking of errors even if info_request fails
-                render :action => 'index'
-            end
+        valid = @info_request.valid? 
+        valid &&= @outgoing_message.valid? # XXX maybe there is a nicer way of preventing lazy boolean evaluation than this
+        if valid
+            @info_request.save!
+            @outgoing_message.save!
+        else
+            render :action => 'index'
         end
     end
 
diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb
index ef768955d..cc1b7ad54 100644
--- a/app/models/outgoing_message.rb
+++ b/app/models/outgoing_message.rb
@@ -5,11 +5,11 @@
 # 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.3 2007-09-11 15:26:12 francis Exp $
+# $Id: outgoing_message.rb,v 1.4 2007-09-12 15:56:18 francis Exp $
 
 class OutgoingMessage < ActiveRecord::Base
     belongs_to :info_request
-    validates_presence_of :info_request_id
+    validates_presence_of :info_request
 
     validates_presence_of :body
     validates_inclusion_of :status, :in => ['ready', 'sent', 'failed']
diff --git a/todo.txt b/todo.txt
index c1f7299d8..dae3557ec 100644
--- a/todo.txt
+++ b/todo.txt
@@ -1,7 +1,5 @@
 Give a better login dialog
 
-Tidy up error message text (like "body must be filled in") on info request form
-
 Redirect the front page to the new FOI request page
 Make it say "dear" as default letter
 
@@ -10,8 +8,8 @@ Write some tests (try it their way, at every level)
 Tidying
 =======
 
-If just summary/title is blank says "Info request must be filled in" as spurious extra error
 If summary is blank, says "title must be filled in" grrrr
+Tidy up error message text (like "body must be filled in") on info request form
 
 Rename "file_request" controller to "new" ?
 Set "null" and "default" options more in schema