Skip to content

Commit

Permalink
Add locale URL redirection
Browse files Browse the repository at this point in the history
When processing requests for URLs where the locale is a prefix, we now:
1. Redirect to the same URL with the locale as a parameter, this is done
   via routing redirects.

2. Set the locale in the session. This is done in the existing
   `set_gettext_locale` before action callback.

3. Finally, we redirect to the same URL without the locale at all as
   this information is retrieved from the session or cookies.

The redirect is skipped for the `PublicBodyController#show` action as
there is already redirection in place there to redirect to localised
version of the `PublicBody#url_name`.
  • Loading branch information
alexander-griffen authored and gbp committed Mar 13, 2024
1 parent 510516a commit 8c60666
Show file tree
Hide file tree
Showing 11 changed files with 148 additions and 120 deletions.
10 changes: 9 additions & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class PermissionDenied < StandardError
class RouteNotFound < StandardError
end

before_action :set_gettext_locale
before_action :set_gettext_locale, :redirect_gettext_locale
before_action :collect_locales

protect_from_forgery if: :authenticated?, with: :exception
Expand Down Expand Up @@ -81,6 +81,9 @@ def set_gettext_locale
AlaveteliLocalization.default_locale
)

# set response header informing the browser what language the page is in
response.headers['Content-Language'] = I18n.locale.to_s

# set the current stored locale to the requested_locale
current_session_locale = session[:locale]
if current_session_locale != locale
Expand All @@ -95,6 +98,11 @@ def set_gettext_locale
current_user.update_columns(locale: locale) if current_user
end

def redirect_gettext_locale
# redirect to remove locale params if present
redirect_to current_path_without_locale if params[:locale]
end

# Help work out which request causes RAM spike.
# http://www.codeweblog.com/rails-to-monitor-the-process-of-memory-leaks-skills/
# This shows the memory use increase of the Ruby process due to the request.
Expand Down
1 change: 1 addition & 0 deletions app/controllers/public_body_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

class PublicBodyController < ApplicationController
skip_before_action :html_response, only: [:show, :list_all_csv]
skip_before_action :redirect_gettext_locale, only: :show

MAX_RESULTS = 500
# TODO: tidy this up with better error messages, and a more standard infrastructure for the redirect to canonical URL
Expand Down
21 changes: 21 additions & 0 deletions config/routes/redirects.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,24 @@
get '/:prefix/request/:url_title',
constraints: { prefix: 'categorise' },
to: info_request_redirect

locale_constraint = ->(request) do
path_locale = request.path.split('/')[1]
begin
locale = AlaveteliLocalization::Locale.parse(path_locale).canonicalize
available = AlaveteliConfiguration.available_locales.split(' ').map do |l|
AlaveteliLocalization::Locale.parse(l).canonicalize
end
available.include?(locale)
rescue ArgumentError
false
end
end

get ':locale',
constraints: locale_constraint,
to: redirect('?locale=%{locale}')

get ':locale/*path',
constraints: locale_constraint,
to: redirect('%{path}?locale=%{locale}')
6 changes: 3 additions & 3 deletions spec/controllers/admin_public_body_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
end

it "searches for 'humpa' in another locale" do
get :index, params: { query: "humpa", locale: "es" }
get :index, params: { query: "humpa" }, session: { locale: "es" }
expect(assigns[:public_bodies]).
to eq([public_bodies(:humpadink_public_body)])
end
Expand Down Expand Up @@ -45,7 +45,7 @@
public_body.save!
end
sign_in admin_user
get :show, params: { id: public_body.id, locale: "es" }
get :show, params: { id: public_body.id }, session: { locale: 'es' }
expect(assigns[:public_body].name).to eq 'El Public Body'
end

Expand Down Expand Up @@ -329,7 +329,7 @@
end

it "edits a public body in another locale" do
get :edit, params: { id: 3, locale: :en }
get :edit, params: { id: 3 }, session: { locale: 'en' }

# When editing a body, the controller returns all available translations
expect(assigns[:public_body].find_translation_by_locale("es").name).
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/general_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@

describe 'when using locales' do
it "should use our test PO files rather than the application one" do
get :frontpage, params: { locale: 'es' }
get :frontpage, session: { locale: 'es' }
expect(response.body).to match(/XOXO/)
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/help_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
end

it 'should render the locale-specific template if available' do
get :contact, params: { locale: 'es' }
get :contact, session: { locale: 'es' }
expect(response.body).to match('contáctenos theme one')
end
end
Expand Down
36 changes: 18 additions & 18 deletions spec/controllers/public_body_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,15 @@
end

it "should display the body using same locale as that used in url_name" do
get :show, params: { url_name: "edfh", view: 'all', locale: "es" }
get :show, params: { url_name: "edfh", view: 'all' },
session: { locale: 'es' }
expect(response.body).to have_content("Baguette")
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 :show, params: { url_name: 'dfh',
view: 'all',
locale: 'he_IL' }
get :show, params: { url_name: 'dfh', view: 'all' },
session: { locale: 'he_IL' }
expect(response.body).to have_content('Hebrew Humpadinking')
end

