FIX: Hide PM tags if the site setting is disabled (#10089)

* FIX: Hide PM tags if the site setting is disabled

* Apply code suggestions

app/controllers/list_controller.rb

@@ -144,6 +144,10 @@ class ListController < ApplicationController
 
   def self.generate_message_route(action)
     define_method("#{action}") do
+      if action == :private_messages_tag && !guardian.can_tag_pms?
+        raise Discourse::NotFound
+      end
+
       list_opts = build_topic_list_options
       target_user = fetch_user_from_params({ include_inactive: current_user.try(:staff?) }, [:user_stat, :user_option])
       guardian.ensure_can_see_private_messages!(target_user.id)

app/controllers/tags_controller.rb

@@ -37,10 +37,10 @@ class TagsController < ::ApplicationController
       ungrouped_tags = ungrouped_tags.where("tags.topic_count > 0") unless show_all_tags
 
       grouped_tag_counts = TagGroup.visible(guardian).order('name ASC').includes(:tags).map do |tag_group|
-        { id: tag_group.id, name: tag_group.name, tags: self.class.tag_counts_json(tag_group.tags.where(target_tag_id: nil)) }
+        { id: tag_group.id, name: tag_group.name, tags: self.class.tag_counts_json(tag_group.tags.where(target_tag_id: nil), show_pm_tags: guardian.can_tag_pms?) }
       end
 
-      @tags = self.class.tag_counts_json(ungrouped_tags)
+      @tags = self.class.tag_counts_json(ungrouped_tags, show_pm_tags: guardian.can_tag_pms?)
       @extras = { tag_groups: grouped_tag_counts }
     else
       tags = show_all_tags ? Tag.all : Tag.where("tags.topic_count > 0")
@@ -54,7 +54,7 @@ class TagsController < ::ApplicationController
         { id: c.id, tags: self.class.tag_counts_json(c.tags.where(target_tag_id: nil)) }
       end
 
-      @tags = self.class.tag_counts_json(unrestricted_tags)
+      @tags = self.class.tag_counts_json(unrestricted_tags, show_pm_tags: guardian.can_tag_pms?)
       @extras = { categories: category_tag_counts }
     end
 
@@ -231,7 +231,7 @@ class TagsController < ::ApplicationController
       filter_params
     )
 
-    tags = self.class.tag_counts_json(tags_with_counts)
+    tags = self.class.tag_counts_json(tags_with_counts, show_pm_tags: guardian.can_tag_pms?)
 
     json_response = { results: tags }
 
@@ -336,17 +336,19 @@ class TagsController < ::ApplicationController
     raise Discourse::NotFound if DiscourseTagging.hidden_tag_names(guardian).include?(params[:tag_id])
   end
 
-  def self.tag_counts_json(tags)
+  def self.tag_counts_json(tags, show_pm_tags: true)
     target_tags = Tag.where(id: tags.map(&:target_tag_id).compact.uniq).select(:id, :name)
     tags.map do |t|
+      next if t.topic_count == 0 && t.pm_topic_count > 0 && !show_pm_tags
+
       {
         id: t.name,
         text: t.name,
         count: t.topic_count,
-        pm_count: t.pm_topic_count,
+        pm_count: show_pm_tags ? t.pm_topic_count : 0,
         target_tag: t.target_tag_id ? target_tags.find { |x| x.id == t.target_tag_id }&.name : nil
       }
-    end
+    end.compact
   end
 
   def set_category_from_params

config/routes.rb

@@ -736,7 +736,7 @@ Discourse::Application.routes.draw do
       get "private-messages-sent/:username" => "list#private_messages_sent", as: "topics_private_messages_sent", defaults: { format: :json }
       get "private-messages-archive/:username" => "list#private_messages_archive", as: "topics_private_messages_archive", defaults: { format: :json }
       get "private-messages-unread/:username" => "list#private_messages_unread", as: "topics_private_messages_unread", defaults: { format: :json }
-      get "private-messages-tags/:username/:tag_id.json" => "list#private_messages_tag", as: "topics_private_messages_tag", constraints: StaffConstraint.new
+      get "private-messages-tags/:username/:tag_id.json" => "list#private_messages_tag", as: "topics_private_messages_tag", defaults: { format: :json }
       get "groups/:group_name" => "list#group_topics", as: "group_topics", group_name: RouteFormat.username
 
       scope "/private-messages-group/:username", group_name: RouteFormat.username do

spec/requests/list_controller_spec.rb

@@ -142,6 +142,16 @@ RSpec.describe ListController do
       expect(response.status).to eq(404)
     end
 
+    it 'should fail for staff users if disabled' do
+      SiteSetting.allow_staff_to_tag_pms = false
+
+      [moderator, admin].each do |user|
+        sign_in(user)
+        get "/topics/private-messages-tags/#{user.username}/#{tag.name}.json"
+        expect(response.status).to eq(404)
+      end
+    end
+
     it 'should be success for staff users' do
       [moderator, admin].each do |user|
         sign_in(user)

spec/requests/tags_controller_spec.rb

@@ -33,6 +33,55 @@ describe TagsController do
       end
     end
 
+    context "with allow_staff_to_tag_pms" do
+      fab!(:admin) { Fabricate(:admin) }
+      fab!(:topic) { Fabricate(:topic, tags: [topic_tag]) }
+      fab!(:pm) do
+        Fabricate(
+          :private_message_topic,
+          tags: [test_tag],
+          topic_allowed_users: [
+            Fabricate.build(:topic_allowed_user, user: admin)
+          ]
+        )
+      end
+
+      context "enabled" do
+        before do
+          SiteSetting.allow_staff_to_tag_pms = true
+          sign_in(admin)
+        end
+
+        it "shows topic tags and pm tags" do
+          get "/tags.json"
+          tags = response.parsed_body["tags"]
+          expect(tags.length).to eq(2)
+
+          serialized_tag = tags.find { |t| t["id"] == topic_tag.name }
+          expect(serialized_tag["count"]).to eq(2)
+          expect(serialized_tag["pm_count"]).to eq(0)
+
+          serialized_tag = tags.find { |t| t["id"] == test_tag.name }
+          expect(serialized_tag["count"]).to eq(0)
+          expect(serialized_tag["pm_count"]).to eq(1)
+        end
+      end
+
+      context "disabled" do
+        before do
+          SiteSetting.allow_staff_to_tag_pms = false
+          sign_in(admin)
+        end
+
+        it "hides pm tags" do
+          get "/tags.json"
+          tags = response.parsed_body["tags"]
+          expect(tags.length).to eq(1)
+          expect(tags[0]["id"]).to eq(topic_tag.name)
+        end
+      end
+    end
+
     context "with tags_listed_by_group enabled" do
       before { SiteSetting.tags_listed_by_group = true }
       include_examples "successfully retrieve tags with topic_count > 0"