Add latest changes from gitlab-org/gitlab@master

7 files changed, 157 insertions, 122 deletions

diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb
index 5dc6b6176ee..bb68a8dcbbb 100644
--- a/spec/services/issues/create_service_spec.rb
+++ b/spec/services/issues/create_service_spec.rb
@@ -385,7 +385,7 @@ describe Issues::CreateService do
 
       context 'when recaptcha was not verified' do
         before do
-          expect_next_instance_of(SpamService) do |spam_service|
+          expect_next_instance_of(SpamCheckService) do |spam_service|
             expect(spam_service).to receive_messages(check_for_spam?: true)
           end
         end
@@ -408,7 +408,7 @@ describe Issues::CreateService do
 
             it 'creates a new spam_log' do
               expect { issue }
-                .to log_spam(title: issue.title, description: issue.description, user_id: user.id, noteable_type: 'Issue')
+                  .to have_spam_log(title: issue.title, description: issue.description, user_id: user.id, noteable_type: 'Issue')
             end
 
             it 'assigns a spam_log to an issue' do
@@ -431,7 +431,7 @@ describe Issues::CreateService do
 
             it 'creates a new spam_log' do
               expect { issue }
-                .to log_spam(title: issue.title, description: issue.description, user_id: user.id, noteable_type: 'Issue')
+                  .to have_spam_log(title: issue.title, description: issue.description, user_id: user.id, noteable_type: 'Issue')
             end
 
             it 'assigns a spam_log to an issue' do
diff --git a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
index 203048984a1..4d87fa3e832 100644
--- a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
+++ b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
@@ -4,7 +4,6 @@ require 'spec_helper'
 
 describe MergeRequests::AddTodoWhenBuildFailsService do
   let(:user) { create(:user) }
-  let(:merge_request) { create(:merge_request) }
   let(:project) { create(:project, :repository) }
   let(:sha) { '1234567890abcdef1234567890abcdef12345678' }
   let(:ref) { merge_request.source_branch }
diff --git a/spec/services/notes/quick_actions_service_spec.rb b/spec/services/notes/quick_actions_service_spec.rb
index 7cb0bd41f13..7eea2a7afc6 100644
--- a/spec/services/notes/quick_actions_service_spec.rb
+++ b/spec/services/notes/quick_actions_service_spec.rb
@@ -176,7 +176,6 @@ describe Notes::QuickActionsService do
   context 'CE restriction for issue assignees' do
     describe '/assign' do
       let(:project) { create(:project) }
-      let(:maintainer) { create(:user).tap { |u| project.add_maintainer(u) } }
       let(:assignee) { create(:user) }
       let(:maintainer) { create(:user) }
       let(:service) { described_class.new(project, maintainer) }
diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb
index b2576cae575..c56c23ffc70 100644
--- a/spec/services/quick_actions/interpret_service_spec.rb
+++ b/spec/services/quick_actions/interpret_service_spec.rb
@@ -1322,11 +1322,6 @@ describe QuickActions::InterpretService do
         let(:issuable) { issue }
       end
 
-      it_behaves_like 'empty command', _('Failed to mark this issue as a duplicate because referenced issue was not found.') do
-        let(:content) { '/duplicate #{issue.to_reference}' }
-        let(:issuable) { issue }
-      end
-
       it_behaves_like 'empty command' do
         let(:content) { '/lock' }
         let(:issuable) { issue }
diff --git a/spec/services/snippets/create_service_spec.rb b/spec/services/snippets/create_service_spec.rb
index 6f7ce7959ff..3ada50e80aa 100644
--- a/spec/services/snippets/create_service_spec.rb
+++ b/spec/services/snippets/create_service_spec.rb
@@ -86,7 +86,7 @@ describe Snippets::CreateService do
 
         it 'creates a new spam_log' do
           expect { snippet }
