diff --git a/app/assets/javascripts/discourse/components/edit-category-settings.js.es6 b/app/assets/javascripts/discourse/components/edit-category-settings.js.es6 index b8a63ee653a..1cc6a2d8d92 100644 --- a/app/assets/javascripts/discourse/components/edit-category-settings.js.es6 +++ b/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 [ diff --git a/app/assets/javascripts/discourse/models/category.js.es6 b/app/assets/javascripts/discourse/models/category.js.es6 index 9e0c0470dcd..ce8a3c5d008 100644 --- a/app/assets/javascripts/discourse/models/category.js.es6 +++ b/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" }); diff --git a/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs b/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs index d2783f1dd3b..8c1f5472d33 100644 --- a/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs +++ b/app/assets/javascripts/discourse/templates/components/edit-category-settings.hbs @@ -25,7 +25,7 @@ {{i18n "category.num_featured_topics"}} {{/if}} - {{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"}}
@@ -85,6 +85,19 @@

{{i18n 'category.settings_sections.moderation'}}

+ {{#if siteSettings.enable_category_group_review}} +
+ + {{group-selector + groupFinder=groupFinder + single="true" + groupNames=category.reviewable_by_group_name + placeholderKey="category.review_group_name"}} +
+ {{/if}} +
{{/if}} @@ -125,7 +138,7 @@ - {{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"}}
diff --git a/app/assets/javascripts/discourse/widgets/hamburger-menu.js.es6 b/app/assets/javascripts/discourse/widgets/hamburger-menu.js.es6 index 24032f83041..ad610b70e5f 100644 --- a/app/assets/javascripts/discourse/widgets/hamburger-menu.js.es6 +++ b/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", diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index 44435836da7..7723b7fe5e1 100644 --- a/app/controllers/categories_controller.rb +++ b/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 diff --git a/app/jobs/regular/notify_reviewable.rb b/app/jobs/regular/notify_reviewable.rb index 734121a5e10..110fef1c7a5 100644 --- a/app/jobs/regular/notify_reviewable.rb +++ b/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 diff --git a/app/models/category.rb b/app/models/category.rb index fac08a8e8a5..a4cc14ecefd 100644 --- a/app/models/category.rb +++ b/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 diff --git a/app/models/group.rb b/app/models/group.rb index ce64c08c3a9..82ceee125ed 100644 --- a/app/models/group.rb +++ b/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 diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb index 3ac334b472e..d022a4c167b 100644 --- a/app/models/reviewable.rb +++ b/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 diff --git a/app/models/reviewable_queued_post.rb b/app/models/reviewable_queued_post.rb index e05f164bc1b..30c854df507 100644 --- a/app/models/reviewable_queued_post.rb +++ b/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 diff --git a/app/serializers/category_serializer.rb b/app/serializers/category_serializer.rb index 64f81fd6285..e3e7e5d4c46 100644 --- a/app/serializers/category_serializer.rb +++ b/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 diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 336179ecb24..63f6b116c7f 100644 --- a/config/locales/client.en.yml +++ b/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" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ead623ebb41..c3332cf26fb 100644 --- a/config/locales/server.en.yml +++ b/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: Customize subject format for standard emails" allow_anonymous_posting: "Allow users to switch to anonymous mode" anonymous_posting_min_trust_level: "Minimum trust level required to enable anonymous posting" diff --git a/config/site_settings.yml b/config/site_settings.yml index 82f55ee97d9..564d909e9d3 100644 --- a/config/site_settings.yml +++ b/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: diff --git a/db/migrate/20190417203622_add_review_group_to_category.rb b/db/migrate/20190417203622_add_review_group_to_category.rb new file mode 100644 index 00000000000..7dc6626336d --- /dev/null +++ b/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 diff --git a/spec/jobs/notify_reviewable_spec.rb b/spec/jobs/notify_reviewable_spec.rb index 69866c9da78..7ce15536d32 100644 --- a/spec/jobs/notify_reviewable_spec.rb +++ b/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) diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index a7755e0f7fc..7aa2bd87af6 100644 --- a/spec/models/category_spec.rb +++ b/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 diff --git a/spec/models/reviewable_spec.rb b/spec/models/reviewable_spec.rb index 8d1539f55cb..c661ac25b38 100644 --- a/spec/models/reviewable_spec.rb +++ b/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) } diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb index 4d919c0fcc4..cc69d5f4c00 100644 --- a/spec/requests/categories_controller_spec.rb +++ b/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 diff --git a/spec/serializers/category_serializer_spec.rb b/spec/serializers/category_serializer_spec.rb index c141ceebb6a..8a122e2336d 100644 --- a/spec/serializers/category_serializer_spec.rb +++ b/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