FIX: Remove custom fields from unapproved posts when user is removed as expert (#40)

FIX: Remove custom fields from unapproved posts when user is removed as expert (#40)

diff --git a/app/jobs/regular/unapprove_past_category_expert_posts.rb b/app/jobs/regular/unapprove_past_category_expert_posts.rb
index fc60d52..d7c390b 100644
--- a/app/jobs/regular/unapprove_past_category_expert_posts.rb
+++ b/app/jobs/regular/unapprove_past_category_expert_posts.rb
@@ -6,8 +6,6 @@ module Jobs
     sidekiq_options queue: 'low'
 
     def execute(args)
-      return unless SiteSetting.approve_past_posts_on_becoming_category_expert
-
       unless args[:user_id].kind_of?(Integer)
         raise Discourse::InvalidParameters.new(:user_id)
       end
@@ -16,25 +14,44 @@ module Jobs
         raise Discourse::InvalidParameters.new(:category_ids)
       end
 
-      posts = Post.joins(:topic)
-        .where(user_id: args[:user_id])
-        .where(topic: { category_id: args[:category_ids] })
+      user = User.find_by(id: args[:user_id])
+      raise Discourse::InvalidParameters.new(:user_id) unless user
+
+      # The user was removed as category expert from 1 group. They could
+      # still be a category expert by another group memebership.
+      # Filter out those categories.
+      categories = Category.where(id: args[:category_ids]).filter do |c|
+        user.expert_group_ids_for_category(c).empty?
+      end
+
+      posts = Post
+        .joins(:topic)
+        .joins("LEFT OUTER JOIN post_custom_fields AS pcf ON pcf.post_id = posts.id")
+        .where("pcf.name = ? or pcf.name = ?",
+               CategoryExperts::POST_APPROVED_GROUP_NAME,
+               CategoryExperts::POST_PENDING_EXPERT_APPROVAL)
+        .where(user_id: user.id)
+        .where(topic: { category_id: categories.map(&:id) })
 
       posts.group_by(&:topic).each do |topic, grouped_posts|
         grouped_posts.each do |post|
-          post.custom_fields.delete(CategoryExperts::POST_APPROVED_GROUP_NAME)
+          if SiteSetting.approve_past_posts_on_becoming_category_expert
+            post.custom_fields.delete(CategoryExperts::POST_APPROVED_GROUP_NAME)
+          end
           post.custom_fields.delete(CategoryExperts::POST_PENDING_EXPERT_APPROVAL)
           post.save
         end
 
-        other_approved_post_count = PostCustomField
-          .where(post_id: topic.post_ids)
-          .where(name: CategoryExperts::POST_APPROVED_GROUP_NAME)
-          .count
+        if SiteSetting.approve_past_posts_on_becoming_category_expert
+          other_approved_post_count = PostCustomField
+            .where(post_id: topic.post_ids)
+            .where(name: CategoryExperts::POST_APPROVED_GROUP_NAME)
+            .count
 
-        if other_approved_post_count == 0
-          topic.custom_fields.delete(CategoryExperts::TOPIC_EXPERT_POST_GROUP_NAMES)
-          topic.save
+          if other_approved_post_count == 0
+            topic.custom_fields.delete(CategoryExperts::TOPIC_EXPERT_POST_GROUP_NAMES)
+            topic.save
+          end
         end
       end
     end
diff --git a/lib/category_experts/post_handler.rb b/lib/category_experts/post_handler.rb
index 978c0cb..51b2130 100644
--- a/lib/category_experts/post_handler.rb
+++ b/lib/category_experts/post_handler.rb
@@ -91,20 +91,13 @@ module CategoryExperts
     private
 
     def ensure_poster_is_category_expert
-      expert_group_ids.length && users_expert_group
-    end
-
-    def expert_group_ids
-      unsplit_group_ids = post.topic.category&.custom_fields&.[](CategoryExperts::CATEGORY_EXPERT_GROUP_IDS)
-      return [] if unsplit_group_ids.nil?
-
-      unsplit_group_ids.split("|").map(&:to_i)
+      !users_expert_group.nil?
     end
 
     def users_expert_group
       return @users_expert_group if defined?(@users_expert_group) # memoizing a potentially falsy value
 
-      group_id = ((expert_group_ids & user.group_ids) || []).first
+      group_id = user.expert_group_ids_for_category(post.topic.category)&.first
       @users_expert_group = group_id.nil? ? nil : Group.find_by(id: group_id)
     end
   end
diff --git a/plugin.rb b/plugin.rb
index b86d426..e767416 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -108,6 +108,13 @@ after_initialize do
     given_category_expert_endorsements.where(endorsed_user_id: user.id)
   end
 
+  add_to_class(:user, :expert_group_ids_for_category) do |category|
+    unsplit_expert_group_ids = category.custom_fields&.[](CategoryExperts::CATEGORY_EXPERT_GROUP_IDS)
+    return [] if unsplit_expert_group_ids.nil?
+
+    unsplit_expert_group_ids.split("|").map(&:to_i) & group_ids
+  end
+
   add_to_serializer(:current_user, :expert_for_category_ids) do
     user_group_ids = object.group_ids
     return [] if user_group_ids.empty?
@@ -321,8 +328,6 @@ after_initialize do
   end
 
   DiscourseEvent.on(:user_removed_from_group) do |user, group|
-    next if !SiteSetting.approve_past_posts_on_becoming_category_expert
-
     category_ids = group.category_expert_category_ids
     next if category_ids.empty?
 
diff --git a/spec/jobs/unapprove_past_category_expert_posts_spec.rb b/spec/jobs/unapprove_past_category_expert_posts_spec.rb
new file mode 100644
index 0000000..88477bf
--- /dev/null
+++ b/spec/jobs/unapprove_past_category_expert_posts_spec.rb
@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Jobs::UnapprovePastCategoryExpertPosts do
+  fab!(:user) { Fabricate(:user) }
+  fab!(:group) { Fabricate(:group, users: [user]) }
+  fab!(:category) { fabricate_category_with_category_experts }
+  fab!(:topic) { Fabricate(:topic, category: category) }
+  fab!(:p1) { create_post(user: user, topic: topic) }
+  fab!(:p2) { create_post(user: user, topic: topic) }
+  fab!(:p3) { create_post(user: user, topic: topic) }
+  fab!(:p4) { create_post(user: user, topic: topic) }
+
+  def fabricate_category_with_category_experts
+    category = Fabricate(:category)
+    category.custom_fields[CategoryExperts::CATEGORY_EXPERT_GROUP_IDS] = group.id
+    category.save
+    category
+  end
+
+  before do
+    SiteSetting.enable_category_experts = true
+    Jobs.run_immediately!
+
+    topic.custom_fields[CategoryExperts::TOPIC_EXPERT_POST_GROUP_NAMES] = group.name
+    topic.save!
+
+    # Create 2 approved posts and 2 pending approval
+    p1.custom_fields[CategoryExperts::POST_APPROVED_GROUP_NAME] = group.name
+    p1.save!
+
+    p2.custom_fields[CategoryExperts::POST_APPROVED_GROUP_NAME] = group.name
+    p2.save!
+
+    p3.custom_fields[CategoryExperts::POST_PENDING_EXPERT_APPROVAL] = "t"
+    p3.save!
+
+    p4.custom_fields[CategoryExperts::POST_PENDING_EXPERT_APPROVAL] = "t"
+    p4.save!
+  end
+
+  describe "Site setting approve_past_posts_on_becoming_category_expert is false" do
+    it "only removed pending approval" do
+      SiteSetting.approve_past_posts_on_becoming_category_expert = false
+      group.remove(user)
+
+      expect(p1.reload.custom_fields[CategoryExperts::POST_APPROVED_GROUP_NAME]).to eq(group.name)
+      expect(p2.reload.custom_fields[CategoryExperts::POST_APPROVED_GROUP_NAME]).to eq(group.name)
+      expect(p3.reload.custom_fields[CategoryExperts::POST_PENDING_EXPERT_APPROVAL]).to eq(nil)
+      expect(p4.reload.custom_fields[CategoryExperts::POST_PENDING_EXPERT_APPROVAL]).to eq(nil)
+      expect(topic.reload.custom_fields[CategoryExperts::TOPIC_EXPERT_POST_GROUP_NAMES]).to eq(group.name)
+    end
+  end
+
+  describe "Site setting approve_past_posts_on_becoming_category_expert is true" do
+    it "only removed pending approval" do
+      SiteSetting.approve_past_posts_on_becoming_category_expert = true
+      group.remove(user)
+
+      expect(p1.reload.custom_fields[CategoryExperts::POST_APPROVED_GROUP_NAME]).to eq(nil)
+      expect(p2.reload.custom_fields[CategoryExperts::POST_APPROVED_GROUP_NAME]).to eq(nil)
+      expect(p3.reload.custom_fields[CategoryExperts::POST_PENDING_EXPERT_APPROVAL]).to eq(nil)
+      expect(p4.reload.custom_fields[CategoryExperts::POST_PENDING_EXPERT_APPROVAL]).to eq(nil)
+      expect(topic.reload.custom_fields[CategoryExperts::TOPIC_EXPERT_POST_GROUP_NAMES]).to eq(nil)
+    end
+  end
+end

GitHub sha: dd7fbd2f0999adf3fee234119ac299839816abb4

This commit appears in #40 which was approved by eviltrout. It was merged by markvanlan.