Add latest changes from gitlab-org/gitlab@master

.rubocop_todo.yml

@@ -172,12 +172,6 @@ Lint/DuplicateMethods:
     - 'lib/gitlab/git/tree.rb'
     - 'lib/gitlab/git/wiki_page.rb'
 
-# Offense count: 3
-Lint/InterpolationCheck:
-  Exclude:
-    - 'spec/features/issues/filtered_search/filter_issues_spec.rb'
-    - 'spec/services/quick_actions/interpret_service_spec.rb'
-
 # Offense count: 122
 # Configuration parameters: MaximumRangeSize.
 Lint/MissingCopEnableDirective:
@@ -275,13 +269,6 @@ RSpec/ItBehavesLike:
     - 'spec/lib/gitlab/git/repository_spec.rb'
     - 'spec/services/notification_service_spec.rb'
 
-# Offense count: 3
-RSpec/IteratedExpectation:
-  Exclude:
-    - 'spec/features/admin/admin_settings_spec.rb'
-    - 'spec/lib/gitlab/gitlab_import/client_spec.rb'
-    - 'spec/lib/gitlab/legacy_github_import/client_spec.rb'
-
 # Offense count: 68
 # Cop supports --auto-correct.
 RSpec/LetBeforeExamples:
@@ -303,13 +290,6 @@ RSpec/MultipleSubjects:
   Exclude:
     - 'spec/services/merge_requests/create_from_issue_service_spec.rb'
 
-# Offense count: 3
-RSpec/OverwritingSetup:
-  Exclude:
-    - 'spec/models/email_spec.rb'
-    - 'spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb'
-    - 'spec/services/notes/quick_actions_service_spec.rb'
-
 # Offense count: 2018
 # Cop supports --auto-correct.
 # Configuration parameters: Strict, EnforcedStyle, AllowedExplicitMatchers.
@@ -556,13 +536,6 @@ Style/IfUnlessModifier:
 Style/Lambda:
   Enabled: false
 
-# Offense count: 3
-# Cop supports --auto-correct.
-Style/LineEndConcatenation:
-  Exclude:
-    - 'spec/lib/gitlab/gfm/reference_rewriter_spec.rb'
-    - 'spec/lib/gitlab/incoming_email_spec.rb'
-
 # Offense count: 17
 Style/MethodMissingSuper:
   Enabled: false
@@ -659,14 +632,6 @@ Style/PerlBackrefs:
 Style/RaiseArgs:
   Enabled: false
 
-# Offense count: 3
-# Cop supports --auto-correct.
-Style/RedundantBegin:
-  Exclude:
-    - 'app/models/merge_request.rb'
-    - 'app/services/projects/import_service.rb'
-    - 'lib/gitlab/health_checks/base_abstract_check.rb'
-
 # Offense count: 1
 # Cop supports --auto-correct.
 Style/RedundantConditional:
@@ -771,14 +736,6 @@ Style/TernaryParentheses:
     - 'spec/requests/api/pipeline_schedules_spec.rb'
     - 'spec/support/capybara.rb'
 
-# Offense count: 3
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyleForMultiline.
-# SupportedStylesForMultiline: comma, consistent_comma, no_comma
-Style/TrailingCommaInArguments:
-  Exclude:
-    - 'spec/features/markdown/copy_as_gfm_spec.rb'
-
 # Offense count: 2
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyleForMultiline.

Gemfile

@@ -302,7 +302,7 @@ gem 'sentry-raven', '~> 2.9'
 gem 'premailer-rails', '~> 1.10.3'
 
 # LabKit: Tracing and Correlation
-gem 'gitlab-labkit', '0.8.0'
+gem 'gitlab-labkit', '0.9.1'
 
 # I18n
 gem 'ruby_parser', '~> 3.8', require: false

Gemfile.lock

@@ -367,7 +367,7 @@ GEM
     github-markup (1.7.0)
     gitlab-chronic (0.10.5)
       numerizer (~> 0.2)
-    gitlab-labkit (0.8.0)
+    gitlab-labkit (0.9.1)
       actionpack (>= 5.0.0, < 6.1.0)
       activesupport (>= 5.0.0, < 6.1.0)
       grpc (~> 1.19)
@@ -1215,7 +1215,7 @@ DEPENDENCIES
   gitaly (~> 1.81.0)
   github-markup (~> 1.7.0)
   gitlab-chronic (~> 0.10.5)
-  gitlab-labkit (= 0.8.0)
+  gitlab-labkit (= 0.9.1)
   gitlab-license (~> 1.0)
   gitlab-markup (~> 1.7.0)
   gitlab-net-dns (~> 0.9.1)

app/controllers/application_controller.rb

@@ -454,6 +454,7 @@ class ApplicationController < ActionController::Base
       user: -> { auth_user },
       project: -> { @project },
       namespace: -> { @group },
+      caller_id: full_action_name,
       &block)
   end
 
@@ -551,6 +552,10 @@ class ApplicationController < ActionController::Base
     end
   end
 
