FIX: Allow category moderators to move topics to their categories (#20896)

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
         }
 

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

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)

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

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)