Add the specific salutations to each request.

Also, wrap model creation in a transaction and do the message sending
separately - we may ultimately want to do this outside the request
cycle.

3 files changed, 73 insertions, 25 deletions

diff --git a/app/models/info_request_batch.rb b/app/models/info_request_batch.rb
index 3c6addb70..1a2605e36 100644
--- a/app/models/info_request_batch.rb
+++ b/app/models/info_request_batch.rb
@@ -28,26 +28,41 @@ class InfoRequestBatch < ActiveRecord::Base
                      :include => :info_requests)
     end
 
-
+    # Create a batch of information requests, returning the batch, and a list of public bodies
+    # that are unrequestable from the initial list of public body ids passed.
     def InfoRequestBatch.create_batch!(info_request_params, outgoing_message_params, public_body_ids, user)
-        info_request_batch = InfoRequestBatch.create!(:title => info_request_params[:title],
-                                                      :body => outgoing_message_params[:body],
-                                                      :user => user)
-       public_bodies = PublicBody.where({:id => public_body_ids}).all
-       unrequestable = []
-       public_bodies.each do |public_body|
-           if public_body.is_requestable?
-               info_request = InfoRequest.create_from_attributes(info_request_params,
-                                                                 outgoing_message_params,
-                                                                 user)
-               info_request.public_body_id = public_body.id
-               info_request.info_request_batch = info_request_batch
-               info_request.save!
-               info_request.outgoing_messages.first.send_message
-           else
-                unrequestable << public_body
-           end
-       end
-       return {:batch => info_request_batch, :unrequestable => unrequestable}
+        info_request_batch = nil
+        unrequestable = []
+        created = []
+        public_bodies = PublicBody.where({:id => public_body_ids}).all
+        ActiveRecord::Base.transaction do
+            info_request_batch = InfoRequestBatch.create!(:title => info_request_params[:title],
+                                                          :body => outgoing_message_params[:body],
+                                                          :user => user)
+            public_bodies.each do |public_body|
+                if public_body.is_requestable?
+                    created << info_request_batch.create_request!(public_body,
+                                                                  info_request_params,
+                                                                  outgoing_message_params,
+                                                                  user)
+                else
+                    unrequestable << public_body
+                end
+            end
+        end
+        created.each{ |info_request| info_request.outgoing_messages.first.send_message }
+        return {:batch => info_request_batch, :unrequestable => unrequestable}
+    end
+
+    # Create and send an FOI request to a public body
+    def create_request!(public_body, info_request_params, outgoing_message_params, user)
+        body = OutgoingMessage.fill_in_salutation(outgoing_message_params[:body], public_body)
+        info_request = InfoRequest.create_from_attributes(info_request_params,
+                                                          outgoing_message_params.merge(:body => body),
+                                                          user)
+        info_request.public_body_id = public_body.id
+        info_request.info_request_batch = self
+        info_request.save!
+        info_request
     end
 end
diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb
index 7132ee223..a435511d3 100644
--- a/app/models/outgoing_message.rb
+++ b/app/models/outgoing_message.rb
@@ -68,19 +68,30 @@ class OutgoingMessage < ActiveRecord::Base
 
     # How the default letter starts and ends
     def get_salutation
+        if self.info_request.is_batch_request_template?
+            return OutgoingMessage.placeholder_salutation
+        end
         ret = ""
         if self.message_type == 'followup' && !self.incoming_message_followup.nil? && !self.incoming_message_followup.safe_mail_from.nil? && self.incoming_message_followup.valid_to_reply_to?
             ret = ret + OutgoingMailer.name_for_followup(self.info_request, self.incoming_message_followup)
         else
-            if self.info_request.is_batch_request_template?
-                ret = ret + _("[Authority name]")
-            else
-                ret = ret + self.info_request.public_body.name
-            end
+            return OutgoingMessage.default_salutation(self.info_request.public_body)
         end
         salutation = _("Dear {{public_body_name}},", :public_body_name => ret)
     end
 
+    def OutgoingMessage.default_salutation(public_body)
+        _("Dear {{public_body_name}},", :public_body_name => public_body.name)
+    end
+
+    def OutgoingMessage.placeholder_salutation
+        _("Dear [Authority name],")
+    end
+
+    def OutgoingMessage.fill_in_salutation(body, public_body)
+        body.gsub(placeholder_salutation, default_salutation(public_body))
+    end
+
     def get_signoff
         if self.message_type == 'followup' && !self.incoming_message_followup.nil? && !self.incoming_message_followup.safe_mail_from.nil? && self.incoming_message_followup.valid_to_reply_to?
             return _("Yours sincerely,")
diff --git a/spec/models/info_request_batch_spec.rb b/spec/models/info_request_batch_spec.rb
index 787b812cf..ba4bcd181 100644
--- a/spec/models/info_request_batch_spec.rb
+++ b/spec/models/info_request_batch_spec.rb
@@ -64,3 +64,25 @@ describe InfoRequestBatch, "when finding an existing batch" do
                                        [@first_request.public_body_id]).should be_nil
     end
 end
+
+describe InfoRequestBatch, "when creating a batch", :focus => true do
+
+    it 'should substitute authority name for the placeholder in each request' do
+        info_request_params = {:title => 'A test title'}
+        outgoing_message_params = {:body => "Dear [Authority name],\nA message\nYours faithfully,\nRequester",
+                                  }
+        first_public_body = FactoryGirl.create(:public_body)
+        second_public_body = FactoryGirl.create(:public_body)
+        user = FactoryGirl.create(:user)
+        results = InfoRequestBatch.create_batch!(info_request_params,
+                                                 outgoing_message_params,
+                                                 [first_public_body.id, second_public_body.id],
+                                                 user)
+        info_requests = results[:batch].info_requests
+        [first_public_body, second_public_body].each do |public_body|
+            request = info_requests.detect{|info_request| info_request.public_body == public_body}
+            request.outgoing_messages.first.body.should == "Dear #{public_body.name},\nA message\nYours faithfully,\nRequester"
+        end
+    end
+
+end