diff options
| -rw-r--r-- | app/controllers/admin_request_controller.rb | 19 | ||||
| -rw-r--r-- | app/models/info_request.rb | 27 | ||||
| -rw-r--r-- | app/models/public_body.rb | 19 | ||||
| -rw-r--r-- | spec/models/info_request_spec.rb | 113 |
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 |