github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-06-07 09:04:41 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FIX: Include locale in extra-locales URLs (#31480)

Browse Source 
Previously the rendered locale was based on the current session's
locale. Now that we're routing requests via the CDN, we can't rely on
the user's session, and should instead include the locale name in the
URL. Also adds a `.js` suffix for parity with our other JS assets.
This commit is contained in:
David Taylor
2025-02-24 17:20:46 +00:00
committed by GitHub
parent 2062e4eb90
commit 1f5cce705c
4 changed files with 77 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
app/assets/javascripts/discourse/tests/index.html
View File

@ -54,9 +54,9 @@
<script src="{{rootURL}}assets/discourse.js"></script>
<script src="{{rootURL}}assets/locales/en.js" data-embroider-ignore></script>
<script src="{{rootURL}}extra-locales/0000000000000000000000000000000000000000/mf" data-embroider-ignore></script>
<script src="{{rootURL}}extra-locales/0000000000000000000000000000000000000000/admin" data-embroider-ignore></script>
<script src="{{rootURL}}extra-locales/0000000000000000000000000000000000000000/wizard" data-embroider-ignore></script>
<script src="{{rootURL}}extra-locales/0000000000000000000000000000000000000000/en/mf.js" data-embroider-ignore></script>
<script src="{{rootURL}}extra-locales/0000000000000000000000000000000000000000/en/admin.js" data-embroider-ignore></script>
<script src="{{rootURL}}extra-locales/0000000000000000000000000000000000000000/en/wizard.js" data-embroider-ignore></script>
<script src="{{rootURL}}assets/test-site-settings.js" data-embroider-ignore></script>
<script src="{{rootURL}}assets/admin.js" data-embroider-ignore></script>

35
app/controllers/extra_locales_controller.rb
View File

@ -23,38 +23,41 @@ class ExtraLocalesController < ApplicationController
@js_digests ||= { site_specific: {}, shared: {} }
end
def bundle_js_hash(bundle)
bundle_key = "#{bundle}_#{I18n.locale}"
def bundle_js_hash(bundle, locale:)
bundle_key = "#{bundle}_#{locale}"
if bundle.in?(SITE_SPECIFIC_BUNDLES)
site = RailsMultisite::ConnectionManagement.current_db
js_digests[:site_specific][site] ||= {}
js_digests[:site_specific][site][bundle_key] ||= begin
js = bundle_js(bundle)
js = bundle_js(bundle, locale: locale)
js.present? ? digest_for_content(js) : nil
end
elsif bundle.in?(SHARED_BUNDLES)
js_digests[:shared][bundle_key] ||= digest_for_content(bundle_js(bundle))
js_digests[:shared][bundle_key] ||= digest_for_content(bundle_js(bundle, locale: locale))
else
raise "Unknown bundle: #{bundle}"
end
end
def url(bundle)
def url(bundle, locale: I18n.locale)
hash = bundle_js_hash(bundle, locale:)
base = "#{GlobalSetting.cdn_url}#{Discourse.base_path}"
path = "/extra-locales/#{bundle_js_hash(bundle)}/#{bundle}"
path = "/extra-locales/#{hash}/#{locale}/#{bundle}.js"
query = SITE_SPECIFIC_BUNDLES.include?(bundle) ? "?__ws=#{Discourse.current_hostname}" : ""
"#{base}#{path}#{query}"
end
def client_overrides_exist?
bundle_js_hash(OVERRIDES_BUNDLE).present?
def client_overrides_exist?(locale: I18n.locale)
bundle_js_hash(OVERRIDES_BUNDLE, locale: locale).present?
end
def bundle_js(bundle)
locale_str = I18n.locale.to_s
def bundle_js(bundle, locale:)
locale_str = locale.to_s
bundle_str = "#{bundle}_js"
I18n.with_locale(locale) do
case bundle
when OVERRIDES_BUNDLE
JsLocaleHelper.output_client_overrides(locale_str)
@ -64,9 +67,10 @@ class ExtraLocalesController < ApplicationController
JsLocaleHelper.output_extra_locales(bundle_str, locale_str)
end
end
end
def bundle_js_with_hash(bundle)
js = bundle_js(bundle)
def bundle_js_with_hash(bundle, locale:)
js = bundle_js(bundle, locale: locale)
[js, digest_for_content(js)]
end
@ -82,14 +86,17 @@ class ExtraLocalesController < ApplicationController
def show
bundle = params[:bundle]
raise Discourse::InvalidAccess.new if !valid_bundle?(bundle)
raise Discourse::NotFound if !valid_bundle?(bundle)
locale = params[:locale]
raise Discourse::NotFound if !I18n.available_locales.include?(locale.to_sym)
digest = params[:digest]
if digest.present?
raise Discourse::InvalidParameters.new(:digest) unless digest.to_s.size == SHA1_HASH_LENGTH
end
content, hash = ExtraLocalesController.bundle_js_with_hash(bundle)
content, hash = ExtraLocalesController.bundle_js_with_hash(bundle, locale:)
immutable_for(1.year) if hash == digest
render plain: content, content_type: "application/javascript"

5
config/routes.rb
View File

@ -460,7 +460,10 @@ Discourse::Application.routes.draw do
get "email/unsubscribed" => "email#unsubscribed", :as => "email_unsubscribed"
post "email/unsubscribe/:key" => "email#perform_unsubscribe", :as => "email_perform_unsubscribe"
get "extra-locales/:digest/:bundle" => "extra_locales#show"
get "extra-locales/:digest/:locale/:bundle" => "extra_locales#show",
:constraints => {
format: :js,
}
resources :session, id: RouteFormat.username, only: %i[create destroy become] do
get "become" if !Rails.env.production?

76
spec/requests/extra_locales_controller_spec.rb
View File

@ -6,35 +6,35 @@ RSpec.describe ExtraLocalesController do
describe "#show" do
it "won't work with a weird parameter" do
get "/extra-locales/#{fake_digest}/-invalid..character!!"
get "/extra-locales/#{fake_digest}/en/-invalid..character!!.js"
expect(response.status).to eq(404)
end
it "needs a valid bundle" do
get "/extra-locales/#{fake_digest}/made-up-bundle"
expect(response.status).to eq(403)
get "/extra-locales/#{fake_digest}/en/made-up-bundle.js"
expect(response.status).to eq(404)
end
it "requires a valid digest" do
get "/extra-locales/ZZZ/overrides"
get "/extra-locales/ZZZ/en/overrides.js"
expect(response.status).to eq(400)
end
it "caches for 1 year if version is provided and it matches current hash" do
get "/extra-locales/#{ExtraLocalesController.bundle_js_hash("admin")}/admin"
get "/extra-locales/#{ExtraLocalesController.bundle_js_hash("admin", locale: :en)}/en/admin.js"
expect(response.status).to eq(200)
expect(response.headers["Cache-Control"]).to eq("max-age=31556952, public, immutable")
end
it "does not cache at all if version is invalid" do
get "/extra-locales/#{fake_digest}/admin"
get "/extra-locales/#{fake_digest}/en/admin.js"
expect(response.status).to eq(200)
expect(response.headers["Cache-Control"]).not_to include("max-age", "public", "immutable")
end
it "doesn’t generate the bundle twice" do
described_class.expects(:bundle_js).returns("JS").once
get "/extra-locales/#{fake_digest}/admin"
get "/extra-locales/#{fake_digest}/en/admin.js"
end
context "with plugin" do
@ -60,7 +60,7 @@ RSpec.describe ExtraLocalesController do
after { JsLocaleHelper.clear_cache! }
it "includes plugin translations" do
get "/extra-locales/#{fake_digest}/admin"
get "/extra-locales/#{fake_digest}/en/admin.js"
expect(response.status).to eq(200)
expect(response.body.include?("github_badges")).to eq(true)
end
@ -72,13 +72,13 @@ RSpec.describe ExtraLocalesController do
it "works for anonymous users" do
TranslationOverride.upsert!(I18n.locale, "js.some_key", "client-side translation")
get "/extra-locales/#{ExtraLocalesController.bundle_js_hash("overrides")}/overrides"
get "/extra-locales/#{ExtraLocalesController.bundle_js_hash("overrides", locale: :en)}/en/overrides.js"
expect(response.status).to eq(200)
expect(response.headers["Cache-Control"]).to eq("max-age=31556952, public, immutable")
end
it "returns nothing when there are not overridden translations" do
get "/extra-locales/#{fake_digest}/overrides"
get "/extra-locales/#{fake_digest}/en/overrides.js"
expect(response.status).to eq(200)
expect(response.body).to be_empty
end
@ -99,7 +99,7 @@ RSpec.describe ExtraLocalesController do
"{NUM_RESULTS, plural, one {1 result} other {many} }",
)
get "/extra-locales/#{fake_digest}/overrides"
get "/extra-locales/#{fake_digest}/en/overrides.js"
expect(response.status).to eq(200)
expect(response.body).to_not include("server.some_key", "server.some_MF")
@ -144,12 +144,7 @@ RSpec.describe ExtraLocalesController do
"{NUM_RESULTS, plural, one {1 result} other {many} }",
)
SiteSetting.allow_user_locale = true
user = Fabricate(:user, locale: :fr)
sign_in(user)
get "/extra-locales/#{fake_digest}/overrides"
expect(response.status).to eq(200)
get ExtraLocalesController.url("overrides", locale: :fr)
ctx = MiniRacer::Context.new
ctx.eval("I18n = {};")
@ -169,7 +164,7 @@ RSpec.describe ExtraLocalesController do
before { JsLocaleHelper.stubs(:output_MF).with("en").returns("MF_TRANSLATIONS") }
it "returns the translations properly" do
get "/extra-locales/#{fake_digest}/mf"
get ExtraLocalesController.url("mf")
expect(response.body).to eq("MF_TRANSLATIONS")
end
end
@ -177,28 +172,32 @@ RSpec.describe ExtraLocalesController do
describe ".bundle_js_hash" do
it "doesn't call bundle_js more than once for the same locale and bundle" do
I18n.locale = :de
ExtraLocalesController.expects(:bundle_js).with("admin").returns("admin_js DE").once
locale = :de
ExtraLocalesController.expects(:bundle_js).with("admin", locale:).returns("admin_js DE").once
expected_hash_de = Digest::SHA1.hexdigest("admin_js DE")
expect(ExtraLocalesController.bundle_js_hash("admin")).to eq(expected_hash_de)
expect(ExtraLocalesController.bundle_js_hash("admin")).to eq(expected_hash_de)
expect(ExtraLocalesController.bundle_js_hash("admin", locale:)).to eq(expected_hash_de)
expect(ExtraLocalesController.bundle_js_hash("admin", locale:)).to eq(expected_hash_de)
I18n.locale = :fr
ExtraLocalesController.expects(:bundle_js).with("admin").returns("admin_js FR").once
locale = :fr
ExtraLocalesController.expects(:bundle_js).with("admin", locale:).returns("admin_js FR").once
expected_hash_fr = Digest::SHA1.hexdigest("admin_js FR")
expect(ExtraLocalesController.bundle_js_hash("admin")).to eq(expected_hash_fr)
expect(ExtraLocalesController.bundle_js_hash("admin")).to eq(expected_hash_fr)
expect(ExtraLocalesController.bundle_js_hash("admin", locale:)).to eq(expected_hash_fr)
expect(ExtraLocalesController.bundle_js_hash("admin", locale:)).to eq(expected_hash_fr)
I18n.locale = :de
expect(ExtraLocalesController.bundle_js_hash("admin")).to eq(expected_hash_de)
locale = :de
expect(ExtraLocalesController.bundle_js_hash("admin", locale:)).to eq(expected_hash_de)
ExtraLocalesController.expects(:bundle_js).with("wizard").returns("wizard_js DE").once
ExtraLocalesController
.expects(:bundle_js)
.with("wizard", locale:)
.returns("wizard_js DE")
.once
expected_hash_de = Digest::SHA1.hexdigest("wizard_js DE")
expect(ExtraLocalesController.bundle_js_hash("wizard")).to eq(expected_hash_de)
expect(ExtraLocalesController.bundle_js_hash("wizard")).to eq(expected_hash_de)
expect(ExtraLocalesController.bundle_js_hash("wizard", locale:)).to eq(expected_hash_de)
expect(ExtraLocalesController.bundle_js_hash("wizard", locale:)).to eq(expected_hash_de)
end
end
@ -224,10 +223,10 @@ RSpec.describe ExtraLocalesController do
end
describe ".bundle_js_with_hash" do
before { described_class.stubs(:bundle_js).with("admin").returns("JS") }
before { described_class.stubs(:bundle_js).with("admin", locale: :en).returns("JS") }
it "returns both JS and its hash for a given bundle" do
expect(described_class.bundle_js_with_hash("admin")).to eq(
expect(described_class.bundle_js_with_hash("admin", locale: :en)).to eq(
["JS", Digest::SHA1.hexdigest("JS")],
)
end
@ -235,7 +234,9 @@ RSpec.describe ExtraLocalesController do
describe ".url" do
it "works" do
expect(ExtraLocalesController.url("admin")).to match(%r{\A/extra-locales/\h{40}\/admin\z})
expect(ExtraLocalesController.url("admin")).to match(
%r{\A/extra-locales/\h{40}/en/admin\.js\z},
)
end
it "includes subfolder path" do
@ -256,5 +257,12 @@ RSpec.describe ExtraLocalesController do
"https://cdn.example.com/extra-locales/",
)
end
it "includes locale correctly" do
expect(ExtraLocalesController.url("admin")).to include("/en/admin.js")
I18n.with_locale(:fr) do
expect(ExtraLocalesController.url("admin")).to include("/fr/admin.js")
end
end
end
end