Merge pull request #5730 from techAPJ/enforce-tagging

FEATURE: enforce tagging on categories

app/assets/javascripts/discourse/controllers/composer.js.es6

@@ -779,11 +779,19 @@ export default Ember.Controller.extend({
 
   @computed('model.categoryId', 'lastValidatedAt')
   categoryValidation(categoryId, lastValidatedAt) {
-    if( !this.siteSettings.allow_uncategorized_topics && !categoryId) {
+    if(!this.siteSettings.allow_uncategorized_topics && !categoryId) {
       return InputValidation.create({ failed: true, reason: I18n.t('composer.error.category_missing'), lastShownAt: lastValidatedAt });
     }
   },
 
+  @computed('model.category', 'model.tags', 'lastValidatedAt')
+  tagValidation(category, tags, lastValidatedAt) {
+    const tagsArray = tags || [];
+    if (this.site.get('can_tag_topics') && category && category.get('minimum_required_tags') > tagsArray.length) {
+      return InputValidation.create({ failed: true, reason: I18n.t('composer.error.tags_missing', {count: category.get('minimum_required_tags')}), lastShownAt: lastValidatedAt });
+    }
+  },
+
   collapse() {
     this._saveDraft();
     this.set('model.composeState', Composer.DRAFT);

app/assets/javascripts/discourse/models/category.js.es6

@@ -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'
     });

app/assets/javascripts/discourse/models/composer.js.es6

@@ -105,6 +105,11 @@ const Composer = RestModel.extend({
     return categoryId ? this.site.categories.findBy('id', categoryId) : null;
   },
 
+  @computed('category')
+  minimumRequiredTags(category) {
+    return (category && category.get('minimum_required_tags') > 0) ? category.get('minimum_required_tags') : null;
+  },
+
   creatingTopic: Em.computed.equal('action', CREATE_TOPIC),
   creatingSharedDraft: Em.computed.equal('action', CREATE_SHARED_DRAFT),
   creatingPrivateMessage: Em.computed.equal('action', PRIVATE_MESSAGE),
@@ -246,28 +251,41 @@ const Composer = RestModel.extend({
     return options;
   },
 
-  // whether to disable the post button
-  cantSubmitPost: function() {
+  @computed
+  isStaffUser() {
+    const currentUser = Discourse.User.current();
+    return currentUser && currentUser.get('staff');
+  },
+
+  @computed('loading', 'canEditTitle', 'titleLength', 'targetUsernames', 'replyLength', 'categoryId', 'missingReplyCharacters', 'tags', 'topicFirstPost', 'minimumRequiredTags', 'isStaffUser')
+  cantSubmitPost(loading, canEditTitle, titleLength, targetUsernames, replyLength, categoryId, missingReplyCharacters, tags, topicFirstPost, minimumRequiredTags, isStaffUser) {
 
     // can't submit while loading
-    if (this.get('loading')) return true;
+    if (loading) return true;
 
     // title is required when
     //  - creating a new topic/private message
     //  - editing the 1st post
-    if (this.get('canEditTitle') && !this.get('titleLengthValid')) return true;
+    if (canEditTitle && !this.get('titleLengthValid')) return true;
 
     // reply is always required
-    if (this.get('missingReplyCharacters') > 0) return true;
+    if (missingReplyCharacters > 0) return true;
+
+    if (this.site.get('can_tag_topics') && !isStaffUser && topicFirstPost && minimumRequiredTags) {
+      const tagsArray = tags || [];
+      if (tagsArray.length < minimumRequiredTags) {
+        return true;
+      }
+    }
 
     if (this.get("privateMessage")) {
       // need at least one user when sending a PM
-      return this.get('targetUsernames') && (this.get('targetUsernames').trim() + ',').indexOf(',') === 0;
+      return targetUsernames && (targetUsernames.trim() + ',').indexOf(',') === 0;
     } else {
       // has a category? (when needed)
       return this.get('requiredCategoryMissing');
     }
-  }.property('loading', 'canEditTitle', 'titleLength', 'targetUsernames', 'replyLength', 'categoryId', 'missingReplyCharacters'),
+  },
 
   @computed('canCategorize', 'categoryId')
   requiredCategoryMissing(canCategorize, categoryId) {

app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs

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

app/assets/javascripts/discourse/templates/composer.hbs

@@ -64,7 +64,8 @@
                   </div>
                 {{/if}}
                 {{#if canEditTags}}
-                  {{mini-tag-chooser tags=model.tags tabindex="4" categoryId=model.categoryId}}
+                  {{mini-tag-chooser tags=model.tags tabindex="4" categoryId=model.categoryId minimum=model.minimumRequiredTags}}
+                  {{popup-input-tip validation=tagValidation}}
                 {{/if}}
 
 

app/assets/stylesheets/common/base/modal.scss

@@ -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%;
 }
 
-  

app/controllers/categories_controller.rb

@@ -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: [],

app/models/category.rb

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

app/models/post_custom_field.rb

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

app/models/shared_draft.rb

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

app/models/tag_group_permission.rb

@@ -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)
+#

app/models/web_crawler_request.rb

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

app/serializers/basic_category_serializer.rb

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

config/locales/client.en.yml

@@ -1303,6 +1303,7 @@ en:
         post_length: "Post must be at least {{min}} characters"
         try_like: 'Have you tried the <i class="fa fa-heart"></i> button?'
         category_missing: "You must choose a category"
+        tags_missing: "You must choose at least {{count}} tags"
 
       save_edit: "Save Edit"
       reply_original: "Reply on Original Topic"
@@ -2267,6 +2268,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:

config/locales/server.en.yml

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

db/migrate/20180331125522_add_minimum_required_tags_to_categories.rb

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

lib/discourse_tagging.rb

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

lib/topic_creator.rb

@@ -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? || !guardian.can_tag?(topic)
+        # 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)

spec/components/discourse_tagging_spec.rb

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

spec/components/topic_creator_spec.rb

@@ -60,6 +60,72 @@ 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
+
+        it "lets new user create a topic if they don't have sufficient trust level to tag topics" do
+          SiteSetting.min_trust_level_to_tag_topics = 1
+          new_user = Fabricate(:newuser)
+          topic = TopicCreator.create(new_user, Guardian.new(new_user), valid_attrs.merge(category: "beta"))
+          expect(topic).to be_valid
+        end
+      end
+    end
+
     context 'private message' do
 
       context 'success cases' do