From d00b4a2eb11dbc90dc0cee781d55e14b68efe265 Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Thu, 17 Jan 2019 11:39:28 +0000
Subject: [PATCH] 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.
---
 app/models/issue.rb                           |  3 +-
 ...n-t-check-confidential-issues-for-spam.yml |  5 +++
 spec/models/issue_spec.rb                     | 45 +++++++------------
 3 files changed, 24 insertions(+), 29 deletions(-)
 create mode 100644 changelogs/unreleased/56371-don-t-check-confidential-issues-for-spam.yml

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
+        issue.assign_attributes(new_attributes)
 
-    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 }
-
-      it 'returns false' do
-        is_expected.to be_falsey
+        expect(issue.check_for_spam?).to eq(check_for_spam?)
       end
     end
   end