Don't check confidential issues for spam

Spam checks are meant for content that could be indexed by search
engines. Confidential issues aren't indexed by search engines, so we
don't need to do spam checks for them.

We do need to check for spam when an issue changes from confidential to
public, even if nothing else changed.

3 files changed, 24 insertions, 29 deletions

diff --git a/app/models/issue.rb b/app/models/issue.rb
index b7e13bcbccf..5c4ecbfdf4e 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -230,7 +230,8 @@ class Issue < ActiveRecord::Base
   end
 
   def check_for_spam?
-    project.public? && (title_changed? || description_changed?)
+    publicly_visible? &&
+      (title_changed? || description_changed? || confidential_changed?)
   end
 
   def as_json(options = {})
diff --git a/changelogs/unreleased/56371-don-t-check-confidential-issues-for-spam.yml b/changelogs/unreleased/56371-don-t-check-confidential-issues-for-spam.yml
new file mode 100644
index 00000000000..fcfa29977d1
--- /dev/null
+++ b/changelogs/unreleased/56371-don-t-check-confidential-issues-for-spam.yml
@@ -0,0 +1,5 @@
+---
+title: Do not run spam checks on confidential issues
+merge_request: 24453
+author:
+type: fixed
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 6f900a60213..5d18e085a6f 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -721,39 +721,28 @@ describe Issue do
     end
   end
 
-  describe '#check_for_spam' do
-    let(:project) { create :project, visibility_level: visibility_level }
-    let(:issue) { create :issue, project: project }
+  describe '#check_for_spam?' do
+    using RSpec::Parameterized::TableSyntax
 
-    subject do
-      issue.assign_attributes(description: description)
-      issue.check_for_spam?
+    where(:visibility_level, :confidential, :new_attributes, :check_for_spam?) do
+      Gitlab::VisibilityLevel::PUBLIC   | false | { description: 'woo' } | true
+      Gitlab::VisibilityLevel::PUBLIC   | false | { title: 'woo' } | true
+      Gitlab::VisibilityLevel::PUBLIC   | true  | { confidential: false } | true
+      Gitlab::VisibilityLevel::PUBLIC   | true  | { description: 'woo' } | false
+      Gitlab::VisibilityLevel::PUBLIC   | false | { title: 'woo', confidential: true } | false
+      Gitlab::VisibilityLevel::PUBLIC   | false | { description: 'original description' } | false
+      Gitlab::VisibilityLevel::INTERNAL | false | { description: 'woo' } | false
+      Gitlab::VisibilityLevel::PRIVATE  | false | { description: 'woo' } | false
     end
 
-    context 'when project is public and spammable attributes changed' do
-      let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC }
-      let(:description) { 'woo' }
+    with_them do
+      it 'checks for spam on issues that can be seen anonymously' do
+        project = create(:project, visibility_level: visibility_level)
+        issue = create(:issue, project: project, confidential: confidential, description: 'original description')
 
-      it 'returns true' do
-        is_expected.to be_truthy
-      end
-    end
-
-    context 'when project is private' do
-      let(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE }
-      let(:description) { issue.description }
-
-      it 'returns false' do
-        is_expected.to be_falsey
-      end
-    end
-
-    context 'when spammable attributes have not changed' do
-      let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC }
-      let(:description) { issue.description }
+        issue.assign_attributes(new_attributes)
 
-      it 'returns false' do
-        is_expected.to be_falsey
+        expect(issue.check_for_spam?).to eq(check_for_spam?)
       end
     end
   end