Skip to content

Commit

Permalink
Merge branch '7786-remove-locale-prefix-urls' into develop
Browse files Browse the repository at this point in the history
  • Loading branch information
gbp committed Mar 27, 2024
2 parents d46be24 + 11cbe40 commit f78788d
Show file tree
Hide file tree
Showing 44 changed files with 238 additions and 303 deletions.
1 change: 0 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ gem 'gettext_i18n_rails', '~> 1.12.0'
gem 'gettext', '~> 3.4.9'
gem 'globalize', '~> 6.3.0'
gem 'locale', '~> 2.1.4'
gem 'routing-filter', '~> 0.7.0'
gem 'unicode', '~> 0.4.4'
gem 'unidecoder', '~> 1.1.0'
gem 'money', '~> 6.19.0'
Expand Down
4 changes: 0 additions & 4 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -446,9 +446,6 @@ GEM
rexml (3.2.6)
rolify (6.0.1)
rotp (6.2.2)
routing-filter (0.7.0)
actionpack (>= 6.1)
activesupport (>= 6.1)
rspec-activemodel-mocks (1.2.0)
activemodel (>= 3.0)
activesupport (>= 3.0)
Expand Down Expand Up @@ -646,7 +643,6 @@ DEPENDENCIES
redcarpet (~> 3.6.0)
redis (~> 4.8.1)
rolify (~> 6.0.1)
routing-filter (~> 0.7.0)
rspec-activemodel-mocks (~> 1.2.0)
rspec-rails (~> 6.1.2)
rubocop (~> 1.62.1)
Expand Down
28 changes: 20 additions & 8 deletions 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 @@ -72,25 +72,37 @@ def long_cache
def set_gettext_locale
params_locale = params[:locale]

if AlaveteliConfiguration.include_default_locale_in_urls == false
params_locale ||= AlaveteliLocalization.default_locale
end

if AlaveteliConfiguration.use_default_browser_language
browser_locale = request.env['HTTP_ACCEPT_LANGUAGE']
end

locale = AlaveteliLocalization.set_session_locale(
params_locale, session[:locale], cookies[:locale], browser_locale
params_locale, session[:locale], cookies[:locale], browser_locale,
AlaveteliLocalization.default_locale
)

# set the current locale to the requested_locale
session[:locale] = 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
session[:locale] = locale

# we need to set something other than StripEmptySessions::STRIPPABLE_KEYS
# otherwise the cookie will be striped from the response
session[:previous_locale] = current_session_locale
end

# ensure current user locale attribute is up-to-date
current_user.update_column(: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
6 changes: 6 additions & 0 deletions app/helpers/link_to_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,12 @@ def unhappy_url(info_request = nil, options = {})
end
end

def current_path_without_locale
unsafe_keys = %w[protocol host locale]
sanitized_params = params.reject { |k| unsafe_keys.include?(k) }.permit!
url_for(sanitized_params.merge(only_path: true))
end

def current_path_with_locale(locale)
unsafe_keys = %w[protocol host]
sanitized_params = params.reject { |k| unsafe_keys.include?(k) }.permit!
Expand Down
13 changes: 0 additions & 13 deletions config/general.yml-example
Original file line number Diff line number Diff line change
Expand Up @@ -180,19 +180,6 @@ DEFAULT_LOCALE: 'en'
# ---
USE_DEFAULT_BROWSER_LANGUAGE: true

# Normally, Alaveteli will put the locale into its URLs, like this
# www.example.com/en/body/list/all. If you don't want this behaviour whenever
# the locale is the default one, set INCLUDE_DEFAULT_LOCALE_IN_URLS to false.
#
# INCLUDE_DEFAULT_LOCALE_IN_URLS: Boolean (default: true)
#
# Examples:
#
# INCLUDE_DEFAULT_LOCALE_IN_URLS: false
#
# ---
INCLUDE_DEFAULT_LOCALE_IN_URLS: true

# Are authorities required to respond by law?
#
# AUTHORITY_MUST_RESPOND: Boolean (default: true)
Expand Down
2 changes: 0 additions & 2 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,4 @@ def matches?(request)
end
end
####

filter :conditionallyprependlocale
end
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}')
1 change: 1 addition & 0 deletions doc/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Highlighted Features

* Remove locale prefixes from URLs (Alexander Griffen, Graeme Porteous)
* Update Twitter/X logos and wording (Lucas Cumsille Montesinos)
* Fix missing headers when exporting Project data (Gareth Rees)
* Reduce amount of storage related background jobs (Graeme Porteous)
Expand Down
10 changes: 0 additions & 10 deletions lib/alaveteli_localization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,13 @@ def set_locales(available_locales, default_locale)

