FIX: Don't store translated trust level names in anonymous cache (#13224)

Refactors `TrustLevel` and moves translations from server to client

Additional changes:
  * "staff" and "admin" wasn't translatable in site settings
  * it replaces a concatenated string with a translation
  * uses translation for trust levels in users_by_trust_level report
  * adds a DB migration to rename keys of translation overrides affected by this commit

app/assets/javascripts/discourse/app/models/site.js

@@ -179,8 +179,10 @@ Site.reopenClass(Singleton, {
     }
 
     if (result.trust_levels) {
-      result.trustLevels = result.trust_levels.map((tl) =>
-        TrustLevel.create(tl)
+      result.trustLevels = Object.entries(result.trust_levels).map(
+        ([key, id]) => {
+          return new TrustLevel(id, key);
+        }
       );
       delete result.trust_levels;
     }

app/assets/javascripts/discourse/app/models/trust-level.js

@@ -1,6 +1,22 @@
-import RestModel from "discourse/models/rest";
-import { fmt } from "discourse/lib/computed";
+import { computed } from "@ember/object";
+import I18n from "I18n";
 
-export default RestModel.extend({
-  detailedName: fmt("id", "name", "%@ - %@"),
-});
+export default class TrustLevel {
+  constructor(id, key) {
+    this.id = id;
+    this._key = key;
+  }
+
+  @computed
+  get name() {
+    return I18n.t(`trust_levels.names.${this._key}`);
+  }
+
+  @computed
+  get detailedName() {
+    return I18n.t("trust_levels.detailed_name", {
+      level: this.id,
+      name: this.name,
+    });
+  }
+}

app/assets/javascripts/discourse/tests/fixtures/site-fixtures.js

@@ -12,6 +12,13 @@ export default {
         small_action: 3,
         whisper: 4,
       },
+      trust_levels: {
+        newuser: 0,
+        basic: 1,
+        member: 2,
+        regular: 3,
+        leader: 4,
+      },
       groups: [
         { id: 0, name: "everyone" },
         { id: 1, name: "admins" },
@@ -535,28 +542,6 @@ export default {
           is_custom_flag: true,
         },
       ],
-      trust_levels: [
-        {
-          id: 0,
-          name: "new user",
-        },
-        {
-          id: 1,
-          name: "basic user",
-        },
-        {
-          id: 2,
-          name: "member",
-        },
-        {
-          id: 3,
-          name: "regular",
-        },
-        {
-          id: 4,
-          name: "leader",
-        },
-      ],
       archetypes: [
         {
           id: "regular",

app/assets/javascripts/discourse/tests/helpers/site.js

@@ -16,6 +16,13 @@ PreloadStore.store("site", {
     moved_post: 10,
   },
   post_types: { regular: 1, moderator_action: 2 },
+  trust_levels: {
+    newuser: 0,
+    basic: 1,
+    member: 2,
+    regular: 3,
+    leader: 4,
+  },
   groups: [
     { id: 0, name: "everyone" },
     { id: 1, name: "admins" },
@@ -349,12 +356,5 @@ PreloadStore.store("site", {
       is_custom_flag: true,
     },
   ],
-  trust_levels: [
-    { id: 0, name: "new user" },
-    { id: 1, name: "basic user" },
-    { id: 2, name: "member" },
-    { id: 3, name: "regular" },
-    { id: 4, name: "leader" },
-  ],
   archetypes: [{ id: "regular", name: "Regular Topic", options: [] }],
 });

app/models/concerns/reports/users_by_trust_level.rb

@@ -24,7 +24,7 @@ module Reports::UsersByTrustLevel
       ]
 
       User.real.group('trust_level').count.sort.each do |level, count|
-        key = TrustLevel.levels[level.to_i]
+        key = TrustLevel.name(level.to_i)
         url = Proc.new { |k| "/admin/users/list/#{k}" }
         report.data << { url: url.call(key), key: key, x: level.to_i, y: count }
       end

app/models/site.rb

@@ -21,7 +21,7 @@ class Site
   end
 
   def trust_levels
-    TrustLevel.all
+    TrustLevel.levels
   end
 
   def user_fields

app/models/trust_level_and_staff_setting.rb

@@ -18,4 +18,12 @@ class TrustLevelAndStaffSetting < TrustLevelSetting
   def self.special_groups
     ['staff', 'admin']
   end
+
+  def self.translation(value)
+    if special_group?(value)
+      I18n.t("trust_levels.#{value}")
+    else
+      super
+    end
+  end
 end

app/models/trust_level_setting.rb

@@ -8,18 +8,23 @@ class TrustLevelSetting < EnumSiteSetting
   end
 
   def self.values
-    levels = TrustLevel.all
-    valid_values.map { |x|
-      {
-        name: x.is_a?(Integer) ? "#{x}: #{levels[x.to_i].name}" : x,
-        value: x
-      }
-    }
+    valid_values.map do |value|
+      { name: translation(value), value: value }
+    end
   end
 
   def self.valid_values
     TrustLevel.valid_range.to_a
   end
 
+  def self.translation(value)
+    I18n.t(
+      "js.trust_levels.detailed_name",
+      level: value,
+      name: TrustLevel.name(value)
+    )
+  end
+
   private_class_method :valid_values
+  private_class_method :translation
 end

app/serializers/site_serializer.rb

@@ -6,6 +6,7 @@ class SiteSerializer < ApplicationSerializer
     :default_archetype,
     :notification_types,
     :post_types,
+    :trust_levels,
     :groups,
     :filters,
     :periods,
@@ -32,7 +33,6 @@ class SiteSerializer < ApplicationSerializer
   )
 
   has_many :categories, serializer: SiteCategorySerializer, embed: :objects
-  has_many :trust_levels, embed: :objects
   has_many :archetypes, embed: :objects, serializer: ArchetypeSerializer
   has_many :user_fields, embed: :objects, serializer: UserFieldSerializer
   has_many :auth_providers, embed: :objects, serializer: AuthProviderSerializer

app/serializers/trust_level_serializer.rb

@@ -1,7 +0,0 @@
-# frozen_string_literal: true
-
-class TrustLevelSerializer < ApplicationSerializer
-
-  attributes :id, :name
-
-end

config/locales/client.en.yml

@@ -702,7 +702,7 @@ en:
           smtp_instructions: "When you enable SMTP for the group, all outbound emails sent from the group's inbox will be sent via the SMTP settings specified here instead of the mail server configured for other emails sent by your forum."
           imap_title: "IMAP"
           imap_additional_settings: "Additional Settings"
-          imap_instructions: "When you enable IMAP for the group, emails are synced between the group inbox and the provided IMAP server and mailbox. SMTP must be enabled with valid and tested credentials before IMAP can be enabled. The email username and password used for SMTP will be used for IMAP. For more information see <a target=\"_blank\" href=\"https://meta.discourse.org/t/imap-support-for-group-inboxes/160588\">feature announcement on Discourse Meta</a>."
+          imap_instructions: 'When you enable IMAP for the group, emails are synced between the group inbox and the provided IMAP server and mailbox. SMTP must be enabled with valid and tested credentials before IMAP can be enabled. The email username and password used for SMTP will be used for IMAP. For more information see <a target="_blank" href="https://meta.discourse.org/t/imap-support-for-group-inboxes/160588">feature announcement on Discourse Meta</a>.'
           imap_alpha_warning: "Warning: This is an alpha-stage feature. Only Gmail is officially supported. Use at your own risk!"
           imap_settings_valid: "IMAP settings valid."
           smtp_disable_confirm: "If you disable SMTP, all SMTP and IMAP settings will be reset and the associated functionality will be disabled. Are you sure you want to continue?"
@@ -3764,6 +3764,15 @@ en:
         custom: "Custom"
       set_schedule: "Set a notification schedule"
 
+    trust_levels:
+      names:
+        newuser: "new user"
+        basic: "basic user"
+        member: "member"
+        regular: "regular"
+        leader: "leader"
+      detailed_name: "%{level}: %{name}"
+
   # This section is exported to the javascript for i18n in the admin section
   admin_js:
     type_to_filter: "type to filter..."

config/locales/server.en.yml

@@ -717,16 +717,8 @@ en:
       topic_exists_no_oldest: "Can't delete this category because topic count is %{count}."
     uncategorized_description: "Topics that don't need a category, or don't fit into any other existing category."
   trust_levels:
-    newuser:
-      title: "new user"
-    basic:
-      title: "basic user"
-    member:
-      title: "member"
-    regular:
-      title: "regular"
-    leader:
-      title: "leader"
+    admin: "Admin"
+    staff: "Staff"
     change_failed_explanation: "You attempted to demote %{user_name} to '%{new_trust_level}'. However their trust level is already '%{current_trust_level}'. %{user_name} will remain at '%{current_trust_level}' - if you wish to demote user lock trust level first"
 
   post:
@@ -994,7 +986,7 @@ en:
     imap_authentication_error: "There was an issue with the IMAP credentials provided, check the username and password and try again."
     imap_no_response_error: "An error occurred when communicating with the IMAP server. %{message}"
     smtp_authentication_error: "There was an issue with the SMTP credentials provided, check the username and password and try again."
-    authentication_error_gmail_app_password: "Application-specific password required. Learn more at <a target=\"_blank\" href=\"https://support.google.com/accounts/answer/185833\">this Google Help article</a>"
+    authentication_error_gmail_app_password: 'Application-specific password required. Learn more at <a target="_blank" href="https://support.google.com/accounts/answer/185833">this Google Help article</a>'
     smtp_server_busy_error: "The SMTP server is currently busy, try again later."
     smtp_unhandled_error: "There was an unhandled error when communicating with the SMTP server. %{message}"
     imap_unhandled_error: "There was an unhandled error when communicating with the IMAP server. %{message}"

db/migrate/20210601002145_rename_trust_level_translations.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class RenameTrustLevelTranslations < ActiveRecord::Migration[6.1]
+  KEYS = %w[newuser basic member regular leader]
+
+  def up
+    KEYS.each do |key|
+      execute <<~SQL
+        UPDATE translation_overrides
+        SET translation_key = 'js.trust_levels.names.#{key}'
+        WHERE translation_key = 'trust_levels.#{key}.title'
+      SQL
+    end
+  end
+
+  def down
+    KEYS.each do |key|
+      execute <<~SQL
+        UPDATE translation_overrides
+        SET translation_key = 'trust_levels.#{key}.title'
+        WHERE translation_key = 'js.trust_levels.names.#{key}'
+      SQL
+    end
+  end
+end

lib/trust_level.rb

@@ -4,8 +4,6 @@ class InvalidTrustLevel < StandardError; end
 
 class TrustLevel
 
-  attr_reader :id, :name
-
   class << self
 
     def [](level)
@@ -17,12 +15,6 @@ class TrustLevel
       @levels ||= Enum.new(:newuser, :basic, :member, :regular, :leader, start: 0)
     end
 
-    def all
-      levels.map do |name_key, id|
-        TrustLevel.new(name_key, id)
-      end
-    end
-
     def valid?(level)
       valid_range === level
     end
@@ -35,15 +27,9 @@ class TrustLevel
       (current_level || 0) >= level
     end
 
-  end
-
-  def initialize(name_key, id)
-    @name = I18n.t("trust_levels.#{name_key}.title")
-    @id = id
-  end
-
-  def serializable_hash
-    { id: @id, name: @name }
+    def name(level)
+      I18n.t("js.trust_levels.names.#{levels[level]}")
+    end
   end
 
 end

spec/integrity/i18n_spec.rb

@@ -24,12 +24,6 @@ def is_yaml_compatible?(english, translated)
 end
 
 describe "i18n integrity checks" do
-  it 'has an i18n key for each Trust Levels' do
-    TrustLevel.all.each do |ts|
-      expect(ts.name).not_to match(/translation missing/)
-    end
-  end
-
   it "has an i18n key for each Site Setting" do
     SiteSetting.all_settings.each do |s|
       next if s[:setting][/^test_/]

spec/models/trust_level_and_staff_setting_spec.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe TrustLevelAndStaffSetting do
+  describe ".values" do
+    it "returns translated names" do
+      TranslationOverride.upsert!(I18n.locale, "js.trust_levels.names.newuser", "New Member")
+      TranslationOverride.upsert!(I18n.locale, "trust_levels.admin", "Hero")
+
+      values = TrustLevelAndStaffSetting.values
+
+      value = values.find { |v| v[:value] == 0 }
+      expect(value).to be_present
+      expect(value[:name]).to eq(I18n.t("js.trust_levels.detailed_name", level: 0, name: "New Member"))
+
+      value = values.find { |v| v[:value] == "admin" }
+      expect(value).to be_present
+      expect(value[:name]).to eq("Hero")
+
+      value = values.find { |v| v[:value] == "staff" }
+      expect(value).to be_present
+      expect(value[:name]).to eq(I18n.t("trust_levels.staff"))
+    end
+  end
+end

spec/models/trust_level_setting_spec.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe TrustLevelSetting do
+  describe ".values" do
+    it "returns translated names" do
+      TranslationOverride.upsert!(I18n.locale, "js.trust_levels.names.newuser", "New Member")
+
+      value = TrustLevelSetting.values.first
+      expect(value[:name]).to eq(I18n.t("js.trust_levels.detailed_name", level: 0, name: "New Member"))
+      expect(value[:value]).to eq(0)
+    end
+  end
+end