diff --git a/app/assets/javascripts/discourse/components/edit-category-tags.js.es6 b/app/assets/javascripts/discourse/components/edit-category-tags.js.es6 index 1c583fda2b9..6705f9e9f0f 100644 --- a/app/assets/javascripts/discourse/components/edit-category-tags.js.es6 +++ b/app/assets/javascripts/discourse/components/edit-category-tags.js.es6 @@ -1,3 +1,11 @@ import { buildCategoryPanel } from "discourse/components/edit-category-panel"; +import computed from "ember-addons/ember-computed-decorators"; -export default buildCategoryPanel("tags", {}); +export default buildCategoryPanel("tags", { + allowedTagsEmpty: Ember.computed.empty("category.allowed_tags"), + allowedTagGroupsEmpty: Ember.computed.empty("category.allowed_tag_groups"), + disableAllowGlobalTags: Ember.computed.and( + "allowedTagsEmpty", + "allowedTagGroupsEmpty" + ) +}); diff --git a/app/assets/javascripts/discourse/models/category.js.es6 b/app/assets/javascripts/discourse/models/category.js.es6 index d31fcb72b45..9e0c0470dcd 100644 --- a/app/assets/javascripts/discourse/models/category.js.es6 +++ b/app/assets/javascripts/discourse/models/category.js.es6 @@ -118,6 +118,7 @@ const Category = RestModel.extend({ all_topics_wiki: this.get("all_topics_wiki"), allowed_tags: this.get("allowed_tags"), allowed_tag_groups: this.get("allowed_tag_groups"), + allow_global_tags: this.get("allow_global_tags"), sort_order: this.get("sort_order"), sort_ascending: this.get("sort_ascending"), topic_featured_link_allowed: this.get("topic_featured_link_allowed"), diff --git a/app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs b/app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs index dcdc769b765..a0af5fb2fb9 100644 --- a/app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs +++ b/app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs @@ -11,6 +11,14 @@
{{tag-group-chooser id="category-allowed-tag-groups" tagGroups=category.allowed_tag_groups}} + {{#link-to 'tagGroups'}}{{i18n 'category.manage_tag_groups_link'}}{{/link-to}} +
+ +
+
diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index 3b9195c3176..ab9fdbe3a69 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -295,6 +295,7 @@ class CategoriesController < ApplicationController :minimum_required_tags, :navigate_to_first_post_after_read, :search_priority, + :allow_global_tags, custom_fields: [params[:custom_fields].try(:keys)], permissions: [*p.try(:keys)], allowed_tags: [], diff --git a/app/serializers/category_serializer.rb b/app/serializers/category_serializer.rb index d7973eb0355..64f81fd6285 100644 --- a/app/serializers/category_serializer.rb +++ b/app/serializers/category_serializer.rb @@ -18,6 +18,7 @@ class CategorySerializer < BasicCategorySerializer :custom_fields, :allowed_tags, :allowed_tag_groups, + :allow_global_tags, :topic_featured_link_allowed, :search_priority diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index e8b9c9a7707..01cdc9a9790 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2483,11 +2483,13 @@ en: settings: "Settings" topic_template: "Topic Template" tags: "Tags" - tags_allowed_tags: "Only allow these tags to be used in this category:" - tags_allowed_tag_groups: "Only allow tags from these groups to be used in this category:" + tags_allowed_tags: "Restrict these tags to this category:" + tags_allowed_tag_groups: "Restrict these tag groups to this category:" tags_placeholder: "(Optional) list of allowed tags" tags_tab_description: "Tags and tag groups specified here will only be available in this category and other categories that also specify them. They won't be available for use in other categories." tag_groups_placeholder: "(Optional) list of allowed tag groups" + manage_tag_groups_link: "Manage tag groups here." + allow_global_tags_label: "Also allow other tags" topic_featured_link_allowed: "Allow featured links in this category" delete: "Delete Category" create: "New Category" diff --git a/db/migrate/20190403180142_add_allow_global_tags_to_categories.rb b/db/migrate/20190403180142_add_allow_global_tags_to_categories.rb new file mode 100644 index 00000000000..3ab37e74829 --- /dev/null +++ b/db/migrate/20190403180142_add_allow_global_tags_to_categories.rb @@ -0,0 +1,5 @@ +class AddAllowGlobalTagsToCategories < ActiveRecord::Migration[5.2] + def change + add_column :categories, :allow_global_tags, :boolean, default: false, null: false + end +end diff --git a/lib/discourse_tagging.rb b/lib/discourse_tagging.rb index 5ba25636534..1b26986707c 100644 --- a/lib/discourse_tagging.rb +++ b/lib/discourse_tagging.rb @@ -100,21 +100,35 @@ module DiscourseTagging category = opts[:category] if category && (category.tags.count > 0 || category.tag_groups.count > 0) - if category.tags.count > 0 && category.tag_groups.count > 0 - tag_group_ids = category.tag_groups.pluck(:id) - - query = query.where( - "tags.id IN (SELECT tag_id FROM category_tags WHERE category_id = ? + if category.allow_global_tags + # Select tags that: + # * are restricted to the given category + # * belong to no tag groups and aren't restricted to other categories + # * belong to tag groups that are not restricted to any categories + query = query.where(<<~SQL, category.tag_groups.pluck(:id), category.id) + tags.id IN ( + SELECT t.id FROM tags t + LEFT JOIN category_tags ct ON t.id = ct.tag_id + LEFT JOIN ( + SELECT xtgm.tag_id, xtgm.tag_group_id + FROM tag_group_memberships xtgm + INNER JOIN category_tag_groups ctg + ON xtgm.tag_group_id = ctg.tag_group_id + ) AS tgm ON t.id = tgm.tag_id + WHERE (tgm.tag_group_id IS NULL AND ct.category_id IS NULL) + OR tgm.tag_group_id IN (?) + OR ct.category_id = ? + ) + SQL + else + # Select only tags that are restricted to the given category + query = query.where(<<~SQL, category.id, category.tag_groups.pluck(:id)) + tags.id IN ( + SELECT tag_id FROM category_tags WHERE category_id = ? UNION - SELECT tag_id FROM tag_group_memberships WHERE tag_group_id IN (?))", - category.id, tag_group_ids - ) - elsif category.tags.count > 0 - query = query.where("tags.id IN (SELECT tag_id FROM category_tags WHERE category_id = ?)", category.id) - else # category.tag_groups.count > 0 - tag_group_ids = category.tag_groups.pluck(:id) - - query = query.where("tags.id IN (SELECT tag_id FROM tag_group_memberships WHERE tag_group_id IN (?))", tag_group_ids) + SELECT tag_id FROM tag_group_memberships WHERE tag_group_id IN (?) + ) + SQL end elsif opts[:for_input] || opts[:for_topic] || category # exclude tags that are restricted to other categories diff --git a/spec/integration/category_tag_spec.rb b/spec/integration/category_tag_spec.rb index 86ff857cb34..11b003e35c6 100644 --- a/spec/integration/category_tag_spec.rb +++ b/spec/integration/category_tag_spec.rb @@ -28,8 +28,8 @@ describe "category tag restrictions" do end context "tags restricted to one category" do - let(:category_with_tags) { Fabricate(:category) } - let(:other_category) { Fabricate(:category) } + let!(:category_with_tags) { Fabricate(:category) } + let!(:other_category) { Fabricate(:category) } before do category_with_tags.tags = [tag1, tag2] @@ -49,8 +49,13 @@ describe "category tag restrictions" do expect(filter_allowed_tags(for_input: true)).to contain_exactly(tag3, tag4) expect(filter_allowed_tags(for_input: true, category: category_with_tags, selected_tags: [tag1.name])).to contain_exactly(tag2) expect(filter_allowed_tags(for_input: true, category: category_with_tags, selected_tags: [tag1.name], term: 'tag')).to contain_exactly(tag2) + expect(filter_allowed_tags(for_input: true, category: other_category)).to contain_exactly(tag3, tag4) + expect(filter_allowed_tags(for_input: true, category: other_category, selected_tags: [tag3.name])).to contain_exactly(tag4) + expect(filter_allowed_tags(for_input: true, category: other_category, selected_tags: [tag3.name], term: 'tag')).to contain_exactly(tag4) end + + it "can't create new tags in a restricted category" do post = create_post(category: category_with_tags, tags: [tag1.name, "newtag"]) expect(post.topic.tags).to contain_exactly(tag1) @@ -67,21 +72,44 @@ describe "category tag restrictions" do expect { other_category.update(allowed_tags: ['newtag']) }.to change { Tag.count }.by(1) expect { other_category.update(allowed_tags: [tag1.name, 'tag-stuff', tag2.name, 'another-tag']) }.to change { Tag.count }.by(2) end + + context 'category allows other tags to be used' do + before do + category_with_tags.update_attributes!(allow_global_tags: true) + end + + it "search can show the permitted tags" do + expect(filter_allowed_tags.count).to eq(Tag.count) + expect(filter_allowed_tags(for_input: true, category: category_with_tags)).to contain_exactly(tag1, tag2, tag3, tag4) + expect(filter_allowed_tags(for_input: true)).to contain_exactly(tag3, tag4) + expect(filter_allowed_tags(for_input: true, category: category_with_tags, selected_tags: [tag1.name])).to contain_exactly(tag2, tag3, tag4) + expect(filter_allowed_tags(for_input: true, category: category_with_tags, selected_tags: [tag1.name], term: 'tag')).to contain_exactly(tag2, tag3, tag4) + expect(filter_allowed_tags(for_input: true, category: other_category)).to contain_exactly(tag3, tag4) + expect(filter_allowed_tags(for_input: true, category: other_category, selected_tags: [tag3.name])).to contain_exactly(tag4) + expect(filter_allowed_tags(for_input: true, category: other_category, selected_tags: [tag3.name], term: 'tag')).to contain_exactly(tag4) + end + + it "works if no tags are restricted to the category" do + other_category.update_attributes!(allow_global_tags: true) + expect(filter_allowed_tags(for_input: true, category: other_category)).to contain_exactly(tag3, tag4) + expect(filter_allowed_tags(for_input: true, category: other_category, selected_tags: [tag3.name])).to contain_exactly(tag4) + expect(filter_allowed_tags(for_input: true, category: other_category, selected_tags: [tag3.name], term: 'tag')).to contain_exactly(tag4) + end + end end context "tag groups restricted to a category" do let!(:tag_group1) { Fabricate(:tag_group) } - let(:category) { Fabricate(:category) } - let(:other_category) { Fabricate(:category) } + let!(:category) { Fabricate(:category) } + let!(:other_category) { Fabricate(:category) } before do tag_group1.tags = [tag1, tag2] + category.allowed_tag_groups = [tag_group1.name] + category.reload end it "tags in the group are used by category tag restrictions" do - category.allowed_tag_groups = [tag_group1.name] - category.reload - expect(filter_allowed_tags(for_input: true, category: category)).to contain_exactly(tag1, tag2) expect(filter_allowed_tags(for_input: true)).to contain_exactly(tag3, tag4) @@ -92,7 +120,6 @@ describe "category tag restrictions" do end it "groups and individual tags can be mixed" do - category.allowed_tag_groups = [tag_group1.name] category.allowed_tags = [tag4.name] category.reload @@ -102,11 +129,72 @@ describe "category tag restrictions" do end it "enforces restrictions when creating a topic" do - category.allowed_tag_groups = [tag_group1.name] - category.reload post = create_post(category: category, tags: [tag1.name, "newtag"]) expect(post.topic.tags.map(&:name)).to eq([tag1.name]) end + + context 'category allows other tags to be used' do + before do + category.update_attributes!(allow_global_tags: true) + end + + it 'filters tags correctly' do + expect(filter_allowed_tags(for_input: true, category: category)).to contain_exactly(tag1, tag2, tag3, tag4) + expect(filter_allowed_tags(for_input: true)).to contain_exactly(tag3, tag4) + expect(filter_allowed_tags(for_input: true, category: other_category)).to contain_exactly(tag3, tag4) + + tag_group1.tags = [tag2, tag3, tag4] + expect(filter_allowed_tags(for_input: true, category: category)).to contain_exactly(tag1, tag2, tag3, tag4) + expect(filter_allowed_tags(for_input: true)).to contain_exactly(tag1) + expect(filter_allowed_tags(for_input: true, category: other_category)).to contain_exactly(tag1) + end + + it "works if no tags are restricted to the category" do + other_category.update_attributes!(allow_global_tags: true) + expect(filter_allowed_tags(for_input: true, category: other_category)).to contain_exactly(tag3, tag4) + tag_group1.tags = [tag2, tag3, tag4] + expect(filter_allowed_tags(for_input: true, category: other_category)).to contain_exactly(tag1) + end + + context 'another category has restricted tags using groups' do + let(:category2) { Fabricate(:category) } + let(:tag_group2) { Fabricate(:tag_group) } + + before do + tag_group2.tags = [tag2, tag3] + category2.allowed_tag_groups = [tag_group2.name] + category2.reload + end + + it 'filters tags correctly' do + expect(filter_allowed_tags(for_input: true, category: category2)).to contain_exactly(tag2, tag3) + expect(filter_allowed_tags(for_input: true)).to contain_exactly(tag4) + expect(filter_allowed_tags(for_input: true, category: other_category)).to contain_exactly(tag4) + expect(filter_allowed_tags(for_input: true, category: category)).to contain_exactly(tag1, tag2, tag4) + end + + it "doesn't care about tags in a group that isn't used in a category" do + unused_tag_group = Fabricate(:tag_group) + unused_tag_group.tags = [tag4] + expect(filter_allowed_tags(for_input: true, category: category2)).to contain_exactly(tag2, tag3) + expect(filter_allowed_tags(for_input: true)).to contain_exactly(tag4) + expect(filter_allowed_tags(for_input: true, category: other_category)).to contain_exactly(tag4) + expect(filter_allowed_tags(for_input: true, category: category)).to contain_exactly(tag1, tag2, tag4) # tag4 missing + end + end + + context 'another category has restricted tags' do + let(:category2) { Fabricate(:category) } + + it "doesn't filter tags that are also restricted in another category" do + category2.tags = [tag2, tag3] + expect(filter_allowed_tags(for_input: true, category: category2)).to contain_exactly(tag2, tag3) + expect(filter_allowed_tags(for_input: true)).to contain_exactly(tag4) + expect(filter_allowed_tags(for_input: true, category: other_category)).to contain_exactly(tag4) + expect(filter_allowed_tags(for_input: true, category: category)).to contain_exactly(tag1, tag2, tag4) # tag2 missing + end + end + end end context "tag groups with parent tag" do diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb index 821e55790d9..b94ef713cfb 100644 --- a/spec/requests/categories_controller_spec.rb +++ b/spec/requests/categories_controller_spec.rb @@ -311,7 +311,7 @@ describe CategoriesController do end describe "success" do - it "updates the group correctly" do + it "updates attributes correctly" do readonly = CategoryGroup.permission_types[:readonly] create_post = CategoryGroup.permission_types[:create_post] @@ -328,7 +328,8 @@ describe CategoriesController do custom_fields: { "dancing" => "frogs" }, - minimum_required_tags: "" + minimum_required_tags: "", + allow_global_tags: 'true' } expect(response.status).to eq(200) @@ -342,6 +343,7 @@ describe CategoriesController do expect(category.auto_close_hours).to eq(72) expect(category.custom_fields).to eq("dancing" => "frogs") expect(category.minimum_required_tags).to eq(0) + expect(category.allow_global_tags).to eq(true) end it 'logs the changes correctly' do