FIX: Disable 'Create Topic' button if tag is staff-only. (#6984)

* FIX: Disable 'Create Topic' button if tag is staff-only.

* FIX: Staff-only tags should always return 404.

app/assets/javascripts/discourse/controllers/tags-show.js.es6

@@ -74,9 +74,25 @@ export default Ember.Controller.extend(BulkTopicSelection, {
     return listDraft ? "topic.open_draft" : "topic.create";
   },
 
-  @computed("canCreateTopic", "category", "canCreateTopicOnCategory")
-  createTopicDisabled(canCreateTopic, category, canCreateTopicOnCategory) {
-    return !canCreateTopic || (category && !canCreateTopicOnCategory);
+  @computed(
+    "canCreateTopic",
+    "category",
+    "canCreateTopicOnCategory",
+    "tag",
+    "canCreateTopicOnTag"
+  )
+  createTopicDisabled(
+    canCreateTopic,
+    category,
+    canCreateTopicOnCategory,
+    tag,
+    canCreateTopicOnTag
+  ) {
+    return (
+      !canCreateTopic ||
+      (category && !canCreateTopicOnCategory) ||
+      (tag && !canCreateTopicOnTag)
+    );
   },
 
   queryParams: [

app/assets/javascripts/discourse/routes/tags-show.js.es6

@@ -111,14 +111,18 @@ export default Discourse.Route.extend({
     ).then(list => {
       if (list.topic_list.tags && list.topic_list.tags.length === 1) {
         // Update name of tag (case might be different)
-        tag.set("id", list.topic_list.tags[0].name);
+        tag.setProperties({
+          id: list.topic_list.tags[0].name,
+          staff: list.topic_list.tags[0].staff
+        });
       }
       controller.setProperties({
         list,
         canCreateTopic: list.get("can_create_topic"),
         loading: false,
         canCreateTopicOnCategory:
-          this.get("category.permission") === PermissionType.FULL
+          this.get("category.permission") === PermissionType.FULL,
+        canCreateTopicOnTag: !tag.get("staff") || this.get("currentUser.staff")
       });
     });
   },

app/controllers/tags_controller.rb

@@ -98,6 +98,8 @@ class TagsController < ::ApplicationController
   end
 
   def show
+    raise Discourse::NotFound if DiscourseTagging.hidden_tag_names(guardian).include?(params[:tag_id])
+
     show_latest
   end
 

app/models/tag_group.rb

@@ -12,6 +12,8 @@ class TagGroup < ActiveRecord::Base
   before_create :init_permissions
   before_save :apply_permissions
 
+  after_commit { DiscourseTagging.clear_cache! }
+
   attr_accessor :permissions
 
   def tag_names=(tag_names_arg)

app/serializers/tag_serializer.rb

@@ -1,3 +1,7 @@
 class TagSerializer < ApplicationSerializer
-  attributes :id, :name, :topic_count
+  attributes :id, :name, :topic_count, :staff
+
+  def staff
+    DiscourseTagging.staff_tag_names.include?(name)
+  end
 end

lib/discourse_tagging.rb

@@ -2,6 +2,7 @@ module DiscourseTagging
 
   TAGS_FIELD_NAME = "tags"
   TAGS_FILTER_REGEXP = /[\/\?#\[\]@!\$&'\(\)\*\+,;=\.%\\`^\s|\{\}"<>]+/ # /?#[]@!$&'()*+,;=.%\`^|{}"<>
+  TAGS_STAFF_CACHE_KEY = "staff_tag_names"
 
   def self.tag_topic_by_names(topic, guardian, tag_names_arg, append: false)
     if guardian.can_tag?(topic)
@@ -194,11 +195,22 @@ module DiscourseTagging
   end
 
   def self.staff_tag_names
-    Tag.joins(tag_groups: :tag_group_permissions)
-      .where('tag_group_permissions.group_id = ? AND tag_group_permissions.permission_type = ?',
-        Group::AUTO_GROUPS[:everyone],
-        TagGroupPermission.permission_types[:readonly]
-      ).pluck(:name)
+    tag_names = Discourse.cache.read(TAGS_STAFF_CACHE_KEY, tag_names)
+
+    if !tag_names
+      tag_names = Tag.joins(tag_groups: :tag_group_permissions)
+        .where('tag_group_permissions.group_id = ? AND tag_group_permissions.permission_type = ?',
+          Group::AUTO_GROUPS[:everyone],
+          TagGroupPermission.permission_types[:readonly]
+        ).pluck(:name)
+      Discourse.cache.write(TAGS_STAFF_CACHE_KEY, tag_names, expires_in: 1.hour)
+    end
+
+    tag_names
+  end
+
+  def self.clear_cache!
+    Discourse.cache.delete(TAGS_STAFF_CACHE_KEY)
   end
 
   def self.clean_tag(tag)

spec/components/discourse_tagging_spec.rb

@@ -216,4 +216,29 @@ describe DiscourseTagging do
       end
     end
   end
+
+  describe "staff_tag_names" do
+    let(:tag) { Fabricate(:tag) }
+
+    let(:staff_tag) { Fabricate(:tag) }
+    let(:other_staff_tag) { Fabricate(:tag) }
+
+    let!(:staff_tag_group) {
+      Fabricate(
+        :tag_group,
+        permissions: { "staff" => 1, "everyone" => 3 },
+        tag_names: [staff_tag.name]
+      )
+    }
+
+    it "returns all staff tags" do
+      expect(DiscourseTagging.staff_tag_names).to contain_exactly(staff_tag.name)
+
+      staff_tag_group.update(tag_names: [staff_tag.name, other_staff_tag.name])
+      expect(DiscourseTagging.staff_tag_names).to contain_exactly(staff_tag.name, other_staff_tag.name)
+
+      staff_tag_group.update(tag_names: [other_staff_tag.name])
+      expect(DiscourseTagging.staff_tag_names).to contain_exactly(other_staff_tag.name)
+    end
+  end
 end

spec/requests/tags_controller_spec.rb

@@ -64,6 +64,18 @@ describe TagsController do
       get "/tags/%2ftest%2f"
       expect(response.status).to eq(404)
     end
+
+    it "does not show staff-only tags" do
+      tag_group = Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: ["test"])
+
+      get "/tags/test"
+      expect(response.status).to eq(404)
+
+      sign_in(Fabricate(:admin))
+
+      get "/tags/test"
+      expect(response.status).to eq(200)
+    end
   end
 
   describe '#check_hashtag' do

test/javascripts/acceptance/tags-test.js.es6

@@ -1,4 +1,4 @@
-import { acceptance } from "helpers/qunit-helpers";
+import { replaceCurrentUser, acceptance } from "helpers/qunit-helpers";
 acceptance("Tags", { loggedIn: true });
 
 QUnit.test("list the tags", async assert => {
@@ -92,3 +92,83 @@ QUnit.test("list the tags in groups", async assert => {
     "always uses lowercase URLs for mixed case tags"
   );
 });
+
+test("new topic button is not available for staff-only tags", async assert => {
+  /* global server */
+  server.get("/tags/regular-tag/notifications", () => [
+    200,
+    { "Content-Type": "application/json" },
+    { tag_notification: { id: "regular-tag", notification_level: 1 } }
+  ]);
+
+  server.get("/tags/regular-tag/l/latest.json", () => [
+    200,
+    { "Content-Type": "application/json" },
+    {
+      users: [],
+      primary_groups: [],
+      topic_list: {
+        can_create_topic: true,
+        draft: null,
+        draft_key: "new_topic",
+        draft_sequence: 1,
+        per_page: 30,
+        tags: [
+          {
+            id: 1,
+            name: "regular-tag",
+            topic_count: 1
+          }
+        ],
+        topics: []
+      }
+    }
+  ]);
+
+  server.get("/tags/staff-only-tag/notifications", () => [
+    200,
+    { "Content-Type": "application/json" },
+    { tag_notification: { id: "staff-only-tag", notification_level: 1 } }
+  ]);
+
+  server.get("/tags/staff-only-tag/l/latest.json", () => [
+    200,
+    { "Content-Type": "application/json" },
+    {
+      users: [],
+      primary_groups: [],
+      topic_list: {
+        can_create_topic: true,
+        draft: null,
+        draft_key: "new_topic",
+        draft_sequence: 1,
+        per_page: 30,
+        tags: [
+          {
+            id: 1,
+            name: "staff-only-tag",
+            topic_count: 1,
+            staff: true
+          }
+        ],
+        topics: []
+      }
+    }
+  ]);
+
+  replaceCurrentUser({ staff: false });
+
+  await visit("/tags/regular-tag");
+  assert.ok(find("#create-topic:disabled").length === 0);
+
+  await visit("/tags/staff-only-tag");
+  assert.ok(find("#create-topic:disabled").length === 1);
+
+  replaceCurrentUser({ staff: true });
+
+  await visit("/tags/regular-tag");
+  assert.ok(find("#create-topic:disabled").length === 0);
+
+  await visit("/tags/staff-only-tag");
+  assert.ok(find("#create-topic:disabled").length === 0);
+});