FIX: Make score's reason link building more explicit (#14475)

We relied on backticks to identify and replace site setting names with links. Unfortunately, some translations don't follow this convention, breaking this feature.

Additionally, this lets us linkify `category settings` and `watched words` without using HTML in the translations.

You may notice that I split the texts we want to linkify into two groups. I did this on purpose to emphasize those that should be translated (regular_links) from those who don't (site_settings_link). If you can think of a better solution, I'm open to suggestions.
This commit is contained in:
Roman Rizzi 2021-10-04 16:55:09 -03:00 committed by GitHub
parent 90a3fbc07b
commit a9d20610d4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 126 additions and 24 deletions

View File

@ -19,18 +19,19 @@ class ReviewableScoreSerializer < ApplicationSerializer
def reason
return unless object.reason
if text = I18n.t("reviewables.reasons.#{object.reason}", base_url: Discourse.base_url, default: nil)
# Create a convenient link to any site settings if the user is staff
settings_url = "#{Discourse.base_url}/admin/site_settings/category/all_results?filter="
link_text = I18n.t("reviewables.reasons.site_setting_links.#{object.reason}", default: nil)
link_text = I18n.t("reviewables.reasons.regular_links.#{object.reason}", default: nil) if link_text.nil?
text.gsub!(/`[a-z_]+`/) do |m|
if scope.is_staff?
setting = m[1..-2]
"<a href=\"#{settings_url}#{setting}\">#{setting.gsub('_', ' ')}</a>"
else
m.gsub('_', ' ')
end
end
if link_text
link = build_link_for(object.reason, link_text)
text = I18n.t("reviewables.reasons.#{object.reason}", link: link, default: nil)
else
text = I18n.t("reviewables.reasons.#{object.reason}", default: nil)
# TODO(roman): Remove after the 2.8 release.
# The discourse-antivirus and akismet plugins still use the backtick format for settings.
# It'll be hard to migrate them to the new format without breaking backwards compatibility, so I'm keeping the old behavior for now.
# Will remove after the 2.8 release.
linkify_backticks(object.reason, text)
end
text
@ -40,4 +41,33 @@ class ReviewableScoreSerializer < ApplicationSerializer
reason.present?
end
private
def url_for(reason, text)
case reason
when 'watched_word'
"#{Discourse.base_url}/admin/customize/watched_words"
when 'category'
"#{Discourse.base_url}/c/#{object.reviewable.category&.name}/edit/settings"
else
"#{Discourse.base_url}/admin/site_settings/category/all_results?filter=#{text}"
end
end
def build_link_for(reason, text)
return text.gsub('_', ' ') unless scope.is_staff?
"<a href=\"#{url_for(reason, text)}\">#{text.gsub('_', ' ')}</a>"
end
def linkify_backticks(reason, text)
text.gsub!(/`[a-z_]+`/) do |m|
if scope.is_staff?
setting = m[1..-2]
"<a href=\"#{url_for(reason, setting)}\">#{setting.gsub('_', ' ')}</a>"
else
m.gsub('_', ' ')
end
end
end
end

View File

