Merge branch 'initialize-info-request' into rails-3-develop

4 files changed, 152 insertions, 26 deletions

diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb
index 1e083f57e..db20e8dcc 100644
--- a/app/controllers/admin_request_controller.rb
+++ b/app/controllers/admin_request_controller.rb
@@ -114,21 +114,14 @@ class AdminRequestController < AdminController
             end
             redirect_to admin_request_url(info_request)
         elsif params[:commit] == 'Move request to authority' && !params[:public_body_url_name].blank?
-            old_public_body = info_request.public_body
             destination_public_body = PublicBody.find_by_url_name(params[:public_body_url_name])
-            if destination_public_body.nil?
-                flash[:error] = "Couldn't find public body '" + params[:public_body_url_name] + "'"
-            else
-                info_request.public_body = destination_public_body
-                info_request.save!
-                info_request.log_event("move_request", {
-                        :editor => admin_current_user(),
-                        :old_public_body_url_name => old_public_body.url_name,
-                        :public_body_url_name => destination_public_body.url_name
-                })
 
-                info_request.reindex_request_events
-                flash[:notice] = "Request has been moved to new body"
+            if info_request.move_to_public_body(destination_public_body,
+                                                :editor => admin_current_user,
+                                                :reindex => true)
+              flash[:notice] = "Request has been moved to new body"
+            else
+              flash[:error] = "Couldn't find public body '#{ params[:public_body_url_name] }'"
             end
 
             redirect_to admin_request_url(info_request)
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index f9f6cffa9..0ca3a1279 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -1359,6 +1359,30 @@ public
                 order('last_event_time')
     end
 
+    def move_to_public_body(destination_public_body, opts = {})
+        old_body = public_body
+        editor = opts.fetch(:editor)
+
+        attrs = { :public_body => destination_public_body }
+
+        if destination_public_body
+          attrs.merge!({
+            :law_used => destination_public_body.law_only_short.downcase
+          })
+        end
+
+        if update_attributes(attrs)
+            log_event('move_request',
+                      :editor => editor,
+                      :public_body_url_name => public_body.url_name,
+                      :old_public_body_url_name => old_body.url_name)
+
+            reindex_request_events
+
+            public_body
+        end
+    end
+
     private
 
     def set_defaults
@@ -1370,8 +1394,9 @@ public
             # this should only happen on Model.exists?() call. It can be safely ignored.
             # See http://www.tatvartha.com/2011/03/activerecordmissingattributeerror-missing-attribute-a-bug-or-a-features/
         end
+
         # FOI or EIR?
-        if !self.public_body.nil? && self.public_body.eir_only?
+        if new_record? && public_body && public_body.eir_only?
             self.law_used = 'eir'
         end
     end
diff --git a/app/models/public_body.rb b/app/models/public_body.rb
index 5bce8ecc7..dac77d4c5 100644
--- a/app/models/public_body.rb
+++ b/app/models/public_body.rb
@@ -339,19 +339,20 @@ class PublicBody < ActiveRecord::Base
 
     # Are all requests to this body under the Environmental Information Regulations?
     def eir_only?
-        return self.has_tag?('eir_only')
+        has_tag?('eir_only')
     end
+
     def law_only_short
-        if self.eir_only?
-            return "EIR"
-        else
-            return "FOI"
-        end
+        eir_only? ? 'EIR' : 'FOI'
     end
 
     # Schools are allowed more time in holidays, so we change some wordings
     def is_school?
-        return self.has_tag?('school')
+        has_tag?('school')
+    end
+
+    def site_administration?
+        has_tag?('site_administration')
     end
 
     # The "internal admin" is a special body for internal use.
@@ -379,10 +380,6 @@ class PublicBody < ActiveRecord::Base
         end
     end
 
-    def site_administration?
-        has_tag?('site_administration')
-    end
-
     class ImportCSVDryRun < StandardError
     end
 
diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb
index d8c0c59b1..1ead1e0bf 100644
--- a/spec/models/info_request_spec.rb
+++ b/spec/models/info_request_spec.rb
@@ -28,6 +28,117 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
 
 describe InfoRequest do
 
