diff --git a/app/assets/javascripts/discourse/controllers/tags-show.js.es6 b/app/assets/javascripts/discourse/controllers/tags-show.js.es6 index 65062dfbc4e..32141ceaa9e 100644 --- a/app/assets/javascripts/discourse/controllers/tags-show.js.es6 +++ b/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: [ diff --git a/app/assets/javascripts/discourse/routes/tags-show.js.es6 b/app/assets/javascripts/discourse/routes/tags-show.js.es6 index 88ec2a05d4d..978816b7df4 100644 --- a/app/assets/javascripts/discourse/routes/tags-show.js.es6 +++ b/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") }); }); }, diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index f1e49da2906..4c9bfc2a7f3 100644 --- a/app/controllers/tags_controller.rb +++ b/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 diff --git a/app/models/tag_group.rb b/app/models/tag_group.rb index 5a5a74af02f..91018c523d1 100644 --- a/app/models/tag_group.rb +++ b/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) diff --git a/app/serializers/tag_serializer.rb b/app/serializers/tag_serializer.rb index 9f050800295..5169ea1a65a 100644 --- a/app/serializers/tag_serializer.rb +++ b/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 diff --git a/lib/discourse_tagging.rb b/lib/discourse_tagging.rb index ac2061b41f9..5ba25636534 100644 --- a/lib/discourse_tagging.rb +++ b/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) diff --git a/spec/components/discourse_tagging_spec.rb b/spec/components/discourse_tagging_spec.rb index f080a03f8cb..c9ee958a709 100644 --- a/spec/components/discourse_tagging_spec.rb +++ b/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 diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb index ade557a66e5..82c53eaaf2f 100644 --- a/spec/requests/tags_controller_spec.rb +++ b/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 diff --git a/test/javascripts/acceptance/tags-test.js.es6 b/test/javascripts/acceptance/tags-test.js.es6 index b32fabca8aa..81b9e49ed49 100644 --- a/test/javascripts/acceptance/tags-test.js.es6 +++ b/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); +});