FEATURE: enforce tagging on categories

This commit is contained in:
Arpit Jalan 2018-03-29 00:10:26 +05:30
parent abf0b1c5bd
commit 9ca6ebe8fe
17 changed files with 209 additions and 19 deletions

View File

@ -108,7 +108,8 @@ const Category = RestModel.extend({
num_featured_topics: this.get('num_featured_topics'),
default_view: this.get('default_view'),
subcategory_list_style: this.get('subcategory_list_style'),
default_top_period: this.get('default_top_period')
default_top_period: this.get('default_top_period'),
minimum_required_tags: this.get('minimum_required_tags')
},
type: id ? 'PUT' : 'POST'
});

View File

@ -152,4 +152,13 @@
</section>
{{/unless}}
{{#if siteSettings.tagging_enabled}}
<section class='field minimum-required-tags'>
<label>
{{i18n 'category.minimum_required_tags'}}
{{text-field value=category.minimum_required_tags type="number"}}
</label>
</section>
{{/if}}
{{plugin-outlet name="category-custom-settings" args=(hash category=category)}}

View File

@ -29,13 +29,13 @@
.modal-header {
display: flex;
align-items: center;
padding: 10px 15px;
padding: 10px 15px;
border-bottom: 1px solid $primary-low;
h3 {
margin-bottom: 0;
}
.modal-close {
order: 2;
order: 2;
margin-left: auto;
}
}
@ -372,6 +372,12 @@
}
}
}
section.minimum-required-tags {
input[type=number] {
width: 50px;
}
}
}
.incoming-email-modal {
@ -421,22 +427,22 @@
}
.change-timestamp {
.date-picker {
width: 10em;
}
#date-container {
.pika-single {
position: relative !important; // overriding another important
display: inline-block;
}
}
input[type=time] {
width: 6em;
}
form {
margin: 0;
}
@ -491,4 +497,3 @@
width: 95%;
}

View File

@ -283,6 +283,7 @@ class CategoriesController < ApplicationController
:default_view,
:subcategory_list_style,
:default_top_period,
:minimum_required_tags,
custom_fields: [params[:custom_fields].try(:keys)],
permissions: [*p.try(:keys)],
allowed_tags: [],

View File

@ -573,10 +573,11 @@ end
# default_top_period :string(20) default("all")
# mailinglist_mirror :boolean default(FALSE), not null
# suppress_from_latest :boolean default(FALSE)
# minimum_required_tags :integer default(0)
#
# Indexes
#
# index_categories_on_email_in (email_in) UNIQUE
# index_categories_on_topic_count (topic_count)
# unique_index_categories_on_name ((COALESCE(parent_category_id, '-1'::integer)), name) UNIQUE
# unique_index_categories_on_name (COALESCE(parent_category_id, '-1'::integer), name) UNIQUE
#

View File

@ -15,6 +15,7 @@ end
#
# Indexes
#
# idx_post_custom_fields_akismet (post_id)
# index_post_custom_fields_on_name_and_value (name, "left"(value, 200))
# index_post_custom_fields_on_post_id_and_name (post_id,name)
#

View File

@ -2,3 +2,19 @@ class SharedDraft < ActiveRecord::Base
belongs_to :topic
belongs_to :category
end
# == Schema Information
#
# Table name: shared_drafts
#
# topic_id :integer not null
# category_id :integer not null
# created_at :datetime not null
# updated_at :datetime not null
# id :integer not null, primary key
#
# Indexes
#
# index_shared_drafts_on_category_id (category_id)
# index_shared_drafts_on_topic_id (topic_id) UNIQUE
#

View File

