diff options
| -rw-r--r-- | Gemfile | 1 | ||||
| -rw-r--r-- | Gemfile.lock | 5 | ||||
| -rw-r--r-- | app/controllers/admin_request_controller.rb | 19 | ||||
| -rw-r--r-- | app/models/info_request.rb | 27 | ||||
| -rw-r--r-- | app/models/info_request_event.rb | 8 | ||||
| -rw-r--r-- | app/models/post_redirect.rb | 6 | ||||
| -rw-r--r-- | app/models/public_body.rb | 19 | ||||
| -rw-r--r-- | config/application.rb | 3 | ||||
| -rw-r--r-- | config/environments/development.rb | 8 | ||||
| -rw-r--r-- | config/general.yml-example | 8 | ||||
| -rw-r--r-- | config/initializers/alaveteli.rb | 3 | ||||
| -rw-r--r-- | lib/configuration.rb | 3 | ||||
| -rw-r--r-- | lib/ruby19.rb | 8 | ||||
| -rw-r--r-- | locale/es_NI/app.po | 6 | ||||
| -rw-r--r-- | spec/models/info_request_event_spec.rb | 6 | ||||
| -rw-r--r-- | spec/models/info_request_spec.rb | 113 | ||||
| -rw-r--r-- | spec/models/post_redirect_spec.rb | 10 |
17 files changed, 209 insertions, 44 deletions
@@ -70,6 +70,7 @@ group :test do end group :test, :development do + gem 'bullet', '~> 4.14.6' gem 'factory_girl_rails', '~> 1.7' gem 'rspec-rails', '~> 2.13.2' gem 'spork-rails', '~> 3.2.1' diff --git a/Gemfile.lock b/Gemfile.lock index f401adfc4..9353b9145 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -60,6 +60,9 @@ GEM bootstrap-sass (2.3.1.2) sass (~> 3.2) builder (3.0.4) + bullet (4.14.6) + activesupport (>= 3.0.0) + uniform_notifier (~> 1.9.0) capistrano (2.15.4) highline net-scp (>= 1.0.0) @@ -285,6 +288,7 @@ GEM multi_json (~> 1.0, >= 1.0.2) unicode (0.4.4) unidecoder (1.1.2) + uniform_notifier (1.9.0) vpim (13.11.11) webrat (0.7.3) nokogiri (>= 1.2.0) @@ -302,6 +306,7 @@ DEPENDENCIES acts_as_versioned! annotate (~> 2.5.0) bootstrap-sass (~> 2.3.1.2) + bullet (~> 4.14.6) capistrano (~> 2.15.4) charlock_holmes (~> 0.6.9.4) coffee-rails (~> 3.2.1) 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/info_request_event.rb b/app/models/info_request_event.rb index 635ba8f58..0ee82d30c 100644 --- a/app/models/info_request_event.rb +++ b/app/models/info_request_event.rb @@ -278,9 +278,15 @@ class InfoRequestEvent < ActiveRecord::Base end self.params_yaml = params.to_yaml end + def params - YAML.load(self.params_yaml) + param_hash = YAML.load(params_yaml) + param_hash.each do |key, value| + param_hash[key] = value.force_encoding('UTF-8') if value.respond_to?(:force_encoding) + end + param_hash end + def params_yaml_as_html ret = '' # split out parameters into old/new diffs, and other ones diff --git a/app/models/post_redirect.rb b/app/models/post_redirect.rb index 8049349d0..59160381c 100644 --- a/app/models/post_redirect.rb +++ b/app/models/post_redirect.rb @@ -71,7 +71,11 @@ class PostRedirect < ActiveRecord::Base end def reason_params - YAML.load(reason_params_yaml) + param_hash = YAML.load(reason_params_yaml) + param_hash.each do |key, value| + param_hash[key] = value.force_encoding('UTF-8') if value.respond_to?(:force_encoding) + end + param_hash end # Extract just local path part, without domain or # 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/config/application.rb b/config/application.rb index 472077f06..46c4eb93b 100644 --- a/config/application.rb +++ b/config/application.rb @@ -60,7 +60,8 @@ module Alaveteli config.time_zone = ::AlaveteliConfiguration::time_zone # Set the cache to use a memcached backend - config.cache_store = :mem_cache_store, { :namespace => AlaveteliConfiguration::domain } + config.cache_store = :mem_cache_store, + { :namespace => "#{AlaveteliConfiguration::domain}_#{RUBY_VERSION}" } config.action_dispatch.rack_cache = nil config.after_initialize do |app| diff --git a/config/environments/development.rb b/config/environments/development.rb index dbf8d7b2a..b334a1e19 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -46,4 +46,12 @@ Alaveteli::Application.configure do # with SQLite, MySQL, and PostgreSQL) config.active_record.auto_explain_threshold_in_seconds = 0.5 + if AlaveteliConfiguration.use_bullet_in_development + config.after_initialize do + Bullet.enable = true + Bullet.bullet_logger = true + Bullet.console = true + Bullet.add_footer = true + end + end end diff --git a/config/general.yml-example b/config/general.yml-example index df140136c..a6980b71c 100644 --- a/config/general.yml-example +++ b/config/general.yml-example @@ -673,6 +673,14 @@ PUBLIC_BODY_LIST_FALLBACK_TO_DEFAULT_LOCALE: false # --- USE_MAILCATCHER_IN_DEVELOPMENT: true +# Bullet is a tool to help to kill N+1 queries and unused eager loading +# https://github.com/flyerhzm/bullet +# +# USE_BULLET_IN_DEVELOPMENT - Boolean (default: false) +# +# --- +USE_BULLET_IN_DEVELOPMENT: false + # Use memcached to cache HTML fragments for better performance. This will # only have an effect in environments where # config.action_controller.perform_caching is set to true diff --git a/config/initializers/alaveteli.rb b/config/initializers/alaveteli.rb index dbd7aace2..035d744b8 100644 --- a/config/initializers/alaveteli.rb +++ b/config/initializers/alaveteli.rb @@ -10,7 +10,7 @@ load "debug_helpers.rb" load "util.rb" # Application version -ALAVETELI_VERSION = '0.21.0.25' +ALAVETELI_VERSION = '0.21.0.26' # Add new inflection rules using the following format # (all these examples are active by default): @@ -35,7 +35,6 @@ end # Load monkey patches and other things from lib/ -require 'ruby19.rb' require 'activesupport_cache_extensions.rb' require 'use_spans_for_errors.rb' require 'activerecord_errors_extensions.rb' diff --git a/lib/configuration.rb b/lib/configuration.rb index c983152e0..89f148602 100644 --- a/lib/configuration.rb +++ b/lib/configuration.rb @@ -86,7 +86,8 @@ module AlaveteliConfiguration :USE_MAILCATCHER_IN_DEVELOPMENT => true, :UTILITY_SEARCH_PATH => ["/usr/bin", "/usr/local/bin"], :VARNISH_HOST => '', - :WORKING_OR_CALENDAR_DAYS => 'working' + :WORKING_OR_CALENDAR_DAYS => 'working', + :USE_BULLET_IN_DEVELOPMENT => false } end diff --git a/lib/ruby19.rb b/lib/ruby19.rb deleted file mode 100644 index 39f48d74e..000000000 --- a/lib/ruby19.rb +++ /dev/null @@ -1,8 +0,0 @@ -if RUBY_VERSION.to_f == 1.9 - class String - # @see syck/lib/syck/rubytypes.rb - def is_binary_data? - self.count("\x00-\x7F", "^ -~\t\r\n").fdiv(self.size) > 0.3 || self.index("\x00") unless self.empty? - end - end -end
\ No newline at end of file diff --git a/locale/es_NI/app.po b/locale/es_NI/app.po index 027562a91..e7b7eb330 100644 --- a/locale/es_NI/app.po +++ b/locale/es_NI/app.po @@ -19,8 +19,8 @@ msgstr "" "Project-Id-Version: alaveteli\n" "Report-Msgid-Bugs-To: \n" "POT-Creation-Date: 2015-03-24 10:21+0000\n" -"PO-Revision-Date: 2015-04-24 16:21+0000\n" -"Last-Translator: Gareth Rees <gareth@mysociety.org>\n" +"PO-Revision-Date: 2015-05-05 17:14+0000\n" +"Last-Translator: jbaezni <jbaezni@gmail.com>\n" "Language-Team: Spanish (Nicaragua) (http://www.transifex.com/projects/p/alaveteli/language/es_NI/)\n" "Language: es_NI\n" "MIME-Version: 1.0\n" @@ -3721,7 +3721,7 @@ msgid "Your request to update {{public_body_name}} on {{site_name}}" msgstr "Su solicitud para agregar {{public_body_name}} a {{site_name}}" msgid "Your request was called {{info_request}}. Letting everyone know whether you got the information will help us keep tabs on" -msgstr "Tu solicitud se llamaba {{info_request}}. Haznos saber si has recibido la información para ayudarnos a controlar a" +msgstr "Tu solicitud se llamaba {{info_request}}. Haznos saber si has recibido la información para ayudarnos a darle seguimiento a la misma" msgid "Your request:" msgstr "Tu solicitud:" diff --git a/spec/models/info_request_event_spec.rb b/spec/models/info_request_event_spec.rb index 53c83bd46..1299dfb63 100644 --- a/spec/models/info_request_event_spec.rb +++ b/spec/models/info_request_event_spec.rb @@ -30,6 +30,12 @@ describe InfoRequestEvent do ire.params.should == example_params end + it "should restore UTF8-heavy params stored under ruby 1.8 as UTF-8" do + ire = InfoRequestEvent.new + utf8_params = "--- \n:foo: !binary |\n 0KLQvtCz0LDRiCDR\n" + ire.params_yaml = utf8_params + ire.params[:foo].encoding.to_s.should == 'UTF-8' if ire.params[:foo].respond_to?(:encoding) + end end describe 'when deciding if it is indexed by search' do 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 diff --git a/spec/models/post_redirect_spec.rb b/spec/models/post_redirect_spec.rb index 73740e914..70b221f10 100644 --- a/spec/models/post_redirect_spec.rb +++ b/spec/models/post_redirect_spec.rb @@ -65,11 +65,19 @@ describe PostRedirect, " when accessing values" do end it "should convert reason parameters into YAML and back successfully" do - pr = PostRedirect.new + pr = PostRedirect.new example_reason_params = { :foo => 'this is stuff', :bar => 83, :humbug => "yikes!!!" } pr.reason_params = example_reason_params pr.reason_params_yaml.should == example_reason_params.to_yaml pr.reason_params.should == example_reason_params end + + it "should restore UTF8-heavy params stored under ruby 1.8 as UTF-8" do + pr = PostRedirect.new + utf8_params = "--- \n:foo: !binary |\n 0KLQvtCz0LDRiCDR\n" + pr.reason_params_yaml = utf8_params + puts pr.reason_params + pr.reason_params[:foo].encoding.to_s.should == 'UTF-8' if pr.reason_params[:foo].respond_to?(:encoding) + end end |