-            .to log_spam(title: snippet.title, noteable_type: snippet.class.name)
+            .to have_spam_log(title: snippet.title, noteable_type: snippet.class.name)
         end
 
         it 'assigns a spam_log to an issue' do
diff --git a/spec/services/spam_check_service_spec.rb b/spec/services/spam_check_service_spec.rb
new file mode 100644
index 00000000000..a58c8d9cfd8
--- /dev/null
+++ b/spec/services/spam_check_service_spec.rb
@@ -0,0 +1,153 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe SpamCheckService do
+  let(:fake_ip) { '1.2.3.4' }
+  let(:fake_user_agent) { 'fake-user-agent' }
+  let(:fake_referrer) { 'fake-http-referrer' }
+  let(:env) do
+    { 'action_dispatch.remote_ip' => fake_ip,
+      'HTTP_USER_AGENT' => fake_user_agent,
+      'HTTP_REFERRER' => fake_referrer }
+  end
+  let(:request) { double(:request, env: env) }
+
+  let_it_be(:project) { create(:project, :public) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:issue) { create(:issue, project: project, author: user) }
+
+  before do
+    issue.spam = false
+  end
+
+  describe '#initialize' do
+    subject { described_class.new(spammable: issue, request: request) }
+
+    context 'when the request is nil' do
+      let(:request) { nil }
+
+      it 'assembles the options with information from the spammable' do
+        aggregate_failures do
+          expect(subject.options[:ip_address]).to eq(issue.ip_address)
+          expect(subject.options[:user_agent]).to eq(issue.user_agent)
+          expect(subject.options.key?(:referrer)).to be_falsey
+        end
+      end
+    end
+
+    context 'when the request is present' do
+      let(:request) { double(:request, env: env) }
+
+      it 'assembles the options with information from the spammable' do
+        aggregate_failures do
+          expect(subject.options[:ip_address]).to eq(fake_ip)
+          expect(subject.options[:user_agent]).to eq(fake_user_agent)
+          expect(subject.options[:referrer]).to eq(fake_referrer)
+        end
+      end
+    end
+  end
+
+  describe '#execute' do
+    let(:request) { double(:request, env: env) }
+
+    let_it_be(:existing_spam_log) { create(:spam_log, user: user, recaptcha_verified: false) }
+
+    subject do
+      described_service = described_class.new(spammable: issue, request: request)
+      described_service.execute(user_id: user.id, api: nil, recaptcha_verified: recaptcha_verified, spam_log_id: existing_spam_log.id)
+    end
+
+    context 'when recaptcha was already verified' do
+      let(:recaptcha_verified) { true }
+
+      it "updates spam log and doesn't check Akismet" do
+        aggregate_failures do
+          expect(SpamLog).not_to receive(:create!)
+          expect(an_instance_of(described_class)).not_to receive(:check)
+        end
+
+        subject
+      end
+
+      it 'updates spam log' do
+        subject
+
+        expect(existing_spam_log.reload.recaptcha_verified).to be_truthy
+      end
+    end
+
+    context 'when recaptcha was not verified' do
+      let(:recaptcha_verified) { false }
+
+      context 'when spammable attributes have not changed' do
+        before do
+          issue.closed_at = Time.zone.now
+
+          allow(AkismetService).to receive(:new).and_return(double(spam?: true))
+        end
+
+        it 'returns false' do
+          expect(subject).to be_falsey
+        end
+
+        it 'does not create a spam log' do
+          expect { subject }
+            .not_to change { SpamLog.count }
+        end
+      end
+
+      context 'when spammable attributes have changed' do
+        before do
+          issue.description = 'SPAM!'
+        end
+
+        context 'when indicated as spam by akismet' do
+          before do
+            allow(AkismetService).to receive(:new).and_return(double(spam?: true))
+          end
+
+          context 'when allow_possible_spam feature flag is false' do
+            before do
+              stub_feature_flags(allow_possible_spam: false)
+            end
+
+            it_behaves_like 'akismet spam'
+
+            it 'checks as spam' do
+              subject
+
+              expect(issue.reload.spam).to be_truthy
+            end
+          end
+
+          context 'when allow_possible_spam feature flag is true' do
+            it_behaves_like 'akismet spam'
+
+            it 'does not check as spam' do
+              subject
+
+              expect(issue.spam).to be_falsey
+            end
+          end
+        end
+
+        context 'when not indicated as spam by akismet' do
+          before do
+            allow(AkismetService).to receive(:new).and_return(double(spam?: false))
+          end
+
+          it 'returns false' do
+            expect(subject).to be_falsey
+          end
+
+          it 'does not create a spam log' do
+            expect { subject }
+              .not_to change { SpamLog.count }
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/services/spam_service_spec.rb b/spec/services/spam_service_spec.rb
deleted file mode 100644
index c8ebe87e4d2..00000000000
--- a/spec/services/spam_service_spec.rb
+++ /dev/null
@@ -1,111 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe SpamService do
-  describe '#when_recaptcha_verified' do
-    def check_spam(issue, request, recaptcha_verified)
-      described_class.new(spammable: issue, request: request).when_recaptcha_verified(recaptcha_verified) do
-        'yielded'
-      end
-    end
-
-    it 'yields block when recaptcha was already verified' do
-      issue = build_stubbed(:issue)
-
-      expect(check_spam(issue, nil, true)).to eql('yielded')
-    end
-
-    context 'when recaptcha was not verified' do
-      let(:project) { create(:project, :public) }
-      let(:issue)   { create(:issue, project: project) }
-      let(:request) { double(:request, env: {}) }
-
-      context 'when spammable attributes have not changed' do
-        before do
-          issue.closed_at = Time.zone.now
-
-          allow(AkismetService).to receive(:new).and_return(double(spam?: true))
-        end
-
-        it 'returns false' do
-          expect(check_spam(issue, request, false)).to be_falsey
-        end
-
-        it 'does not create a spam log' do
-          expect { check_spam(issue, request, false) }
-            .not_to change { SpamLog.count }
-        end
-      end
-
-      context 'when spammable attributes have changed' do
-        before do
-          issue.description = 'SPAM!'
-        end
-
-        context 'when indicated as spam by akismet' do
-          shared_examples 'akismet spam' do
-            it "doesn't check as spam when request is missing" do
-              check_spam(issue, nil, false)
-
-              expect(issue).not_to be_spam
-            end
-
-            it 'creates a spam log' do
-              expect { check_spam(issue, request, false) }
-                .to log_spam(title: issue.title, description: issue.description, noteable_type: 'Issue')
-            end
-
-            it 'does not yield to the block' do
-              expect(check_spam(issue, request, false))
-                .to eql(SpamLog.last)
-            end
-          end
-
-          before do
-            allow(AkismetService).to receive(:new).and_return(double(spam?: true))
-          end
-
-          context 'when allow_possible_spam feature flag is false' do
-            before do
-              stub_feature_flags(allow_possible_spam: false)
-            end
-
-            it_behaves_like 'akismet spam'
-
-            it 'checks as spam' do
-              check_spam(issue, request, false)
-
-              expect(issue.spam).to be_truthy
-            end
-          end
-
-          context 'when allow_possible_spam feature flag is true' do
-            it_behaves_like 'akismet spam'
-
-            it 'does not check as spam' do
-              check_spam(issue, request, false)
-
-              expect(issue.spam).to be_nil
-            end
-          end
-        end
-
-        context 'when not indicated as spam by akismet' do
-          before do
-            allow(AkismetService).to receive(:new).and_return(double(spam?: false))
-          end
-
-          it 'returns false' do
-            expect(check_spam(issue, request, false)).to be_falsey
-          end
-
-          it 'does not create a spam log' do
-            expect { check_spam(issue, request, false) }
-              .not_to change { SpamLog.count }
-          end
-        end
-      end
-    end
-  end
-end