FEATURE: categories can require topics have a tag from a tag group

In a category's settings, the Tags tab has two new fields to
specify the number of tags that must be added to a topic
from a tag group. When creating a new topic, an error will be
shown to the user if the requirement isn't met.
This commit is contained in:
Neil Lalonde 2019-10-30 14:49:00 -04:00
parent 091d058ff2
commit d777844ed6
15 changed files with 226 additions and 21 deletions

View File

@ -31,6 +31,13 @@ const Category = RestModel.extend({
} }
}, },
@on("init")
setupRequiredTagGroups() {
if (this.required_tag_group_name) {
this.set("required_tag_groups", [this.required_tag_group_name]);
}
},
@computed @computed
availablePermissions() { availablePermissions() {
return [ return [
@ -127,6 +134,10 @@ const Category = RestModel.extend({
allowed_tags: this.allowed_tags, allowed_tags: this.allowed_tags,
allowed_tag_groups: this.allowed_tag_groups, allowed_tag_groups: this.allowed_tag_groups,
allow_global_tags: this.allow_global_tags, allow_global_tags: this.allow_global_tags,
required_tag_group_name: this.required_tag_groups
? this.required_tag_groups[0]
: null,
min_tags_from_required_group: this.min_tags_from_required_group,
sort_order: this.sort_order, sort_order: this.sort_order,
sort_ascending: this.sort_ascending, sort_ascending: this.sort_ascending,
topic_featured_link_allowed: this.topic_featured_link_allowed, topic_featured_link_allowed: this.topic_featured_link_allowed,

View File

@ -24,3 +24,23 @@
<section class='field'> <section class='field'>
{{i18n 'category.tags_tab_description'}} {{i18n 'category.tags_tab_description'}}
</section> </section>
<section class="field">
{{i18n 'category.required_tag_group_description'}}
</section>
<section class="field with-items">
<section class="field-item">
<label>{{i18n 'category.min_tags_from_required_group_label'}}</label>
{{text-field value=category.min_tags_from_required_group id="category-min-tags-from-group" type="number" min="1"}}
</section>
<section class="field-item">
<label>{{i18n 'category.required_tag_group_label'}}</label>
{{tag-group-chooser
id="category-required-tag-group"
tagGroups=category.required_tag_groups
maximum=1
filterPlaceholder="category.tag_group_selector_placeholder"
}}
</section>
</section>

View File

@ -268,11 +268,14 @@
section.field { section.field {
padding: 0.25em 0; padding: 0.25em 0;
margin-bottom: 5px; margin-bottom: 5px;
} &.with-items {
display: flex;
section.field .field-item { align-items: flex-start;
display: inline-block; }
margin-right: 10px; .field-item {
display: inline-block;
margin-right: 10px;
}
} }
// password reset modal // password reset modal
@ -446,6 +449,9 @@
} }
} }
} }
#category-min-tags-from-group {
width: 100px;
}
} }
.incoming-email-modal { .incoming-email-modal {

View File

@ -313,6 +313,8 @@ class CategoriesController < ApplicationController
:navigate_to_first_post_after_read, :navigate_to_first_post_after_read,
:search_priority, :search_priority,
:allow_global_tags, :allow_global_tags,
:required_tag_group_name,
:min_tags_from_required_group,
custom_fields: [params[:custom_fields].try(:keys)], custom_fields: [params[:custom_fields].try(:keys)],
permissions: [*p.try(:keys)], permissions: [*p.try(:keys)],
allowed_tags: [], allowed_tags: [],

View File

@ -51,6 +51,7 @@ class Category < ActiveRecord::Base
validates :num_featured_topics, numericality: { only_integer: true, greater_than: 0 } validates :num_featured_topics, numericality: { only_integer: true, greater_than: 0 }
validates :search_priority, inclusion: { in: Searchable::PRIORITIES.values } validates :search_priority, inclusion: { in: Searchable::PRIORITIES.values }
validates :min_tags_from_required_group, numericality: { only_integer: true, greater_than: 0 }
validate :parent_category_validator validate :parent_category_validator
validate :email_in_validator validate :email_in_validator
@ -93,6 +94,8 @@ class Category < ActiveRecord::Base
has_many :tags, through: :category_tags has_many :tags, through: :category_tags
has_many :category_tag_groups, dependent: :destroy has_many :category_tag_groups, dependent: :destroy
has_many :tag_groups, through: :category_tag_groups has_many :tag_groups, through: :category_tag_groups
belongs_to :required_tag_group, class_name: 'TagGroup'
belongs_to :reviewable_by_group, class_name: 'Group' belongs_to :reviewable_by_group, class_name: 'Group'
scope :latest, -> { order('topic_count DESC') } scope :latest, -> { order('topic_count DESC') }
@ -555,6 +558,10 @@ class Category < ActiveRecord::Base
self.tag_groups = TagGroup.where(name: group_names).all.to_a self.tag_groups = TagGroup.where(name: group_names).all.to_a
end end
def required_tag_group_name=(group_name)
self.required_tag_group = group_name ? TagGroup.where(name: group_name).first : nil
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

@ -4,7 +4,9 @@ class SiteCategorySerializer < BasicCategorySerializer
attributes :allowed_tags, attributes :allowed_tags,
:allowed_tag_groups, :allowed_tag_groups,
:allow_global_tags :allow_global_tags,
:min_tags_from_required_group,
:required_tag_group_name
def include_allowed_tags? def include_allowed_tags?
SiteSetting.tagging_enabled SiteSetting.tagging_enabled
@ -26,4 +28,8 @@ class SiteCategorySerializer < BasicCategorySerializer
SiteSetting.tagging_enabled SiteSetting.tagging_enabled
end end
def required_tag_group_name
object.required_tag_group&.name
end
end end

View File

@ -2606,10 +2606,14 @@ en:
tags_allowed_tags: "Restrict these tags to this category:" tags_allowed_tags: "Restrict these tags to this category:"
tags_allowed_tag_groups: "Restrict these tag groups to this category:" tags_allowed_tag_groups: "Restrict these tag groups to this category:"
tags_placeholder: "(Optional) list of allowed tags" 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." tags_tab_description: "Tags and tag groups specified above 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" tag_groups_placeholder: "(Optional) list of allowed tag groups"
manage_tag_groups_link: "Manage tag groups here." manage_tag_groups_link: "Manage tag groups here."
allow_global_tags_label: "Also allow other tags" allow_global_tags_label: "Also allow other tags"
tag_group_selector_placeholder: "(Optional) Tag group"
required_tag_group_description: "Require new topics to have tags from a tag group:"
min_tags_from_required_group_label: 'Num Tags:'
required_tag_group_label: "Tag group:"
topic_featured_link_allowed: "Allow featured links in this category" topic_featured_link_allowed: "Allow featured links in this category"
delete: "Delete Category" delete: "Delete Category"
create: "New Category" create: "New Category"

View File

@ -4344,6 +4344,9 @@ en:
restricted_to: restricted_to:
one: '"%{tag_name}" is restricted to the "%{category_names}" category' one: '"%{tag_name}" is restricted to the "%{category_names}" category'
other: '"%{tag_name}" is restricted to the following categories: %{category_names}' other: '"%{tag_name}" is restricted to the following categories: %{category_names}'
required_tags_from_group:
one: "You must include at least %{count} %{tag_group_name} tag."
other: "You must include at least %{count} %{tag_group_name} tags."
rss_by_tag: "Topics tagged %{tag}" rss_by_tag: "Topics tagged %{tag}"
finish_installation: finish_installation:

View File

@ -0,0 +1,8 @@
# frozen_string_literal: true
class AddRequiredTagGroupToCategories < ActiveRecord::Migration[6.0]
def change
add_column :categories, :required_tag_group_id, :integer, null: true
add_column :categories, :min_tags_from_required_group, :integer, null: false, default: 1
end
end

View File

@ -88,11 +88,8 @@ module DiscourseTagging
tags = tags + Tag.where(id: missing_parent_tag_ids).all tags = tags + Tag.where(id: missing_parent_tag_ids).all
end end
# validate minimum required tags for a category return false unless validate_min_required_tags_for_category(guardian, topic, category, tags)
if !guardian.is_staff? && category && category.minimum_required_tags > 0 && tags.length < category.minimum_required_tags return false unless validate_required_tags_from_group(guardian, topic, category, tags)
topic.errors.add(:base, I18n.t("tags.minimum_required_tags", count: category.minimum_required_tags))
return false
end
if tags.size == 0 if tags.size == 0
topic.errors.add(:base, I18n.t("tags.forbidden.invalid", count: new_tag_names.size)) topic.errors.add(:base, I18n.t("tags.forbidden.invalid", count: new_tag_names.size))
@ -101,11 +98,8 @@ module DiscourseTagging
topic.tags = tags topic.tags = tags
else else
# validate minimum required tags for a category return false unless validate_min_required_tags_for_category(guardian, topic, category)
if !guardian.is_staff? && category && category.minimum_required_tags > 0 return false unless validate_required_tags_from_group(guardian, topic, category)
topic.errors.add(:base, I18n.t("tags.minimum_required_tags", count: category.minimum_required_tags))
return false
end
topic.tags = [] topic.tags = []
end end
@ -114,6 +108,39 @@ module DiscourseTagging
true true
end end
def self.validate_min_required_tags_for_category(guardian, topic, category, tags = [])
if !guardian.is_staff? &&
category &&
category.minimum_required_tags > 0 &&
tags.length < category.minimum_required_tags
topic.errors.add(:base, I18n.t("tags.minimum_required_tags", count: category.minimum_required_tags))
false
else
true
end
end
def self.validate_required_tags_from_group(guardian, topic, category, tags = [])
if !guardian.is_staff? &&
category &&
category.required_tag_group &&
(tags.length < category.min_tags_from_required_group ||
category.required_tag_group.tags.where("tags.id in (?)", tags.map(&:id)).count < category.min_tags_from_required_group)
topic.errors.add(:base,
I18n.t(
"tags.required_tags_from_group",
count: category.min_tags_from_required_group,
tag_group_name: category.required_tag_group.name
)
)
false
else
true
end
end
# Options: # Options:
# term: a search term to filter tags by name # term: a search term to filter tags by name
# category: a Category to which the object being tagged belongs # category: a Category to which the object being tagged belongs

View File

@ -147,10 +147,10 @@ class TopicCreator
def setup_tags(topic) def setup_tags(topic)
if @opts[:tags].blank? if @opts[:tags].blank?
unless @guardian.is_staff? || !guardian.can_tag?(topic) unless @guardian.is_staff? || !guardian.can_tag?(topic)
# Validate minimum required tags for a category
category = find_category category = find_category
if category.present? && category.minimum_required_tags > 0
topic.errors.add(:base, I18n.t("tags.minimum_required_tags", count: category.minimum_required_tags)) if !DiscourseTagging.validate_min_required_tags_for_category(@guardian, topic, category) ||
!DiscourseTagging.validate_required_tags_from_group(@guardian, topic, category)
rollback_from_errors!(topic) rollback_from_errors!(topic)
end end
end end

View File

@ -236,6 +236,52 @@ describe DiscourseTagging do
expect(topic.reload.tags.map(&:name)).to contain_exactly(*[parent_tag, common].map(&:name)) expect(topic.reload.tags.map(&:name)).to contain_exactly(*[parent_tag, common].map(&:name))
end end
end end
context "enforces required tags from a tag group" do
fab!(:category) { Fabricate(:category) }
fab!(:tag_group) { Fabricate(:tag_group) }
fab!(:topic) { Fabricate(:topic, category: category) }
before do
tag_group.tags = [tag1, tag2]
category.update(
required_tag_group: tag_group,
min_tags_from_required_group: 1
)
end
it "when no tags are present" do
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [])
expect(valid).to eq(false)
expect(topic.errors[:base]&.first).to eq(
I18n.t("tags.required_tags_from_group", count: 1, tag_group_name: tag_group.name)
)
end
it "when tags are not part of the tag group" do
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [tag3.name])
expect(valid).to eq(false)
expect(topic.errors[:base]&.first).to eq(
I18n.t("tags.required_tags_from_group", count: 1, tag_group_name: tag_group.name)
)
end
it "when requirement is met" do
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [tag1.name])
expect(valid).to eq(true)
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [tag1.name, tag2.name])
expect(valid).to eq(true)
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [tag2.name, tag3.name])
expect(valid).to eq(true)
end
it "lets staff ignore the restriction" do
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(admin), [])
expect(valid).to eq(true)
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(admin), [tag3.name])
expect(valid).to eq(true)
end
end
end end
describe '#tags_for_saving' do describe '#tags_for_saving' do

