Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#2482] Fix translated public body sort #2619

Merged
merged 3 commits into from
Aug 6, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 35 additions & 20 deletions app/controllers/public_body_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,24 +175,31 @@ def list
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:
if DatabaseCollation.supports?(underscore_locale)
select_sql = %Q(SELECT public_bodies.*, COALESCE(current_locale.name, default_locale.name) COLLATE "#{ underscore_locale }" AS display_name)
else
select_sql = %Q(SELECT public_bodies.*, COALESCE(current_locale.name, default_locale.name) AS display_name)
end

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, like_query]
sql.push @tag if first_letter
sql += [underscore_default_locale, like_query, like_query, like_query]
sql.push @tag if first_letter
sql += where_parameters
#{ select_sql }
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, like_query]
@sql.push @tag if first_letter
@sql += [underscore_default_locale, like_query, like_query, like_query]
@sql.push @tag if first_letter
@sql += where_parameters
@public_bodies = PublicBody.paginate_by_sql(
sql,
@sql,
:page => params[:page],
:per_page => 100)
else
Expand All @@ -202,10 +209,18 @@ def list
where_sql = [where_condition, like_query, 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)

if DatabaseCollation.supports?(underscore_locale)
@public_bodies = PublicBody.where(where_sql).
joins(:translations).
order(%Q(public_body_translations.name COLLATE "#{ underscore_locale }")).
paginate(:page => params[:page], :per_page => 100)
else
@public_bodies = PublicBody.where(where_sql).
joins(:translations).
order('public_body_translations.name').
paginate(:page => params[:page], :per_page => 100)
end
end

respond_to do |format|
Expand Down
1 change: 1 addition & 0 deletions config/initializers/alaveteli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
require 'public_body_csv'
require 'routing_filters'
require 'alaveteli_text_masker'
require 'database_collation'

AlaveteliLocalization.set_locales(AlaveteliConfiguration::available_locales,
AlaveteliConfiguration::default_locale)
Expand Down
4 changes: 4 additions & 0 deletions doc/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Highlighted Features

* The sorting on PublicBodyController#list now uses `COLLATE` to sort in the
correct order for a locale if a collation is available for the language. See
http://alaveteli.org/docs/developers/i18n/#internationalised-sorting for
adding collations. This requires PostgreSQL >= 9.1.12. (Gareth Rees)
* The new widget template can now be translated (Gareth Rees).
* Various design and markup improvements to the layout, home page and request
page (Martin Wright).
Expand Down
66 changes: 66 additions & 0 deletions lib/database_collation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# -*- encoding : utf-8 -*-
#
# Public: Class to check whether the current database supports collation for
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could do with an encoding line

# a given language. Prefer the class method .supports? rather than creating a
# new instance.
class DatabaseCollation
DEFAULT_CONNECTION = ActiveRecord::Base.connection
MINIMUM_POSTGRESQL_VERSION = 90112

attr_reader :connection

# Public: Does the connected database support collation in the given locale?
# Delegates to an instance configured with the DEFAULT_CONNECTION. See
# DatabaseCollation#supports? for more documentation.
def self.supports?(locale)
instance.supports?(locale)
end

def self.instance
@instance ||= new
end

def initialize(connection = DEFAULT_CONNECTION)
@connection = connection
end

# Public: Does the connected database support collation in the given locale?
#
# locale - String locale name
#
# Examples
#
# database.supports? 'en_GB'
# # => true
# database.supports? 'es'
# # => false
#
# Returns a Boolean
def supports?(locale)
exist? && supported_collations.include?(locale)
end

private

def exist?
postgresql? && postgresql_version >= MINIMUM_POSTGRESQL_VERSION
end

def postgresql?
adapter_name == 'PostgreSQL'
end

def postgresql_version
@postgresql_version ||= connection.send(:postgresql_version) if postgresql?
end

def supported_collations
@supported_collations ||= connection.
execute(%q(SELECT collname FROM pg_collation;)).
map { |row| row['collname'] }
end

def adapter_name
@adapter_name ||= connection.adapter_name
end
end
51 changes: 51 additions & 0 deletions spec/controllers/public_body_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,57 @@ def make_single_language_example(locale)
assigns[:description].should == ""
end