Expand Down Expand Up @@ -147,7 +147,7 @@ def make_single_language_example(locale)
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, params: { locale: 'es' }
get :list, session: { locale: 'es' }
expect(assigns[:public_bodies].include?(@english_only)).to eq(false)
expect(assigns[:public_bodies].include?(@spanish_only)).to eq(true)
end
Expand All @@ -157,7 +157,7 @@ def make_single_language_example(locale)
to receive(:public_body_list_fallback_to_default_locale).
and_return(true)
@english_only = make_single_language_example :en
get :list, params: { locale: 'es' }
get :list, session: { locale: 'es' }
expect(assigns[:public_bodies].include?(@english_only)).to eq(true)
end

Expand All @@ -166,36 +166,36 @@ def make_single_language_example(locale)
to receive(:public_body_list_fallback_to_default_locale).
and_return(true)
@spanish_only = make_single_language_example :es
get :list, params: { locale: 'es' }
get :list, session: { locale: 'es' }
expect(assigns[:public_bodies].include?(@spanish_only)).to eq(true)
end

it "if fallback is requested, make sure that there are no duplicates listed" do
allow(AlaveteliConfiguration).
to receive(:public_body_list_fallback_to_default_locale).
and_return(true)
get :list, params: { locale: 'es' }
get :list, session: { locale: 'es' }
pb_ids = assigns[:public_bodies].map(&:id)
unique_pb_ids = pb_ids.uniq
expect(pb_ids.sort).to eq(unique_pb_ids.sort)
end

it 'should show public body names in the selected locale language if present' do
get :list, params: { locale: 'es' }
get :list, session: { locale: 'es' }
expect(response.body).to have_content('El Department for Humpadinking')
end

it 'show public body names of the selected underscore locale language' do
AlaveteliLocalization.set_locales(available_locales='en en_GB',
default_locale='en')
@gb_only = make_single_language_example :en_GB
get :list, params: { locale: 'en_GB' }
get :list, session: { locale: 'en_GB' }
expect(response.body).to have_content(@gb_only.name)
end

it 'should not show the internal admin authority' do
PublicBody.internal_admin_body
get :list, params: { locale: 'en' }
get :list, session: { locale: 'en' }
expect(response.body).not_to have_content('Internal admin authority')
end

Expand All @@ -206,7 +206,7 @@ def make_single_language_example(locale)
allow(AlaveteliConfiguration).
to receive(:public_body_list_fallback_to_default_locale).
and_return(true)
get :list, params: { locale: 'es' }
get :list, session: { locale: 'es' }
parsed = Nokogiri::HTML(response.body)
public_body_names = parsed.xpath '//span[@class="head"]/a/text()'
public_body_names = public_body_names.map(&:to_s)
Expand All @@ -215,7 +215,7 @@ def make_single_language_example(locale)

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, params: { locale: 'he_IL' }
get :list, session: { locale: 'he_IL' }
expect(response.body).to have_content('Hebrew Humpadinking')
end

Expand Down Expand Up @@ -248,7 +248,7 @@ def make_single_language_example(locale)
with(an_instance_of(String)).
and_return(true)

get :list, params: { locale: 'en_GB' }
get :list, session: { locale: 'en_GB' }
expect(assigns[:public_bodies].to_sql).to include('COLLATE')
end

Expand All @@ -261,7 +261,7 @@ def make_single_language_example(locale)
with(an_instance_of(String)).
and_return(false)

get :list, params: { locale: 'unknown' }
get :list, session: { locale: 'unknown' }

expect(assigns[:public_bodies].to_sql).to_not include('COLLATE')
end
Expand All @@ -275,7 +275,7 @@ def make_single_language_example(locale)
with(an_instance_of(String)).
and_return(true)

get :list, params: { locale: 'en_GB' }
get :list, session: { locale: 'en_GB' }

expect(assigns[:public_bodies].to_sql).to include('COLLATE')
end
Expand All @@ -289,7 +289,7 @@ def make_single_language_example(locale)
with(an_instance_of(String)).
and_return(false)

get :list, params: { locale: 'unknown' }
get :list, session: { locale: 'unknown' }

expect(assigns[:public_bodies].to_sql).to_not include('COLLATE')
end
Expand Down Expand Up @@ -429,7 +429,7 @@ def make_single_language_example(locale)
FactoryBot.create(:public_body, name: "Åčçèñtéd Authority")
end

get :list, params: { tag: "å", locale: 'cs' }
get :list, params: { tag: "å" }, session: { locale: 'cs' }
expect(response).to render_template('list')
expect(assigns[:public_bodies]).to eq([ authority ])
expect(assigns[:tag]).to eq("Å")
Expand Down
4 changes: 3 additions & 1 deletion spec/integration/errors_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@

it 'should assign the locale for the general/exception_caught template' do
allow(InfoRequest).to receive(:find_by_url_title!).and_raise("An example error")
get "/request/example?locale=es"
get "/es"
follow_redirect!
get "/request/example"
expect(response).to render_template('general/exception_caught')
expect(response.body).to match('Lo sentimos, hubo un problema procesando esta página')
end
Expand Down

0 comments on commit 8c60666

Please sign in to comment.