@ -7,3 +7,20 @@ class TagGroupPermission < ActiveRecord::Base
@permission_types ||= Enum.new(full: 1) #, see: 2
end
end
# == Schema Information
#
# Table name: tag_group_permissions
#
# id :integer not null, primary key
# tag_group_id :integer not null
# group_id :integer not null
# permission_type :integer default(1), not null
# created_at :datetime not null
# updated_at :datetime not null
#
# Indexes
#
# index_tag_group_permissions_on_group_id (group_id)
# index_tag_group_permissions_on_tag_group_id (tag_group_id)
#

View File

@ -74,3 +74,17 @@ class WebCrawlerRequest < ActiveRecord::Base
request_id(date: date, user_agent: user_agent)
end
end
# == Schema Information
#
# Table name: web_crawler_requests
#
# id :integer not null, primary key
# date :date not null
# user_agent :string not null
# count :integer default(0), not null
#
# Indexes
#
# index_web_crawler_requests_on_date_and_user_agent (date,user_agent) UNIQUE
#

View File

@ -24,7 +24,8 @@ class BasicCategorySerializer < ApplicationSerializer
:num_featured_topics,
:default_view,
:subcategory_list_style,
:default_top_period
:default_top_period,
:minimum_required_tags
has_one :uploaded_logo, embed: :object, serializer: CategoryUploadSerializer
has_one :uploaded_background, embed: :object, serializer: CategoryUploadSerializer

View File

@ -2267,6 +2267,7 @@ en:
default_position: "Default Position"
position_disabled: "Categories will be displayed in order of activity. To control the order of categories in lists, "
position_disabled_click: 'enable the "fixed category positions" setting.'
minimum_required_tags: 'Minimum number of tags required in a topic:'
parent: "Parent Category"
notifications:
watching:

View File

@ -3632,6 +3632,7 @@ en:
title: "Tags"
staff_tag_disallowed: "The tag \"%{tag}\" may only be applied by staff."
staff_tag_remove_disallowed: "The tag \"%{tag}\" may only be removed by staff."
minimum_required_tags: "You must select at least %{count} tags."
rss_by_tag: "Topics tagged %{tag}"
finish_installation:

View File

@ -0,0 +1,5 @@
class AddMinimumRequiredTagsToCategories < ActiveRecord::Migration[5.1]
def change
add_column :categories, :minimum_required_tags, :integer, default: 0
end
end

View File

@ -26,10 +26,16 @@ module DiscourseTagging
end
end
if tag_names.present?
category = topic.category
tag_names = tag_names + old_tag_names if append
category = topic.category
tag_names = tag_names + old_tag_names if append
# validate minimum required tags for a category
if !guardian.is_staff? && category && category.minimum_required_tags > 0 && tag_names.length < category.minimum_required_tags
topic.errors[:base] << I18n.t("tags.minimum_required_tags", count: category.minimum_required_tags)
return false
end
if tag_names.present?
# guardian is explicitly nil cause we don't want to strip all
# staff tags that already passed validation
tags = filter_allowed_tags(

View File

@ -24,11 +24,6 @@ class TopicCreator
# this allows us to add errors
valid = topic.valid?
# not sure where this should go
if !@guardian.is_staff? && staff_only = DiscourseTagging.staff_only_tags(@opts[:tags])
topic.errors[:base] << I18n.t("tags.staff_tag_disallowed", tag: staff_only.join(" "))
end
DiscourseEvent.trigger(:after_validate_topic, topic, self)
valid &&= topic.errors.empty?
@ -159,7 +154,19 @@ class TopicCreator
end
def setup_tags(topic)
DiscourseTagging.tag_topic_by_names(topic, @guardian, @opts[:tags])
if @opts[:tags].blank?
unless @guardian.is_staff?
# Validate minimum required tags for a category
category = find_category
if category.present? && category.minimum_required_tags > 0
topic.errors[:base] << I18n.t("tags.minimum_required_tags", count: category.minimum_required_tags)
rollback_from_errors!(topic)
end
end
else
valid_tags = DiscourseTagging.tag_topic_by_names(topic, @guardian, @opts[:tags])
rollback_from_errors!(topic) unless valid_tags
end
end
def setup_auto_close_time(topic)

View File

@ -75,4 +75,49 @@ describe DiscourseTagging do
expect(tags).to contain_exactly(tag1, tag2, tag3)
end
end
describe 'tag_topic_by_names' do
context 'staff-only tags' do
let(:topic) { Fabricate(:topic) }
before do
SiteSetting.staff_tags = "alpha"
end
it "regular users can't add staff-only tags" do
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), ['alpha'])
expect(valid).to eq(false)
expect(topic.errors[:base]&.first).to eq(I18n.t("tags.staff_tag_disallowed", tag: 'alpha'))
end
it 'staff can add staff-only tags' do
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(admin), ['alpha'])
expect(valid).to eq(true)
expect(topic.errors[:base]).to be_empty
end
end
context 'respects category minimum_required_tags setting' do
let(:category) { Fabricate(:category, minimum_required_tags: 2) }
let(:topic) { Fabricate(:topic, category: category) }
it 'when tags are less than minimum_required_tags' do
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [tag1.name])
expect(valid).to eq(false)
expect(topic.errors[:base]&.first).to eq(I18n.t("tags.minimum_required_tags", count: category.minimum_required_tags))
end
it 'when tags are equal to minimum_required_tags' do
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(user), [tag1.name, tag2.name])
expect(valid).to eq(true)
expect(topic.errors[:base]).to be_empty
end
it 'lets admin tag a topic regardless of minimum_required_tags' do
valid = DiscourseTagging.tag_topic_by_names(topic, Guardian.new(admin), [tag1.name])
expect(valid).to eq(true)
expect(topic.errors[:base]).to be_empty
end
end
end
end