+  def full_action_name
+    "#{self.class.name}##{action_name}"
+  end
+
   # A user requires a role and have the setup_for_company attribute set when they are part of the experimental signup
   # flow (executed by the Growth team). Users are redirected to the welcome page when their role is required and the
   # experiment is enabled for the current user.

app/models/merge_request.rb

@@ -821,7 +821,7 @@ class MergeRequest < ApplicationRecord
   end
 
   def check_mergeability(async: false)
-    return if Feature.enabled?(:merge_requests_conditional_mergeability_check, default_enabled: true) && !recheck_merge_status?
+    return unless recheck_merge_status?
 
     check_service = MergeRequests::MergeabilityCheckService.new(self)
 
@@ -1201,12 +1201,10 @@ class MergeRequest < ApplicationRecord
   end
 
   def in_locked_state
-    begin
-      lock_mr
-      yield
-    ensure
-      unlock_mr
-    end
+    lock_mr
+    yield
+  ensure
+    unlock_mr
   end
 
   def diverged_commits_count

app/models/spam_log.rb

@@ -12,4 +12,8 @@ class SpamLog < ApplicationRecord
   def text
     [title, description].join("\n")
   end
+
+  def self.verify_recaptcha!(id:, user_id:)
+    find_by(id: id, user_id: user_id)&.update!(recaptcha_verified: true)
+  end
 end

app/services/concerns/spam_check_methods.rb

@@ -22,14 +22,15 @@ module SpamCheckMethods
   # a dirty instance, which means it should be already assigned with the new
   # attribute values.
   # rubocop:disable Gitlab/ModuleWithInstanceVariables
-  # rubocop: disable CodeReuse/ActiveRecord
   def spam_check(spammable, user)
-    spam_service = SpamService.new(spammable: spammable, request: @request)
-
-    spam_service.when_recaptcha_verified(@recaptcha_verified, @api) do
-      user.spam_logs.find_by(id: @spam_log_id)&.update!(recaptcha_verified: true)
-    end
+    SpamCheckService.new(
+      spammable: spammable,
+      request: @request
+    ).execute(
+      api: @api,
+      recaptcha_verified: @recaptcha_verified,
+      spam_log_id: @spam_log_id,
+      user_id: user.id)
   end
-  # rubocop: enable CodeReuse/ActiveRecord
   # rubocop:enable Gitlab/ModuleWithInstanceVariables
 end

app/services/projects/import_service.rb

@@ -66,23 +66,21 @@ module Projects
     end
 
     def import_repository
-      begin
-        refmap = importer_class.try(:refmap) if has_importer?
-
-        if refmap
-          project.ensure_repository
-          project.repository.fetch_as_mirror(project.import_url, refmap: refmap)
-        else
-          gitlab_shell.import_project_repository(project)
-        end
-      rescue Gitlab::Shell::Error => e
-        # Expire cache to prevent scenarios such as:
-        # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true
-        # 2. Retried import, repo is broken or not imported but +exists?+ still returns true
-        project.repository.expire_content_cache if project.repository_exists?
+      refmap = importer_class.try(:refmap) if has_importer?
 
-        raise Error, e.message
+      if refmap
+        project.ensure_repository
+        project.repository.fetch_as_mirror(project.import_url, refmap: refmap)
+      else
+        gitlab_shell.import_project_repository(project)
       end
+    rescue Gitlab::Shell::Error => e
+      # Expire cache to prevent scenarios such as:
+      # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true
+      # 2. Retried import, repo is broken or not imported but +exists?+ still returns true
+      project.repository.expire_content_cache if project.repository_exists?
+
+      raise Error, e.message
     end
 
     def download_lfs_objects

app/services/spam_service.rb → app/services/spam_check_service.rb

 # frozen_string_literal: true
 
-class SpamService
+class SpamCheckService
   include AkismetMethods
 
   attr_accessor :spammable, :request, :options
@@ -21,14 +21,14 @@ class SpamService
     end
   end
 
-  def when_recaptcha_verified(recaptcha_verified, api = false)
-    # In case it's a request which is already verified through recaptcha, yield
-    # block.
+  def execute(api: false, recaptcha_verified:, spam_log_id:, user_id:)
     if recaptcha_verified
-      yield
+      # If it's a request which is already verified through recaptcha,
+      # update the spam log accordingly.
+      SpamLog.verify_recaptcha!(user_id: user_id, id: spam_log_id)
     else
-      # Otherwise, it goes to Akismet and check if it's a spam. If that's the
-      # case, it assigns spammable record as "spam" and create a SpamLog record.
+      # Otherwise, it goes to Akismet for spam check.
+      # If so, it assigns spammable object as "spam" and creates a SpamLog record.
       possible_spam = check(api)
       spammable.spam = possible_spam unless spammable.allow_possible_spam?
       spammable.spam_log = spam_log
@@ -38,9 +38,9 @@ class SpamService
   private
 
   def check(api)
-    return false unless request && check_for_spam?
-
-    return false unless akismet.spam?
+    return unless request
+    return unless check_for_spam?
+    return unless akismet.spam?
 
     create_spam_log(api)
     true

