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

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

models/reviewable_akismet_user.rb

@@ -24,6 +24,10 @@ def perform_reject_user_delete(performed_by, _args)
     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 @@ def user_deletion_opts(performed_by)
     base = {
       context: I18n.t('akismet.delete_reason', performed_by: performed_by.username),
       delete_posts: true,
+      delete_as_spammer: true,
       quiet: true
     }
 

plugin.rb

@@ -29,10 +29,15 @@
 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 }

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

spec/models/reviewable_akismet_user_spec.rb

@@ -123,4 +123,27 @@ def assert_history_reflects_action(action, admin, action_name)
       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