From a9d20610d4455d66d406a0d234908867a7e429ef Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Mon, 4 Oct 2021 16:55:09 -0300 Subject: [PATCH] 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. --- .../reviewable_score_serializer.rb | 52 +++++++++++++---- config/locales/server.en.yml | 41 ++++++++----- .../reviewable_score_serializer_spec.rb | 57 +++++++++++++++++++ 3 files changed, 126 insertions(+), 24 deletions(-) create mode 100644 spec/serializers/reviewable_score_serializer_spec.rb diff --git a/app/serializers/reviewable_score_serializer.rb b/app/serializers/reviewable_score_serializer.rb index 4232c0c25c5..db6331be6b9 100644 --- a/app/serializers/reviewable_score_serializer.rb +++ b/app/serializers/reviewable_score_serializer.rb @@ -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] - "#{setting.gsub('_', ' ')}" - 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? + + "#{text.gsub('_', ' ')}" + end + + def linkify_backticks(reason, text) + text.gsub!(/`[a-z_]+`/) do |m| + if scope.is_staff? + setting = m[1..-2] + "#{setting.gsub('_', ' ')}" + else + m.gsub('_', ' ') + end + end + end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 24dd820098d..c20f6d698f2 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -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 list of watched words." - 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: diff --git a/spec/serializers/reviewable_score_serializer_spec.rb b/spec/serializers/reviewable_score_serializer_spec.rb new file mode 100644 index 00000000000..103148de6f1 --- /dev/null +++ b/spec/serializers/reviewable_score_serializer_spec.rb @@ -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 = "#{I18n.t('reviewables.reasons.regular_links.watched_word')}" + + 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 = "#{I18n.t('reviewables.reasons.regular_links.category')}" + + 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 = "#{setting_name.gsub('_', ' ')}" + + 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