changelogs/unreleased/add-extra-fields-to-the-application-context.yml

+---
+title: Add extra fields to the application context
+merge_request: 22792
+author:
+type: added

doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md

@@ -542,6 +542,20 @@ group = Group.find_by_full_path 'group'
 user.max_member_access_for_group group.id
 ```
 
+### Change user password
+
+```ruby
+password = "your password"
+user = User.find_by_username('your username')
+password_attributes = {
+  password: password,
+  password_confirmation: password,
+  password_automatically_set: false
+}
+
+result = Users::UpdateService.new(user, password_attributes.merge(user: user)).execute
+```
+
 ## Groups
 
 ### Count unique users in a group and sub-groups

lib/api/api.rb

@@ -47,7 +47,8 @@ module API
       Gitlab::ApplicationContext.push(
         user: -> { current_user },
         project: -> { @project },
-        namespace: -> { @group }
+        namespace: -> { @group },
+        caller_id: route.origin
       )
     end
 

lib/api/internal/base.rb

@@ -9,7 +9,8 @@ module API
       before do
         Gitlab::ApplicationContext.push(
           user: -> { actor&.user },
-          project: -> { project }
+          project: -> { project },
+          caller_id: route.origin
         )
       end
 

lib/declarative_policy/runner.rb

@@ -33,6 +33,7 @@ module DeclarativePolicy
     attr_reader :steps
     def initialize(steps)
       @steps = steps
+      @state = nil
     end
 
     # We make sure only to run any given Runner once,

lib/feature/gitaly.rb

@@ -11,6 +11,7 @@ class Feature
         inforef_uploadpack_cache
         get_tag_messages_go
         filter_shas_with_signatures_go
+        commit_without_batch_check
       ].freeze
 
     DEFAULT_ON_FLAGS = Set.new([]).freeze

lib/gitlab/application_context.rb

@@ -5,12 +5,13 @@ module Gitlab
   class ApplicationContext
     include Gitlab::Utils::LazyAttributes
 
-    Attribute = Struct.new(:name, :type)
+    Attribute = Struct.new(:name, :type, :evaluation)
 
     APPLICATION_ATTRIBUTES = [
       Attribute.new(:project, Project),
       Attribute.new(:namespace, Namespace),
-      Attribute.new(:user, User)
+      Attribute.new(:user, User),
+      Attribute.new(:caller_id, String)
     ].freeze
 
     def self.with_context(args, &block)
@@ -37,6 +38,7 @@ module Gitlab
         hash[:user] = -> { username } if set_values.include?(:user)
         hash[:project] = -> { project_path } if set_values.include?(:project)
         hash[:root_namespace] = -> { root_namespace_path } if include_namespace?
+        hash[:caller_id] = caller_id if set_values.include?(:caller_id)
       end
     end
 
@@ -75,3 +77,5 @@ module Gitlab
     end
   end
 end
+
+Gitlab::ApplicationContext.prepend_if_ee('EE::Gitlab::ApplicationContext')

lib/gitlab/health_checks/base_abstract_check.rb

@@ -32,11 +32,9 @@ module Gitlab
       end
 
       def catch_timeout(seconds, &block)
-        begin
-          Timeout.timeout(seconds.to_i, &block)
-        rescue Timeout::Error => ex
-          ex
-        end
+        Timeout.timeout(seconds.to_i, &block)
+      rescue Timeout::Error => ex
+        ex
       end
     end
   end

lib/gitlab/marginalia.rb

@@ -2,6 +2,8 @@
 
 module Gitlab
   module Marginalia
+    cattr_accessor :enabled, default: false
+
     MARGINALIA_FEATURE_FLAG = :marginalia
 
     def self.set_application_name
@@ -15,14 +17,14 @@ module Gitlab
     end
 
     def self.cached_feature_enabled?
-      !!@enabled
+      enabled
     end
 
     def self.set_feature_cache
       # During db:create and db:bootstrap skip feature query as DB is not available yet.
       return false unless ActiveRecord::Base.connected? && Gitlab::Database.cached_table_exists?('features')
 
-      @enabled = Feature.enabled?(MARGINALIA_FEATURE_FLAG)
+      self.enabled = Feature.enabled?(MARGINALIA_FEATURE_FLAG)
     end
   end
 end

lib/gitlab/sidekiq_middleware/client_metrics.rb

@@ -9,8 +9,10 @@ module Gitlab
         @metrics = init_metrics
       end
 
-      def call(worker, _job, queue, _redis_pool)
-        labels = create_labels(worker.class, queue)
+      def call(worker_class, _job, queue, _redis_pool)
+        # worker_class can either be the string or class of the worker being enqueued.
+        worker_class = worker_class.safe_constantize if worker_class.respond_to?(:safe_constantize)
+        labels = create_labels(worker_class, queue)
 
         @metrics.fetch(ENQUEUED).increment(labels, 1)
 

lib/gitlab/sidekiq_middleware/metrics.rb

@@ -10,7 +10,7 @@ module Gitlab
 
       def create_labels(worker_class, queue)
         labels = { queue: queue.to_s, latency_sensitive: FALSE_LABEL, external_dependencies: FALSE_LABEL, feature_category: "", boundary: "" }
-        return labels unless worker_class.include? WorkerAttributes
+        return labels unless worker_class && worker_class.include?(WorkerAttributes)
 
         labels[:latency_sensitive] = bool_as_label(worker_class.latency_sensitive_worker?)
         labels[:external_dependencies] = bool_as_label(worker_class.worker_has_external_dependencies?)

locale/gitlab.pot

@@ -3341,6 +3341,9 @@ msgstr ""
 msgid "Checkout|$%{selectedPlanPrice} per user per year"
 msgstr ""
 
+msgid "Checkout|%{cardType} ending in %{lastFourDigits}"
+msgstr ""
+
 msgid "Checkout|%{name}'s GitLab subscription"
 msgstr ""
 
@@ -3362,15 +3365,57 @@ msgstr ""
 msgid "Checkout|3. Your GitLab group"
 msgstr ""
 
+msgid "Checkout|Billing address"
+msgstr ""
+
 msgid "Checkout|Checkout"
 msgstr ""
 
+msgid "Checkout|City"
+msgstr ""
+
+msgid "Checkout|Confirm purchase"
+msgstr ""
+
+msgid "Checkout|Confirming..."
+msgstr ""
+
 msgid "Checkout|Continue to billing"
 msgstr ""
 
+msgid "Checkout|Continue to payment"
+msgstr ""
+
+msgid "Checkout|Country"
+msgstr ""
+
+msgid "Checkout|Credit card form failed to load. Please try again."
+msgstr ""
+
+msgid "Checkout|Credit card form failed to load: %{message}"
+msgstr ""
+
 msgid "Checkout|Edit"
 msgstr ""
 
+msgid "Checkout|Exp %{expirationMonth}/%{expirationYear}"
+msgstr ""
+
+msgid "Checkout|Failed to confirm your order! Please try again."
+msgstr ""
+
+msgid "Checkout|Failed to confirm your order: %{message}. Please try again."
+msgstr ""
+
+msgid "Checkout|Failed to load countries. Please try again."
+msgstr ""
+
+msgid "Checkout|Failed to load states. Please try again."
+msgstr ""
+
+msgid "Checkout|Failed to register credit card. Please try again."
+msgstr ""
+
 msgid "Checkout|GitLab plan"
 msgstr ""
 
@@ -3386,6 +3431,24 @@ msgstr ""
 msgid "Checkout|Number of users"
 msgstr ""
 
+msgid "Checkout|Payment method"
+msgstr ""
+
+msgid "Checkout|Please select a country"
+msgstr ""
+
+msgid "Checkout|Please select a state"
+msgstr ""
+
+msgid "Checkout|State"
+msgstr ""
+
+msgid "Checkout|Street address"
+msgstr ""
+
+msgid "Checkout|Submitting the credit card form failed with code %{errorCode}: %{errorMessage}"
+msgstr ""
+
 msgid "Checkout|Subscription details"
 msgstr ""
 
@@ -3404,6 +3467,9 @@ msgstr ""
 msgid "Checkout|Your organization"
 msgstr ""
 
+msgid "Checkout|Zip code"
+msgstr ""
+
 msgid "Checkout|company or team"
 msgstr ""
 

package.json

@@ -39,8 +39,8 @@
     "@babel/plugin-syntax-dynamic-import": "^7.2.0",
     "@babel/plugin-syntax-import-meta": "^7.2.0",
     "@babel/preset-env": "^7.6.2",
-    "@gitlab/svgs": "^1.89.0",
-    "@gitlab/ui": "^8.18.0",
+    "@gitlab/svgs": "^1.90.0",
+    "@gitlab/ui": "^8.20.0",
     "@gitlab/visual-review-tools": "1.5.1",
     "@sentry/browser": "^5.10.2",
     "@sourcegraph/code-host-integration": "0.0.21",

spec/controllers/application_controller_spec.rb

@@ -924,7 +924,7 @@ describe ApplicationController do
     end
 
     it 'sets the group if it was available' do
-      group = build_stubbed(:group)
+      group = build(:group)
       controller.instance_variable_set(:@group, group)
 
       get :index, format: :json
@@ -933,12 +933,18 @@ describe ApplicationController do
     end
 
     it 'sets the project if one was available' do
-      project = build_stubbed(:project)
+      project = build(:project)
       controller.instance_variable_set(:@project, project)
 
       get :index, format: :json
 
       expect(json_response['meta.project']).to eq(project.full_path)
     end
+
+    it 'sets the caller_id as controller#action' do
+      get :index, format: :json
+
+      expect(json_response['meta.caller_id']).to eq('AnonymousController#index')
+    end
   end
 end

spec/features/admin/admin_settings_spec.rb

@@ -228,9 +228,7 @@ describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_not_moc
 
         click_link 'Slack notifications'
 
-        page.all('input[type=checkbox]').each do |checkbox|
-          expect(checkbox).to be_checked
-        end
+        expect(page.all('input[type=checkbox]')).to all(be_checked)
         expect(find_field('Webhook').value).to eq 'http://localhost'
         expect(find_field('Username').value).to eq 'test_user'
         expect(find('#service_push_channel').value).to eq '#test_channel'

spec/features/issues/filtered_search/filter_issues_spec.rb

@@ -192,7 +192,7 @@ describe 'Filter issues', :js do
       end
 
       it 'filters issues by label containing special characters' do
-        special_label = create(:label, project: project, title: '!@#{$%^&*()-+[]<>?/:{}|\}')
+        special_label = create(:label, project: project, title: '!@#$%^&*()-+[]<>?/:{}|\\')
         special_issue = create(:issue, title: "Issue with special character label", project: project)
         special_issue.labels << special_label
 
@@ -204,7 +204,7 @@ describe 'Filter issues', :js do
       end
 
       it 'filters issues by label not containing special characters' do
-        special_label = create(:label, project: project, title: '!@#{$%^&*()-+[]<>?/:{}|\}')
+        special_label = create(:label, project: project, title: '!@#$%^&*()-+[]<>?/:{}|\\')
         special_issue = create(:issue, title: "Issue with special character label", project: project)
         special_issue.labels << special_label
 

spec/features/markdown/copy_as_gfm_spec.rb

@@ -624,7 +624,7 @@ describe 'Copy as GFM', :js do
         GFM
 
         # table with empty heading
-        <<~GFM,
+        <<~GFM
           |  | x | y |
           |--|---|---|
           | a | 1 | 0 |
@@ -784,7 +784,7 @@ describe 'Copy as GFM', :js do
           verify(
             '.line[id="LC9"], .line[id="LC10"]',
 
-            <<~GFM,
+            <<~GFM
               ```ruby
                     raise RuntimeError, "System commands must be given as an array of strings"
                   end
