FEATURE: restrict tags to be used in a category

This commit is contained in:
Neil Lalonde 2016-05-30 16:37:06 -04:00
parent 26f25fc0d9
commit 6796b15857
17 changed files with 168 additions and 32 deletions

View File

@ -0,0 +1,4 @@
import { buildCategoryPanel } from 'discourse/components/edit-category-panel';
export default buildCategoryPanel('tags', {
});

View File

@ -6,7 +6,7 @@ function formatTag(t) {
export default Ember.TextField.extend({ export default Ember.TextField.extend({
classNameBindings: [':tag-chooser'], classNameBindings: [':tag-chooser'],
attributeBindings: ['tabIndex'], attributeBindings: ['tabIndex', 'placeholderKey', 'categoryId'],
_setupTags: function() { _setupTags: function() {
const tags = this.get('tags') || []; const tags = this.get('tags') || [];
@ -25,7 +25,7 @@ export default Ember.TextField.extend({
this.$().select2({ this.$().select2({
tags: true, 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, maximumInputLength: this.siteSettings.max_tag_length,
maximumSelectionSize: this.siteSettings.max_tags_per_topic, maximumSelectionSize: this.siteSettings.max_tags_per_topic,
initSelection(element, callback) { initSelection(element, callback) {
@ -78,7 +78,7 @@ export default Ember.TextField.extend({
url: Discourse.getURL("/tags/filter/search"), url: Discourse.getURL("/tags/filter/search"),
dataType: 'json', dataType: 'json',
data: function (term) { 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) { results: function (data) {
if (self.siteSettings.tags_sort_alphabetically) { if (self.siteSettings.tags_sort_alphabetically) {

View File

@ -86,7 +86,8 @@ const Category = RestModel.extend({
allow_badges: this.get('allow_badges'), allow_badges: this.get('allow_badges'),
custom_fields: this.get('custom_fields'), custom_fields: this.get('custom_fields'),
topic_template: this.get('topic_template'), 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' type: this.get('id') ? 'PUT' : 'POST'
}); });

View File

@ -0,0 +1,4 @@
<section class="field">
<p>{{i18n 'category.tags_allowed_tags'}}</p>
{{tag-chooser placeholderKey="category.tags_placeholder" tags=category.allowed_tags}}
</section>

View File

@ -98,7 +98,7 @@
<div class='submit-panel'> <div class='submit-panel'>
{{plugin-outlet "composer-fields-below"}} {{plugin-outlet "composer-fields-below"}}
{{#if canEditTags}} {{#if canEditTags}}
{{tag-chooser tags=model.tags tabIndex="4"}} {{tag-chooser tags=model.tags tabIndex="4" categoryId=model.categoryId}}
{{/if}} {{/if}}
<button {{action "save"}} tabindex="5" class="btn btn-primary create {{if disableSubmit 'disabled'}}" title="{{i18n 'composer.title'}}">{{{model.saveIcon}}}{{model.saveText}}</button> <button {{action "save"}} tabindex="5" class="btn btn-primary create {{if disableSubmit 'disabled'}}" title="{{i18n 'composer.title'}}">{{{model.saveIcon}}}{{model.saveText}}</button>
<a href {{action "cancel"}} class='cancel' tabindex="6">{{i18n 'cancel'}}</a> <a href {{action "cancel"}} class='cancel' tabindex="6">{{i18n 'cancel'}}</a>

View File

@ -7,6 +7,9 @@
{{edit-category-tab panels=panels selectedTab=selectedTab tab="settings"}} {{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="images"}}
{{edit-category-tab panels=panels selectedTab=selectedTab tab="topic-template"}} {{edit-category-tab panels=panels selectedTab=selectedTab tab="topic-template"}}
{{#if siteSettings.tagging_enabled}}
{{edit-category-tab panels=panels selectedTab=selectedTab tab="tags"}}
{{/if}}
</ul> </ul>
<div class="modal-body"> <div class="modal-body">

View File

@ -25,7 +25,7 @@
{{#if canEditTags}} {{#if canEditTags}}
<br> <br>
{{tag-chooser tags=buffered.tags}} {{tag-chooser tags=buffered.tags categoryId=buffered.category_id}}
{{/if}} {{/if}}
{{plugin-outlet "edit-topic"}} {{plugin-outlet "edit-topic"}}

View File

@ -194,7 +194,8 @@ class CategoriesController < ApplicationController
:allow_badges, :allow_badges,
:topic_template, :topic_template,
:custom_fields => [params[:custom_fields].try(:keys)], :custom_fields => [params[:custom_fields].try(:keys)],
:permissions => [*p.try(:keys)]) :permissions => [*p.try(:keys)],
:allowed_tags => [])
end end
end end

View File

@ -104,18 +104,15 @@ class TagsController < ::ApplicationController
end end
def search def search
query = self.class.tags_by_count(guardian, params.slice(:limit)) query = DiscourseTagging.filter_allowed_tags(
term = params[:q] self.class.tags_by_count(guardian, params.slice(:limit)),
if term.present? guardian,
term.gsub!(/[^a-z0-9\.\-\_]*/, '') {
term.gsub!("_", "\\_") for_input: params[:filterForInput],
query = query.where('tags.name like ?', "%#{term}%") term: params[:q],
end category: params[:categoryId] ? Category.find_by_id(params[:categoryId]) : nil
}
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
tags = query.count.map {|t, c| { id: t, text: t, count: c } } tags = query.count.map {|t, c| { id: t, text: t, count: c } }

View File

@ -55,6 +55,9 @@ class Category < ActiveRecord::Base
belongs_to :parent_category, class_name: 'Category' belongs_to :parent_category, class_name: 'Category'
has_many :subcategories, class_name: 'Category', foreign_key: 'parent_category_id' 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 :latest, ->{ order('topic_count desc') }
scope :secured, ->(guardian = nil) { scope :secured, ->(guardian = nil) {
@ -312,6 +315,12 @@ SQL
end end
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 def downcase_email
self.email_in = (email_in || "").strip.downcase.presence self.email_in = (email_in || "").strip.downcase.presence
end end

View File

@ -0,0 +1,4 @@
class CategoryTag < ActiveRecord::Base
belongs_to :category
belongs_to :tag
end

View File

@ -1,8 +1,13 @@
class Tag < ActiveRecord::Base class Tag < ActiveRecord::Base
validates :name, presence: true, uniqueness: true validates :name, presence: true, uniqueness: true
has_many :tag_users # notification settings
has_many :topic_tags, dependent: :destroy has_many :topic_tags, dependent: :destroy
has_many :topics, through: :topic_tags 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={}) def self.tags_by_count_query(opts={})
q = TopicTag.joins(:tag, :topic).group("topic_tags.tag_id, tags.name").order('count_all DESC') q = TopicTag.joins(:tag, :topic).group("topic_tags.tag_id, tags.name").order('count_all DESC')

View File

@ -13,7 +13,8 @@ class CategorySerializer < BasicCategorySerializer
:cannot_delete_reason, :cannot_delete_reason,
:is_special, :is_special,
:allow_badges, :allow_badges,
:custom_fields :custom_fields,
:allowed_tags
def group_permissions def group_permissions
@group_permissions ||= begin @group_permissions ||= begin
@ -77,4 +78,12 @@ class CategorySerializer < BasicCategorySerializer
(user && CategoryUser.where(user: user, category: object).first.try(:notification_level)) (user && CategoryUser.where(user: user, category: object).first.try(:notification_level))
end end
def include_allowed_tags?
SiteSetting.tagging_enabled
end
def allowed_tags
object.tags.pluck(:name)
end
end end

View File

@ -1716,6 +1716,9 @@ en:
general: 'General' general: 'General'
settings: 'Settings' settings: 'Settings'
topic_template: "Topic Template" 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' delete: 'Delete Category'
create: 'New Category' create: 'New Category'
create_long: 'Create a new category' create_long: 'Create a new category'

View File

@ -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

View File

@ -32,12 +32,14 @@ module DiscourseTagging
end end
if tag_names.present? if tag_names.present?
tags = Tag.where(name: tag_names).all category = topic.category
if tags.size < tag_names.size tags = filter_allowed_tags(Tag.where(name: tag_names), guardian, { for_input: true, category: category }).to_a
existing_names = tags.map(&:name)
if tags.size < tag_names.size && (category.nil? || category.tags.count == 0)
tag_names.each do |name| tag_names.each do |name|
next if existing_names.include?(name) unless Tag.where(name: name).exists?
tags << Tag.create(name: name) tags << Tag.create(name: name)
end
end end
end end
@ -52,6 +54,34 @@ module DiscourseTagging
true true
end 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) def self.auto_notify_for(tags, topic)
TagUser.auto_watch_new_topic(topic, tags) TagUser.auto_watch_new_topic(topic, tags)
TagUser.auto_track_new_topic(topic, tags) TagUser.auto_track_new_topic(topic, tags)
@ -78,17 +108,16 @@ module DiscourseTagging
return unless tags.present? return unless tags.present?
tags.map! {|t| clean_tag(t) } tag_names = tags.map {|t| clean_tag(t) }
tags.delete_if {|t| t.blank? } tag_names.delete_if {|t| t.blank? }
tags.uniq! tag_names.uniq!
# If the user can't create tags, remove any tags that don't already exist # 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? unless guardian.can_create_tag?
tags = Tag.where(name: tags).pluck(:name) tag_names = Tag.where(name: tag_names).pluck(:name)
end end
return tags[0...SiteSetting.max_tags_per_topic] return tag_names[0...SiteSetting.max_tags_per_topic]
end end
def self.notification_key(tag_id) def self.notification_key(tag_id)

View File

@ -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