Add latest changes from gitlab-org/gitlab@master

spec/requests/api/issues/put_projects_issues_spec.rb

@@ -181,6 +181,10 @@ describe API::Issues do
   end
   describe 'PUT /projects/:id/issues/:issue_iid with spam filtering' do
+    def update_issue
+      put api("/projects/#{project.id}/issues/#{issue.iid}", user), params: params
+    end
+
     let(:params) do
       {
         title: 'updated title',
@@ -189,21 +193,52 @@ describe API::Issues do
       }
     end
-    it 'does not create a new project issue' do
-      allow_any_instance_of(SpamService).to receive_messages(check_for_spam?: true)
-      allow_any_instance_of(AkismetService).to receive_messages(spam?: true)
-      put api("/projects/#{project.id}/issues/#{issue.iid}", user), params: params
-      expect(response).to have_gitlab_http_status(400)
-      expect(json_response['message']).to eq({ 'error' => 'Spam detected' })
-
-      spam_logs = SpamLog.all
-      expect(spam_logs.count).to eq(1)
-      expect(spam_logs[0].title).to eq('updated title')
-      expect(spam_logs[0].description).to eq('content here')
-      expect(spam_logs[0].user).to eq(user)
-      expect(spam_logs[0].noteable_type).to eq('Issue')
+    before do
+      expect_next_instance_of(SpamService) do |spam_service|
+        expect(spam_service).to receive_messages(check_for_spam?: true)
+      end
+      expect_next_instance_of(AkismetService) do |akismet_service|
+        expect(akismet_service).to receive_messages(spam?: true)
+      end
+    end
+    context 'when allow_possible_spam feature flag is false' do
+      before do
+        stub_feature_flags(allow_possible_spam: false)
+      end
+      it 'does not update a project issue' do
+        expect { update_issue }.not_to change { issue.reload.title }
+      end
+
+      it 'returns correct status and message' do
+        update_issue
+
+        expect(response).to have_gitlab_http_status(400)
+        expect(json_response).to include('message' => { 'error' => 'Spam detected' })
+      end
+
+      it 'creates a new spam log entry' do
+        expect { update_issue }
+          .to log_spam(title: 'updated title', description: 'content here', user_id: user.id, noteable_type: 'Issue')
+      end
+    end
+
+    context 'when allow_possible_spam feature flag is true' do
+      it 'updates a project issue' do
+        expect { update_issue }.to change { issue.reload.title }
+      end
+
+      it 'returns correct status and message' do
+        update_issue
+
+        expect(response).to have_gitlab_http_status(200)
+      end
+
+      it 'creates a new spam log entry' do
+        expect { update_issue }
+          .to log_spam(title: 'updated title', description: 'content here', user_id: user.id, noteable_type: 'Issue')
+      end
     end
   end

spec/requests/api/project_snippets_spec.rb

@@ -198,7 +198,7 @@ describe API::ProjectSnippets do
         it 'creates a spam log' do
           expect { create_snippet(project, visibility: 'public') }
-            .to change { SpamLog.count }.by(1)
+            .to log_spam(title: 'Test Title', user_id: user.id, noteable_type: 'ProjectSnippet')
         end
       end
     end
@@ -289,7 +289,7 @@ describe API::ProjectSnippets do
         it 'creates a spam log' do
           expect { update_snippet(title: 'Foo') }
-            .to change { SpamLog.count }.by(1)
+            .to log_spam(title: 'Foo', user_id: admin.id, noteable_type: 'ProjectSnippet')
         end
       end
@@ -306,7 +306,7 @@ describe API::ProjectSnippets do
         it 'creates a spam log' do
           expect { update_snippet(title: 'Foo', visibility: 'public') }
-            .to change { SpamLog.count }.by(1)
+            .to log_spam(title: 'Foo', user_id: admin.id, noteable_type: 'ProjectSnippet')
         end
       end
     end

spec/requests/api/snippets_spec.rb

@@ -254,7 +254,7 @@ describe API::Snippets do
         it 'creates a spam log' do
           expect { create_snippet(visibility: 'public') }
-            .to change { SpamLog.count }.by(1)
+            .to log_spam(title: 'Test Title', user_id: user.id, noteable_type: 'PersonalSnippet')
         end
       end
     end