View File

@ -810,6 +810,37 @@ describe PostRevisor do
end end
end end
context "required tag group" do
fab!(:tag1) { Fabricate(:tag) }
fab!(:tag2) { Fabricate(:tag) }
fab!(:tag3) { Fabricate(:tag) }
fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1, tag2]) }
fab!(:category) { Fabricate(:category, name: "beta", required_tag_group: tag_group, min_tags_from_required_group: 1) }
before do
post.topic.update(category: category)
end
it "doesn't allow removing all tags from the group" do
post.topic.tags = [tag1, tag2]
result = subject.revise!(user, raw: "lets totally update the body", tags: [])
expect(result).to eq(false)
end
it "allows removing some tags" do
post.topic.tags = [tag1, tag2, tag3]
result = subject.revise!(user, raw: "lets totally update the body", tags: [tag1.name])
expect(result).to eq(true)
expect(post.reload.topic.tags.map(&:name)).to eq([tag1.name])
end
it "allows admins to remove the tags" do
post.topic.tags = [tag1, tag2, tag3]
result = subject.revise!(admin, raw: "lets totally update the body", tags: [])
expect(result).to eq(true)
expect(post.reload.topic.tags.size).to eq(0)
end
end
end end
context "cannot create tags" do context "cannot create tags" do

