From 6796b1585797306cff8bb0a9104f7bb7f19ac919 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Mon, 30 May 2016 16:37:06 -0400 Subject: [PATCH] FEATURE: restrict tags to be used in a category --- .../components/edit-category-tags.js.es6 | 4 ++ .../discourse/components/tag-chooser.js.es6 | 6 +- .../discourse/models/category.js.es6 | 3 +- .../components/edit-category-tags.hbs | 4 ++ .../discourse/templates/composer.hbs | 2 +- .../templates/modal/edit-category.hbs | 3 + .../javascripts/discourse/templates/topic.hbs | 2 +- app/controllers/categories_controller.rb | 3 +- app/controllers/tags_controller.rb | 21 +++---- app/models/category.rb | 9 +++ app/models/category_tag.rb | 4 ++ app/models/tag.rb | 7 ++- app/serializers/category_serializer.rb | 11 +++- config/locales/client.en.yml | 3 + .../20160527191614_create_category_tags.rb | 12 ++++ lib/discourse_tagging.rb | 51 +++++++++++++---- spec/integration/category_tag_spec.rb | 55 +++++++++++++++++++ 17 files changed, 168 insertions(+), 32 deletions(-) create mode 100644 app/assets/javascripts/discourse/components/edit-category-tags.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs create mode 100644 app/models/category_tag.rb create mode 100644 db/migrate/20160527191614_create_category_tags.rb create mode 100644 spec/integration/category_tag_spec.rb diff --git a/app/assets/javascripts/discourse/components/edit-category-tags.js.es6 b/app/assets/javascripts/discourse/components/edit-category-tags.js.es6 new file mode 100644 index 0000000000..22bf364e96 --- /dev/null +++ b/app/assets/javascripts/discourse/components/edit-category-tags.js.es6 @@ -0,0 +1,4 @@ +import { buildCategoryPanel } from 'discourse/components/edit-category-panel'; + +export default buildCategoryPanel('tags', { +}); diff --git a/app/assets/javascripts/discourse/components/tag-chooser.js.es6 b/app/assets/javascripts/discourse/components/tag-chooser.js.es6 index 88e7577d1e..e69ba2df5c 100644 --- a/app/assets/javascripts/discourse/components/tag-chooser.js.es6 +++ b/app/assets/javascripts/discourse/components/tag-chooser.js.es6 @@ -6,7 +6,7 @@ function formatTag(t) { export default Ember.TextField.extend({ classNameBindings: [':tag-chooser'], - attributeBindings: ['tabIndex'], + attributeBindings: ['tabIndex', 'placeholderKey', 'categoryId'], _setupTags: function() { const tags = this.get('tags') || []; @@ -25,7 +25,7 @@ export default Ember.TextField.extend({ this.$().select2({ tags: true, - placeholder: I18n.t('tagging.choose_for_topic'), + placeholder: I18n.t(this.get('placeholderKey') || 'tagging.choose_for_topic'), maximumInputLength: this.siteSettings.max_tag_length, maximumSelectionSize: this.siteSettings.max_tags_per_topic, initSelection(element, callback) { @@ -78,7 +78,7 @@ export default Ember.TextField.extend({ url: Discourse.getURL("/tags/filter/search"), dataType: 'json', data: function (term) { - return { q: term, limit: self.siteSettings.max_tag_search_results, filterForInput: true }; + return { q: term, limit: self.siteSettings.max_tag_search_results, filterForInput: true, categoryId: self.get('categoryId') }; }, results: function (data) { if (self.siteSettings.tags_sort_alphabetically) { diff --git a/app/assets/javascripts/discourse/models/category.js.es6 b/app/assets/javascripts/discourse/models/category.js.es6 index 37cfd9f297..0e013cf833 100644 --- a/app/assets/javascripts/discourse/models/category.js.es6 +++ b/app/assets/javascripts/discourse/models/category.js.es6 @@ -86,7 +86,8 @@ const Category = RestModel.extend({ allow_badges: this.get('allow_badges'), custom_fields: this.get('custom_fields'), topic_template: this.get('topic_template'), - suppress_from_homepage: this.get('suppress_from_homepage') + suppress_from_homepage: this.get('suppress_from_homepage'), + allowed_tags: this.get('allowed_tags') }, type: this.get('id') ? 'PUT' : 'POST' }); diff --git a/app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs b/app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs new file mode 100644 index 0000000000..632690bedc --- /dev/null +++ b/app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs @@ -0,0 +1,4 @@ +
+

{{i18n 'category.tags_allowed_tags'}}

+ {{tag-chooser placeholderKey="category.tags_placeholder" tags=category.allowed_tags}} +
diff --git a/app/assets/javascripts/discourse/templates/composer.hbs b/app/assets/javascripts/discourse/templates/composer.hbs index 44c91b570c..eb72ca673b 100644 --- a/app/assets/javascripts/discourse/templates/composer.hbs +++ b/app/assets/javascripts/discourse/templates/composer.hbs @@ -98,7 +98,7 @@
{{plugin-outlet "composer-fields-below"}} {{#if canEditTags}} - {{tag-chooser tags=model.tags tabIndex="4"}} + {{tag-chooser tags=model.tags tabIndex="4" categoryId=model.categoryId}} {{/if}} {{i18n 'cancel'}} diff --git a/app/assets/javascripts/discourse/templates/modal/edit-category.hbs b/app/assets/javascripts/discourse/templates/modal/edit-category.hbs index 6cde3451da..10282704c8 100644 --- a/app/assets/javascripts/discourse/templates/modal/edit-category.hbs +++ b/app/assets/javascripts/discourse/templates/modal/edit-category.hbs @@ -7,6 +7,9 @@ {{edit-category-tab panels=panels selectedTab=selectedTab tab="settings"}} {{edit-category-tab panels=panels selectedTab=selectedTab tab="images"}} {{edit-category-tab panels=panels selectedTab=selectedTab tab="topic-template"}} + {{#if siteSettings.tagging_enabled}} + {{edit-category-tab panels=panels selectedTab=selectedTab tab="tags"}} + {{/if}}
diff --git a/app/assets/javascripts/discourse/templates/topic.hbs b/app/assets/javascripts/discourse/templates/topic.hbs index e654107a45..cb84e78b4f 100644 --- a/app/assets/javascripts/discourse/templates/topic.hbs +++ b/app/assets/javascripts/discourse/templates/topic.hbs @@ -25,7 +25,7 @@ {{#if canEditTags}}
- {{tag-chooser tags=buffered.tags}} + {{tag-chooser tags=buffered.tags categoryId=buffered.category_id}} {{/if}} {{plugin-outlet "edit-topic"}} diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index 5eea9591f0..4cfbeea2a8 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -194,7 +194,8 @@ class CategoriesController < ApplicationController :allow_badges, :topic_template, :custom_fields => [params[:custom_fields].try(:keys)], - :permissions => [*p.try(:keys)]) + :permissions => [*p.try(:keys)], + :allowed_tags => []) end end diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index c5040d3a2b..035febb601 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -104,18 +104,15 @@ class TagsController < ::ApplicationController end def search - query = self.class.tags_by_count(guardian, params.slice(:limit)) - term = params[:q] - if term.present? - term.gsub!(/[^a-z0-9\.\-\_]*/, '') - term.gsub!("_", "\\_") - query = query.where('tags.name like ?', "%#{term}%") - end - - if params[:filterForInput] && !guardian.is_staff? - staff_tag_names = SiteSetting.staff_tags.split("|") - query = query.where('tags.name NOT IN (?)', staff_tag_names) if staff_tag_names.present? - end + query = DiscourseTagging.filter_allowed_tags( + self.class.tags_by_count(guardian, params.slice(:limit)), + guardian, + { + for_input: params[:filterForInput], + term: params[:q], + category: params[:categoryId] ? Category.find_by_id(params[:categoryId]) : nil + } + ) tags = query.count.map {|t, c| { id: t, text: t, count: c } } diff --git a/app/models/category.rb b/app/models/category.rb index 37142c7f86..506950ec1f 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -55,6 +55,9 @@ class Category < ActiveRecord::Base belongs_to :parent_category, class_name: 'Category' has_many :subcategories, class_name: 'Category', foreign_key: 'parent_category_id' + has_many :category_tags + has_many :tags, through: :category_tags + scope :latest, ->{ order('topic_count desc') } scope :secured, ->(guardian = nil) { @@ -312,6 +315,12 @@ SQL end end + def allowed_tags=(tag_names) + if self.tags.pluck(:name).sort != tag_names.sort + self.tags = Tag.where(name: tag_names).all + end + end + def downcase_email self.email_in = (email_in || "").strip.downcase.presence end diff --git a/app/models/category_tag.rb b/app/models/category_tag.rb new file mode 100644 index 0000000000..dcda998ffa --- /dev/null +++ b/app/models/category_tag.rb @@ -0,0 +1,4 @@ +class CategoryTag < ActiveRecord::Base + belongs_to :category + belongs_to :tag +end diff --git a/app/models/tag.rb b/app/models/tag.rb index 3d03aa83d3..df544cd022 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -1,8 +1,13 @@ class Tag < ActiveRecord::Base validates :name, presence: true, uniqueness: true + + has_many :tag_users # notification settings + has_many :topic_tags, dependent: :destroy has_many :topics, through: :topic_tags - has_many :tag_users + + has_many :category_tags, dependent: :destroy + has_many :categories, through: :category_tags def self.tags_by_count_query(opts={}) q = TopicTag.joins(:tag, :topic).group("topic_tags.tag_id, tags.name").order('count_all DESC') diff --git a/app/serializers/category_serializer.rb b/app/serializers/category_serializer.rb index 74947fde9b..bf83663d48 100644 --- a/app/serializers/category_serializer.rb +++ b/app/serializers/category_serializer.rb @@ -13,7 +13,8 @@ class CategorySerializer < BasicCategorySerializer :cannot_delete_reason, :is_special, :allow_badges, - :custom_fields + :custom_fields, + :allowed_tags def group_permissions @group_permissions ||= begin @@ -77,4 +78,12 @@ class CategorySerializer < BasicCategorySerializer (user && CategoryUser.where(user: user, category: object).first.try(:notification_level)) end + def include_allowed_tags? + SiteSetting.tagging_enabled + end + + def allowed_tags + object.tags.pluck(:name) + end + end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index a6dcf4e17a..d583c2bd8c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1716,6 +1716,9 @@ en: general: 'General' settings: 'Settings' topic_template: "Topic Template" + tags: "Tags" + tags_allowed_tags: "Tags that can only be used in this category:" + tags_placeholder: "(Optional) list of allowed tags" delete: 'Delete Category' create: 'New Category' create_long: 'Create a new category' diff --git a/db/migrate/20160527191614_create_category_tags.rb b/db/migrate/20160527191614_create_category_tags.rb new file mode 100644 index 0000000000..9ec7566e6f --- /dev/null +++ b/db/migrate/20160527191614_create_category_tags.rb @@ -0,0 +1,12 @@ +class CreateCategoryTags < ActiveRecord::Migration + def change + create_table :category_tags do |t| + t.references :category, null: false + t.references :tag, null: false + t.timestamps + end + + add_index :category_tags, [:category_id, :tag_id], name: "idx_category_tags_ix1", unique: true + add_index :category_tags, [:tag_id, :category_id], name: "idx_category_tags_ix2", unique: true + end +end diff --git a/lib/discourse_tagging.rb b/lib/discourse_tagging.rb index cb5ce1d7c2..ca6b9bdc5a 100644 --- a/lib/discourse_tagging.rb +++ b/lib/discourse_tagging.rb @@ -32,12 +32,14 @@ module DiscourseTagging end if tag_names.present? - tags = Tag.where(name: tag_names).all - if tags.size < tag_names.size - existing_names = tags.map(&:name) + category = topic.category + tags = filter_allowed_tags(Tag.where(name: tag_names), guardian, { for_input: true, category: category }).to_a + + if tags.size < tag_names.size && (category.nil? || category.tags.count == 0) tag_names.each do |name| - next if existing_names.include?(name) - tags << Tag.create(name: name) + unless Tag.where(name: name).exists? + tags << Tag.create(name: name) + end end end @@ -52,6 +54,34 @@ module DiscourseTagging true end + # Options: + # term: a search term to filter tags by name + # for_input: result is for an input field, so only show permitted tags + # category: a Category to which the object being tagged belongs + def self.filter_allowed_tags(query, guardian, opts={}) + term = opts[:term] + if term.present? + term.gsub!(/[^a-z0-9\.\-\_]*/, '') + term.gsub!("_", "\\_") + query = query.where('tags.name like ?', "%#{term}%") + end + + if opts[:for_input] + unless guardian.is_staff? + staff_tag_names = SiteSetting.staff_tags.split("|") + query = query.where('tags.name NOT IN (?)', staff_tag_names) if staff_tag_names.present? + end + + if opts[:category] && opts[:category].tags.count > 0 + query = query.where("tags.id IN (SELECT tag_id FROM category_tags WHERE category_id = ?)", opts[:category].id) + elsif CategoryTag.exists? + query = query.where("tags.id NOT IN (SELECT tag_id FROM category_tags)") + end + end + + query + end + def self.auto_notify_for(tags, topic) TagUser.auto_watch_new_topic(topic, tags) TagUser.auto_track_new_topic(topic, tags) @@ -78,17 +108,16 @@ module DiscourseTagging return unless tags.present? - tags.map! {|t| clean_tag(t) } - tags.delete_if {|t| t.blank? } - tags.uniq! + tag_names = tags.map {|t| clean_tag(t) } + tag_names.delete_if {|t| t.blank? } + tag_names.uniq! # If the user can't create tags, remove any tags that don't already exist - # TODO: this is doing a full count, it should just check first or use a cache unless guardian.can_create_tag? - tags = Tag.where(name: tags).pluck(:name) + tag_names = Tag.where(name: tag_names).pluck(:name) end - return tags[0...SiteSetting.max_tags_per_topic] + return tag_names[0...SiteSetting.max_tags_per_topic] end def self.notification_key(tag_id) diff --git a/spec/integration/category_tag_spec.rb b/spec/integration/category_tag_spec.rb new file mode 100644 index 0000000000..0c3c4fd95f --- /dev/null +++ b/spec/integration/category_tag_spec.rb @@ -0,0 +1,55 @@ +# encoding: UTF-8 + +require 'rails_helper' +require_dependency 'post_creator' + +describe "category tag restrictions" do + let!(:tag1) { Fabricate(:tag) } + let!(:tag2) { Fabricate(:tag) } + let!(:tag3) { Fabricate(:tag) } + let!(:tag4) { Fabricate(:tag) } + + let(:user) { Fabricate(:user) } + let(:admin) { Fabricate(:admin) } + + before do + SiteSetting.tagging_enabled = true + SiteSetting.min_trust_to_create_tag = 0 + SiteSetting.min_trust_level_to_tag_topics = 0 + end + + context "tags restricted to one category" do + let(:category_with_tags) { Fabricate(:category) } + let(:other_category) { Fabricate(:category) } + + before do + category_with_tags.tags = [tag1, tag2] + end + + it "tags belonging to that category can only be used there" do + post = create_post(category: category_with_tags, tags: [tag1.name, tag2.name, tag3.name]) + expect(post.topic.tags.map(&:name).sort).to eq([tag1.name, tag2.name].sort) + + post = create_post(category: other_category, tags: [tag1.name, tag2.name, tag3.name]) + expect(post.topic.tags.map(&:name)).to eq([tag3.name]) + end + + it "search can show only permitted tags" do + expect(DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user)).count).to eq(Tag.count) + expect(DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), {for_input: true, category: category_with_tags}).pluck(:name).sort).to eq([tag1.name, tag2.name].sort) + expect(DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), {for_input: true}).pluck(:name).sort).to eq([tag3.name, tag4.name].sort) + 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.map(&:name)).to eq([tag1.name]) + post = create_post(category: category_with_tags, tags: [tag1.name, "newtag"], user: admin) + expect(post.topic.tags.map(&:name)).to eq([tag1.name]) + end + + it "can create new tags in a non-restricted category" do + post = create_post(category: other_category, tags: [tag3.name, "newtag"]) + expect(post.topic.tags.map(&:name).sort).to eq([tag3.name, "newtag"].sort) + end + end +end