@@ -826,7 +826,7 @@ describe 'Copy as GFM', :js do
           verify(
             '.line[id="LC27"], .line[id="LC28"]',
 
-            <<~GFM,
+            <<~GFM
               ```json
                   "bio": null,
                   "skype": "",

spec/features/users/login_spec.rb

@@ -152,7 +152,7 @@ describe 'Login' do
     end
   end
 
-  describe 'with two-factor authentication' do
+  describe 'with two-factor authentication', :js do
     def enter_code(code)
       fill_in 'user_otp_attempt', with: code
       click_button 'Verify code'

spec/lib/gitlab/application_context_spec.rb

@@ -43,11 +43,11 @@ describe Gitlab::ApplicationContext do
   describe '#to_lazy_hash' do
     let(:user) { build(:user) }
     let(:project) { build(:project) }
-    let(:namespace) { build(:group) }
-    let(:subgroup) { build(:group, parent: namespace) }
+    let(:namespace) { create(:group) }
+    let(:subgroup) { create(:group, parent: namespace) }
 
     def result(context)
-      context.to_lazy_hash.transform_values { |v| v.call }
+      context.to_lazy_hash.transform_values { |v| v.respond_to?(:call) ? v.call : v }
     end
 
     it 'does not call the attributes until needed' do
@@ -78,27 +78,5 @@ describe Gitlab::ApplicationContext do
       expect(result(context))
         .to include(project: project.full_path, root_namespace: project.full_path_components.first)
     end
-
-    context 'only include values for which an option was specified' do
-      using RSpec::Parameterized::TableSyntax
-
-      where(:provided_options, :expected_context_keys) do
-        [:user, :namespace, :project] | [:user, :project, :root_namespace]
-        [:user, :project]             | [:user, :project, :root_namespace]
-        [:user, :namespace]           | [:user, :root_namespace]
-        [:user]                       | [:user]
-        []                            | []
-      end
-
-      with_them do
-        it do
-          # Build a hash that has all `provided_options` as keys, and `nil` as value
-          provided_values = provided_options.map { |key| [key, nil] }.to_h
-          context = described_class.new(provided_values)
-
-          expect(context.to_lazy_hash.keys).to contain_exactly(*expected_context_keys)
-        end
-      end
-    end
   end
 end

spec/lib/gitlab/gfm/reference_rewriter_spec.rb

@@ -35,7 +35,7 @@ describe Gitlab::Gfm::ReferenceRewriter do
 
       context 'description with ignored elements' do
         let(:text) do
-          "Hi. This references #1, but not `#2`\n" +
+          "Hi. This references #1, but not `#2`\n" \
             '<pre>and not !1</pre>'
         end
 

spec/lib/gitlab/gitlab_import/client_spec.rb

@@ -13,9 +13,7 @@ describe Gitlab::GitlabImport::Client do
   end
 
   it 'all OAuth2 client options are symbols' do
-    client.client.options.keys.each do |key|
-      expect(key).to be_kind_of(Symbol)
-    end
+    expect(client.client.options.keys).to all(be_kind_of(Symbol))
   end
 
   it 'uses membership and simple flags' do

spec/lib/gitlab/incoming_email_spec.rb

@@ -99,8 +99,8 @@ describe Gitlab::IncomingEmail do
 
   context 'self.scan_fallback_references' do
     let(:references) do
-      '<issue_1@localhost>' +
-        ' <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>' +
+      '<issue_1@localhost>' \
+        ' <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>' \
         ',<exchange@microsoft.com>'
     end
 

spec/lib/gitlab/legacy_github_import/client_spec.rb

@@ -13,9 +13,7 @@ describe Gitlab::LegacyGithubImport::Client do
   end
 
   it 'convert OAuth2 client options to symbols' do
-    client.client.options.keys.each do |key|
-      expect(key).to be_kind_of(Symbol)
-    end
+    expect(client.client.options.keys).to all(be_kind_of(Symbol))
   end
 
   it 'does not crash (e.g. Settingslogic::MissingSetting) when verify_ssl config is not present' do

spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb

@@ -21,13 +21,19 @@ describe Gitlab::SidekiqMiddleware::ClientMetrics do
 
         describe '#call' do
           it 'yields block' do
-            expect { |b| subject.call(worker, job, :test, double, &b) }.to yield_control.once
+            expect { |b| subject.call(worker_class, job, :test, double, &b) }.to yield_control.once
           end
 
-          it 'increments enqueued jobs metric' do
+          it 'increments enqueued jobs metric with correct labels when worker is a string of the class' do
             expect(enqueued_jobs_metric).to receive(:increment).with(labels, 1)
 
-            subject.call(worker, job, :test, double) { nil }
+            subject.call(worker_class.to_s, job, :test, double) { nil }
+          end
+
+          it 'increments enqueued jobs metric with correct labels' do
+            expect(enqueued_jobs_metric).to receive(:increment).with(labels, 1)
+
+            subject.call(worker_class, job, :test, double) { nil }
           end
         end
       end
@@ -46,7 +52,7 @@ describe Gitlab::SidekiqMiddleware::ClientMetrics do
 
     context "when workers are attributed" do
       def create_attributed_worker_class(latency_sensitive, external_dependencies, resource_boundary, category)
-        Class.new do
+        klass = Class.new do
           include Sidekiq::Worker
           include WorkerAttributes
 
@@ -55,6 +61,7 @@ describe Gitlab::SidekiqMiddleware::ClientMetrics do
           worker_resource_boundary resource_boundary unless resource_boundary == :unknown
           feature_category category unless category.nil?
         end
+        stub_const("TestAttributedWorker", klass)
       end
 
       let(:latency_sensitive) { false }

spec/models/email_spec.rb

@@ -15,11 +15,6 @@ describe Email do
   end
 
   describe '#update_invalid_gpg_signatures' do
-    let(:user) do
-      create(:user, email: 'tula.torphy@abshire.ca').tap do |user|
-        user.skip_reconfirmation!
-      end
-    end
     let(:user) { create(:user) }
 
     it 'synchronizes the gpg keys when the email is updated' do

spec/models/merge_request_spec.rb

@@ -2150,20 +2150,10 @@ describe MergeRequest do
         subject.mark_as_mergeable!
       end
 
-      context 'and merge_requests_conditional_mergeability_check feature flag is enabled' do
-        it 'does not call MergeabilityCheckService' do
-          expect(MergeRequests::MergeabilityCheckService).not_to receive(:new)
+      it 'does not call MergeabilityCheckService' do
+        expect(MergeRequests::MergeabilityCheckService).not_to receive(:new)
 
-          subject.check_mergeability
-        end
-      end
-
-      context 'and merge_requests_conditional_mergeability_check feature flag is disabled' do
-        before do
-          stub_feature_flags(merge_requests_conditional_mergeability_check: false)
-        end
-
-        it_behaves_like 'method that executes MergeabilityCheckService'
+        subject.check_mergeability
       end
     end
   end

spec/models/spam_log_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe SpamLog do
-  let(:admin) { create(:admin) }
+  let_it_be(:admin) { create(:admin) }
 
   describe 'associations' do
     it { is_expected.to belong_to(:user) }
@@ -31,4 +31,29 @@ describe SpamLog do
       expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound)
     end
   end