@@ -344,8 +344,7 @@ describe API::Snippets do
         end
         it 'creates a spam log' do
-          expect { update_snippet(title: 'Foo') }
-            .to change { SpamLog.count }.by(1)
+          expect { update_snippet(title: 'Foo') }.to log_spam(title: 'Foo', user_id: user.id, noteable_type: 'PersonalSnippet')
         end
       end
@@ -359,7 +358,7 @@ describe API::Snippets do
         it 'creates a spam log' do
           expect { update_snippet(title: 'Foo', visibility: 'public') }
-            .to change { SpamLog.count }.by(1)
+            .to log_spam(title: 'Foo', user_id: user.id, noteable_type: 'PersonalSnippet')
         end
       end
     end

spec/services/create_snippet_service_spec.rb

@@ -3,26 +3,28 @@
 require 'spec_helper'
 describe CreateSnippetService do
-  before do
-    @user = create :user
-    @admin = create :user, admin: true
-    @opts = {
+  let(:user) { create(:user) }
+  let(:admin) { create(:user, :admin) }
+  let(:opts) { base_opts.merge(extra_opts) }
+  let(:base_opts) do
+    {
       title: 'Test snippet',
       file_name: 'snippet.rb',
       content: 'puts "hello world"',
       visibility_level: Gitlab::VisibilityLevel::PRIVATE
     }
   end
+  let(:extra_opts) { {} }
   context 'When public visibility is restricted' do
+    let(:extra_opts) { { visibility_level: Gitlab::VisibilityLevel::PUBLIC } }
+
     before do
       stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
-
-      @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
     end
     it 'non-admins are not able to create a public snippet' do
-      snippet = create_snippet(nil, @user, @opts)
+      snippet = create_snippet(nil, user, opts)
       expect(snippet.errors.messages).to have_key(:visibility_level)
       expect(snippet.errors.messages[:visibility_level].first).to(
         match('has been restricted')
@@ -30,37 +32,81 @@ describe CreateSnippetService do
     end
     it 'admins are able to create a public snippet' do
-      snippet = create_snippet(nil, @admin, @opts)
+      snippet = create_snippet(nil, admin, opts)
       expect(snippet.errors.any?).to be_falsey
       expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC)
     end
     describe "when visibility level is passed as a string" do
+      let(:extra_opts) { { visibility: 'internal' } }
+
       before do
-        @opts[:visibility] = 'internal'
-        @opts.delete(:visibility_level)
+        base_opts.delete(:visibility_level)
       end
       it "assigns the correct visibility level" do
-        snippet = create_snippet(nil, @user, @opts)
+        snippet = create_snippet(nil, user, opts)
         expect(snippet.errors.any?).to be_falsey
         expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
       end
     end
   end
+  context 'checking spam' do
+    shared_examples 'marked as spam' do
+      let(:snippet) { create_snippet(nil, admin, opts) }
+
+      it 'marks a snippet as a spam ' do
+        expect(snippet).to be_spam
+      end
+
+      it 'invalidates the snippet' do
+        expect(snippet).to be_invalid
+      end
+
+      it 'creates a new spam_log' do
+        expect { snippet }
+          .to log_spam(title: snippet.title, noteable_type: 'PersonalSnippet')
+      end
+
+      it 'assigns a spam_log to an issue' do
+        expect(snippet.spam_log).to eq(SpamLog.last)
+      end
+    end
+
+    let(:extra_opts) do
+      { visibility_level: Gitlab::VisibilityLevel::PUBLIC, request: double(:request, env: {}) }
+    end
+
+    before do
+      expect_next_instance_of(AkismetService) do |akismet_service|
+        expect(akismet_service).to receive_messages(spam?: true)
+      end
+    end
+
+    [true, false, nil].each do |allow_possible_spam|
+      context "when recaptcha_disabled flag is #{allow_possible_spam.inspect}" do
+        before do
+          stub_feature_flags(allow_possible_spam: allow_possible_spam) unless allow_possible_spam.nil?
+        end
+
+        it_behaves_like 'marked as spam'
+      end
+    end
+  end
+
   describe 'usage counter' do
     let(:counter) { Gitlab::UsageDataCounters::SnippetCounter }
     it 'increments count' do
       expect do
-        create_snippet(nil, @admin, @opts)
+        create_snippet(nil, admin, opts)
       end.to change { counter.read(:create) }.by 1
     end
     it 'does not increment count if create fails' do
       expect do
-        create_snippet(nil, @admin, {})
+        create_snippet(nil, admin, {})
       end.not_to change { counter.read(:create) }
     end
   end

spec/services/issues/create_service_spec.rb

@@ -344,7 +344,7 @@ describe Issues::CreateService do
       end
       before do
-        allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true)
+        stub_feature_flags(allow_possible_spam: false)
       end
       context 'when recaptcha was verified' do
