Merge branch 'master' into feature/multi-level-container-registry-images

* master: (57 commits)
  Ensure we generate unique usernames otherwise validations fail
  Fix a Knapsack issue that would load support/capybara.rb before support/env.rb
  Ensure users have a short username otherwise a click event is triggered ...
  Enable the `bullet_logger` setting; enable `raise` in test environment
  Fix Rubocop offenses
  Set the right timeout for Gitlab::Shell#fetch_remote
  Refactoring Projects::ImportService
  Move methods that are not related to mirroring to the repository model
  Fix GitHub pull request formatter spec
  Rename skip_metrics to imported on the importable concern
  Add CHANGELOG
  Remove unused include from RepositoryImportWorker
  Skip MR metrics when importing projects from GitHub
  Fetch GitHub project as a mirror to get all refs at once
  Make file templates easy to use and discover
  Ensure user has a unique username otherwise `user10` would match `user1`
  Ensure the AbuseReport fixtures create unique reported users
  Don't use FFaker in factories, use sequences instead
  Fix brittle specs
  Fix the AbuseReport seeder
  ...

Conflicts:
	db/schema.rb

8 files changed, 70 insertions, 33 deletions

diff --git a/app/services/base_service.rb b/app/services/base_service.rb
index 745c2c4b681..a0cb00dba58 100644
--- a/app/services/base_service.rb
+++ b/app/services/base_service.rb
@@ -24,6 +24,10 @@ class BaseService
     Gitlab::AppLogger.info message
   end
 
+  def log_error(message)
+    Gitlab::AppLogger.error message
+  end
+
   def system_hook_service
     SystemHooksService.new
   end
diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb
index 2935d00c075..33edcd60944 100644
--- a/app/services/ci/process_pipeline_service.rb
+++ b/app/services/ci/process_pipeline_service.rb
@@ -5,8 +5,6 @@ module Ci
     def execute(pipeline)
       @pipeline = pipeline
 
-      ensure_created_builds! # TODO, remove me in 9.0
-
       new_builds =
         stage_indexes_of_created_builds.map do |index|
           process_stage(index)
@@ -73,18 +71,5 @@ module Ci
     def created_builds
       pipeline.builds.created
     end
-
-    # This method is DEPRECATED and should be removed in 9.0.
-    #
-    # We need it to maintain backwards compatibility with  previous versions
-    # when builds were not created within one transaction with the pipeline.
-    #
-    def ensure_created_builds!
-      return if created_builds.any?
-
-      Ci::CreatePipelineBuildsService
-        .new(project, current_user)
-        .execute(pipeline)
-    end
   end
 end
diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb
index 940e850600f..8bb995158de 100644
--- a/app/services/notification_recipient_service.rb
+++ b/app/services/notification_recipient_service.rb
@@ -3,7 +3,7 @@
 #
 class NotificationRecipientService
   attr_reader :project
-  
+
   def initialize(project)
     @project = project
   end
@@ -12,11 +12,7 @@ class NotificationRecipientService
     custom_action = build_custom_key(action, target)
 
     recipients = target.participants(current_user)
-
-    unless NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(custom_action)
-      recipients = add_project_watchers(recipients)
-    end
-
+    recipients = add_project_watchers(recipients)
     recipients = add_custom_notifications(recipients, custom_action)
     recipients = reject_mention_users(recipients)
 
@@ -43,6 +39,28 @@ class NotificationRecipientService
     recipients.uniq
   end
 
+  def build_pipeline_recipients(target, current_user, action:)
+    return [] unless current_user
+
+    custom_action =
+      case action.to_s
+      when 'failed'
+        :failed_pipeline
+      when 'success'
+        :success_pipeline
+      end
+
+    notification_setting = notification_setting_for_user_project(current_user, target.project)
+
+    return [] if notification_setting.mention? || notification_setting.disabled?
+
+    return [] if notification_setting.custom? && !notification_setting.public_send(custom_action)
+
+    return [] if (notification_setting.watch? || notification_setting.participating?) && NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(custom_action)
+
+    reject_users_without_access([current_user], target)
+  end
+
   def build_relabeled_recipients(target, current_user, labels:)
     recipients = add_labels_subscribers([], target, labels: labels)
     recipients = reject_unsubscribed_users(recipients, target)
