Merge remote-tracking branch 'origin/master' into 38759-fetch-available-parameters-directly-from-gke-when-creating-a-cluster

13 files changed, 141 insertions, 49 deletions

diff --git a/app/services/boards/issues/create_service.rb b/app/services/boards/issues/create_service.rb
index 7c4a79f555e..3025029755c 100644
--- a/app/services/boards/issues/create_service.rb
+++ b/app/services/boards/issues/create_service.rb
@@ -10,11 +10,15 @@ module Boards
       end
 
       def execute
-        create_issue(params.merge(label_ids: [list.label_id]))
+        create_issue(params.merge(issue_params))
       end
 
       private
 
+      def issue_params
+        { label_ids: [list.label_id] }
+      end
+
       def board
         @board ||= parent.boards.find(params.delete(:board_id))
       end
diff --git a/app/services/clusters/applications/schedule_installation_service.rb b/app/services/clusters/applications/schedule_installation_service.rb
index eb8caa68ef7..9c5461e85e1 100644
--- a/app/services/clusters/applications/schedule_installation_service.rb
+++ b/app/services/clusters/applications/schedule_installation_service.rb
@@ -1,21 +1,10 @@
 module Clusters
   module Applications
     class ScheduleInstallationService < ::BaseService
-      def execute
-        application_class.find_or_create_by!(cluster: cluster).try do |application|
-          application.make_scheduled!
-          ClusterInstallAppWorker.perform_async(application.name, application.id)
-        end
-      end
-
-      private
-
-      def application_class
-        params[:application_class]
-      end
+      def execute(application)
+        application.make_scheduled!
 
-      def cluster
-        params[:cluster]
+        ClusterInstallAppWorker.perform_async(application.name, application.id)
       end
     end
   end
diff --git a/app/services/keys/base_service.rb b/app/services/keys/base_service.rb
index f78791932a7..df8e82f5f60 100644
--- a/app/services/keys/base_service.rb
+++ b/app/services/keys/base_service.rb
@@ -2,7 +2,7 @@ module Keys
   class BaseService
     attr_accessor :user, :params
 
-    def initialize(user, params)
+    def initialize(user, params = {})
       @user, @params = user, params
       @ip_address = @params.delete(:ip_address)
     end
diff --git a/app/services/keys/destroy_service.rb b/app/services/keys/destroy_service.rb
new file mode 100644
index 00000000000..785cfa3a1d8
--- /dev/null
+++ b/app/services/keys/destroy_service.rb
@@ -0,0 +1,12 @@
+module Keys
+  class DestroyService < ::Keys::BaseService
+    def execute(key)
+      key.destroy if destroy_possible?(key)
+    end
+
+    # overriden in EE::Keys::DestroyService
+    def destroy_possible?(key)
+      true
+    end
+  end
+end
diff --git a/app/services/lfs/unlock_file_service.rb b/app/services/lfs/unlock_file_service.rb
index 6c93dc69bb0..7eb89339a92 100644
--- a/app/services/lfs/unlock_file_service.rb
+++ b/app/services/lfs/unlock_file_service.rb
@@ -2,14 +2,14 @@ module Lfs
   class UnlockFileService < BaseService
     def execute
       unless can?(current_user, :push_code, project)
-        raise Gitlab::GitAccess::UnauthorizedError, 'You have no permissions'
+        raise Gitlab::GitAccess::UnauthorizedError, _('You have no permissions')
       end
 
       unlock_file
     rescue Gitlab::GitAccess::UnauthorizedError => ex
       error(ex.message, 403)
     rescue ActiveRecord::RecordNotFound
-      error('Lock not found', 404)
+      error(_('Lock not found'), 404)
     rescue => ex
       error(ex.message, 500)
     end
@@ -24,9 +24,9 @@ module Lfs
 
         success(lock: lock, http_status: :ok)
       elsif forced
-        error('You must have master access to force delete a lock', 403)
+        error(_('You must have master access to force delete a lock'), 403)
       else
-        error("#{lock.path} is locked by GitLab User #{lock.user_id}", 403)
+        error(_("%{lock_path} is locked by GitLab User %{lock_user_id}") % { lock_path: lock.path, lock_user_id: lock.user_id }, 403)
       end
     end
 
diff --git a/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb b/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb
index 850deb0ac7a..9a4e6eb2e88 100644
--- a/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb
+++ b/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb
@@ -24,11 +24,7 @@ module MergeRequests
 
       pipeline_merge_requests(pipeline) do |merge_request|
         next unless merge_request.merge_when_pipeline_succeeds?