it 'list bodies in collate order according to the locale with the fallback set' do
AlaveteliConfiguration.stub(:public_body_list_fallback_to_default_locale).
and_return(true)

DatabaseCollation.stub(:supports?).
with(an_instance_of(String)).
and_return(true)

get :list, :locale => 'en_GB'
expect(assigns[:sql].to_s).to include('COLLATE')
end

it 'list bodies in default order according to the locale with the fallback set' do
AlaveteliConfiguration.stub(:public_body_list_fallback_to_default_locale).
and_return(true)

DatabaseCollation.stub(:supports?).
with(an_instance_of(String)).
and_return(false)

get :list, :locale => 'unknown'

expect(assigns[:sql].to_s).to_not include('COLLATE')
end

it 'list bodies in collate order according to the locale' do
AlaveteliConfiguration.stub(:public_body_list_fallback_to_default_locale).
and_return(false)

DatabaseCollation.stub(:supports?).
with(an_instance_of(String)).
and_return(true)

get :list, :locale => 'en_GB'

expect(assigns[:public_bodies].to_sql).to include('COLLATE')
end

it 'list bodies in alphabetical order according to the locale' do
AlaveteliConfiguration.stub(:public_body_list_fallback_to_default_locale).
and_return(false)

DatabaseCollation.stub(:supports?).
with(an_instance_of(String)).
and_return(false)

get :list, :locale => 'unknown'

expect(assigns[:public_bodies].to_sql).to_not include('COLLATE')
end

it "should support simple searching of bodies by title" do
get :list, :public_body_query => 'quango'
assigns[:public_bodies].should == [ public_bodies(:geraldine_public_body) ]
Expand Down
101 changes: 101 additions & 0 deletions spec/lib/database_collation_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# -*- encoding : utf-8 -*-
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')

describe DatabaseCollation do

describe '.supports?' do

it 'delegates to an instance of the class' do
collation = double
DatabaseCollation.stub(:instance).and_return(collation)
collation.should_receive(:supports?).with('en_GB')
DatabaseCollation.supports?('en_GB')
end

end

describe '.instance' do

it 'creates a new instance' do
expect(DatabaseCollation.instance).to be_instance_of(DatabaseCollation)
end

it 'caches the instance' do
expect(DatabaseCollation.instance).to equal(DatabaseCollation.instance)
end

it 'configures the instance with the default connection' do
expect(DatabaseCollation.instance.connection).
to equal(DatabaseCollation::DEFAULT_CONNECTION)
end

end

describe :new do

it 'defaults to the ActiveRecord::Base connection' do
expect(DatabaseCollation.new.connection).
to eq(ActiveRecord::Base.connection)
end

it 'allows a connection to be specified' do
mock_connection = double
expect(DatabaseCollation.new(mock_connection).connection).
to eq(mock_connection)
end

end

describe :supports? do

it 'does not support collation if the database is not postgresql' do
database = DatabaseCollation.
new(mock_connection(:adapter_name => 'MySQL'))
expect(database.supports?('en_GB')).to be_false
end

it 'does not support collation if the postgresql version is too old' do
database = DatabaseCollation.
new(mock_connection(:postgresql_version => 90111))
expect(database.supports?('en_GB')).to be_false
end

it 'does not support collation if the collation does not exist' do
database = DatabaseCollation.new(mock_connection)
expect(database.supports?('es')).to be_false
end

it 'supports collation if the collation exists' do
database = DatabaseCollation.new(mock_connection)
expect(database.supports?('en_GB')).to be_true
end

end

end

def mock_connection(connection_double_opts = {})
# Connection must be PostgreSQL 90112 or greater
default_double_opts = { :adapter_name => 'PostgreSQL',
:postgresql_version => 90112 }

connection_double_opts = default_double_opts.merge(connection_double_opts)

connection = double('ActiveRecord::FakeConnection', connection_double_opts)

installed_collations = [
{ "collname" => "default" },
{ "collname" => "C" },
{ "collname" => "POSIX" },
{ "collname" => "C.UTF-8" },
{ "collname" => "en_GB" },
{ "collname" => "en_GB.utf8" }
]

connection.
stub(:execute).
with(%q(SELECT collname FROM pg_collation;)).
and_return(installed_collations)

connection
end