@@ -290,4 +308,16 @@ class NotificationRecipientService
   def build_custom_key(action, object)
     "#{action}_#{object.class.model_name.name.underscore}".to_sym
   end
+
+  def notification_setting_for_user_project(user, project)
+    project_setting = user.notification_settings_for(project)
+
+    return project_setting unless project_setting.global?
+
+    group_setting = user.notification_settings_for(project.group)
+
+    return group_setting unless group_setting.global?
+
+    user.global_notification_setting
+  end
 end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 2c6f849259e..6b186263bd1 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -278,11 +278,11 @@ class NotificationService
 
     return unless mailer.respond_to?(email_template)
 
-    recipients ||= NotificationRecipientService.new(pipeline.project).build_recipients(
+    recipients ||= NotificationRecipientService.new(pipeline.project).build_pipeline_recipients(
       pipeline,
       pipeline.user,
       action: pipeline.status,
-      skip_current_user: false).map(&:notification_email)
+    ).map(&:notification_email)
 
     if recipients.any?
       mailer.public_send(email_template, pipeline, recipients).deliver_later
diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb
index d484a96f785..4c72d5e117d 100644
--- a/app/services/projects/import_service.rb
+++ b/app/services/projects/import_service.rb
@@ -11,7 +11,7 @@ module Projects
 
       success
     rescue => e
-      error(e.message)
+      error("Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}")
     end
 
     private
@@ -32,23 +32,40 @@ module Projects
     end
 
     def import_repository
+      raise Error, 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url)
+
       begin
-        raise Error, "Blocked import URL." if Gitlab::UrlBlocker.blocked_url?(project.import_url)
-        gitlab_shell.import_repository(project.repository_storage_path, project.path_with_namespace, project.import_url)
-      rescue => e
+        if project.github_import? || project.gitea_import?
+          fetch_repository
+        else
+          clone_repository
+        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.before_import if project.repository_exists?
+        project.repository.expire_content_cache if project.repository_exists?
 
-        raise Error, "Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}"
+        raise Error, e.message
       end
     end
 
+    def clone_repository
+      gitlab_shell.import_repository(project.repository_storage_path, project.path_with_namespace, project.import_url)
+    end
+
+    def fetch_repository
+      project.create_repository
+      project.repository.add_remote(project.import_type, project.import_url)
+      project.repository.set_remote_as_mirror(project.import_type)
+      project.repository.fetch_remote(project.import_type, forced: true)
+      project.repository.remove_remote(project.import_type)
+    end
+
     def import_data
       return unless has_importer?
 
-      project.repository.before_import unless project.gitlab_project_import?
+      project.repository.expire_content_cache unless project.gitlab_project_import?
 
       unless importer.execute
         raise Error, 'The remote data could not be imported.'
diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb
index 523b9f41916..17cf71cf098 100644
--- a/app/services/projects/update_pages_service.rb
+++ b/app/services/projects/update_pages_service.rb
@@ -46,6 +46,7 @@ module Projects
     end
 
     def error(message, http_status = nil)
+      log_error("Projects::UpdatePagesService: #{message}")
       @status.allow_failure = !latest?
       @status.description = message
       @status.drop
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb
index 2c56cb4c680..b6e88b0280f 100644
--- a/app/services/todo_service.rb
+++ b/app/services/todo_service.rb
@@ -204,7 +204,7 @@ class TodoService
     # Only update those that are not really on that state
     todos = todos.where.not(state: state)
     todos_ids = todos.pluck(:id)
-    todos.update_all(state: state)
+    todos.unscope(:order).update_all(state: state)
     current_user.update_todos_count_cache
     todos_ids
   end
diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb
index 833da5bc5d1..a3b32a71a64 100644
--- a/app/services/users/destroy_service.rb
+++ b/app/services/users/destroy_service.rb
@@ -20,10 +20,10 @@ module Users
         Groups::DestroyService.new(group, current_user).execute
       end
 
-      user.personal_projects.each do |project|
+      user.personal_projects.with_deleted.each do |project|
         # Skip repository removal because we remove directory with namespace
         # that contain all this repositories
-        ::Projects::DestroyService.new(project, current_user, skip_repo: true).async_execute
+        ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute
       end
 
       move_issues_to_ghost_user(user)