Merge branch 'cablett-conditional-allow-spam' into 'master'

Decouple reCAPTCHA from conditional allow

See merge request gitlab-org/gitlab!32805

app/services/spam/spam_action_service.rb

@@ -49,7 +49,8 @@ module Spam
       # ask the SpamVerdictService what to do with the target.
       spam_verdict_service.execute.tap do |result|
         case result
-        when REQUIRE_RECAPTCHA
+        when CONDITIONAL_ALLOW
+          # at the moment, this means "ask for reCAPTCHA"
           create_spam_log(api)
 
           break if target.allow_possible_spam?

app/services/spam/spam_constants.rb

@@ -2,7 +2,7 @@
 
 module Spam
   module SpamConstants
-    REQUIRE_RECAPTCHA = "recaptcha"
+    CONDITIONAL_ALLOW = "conditional_allow"
     DISALLOW = "disallow"
     ALLOW = "allow"
     BLOCK_USER = "block"
@@ -14,7 +14,7 @@ module Spam
       DISALLOW => {
         priority: 2
       },
-      REQUIRE_RECAPTCHA => {
+      CONDITIONAL_ALLOW => {
         priority: 3
       },
       ALLOW => {

app/services/spam/spam_verdict_service.rb

@@ -31,7 +31,7 @@ module Spam
 
     def akismet_verdict
       if akismet.spam?
-        Gitlab::Recaptcha.enabled? ? REQUIRE_RECAPTCHA : DISALLOW
+        Gitlab::Recaptcha.enabled? ? CONDITIONAL_ALLOW : DISALLOW
       else
         ALLOW
       end

spec/controllers/projects/issues_controller_spec.rb

@@ -536,7 +536,7 @@ describe Projects::IssuesController do
         before do
           stub_application_setting(recaptcha_enabled: true)
           expect_next_instance_of(Spam::SpamVerdictService) do |verdict_service|
-            expect(verdict_service).to receive(:execute).and_return(REQUIRE_RECAPTCHA)
+            expect(verdict_service).to receive(:execute).and_return(CONDITIONAL_ALLOW)
           end
         end
 
@@ -851,7 +851,7 @@ describe Projects::IssuesController do
           context 'when recaptcha is not verified' do
             before do
               expect_next_instance_of(Spam::SpamVerdictService) do |verdict_service|
-                expect(verdict_service).to receive(:execute).and_return(REQUIRE_RECAPTCHA)
+                expect(verdict_service).to receive(:execute).and_return(CONDITIONAL_ALLOW)
               end
             end
 
@@ -1103,7 +1103,7 @@ describe Projects::IssuesController do
         context 'when captcha is not verified' do
           before do
             expect_next_instance_of(Spam::SpamVerdictService) do |verdict_service|
-              expect(verdict_service).to receive(:execute).and_return(REQUIRE_RECAPTCHA)
+              expect(verdict_service).to receive(:execute).and_return(CONDITIONAL_ALLOW)
             end
           end
 

spec/features/issues/spam_issues_spec.rb

@@ -81,7 +81,7 @@ describe 'New issue', :js do
 
     before do
       allow_next_instance_of(Spam::SpamVerdictService) do |verdict_service|
-        allow(verdict_service).to receive(:execute).and_return(REQUIRE_RECAPTCHA)
+        allow(verdict_service).to receive(:execute).and_return(CONDITIONAL_ALLOW)
       end
 
       visit new_project_issue_path(project)

spec/features/snippets/spam_snippets_spec.rb

@@ -68,7 +68,7 @@ shared_examples_for 'snippet editor' do
   context 'when SpamVerdictService requires recaptcha' do
     before do
       expect_next_instance_of(Spam::SpamVerdictService) do |verdict_service|
-        expect(verdict_service).to receive(:execute).and_return(REQUIRE_RECAPTCHA)
+        expect(verdict_service).to receive(:execute).and_return(CONDITIONAL_ALLOW)
       end
     end
 

spec/services/issues/create_service_spec.rb

@@ -458,7 +458,7 @@ describe Issues::CreateService do
         context 'when SpamVerdictService requires reCAPTCHA' do
           before do
             expect_next_instance_of(Spam::SpamVerdictService) do |verdict_service|
-              expect(verdict_service).to receive(:execute).and_return(REQUIRE_RECAPTCHA)
+              expect(verdict_service).to receive(:execute).and_return(CONDITIONAL_ALLOW)
             end
           end
 

spec/services/spam/spam_action_service_spec.rb

@@ -163,9 +163,9 @@ describe Spam::SpamActionService do
           end
         end
 
-        context 'when spam verdict service requires reCAPTCHA' do
+        context 'when spam verdict service conditionally allows' do
           before do
-            allow(fake_verdict_service).to receive(:execute).and_return(REQUIRE_RECAPTCHA)
+            allow(fake_verdict_service).to receive(:execute).and_return(CONDITIONAL_ALLOW)
           end
 
           context 'when allow_possible_spam feature flag is false' do

spec/services/spam/spam_verdict_service_spec.rb

@@ -123,8 +123,8 @@ describe Spam::SpamVerdictService do
             stub_application_setting(recaptcha_enabled: true)
           end
 
-          it 'returns require reCAPTCHA verdict' do
-            expect(subject).to eq REQUIRE_RECAPTCHA
+          it 'returns conditionally allow verdict' do
+            expect(subject).to eq CONDITIONAL_ALLOW
           end
         end
 

spec/support/shared_contexts/spam_constants.rb

@@ -2,7 +2,7 @@
 
 shared_context 'includes Spam constants' do
   before do
-    stub_const('REQUIRE_RECAPTCHA', Spam::SpamConstants::REQUIRE_RECAPTCHA)
+    stub_const('CONDITIONAL_ALLOW', Spam::SpamConstants::CONDITIONAL_ALLOW)
     stub_const('DISALLOW', Spam::SpamConstants::DISALLOW)
     stub_const('ALLOW', Spam::SpamConstants::ALLOW)
     stub_const('BLOCK_USER', Spam::SpamConstants::BLOCK_USER)