@@ -384,31 +384,67 @@ describe Issues::CreateService do
       end
       context 'when recaptcha was not verified' do
+        before do
+          expect_next_instance_of(SpamService) do |spam_service|
+            expect(spam_service).to receive_messages(check_for_spam?: true)
+          end
+        end
+
         context 'when akismet detects spam' do
           before do
-            allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true)
+            expect_next_instance_of(AkismetService) do |akismet_service|
+              expect(akismet_service).to receive_messages(spam?: true)
+            end
           end
-          it 'marks an issue as a spam ' do
-            expect(issue).to be_spam
-          end
-          it 'an issue is not valid ' do
-            expect(issue.valid?).to be_falsey
-          end
-          it 'creates a new spam_log' do
-            expect {issue}.to change {SpamLog.count}.from(0).to(1)
+          context 'when issuables_recaptcha_enabled feature flag is true' do
+            it 'marks an issue as a spam ' do
+              expect(issue).to be_spam
+            end
+            it 'invalidates the issue' do
+              expect(issue).to be_invalid
+            end
+
+            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')
+            end
+            it 'assigns a spam_log to an issue' do
+              expect(issue.spam_log).to eq(SpamLog.last)
+            end
           end
-          it 'assigns a spam_log to an issue' do
-            expect(issue.spam_log).to eq(SpamLog.last)
+          context 'when issuable_recaptcha_enabled feature flag is false' do
+            before do
+              stub_feature_flags(allow_possible_spam: true)
+            end
+
+            it 'does not mark an issue as a spam ' do
+              expect(issue).not_to be_spam
+            end
+
+            it 'accepts the ​issue as valid' do
+              expect(issue).to be_valid
+            end
+
+            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')
+            end
+
+            it 'assigns a spam_log to an issue' do
+              expect(issue.spam_log).to eq(SpamLog.last)
+            end
           end
         end
         context 'when akismet does not detect spam' do
           before do
-            allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false)
+            expect_next_instance_of(AkismetService) do |akismet_service|
+              expect(akismet_service).to receive_messages(spam?: false)
+            end
           end
           it 'does not mark an issue as a spam ' do

spec/services/spam_service_spec.rb

@@ -44,30 +44,50 @@ describe SpamService do
         end
         context 'when indicated as spam by akismet' do
+          shared_examples 'akismet spam' do
+            it 'doesnt 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
-          it 'doesnt check as spam when request is missing' do
-            check_spam(issue, nil, false)
-            expect(issue.spam).to be_falsey
-          end
-          it 'checks as spam' do
-            check_spam(issue, request, false)
-            expect(issue.spam).to be_truthy
+          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
-          it 'creates a spam log' do
-            expect { check_spam(issue, request, false) }
-              .to change { SpamLog.count }.from(0).to(1)
-          end
-          it 'doesnt yield block' do
-            expect(check_spam(issue, request, false))
-              .to eql(SpamLog.last)
+          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

spec/support/matchers/log_spam.rb

+# frozen_string_literal: true
+
+# This matcher checkes if one spam log with provided attributes was created
+#
+# Example:
+#
+# expect { create_issue }.to log_spam
+RSpec::Matchers.define :log_spam do |expected|
+  def spam_logs
+    SpamLog.all
+  end
+
+  match do |block|
+    block.call
+
+    expect(spam_logs).to contain_exactly(
+      have_attributes(expected)
+    )
+  end
+
+  description do
+    count = spam_logs.count
+
+    if count == 1
+      keys = expected.keys.map(&:to_s)
+      actual = spam_logs.first.attributes.slice(*keys)
+      "create a spam log with #{expected} attributes. #{actual} created instead."
+    else
+      "create exactly 1 spam log with #{expected} attributes. #{count} spam logs created instead."
+    end
+  end
+
+  supports_block_expectations
+end