added test for CleanUpSpammyUsers job + improved system user handling

Author: Oaphi

app/helpers/application_helper.rb

@@ -339,7 +339,7 @@ def current_user
   # Gets the special System user
   # @return [User, nil]
   def system_user
-    User.find(-1)
+    User.system
   end
 
   ##

app/jobs/clean_up_spammy_users_job.rb

@@ -8,14 +8,17 @@ def perform(created_after: 1.month.ago)
                             .where('users.created_at >= ?', created_after)
                             .where(users: { deleted: false }).group('users.id').having('count(posts.id) > 0')
                             .having('count(distinct if(posts.deleted = true, null, posts.id)) = 0')
+
     possible_spammers.each do |spammer|
       all_posts_spam = spammer.posts.all? do |post|
         # A post is considered spam if there are any helpful spam flags on it.
-        post.flags.any? { |flag| flag.post_flag_type.name == "it's spam" && flag.status == 'helpful' }
+        post.flags.any? do |flag|
+          flag.post_flag_type.name == "it's spam" && flag.status == 'helpful'
+        end
       end
       if all_posts_spam
         spammer.block('automatic block from spam cleanup job', length: 2.years)
-        spammer.soft_delete(User.find(-1))
+        spammer.soft_delete(User.system)
       end
     end
   end

app/models/user.rb

@@ -60,6 +60,12 @@ def self.search(term)
     where('username LIKE ?', "%#{sanitize_sql_like(term)}%")
   end
 
+  # Gets the system user
+  # @return [User, nil]
+  def self.system
+    find_by(id: -1)
+  end
+
   # Safely gets the user's reputation even if they don't have a community user
   # @return [Integer] user's reputation
   def reputation

test/fixtures/flags.yml

@@ -44,3 +44,11 @@ helpful_flag_scorer:
   user: flag_scorer
   community: sample
   status: helpful
+
+helpful_on_spam_question:
+  reason: It's obviously spam
+  post: deleted_spam_question (Post)
+  post_flag_type: spam
+  user: standard_user
+  community: sample
+  status: helpful

test/fixtures/post_flag_types.yml

@@ -19,3 +19,8 @@ not_confidential:
   community: sample
   confidential: false
   name: Definitely not confidential
+
+spam:
+  community: sample
+  confidential: false
+  name: it's spam

test/fixtures/posts.yml

@@ -530,3 +530,20 @@ imported_question:
   community: sample
   category: main
   license: cc_by_sa
+
+deleted_spam_question:
+  post_type: question
+  title: Best spam in the neighborhood! Call 555-55-55
+  body: Find the best spam in your neighborhood on Spam Central
+  body_markdown: <p>Find the best spam in your neighborhood on <a href="https://example.com/spam">Spam Central</a></p>
+  tags_cache:
+    - discussion
+  tags:
+    - discussion
+  user: spammer
+  community: sample
+  category: main
+  license: cc_by_sa
+  deleted: true
+  deleted_at: 2019-01-01T00:00:00.000000Z
+  deleted_by: moderator

test/jobs/clean_up_spammy_users_job_test.rb

@@ -1,7 +1,19 @@
 require 'test_helper'
 
 class CleanUpSpammyUsersJobTest < ActiveJob::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  test 'should correctly clean up spammy users' do
+    spammer = users(:spammer)
+
+    assert_not spammer.deleted?
+
+    perform_enqueued_jobs do
+      CleanUpSpammyUsersJob.perform_later
+    end
+
+    assert_performed_jobs 1
+    spammer.reload
+
+    assert spammer.deleted?
+    assert spammer.deleted_by.same_as?(User.system)
+  end
 end

test/models/user_test.rb

@@ -234,9 +234,8 @@ class UserTest < ActiveSupport::TestCase
     ability = abilities(:unrestricted)
     community = communities(:sample)
     basic = users(:basic_user)
-    system = users(:system)
 
-    restricted_users = [basic, system]
+    restricted_users = [basic, User.system]
 
     restricted_users.each do |user|
       assert_not user.ability_on?(community.id, ability.internal_id),