View File

@ -60,6 +60,65 @@ describe TopicCreator do
end
end
context 'tags' do
let!(:tag1) { Fabricate(:tag, name: "fun") }
let!(:tag2) { Fabricate(:tag, name: "fun2") }
before do
SiteSetting.tagging_enabled = true
SiteSetting.min_trust_to_create_tag = 0
SiteSetting.min_trust_level_to_tag_topics = 0
end
context 'regular tags' do
it "user can add tags to topic" do
topic = TopicCreator.create(user, Guardian.new(user), valid_attrs.merge(tags: [tag1.name]))
expect(topic).to be_valid
expect(topic.tags.length).to eq(1)
end
end
context 'staff-only tags' do
before do
SiteSetting.staff_tags = "alpha"
end
it "regular users can't add staff-only tags" do
expect do
TopicCreator.create(user, Guardian.new(user), valid_attrs.merge(tags: ['alpha']))
end.to raise_error(ActiveRecord::Rollback)
end
it 'staff can add staff-only tags' do
topic = TopicCreator.create(admin, Guardian.new(admin), valid_attrs.merge(tags: ['alpha']))
expect(topic).to be_valid
expect(topic.tags.length).to eq(1)
end
end
context 'minimum_required_tags is present' do
let!(:category) { Fabricate(:category, name: "beta", minimum_required_tags: 2) }
it "fails for regular user if minimum_required_tags is not satisfied" do
expect do
TopicCreator.create(user, Guardian.new(user), valid_attrs.merge(category: "beta"))
end.to raise_error(ActiveRecord::Rollback)
end
it "lets admin create a topic regardless of minimum_required_tags" do
topic = TopicCreator.create(admin, Guardian.new(admin), valid_attrs.merge(tags: [tag1.name], category: "beta"))
expect(topic).to be_valid
expect(topic.tags.length).to eq(1)
end
it "works for regular user if minimum_required_tags is satisfied" do
topic = TopicCreator.create(user, Guardian.new(user), valid_attrs.merge(tags: [tag1.name, tag2.name], category: "beta"))
expect(topic).to be_valid
expect(topic.tags.length).to eq(2)
end
end
end
context 'private message' do
context 'success cases' do