+
+  describe '.verify_recaptcha!' do
+    let_it_be(:spam_log) { create(:spam_log, user: admin, recaptcha_verified: false) }
+
+    context 'the record cannot be found' do
+      it 'updates nothing' do
+        expect(instance_of(described_class)).not_to receive(:update!)
+
+        described_class.verify_recaptcha!(id: spam_log.id, user_id: admin.id)
+
+        expect(spam_log.recaptcha_verified).to be_falsey
+      end
+
+      it 'does not error despite not finding a record' do
+        expect { described_class.verify_recaptcha!(id: -1, user_id: admin.id) }.not_to raise_error
+      end
+    end
+
+    context 'the record exists' do
+      it 'updates recaptcha_verified' do
+        expect { described_class.verify_recaptcha!(id: spam_log.id, user_id: admin.id) }
+          .to change { spam_log.reload.recaptcha_verified }.from(false).to(true)
+      end
+    end
+  end
 end

spec/requests/api/internal/base_spec.rb

@@ -326,7 +326,7 @@ describe API::Internal::Base do
           expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path)
           expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage))
           expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage))
-          expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-inforef-uploadpack-cache' => 'true', 'gitaly-feature-get-tag-messages-go' => 'true', 'gitaly-feature-filter-shas-with-signatures-go' => 'true', 'gitaly-feature-cache-invalidator' => 'true')
+          expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-inforef-uploadpack-cache' => 'true', 'gitaly-feature-get-tag-messages-go' => 'true', 'gitaly-feature-filter-shas-with-signatures-go' => 'true', 'gitaly-feature-cache-invalidator' => 'true', 'gitaly-feature-commit-without-batch-check' => 'true')
           expect(user.reload.last_activity_on).to eql(Date.today)
         end
       end