@ -4997,21 +4997,36 @@ en:
missing_version: "You must supply a version parameter"
conflict: "There was an update conflict preventing you from doing that."
reasons:
post_count: "The first few posts from every user must be approved by staff. See `approve_post_count`."
trust_level: "Users at low trust levels must have replies approved by staff. See `approve_unless_trust_level`."
new_topics_unless_trust_level: "Users at low trust levels must have topics approved by staff. See `approve_new_topics_unless_trust_level`."
fast_typer: "New user typed their first post suspiciously fast, suspected bot or spammer behavior. See `min_first_post_typing_time`."
auto_silence_regexp: "New user whose first post matches the `auto_silence_first_post_regex` setting."
watched_word: "This post included a Watched Word. See your <a href='%{base_url}/admin/customize/watched_words'>list of watched words</a>."
staged: "New topics and posts for staged users must be approved by staff. See `approve_unless_staged`."
category: "Posts in this category require manual approval by staff. See the category settings."
must_approve_users: "All new users must be approved by staff. See `must_approve_users`."
invite_only: "All new users should be invited. See `invite_only`."
post_count: "The first few posts from every user must be approved by staff. See %{link}."
trust_level: "Users at low trust levels must have replies approved by staff. See %{link}."
new_topics_unless_trust_level: "Users at low trust levels must have topics approved by staff. See %{link}."
fast_typer: "New user typed their first post suspiciously fast, suspected bot or spammer behavior. See %{link}."
auto_silence_regexp: "New user whose first post matches the %{link} setting."
watched_word: "This post included a Watched Word. See your %{link}."
staged: "New topics and posts for staged users must be approved by staff. See %{link}."
category: "Posts in this category require manual approval by staff. See the %{link}."
must_approve_users: "All new users must be approved by staff. See %{link}."
invite_only: "All new users should be invited. See %{link}."
email_auth_res_enqueue: "This email failed a DMARC check, it most likely isn't from whom it seems to be from. Check the raw email headers for more information."
email_spam: "This email was flagged as spam by the header defined in `email_in_spam_header`."
suspect_user: "This new user entered profile information without reading any topics or posts, which strongly suggests they may be a spammer. See `approve_suspect_users`."
contains_media: "This post includes embedded media. See `review_media_unless_trust_level`."
email_spam: "This email was flagged as spam by the header defined in %{link}."
suspect_user: "This new user entered profile information without reading any topics or posts, which strongly suggests they may be a spammer. See %{link}."
contains_media: "This post includes embedded media. See %{link}."
queued_by_staff: "A staff member thinks this post needs review. It'll remain hidden until then."
regular_links:
watched_word: list of watched words
category: category settings
site_setting_links:
post_count: approve_post_count
trust_level: approve_unless_trust_level
new_topics_unless_trust_level: approve_new_topics_unless_trust_level
fast_typer: min_first_post_typing_time
auto_silence_regexp: auto_silence_first_post_regex
staged: approve_unless_staged
must_approve_users: must_approve_users
invite_only: invite_only
email_spam: email_in_spam_header
suspect_user: approve_suspect_users
contains_media: review_media_unless_trust_level
actions:
agree:

View File

@ -0,0 +1,57 @@
# frozen_string_literal: true
require 'rails_helper'
describe ReviewableScoreSerializer do
fab!(:reviewable) { Fabricate(:reviewable_flagged_post) }
fab!(:admin) { Fabricate(:admin) }
describe '#reason' do
context 'regular links' do
it 'adds a link for watched words' do
serialized = serialized_score('watched_word')
link_url = "#{Discourse.base_url}/admin/customize/watched_words"
watched_words_link = "<a href=\"#{link_url}\">#{I18n.t('reviewables.reasons.regular_links.watched_word')}</a>"
expect(serialized.reason).to include(watched_words_link)
end
it 'adds a link for category settings' do
category = Fabricate.build(:category)
reviewable.category = category
serialized = serialized_score('category')
link_url = "#{Discourse.base_url}/c/#{category.name}/edit/settings"
category_link = "<a href=\"#{link_url}\">#{I18n.t('reviewables.reasons.regular_links.category')}</a>"
expect(serialized.reason).to include(category_link)
end
end
context 'site setting links' do
reasons = %w[
post_count trust_level new_topics_unless_trust_level fast_typer auto_silence_regexp
staged must_approve_users invite_only email_spam suspect_user contains_media
]
reasons.each do |r|
it "addd a link to a site setting for the #{r} reason" do
serialized = serialized_score(r)
setting_name = I18n.t("reviewables.reasons.site_setting_links.#{r}")
link_url = "#{Discourse.base_url}/admin/site_settings/category/all_results?filter=#{setting_name}"
link = "<a href=\"#{link_url}\">#{setting_name.gsub('_', ' ')}</a>"
expect(serialized.reason).to include(link)
end
end
end
end
def serialized_score(reason)
score = ReviewableScore.new(
reviewable: reviewable,
reason: reason
)
described_class.new(score, scope: Guardian.new(admin), root: nil)
end
end