Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Jun 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions app/assets/javascripts/discourse/app/models/site.js
Expand Up @@ -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;
}
Expand Down
26 changes: 21 additions & 5 deletions 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I like to see JS classes.

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,
});
}
}
29 changes: 7 additions & 22 deletions app/assets/javascripts/discourse/tests/fixtures/site-fixtures.js
Expand Up @@ -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" },
Expand Down Expand Up @@ -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",
Expand Down
14 changes: 7 additions & 7 deletions app/assets/javascripts/discourse/tests/helpers/site.js
Expand Up @@ -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" },
Expand Down Expand Up @@ -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: [] }],
});
2 changes: 1 addition & 1 deletion app/models/concerns/reports/users_by_trust_level.rb
Expand Up @@ -24,7 +24,7 @@ def report_users_by_trust_level(report)
]

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
Expand Down
2 changes: 1 addition & 1 deletion app/models/site.rb
Expand Up @@ -21,7 +21,7 @@ def notification_types
end

def trust_levels
TrustLevel.all
TrustLevel.levels
end

def user_fields
Expand Down
8 changes: 8 additions & 0 deletions app/models/trust_level_and_staff_setting.rb
Expand Up @@ -18,4 +18,12 @@ def self.special_group?(val)
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
19 changes: 12 additions & 7 deletions app/models/trust_level_setting.rb
Expand Up @@ -8,18 +8,23 @@ def self.valid_value?(val)
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
2 changes: 1 addition & 1 deletion app/serializers/site_serializer.rb
Expand Up @@ -6,6 +6,7 @@ class SiteSerializer < ApplicationSerializer
:default_archetype,
:notification_types,
:post_types,
:trust_levels,
:groups,
:filters,
:periods,
Expand All @@ -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
Expand Down
7 changes: 0 additions & 7 deletions app/serializers/trust_level_serializer.rb

This file was deleted.

11 changes: 10 additions & 1 deletion config/locales/client.en.yml
Expand Up @@ -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?"
Expand Down Expand Up @@ -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..."
Expand Down
14 changes: 3 additions & 11 deletions config/locales/server.en.yml
Expand Up @@ -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:
Expand Down Expand Up @@ -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}"
Expand Down
25 changes: 25 additions & 0 deletions 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
20 changes: 3 additions & 17 deletions lib/trust_level.rb
Expand Up @@ -4,8 +4,6 @@ class InvalidTrustLevel < StandardError; end

class TrustLevel

attr_reader :id, :name

class << self

def [](level)
Expand All @@ -17,12 +15,6 @@ def levels
@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
Expand All @@ -35,15 +27,9 @@ def compare(current_level, level)
(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
6 changes: 0 additions & 6 deletions spec/integrity/i18n_spec.rb
Expand Up @@ -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_/]
Expand Down
26 changes: 26 additions & 0 deletions 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
15 changes: 15 additions & 0 deletions 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