@@ -346,7 +346,7 @@ describe API::Internal::Base do
             expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path)
             expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage))
             expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage))
-            expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-inforef-uploadpack-cache' => 'true', 'gitaly-feature-get-tag-messages-go' => 'true', 'gitaly-feature-filter-shas-with-signatures-go' => 'true', 'gitaly-feature-cache-invalidator' => 'true')
+            expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-inforef-uploadpack-cache' => 'true', 'gitaly-feature-get-tag-messages-go' => 'true', 'gitaly-feature-filter-shas-with-signatures-go' => 'true', 'gitaly-feature-cache-invalidator' => 'true', 'gitaly-feature-commit-without-batch-check' => 'true')
             expect(user.reload.last_activity_on).to be_nil
           end
         end
@@ -594,7 +594,7 @@ describe API::Internal::Base do
           expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path)
           expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage))
           expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage))
-          expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-inforef-uploadpack-cache' => 'true', 'gitaly-feature-get-tag-messages-go' => 'true', 'gitaly-feature-filter-shas-with-signatures-go' => 'true', 'gitaly-feature-cache-invalidator' => 'true')
+          expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-inforef-uploadpack-cache' => 'true', 'gitaly-feature-get-tag-messages-go' => 'true', 'gitaly-feature-filter-shas-with-signatures-go' => 'true', 'gitaly-feature-cache-invalidator' => 'true', 'gitaly-feature-commit-without-batch-check' => 'true')
         end
       end
 

