diff options
| -rw-r--r-- | Gemfile | 1 | ||||
| -rw-r--r-- | Gemfile.lock | 1 | ||||
| -rw-r--r-- | app/controllers/admin_public_body_controller.rb | 5 | ||||
| -rw-r--r-- | app/controllers/public_body_controller.rb | 102 | ||||
| -rw-r--r-- | config/general.yml-example | 5 | ||||
| -rw-r--r-- | lib/configuration.rb | 1 | ||||
| -rw-r--r-- | spec/controllers/public_body_controller_spec.rb | 74 |
7 files changed, 158 insertions, 31 deletions
@@ -55,6 +55,7 @@ group :test do gem 'fakeweb' gem 'coveralls', :require => false gem 'webrat' + gem 'nokogiri' end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index 8d5059d7e..15c5245ff 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -270,6 +270,7 @@ DEPENDENCIES net-http-local net-purge newrelic_rpm + nokogiri pg rack rails (= 3.1.12) diff --git a/app/controllers/admin_public_body_controller.rb b/app/controllers/admin_public_body_controller.rb index ec2a08dbc..e0da234b0 100644 --- a/app/controllers/admin_public_body_controller.rb +++ b/app/controllers/admin_public_body_controller.rb @@ -14,6 +14,7 @@ class AdminPublicBodyController < AdminController def _lookup_query_internal @locale = self.locale_from_params() + underscore_locale = @locale.gsub '-', '_' I18n.with_locale(@locale) do @query = params[:query] if @query == "" @@ -23,10 +24,10 @@ class AdminPublicBodyController < AdminController if @page == "" @page = nil end - @public_bodies = PublicBody.joins(:translations).where(@query.nil? ? "public_body_translations.locale = '#{@locale}'" : + @public_bodies = PublicBody.joins(:translations).where(@query.nil? ? "public_body_translations.locale = '#{underscore_locale}'" : ["(lower(public_body_translations.name) like lower('%'||?||'%') or lower(public_body_translations.short_name) like lower('%'||?||'%') or - lower(public_body_translations.request_email) like lower('%'||?||'%' )) AND (public_body_translations.locale = '#{@locale}')", @query, @query, @query]).paginate :order => "public_body_translations.name", :page => @page, :per_page => 100 + lower(public_body_translations.request_email) like lower('%'||?||'%' )) AND (public_body_translations.locale = '#{underscore_locale}')", @query, @query, @query]).paginate :order => "public_body_translations.name", :page => @page, :per_page => 100 end @public_bodies_by_tag = PublicBody.find_by_tag(@query) end diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb index 9f692c5ba..b6dda825c 100644 --- a/app/controllers/public_body_controller.rb +++ b/app/controllers/public_body_controller.rb @@ -86,34 +86,45 @@ class PublicBodyController < ApplicationController def list long_cache # XXX move some of these tag SQL queries into has_tag_string.rb - @query = "%#{params[:public_body_query].nil? ? "" : params[:public_body_query]}%" + + like_query = params[:public_body_query] + like_query = "" if like_query.nil? + like_query = "%#{like_query}%" + @tag = params[:tag] - @locale = self.locale_from_params() - default_locale = I18n.default_locale.to_s - locale_condition = "(upper(public_body_translations.name) LIKE upper(?) - OR upper(public_body_translations.notes) LIKE upper (?)) - AND public_body_translations.locale = ? - AND public_bodies.id <> #{PublicBody.internal_admin_body.id}" + + @locale = self.locale_from_params + underscore_locale = @locale.gsub '-', '_' + underscore_default_locale = I18n.default_locale.to_s.gsub '-', '_' + + where_condition = "public_bodies.id <> #{PublicBody.internal_admin_body.id}" + where_parameters = [] + + first_letter = false + + base_tag_condition = " AND (SELECT count(*) FROM has_tag_string_tags" \ + " WHERE has_tag_string_tags.model_id = public_bodies.id" \ + " AND has_tag_string_tags.model = 'PublicBody'" + + # Restrict the public bodies shown according to the tag + # parameter supplied in the URL: if @tag.nil? or @tag == "all" @tag = "all" - conditions = [locale_condition, @query, @query, default_locale] elsif @tag == 'other' category_list = PublicBodyCategories::get().tags().map{|c| "'"+c+"'"}.join(",") - conditions = [locale_condition + ' AND (select count(*) from has_tag_string_tags where has_tag_string_tags.model_id = public_bodies.id - and has_tag_string_tags.model = \'PublicBody\' - and has_tag_string_tags.name in (' + category_list + ')) = 0', @query, @query, default_locale] + where_condition += base_tag_condition + " AND has_tag_string_tags.name in (#{category_list})) = 0" elsif @tag.size == 1 @tag.upcase! - conditions = [locale_condition + ' AND public_body_translations.first_letter = ?', @query, @query, default_locale, @tag] + # The first letter queries have to be done on + # translations, so just indicate to add that later: + first_letter = true elsif @tag.include?(":") name, value = HasTagString::HasTagStringTag.split_tag_into_name_value(@tag) - conditions = [locale_condition + ' AND (select count(*) from has_tag_string_tags where has_tag_string_tags.model_id = public_bodies.id - and has_tag_string_tags.model = \'PublicBody\' - and has_tag_string_tags.name = ? and has_tag_string_tags.value = ?) > 0', @query, @query, default_locale, name, value] + where_condition += base_tag_condition + " AND has_tag_string_tags.name = ? AND has_tag_string_tags.value = ?) > 0" + where_parameters.concat [name, value] else - conditions = [locale_condition + ' AND (select count(*) from has_tag_string_tags where has_tag_string_tags.model_id = public_bodies.id - and has_tag_string_tags.model = \'PublicBody\' - and has_tag_string_tags.name = ?) > 0', @query, @query, default_locale, @tag] + where_condition += base_tag_condition + " AND has_tag_string_tags.name = ?) > 0" + where_parameters.concat [@tag] end if @tag == "all" @@ -128,10 +139,45 @@ class PublicBodyController < ApplicationController @description = _("in the category ‘{{category_name}}’", :category_name=>category_name) end end + I18n.with_locale(@locale) do - @public_bodies = PublicBody.where(conditions).joins(:translations).order("public_body_translations.name").paginate( - :page => params[:page], :per_page => 100 - ) + + if AlaveteliConfiguration::public_body_list_fallback_to_default_locale + # Unfortunately, when we might fall back to the + # default locale, this is a rather complex query: + query = %Q{ + SELECT public_bodies.*, COALESCE(current_locale.name, default_locale.name) AS display_name + FROM public_bodies + LEFT OUTER JOIN public_body_translations as current_locale + ON (public_bodies.id = current_locale.public_body_id + AND current_locale.locale = ? AND #{get_public_body_list_translated_condition 'current_locale', first_letter}) + LEFT OUTER JOIN public_body_translations as default_locale + ON (public_bodies.id = default_locale.public_body_id + AND default_locale.locale = ? AND #{get_public_body_list_translated_condition 'default_locale', first_letter}) + WHERE #{where_condition} AND COALESCE(current_locale.name, default_locale.name) IS NOT NULL + ORDER BY display_name} + sql = [query, underscore_locale, like_query, like_query] + sql.push @tag if first_letter + sql += [underscore_default_locale, like_query, like_query] + sql.push @tag if first_letter + sql += where_parameters + @public_bodies = PublicBody.paginate_by_sql( + sql, + :page => params[:page], + :per_page => 100) + else + # The simpler case where we're just searching in the current locale: + where_condition = get_public_body_list_translated_condition('public_body_translations', first_letter, true) + + ' AND ' + where_condition + where_sql = [where_condition, like_query, like_query] + where_sql.push @tag if first_letter + where_sql += [underscore_locale] + where_parameters + @public_bodies = PublicBody.where(where_sql) \ + .joins(:translations) \ + .order("public_body_translations.name") \ + .paginate(:page => params[:page], :per_page => 100) + end + respond_to do |format| format.html { render :template => "public_body/list" } end @@ -236,4 +282,18 @@ class PublicBodyController < ApplicationController @xapian_requests = perform_search_typeahead(query, PublicBody) render :partial => "public_body/search_ahead" end + + private + def get_public_body_list_translated_condition(table, first_letter=false, locale=nil) + result = "(upper(#{table}.name) LIKE upper(?)" \ + " OR upper(#{table}.notes) LIKE upper (?))" + if first_letter + result += " AND #{table}.first_letter = ?" + end + if locale + result += " AND #{table}.locale = ?" + end + result + end + end diff --git a/config/general.yml-example b/config/general.yml-example index 37b0c2fc9..5f3697a36 100644 --- a/config/general.yml-example +++ b/config/general.yml-example @@ -200,3 +200,8 @@ PUBLIC_BODY_STATISTICS_PAGE: false # The page of statistics for public bodies will only consider public # bodies that have had at least this number of requests: MINIMUM_REQUESTS_FOR_STATISTICS: 50 + +# If only some of the public bodies have been translated into every +# available locale, you can allow a fallback to the default locale for +# listing of public bodies. +PUBLIC_BODY_LIST_FALLBACK_TO_DEFAULT_LOCALE: false diff --git a/lib/configuration.rb b/lib/configuration.rb index d6fd8765f..ab985c8bf 100644 --- a/lib/configuration.rb +++ b/lib/configuration.rb @@ -49,6 +49,7 @@ module AlaveteliConfiguration :NEW_RESPONSE_REMINDER_AFTER_DAYS => [3, 10, 24], :OVERRIDE_ALL_PUBLIC_BODY_REQUEST_EMAILS => '', :PUBLIC_BODY_STATISTICS_PAGE => false, + :PUBLIC_BODY_LIST_FALLBACK_TO_DEFAULT_LOCALE => false, :RAW_EMAILS_LOCATION => 'files/raw_emails', :READ_ONLY => '', :RECAPTCHA_PRIVATE_KEY => 'x', diff --git a/spec/controllers/public_body_controller_spec.rb b/spec/controllers/public_body_controller_spec.rb index 92130f3d0..2d1b1466f 100644 --- a/spec/controllers/public_body_controller_spec.rb +++ b/spec/controllers/public_body_controller_spec.rb @@ -1,6 +1,8 @@ # coding: utf-8 require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') +require 'nokogiri' + describe PublicBodyController, "when showing a body" do render_views @@ -78,24 +80,80 @@ describe PublicBodyController, "when listing bodies" do response.should be_success end - it "should list all bodies from default locale, even when there are no translations for selected locale" do - I18n.with_locale(:en) do - @english_only = PublicBody.new(:name => 'English only', - :short_name => 'EO', - :request_email => 'english@flourish.org', - :last_edit_editor => 'test', - :last_edit_comment => '') - @english_only.save + def make_single_language_example(locale) + result = nil + I18n.with_locale(locale) do + case locale + when :en + result = PublicBody.new(:name => 'English only', + :short_name => 'EO') + when :es + result = PublicBody.new(:name => 'Español Solamente', + :short_name => 'ES') + else + raise StandardError.new "Unknown locale #{locale}" + end + result.request_email = "#{locale}@example.org" + result.last_edit_editor = 'test' + result.last_edit_comment = '' + result.save end + result + end + + it "with no fallback, should only return bodies from the current locale" do + @english_only = make_single_language_example :en + @spanish_only = make_single_language_example :es + get :list, {:locale => 'es'} + assigns[:public_bodies].include?(@english_only).should == false + assigns[:public_bodies].include?(@spanish_only).should == true + end + + it "if fallback is requested, should list all bodies from default locale, even when there are no translations for selected locale" do + AlaveteliConfiguration.stub!(:public_body_list_fallback_to_default_locale).and_return(true) + @english_only = make_single_language_example :en get :list, {:locale => 'es'} assigns[:public_bodies].include?(@english_only).should == true end + it 'if fallback is requested, should still list public bodies only with translations in the current locale' do + AlaveteliConfiguration.stub!(:public_body_list_fallback_to_default_locale).and_return(true) + @spanish_only = make_single_language_example :es + get :list, {:locale => 'es'} + assigns[:public_bodies].include?(@spanish_only).should == true + end + + it "if fallback is requested, make sure that there are no duplicates listed" do + AlaveteliConfiguration.stub!(:public_body_list_fallback_to_default_locale).and_return(true) + get :list, {:locale => 'es'} + pb_ids = assigns[:public_bodies].map { |pb| pb.id } + unique_pb_ids = pb_ids.uniq + pb_ids.sort.should === unique_pb_ids.sort + end + it 'should show public body names in the selected locale language if present' do get :list, {:locale => 'es'} response.should contain('El Department for Humpadinking') end + it 'should not show the internal admin authority' do + PublicBody.internal_admin_body + get :list, {:locale => 'en'} + response.should_not contain('Internal admin authority') + end + + it 'should order on the translated name, even with the fallback' do + # The names of each public body is in: + # <span class="head"><a>Public Body Name</a></span> + # ... eo extract all of those, and check that they are ordered: + AlaveteliConfiguration.stub!(:public_body_list_fallback_to_default_locale).and_return(true) + get :list, {:locale => 'es'} + parsed = Nokogiri::HTML(response.body) + public_body_names = parsed.xpath '//span[@class="head"]/a/text()' + public_body_names = public_body_names.map { |pb| pb.to_s } + public_body_names.should == public_body_names.sort + end + it 'should show public body names in the selected locale language if present for a locale with underscores' do AlaveteliLocalization.set_locales('he_IL en', 'en') get :list, {:locale => 'he_IL'} |