Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FEATURE: tag group options: limit usage of one tag per group, tags in a group can't be used unless a prerequisite tag is used

This commit is contained in:
Neil Lalonde 2016-06-09 16:00:19 -04:00
parent 171dbd4b09
commit a6090339a7
14 changed files with 243 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
app/assets/javascripts/discourse/components/tag-chooser.js.es6
View File

@ -36,12 +36,19 @@ export default Ember.TextField.extend({
const site = this.site,
self = this,
filterRegexp = new RegExp(this.site.tags_filter_regexp, "g");
var limit = this.siteSettings.max_tags_per_topic;
if (this.get('unlimitedTagCount')) {
limit = null;
} else if (this.get('limit')) {
limit = parseInt(this.get('limit'));
}
this.$().select2({
tags: true,
placeholder: I18n.t(this.get('placeholderKey') || 'tagging.choose_for_topic'),
maximumInputLength: this.siteSettings.max_tag_length,
maximumSelectionSize: self.get('unlimitedTagCount') ? null : this.siteSettings.max_tags_per_topic,
maximumSelectionSize: limit,
initSelection(element, callback) {
const data = [];
@ -92,7 +99,12 @@ export default Ember.TextField.extend({
url: Discourse.getURL("/tags/filter/search"),
dataType: 'json',
data: function (term) {
const d = { q: term, limit: self.siteSettings.max_tag_search_results, categoryId: self.get('categoryId') };
const d = {
q: term,
limit: self.siteSettings.max_tag_search_results,
categoryId: self.get('categoryId'),
selected_tags: self.get('tags')
};
if (!self.get('everyTag')) {
d.filterForInput = true;
}

4
app/assets/javascripts/discourse/models/tag-group.js.es6
View File

@ -20,7 +20,9 @@ const TagGroup = RestModel.extend({
return Discourse.ajax(url, {
data: {
name: this.get('name'),
tag_names: this.get('tag_names')
tag_names: this.get('tag_names'),
parent_tag_name: this.get('parent_tag_name') ? this.get('parent_tag_name') : undefined,
one_per_topic: this.get('one_per_topic')
},
type: this.get('id') ? 'PUT' : 'POST'
}).then(function(result) {

26
app/assets/javascripts/discourse/templates/tag-groups-show.hbs
View File

@ -1,13 +1,25 @@
<div class="tag-group-content">
<h1>{{text-field value=model.name}}</h1>
<br/>
<div class="group-tags-list">
<label>{{i18n 'tagging.groups.tags_label'}}</label>
<br/>
{{tag-chooser tags=model.tag_names everyTag="false" unlimitedTagCount="true"}}
</div>
<br/>
<section class="group-tags-list">
<label>{{i18n 'tagging.groups.tags_label'}}</label><br/>
{{tag-chooser tags=model.tag_names everyTag="true" unlimitedTagCount="true"}}
</section>
<section class="parent-tag-section">
<label>{{i18n 'tagging.groups.parent_tag_label'}}</label>
{{tag-chooser tags=model.parent_tag_name everyTag="true" limit="1" placeholderKey="tagging.groups.parent_tag_placeholder"}}
<span class="description">{{i18n 'tagging.groups.parent_tag_description'}}</span>
</section>
<section class="group-one-per-topic">
<label>
{{input type="checkbox" checked=model.one_per_topic name="onepertopic"}}
{{i18n 'tagging.groups.one_per_topic_label'}}
</label>
</section>
<button {{action "save"}} disabled={{model.disableSave}} class='btn'>{{i18n 'tagging.groups.save'}}</button>
<button {{action "destroy"}} class='btn btn-danger'><i class="fa fa-trash-o"></i> {{i18n 'tagging.groups.delete'}}</button>
<button {{action "destroy"}} disabled={{model.disableSave}} class='btn btn-danger'><i class="fa fa-trash-o"></i> {{i18n 'tagging.groups.delete'}}</button>
<span class="saving {{unless model.savingStatus 'hidden'}}">{{model.savingStatus}}</span>
</div>

14
app/assets/stylesheets/common/base/tagging.scss
View File

@ -222,6 +222,14 @@ header .discourse-tag {color: $tag-color !important; }
.tag-group-content {
width: 75%;
float: right;
section {
margin-bottom: 20px;
}
label {
font-size: 1em;
display: inline-block;
margin-right: 10px;
}
}
.group-tags-list .tag-chooser {
height: 150px !important;
@ -233,4 +241,10 @@ header .discourse-tag {color: $tag-color !important; }
.saving {
margin-left: 20px;
}
.parent-tag-section {
.tag-chooser {
width: 200px;
margin-right: 10px;
}
}
}

7
app/controllers/tag_groups_controller.rb
View File

@ -69,7 +69,10 @@ class TagGroupsController < ApplicationController
end
def tag_groups_params
params[:tag_names] ||= []
params.permit(:id, :name, :tag_names => [])
result = params.permit(:id, :name, :one_per_topic, :tag_names => [], :parent_tag_name => [])
result[:tag_names] ||= []
result[:parent_tag_name] ||= []
result[:one_per_topic] = (params[:one_per_topic] == "true")
result
end
end

9
app/controllers/tags_controller.rb
View File

@ -117,7 +117,12 @@ class TagsController < ::ApplicationController
tags_with_counts = DiscourseTagging.filter_allowed_tags(
self.class.tags_by_count(guardian, params.slice(:limit)),
guardian,
{ for_input: params[:filterForInput], term: params[:q], category: category }
{
for_input: params[:filterForInput],
term: params[:q],
category: category,
selected_tags: params[:selected_tags]
}
)
tags = tags_with_counts.count.map {|t, c| { id: t, text: t, count: c } }
@ -125,7 +130,7 @@ class TagsController < ::ApplicationController
unused_tags = DiscourseTagging.filter_allowed_tags(
Tag.where(topic_count: 0),
guardian,
{ for_input: params[:filterForInput], term: params[:q], category: category }
{ for_input: params[:filterForInput], term: params[:q], category: category, selected_tags: params[:selected_tags] }
)
unused_tags.each do |t|

12
app/models/tag_group.rb
View File

@ -6,7 +6,19 @@ class TagGroup < ActiveRecord::Base
has_many :category_tag_groups, dependent: :destroy
has_many :categories, through: :category_tag_groups
belongs_to :parent_tag, class_name: 'Tag'
def tag_names=(tag_names_arg)
DiscourseTagging.add_or_create_tags_by_name(self, tag_names_arg)
end
def parent_tag_name=(tag_names_arg)
if tag_names_arg.empty?
self.parent_tag = nil
else
if tag_name = DiscourseTagging.tags_for_saving(tag_names_arg, Guardian.new(Discourse.system_user)).first
self.parent_tag = Tag.find_by_name(tag_name) || Tag.create(name: tag_name)
end
end
end
end

2
app/models/tag_group_membership.rb
View File

@ -1,4 +1,4 @@
class TagGroupMembership < ActiveRecord::Base
belongs_to :tag
belongs_to :tag_group, counter_cache: "tag_count"
belongs_to :tag_group, counter_cache: "tag_count" # TODO: remove counter cache
end

6
app/serializers/tag_group_serializer.rb
View File

@ -1,7 +1,11 @@
class TagGroupSerializer < ApplicationSerializer
attributes :id, :name, :tag_names
attributes :id, :name, :tag_names, :parent_tag_name, :one_per_topic
def tag_names
object.tags.pluck(:name).sort
end
def parent_tag_name
[object.parent_tag.try(:name)].compact
end
end

4
config/locales/client.en.yml
View File

@ -3015,6 +3015,10 @@ en:
about: "Add tags to groups to manage them more easily."
new: "New Group"
tags_label: "Tags in this group:"
parent_tag_label: "Parent tag:"
parent_tag_placeholder: "Optional"
parent_tag_description: "Tags from this group can't be used unless the parent tag is present."
one_per_topic_label: "Limit one tag per topic from this group"
new_name: "New Tag Group"
save: "Save"
delete: "Delete"

6
db/migrate/20160607213656_add_tag_group_options.rb Normal file
View File

@ -0,0 +1,6 @@
class AddTagGroupOptions < ActiveRecord::Migration
def change
add_column :tag_groups, :parent_tag_id, :integer
add_column :tag_groups, :one_per_topic, :boolean, default: false
end
end

67
lib/discourse_tagging.rb
View File

@ -33,7 +33,7 @@ module DiscourseTagging
if tag_names.present?
category = topic.category
tags = filter_allowed_tags(Tag.where(name: tag_names), guardian, { for_input: true, category: category }).to_a
tags = filter_allowed_tags(Tag.where(name: tag_names), guardian, { for_input: true, category: category, selected_tags: tag_names }).to_a
if tags.size < tag_names.size && (category.nil? || category.tags.count == 0)
tag_names.each do |name|
@ -56,8 +56,9 @@ module DiscourseTagging
# 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
# for_input: result is for an input field, so only show permitted tags
# selected_tags: an array of tag names that are in the current selection
def self.filter_allowed_tags(query, guardian, opts={})
term = opts[:term]
if term.present?
@ -67,6 +68,8 @@ module DiscourseTagging
end
if opts[:for_input]
selected_tag_ids = opts[:selected_tags] ? Tag.where(name: opts[:selected_tags]).pluck(:id) : []
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?
@ -74,20 +77,24 @@ module DiscourseTagging
# 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)
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 = ?
UNION
SELECT tag_id FROM tag_group_memberships WHERE tag_group_id = ?)",
opts[:category].id, tag_group_ids
SELECT tag_id FROM tag_group_memberships WHERE tag_group_id IN (?))",
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)
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)
end
else
# exclude tags that are restricted to other categories
@ -97,7 +104,41 @@ module DiscourseTagging
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)
query = query.where("tags.id NOT IN (SELECT tag_id FROM tag_group_memberships WHERE tag_group_id IN (?))", tag_group_ids)
end
end
# exclude tag groups that have a parent tag which is missing from selected_tags
select_sql = <<-SQL
SELECT tag_id
FROM tag_group_memberships tgm
INNER JOIN tag_groups tg
ON tgm.tag_group_id = tg.id
SQL
if selected_tag_ids.empty?
sql = "tags.id NOT IN (#{select_sql} WHERE tg.parent_tag_id IS NOT NULL)"
query = query.where(sql)
else
# One tag per group restriction
exclude_group_ids = TagGroup.where(one_per_topic: true)
.joins(:tag_group_memberships)
.where('tag_group_memberships.tag_id in (?)', selected_tag_ids)
.pluck(:id)
if exclude_group_ids.empty?
sql = "tags.id NOT IN (#{select_sql} WHERE tg.parent_tag_id NOT IN (?))"
query = query.where(sql, selected_tag_ids)
else
# It's possible that the selected tags violate some one-tag-per-group restrictions,
# so filter them out by picking one from each group.
limit_tag_ids = TagGroupMembership.select('distinct on (tag_group_id) tag_id')
.where(tag_id: selected_tag_ids)
.where(tag_group_id: exclude_group_ids)
.map(&:tag_id)
sql = "(tags.id NOT IN (#{select_sql} WHERE (tg.parent_tag_id NOT IN (?) OR tg.id in (?))) OR tags.id IN (?))"
query = query.where(sql, selected_tag_ids, exclude_group_ids, limit_tag_ids)
end
end
end

2
spec/fabricators/tag_fabricator.rb
View File

@ -1,3 +1,3 @@
Fabricator(:tag) do
name { sequence(:name) { |i| "tag#{i}" } }
name { sequence(:name) { |i| "tag#{i+1}" } }
end

109
spec/integration/category_tag_spec.rb
View File

@ -9,6 +9,10 @@ describe "category tag restrictions" do
tag_records.map(&:name).sort
end
def filter_allowed_tags(opts={})
DiscourseTagging.filter_allowed_tags(Tag.all, Guardian.new(user), opts)
end
let!(:tag1) { Fabricate(:tag) }
let!(:tag2) { Fabricate(:tag) }
let!(:tag3) { Fabricate(:tag) }
@ -40,9 +44,9 @@ describe "category tag restrictions" do
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)
expect(filter_allowed_tags.count).to eq(Tag.count)
expect(filter_allowed_tags({for_input: true, category: category_with_tags}).pluck(:name).sort).to eq([tag1.name, tag2.name].sort)
expect(filter_allowed_tags({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
@ -76,13 +80,13 @@ describe "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]))
expect(sorted_tag_names(filter_allowed_tags({for_input: true, category: category}))).to eq(sorted_tag_names([tag1, tag2]))
expect(sorted_tag_names(filter_allowed_tags({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]))
expect(sorted_tag_names(filter_allowed_tags({for_input: true, category: category}))).to eq(sorted_tag_names([tag2, tag3, tag4]))
expect(sorted_tag_names(filter_allowed_tags({for_input: true}))).to eq(sorted_tag_names([tag1]))
expect(sorted_tag_names(filter_allowed_tags({for_input: true, category: other_category}))).to eq(sorted_tag_names([tag1]))
end
it "groups and individual tags can be mixed" do
@ -90,9 +94,92 @@ describe "category tag restrictions" do
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]))
expect(sorted_tag_names(filter_allowed_tags({for_input: true, category: category}))).to eq(sorted_tag_names([tag1, tag2, tag4]))
expect(sorted_tag_names(filter_allowed_tags({for_input: true}))).to eq(sorted_tag_names([tag3]))
expect(sorted_tag_names(filter_allowed_tags({for_input: true, category: other_category}))).to eq(sorted_tag_names([tag3]))
end
end
context "tag groups with parent tag" do
it "filter_allowed_tags returns results based on whether parent tag is present or not" do
tag_group = Fabricate(:tag_group, parent_tag_id: tag1.id)
tag_group.tags = [tag3, tag4]
expect(sorted_tag_names(filter_allowed_tags({for_input: true}))).to eq(sorted_tag_names([tag1, tag2]))
expect(sorted_tag_names(filter_allowed_tags({for_input: true, selected_tags: [tag1.name]}))).to eq(sorted_tag_names([tag1, tag2, tag3, tag4]))
expect(sorted_tag_names(filter_allowed_tags({for_input: true, selected_tags: [tag1.name, tag3.name]}))).to eq(sorted_tag_names([tag1, tag2, tag3, tag4]))
end
context "and category restrictions" do
let(:car_category) { Fabricate(:category) }
let(:other_category) { Fabricate(:category) }
let(:makes) { Fabricate(:tag_group, name: "Makes") }
let(:honda_group) { Fabricate(:tag_group, name: "Honda Models") }
let(:ford_group) { Fabricate(:tag_group, name: "Ford Models") }
before do
@tags = {}
['honda', 'ford', 'civic', 'accord', 'mustang', 'taurus'].each do |name|
@tags[name] = Fabricate(:tag, name: name)
end
makes.tags = [@tags['honda'], @tags['ford']]
honda_group.parent_tag_id = @tags['honda'].id
honda_group.save
honda_group.tags = [@tags['civic'], @tags['accord']]
ford_group.parent_tag_id = @tags['ford'].id
ford_group.save
ford_group.tags = [@tags['mustang'], @tags['taurus']]
car_category.allowed_tag_groups = [makes.name, honda_group.name, ford_group.name]
end
it "handles all those rules" do
# car tags can't be used outside of car category:
expect(sorted_tag_names(filter_allowed_tags({for_input: true}))).to eq(sorted_tag_names([tag1, tag2, tag3, tag4]))
expect(sorted_tag_names(filter_allowed_tags({for_input: true, category: other_category}))).to eq(sorted_tag_names([tag1, tag2, tag3, tag4]))
# in car category, a make tag must be given first:
expect(sorted_tag_names(filter_allowed_tags({for_input: true, category: car_category}))).to eq(['ford', 'honda'])
# model tags depend on which make is chosen:
expect(sorted_tag_names(filter_allowed_tags({for_input: true, category: car_category, selected_tags: ['honda']}))).to eq(['accord', 'civic', 'ford', 'honda'])
expect(sorted_tag_names(filter_allowed_tags({for_input: true, category: car_category, selected_tags: ['ford']}))).to eq(['ford', 'honda', 'mustang', 'taurus'])
end
it "can apply the tags to a topic" do
post = create_post(category: car_category, tags: ['ford', 'mustang'])
expect(post.topic.tags.map(&:name).sort).to eq(['ford', 'mustang'])
end
context "limit one tag from each group" do
before do
makes.update_attributes(one_per_topic: true)
honda_group.update_attributes(one_per_topic: true)
ford_group.update_attributes(one_per_topic: true)
end
it "can restrict one tag from each group" do
expect(sorted_tag_names(filter_allowed_tags({for_input: true, category: car_category}))).to eq(['ford', 'honda'])
expect(sorted_tag_names(filter_allowed_tags({for_input: true, category: car_category, selected_tags: ['honda']}))).to eq(['accord', 'civic', 'honda'])
expect(sorted_tag_names(filter_allowed_tags({for_input: true, category: car_category, selected_tags: ['ford']}))).to eq(['ford', 'mustang', 'taurus'])
expect(sorted_tag_names(filter_allowed_tags({for_input: true, category: car_category, selected_tags: ['ford', 'mustang']}))).to eq(['ford', 'mustang'])
end
it "can apply the tags to a topic" do
post = create_post(category: car_category, tags: ['ford', 'mustang'])
expect(post.topic.tags.map(&:name).sort).to eq(['ford', 'mustang'])
end
it "can remove extra tags from the same group" do
# A weird case that input field wouldn't allow.
# Only one tag from car makers is allowed, but we're saying that two have been selected.
names = filter_allowed_tags({for_input: true, category: car_category, selected_tags: ['honda', 'ford']}).map(&:name)
expect(names.include?('honda') && names.include?('ford')).to eq(false)
expect(names.include?('honda') || names.include?('ford')).to eq(true)
end
end
end
end
end