-
-        unless merge_request.mergeable?
-          todo_service.merge_request_became_unmergeable(merge_request)
-          next
-        end
+        next unless merge_request.mergeable?
 
         merge_request.merge_async(merge_request.merge_user_id, merge_request.merge_params)
       end
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index 1fb1796b56c..0127d781686 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -3,6 +3,12 @@ module MergeRequests
     def execute(oldrev, newrev, ref)
       return true unless Gitlab::Git.branch_ref?(ref)
 
+      do_execute(oldrev, newrev, ref)
+    end
+
+    private
+
+    def do_execute(oldrev, newrev, ref)
       @oldrev, @newrev = oldrev, newrev
       @branch_name = Gitlab::Git.ref_name(ref)
 
@@ -28,8 +34,6 @@ module MergeRequests
       true
     end
 
-    private
-
     def close_upon_missing_source_branch_ref
       # MergeRequest#reload_diff ignores not opened MRs. This means it won't
       # create an `empty` diff for `closed` MRs without a source branch, keeping
diff --git a/app/services/milestones/base_service.rb b/app/services/milestones/base_service.rb
index 4963601ea8b..cce0863d611 100644
--- a/app/services/milestones/base_service.rb
+++ b/app/services/milestones/base_service.rb
@@ -5,6 +5,7 @@ module Milestones
 
     def initialize(parent, user, params = {})
       @parent, @current_user, @params = parent, user, params.dup
+      super
     end
   end
 end
diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb
index 5658699664d..4fa38665abc 100644
--- a/app/services/notification_recipient_service.rb
+++ b/app/services/notification_recipient_service.rb
@@ -18,6 +18,10 @@ module NotificationRecipientService
     Builder::NewNote.new(*a).notification_recipients
   end
 
+  def self.build_merge_request_unmergeable_recipients(*a)
+    Builder::MergeRequestUnmergeable.new(*a).notification_recipients
+  end
+
   module Builder
     class Base
       def initialize(*)
@@ -330,5 +334,26 @@ module NotificationRecipientService
         note.author
       end
     end
+
+    class MergeRequestUnmergeable < Base
+      attr_reader :target
+      def initialize(merge_request)
+        @target = merge_request
+      end
+
+      def build!
+        target.merge_participants.each do |user|
+          add_recipients(user, :participating, nil)
+        end
+      end
+
+      def custom_action
+        :unmergeable_merge_request
+      end
+
+      def acting_user
+        nil
+      end
+    end
   end
 end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 55a1735e54b..636cfbf5b45 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -129,6 +129,7 @@ class NotificationService
 
   # When create a merge request we should send an email to:
   #
+  #  * mr author
   #  * mr assignee if their notification level is not Disabled
   #  * project team members with notification level higher then Participating
   #  * watchers of the mr's labels
@@ -148,6 +149,15 @@ class NotificationService
     end
   end
 
+  # When a merge request is found to be unmergeable, we should send an email to:
+  #
+  #  * mr author
+  #  * mr merge user if set
+  #
+  def merge_request_unmergeable(merge_request)
+    merge_request_unmergeable_email(merge_request)
+  end
+
   # When merge request text is updated, we should send an email to:
   #
   #  * newly mentioned project team members with notification level higher than Participating
@@ -484,6 +494,14 @@ class NotificationService
     end
   end
 
+  def merge_request_unmergeable_email(merge_request)
+    recipients = NotificationRecipientService.build_merge_request_unmergeable_recipients(merge_request)
+
+    recipients.each do |recipient|
+      mailer.merge_request_unmergeable_email(recipient.user.id, merge_request.id).deliver_later
+    end
+  end
+
   def mailer
     Notify
   end
diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index adbc498d0bf..077d27c5836 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -51,11 +51,11 @@ module Projects
 
       flush_caches(@project)
 
-      unless mv_repository(removal_path(repo_path), repo_path)
+      unless rollback_repository(removal_path(repo_path), repo_path)
         raise_error('Failed to restore project repository. Please contact the administrator.')
       end
 
-      unless mv_repository(removal_path(wiki_path), wiki_path)
+      unless rollback_repository(removal_path(wiki_path), wiki_path)
         raise_error('Failed to restore wiki repository. Please contact the administrator.')
       end
     end
@@ -84,6 +84,9 @@ module Projects
       # Skip repository removal. We use this flag when remove user or group
       return true if params[:skip_repo] == true
 
+      # There is a possibility project does not have repository or wiki
+      return true unless repo_exists?(path)
+
       new_path = removal_path(path)
 
       if mv_repository(path, new_path)
@@ -98,8 +101,18 @@ module Projects
       end
     end
 
-    def mv_repository(from_path, to_path)
+    def rollback_repository(old_path, new_path)
       # There is a possibility project does not have repository or wiki