spec/requests/api/issues/post_projects_issues_spec.rb

@@ -389,7 +389,7 @@ describe API::Issues do
     end
 
     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
       expect_next_instance_of(AkismetService) do |akismet_service|

spec/requests/api/issues/put_projects_issues_spec.rb

@@ -194,7 +194,7 @@ describe API::Issues do
     end
 
     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
       expect_next_instance_of(AkismetService) do |akismet_service|

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

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 }

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) }

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 }

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

spec/services/spam_service_spec.rb → spec/services/spam_check_service_spec.rb

@@ -2,24 +2,84 @@
 
 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'
+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
 
-    it 'yields block when recaptcha was already verified' do
-      issue = build_stubbed(:issue)
+    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(check_spam(issue, nil, true)).to eql('yielded')
+        expect(existing_spam_log.reload.recaptcha_verified).to be_truthy
+      end
     end
 
     context 'when recaptcha was not verified' do
-      let(:project) { create(:project, :public) }
-      let(:issue)   { create(:issue, project: project) }
-      let(:request) { double(:request, env: {}) }
+      let(:recaptcha_verified) { false }
 
       context 'when spammable attributes have not changed' do
         before do
@@ -29,11 +89,11 @@ describe SpamService do
         end
 
         it 'returns false' do
-          expect(check_spam(issue, request, false)).to be_falsey
+          expect(subject).to be_falsey
         end
 
         it 'does not create a spam log' do
-          expect { check_spam(issue, request, false) }
+          expect { subject }
             .not_to change { SpamLog.count }
         end
       end
@@ -44,24 +104,6 @@ describe SpamService do
         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
@@ -74,9 +116,9 @@ describe SpamService do
             it_behaves_like 'akismet spam'
 
             it 'checks as spam' do
-              check_spam(issue, request, false)
+              subject
 
-              expect(issue.spam).to be_truthy
+              expect(issue.reload.spam).to be_truthy
             end
           end
 
@@ -84,9 +126,9 @@ describe SpamService do
             it_behaves_like 'akismet spam'
 
             it 'does not check as spam' do
-              check_spam(issue, request, false)
+              subject
 
-              expect(issue.spam).to be_nil
+              expect(issue.spam).to be_falsey
             end
           end
         end
@@ -97,11 +139,11 @@ describe SpamService do
           end
 
           it 'returns false' do
-            expect(check_spam(issue, request, false)).to be_falsey
+            expect(subject).to be_falsey
           end
 
           it 'does not create a spam log' do
-            expect { check_spam(issue, request, false) }
+            expect { subject }
               .not_to change { SpamLog.count }
           end
         end

spec/support/helpers/user_login_helper.rb

@@ -13,7 +13,7 @@ module UserLoginHelper
 
   def ensure_tab_pane_counts
     tabs_count = page.all('[role="tab"]').size