View File

@ -126,6 +126,35 @@ describe TopicCreator do
expect(topic).to be_valid expect(topic).to be_valid
end end
end end
context 'required tag group' do
fab!(:tag_group) { Fabricate(:tag_group, tags: [tag1]) }
fab!(:category) { Fabricate(:category, name: "beta", required_tag_group: tag_group, min_tags_from_required_group: 1) }
it "when no tags are not present" do
expect do
TopicCreator.create(user, Guardian.new(user), valid_attrs.merge(category: category.id))
end.to raise_error(ActiveRecord::Rollback)
end
it "when tags are not part of the tag group" do
expect do
TopicCreator.create(user, Guardian.new(user), valid_attrs.merge(category: category.id, tags: ['nope']))
end.to raise_error(ActiveRecord::Rollback)
end
it "when requirement is met" do
topic = TopicCreator.create(user, Guardian.new(user), valid_attrs.merge(category: category.id, tags: [tag1.name, tag2.name]))
expect(topic).to be_valid
expect(topic.tags.length).to eq(2)
end
it "lets staff ignore the restriction" do
topic = TopicCreator.create(user, Guardian.new(admin), valid_attrs.merge(category: category.id))
expect(topic).to be_valid
expect(topic.tags.length).to eq(0)
end
end
end end
context 'personal message' do context 'personal message' do

