diff --git a/app/models/category.rb b/app/models/category.rb index 92e888ebd73..ebbeaddcc94 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -184,7 +184,6 @@ class Category < ActiveRecord::Base if !SiteSetting.allow_uncategorized_topics && !guardian.is_staff? scoped = scoped.where.not(id: SiteSetting.uncategorized_category_id) end - scoped } diff --git a/lib/guardian/category_guardian.rb b/lib/guardian/category_guardian.rb index 09dd6441b76..dc15ed464eb 100644 --- a/lib/guardian/category_guardian.rb +++ b/lib/guardian/category_guardian.rb @@ -73,4 +73,13 @@ module CategoryGuardian @topic_featured_link_allowed_category_ids = Category.where(topic_featured_link_allowed: true).pluck(:id) end + + def topics_need_approval?(category) + return false if is_admin? + return category.require_topic_approval? if !SiteSetting.enable_category_group_moderation + return category.require_topic_approval? if category.reviewable_by_group_id.nil? + if category.reviewable_by_group_id + !self.user.belonging_to_group_ids.include?(category.reviewable_by_group_id) + end + end end diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index c98bbcd96b6..8281798d060 100644 --- a/lib/guardian/topic_guardian.rb +++ b/lib/guardian/topic_guardian.rb @@ -70,8 +70,7 @@ module TopicGuardian Category.find(category || SiteSetting.uncategorized_category_id) end ) - - is_staff? || (can_create_topic_on_category?(category) && !category.require_topic_approval?) + is_staff? || can_create_topic_on_category?(category) && !topics_need_approval?(category) end def can_create_post_on_topic?(topic) diff --git a/spec/lib/category_guardian_spec.rb b/spec/lib/category_guardian_spec.rb index e37deeaf940..2303f867a6b 100644 --- a/spec/lib/category_guardian_spec.rb +++ b/spec/lib/category_guardian_spec.rb @@ -4,9 +4,11 @@ RSpec.describe CategoryGuardian do fab!(:admin) { Fabricate(:admin) } fab!(:user) { Fabricate(:user) } fab!(:can_create_user) { Fabricate(:user) } + fab!(:category) do + Fabricate(:category, category_setting_attributes: { require_topic_approval: false }) + end describe "can_post_in_category?" do - fab!(:category) { Fabricate(:category) } context "when not restricted category" do it "returns false for anonymous user" do expect(Guardian.new.can_post_in_category?(category)).to eq(false) @@ -77,4 +79,59 @@ RSpec.describe CategoryGuardian do end end end + + describe "#topics_need_approval?" do + fab!(:reviewable_group) { Fabricate(:group) } + + it "returns false when admin" do + expect(Guardian.new(admin).topics_need_approval?(category)).to eq(false) + end + + it "returns the value of require_topic_approval when group moderation is off" do + SiteSetting.enable_category_group_moderation = false + category.require_topic_approval = false + category.save! + + expect(Guardian.new(user).topics_need_approval?(category)).to eq(false) + + category.require_topic_approval = true + category.save! + + expect(Guardian.new(user).topics_need_approval?(category)).to eq(true) + end + + it "returns the value of require_topic_approval when group moderation is on and there are no groups set" do + SiteSetting.enable_category_group_moderation = true + category.reviewable_by_group_id = nil + + category.require_topic_approval = false + category.save! + + expect(Guardian.new(user).topics_need_approval?(category)).to eq(false) + + category.require_topic_approval = true + category.save! + + expect(Guardian.new(user).topics_need_approval?(category)).to eq(true) + end + + it "returns false when group moderation is on and the user is in the reviewable group" do + SiteSetting.enable_category_group_moderation = true + category.require_topic_approval = true + category.reviewable_by_group_id = reviewable_group.id + category.save! + Fabricate(:group_user, group: reviewable_group, user: user) + + expect(Guardian.new(user).topics_need_approval?(category)).to eq(false) + end + + it "returns true when group moderation is on and the user is not in the reviewable group" do + SiteSetting.enable_category_group_moderation = true + category.require_topic_approval = false + category.reviewable_by_group_id = Fabricate(:group).id + category.save! + + expect(Guardian.new(user).topics_need_approval?(category)).to eq(true) + end + end end diff --git a/spec/lib/guardian/topic_guardian_spec.rb b/spec/lib/guardian/topic_guardian_spec.rb index 5b070b167c7..38eb822de39 100644 --- a/spec/lib/guardian/topic_guardian_spec.rb +++ b/spec/lib/guardian/topic_guardian_spec.rb @@ -216,6 +216,95 @@ RSpec.describe TopicGuardian do end end + describe "#can_create_topic?" do + before { SiteSetting.uncategorized_category_id = category.id } + + context "when staff" do + it "always returns true and defaults to uncategorized if absent" do + expect(Guardian.new(moderator).can_create_topic?(category)).to eq(true) + expect(Guardian.new(moderator).can_create_topic?(nil)).to eq(true) + end + end + + context "when trust level met" do + before { SiteSetting.min_trust_to_create_topic = 3 } + + it "does not allow user to create the topic if they cannot create posts" do + guardian = Guardian.new(tl3_user) + guardian.stubs(:can_create_post?).with(topic).returns(false) + + expect(guardian.can_create_topic?(topic)).to eq(false) + end + + it "does not allow user to create the topic if they cannot post to the topic's category" do + guardian = Guardian.new(tl3_user) + guardian.stubs(:can_create_post?).with(topic).returns(true) + Category.stubs(:topic_create_allowed).with(guardian).returns(Category.where("1 = 0")) + + expect(guardian.can_create_topic?(topic)).to eq(false) + end + + it "allows user to create the topic if they can create posts and in the topic's category" do + guardian = Guardian.new(tl3_user) + guardian.stubs(:can_create_post?).with(topic).returns(true) + Category + .stubs(:topic_create_allowed) + .with(guardian) + .returns(Category.where(id: topic.category_id)) + + expect(guardian.can_create_topic?(topic)).to eq(true) + end + end + + context "when trust level not met" do + it "returns false" do + SiteSetting.min_trust_to_create_topic = 4 + expect(Guardian.new(tl3_user).can_create_topic?(nil)).to eq(false) + end + end + end + + describe "#can_move_topic_to_category?" do + let(:admin_guardian) { Guardian.new(admin) } + + it "returns true if staff" do + expect(admin_guardian.can_move_topic_to_category?(category)).to eq(true) + expect(admin_guardian.can_move_topic_to_category?(category.id)).to eq(true) + end + + context "when not staff" do + let(:user_guardian) { Guardian.new(user) } + + it "returns false when the user cannot create a topic" do + user_guardian.stubs(:can_create_topic_on_category?).returns(false) + + expect(user_guardian.can_move_topic_to_category?(category)).to eq(false) + end + + it "returns false when the user needs approval in the category" do + user_guardian.stubs(:can_create_topic_on_category?).returns(true) + user_guardian.stubs(:topics_need_approval?).returns(true) + + expect(user_guardian.can_move_topic_to_category?(category)).to eq(false) + end + + it "returns true when the user can create topic and does not need approval in the category" do + user_guardian.stubs(:can_create_topic_on_category?).returns(true) + user_guardian.stubs(:topics_need_approval?).returns(false) + + expect(user_guardian.can_move_topic_to_category?(category)).to eq(true) + end + + it "defaults to uncategorized_category_id if no category provided" do + category = Fabricate(:category, id: 123) + SiteSetting.uncategorized_category_id = 123 + user_guardian.stubs(:can_create_topic_on_category?).with(category) + + user_guardian.can_move_topic_to_category?(nil) + end + end + end + describe "#can_see_unlisted_topics?" do it "is allowed for staff users" do expect(Guardian.new(moderator).can_see_unlisted_topics?).to eq(true)