-    expect(page).to have_selector('[role="tabpanel"]', count: tabs_count)
+    expect(page).to have_selector('[role="tabpanel"]', visible: :all, count: tabs_count)
   end
 
   def ensure_one_active_tab

spec/support/matchers/log_spam.rb

 # frozen_string_literal: true
 
 # This matcher checks if one spam log with provided attributes was created
+# during the block evocation.
 #
 # Example:
 #
-# expect { create_issue }.to log_spam
-RSpec::Matchers.define :log_spam do |expected|
-  def spam_logs
-    SpamLog.all
-  end
+# expect { create_issue }.to log_spam(key1: value1, key2: value2)
 
+RSpec::Matchers.define :log_spam do |expected|
   match do |block|
+    @existing_logs_count = SpamLog.count
+
     block.call
 
-    expect(spam_logs).to contain_exactly(
-      have_attributes(expected)
-    )
+    @new_logs_count = SpamLog.count
+    @last_spam_log = SpamLog.last
+
+    expect(@new_logs_count - @existing_logs_count).to eq 1
+    expect(@last_spam_log).to have_attributes(expected)
   end
 
   description do
-    count = spam_logs.count
+    count = @new_logs_count - @existing_logs_count
 
     if count == 1
       keys = expected.keys.map(&:to_s)
-      actual = spam_logs.first.attributes.slice(*keys)
+      actual = @last_spam_log.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."
@@ -32,3 +34,34 @@ RSpec::Matchers.define :log_spam do |expected|
 
   supports_block_expectations
 end
+
+# This matcher checks that the last spam log
+# has the attributes provided.
+# The spam log does not have to be created during the block evocation.
+# The number of total spam logs just has to be more than one.
+#
+# Example:
+#
+# expect { create_issue }.to have_spam_log(key1: value1, key2: value2)
+
+RSpec::Matchers.define :have_spam_log do |expected|
+  match do |block|
+    block.call
+
+    @total_logs_count = SpamLog.count
+    @latest_spam_log = SpamLog.last
+    expect(SpamLog.last).to have_attributes(expected)
+  end
+
+  description do
+    if @total_logs_count > 0
+      keys = expected.keys.map(&:to_s)
+      actual = @latest_spam_log.attributes.slice(*keys)
+      "the last spam log to have #{expected} attributes. Last spam log has #{actual} attributes instead."
+    else
+      "there to be a spam log, but there are no spam logs."
+    end
+  end
+
+  supports_block_expectations
+end

spec/support/shared_examples/spam_check_shared_examples.rb

+# frozen_string_literal: true
+
+shared_examples 'akismet spam' do
+  context 'when request is missing' do
+    subject { described_class.new(spammable: issue, request: nil) }
+
+    it "doesn't check as spam" do
+      subject
+
+      expect(issue).not_to be_spam
+    end
+  end
+
+  context 'when request exists' do
+    it 'creates a spam log' do
+      expect { subject }
+          .to log_spam(title: issue.title, description: issue.description, noteable_type: 'Issue')
+    end
+  end
+end

yarn.lock

@@ -732,15 +732,15 @@
   dependencies:
     vue-eslint-parser "^6.0.4"
 
-"@gitlab/svgs@^1.89.0":
-  version "1.89.0"
-  resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.89.0.tgz#5bdaff1b0af1cc07ed34e89c21c34c7c6a3e1caa"
-  integrity sha512-vI6VobZs6mq2Bbiej5bYMHyvtn8kD1O/uHSlyY9jgJoa2TXU+jFI9DqUpJmx8EIHt+o0qm/8G3XsFGEr5gLb7Q==
-
-"@gitlab/ui@^8.18.0":
-  version "8.18.0"
-  resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-8.18.0.tgz#11bd7d5fb2db10034fdf2544847dc9afd24cc02c"
-  integrity sha512-ihcXJDVUNvp8kv+ha+0d1rrRIG8IEWfDNICremTpl62V5kN9Eiwo0Csb8Gj20sBp9ERYCycjwpjvfU7dUwyAiw==
+"@gitlab/svgs@^1.90.0":
+  version "1.90.0"
+  resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.90.0.tgz#e6fe0ca3d353fcdbd792c10d82444383c33f539d"
+  integrity sha512-6UikaIMGosmrDAd6Lf3QIJWDM4FwhoGIN+CJuFcWeHDMbZT69LnTabuGfvOQmp2+nlI68baRTSDufaux7m9Ajw==
+
+"@gitlab/ui@^8.20.0":
+  version "8.20.0"
+  resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-8.20.0.tgz#98c0db6ddaa6b3bf8e5dcd1ae869df1819bc4186"
+  integrity sha512-Gjq7030E1Swo4HSXUrc9pKxFsmlS3pX/uq/67hGghAtFI8svJWWmCBLnSDHUVgjEW+DdmOn3hqqTWRPNaXGtOw==
   dependencies:
     "@babel/standalone" "^7.0.0"
     "@gitlab/vue-toasted" "^1.3.0"