Merge branch 'sh-fix-spam-check-author' into 'master'

Fix spam checking to consider updater of issue instead of author

See merge request gitlab-org/gitlab!66408

app/models/concerns/spammable.rb

@@ -111,7 +111,7 @@ module Spammable
   end
 
   # Override in Spammable if further checks are necessary
-  def check_for_spam?
+  def check_for_spam?(user:)
     true
   end
 

app/models/issue.rb

@@ -437,10 +437,10 @@ class Issue < ApplicationRecord
         user, project.external_authorization_classification_label)
   end
 
-  def check_for_spam?
+  def check_for_spam?(user:)
     # content created via support bots is always checked for spam, EVEN if
     # the issue is not publicly visible and/or confidential
-    return true if author.support_bot? && spammable_attribute_changed?
+    return true if user.support_bot? && spammable_attribute_changed?
 
     # Only check for spam on issues which are publicly visible (and thus indexed in search engines)
     return false unless publicly_visible?

app/models/snippet.rb

@@ -246,7 +246,7 @@ class Snippet < ApplicationRecord
     notes.includes(:author)
   end
 
-  def check_for_spam?
+  def check_for_spam?(user:)
     visibility_level_changed?(to: Snippet::PUBLIC) ||
       (public? && (title_changed? || content_changed?))
   end

app/services/spam/spam_action_service.rb

@@ -28,7 +28,7 @@ module Spam
         ServiceResponse.success(message: "CAPTCHA successfully verified")
       else
         return ServiceResponse.success(message: 'Skipped spam check because user was allowlisted') if allowlisted?(user)
-        return ServiceResponse.success(message: 'Skipped spam check because it was not required') unless check_for_spam?
+        return ServiceResponse.success(message: 'Skipped spam check because it was not required') unless check_for_spam?(user: user)
 
         perform_spam_service_check
         ServiceResponse.success(message: "Spam check performed. Check #{target.class.name} spammable model for any errors or CAPTCHA requirement")
@@ -94,7 +94,7 @@ module Spam
     def create_spam_log
       @spam_log = SpamLog.create!(
         {
-          user_id: target.author_id,
+          user_id: user.id,
           title: target.spam_title,
           description: target.spam_description,
           source_ip: spam_params.ip_address,

spec/models/concerns/spammable_spec.rb

@@ -28,11 +28,11 @@ RSpec.describe Spammable do
       it 'returns true for public project' do
         issue.project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
 
-        expect(issue.check_for_spam?).to eq(true)
+        expect(issue.check_for_spam?(user: issue.author)).to eq(true)
       end
 
       it 'returns false for other visibility levels' do
-        expect(issue.check_for_spam?).to eq(false)
+        expect(issue.check_for_spam?(user: issue.author)).to eq(false)
       end
     end
 

spec/models/issue_spec.rb

@@ -1112,14 +1112,14 @@ RSpec.describe Issue do
 
     with_them do
       it 'checks for spam when necessary' do
-        author = support_bot? ? support_bot : user
+        active_user = support_bot? ? support_bot : user
         project = reusable_project
         project.update!(visibility_level: visibility_level)
-        issue = create(:issue, project: project, confidential: confidential, description: 'original description', author: author)
+        issue = create(:issue, project: project, confidential: confidential, description: 'original description', author: support_bot)
 
         issue.assign_attributes(new_attributes)
 
-        expect(issue.check_for_spam?).to eq(check_for_spam?)
+        expect(issue.check_for_spam?(user: active_user)).to eq(check_for_spam?)
       end
     end
   end

spec/models/snippet_spec.rb

@@ -431,7 +431,7 @@ RSpec.describe Snippet do
 
     subject do
       snippet.assign_attributes(title: title)
-      snippet.check_for_spam?
+      snippet.check_for_spam?(user: snippet.author)
     end
 
     context 'when public and spammable attributes changed' do
@@ -455,7 +455,7 @@ RSpec.describe Snippet do
         snippet.save!
         snippet.visibility_level = Snippet::PUBLIC
 
-        expect(snippet.check_for_spam?).to be_truthy
+        expect(snippet.check_for_spam?(user: snippet.author)).to be_truthy
       end
     end
 

spec/services/spam/spam_action_service_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe Spam::SpamActionService do
   include_context 'includes Spam constants'
 
-  let(:issue) { create(:issue, project: project, author: user) }
+  let(:issue) { create(:issue, project: project, author: author) }
   let(:fake_ip) { '1.2.3.4' }
   let(:fake_user_agent) { 'fake-user-agent' }
   let(:fake_referer) { 'fake-http-referer' }
@@ -23,6 +23,7 @@ RSpec.describe Spam::SpamActionService do
 
   let_it_be(:project) { create(:project, :public) }
   let_it_be(:user) { create(:user) }
+  let_it_be(:author) { create(:user) }
 
   before do
     issue.spam = false