Skip to content

Commit 65ff349

Browse files
HammsHamms
authored
Revert "refactor(i18n): improve locale handling (#72511)" (#72639)
This reverts commit 1bdc8ee.
1 parent 1bdc8ee commit 65ff349

23 files changed

Lines changed: 271 additions & 197 deletions

‎dashboard/app/controllers/application_controller.rb‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ class ApplicationController < ActionController::Base
2323

2424
before_action :setup_i18n_tracking
2525

26+
around_action :with_locale
27+
2628
before_action :fix_crawlers_with_bad_accept_headers
2729

2830
before_action :clear_sign_up_session_vars
@@ -235,6 +237,10 @@ def allow_cdo_cors
235237
Thread.current[:current_request_url] = request.url
236238
end
237239

240+
protected def with_locale(&block)
241+
I18n.with_locale(request.locale, &block)
242+
end
243+
238244
protected def milestone_response(options)
239245
response = {
240246
timestamp: DateTime.now.to_milliseconds

‎dashboard/app/controllers/home_controller.rb‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
require 'cdo/i18n'
21
require 'census_helper'
32
require_dependency 'queries/school_info'
43
require_dependency 'queries/script_activity'
@@ -32,7 +31,7 @@ def set_locale
3231
if params[:locale]
3332
redirect_uri = URI(redirect_path)
3433
redirect_params = URI.decode_www_form(redirect_uri.query.to_s).to_h
35-
redirect_params[Cdo::I18n::LOCALE_PARAM_KEY] = params[:locale]
34+
redirect_params[VarnishEnvironment::LOCALE_PARAM_KEY] = params[:locale]
3635
# Query parameter for browser cache to be avoided and load new locale
3736
redirect_params['lang'] = params[:locale].split('|').first
3837
redirect_uri.query = URI.encode_www_form(redirect_params)

‎dashboard/app/controllers/omniauth_callbacks_controller.rb‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,10 @@ def login
378378

379379
private def prepare_locale_cookie(user)
380380
# Set user-account locale only if no cookie is already set.
381-
if user.locale && user.locale != I18n.locale.to_s && cookies[:language_].nil?
381+
if user.locale &&
382+
user.locale != request.env['cdo.locale'] &&
383+
cookies[:language_].nil?
384+
382385
set_locale_cookie(user.locale)
383386
end
384387
end

‎dashboard/app/controllers/test_controller.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ def assign_section_to_course_and_unit
181181
end
182182

183183
def get_i18n_t
184-
locale = params[:locale] || I18n.locale.to_s
184+
locale = params[:locale] || request.env['cdo.locale']
185185
render plain: I18n.t(params.require(:key), locale: locale)
186186
end
187187

‎dashboard/config/application.rb‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
require 'rails/all'
44

55
require 'cdo/geocoder'
6+
require 'varnish_environment'
67
require_relative '../legacy/middleware/files_api'
78
require_relative '../legacy/middleware/channels_api'
89
require 'shared_resources'
@@ -98,9 +99,9 @@ class Application < Rails::Application
9899
config.middleware.insert_before ActionDispatch::Static, ::Rack::Optimize
99100
end
100101

101-
config.middleware.insert_after Rails::Rack::Logger, Middleware::I18n
102-
config.middleware.insert_after Middleware::I18n, Middleware::GlobalEdition
103-
config.middleware.insert_after Middleware::I18n, FilesApi
102+
config.middleware.insert_after Rails::Rack::Logger, VarnishEnvironment
103+
config.middleware.insert_after VarnishEnvironment, Middleware::GlobalEdition
104+
config.middleware.insert_after VarnishEnvironment, FilesApi
104105

105106
config.middleware.insert_after FilesApi, ChannelsApi
106107
config.middleware.insert_after ChannelsApi, SharedResources

‎dashboard/lib/middleware/global_edition.rb‎

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@
22

33
require 'omniauth'
44
require 'request_store'
5-
require 'i18n'
65

76
require 'cdo/global_edition'
8-
require 'cdo/i18n'
97
require 'dynamic_config/dcdo'
8+
require 'helpers/cookies'
109

1110
module Middleware
1211
class GlobalEdition
@@ -15,12 +14,13 @@ def initialize(app)
1514
end
1615

1716
def call(env)
18-
RequestGlobalizer.new(@app, env).call
17+
RouteHandler.new(@app, env).call
1918
end
2019

21-
class RequestGlobalizer
20+
class RouteHandler
21+
include Middleware::Helpers::Cookies
22+
2223
REGION_KEY = Cdo::GlobalEdition::REGION_KEY
23-
LOCALE_KEY = Cdo::I18n::LOCALE_COOKIE_KEY
2424

2525
# HTTP paths that to be excluded from Global Edition scope.
2626
EXCLUDED_PATHS = [
@@ -45,7 +45,7 @@ def initialize(app, env)
4545
@original_path_info = @request.path_info
4646
@original_path = @request.path
4747
@original_region = @request.cookies[REGION_KEY].presence
48-
@original_locale = ::I18n.locale.to_s
48+
@original_locale = @request.locale
4949
end
5050

5151
# Processes the current request within the Global Edition routing context.
@@ -81,14 +81,14 @@ def call
8181
return app.call(env) unless Cdo::GlobalEdition.target_host?(request.hostname)
8282

8383
# Allows setting the GE region via the URL parameter `?ge_region=<region_code>`.
84-
if request.GET.key?(REGION_KEY)
85-
new_region = request.GET[REGION_KEY].presence
84+
if request.params.key?(REGION_KEY)
85+
new_region = request.params[REGION_KEY].presence
8686

8787
redirect_path = ::File.join('/', main_path)
8888
redirect_path = regional_path_for(new_region, redirect_path) if Cdo::GlobalEdition.region_available?(new_region)
8989

9090
redirect_uri = URI(redirect_path)
91-
redirect_uri.query = URI.encode_www_form(request.GET.except(REGION_KEY)).presence
91+
redirect_uri.query = URI.encode_www_form(request.params.except(REGION_KEY)).presence
9292
redirect_path = redirect_uri.to_s
9393

9494
setup_region(new_region)
@@ -122,9 +122,10 @@ def call
122122
response.finish
123123
ensure
124124
Cdo::GlobalEdition.current_region = nil
125-
# GE may rewrite Rack routing state (`script_name`, `path_info`) to route `/<region>/...` as root paths.
126-
# Rack computes `request.path` from both fields (`script_name + path_info`), so we must restore both values.
127-
# If only one side is reverted, upstream middleware can observe an invalid path (for example, a duplicated GE prefix).
125+
# Restore the original `script_name` and `path_info` so that upstream middlewares
126+
# (e.g., VarnishEnvironment's after filter) see a consistent `request.path`.
127+
# Without this, downstream processing may partially restore `path_info` while
128+
# leaving `script_name` modified, causing a doubled GE prefix in `request.path`.
128129
request.script_name = original_script_name
129130
request.path_info = original_path_info
130131
end
@@ -187,23 +188,19 @@ def call
187188
end
188189

189190
private def setup_region(new_region)
191+
return if new_region == original_region
192+
190193
# Resets the region if it's `nil` or sets it only if it's available.
191194
return unless new_region.nil? || Cdo::GlobalEdition.region_available?(new_region)
192195

193-
Cdo::GlobalEdition.current_region = new_region
194-
return if new_region == original_region
196+
# Sets the request cookies to apply changes immediately without needing to reload the page.
197+
request.cookies[REGION_KEY] = Cdo::GlobalEdition.current_region = new_region
198+
request.cookies[LOCALE_KEY] = request.locale = new_locale = resolve_locale_for(new_region)
195199

196200
# Updates the global `ge_region` cookie to lock the platform to the regional version.
197-
request.cookies[REGION_KEY] = new_region
198-
response.set_cdo_cookie(REGION_KEY, new_region, priority: :high)
199-
200-
region_locale = resolve_locale_for(new_region)
201-
unless region_locale == original_locale
202-
# Updates the global `language` cookie to enforce the switch to the regional language.
203-
::I18n.locale = region_locale
204-
request.cookies[LOCALE_KEY] = region_locale
205-
response.set_cdo_cookie(LOCALE_KEY, region_locale)
206-
end
201+
set_global_cookie(REGION_KEY, new_region, high_priority: true)
202+
# Updates the global `language` cookie to enforce the switch to the regional language.
203+
set_locale_cookie(new_locale)
207204

208205
Metrics::Events.log_event(
209206
event_name: 'Global Edition Region Changed',
@@ -213,9 +210,11 @@ def call
213210
old_region: original_region,
214211
old_locale: original_locale,
215212
new_region:,
216-
new_locale: region_locale,
213+
new_locale:,
217214
},
218215
)
216+
ensure
217+
Cdo::GlobalEdition.current_region = request.cookies[REGION_KEY].presence
219218
end
220219

221220
private def existing_route?(path = original_path_info)
@@ -232,12 +231,18 @@ def call
232231
# @return [String, nil] The resolved locale for the region,
233232
# or `nil` if no locale should be preserved during a region reset.
234233
private def resolve_locale_for(region)
235-
return original_locale unless region
234+
resolved_locale = original_locale
236235

237-
return original_locale if Cdo::GlobalEdition.locale_available?(region, original_locale)
238-
return url_locale if Cdo::GlobalEdition.locale_available?(region, url_locale)
236+
if Cdo::GlobalEdition.region_available?(region)
237+
unless Cdo::GlobalEdition.locale_available?(region, resolved_locale)
238+
resolved_locale = url_locale || Cdo::GlobalEdition.main_region_locale(region)
239+
end
240+
else
241+
# Locales locked to a specific region should not be set during a region reset.
242+
resolved_locale = nil if Cdo::GlobalEdition.locales_regions[resolved_locale].present?
243+
end
239244

240-
Cdo::GlobalEdition.main_region_locale(region)
245+
resolved_locale
241246
end
242247

243248
private def excluded_path?(path)
@@ -319,6 +324,6 @@ def call
319324
end
320325
end
321326

322-
private_constant :RequestGlobalizer
327+
private_constant :RouteHandler
323328
end
324329
end

‎dashboard/lib/middleware/i18n.rb‎

Lines changed: 0 additions & 101 deletions
This file was deleted.

‎dashboard/test/controllers/home_controller_test.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,10 @@ class HomeControllerTest < ActionController::TestCase
196196
assert_redirected_to '/users/sign_in'
197197
end
198198

199-
test "language is determined from I18n.locale" do
199+
test "language is determined from cdo.locale" do
200200
skip 'TODO: get :home, and look for a div that still exists'
201201

202-
I18n.locale = "es-ES"
202+
@request.env['cdo.locale'] = "es-ES"
203203

204204
get :index
205205

‎dashboard/test/controllers/sessions_controller_test.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ class SessionsControllerTest < ActionController::TestCase
88
@request.env["devise.mapping"] = Devise.mappings[:user]
99
end
1010

11-
test 'login error derives locale from I18n.locale' do
11+
test 'login error derives locale from cdo.locale' do
1212
locale = 'es-ES'
13-
I18n.locale = locale
13+
@request.env['cdo.locale'] = locale
1414
post :create
1515
assert_select '.alert', I18n.t('devise.failure.not_found_in_database', locale: locale)
1616
end

‎dashboard/test/integration/global_edition_test.rb‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,6 @@ class GlobalEditionTest < ActionDispatch::IntegrationTest
210210
let(:new_locale) {'en-US'}
211211

212212
before do
213-
cookies[:ge_region] = ge_region
214-
cookies[:language_] = ge_region_locale
215-
216213
params.merge!(extra_params)
217214
end
218215

@@ -227,7 +224,7 @@ class GlobalEditionTest < ActionDispatch::IntegrationTest
227224
new_region: nil,
228225
new_locale:,
229226
}
230-
)
227+
).once
231228

232229
get_regional_page
233230

@@ -244,7 +241,7 @@ class GlobalEditionTest < ActionDispatch::IntegrationTest
244241
must_respond_with 200
245242
_(request.fullpath).must_equal "#{international_page_path}?#{extra_params.to_query}"
246243

247-
_(ge_region_html_data).must_be_nil
244+
_(request.locale).must_equal new_locale
248245
_(cookies[:language_]).must_equal new_locale
249246
_(page_lang).must_equal new_locale
250247
end

0 commit comments

Comments
 (0)