FIX: Clean up posts and reviewables when deleteing an Akismet flagged user. (#44)

FIX: Clean up posts and reviewables when deleteing an Akismet flagged user. (#44)

  • FIX: When deleting an Akismet flagged user, we should approve existing reviewables and delete their posts.

  • FIX: Only act on pending akismet flagged posts

diff --git a/jobs/regular/confirm_akismet_flagged_posts.rb b/jobs/regular/confirm_akismet_flagged_posts.rb
new file mode 100644
index 0000000..fde22c6
--- /dev/null
+++ b/jobs/regular/confirm_akismet_flagged_posts.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module Jobs
+  class ConfirmAkismetFlaggedPosts < Jobs::Base
+    def execute(args)
+      raise Discourse::InvalidParameters.new(:user_id) unless args[:user_id]
+      raise Discourse::InvalidParameters.new(:performed_by_id) unless args[:performed_by_id]
+
+      performed_by = User.find_by(id: args[:performed_by_id])
+      post_ids = Post.with_deleted.where(user_id: args[:user_id]).pluck(:id)
+
+      ReviewableAkismetPost.where(target_id: post_ids, status: Reviewable.statuses[:pending]).find_each do |reviewable|
+        reviewable.perform(performed_by, :confirm_spam)
+      end
+    end
+  end
+end
diff --git a/models/reviewable_akismet_user.rb b/models/reviewable_akismet_user.rb
index b999979..cce4766 100644
--- a/models/reviewable_akismet_user.rb
+++ b/models/reviewable_akismet_user.rb
@@ -24,6 +24,10 @@ class ReviewableAkismetUser < Reviewable
     if target && Guardian.new(performed_by).can_delete_user?(target)
       log_confirmation(performed_by, 'confirmed_spam_deleted')
       bouncer.submit_feedback(target, 'spam')
+      Jobs.enqueue(
+        :confirm_akismet_flagged_posts,
+        user_id: target.id, performed_by_id: performed_by.id
+      )
       UserDestroyer.new(performed_by).destroy(target, user_deletion_opts(performed_by))
     end
 
@@ -55,6 +59,7 @@ class ReviewableAkismetUser < Reviewable
     base = {
       context: I18n.t('akismet.delete_reason', performed_by: performed_by.username),
       delete_posts: true,
+      delete_as_spammer: true,
       quiet: true
     }
 
diff --git a/plugin.rb b/plugin.rb
index fd21089..21aa884 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -29,10 +29,15 @@ else
 end
 
 after_initialize do
-  require_dependency File.expand_path('../jobs/check_for_spam_posts.rb', __FILE__)
-  require_dependency File.expand_path('../jobs/regular/check_users_for_spam.rb', __FILE__)
-  require_dependency File.expand_path('../jobs/check_akismet_post.rb', __FILE__)
-  require_dependency File.expand_path('../jobs/update_akismet_status.rb', __FILE__)
+  %W[
+    check_for_spam_posts
+    regular/check_users_for_spam
+    regular/confirm_akismet_flagged_posts
+    check_akismet_post
+    update_akismet_status
+  ].each do |filename|
+    require_dependency File.expand_path("../jobs/#{filename}.rb", __FILE__)
+  end
 
   # We want to include this even if the plugin is not enabled, that's why we use false here.
   add_to_serializer(:site, :reviewable_api_enabled, false) { reviewable_api_enabled }
diff --git a/spec/jobs/regular/confirm_akismet_flagged_posts_spec.rb b/spec/jobs/regular/confirm_akismet_flagged_posts_spec.rb
new file mode 100644
index 0000000..921f194
--- /dev/null
+++ b/spec/jobs/regular/confirm_akismet_flagged_posts_spec.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Jobs::ConfirmAkismetFlaggedPosts do
+  describe '#execute' do
+    let(:user) { Fabricate(:user) }
+
+    it 'raises an exception if :user_id is not provided' do
+      expect do
+        subject.execute({})
+      end.to raise_error(Discourse::InvalidParameters)
+    end
+
+    it 'raises an exception if :performed_by_id is not provided' do
+      expect do
+        subject.execute(user_id: user.id)
+      end.to raise_error(Discourse::InvalidParameters)
+    end
+
+    let(:admin) { Fabricate(:admin) }
+
+    before do
+      @user_post_reviewable = reviewable_post_for(user)
+    end
+
+    it 'approves every flagged post' do
+      subject.execute(user_id: user.id, performed_by_id: admin.id)
+
+      updated_post_reviewable = @user_post_reviewable.reload
+
+      expect(updated_post_reviewable.status).to eq(Reviewable.statuses[:approved])
+    end
+
+    it 'approves every flagged post even if the post was already deleted' do
+      @user_post_reviewable.target.trash!
+      subject.execute(user_id: user.id, performed_by_id: admin.id)
+
+      updated_post_reviewable = @user_post_reviewable.reload
+
+      expect(updated_post_reviewable.status).to eq(Reviewable.statuses[:approved])
+    end
+
+    it 'only approves pending flagged posts' do
+      @user_post_reviewable.perform(admin, :confirm_spam)
+      last_update = @user_post_reviewable.updated_at
+      subject.execute(user_id: user.id, performed_by_id: admin.id)
+
+      updated_post_reviewable = @user_post_reviewable.reload
+
+      expect(updated_post_reviewable.updated_at).to eq(last_update)
+    end
+  end
+
+  def reviewable_post_for(user)
+    post = Fabricate(:post, user: user)
+    ReviewableAkismetPost.needs_review!(target: post, created_by: admin)
+  end
+end
diff --git a/spec/models/reviewable_akismet_user_spec.rb b/spec/models/reviewable_akismet_user_spec.rb
index 3d84f22..7727366 100644
--- a/spec/models/reviewable_akismet_user_spec.rb
+++ b/spec/models/reviewable_akismet_user_spec.rb
@@ -123,4 +123,27 @@ describe 'ReviewableAkismetUser', if: defined?(Reviewable) do
       end
     end
   end
+
+  describe 'deleting existing flagged posts for a flagged user' do
+    let(:admin) { Fabricate(:admin) }
+    let(:user) { Fabricate(:user) }
+    let(:reviewable) { ReviewableAkismetUser.needs_review!(target: user, created_by: admin) }
+
+    before do
+      UserAuthToken.generate!(user_id: user.id)
+    end
+
+    it 'queues a job to approve existing Akismet flagged posts' do
+      expect { reviewable.perform(admin, :reject_user_delete) }.to change(Jobs::ConfirmAkismetFlaggedPosts.jobs, :size).by(1)
+    end
+
+    it 'approved flagged posts by the flagged user' do
+      flagged_post = Fabricate(:post, user: user)
+      flagged_post_reviewable = ReviewableFlaggedPost.needs_review!(target: flagged_post, created_by: admin)
+
+      reviewable.perform admin, :reject_user_delete
+
+      expect(flagged_post_reviewable.reload.status).to eq(Reviewable.statuses[:approved])
+    end
+  end
 end

GitHub sha: 68206f1e