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

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.

diff --git a/app/serializers/reviewable_score_serializer.rb b/app/serializers/reviewable_score_serializer.rb
index 4232c0c..db6331b 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="
-
-      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
+    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?
+
+    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
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 24dd820..c20f6d6 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 <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:
diff --git a/spec/serializers/reviewable_score_serializer_spec.rb b/spec/serializers/reviewable_score_serializer_spec.rb
new file mode 100644
index 0000000..103148d
--- /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 = "<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"

[... diff too long, it was truncated ...]

GitHub sha: a9d20610d4455d66d406a0d234908867a7e429ef

This commit appears in #14475 which was approved by eviltrout. It was merged by romanrizzi.