set_fast_gettext_locales(available, default)
set_i18n_locales(available, default)
set_conditionally_prepend_locale_locales(available, default)
end

def set_default_text_domain(name, repos)
FastGettext.add_text_domain name, type: :chain, chain: repos
FastGettext.default_text_domain = name
end

def set_default_locale_urls(include_default_locale_in_urls)
RoutingFilter::Locale.
include_default_locale = include_default_locale_in_urls
end

# rubocop:disable Naming/AccessorMethodName
def set_default_locale(locale)
locale = Locale.parse(locale)
Expand Down Expand Up @@ -95,10 +89,6 @@ def set_i18n_locales(available, default)
I18n.locale = I18n.default_locale = default.hyphenate.to_sym
end

def set_conditionally_prepend_locale_locales(available, _default)
RoutingFilter::Conditionallyprependlocale.locales = available.map(&:to_s)
end

# Parse String locales to Locale instances
def parse_locales(available_locales, default_locale)
[available_locales.to_s.split(/ /).map { |locale| Locale.parse(locale) },
Expand Down
4 changes: 0 additions & 4 deletions lib/alaveteli_localization/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ class Railtie < Rails::Railtie

I18n::Backend::Simple.send(:include, I18n::Backend::Fallbacks)

AlaveteliLocalization.set_default_locale_urls(
AlaveteliConfiguration.include_default_locale_in_urls
)

if Rails.version < '7.0.0' && Rails.env.development?
##
# Ideally the following would only be called in the `after_initialize`
Expand Down
1 change: 0 additions & 1 deletion lib/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ module AlaveteliConfiguration
FRONTPAGE_PUBLICBODY_EXAMPLES: '',
GA_CODE: '',
GEOIP_DATABASE: 'vendor/data/GeoLite2-Country.mmdb',
INCLUDE_DEFAULT_LOCALE_IN_URLS: true,
INCOMING_EMAIL_DOMAIN: 'localhost',
INCOMING_EMAIL_PREFIX: 'foi+',
INCOMING_EMAIL_SECRET: 'dummysecret',
Expand Down
50 changes: 0 additions & 50 deletions lib/routing_filters.rb
Original file line number Diff line number Diff line change
@@ -1,53 +1,3 @@
module RoutingFilter
class Conditionallyprependlocale < RoutingFilter::Locale
# Override core Locale filter not to prepend locale path segment
# when there's only one locale
def prepend_locale?(locale)
locale &&
AlaveteliLocalization.available_locales.length > 1 &&
(self.class.include_default_locale? || !default_locale?(locale))
end

# And override the generation logic to use FastGettext.locale
# rather than I18n.locale (the latter is what rails uses
# internally and may look like `en-US`, whereas the former is
# what FastGettext and other POSIX-based systems use, and will
# look like `en_US`
def around_generate(*args)
# this is because we might get a call like forum_topics_path(forum, topic, :locale => :en)
params = args.extract_options!
# extract the passed :locale option
locale = params.delete(:locale)
if locale.nil?
# default to underscore locale when locale is nil (could also be false)
locale = AlaveteliLocalization.locale
end
unless valid_locale?(locale)
# reset to no locale when locale is not valid
locale = nil
end
args << params

yield.tap do |result|
next unless prepend_locale?(locale)

result.update prepend_segment(result.url, locale)
end
end

def default_locale?(locale)
AlaveteliLocalization.default_locale?(locale)
end

# Reset the locale pattern when the locales are set.
class << self
def locales_pattern
%r(^/(#{locales.map { |l| Regexp.escape(l.to_s) }.join('|')})(?=/|$))
end
end
end
end

ActionDispatch::Routing::RouteSet::NamedRouteCollection::UrlHelper.class_eval do
def self.optimize_helper?(_route)
false
Expand Down
1 change: 0 additions & 1 deletion script/install-as-user
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ then
-e "s,^( *TIME_ZONE:).*,\\1 'Europe/London'," \
-e "s,^( *BLOG_FEED:).*,\\1 ''," \
-e "s,^( *TWITTER_USERNAME:).*,\\1 ''," \
-e "s,^( *INCLUDE_DEFAULT_LOCALE_IN_URLS:).*,\\1 false," \
-e "s,^( *INCOMING_EMAIL_DOMAIN:).*,\\1 '$HOST'," \
-e "s,^( *INCOMING_EMAIL_PREFIX:).*,\\1 'foi+'," \
-e "s,^( *INCOMING_EMAIL_SECRET:).*,\\1 '$RANDOM_EMAIL_SECRET'," \
Expand Down
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
Loading

0 comments on commit f78788d

Please sign in to comment.