Add latest changes from gitlab-org/gitlab@master

6 files changed, 45 insertions, 39 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 60b5d9b6da8..c6d91308123 100644
--- a/app/controllers/application_controller.rb
+++ b/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.
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 7042bc7074f..50346d97d8b 100644
--- a/app/models/merge_request.rb
+++ b/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
diff --git a/app/models/spam_log.rb b/app/models/spam_log.rb
index 5b9ece8373f..2ec53f58e5f 100644
--- a/app/models/spam_log.rb
+++ b/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
diff --git a/app/services/concerns/spam_check_methods.rb b/app/services/concerns/spam_check_methods.rb
index 75d9759f1d1..5eb663c97ff 100644
--- a/app/services/concerns/spam_check_methods.rb
+++ b/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
diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb
index cc12aacaf02..a4771e864d4 100644
--- a/app/services/projects/import_service.rb
+++ b/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
diff --git a/app/services/spam_service.rb b/app/services/spam_check_service.rb
index 242c0ede57a..e1f5efabcaf 100644
--- a/app/services/spam_service.rb
+++ b/app/services/spam_check_service.rb
@@ -1,6 +1,6 @@
 # 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