mirror of
https://github.com/discourse/discourse.git
synced 2025-08-17 18:04:11 +08:00
FEATURE: Localize topic titles and category names in crawler view (#34212)
If a site has `SiteSetting.content_localization_enabled = true`, we want to start serving localized content. This commit - affects only anon topic lists like /latest, /top, /categories. - fixes an existing N+1 when loading localized category names on topic lists - updates the locale query param from `?lang=ja` to `?tl=ja` | before | after | |---|---| | <img width="1264" height="831" alt="Screenshot 2025-08-11 at 3 18 09 PM" src="https://github.com/user-attachments/assets/52023fbc-1444-4a01-95a3-7d7376a8ffd3" /> mixed | <img width="1264" height="831" alt="Screenshot 2025-08-11 at 3 17 47 PM" src="https://github.com/user-attachments/assets/b6e6ca17-de90-409f-9251-eea6e9b4ce88" /> site default locale | | <img width="1264" height="831" alt="Screenshot 2025-08-11 at 3 23 31 PM" src="https://github.com/user-attachments/assets/d927b0d6-3a43-4ecb-8a9b-d1b97ae59cfc" /> | <img width="1264" height="831" alt="Screenshot 2025-08-11 at 3 23 42 PM" src="https://github.com/user-attachments/assets/d357b52b-396e-4f2d-bb47-7cc2485cf9ca" /> with lang=ja |
This commit is contained in:
parent
e229fa65a5
commit
b4526201ab
7 changed files with 138 additions and 6 deletions
|
@ -5,6 +5,10 @@ module Localizable
|
|||
|
||||
included { has_many :localizations, class_name: "#{model_name}Localization", dependent: :destroy }
|
||||
|
||||
# Returns the localization for the given locale, or the best match if an exact match is not found.
|
||||
# The query used to find the localization is optimized for performance, and assumes
|
||||
# that localizations are indexed by locale, and have been preloaded where necessary.
|
||||
# @return [Localization, nil] the localization object for the given locale, or nil if no match is found.
|
||||
def get_localization(locale = I18n.locale)
|
||||
locale_str = locale.to_s.sub("-", "_")
|
||||
|
||||
|
|
|
@ -133,10 +133,13 @@ class TopicList
|
|||
ft.topic_list = self
|
||||
end
|
||||
|
||||
category_associations = [:parent_category]
|
||||
category_associations << :localizations if SiteSetting.content_localization_enabled
|
||||
|
||||
topic_preloader_associations = [
|
||||
:image_upload,
|
||||
{ topic_thumbnails: :optimized_image },
|
||||
{ category: :parent_category },
|
||||
{ category: category_associations },
|
||||
]
|
||||
|
||||
topic_preloader_associations << :localizations if SiteSetting.content_localization_enabled
|
||||
|
|
|
@ -1714,7 +1714,7 @@ en:
|
|||
allow_user_locale: "Allow users to choose their own language interface preference"
|
||||
set_locale_from_accept_language_header: "Set interface language for anonymous users from their web browser's language headers"
|
||||
set_locale_from_cookie: "Allows setting an anonymous user's locale via the 'locale' browser cookie"
|
||||
set_locale_from_param: "Allows setting an anonymous user's locale via the 'lang' URL param, e.g. ?lang=es"
|
||||
set_locale_from_param: "Allows setting an anonymous user's locale via the 'tl' URL param, e.g. ?tl=es"
|
||||
support_mixed_text_direction: "Support mixed left-to-right and right-to-left text directions"
|
||||
min_post_length: "Minimum allowed post length in characters (excluding personal messages)"
|
||||
min_first_post_length: "Minimum allowed first post (topic body) length (excluding personal messages)"
|
||||
|
|
|
@ -9,6 +9,7 @@ require "git_utils"
|
|||
module Discourse
|
||||
DB_POST_MIGRATE_PATH = "db/post_migrate"
|
||||
MAX_METADATA_FILE_SIZE = 64.kilobytes
|
||||
LOCALE_PARAM = "tl"
|
||||
|
||||
class Utils
|
||||
URI_REGEXP = URI.regexp(%w[http https])
|
||||
|
@ -1219,7 +1220,7 @@ module Discourse
|
|||
end
|
||||
|
||||
def self.anonymous_locale(request)
|
||||
locale = request.params["lang"] if SiteSetting.set_locale_from_param
|
||||
locale = request.params[LOCALE_PARAM] if SiteSetting.set_locale_from_param
|
||||
locale ||= request.cookies["locale"] if SiteSetting.set_locale_from_cookie
|
||||
locale ||=
|
||||
request.env["HTTP_ACCEPT_LANGUAGE"] if SiteSetting.set_locale_from_accept_language_header
|
||||
|
|
|
@ -1,6 +1,5 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
# Helps us respond with a topic list from a controller
|
||||
module TopicListResponder
|
||||
def respond_with_list(list)
|
||||
discourse_expires_in 1.minute
|
||||
|
@ -8,6 +7,8 @@ module TopicListResponder
|
|||
respond_to do |format|
|
||||
format.html do
|
||||
@list = list
|
||||
localize_topic_list_content(list)
|
||||
|
||||
store_preloaded(
|
||||
list.preload_key,
|
||||
MultiJson.dump(TopicListSerializer.new(list, scope: guardian)),
|
||||
|
@ -17,4 +18,33 @@ module TopicListResponder
|
|||
format.json { render_serialized(list, TopicListSerializer) }
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def localize_topic_list_content(list)
|
||||
return if list.topics.blank? || !SiteSetting.content_localization_enabled
|
||||
crawl_locale = params[Discourse::LOCALE_PARAM].presence || SiteSetting.default_locale
|
||||
|
||||
list.topics.each { |topic| replace_topic_attributes(crawl_locale, topic) }
|
||||
end
|
||||
|
||||
def replace_topic_attributes(crawl_locale, topic)
|
||||
if topic.locale.present? && !LocaleNormalizer.is_same?(topic.locale, crawl_locale) &&
|
||||
(loc = topic.get_localization(crawl_locale))
|
||||
# assigning directly to title would commit the change to the database
|
||||
# due to the setter method defined in the Topic model
|
||||
topic.send(:write_attribute, :title, loc.title) if loc.title.present?
|
||||
topic.excerpt = loc.excerpt if loc.excerpt.present?
|
||||
|
||||
category = topic.category
|
||||
replace_category_name(category, crawl_locale)
|
||||
end
|
||||
end
|
||||
|
||||
def replace_category_name(category, crawl_locale)
|
||||
if category.locale.present? && !LocaleNormalizer.is_same?(category.locale, crawl_locale) &&
|
||||
(category_loc = category.get_localization(crawl_locale))
|
||||
category.name = category_loc.name if category_loc.name.present?
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1261,7 +1261,7 @@ RSpec.describe ApplicationController do
|
|||
|
||||
context "with an anonymous user" do
|
||||
it "uses the locale from the param" do
|
||||
get "/latest?lang=es"
|
||||
get "/latest?tl=es"
|
||||
expect(response.status).to eq(200)
|
||||
expect(main_locale_scripts(response.body)).to contain_exactly("es")
|
||||
expect(I18n.locale.to_s).to eq(SiteSettings::DefaultsProvider::DEFAULT_LOCALE) # doesn't leak after requests
|
||||
|
@ -1270,7 +1270,7 @@ RSpec.describe ApplicationController do
|
|||
|
||||
context "when the preferred locale includes a region" do
|
||||
it "returns the locale and region separated by an underscore" do
|
||||
get "/latest?lang=zh-CN"
|
||||
get "/latest?tl=zh-CN"
|
||||
expect(response.status).to eq(200)
|
||||
expect(main_locale_scripts(response.body)).to contain_exactly("zh_CN")
|
||||
end
|
||||
|
|
|
@ -1801,4 +1801,98 @@ RSpec.describe ListController do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
context "when content localization is enabled" do
|
||||
fab!(:category)
|
||||
|
||||
before do
|
||||
SiteSetting.content_localization_enabled = true
|
||||
|
||||
topic.update!(category:)
|
||||
end
|
||||
|
||||
describe "when tl param is absent" do
|
||||
fab!(:pt_topic) do
|
||||
Fabricate(
|
||||
:topic_localization,
|
||||
topic:,
|
||||
locale: "pt",
|
||||
title: "This is a localized portuguese title",
|
||||
)
|
||||
end
|
||||
fab!(:pt_category) do
|
||||
Fabricate(:category_localization, category:, locale: "pt", name: "Localized Category Name")
|
||||
end
|
||||
|
||||
it "localizes topic title for crawler to default locale when localization exists" do
|
||||
# topic is in english but default locale is portuguese
|
||||
topic.update!(locale: "en")
|
||||
topic.category.update!(locale: "en")
|
||||
SiteSetting.default_locale = "pt"
|
||||
|
||||
filter = Discourse.anonymous_filters[0]
|
||||
get "/#{filter}"
|
||||
|
||||
expect(response.body).to include(pt_topic.title)
|
||||
expect(response.body).to include(pt_category.name)
|
||||
end
|
||||
|
||||
it "leaves topic title as-is if no localization" do
|
||||
# no spanish localizations exist for the default locale spanish
|
||||
topic.update!(locale: "en")
|
||||
SiteSetting.default_locale = "es"
|
||||
|
||||
filter = Discourse.anonymous_filters[0]
|
||||
get "/#{filter}"
|
||||
|
||||
expect(response.body).to include(topic.title)
|
||||
expect(response.body).to include(category.name)
|
||||
expect(response.body).not_to include(pt_topic.title)
|
||||
expect(response.body).not_to include(pt_category.name)
|
||||
end
|
||||
end
|
||||
|
||||
describe "when tl param is present ?tl=ja" do
|
||||
fab!(:ja_topic) { Fabricate(:topic_localization, topic:, locale: "ja", title: "こんにちは世界") }
|
||||
fab!(:ja_category) do
|
||||
Fabricate(:category_localization, category:, locale: "ja", name: "カテゴリ名")
|
||||
end
|
||||
|
||||
before do
|
||||
topic.update!(locale: "en")
|
||||
topic.category.update!(locale: "en")
|
||||
end
|
||||
|
||||
it "localizes topic title for crawler" do
|
||||
get "/#{Discourse.anonymous_filters[0]}", params: { tl: "ja" }
|
||||
|
||||
expect(response.body).to include(ja_topic.title)
|
||||
expect(response.body).to include(ja_category.name)
|
||||
end
|
||||
end
|
||||
|
||||
it "should not have N+1s when loading localizations" do
|
||||
Fabricate.times(5, :topic, category:, locale: "en")
|
||||
Topic.all.each { |t| Fabricate(:topic_localization, topic: t, locale: "ja") }
|
||||
|
||||
initial_sql_queries =
|
||||
track_sql_queries do
|
||||
get "/#{Discourse.anonymous_filters[0]}", params: { tl: "ja" }
|
||||
expect(response.status).to eq(200)
|
||||
end.select { |q| q.include?("_localizations") }.count
|
||||
|
||||
new_category = Fabricate(:category, locale: "en")
|
||||
Fabricate(:category_localization, category: new_category, locale: "ja")
|
||||
new_topic = Fabricate(:topic, category: new_category, locale: "en")
|
||||
Fabricate(:topic_localization, topic: new_topic, locale: "ja")
|
||||
|
||||
new_sql_queries =
|
||||
track_sql_queries do
|
||||
get "/#{Discourse.anonymous_filters[0]}", params: { tl: "ja" }
|
||||
expect(response.status).to eq(200)
|
||||
end.select { |q| q.include?("_localizations") }.count
|
||||
|
||||
expect(new_sql_queries).to eq(initial_sql_queries)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue