From a49ace0ffbbeac957f8d2b490a6c447100384b55 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 7 Jun 2016 13:08:59 -0400 Subject: [PATCH] FEATURE: ability to restrict tags to categories using groups --- .../discourse/components/tag-chooser.js.es6 | 4 +- .../components/tag-group-chooser.js.es6 | 84 +++++++++++++++++++ .../discourse/models/category.js.es6 | 3 +- .../components/edit-category-tags.hbs | 3 + app/controllers/categories_controller.rb | 8 +- app/controllers/tag_groups_controller.rb | 13 +++ app/models/category.rb | 8 +- app/models/category_tag_group.rb | 4 + app/models/tag_group.rb | 4 + app/serializers/category_serializer.rb | 11 ++- config/locales/client.en.yml | 2 + config/routes.rb | 7 +- ...160606204319_create_category_tag_groups.rb | 11 +++ lib/discourse_tagging.rb | 31 ++++++- spec/fabricators/tag_group_fabricator.rb | 3 + spec/integration/category_tag_spec.rb | 38 +++++++++ 16 files changed, 222 insertions(+), 12 deletions(-) create mode 100644 app/assets/javascripts/discourse/components/tag-group-chooser.js.es6 create mode 100644 app/models/category_tag_group.rb create mode 100644 db/migrate/20160606204319_create_category_tag_groups.rb create mode 100644 spec/fabricators/tag_group_fabricator.rb diff --git a/app/assets/javascripts/discourse/components/tag-chooser.js.es6 b/app/assets/javascripts/discourse/components/tag-chooser.js.es6 index d3daddf44b4..d9b3863cf58 100644 --- a/app/assets/javascripts/discourse/components/tag-chooser.js.es6 +++ b/app/assets/javascripts/discourse/components/tag-chooser.js.es6 @@ -8,7 +8,7 @@ export default Ember.TextField.extend({ classNameBindings: [':tag-chooser'], attributeBindings: ['tabIndex', 'placeholderKey', 'categoryId'], - _setupTags: function() { + _initValue: function() { const tags = this.get('tags') || []; this.set('value', tags.join(", ")); }.on('init'), @@ -79,7 +79,7 @@ export default Ember.TextField.extend({ list.push(item); }, formatSelection: function (data) { - return data ? renderTag(this.text(data)) : undefined; + return data ? renderTag(this.text(data)) : undefined; }, formatSelectionCssClass: function(){ return "discourse-tag-select2"; diff --git a/app/assets/javascripts/discourse/components/tag-group-chooser.js.es6 b/app/assets/javascripts/discourse/components/tag-group-chooser.js.es6 new file mode 100644 index 00000000000..ae9498af2b5 --- /dev/null +++ b/app/assets/javascripts/discourse/components/tag-group-chooser.js.es6 @@ -0,0 +1,84 @@ +function renderTagGroup(tag) { + return "" + Handlebars.Utils.escapeExpression(tag.text ? tag.text : tag) + ""; +}; + +export default Ember.TextField.extend({ + classNameBindings: [':tag-chooser'], + attributeBindings: ['tabIndex', 'placeholderKey', 'categoryId'], + + _initValue: function() { + const names = this.get('tagGroups') || []; + this.set('value', names.join(", ")); + }.on('init'), + + _valueChanged: function() { + const names = this.get('value').split(',').map(v => v.trim()).reject(v => v.length === 0).uniq(); + this.set('tagGroups', names); + }.observes('value'), + + _tagGroupsChanged: function() { + const $chooser = this.$(), + val = this.get('value'); + + if ($chooser && val !== this.get('tagGroups')) { + if (this.get('tagGroups')) { + const data = this.get('tagGroups').map((t) => {return {id: t, text: t};}); + $chooser.select2('data', data); + } else { + $chooser.select2('data', []); + } + } + }.observes('tagGroups'), + + _initializeChooser: function() { + const self = this; + + this.$().select2({ + tags: true, + placeholder: this.get('placeholderKey') ? I18n.t(this.get('placeholderKey')) : null, + initSelection(element, callback) { + const data = []; + + function splitVal(string, separator) { + var val, i, l; + if (string === null || string.length < 1) return []; + val = string.split(separator); + for (i = 0, l = val.length; i < l; i = i + 1) val[i] = $.trim(val[i]); + return val; + } + + $(splitVal(element.val(), ",")).each(function () { + data.push({ id: this, text: this }); + }); + + callback(data); + }, + formatSelection: function (data) { + return data ? renderTagGroup(this.text(data)) : undefined; + }, + formatSelectionCssClass: function(){ + return "discourse-tag-select2"; + }, + formatResult: renderTagGroup, + multiple: true, + ajax: { + quietMillis: 200, + cache: true, + url: Discourse.getURL("/tag_groups/filter/search"), + dataType: 'json', + data: function (term) { + return { q: term, limit: self.siteSettings.max_tag_search_results }; + }, + results: function (data) { + data.results = data.results.sort(function(a,b) { return a.text > b.text; }); + return data; + } + }, + }); + }.on('didInsertElement'), + + _destroyChooser: function() { + this.$().select2('destroy'); + }.on('willDestroyElement') + +}); diff --git a/app/assets/javascripts/discourse/models/category.js.es6 b/app/assets/javascripts/discourse/models/category.js.es6 index 0e013cf8337..8d1eb8ba879 100644 --- a/app/assets/javascripts/discourse/models/category.js.es6 +++ b/app/assets/javascripts/discourse/models/category.js.es6 @@ -87,7 +87,8 @@ const Category = RestModel.extend({ custom_fields: this.get('custom_fields'), topic_template: this.get('topic_template'), suppress_from_homepage: this.get('suppress_from_homepage'), - allowed_tags: this.get('allowed_tags') + allowed_tags: this.get('allowed_tags'), + allowed_tag_groups: this.get('allowed_tag_groups') }, 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 index 632690bedc2..abd70f23074 100644 --- a/app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs +++ b/app/assets/javascripts/discourse/templates/components/edit-category-tags.hbs @@ -1,4 +1,7 @@

{{i18n 'category.tags_allowed_tags'}}

{{tag-chooser placeholderKey="category.tags_placeholder" tags=category.allowed_tags}} + +

{{i18n 'category.tags_allowed_tag_groups'}}

+ {{tag-group-chooser placeholderKey="category.tag_groups_placeholder" tagGroups=category.allowed_tag_groups}}
diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index 954cd3efcb3..8617970c820 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -180,7 +180,10 @@ class CategoriesController < ApplicationController end end - params[:allowed_tags] ||= [] if SiteSetting.tagging_enabled + if SiteSetting.tagging_enabled + params[:allowed_tags] ||= [] + params[:allowed_tag_groups] ||= [] + end params.permit(*required_param_keys, :position, @@ -197,7 +200,8 @@ class CategoriesController < ApplicationController :topic_template, :custom_fields => [params[:custom_fields].try(:keys)], :permissions => [*p.try(:keys)], - :allowed_tags => []) + :allowed_tags => [], + :allowed_tag_groups => []) end end diff --git a/app/controllers/tag_groups_controller.rb b/app/controllers/tag_groups_controller.rb index 9ba20c9da9c..4bfc943a160 100644 --- a/app/controllers/tag_groups_controller.rb +++ b/app/controllers/tag_groups_controller.rb @@ -49,6 +49,19 @@ class TagGroupsController < ApplicationController render json: success_json end + def search + matches = if params[:q].present? + term = params[:q].strip.downcase + TagGroup.where('lower(name) like ?', "%#{term}%") + else + TagGroup.all + end + + matches = matches.order('name').limit(params[:limit] || 5) + + render json: { results: matches.map { |x| { id: x.name, text: x.name } } } + end + private def fetch_tag_group diff --git a/app/models/category.rb b/app/models/category.rb index 97341b9a48b..92f0b751723 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -55,8 +55,10 @@ 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 :category_tags, dependent: :destroy has_many :tags, through: :category_tags + has_many :category_tag_groups, dependent: :destroy + has_many :tag_groups, through: :category_tag_groups scope :latest, ->{ order('topic_count desc') } @@ -319,6 +321,10 @@ SQL DiscourseTagging.add_or_create_tags_by_name(self, tag_names_arg) end + def allowed_tag_groups=(group_names) + self.tag_groups = TagGroup.where(name: group_names).all.to_a + end + def downcase_email self.email_in = (email_in || "").strip.downcase.presence end diff --git a/app/models/category_tag_group.rb b/app/models/category_tag_group.rb new file mode 100644 index 00000000000..b73c6788ddd --- /dev/null +++ b/app/models/category_tag_group.rb @@ -0,0 +1,4 @@ +class CategoryTagGroup < ActiveRecord::Base + belongs_to :category + belongs_to :tag_group +end diff --git a/app/models/tag_group.rb b/app/models/tag_group.rb index 9bd7f11bb2b..dea1199a62a 100644 --- a/app/models/tag_group.rb +++ b/app/models/tag_group.rb @@ -1,6 +1,10 @@ class TagGroup < ActiveRecord::Base + validates_uniqueness_of :name, case_sensitive: false + has_many :tag_group_memberships, dependent: :destroy has_many :tags, through: :tag_group_memberships + has_many :category_tag_groups, dependent: :destroy + has_many :categories, through: :category_tag_groups def tag_names=(tag_names_arg) DiscourseTagging.add_or_create_tags_by_name(self, tag_names_arg) diff --git a/app/serializers/category_serializer.rb b/app/serializers/category_serializer.rb index bf83663d485..bb9758cad16 100644 --- a/app/serializers/category_serializer.rb +++ b/app/serializers/category_serializer.rb @@ -14,7 +14,8 @@ class CategorySerializer < BasicCategorySerializer :is_special, :allow_badges, :custom_fields, - :allowed_tags + :allowed_tags, + :allowed_tag_groups def group_permissions @group_permissions ||= begin @@ -86,4 +87,12 @@ class CategorySerializer < BasicCategorySerializer object.tags.pluck(:name) end + def include_allowed_tag_groups? + SiteSetting.tagging_enabled + end + + def allowed_tag_groups + object.tag_groups.pluck(:name) + end + end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 4ee08c4f332..3c3a4f6ddd7 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1732,7 +1732,9 @@ en: topic_template: "Topic Template" tags: "Tags" tags_allowed_tags: "Tags that can only be used in this category:" + tags_allowed_tag_groups: "Tag groups that can only be used in this category:" tags_placeholder: "(Optional) list of allowed tags" + tag_groups_placeholder: "(Optional) list of allowed tag groups" delete: 'Delete Category' create: 'New Category' create_long: 'Create a new category' diff --git a/config/routes.rb b/config/routes.rb index ff509dbf720..f7839f043b1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -636,7 +636,12 @@ Discourse::Application.routes.draw do end end end - resources :tag_groups, except: [:new, :edit] + + resources :tag_groups, except: [:new, :edit] do + collection do + get '/filter/search' => 'tag_groups#search' + end + end Discourse.filters.each do |filter| root to: "list##{filter}", constraints: HomePageConstraint.new("#{filter}"), :as => "list_#{filter}" diff --git a/db/migrate/20160606204319_create_category_tag_groups.rb b/db/migrate/20160606204319_create_category_tag_groups.rb new file mode 100644 index 00000000000..628f131ddc2 --- /dev/null +++ b/db/migrate/20160606204319_create_category_tag_groups.rb @@ -0,0 +1,11 @@ +class CreateCategoryTagGroups < ActiveRecord::Migration + def change + create_table :category_tag_groups do |t| + t.references :category, null: false + t.references :tag_group, null: false + t.timestamps + end + + add_index :category_tag_groups, [:category_id, :tag_group_id], name: "idx_category_tag_groups_ix1", unique: true + end +end diff --git a/lib/discourse_tagging.rb b/lib/discourse_tagging.rb index 48ce1c00dc8..72ef4aecd11 100644 --- a/lib/discourse_tagging.rb +++ b/lib/discourse_tagging.rb @@ -72,10 +72,33 @@ module DiscourseTagging 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)") + # Filters for category-specific tags: + + if opts[:category] && (opts[:category].tags.count > 0 || opts[:category].tag_groups.count > 0) + if opts[:category].tags.count > 0 && opts[:category].tag_groups.count > 0 + tag_group_ids = opts[:category].tag_groups.pluck(:id) + query = query.where( + "tags.id IN (SELECT tag_id FROM category_tags WHERE category_id = ? + UNION + SELECT tag_id FROM tag_group_memberships WHERE tag_group_id = ?)", + opts[:category].id, tag_group_ids + ) + elsif opts[:category].tags.count > 0 + query = query.where("tags.id IN (SELECT tag_id FROM category_tags WHERE category_id = ?)", opts[:category].id) + else # opts[:category].tag_groups.count > 0 + tag_group_ids = opts[:category].tag_groups.pluck(:id) + query = query.where("tags.id IN (SELECT tag_id FROM tag_group_memberships WHERE tag_group_id = ?)", tag_group_ids) + end + else + # exclude tags that are restricted to other categories + if CategoryTag.exists? + query = query.where("tags.id NOT IN (SELECT tag_id FROM category_tags)") + end + + if CategoryTagGroup.exists? + tag_group_ids = CategoryTagGroup.pluck(:tag_group_id).uniq + query = query.where("tags.id NOT IN (SELECT tag_id FROM tag_group_memberships WHERE tag_group_id = ?)", tag_group_ids) + end end end diff --git a/spec/fabricators/tag_group_fabricator.rb b/spec/fabricators/tag_group_fabricator.rb new file mode 100644 index 00000000000..f410aa5bd75 --- /dev/null +++ b/spec/fabricators/tag_group_fabricator.rb @@ -0,0 +1,3 @@ +Fabricator(:tag_group) do + name { sequence(:name) { |i| "tag_group_#{i}" } } +end diff --git a/spec/integration/category_tag_spec.rb b/spec/integration/category_tag_spec.rb index 358db1e0551..9b2f0c3a90d 100644 --- a/spec/integration/category_tag_spec.rb +++ b/spec/integration/category_tag_spec.rb @@ -4,6 +4,11 @@ require 'rails_helper' require_dependency 'post_creator' describe "category tag restrictions" do + + def sorted_tag_names(tag_records) + tag_records.map(&:name).sort + end + let!(:tag1) { Fabricate(:tag) } let!(:tag2) { Fabricate(:tag) } let!(:tag3) { Fabricate(:tag) } @@ -57,4 +62,37 @@ describe "category tag restrictions" do expect { other_category.update(allowed_tags: [tag1.name, 'tag-stuff', tag2.name, 'another-tag']) }.to change { Tag.count }.by(2) 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) } + + before do + tag_group1.tags = [tag1, tag2] + end + + it "tags in the group are used by category tag restrictions" do + category.allowed_tag_groups = [tag_group1.name] + category.reload + + expect(sorted_tag_names(DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), {for_input: true, category: category}))).to eq(sorted_tag_names([tag1, tag2])) + expect(sorted_tag_names(DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), {for_input: true}))).to eq(sorted_tag_names([tag3, tag4])) + + tag_group1.tags = [tag2, tag3, tag4] + expect(sorted_tag_names(DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), {for_input: true, category: category}))).to eq(sorted_tag_names([tag2, tag3, tag4])) + expect(sorted_tag_names(DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), {for_input: true}))).to eq(sorted_tag_names([tag1])) + expect(sorted_tag_names(DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), {for_input: true, category: other_category}))).to eq(sorted_tag_names([tag1])) + end + + it "groups and individual tags can be mixed" do + category.allowed_tag_groups = [tag_group1.name] + category.allowed_tags = [tag4.name] + category.reload + + expect(sorted_tag_names(DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), {for_input: true, category: category}))).to eq(sorted_tag_names([tag1, tag2, tag4])) + expect(sorted_tag_names(DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), {for_input: true}))).to eq(sorted_tag_names([tag3])) + expect(sorted_tag_names(DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), {for_input: true, category: other_category}))).to eq(sorted_tag_names([tag3])) + end + end end