View File

@ -349,6 +349,7 @@ describe CategoriesController do
it "updates attributes correctly" do it "updates attributes correctly" do
readonly = CategoryGroup.permission_types[:readonly] readonly = CategoryGroup.permission_types[:readonly]
create_post = CategoryGroup.permission_types[:create_post] create_post = CategoryGroup.permission_types[:create_post]
tag_group = Fabricate(:tag_group)
put "/categories/#{category.id}.json", params: { put "/categories/#{category.id}.json", params: {
name: "hello", name: "hello",
@ -364,7 +365,9 @@ describe CategoriesController do
"dancing" => "frogs" "dancing" => "frogs"
}, },
minimum_required_tags: "", minimum_required_tags: "",
allow_global_tags: 'true' allow_global_tags: 'true',
required_tag_group_name: tag_group.name,
min_tags_from_required_group: 2
} }
expect(response.status).to eq(200) expect(response.status).to eq(200)
@ -379,6 +382,8 @@ describe CategoriesController do
expect(category.custom_fields).to eq("dancing" => "frogs") expect(category.custom_fields).to eq("dancing" => "frogs")
expect(category.minimum_required_tags).to eq(0) expect(category.minimum_required_tags).to eq(0)
expect(category.allow_global_tags).to eq(true) expect(category.allow_global_tags).to eq(true)
expect(category.required_tag_group_id).to eq(tag_group.id)
expect(category.min_tags_from_required_group).to eq(2)
end end
it 'logs the changes correctly' do it 'logs the changes correctly' do