+      return true unless repo_exists?(old_path)
+
+      mv_repository(old_path, new_path)
+    end
+
+    def repo_exists?(path)
+      gitlab_shell.exists?(project.repository_storage, path + '.git')
+    end
+
+    def mv_repository(from_path, to_path)
       return true unless gitlab_shell.exists?(project.repository_storage, from_path + '.git')
 
       gitlab_shell.mv_repository(project.repository_storage, from_path, to_path)
diff --git a/app/services/projects/open_issues_count_service.rb b/app/services/projects/open_issues_count_service.rb
index a975a06a05c..0a004677417 100644
--- a/app/services/projects/open_issues_count_service.rb
+++ b/app/services/projects/open_issues_count_service.rb
@@ -2,14 +2,42 @@ module Projects
   # Service class for counting and caching the number of open issues of a
   # project.
   class OpenIssuesCountService < Projects::CountService
+    include Gitlab::Utils::StrongMemoize
+
+    def initialize(project, user = nil)
+      @user = user
+
+      super(project)
+    end
+
     def cache_key_name
-      'open_issues_count'
+      public_only? ? 'public_open_issues_count' : 'total_open_issues_count'
+    end
+
+    def public_only?
+      !user_is_at_least_reporter?
+    end
+
+    def relation_for_count
+      self.class.query(@project, public_only: public_only?)
+    end
+
+    def user_is_at_least_reporter?
+      strong_memoize(:user_is_at_least_reporter) do
+        @user && @project.team.member?(@user, Gitlab::Access::REPORTER)
+      end
     end
 
-    def self.query(project_ids)
-      # We don't include confidential issues in this number since this would
-      # expose the number of confidential issues to non project members.
-      Issue.opened.public_only.where(project: project_ids)
+    # We only show total issues count for reporters
+    # which are allowed to view confidential issues
+    # This will still show a discrepancy on issues number but should be less than before.
+    # Check https://gitlab.com/gitlab-org/gitlab-ce/issues/38418 description.
+    def self.query(projects, public_only: true)
+      if public_only
+        Issue.opened.public_only.where(project: projects)
+      else
+        Issue.opened.where(project: projects)
+      end
     end
   end
 end
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb
index ffd48e842c2..f91cd03bf5c 100644
--- a/app/services/todo_service.rb
+++ b/app/services/todo_service.rb
@@ -98,12 +98,12 @@ class TodoService
 
   # When a build fails on the HEAD of a merge request we should:
   #
-  #  * create a todo for author of MR to fix it
-  #  * create a todo for merge_user to keep an eye on it
+  #  * create a todo for each merge participant
   #
   def merge_request_build_failed(merge_request)
-    create_build_failed_todo(merge_request, merge_request.author)
-    create_build_failed_todo(merge_request, merge_request.merge_user) if merge_request.merge_when_pipeline_succeeds?
+    merge_request.merge_participants.each do |user|
+      create_build_failed_todo(merge_request, user)
+    end
   end
 
   # When a new commit is pushed to a merge request we should:
@@ -116,20 +116,22 @@ class TodoService
 
   # When a build is retried to a merge request we should:
   #
-  #  * mark all pending todos related to the merge request for the author as done
-  #  * mark all pending todos related to the merge request for the merge_user as done
+  #  * mark all pending todos related to the merge request as done for each merge participant
   #
   def merge_request_build_retried(merge_request)
-    mark_pending_todos_as_done(merge_request, merge_request.author)
-    mark_pending_todos_as_done(merge_request, merge_request.merge_user) if merge_request.merge_when_pipeline_succeeds?
+    merge_request.merge_participants.each do |user|
+      mark_pending_todos_as_done(merge_request, user)
+    end
   end
 
-  # When a merge request could not be automatically merged due to its unmergeable state we should:
+  # When a merge request could not be merged due to its unmergeable state we should:
   #
-  #  * create a todo for a merge_user
+  #  * create a todo for each merge participant
   #
   def merge_request_became_unmergeable(merge_request)
-    create_unmergeable_todo(merge_request, merge_request.merge_user) if merge_request.merge_when_pipeline_succeeds?
+    merge_request.merge_participants.each do |user|
+      create_unmergeable_todo(merge_request, user)
+    end
   end
 
   # When create a note we should:
@@ -275,9 +277,9 @@ class TodoService
     create_todos(todo_author, attributes)
   end
 
-  def create_unmergeable_todo(merge_request, merge_user)
-    attributes = attributes_for_todo(merge_request.project, merge_request, merge_user, Todo::UNMERGEABLE)
-    create_todos(merge_user, attributes)
+  def create_unmergeable_todo(merge_request, todo_author)
+    attributes = attributes_for_todo(merge_request.project, merge_request, todo_author, Todo::UNMERGEABLE)
+    create_todos(todo_author, attributes)
   end
 
   def attributes_for_target(target)