diff --git a/app/models/issue.rb b/app/models/issue.rb
index 5ed8a119035cdf04bfd6f11fd04f6588dc246daa..3b236620ed651d62e6189d70000a83e87bc081f3 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -426,8 +426,15 @@ def visible_to_user?(user = nil)
end
def check_for_spam?
- publicly_visible? &&
- (title_changed? || description_changed? || confidential_changed?)
+ # 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?
+
+ # Only check for spam on issues which are publicly visible (and thus indexed in search engines)
+ return false unless publicly_visible?
+
+ # Only check for spam if certain attributes have changed
+ spammable_attribute_changed?
end
def as_json(options = {})
@@ -515,6 +522,14 @@ def issue_assignee_user_ids
private
+ def spammable_attribute_changed?
+ title_changed? ||
+ description_changed? ||
+ # NOTE: We need to check them for spam when issues are made non-confidential, because spam
+ # may have been added while they were confidential and thus not being checked for spam.
+ confidential_changed?(from: true, to: false)
+ end
+
# Ensure that the metrics association is safely created and respecting the unique constraint on issue_id
override :ensure_metrics
def ensure_metrics
diff --git a/ee/app/models/ee/issue.rb b/ee/app/models/ee/issue.rb
index ad424eb51de9f4b3bf0ba7cb367aaf69e279a023..160b2ccad76d991df6bf49b45d8e010fa835c613 100644
--- a/ee/app/models/ee/issue.rb
+++ b/ee/app/models/ee/issue.rb
@@ -91,11 +91,6 @@ def use_separate_indices?
end
end
- # override
- def check_for_spam?
- author.bot? && (title_changed? || description_changed? || confidential_changed?) || super
- end
-
# override
def allows_multiple_assignees?
project.feature_available?(:multiple_issue_assignees)
diff --git a/ee/spec/models/issue_spec.rb b/ee/spec/models/issue_spec.rb
index e0b572a6977dc4bd08f669a4f135070efab97f68..7eed05a4bd7ae1568305177902087fbe252a144f 100644
--- a/ee/spec/models/issue_spec.rb
+++ b/ee/spec/models/issue_spec.rb
@@ -453,38 +453,6 @@
end
end
- describe '#check_for_spam?' do
- using RSpec::Parameterized::TableSyntax
- let_it_be(:reusable_project) { create(:project) }
- let_it_be(:author) { ::User.support_bot }
-
- 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' } | true
- Gitlab::VisibilityLevel::PUBLIC | false | { title: 'woo', confidential: true } | true
- Gitlab::VisibilityLevel::INTERNAL | false | { description: 'woo' } | true
- Gitlab::VisibilityLevel::PRIVATE | true | { description: 'woo' } | true
- Gitlab::VisibilityLevel::PUBLIC | false | { description: 'original description' } | false
- Gitlab::VisibilityLevel::PRIVATE | true | { weight: 3 } | false
- end
-
- with_them do
- context 'when author is a bot' do
- it 'only checks for spam when description, title, or confidential status is updated' do
- project = reusable_project
- project.update(visibility_level: visibility_level)
- issue = create(:issue, project: project, confidential: confidential, description: 'original description', author: author)
-
- issue.assign_attributes(new_attributes)
-
- expect(issue.check_for_spam?).to eq(check_for_spam?)
- end
- end
- end
- end
-
describe '#weight' do
where(:license_value, :database_value, :expected) do
true | 5 | 5
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index edb93ecf4b6cc5ad283ef3791874f4d128419f82..1aa9019d2400f6c2f72805404534ef0d192e2856 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -1051,23 +1051,53 @@
describe '#check_for_spam?' do
using RSpec::Parameterized::TableSyntax
-
- 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
+ let_it_be(:support_bot) { ::User.support_bot }
+
+ where(:support_bot?, :visibility_level, :confidential, :new_attributes, :check_for_spam?) do
+ ### non-support-bot cases
+ # spammable attributes changing
+ false | Gitlab::VisibilityLevel::PUBLIC | false | { description: 'new' } | true
+ false | Gitlab::VisibilityLevel::PUBLIC | false | { title: 'new' } | true
+ # confidential to non-confidential
+ false | Gitlab::VisibilityLevel::PUBLIC | true | { confidential: false } | true
+ # non-confidential to confidential
+ false | Gitlab::VisibilityLevel::PUBLIC | false | { confidential: true } | false
+ # spammable attributes changing on confidential
+ false | Gitlab::VisibilityLevel::PUBLIC | true | { description: 'new' } | false
+ # spammable attributes changing while changing to confidential
+ false | Gitlab::VisibilityLevel::PUBLIC | false | { title: 'new', confidential: true } | false
+ # spammable attribute not changing
+ false | Gitlab::VisibilityLevel::PUBLIC | false | { description: 'original description' } | false
+ # non-spammable attribute changing
+ false | Gitlab::VisibilityLevel::PUBLIC | false | { weight: 3 } | false
+ # spammable attributes changing on non-public
+ false | Gitlab::VisibilityLevel::INTERNAL | false | { description: 'new' } | false
+ false | Gitlab::VisibilityLevel::PRIVATE | false | { description: 'new' } | false
+
+ ### support-bot cases
+ # confidential to non-confidential
+ true | Gitlab::VisibilityLevel::PUBLIC | true | { confidential: false } | true
+ # non-confidential to confidential
+ true | Gitlab::VisibilityLevel::PUBLIC | false | { confidential: true } | false
+ # spammable attributes changing on confidential
+ true | Gitlab::VisibilityLevel::PUBLIC | true | { description: 'new' } | true
+ # spammable attributes changing while changing to confidential
+ true | Gitlab::VisibilityLevel::PUBLIC | false | { title: 'new', confidential: true } | true
+ # spammable attributes changing on non-public
+ true | Gitlab::VisibilityLevel::INTERNAL | false | { description: 'new' } | true
+ true | Gitlab::VisibilityLevel::PRIVATE | false | { title: 'new' } | true
+ # spammable attribute not changing
+ true | Gitlab::VisibilityLevel::PUBLIC | false | { description: 'original description' } | false
+ # non-spammable attribute changing
+ true | Gitlab::VisibilityLevel::PRIVATE | true | { weight: 3 } | false
end
with_them do
- it 'checks for spam on issues that can be seen anonymously' do
+ it 'checks for spam when necessary' do
+ author = support_bot? ? support_bot : user
project = reusable_project
project.update!(visibility_level: visibility_level)
- issue = create(:issue, project: project, confidential: confidential, description: 'original description')
+ issue = create(:issue, project: project, confidential: confidential, description: 'original description', author: author)
issue.assign_attributes(new_attributes)