FEATURE: Category Reviewable by Group

Allow a group to review content in a particular category.

app/assets/javascripts/discourse/components/edit-category-settings.js.es6

@@ -2,6 +2,7 @@ import { setting } from "discourse/lib/computed";
 import { buildCategoryPanel } from "discourse/components/edit-category-panel";
 import computed from "ember-addons/ember-computed-decorators";
 import { searchPriorities } from "discourse/components/concerns/category_search_priorities";
+import Group from "discourse/models/group";
 
 const categorySortCriteria = [];
 export function addCategorySortCriteria(criteria) {
@@ -41,6 +42,10 @@ export default buildCategoryPanel("settings", {
     ];
   },
 
+  groupFinder(term) {
+    return Group.findAll({ term, ignore_automatic: true });
+  },
+
   @computed
   availableViews() {
     return [

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

@@ -131,7 +131,8 @@ const Category = RestModel.extend({
         navigate_to_first_post_after_read: this.get(
           "navigate_to_first_post_after_read"
         ),
-        search_priority: this.get("search_priority")
+        search_priority: this.get("search_priority"),
+        reviewable_by_group_name: this.get("reviewable_by_group_name")
       },
       type: id ? "PUT" : "POST"
     });

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

@@ -25,7 +25,7 @@
         {{i18n "category.num_featured_topics"}}
       {{/if}}
     </label>
-        {{text-field value=category.num_featured_topics id="category-number-featured-topics" type="number"}}
+    {{text-field value=category.num_featured_topics id="category-number-featured-topics" type="number"}}
   </section>
 
   <section class="field">
@@ -85,6 +85,19 @@
 
   <h3>{{i18n 'category.settings_sections.moderation'}}</h3>
 
+  {{#if siteSettings.enable_category_group_review}}
+    <section class="field">
+      <label for="reviewable-by-group">
+        {{i18n 'category.reviewable_by_group'}}
+      </label>
+      {{group-selector
+        groupFinder=groupFinder
+        single="true"
+        groupNames=category.reviewable_by_group_name
+        placeholderKey="category.review_group_name"}}
+    </section>
+  {{/if}}
+
   <section class="field">
     <label>
       {{input type="checkbox" checked=category.custom_fields.require_topic_approval}}
@@ -104,7 +117,7 @@
       <label for="topic-auto-close">
         {{i18n 'topic.auto_close.label'}}
       </label>
-            {{text-field value=category.auto_close_hours id="topic-auto-close" type="number"}}
+      {{text-field value=category.auto_close_hours id="topic-auto-close" type="number"}}
       <label>
         {{input type="checkbox" checked=category.auto_close_based_on_last_post}}
         {{i18n 'topic.auto_close.based_on_last_post'}}
@@ -117,7 +130,7 @@
       <label for="category-minimum-tags">
         {{i18n 'category.minimum_required_tags'}}
       </label>
-            {{text-field value=category.minimum_required_tags id="category-minimum-tags" type="number" min="0"}}
+      {{text-field value=category.minimum_required_tags id="category-minimum-tags" type="number" min="0"}}
     </section>
   {{/if}}
 
@@ -125,7 +138,7 @@
     <label for="category-number-daily-bump">
       {{i18n "category.num_auto_bump_daily"}}
     </label>
-        {{text-field value=category.custom_fields.num_auto_bump_daily id="category-number-daily-bump" type="number"}}
+    {{text-field value=category.custom_fields.num_auto_bump_daily id="category-number-daily-bump" type="number"}}
   </section>
 
 </section>

app/assets/javascripts/discourse/widgets/hamburger-menu.js.es6

@@ -57,14 +57,6 @@ export default createWidget("hamburger-menu", {
       }
     ];
 
-    links.push({
-      route: "review",
-      className: "review",
-      label: "review.title",
-      badgeCount: "reviewable_count",
-      badgeClass: "reviewables"
-    });
-
     if (currentUser.admin) {
       links.push({
         href: "/admin/site_settings/category/required",
@@ -120,6 +112,20 @@ export default createWidget("hamburger-menu", {
       });
     }
 
+    // Staff always see the review link. Non-staff will see it if there are items to review
+    if (
+      this.currentUser &&
+      (this.currentUser.staff || this.currentUser.reviewable_count)
+    ) {
+      links.push({
+        route: "review",
+        className: "review",
+        label: "review.title",
+        badgeCount: "reviewable_count",
+        badgeClass: "reviewables"
+      });
+    }
+
     links.push({
       route: "discovery.top",
       className: "top-topics-link",

app/controllers/categories_controller.rb

@@ -269,37 +269,44 @@ class CategoriesController < ApplicationController
         params[:allowed_tag_groups] ||= []
       end
 
-      params.permit(*required_param_keys,
-                      :position,
-                      :email_in,
-                      :email_in_allow_strangers,
-                      :mailinglist_mirror,
-                      :suppress_from_latest,
-                      :all_topics_wiki,
-                      :parent_category_id,
-                      :auto_close_hours,
-                      :auto_close_based_on_last_post,
-                      :uploaded_logo_id,
-                      :uploaded_background_id,
-                      :slug,
-                      :allow_badges,
-                      :topic_template,
-                      :sort_order,
-                      :sort_ascending,
-                      :topic_featured_link_allowed,
-                      :show_subcategory_list,
-                      :num_featured_topics,
-                      :default_view,
-                      :subcategory_list_style,
-                      :default_top_period,
-                      :minimum_required_tags,
-                      :navigate_to_first_post_after_read,
-                      :search_priority,
-                      :allow_global_tags,
-                      custom_fields: [params[:custom_fields].try(:keys)],
-                      permissions: [*p.try(:keys)],
-                      allowed_tags: [],
-                      allowed_tag_groups: [])
+      result = params.permit(
+        *required_param_keys,
+        :position,
+        :email_in,
+        :email_in_allow_strangers,
+        :mailinglist_mirror,
+        :suppress_from_latest,
+        :all_topics_wiki,
+        :parent_category_id,
+        :auto_close_hours,
+        :auto_close_based_on_last_post,
+        :uploaded_logo_id,
+        :uploaded_background_id,
+        :slug,
+        :allow_badges,
+        :topic_template,
+        :sort_order,
+        :sort_ascending,
+        :topic_featured_link_allowed,
+        :show_subcategory_list,
+        :num_featured_topics,
+        :default_view,
+        :subcategory_list_style,
+        :default_top_period,
+        :minimum_required_tags,
+        :navigate_to_first_post_after_read,
+        :search_priority,
+        :allow_global_tags,
+        custom_fields: [params[:custom_fields].try(:keys)],
+        permissions: [*p.try(:keys)],
+        allowed_tags: [],
+        allowed_tag_groups: []
+      )
+      if SiteSetting.enable_category_group_review?
+        result[:reviewable_by_group_id] = Group.find_by(name: params[:reviewable_by_group_name])&.id
+      end
+
+      result
     end
   end
 

app/jobs/regular/notify_reviewable.rb

@@ -8,7 +8,9 @@ class Jobs::NotifyReviewable < Jobs::Base
 
     notify_admins
     notify_moderators if reviewable.reviewable_by_moderator?
-    notify_group(reviewable.reviewable_by_group) if reviewable.reviewable_by_group.present?
+    if SiteSetting.enable_category_group_review? && reviewable.reviewable_by_group.present?
+      notify_group(reviewable.reviewable_by_group)
+    end
   end
 
 protected

app/models/category.rb

@@ -71,6 +71,7 @@ class Category < ActiveRecord::Base
   after_save :reset_topic_ids_cache
   after_save :clear_url_cache
   after_save :index_search
+  after_save :update_reviewables
 
   after_destroy :reset_topic_ids_cache
   after_destroy :publish_category_deletion
@@ -92,6 +93,7 @@ class Category < ActiveRecord::Base
   has_many :tags, through: :category_tags
   has_many :category_tag_groups, dependent: :destroy
   has_many :tag_groups, through: :category_tag_groups
+  belongs_to :reviewable_by_group, class_name: 'Group'
 
   scope :latest, -> { order('topic_count DESC') }
 
@@ -612,6 +614,12 @@ class Category < ActiveRecord::Base
     SearchIndexer.index(self)
   end
 
+  def update_reviewables
+    if SiteSetting.enable_category_group_review? && saved_change_to_reviewable_by_group_id?
+      Reviewable.where(category_id: id).update_all(reviewable_by_group_id: reviewable_by_group_id)
+    end
+  end
+
   def self.find_by_slug(category_slug, parent_category_slug = nil)
     if parent_category_slug
       parent_category_id = self.where(slug: parent_category_slug, parent_category_id: nil).pluck(:id).first

app/models/group.rb

@@ -21,6 +21,8 @@ class Group < ActiveRecord::Base
   has_many :users, through: :group_users
   has_many :requesters, through: :group_requests, source: :user
   has_many :group_histories, dependent: :destroy
+  has_many :category_reviews, class_name: 'Category', foreign_key: :reviewable_by_group_id, dependent: :nullify
+  has_many :reviewables, foreign_key: :reviewable_by_group_id, dependent: :nullify
 
   has_and_belongs_to_many :web_hooks
 
@@ -47,6 +49,12 @@ class Group < ActiveRecord::Base
     SvgSprite.expire_cache
   end
 
+  def remove_review_groups
+    puts self.id!
+    Category.where(review_group_id: self.id).update_all(review_group_id: nil)
+    Category.where(review_group_id: self.id).update_all(review_group_id: nil)
+  end
+
   validate :name_format_validator
   validates :name, presence: true
   validate :automatic_membership_email_domains_format_validator

app/models/reviewable.rb

@@ -15,6 +15,7 @@ class Reviewable < ActiveRecord::Base
     end
   end
 
+  before_save :apply_review_group
   attr_accessor :created_new
   validates_presence_of :type, :status, :created_by_id
   belongs_to :target, polymorphic: true
@@ -81,22 +82,20 @@ class Reviewable < ActiveRecord::Base
     reviewable_by_moderator: false,
     potential_spam: true
   )
-    target_created_by_id = target.is_a?(Post) ? target.user_id : nil
-
-    topic = target.topic if topic.blank? && target.is_a?(Post)
-    category_id = topic.category_id if topic.present?
-
-    reviewable = create!(
+    reviewable = new(
       target: target,
-      target_created_by_id: target_created_by_id,
       topic: topic,
       created_by: created_by,
-      category_id: category_id,
       reviewable_by_moderator: reviewable_by_moderator,
       payload: payload,
       potential_spam: potential_spam
     )
     reviewable.created_new = true
+    reviewable.topic = target.topic if reviewable.topic.blank? && target.is_a?(Post)
+    reviewable.target_created_by_id = target.is_a?(Post) ? target.user_id : nil
+    reviewable.category_id = reviewable.topic.category_id if reviewable.topic.present?
+
+    reviewable.save!
     reviewable
 
   rescue ActiveRecord::RecordNotUnique
@@ -166,6 +165,11 @@ class Reviewable < ActiveRecord::Base
     )
   end
 
+  def apply_review_group
+    return unless SiteSetting.enable_category_group_review? && category.present? && category.reviewable_by_group_id
+    self.reviewable_by_group_id = category.reviewable_by_group_id
+  end
+
   def actions_for(guardian, args = nil)
     args ||= {}
     Actions.new(self, guardian).tap { |a| build_actions(a, guardian, args) }
@@ -288,10 +292,12 @@ class Reviewable < ActiveRecord::Base
     end
     return result if user.admin?
 
+    group_ids = SiteSetting.enable_category_group_review? ? user.group_users.pluck(:group_id) : []
+
     result.where(
       '(reviewable_by_moderator AND :staff) OR (reviewable_by_group_id IN (:group_ids))',
       staff: user.staff?,
-      group_ids: user.group_users.pluck(:group_id)
+      group_ids: group_ids
     ).where("category_id IS NULL OR category_id IN (?)", Guardian.new(user).allowed_category_ids)
   end
 

app/models/reviewable_queued_post.rb

@@ -9,28 +9,26 @@ class ReviewableQueuedPost < Reviewable
   end
 
   def build_actions(actions, guardian, args)
-    if guardian.is_staff?
 
-      unless approved?
-        actions.add(:approve_post) do |a|
-          a.icon = 'check'
-          a.label = "reviewables.actions.approve_post.title"
-        end
+    unless approved?
+      actions.add(:approve_post) do |a|
+        a.icon = 'check'
+        a.label = "reviewables.actions.approve_post.title"
       end
+    end
 
-      unless rejected?
-        actions.add(:reject_post) do |a|
-          a.icon = 'times'
-          a.label = "reviewables.actions.reject_post.title"
-        end
+    unless rejected?
+      actions.add(:reject_post) do |a|
+        a.icon = 'times'
+        a.label = "reviewables.actions.reject_post.title"
       end
+    end
 
-      if pending? && guardian.can_delete_user?(created_by)
-        actions.add(:delete_user) do |action|
-          action.icon = 'trash-alt'
-          action.label = 'reviewables.actions.delete_user.title'
-          action.confirm_message = 'reviewables.actions.delete_user.confirm'
-        end
+    if pending? && guardian.can_delete_user?(created_by)
+      actions.add(:delete_user) do |action|
+        action.icon = 'trash-alt'
+        action.label = 'reviewables.actions.delete_user.title'
+        action.confirm_message = 'reviewables.actions.delete_user.confirm'
       end
     end
 

app/serializers/category_serializer.rb

@@ -20,7 +20,16 @@ class CategorySerializer < BasicCategorySerializer
              :allowed_tag_groups,
              :allow_global_tags,
              :topic_featured_link_allowed,
-             :search_priority
+             :search_priority,
+             :reviewable_by_group_name
+
+  def reviewable_by_group_name
+    object.reviewable_by_group.name
+  end
+
+  def include_reviewable_by_group_name?
+    SiteSetting.enable_category_group_review? && object.reviewable_by_group_id.present?
+  end
 
   def group_permissions
     @group_permissions ||= begin

config/locales/client.en.yml

@@ -2549,6 +2549,8 @@ en:
       default_top_period: "Default Top Period:"
       allow_badges_label: "Allow badges to be awarded in this category"
       edit_permissions: "Edit Permissions"
+      reviewable_by_group: "In addition to staff, posts and flags in this category can be also be reviewed by:"
+      review_group_name: "group name"
       require_topic_approval: "Require moderator approval of all new topics"
       require_reply_approval: "Require moderator approval of all new replies"
       this_year: "this year"

config/locales/server.en.yml

@@ -1841,6 +1841,7 @@ en:
     staff_user_custom_fields: "A list of user custom fields that can be retrieved for staff members with the API."
     enable_user_directory: "Provide a directory of users for browsing"
     enable_group_directory: "Provide a directory of groups for browsing"
+    enable_category_group_review: "Allow groups to review content in specific categories"
     group_in_subject: "Set %%{optional_pm} in email subject to name of first group in PM, see: <a href='https://meta.discourse.org/t/customize-subject-format-for-standard-emails/20801' target='_blank'>Customize subject format for standard emails</a>"
     allow_anonymous_posting: "Allow users to switch to anonymous mode"
     anonymous_posting_min_trust_level: "Minimum trust level required to enable anonymous posting"

config/site_settings.yml

@@ -561,6 +561,9 @@ groups:
     default: true
   group_in_subject:
     default: false
+  enable_category_group_review:
+    client: true
+    default: false
 
 posting:
   min_post_length:

db/migrate/20190417203622_add_review_group_to_category.rb

@@ -0,0 +1,7 @@
+class AddReviewGroupToCategory < ActiveRecord::Migration[5.2]
+  def change
+    add_column :categories, :reviewable_by_group_id, :integer, null: true
+    add_index :categories, :reviewable_by_group_id
+    add_index :reviewables, :reviewable_by_group_id
+  end
+end

spec/jobs/notify_reviewable_spec.rb

@@ -11,6 +11,8 @@ describe Jobs::NotifyReviewable do
     let(:group) { group_user.group }
 
     it "will notify users of new reviewable content" do
+      SiteSetting.enable_category_group_review = true
+
       GroupUser.create!(group_id: group.id, user_id: moderator.id)
 
       # Content for admins only
@@ -49,7 +51,20 @@ describe Jobs::NotifyReviewable do
       expect(group_msg.data[:reviewable_count]).to eq(1)
     end
 
+    it "won't notify a group when disabled" do
+      SiteSetting.enable_category_group_review = false
+
+      GroupUser.create!(group_id: group.id, user_id: moderator.id)
+      r3 = Fabricate(:reviewable, reviewable_by_moderator: true, reviewable_by_group: group)
+      messages = MessageBus.track_publish("/reviewable_counts") do
+        described_class.new.execute(reviewable_id: r3.id)
+      end
+      group_msg = messages.find { |m| m.user_ids.include?(user.id) }
+      expect(group_msg).to be_blank
+    end
+
     it "respects visibility" do
+      SiteSetting.enable_category_group_review = true
       SiteSetting.min_score_default_visibility = 2.0
 
       GroupUser.create!(group_id: group.id, user_id: moderator.id)

spec/models/category_spec.rb

@@ -51,6 +51,51 @@ describe Category do
     end
   end
 
+  describe "#review_group_id" do
+    let(:group) { Fabricate(:group) }
+    let(:category) { Fabricate(:category, reviewable_by_group: group) }
+    let(:topic) { Fabricate(:topic, category: category) }
+    let(:post) { Fabricate(:post, topic: topic) }
+    let(:user) { Fabricate(:user) }
+
+    it "will add the group to the reviewable" do
+      SiteSetting.enable_category_group_review = true
+      reviewable = PostActionCreator.spam(user, post).reviewable
+      expect(reviewable.reviewable_by_group_id).to eq(group.id)
+    end
+
+    it "will add the group to the reviewable even if created manually" do
+      SiteSetting.enable_category_group_review = true
+      reviewable = ReviewableFlaggedPost.create!(
+        created_by: user,
+        payload: { raw: 'test raw' },
+        category: category
+      )
+      expect(reviewable.reviewable_by_group_id).to eq(group.id)
+    end
+
+    it "will not add add the group to the reviewable" do
+      SiteSetting.enable_category_group_review = false
+      reviewable = PostActionCreator.spam(user, post).reviewable
+      expect(reviewable.reviewable_by_group_id).to be_nil
+    end
+
+    it "will nullify the group_id if destroyed" do
+      reviewable = PostActionCreator.spam(user, post).reviewable
+      group.destroy
+      expect(category.reload.reviewable_by_group).to be_blank
+      expect(reviewable.reload.reviewable_by_group_id).to be_blank
+    end
+
+    it "will remove the reviewable_by_group if the category is updated" do
+      SiteSetting.enable_category_group_review = true
+      reviewable = PostActionCreator.spam(user, post).reviewable
+      category.reviewable_by_group_id = nil
+      category.save!
+      expect(reviewable.reload.reviewable_by_group_id).to be_nil
+    end
+  end
+
   describe "topic_create_allowed and post_create_allowed" do
     it "works" do
 

spec/models/reviewable_spec.rb

@@ -94,6 +94,7 @@ RSpec.describe Reviewable, type: :model do
       end
 
       it "works with the reviewable by group" do
+        SiteSetting.enable_category_group_review = true
         group = Fabricate(:group)
         reviewable.reviewable_by_group_id = group.id
         reviewable.save!
@@ -108,6 +109,16 @@ RSpec.describe Reviewable, type: :model do
         expect(Reviewable.list_for(user, status: :pending)).to eq([reviewable])
       end
 
+      it "doesn't allow review by group when disabled" do
+        SiteSetting.enable_category_group_review = false
+        group = Fabricate(:group)
+        reviewable.reviewable_by_group_id = group.id
+        reviewable.save!
+
+        GroupUser.create!(group_id: group.id, user_id: user.id)
+        expect(Reviewable.list_for(user, status: :pending)).to be_blank
+      end
+
       context 'Reviewing as an admin' do
         before { user.update_columns(moderator: false, admin: true) }
 

spec/requests/categories_controller_spec.rb

@@ -148,8 +148,11 @@ describe CategoriesController do
 
       describe "success" do
         it "works" do
+          SiteSetting.enable_category_group_review = true
+
           readonly = CategoryGroup.permission_types[:readonly]
           create_post = CategoryGroup.permission_types[:create_post]
+          group = Fabricate(:group)
 
           post "/categories.json", params: {
             name: "hello",
@@ -158,6 +161,7 @@ describe CategoriesController do
             slug: "hello-cat",
             auto_close_hours: 72,
             search_priority: Searchable::PRIORITIES[:ignore],
+            reviewable_by_group_name: group.name,
             permissions: {
               "everyone" => readonly,
               "staff" => create_post
@@ -165,15 +169,19 @@ describe CategoriesController do
           }
 
           expect(response.status).to eq(200)
-          category = Category.find_by(name: "hello")
+          cat_json = ::JSON.parse(response.body)['category']
+          expect(cat_json).to be_present
+          expect(cat_json['reviewable_by_group_name']).to eq(group.name)
+          expect(cat_json['name']).to eq('hello')
+          expect(cat_json['slug']).to eq('hello-cat')
+          expect(cat_json['color']).to eq('ff0')
+          expect(cat_json['auto_close_hours']).to eq(72)
+          expect(cat_json['search_priority']).to eq(Searchable::PRIORITIES[:ignore])
+
+          category = Category.find(cat_json['id'])
           expect(category.category_groups.map { |g| [g.group_id, g.permission_type] }.sort).to eq([
             [Group[:everyone].id, readonly], [Group[:staff].id, create_post]
           ])
-          expect(category.name).to eq("hello")
-          expect(category.slug).to eq("hello-cat")
-          expect(category.color).to eq("ff0")
-          expect(category.auto_close_hours).to eq(72)
-          expect(category.search_priority).to eq(Searchable::PRIORITIES[:ignore])
           expect(UserHistory.count).to eq(4) # 1 + 3 (bootstrap mode)
         end
       end

spec/serializers/category_serializer_spec.rb

@@ -4,7 +4,20 @@ require 'rails_helper'
 require_dependency 'category'
 
 describe CategorySerializer do
-  let(:category) { Fabricate(:category) }
+  let(:group) { Fabricate(:group) }
+  let(:category) { Fabricate(:category, reviewable_by_group_id: group.id) }
+
+  it "includes the reviewable by group name if enabled" do
+    SiteSetting.enable_category_group_review = true
+    json = described_class.new(category, scope: Guardian.new, root: false).as_json
+    expect(json[:reviewable_by_group_name]).to eq(group.name)
+  end
+
+  it "doesn't include the reviewable by group name if disabled" do
+    SiteSetting.enable_category_group_review = false
+    json = described_class.new(category, scope: Guardian.new, root: false).as_json
+    expect(json[:reviewable_by_group_name]).to be_blank
+  end
 
   it "includes custom fields" do
     json = described_class.new(category, scope: Guardian.new, root: false).as_json