+    describe :new do
+
+        it 'sets the default law used' do
+            expect(InfoRequest.new().law_used).to eq('foi')
+        end
+
+        it 'sets the default law used if a body is eir-only' do
+            body = FactoryGirl.create(:public_body, :tag_string => 'eir_only')
+            expect(body.info_requests.build.law_used).to eq('eir')
+        end
+
+        it 'does not try to set the law used for existing requests' do
+            info_request = FactoryGirl.create(:info_request)
+            body = FactoryGirl.create(:public_body, :tag_string => 'eir_only')
+            info_request.update_attributes(:public_body_id => body.id)
+            InfoRequest.any_instance.should_not_receive(:law_used=).and_call_original
+            InfoRequest.find(info_request.id)
+        end
+    end
+
+    describe :move_to_public_body do
+
+        context 'with no options' do
+
+          it 'requires an :editor option' do
+              request = FactoryGirl.create(:info_request)
+              new_body = FactoryGirl.create(:public_body)
+              expect {
+                  request.move_to_public_body(new_body)
+              }.to raise_error IndexError
+          end
+
+        end
+
+        context 'with the :editor option' do
+
+          it 'moves the info request to the new public body' do
+              request = FactoryGirl.create(:info_request)
+              new_body = FactoryGirl.create(:public_body)
+              user = FactoryGirl.create(:user)
+              request.move_to_public_body(new_body, :editor => user)
+              request.reload
+              expect(request.public_body).to eq(new_body)
+          end
+
+          it 'logs the move' do
+              request = FactoryGirl.create(:info_request)
+              old_body = request.public_body
+              new_body = FactoryGirl.create(:public_body)
+              user = FactoryGirl.create(:user)
+              request.move_to_public_body(new_body, :editor => user)
+              request.reload
+              event = request.info_request_events.last
+
+              expect(event.event_type).to eq('move_request')
+              expect(event.params[:editor]).to eq(user)
+              expect(event.params[:public_body_url_name]).to eq(new_body.url_name)
+              expect(event.params[:old_public_body_url_name]).to eq(old_body.url_name)
+          end
+
+          it 'updates the law_used to the new body law' do
+              request = FactoryGirl.create(:info_request)
+              new_body = FactoryGirl.create(:public_body, :tag_string => 'eir_only')
+              user = FactoryGirl.create(:user)
+              request.move_to_public_body(new_body, :editor => user)
+              request.reload
+              expect(request.law_used).to eq('eir')
+          end
+
+          it 'returns the new public body' do
+              request = FactoryGirl.create(:info_request)
+              new_body = FactoryGirl.create(:public_body)
+              user = FactoryGirl.create(:user)
+              expect(request.move_to_public_body(new_body, :editor => user)).to eq(new_body)
+          end
+
+          it 'retains the existing body if the new body does not exist' do
+              request = FactoryGirl.create(:info_request)
+              user = FactoryGirl.create(:user)
+              existing_body = request.public_body
+              request.move_to_public_body(nil, :editor => user)
+              request.reload
+              expect(request.public_body).to eq(existing_body)
+          end
+
+          it 'returns nil if the body cannot be updated' do
+              request = FactoryGirl.create(:info_request)
+              user = FactoryGirl.create(:user)
+              expect(request.move_to_public_body(nil, :editor => user)).to eq(nil)
+          end
+
+          it 'reindexes the info request' do
+              request = FactoryGirl.create(:info_request)
+              new_body = FactoryGirl.create(:public_body)
+              user = FactoryGirl.create(:user)
+              reindex_job = ActsAsXapian::ActsAsXapianJob.
+                where(:model => 'InfoRequestEvent').
+                  delete_all
+
+              request.move_to_public_body(new_body, :editor => user)
+              request.reload
+
+              reindex_job = ActsAsXapian::ActsAsXapianJob.
+                where(:model => 'InfoRequestEvent').
+                  last
+              expect(reindex_job.model_id).to eq(request.info_request_events.last.id)
+          end
+
+        end
+    end
+
     describe 'when validating' do
 
         it 'should accept a summary with ascii characters' do
@@ -42,7 +153,7 @@ describe InfoRequest do
             info_request.errors[:title].should be_empty
         end
 
-        it 'should not accept a summary with no ascii or unicode characters' do
+         it 'should not accept a summary with no ascii or unicode characters' do
             info_request = InfoRequest.new(:title => '55555')
             info_request.valid?
             info_request.errors[:title].should_not be_empty