From d5b328b193c50bd811271e1c1773367f0f3964b2 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 31 Oct 2024 13:18:34 +1000 Subject: [PATCH] DEV: Site setting keyword changes (#29486) This commit contains two changes to how our site setting keyword system works: 1. Crowdin, our translation provider, does not support YAML lists, so we are changing site setting keywords in server.en.yml to be pipe-separated (|) 2. It's unclear to translators what they are supposed to do with aliases of site settings where the name has changed, e.g. min_trust_level_for_here_mention. Instead of getting these as keywords from the yml file, we can discern these from SiteSettings::DeprecatedSettings automatically, and still use them for client-side search These changes should help improve the situation for translators. --- config/locales/server.en.yml | 38 ++----------------------- lib/site_setting_extension.rb | 30 ++++++++++++++++++- spec/lib/site_setting_extension_spec.rb | 30 +++++++++++++++++++ 3 files changed, 62 insertions(+), 36 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 4a3207c0722..f1b85e534ad 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2805,41 +2805,9 @@ en: invalid_search_ranking_weights: "Value is invalid for search_ranking_weights site setting. Example: '{0.1,0.2,0.3,1.0}'. Note that maximum value for each weight is 1.0." keywords: - anonymous_posting_allowed_groups: "anonymous_posting_min_trust_level" - here_mention_allowed_groups: "min_trust_level_for_here_mention" - shared_drafts_allowed_groups: "shared_drafts_min_trust_level" - approve_unless_allowed_groups: "approve_unless_trust_level" - approve_new_topics_unless_allowed_groups: "approve_new_topics_unless_trust_level" - email_in_allowed_groups: "email_in_min_trust" - edit_wiki_post_allowed_groups: "min_trust_to_edit_wiki_post" - uploaded_avatars_allowed_groups: "allow_uploaded_avatars" - create_topic_allowed_groups: "min_trust_to_create_topic" - edit_post_allowed_groups: "min_trust_to_edit_post" - flag_post_allowed_groups: "min_trust_to_flag_posts" - delete_all_posts_and_topics_allowed_groups: "tl4_delete_posts_and_topics" - user_card_background_allowed_groups: "min_trust_level_to_allow_user_card_background" - invite_allowed_groups: "min_trust_level_to_allow_invite" - ignore_allowed_groups: "min_trust_level_to_allow_ignore" - self_wiki_allowed_groups: "min_trust_to_allow_self_wiki" - create_tag_allowed_groups: "min_trust_to_create_tag" - send_email_messages_allowed_groups: "min_trust_to_send_email_messages" - skip_review_media_groups: "review_media_unless_trust_level" - embedded_media_allowed_groups: "min_trust_to_post_embedded_media" - post_links_allowed_groups: "min_trust_to_post_links" - user_api_key_allowed_groups: "min_trust_level_for_user_api_key" - tag_topic_allowed_groups: "min_trust_level_to_tag_topics" - profile_background_allowed_groups: "min_trust_level_to_allow_profile_background" - clean_up_inactive_users_after_days: - - "deactivated" - - "inactive" - - "unactivated" - purge_unactivated_users_grace_period_days: - - "deactivated" - - "inactive" - - "unactivated" - navigation_menu: - - "sidebar" - - "header dropdown" + clean_up_inactive_users_after_days: "deactivated|inactive|unactivated" + purge_unactivated_users_grace_period_days: "deactivated|inactive|unactivated" + navigation_menu: "sidebar|header dropdown" placeholder: discourse_connect_provider_secrets: diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index cd9e7c39cf7..567de4c65ed 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -143,6 +143,12 @@ module SiteSettingExtension @deprecated_settings ||= SiteSettings::DeprecatedSettings::SETTINGS.map(&:first).to_set end + def deprecated_setting_alias(setting_name) + SiteSettings::DeprecatedSettings::SETTINGS + .find { |setting| setting.second.to_s == setting_name.to_s } + &.first + end + def settings_hash result = {} @@ -302,7 +308,29 @@ module SiteSettingExtension end def keywords(setting) - Array.wrap(I18n.t("site_settings.keywords.#{setting}", default: "")) + translated_keywords = I18n.t("site_settings.keywords.#{setting}", default: "") + english_translated_keywords = [] + + if I18n.locale != :en + english_translated_keywords = + I18n.t("site_settings.keywords.#{setting}", default: "", locale: :en).split("|") + end + + # TODO (martin) We can remove this workaround of checking if + # we get an array back once keyword translations in languages other + # than English have been updated not to use YAML arrays. + if translated_keywords.is_a?(Array) + return( + ( + translated_keywords + [deprecated_setting_alias(setting)] + english_translated_keywords + ).compact + ) + end + + translated_keywords + .split("|") + .concat([deprecated_setting_alias(setting)] + english_translated_keywords) + .compact end def placeholder(setting) diff --git a/spec/lib/site_setting_extension_spec.rb b/spec/lib/site_setting_extension_spec.rb index f39ca51b293..15648f95f26 100644 --- a/spec/lib/site_setting_extension_spec.rb +++ b/spec/lib/site_setting_extension_spec.rb @@ -1026,4 +1026,34 @@ RSpec.describe SiteSettingExtension do expect(SiteSetting.exclude_rel_nofollow_domains_map).to eq([]) end end + + describe "keywords" do + it "gets the list of I18n keywords for the setting" do + expect(SiteSetting.keywords(:clean_up_inactive_users_after_days)).to eq( + I18n.t("site_settings.keywords.clean_up_inactive_users_after_days").split("|"), + ) + end + + it "gets the current locale keywords and the english keywords for the setting" do + I18n.locale = :de + expect(SiteSetting.keywords(:clean_up_inactive_users_after_days)).to match_array( + ( + I18n.t("site_settings.keywords.clean_up_inactive_users_after_days").split("|") + + I18n.t("site_settings.keywords.clean_up_inactive_users_after_days", locale: :en).split( + "|", + ) + ).flatten, + ) + end + + context "when a setting also has an alias after renaming" do + before { SiteSetting.stubs(:deprecated_setting_alias).returns("some_old_setting") } + + it "is included with the keywords" do + expect(SiteSetting.keywords(:clean_up_inactive_users_after_days)).to include( + "some_old_setting", + ) + end + end + end end