Merge branch 'reduce-diff-with-ee-in-app-services' into 'master'

Reduce remaining diff with EE in app/services

See merge request gitlab-org/gitlab-ce!24968

app/services/applications/create_service.rb

@@ -2,16 +2,16 @@
 
 module Applications
   class CreateService
-    # rubocop: disable CodeReuse/ActiveRecord
+    attr_reader :current_user, :params
+
     def initialize(current_user, params)
       @current_user = current_user
-      @params = params.except(:ip_address)
+      @params = params.except(:ip_address) # rubocop: disable CodeReuse/ActiveRecord
     end
-    # rubocop: enable CodeReuse/ActiveRecord
 
     # EE would override and use `request` arg
     def execute(request)
-      Doorkeeper::Application.create(@params)
+      Doorkeeper::Application.create(params)
     end
   end
 end

app/services/create_branch_service.rb

 # frozen_string_literal: true
 
 class CreateBranchService < BaseService
-  def execute(branch_name, ref)
-    create_master_branch if project.empty_repo?
+  def execute(branch_name, ref, create_master_if_empty: true)
+    create_master_branch if create_master_if_empty && project.empty_repo?
 
     result = ValidateNewBranchService.new(project, current_user)
       .execute(branch_name)

app/services/emails/base_service.rb

@@ -2,10 +2,11 @@
 
 module Emails
   class BaseService
-    attr_reader :current_user
+    attr_reader :current_user, :params, :user
 
     def initialize(current_user, params = {})
-      @current_user, @params = current_user, params.dup
+      @current_user = current_user
+      @params = params.dup
       @user = params.delete(:user)
     end
   end

app/services/emails/create_service.rb

@@ -3,12 +3,11 @@
 module Emails
   class CreateService < ::Emails::BaseService
     def execute(extra_params = {})
-      skip_confirmation = @params.delete(:skip_confirmation)
-
-      email = @user.emails.create(@params.merge(extra_params))
+      skip_confirmation = params.delete(:skip_confirmation)
 
+      user.emails.create(params.merge(extra_params)).tap do |email|
         email&.confirm if skip_confirmation && current_user.admin?
-      email
+      end
     end
   end
 end

app/services/git_push_service.rb

@@ -140,7 +140,7 @@ class GitPushService < BaseService
       .perform_async(project.id, current_user.id, params[:oldrev], params[:newrev], params[:ref])
 
     EventCreateService.new.push(project, current_user, build_push_data)
-    Ci::CreatePipelineService.new(project, current_user, build_push_data).execute(:push)
+    Ci::CreatePipelineService.new(project, current_user, build_push_data).execute(:push, pipeline_options)
 
     project.execute_hooks(build_push_data.dup, :push_hooks)
     project.execute_services(build_push_data.dup, :push_hooks)
@@ -231,4 +231,10 @@ class GitPushService < BaseService
   def last_pushed_commits
     @last_pushed_commits ||= @push_commits.last(PROCESS_COMMIT_LIMIT)
   end
+
+  private
+
+  def pipeline_options
+    {} # to be overriden in EE
+  end
 end

app/services/git_tag_push_service.rb

@@ -10,7 +10,7 @@ class GitTagPushService < BaseService
     @push_data = build_push_data
 
     EventCreateService.new.push(project, current_user, push_data)
-    Ci::CreatePipelineService.new(project, current_user, push_data).execute(:push)
+    Ci::CreatePipelineService.new(project, current_user, push_data).execute(:push, pipeline_options)
 
     SystemHooksService.new.execute_hooks(build_system_push_data, :tag_push_hooks)
     project.execute_hooks(push_data.dup, :tag_push_hooks)
@@ -59,4 +59,8 @@ class GitTagPushService < BaseService
       [],
       '')
   end
+
+  def pipeline_options
+    {} # to be overriden in EE
+  end
 end

app/services/issues/build_service.rb

@@ -57,9 +57,11 @@ module Issues
     end
 
     def issue_params
-      @issue_params ||= issue_params_with_info_from_discussions.merge(whitelisted_issue_params)
+      @issue_params ||= build_issue_params
     end
 
+    private
+
     def whitelisted_issue_params
       if can?(current_user, :admin_issue, project)
         params.slice(:title, :description, :milestone_id)
@@ -67,5 +69,9 @@ module Issues
         params.slice(:title, :description)
       end
     end
+
+    def build_issue_params
+      issue_params_with_info_from_discussions.merge(whitelisted_issue_params)
+    end
   end
 end

app/services/notes/quick_actions_service.rb

@@ -7,9 +7,14 @@ module Notes
       'MergeRequest' => MergeRequests::UpdateService,
       'Commit' => Commits::TagService
     }.freeze
+    private_constant :UPDATE_SERVICES
+
+    def self.update_services
+      UPDATE_SERVICES
+    end
 
     def self.noteable_update_service(note)
-      UPDATE_SERVICES[note.noteable_type]
+      update_services[note.noteable_type]
     end
 
     def self.supported?(note)

app/services/notification_recipient_service.rb

@@ -249,6 +249,7 @@ module NotificationRecipientService
       attr_reader :action
       attr_reader :previous_assignee
       attr_reader :skip_current_user
+
       def initialize(target, current_user, action:, custom_action: nil, previous_assignee: nil, skip_current_user: true)
         @target = target
         @current_user = current_user
@@ -258,9 +259,13 @@ module NotificationRecipientService
         @skip_current_user = skip_current_user
       end
 
+      def add_watchers
+        add_project_watchers
+      end
+
       def build!
         add_participants(current_user)
-        add_project_watchers
+        add_watchers
         add_custom_notifications
 
         # Re-assign is considered as a mention of the new assignee

app/services/protected_branches/api_service.rb

@@ -3,16 +3,15 @@
 module ProtectedBranches
   class ApiService < BaseService
     def create
-      @push_params = AccessLevelParams.new(:push, params)
-      @merge_params = AccessLevelParams.new(:merge, params)
+      ::ProtectedBranches::CreateService.new(@project, @current_user, protected_branch_params).execute
+    end
 
-      protected_branch_params = {
+    def protected_branch_params
+      {
         name: params[:name],
-        push_access_levels_attributes: @push_params.access_levels,
-        merge_access_levels_attributes: @merge_params.access_levels
+        push_access_levels_attributes: AccessLevelParams.new(:push, params).access_levels,
+        merge_access_levels_attributes: AccessLevelParams.new(:merge, params).access_levels
       }
-
-      ::ProtectedBranches::CreateService.new(@project, @current_user, protected_branch_params).execute
     end
   end
 end

app/services/protected_branches/legacy_api_update_service.rb

@@ -6,30 +6,31 @@
 # lives in this service.
 module ProtectedBranches
   class LegacyApiUpdateService < BaseService
+    attr_reader :protected_branch, :developers_can_push, :developers_can_merge
+
     def execute(protected_branch)
+      @protected_branch = protected_branch
       @developers_can_push = params.delete(:developers_can_push)
       @developers_can_merge = params.delete(:developers_can_merge)
 
-      @protected_branch = protected_branch
-
       protected_branch.transaction do
         delete_redundant_access_levels
 
-        case @developers_can_push
+        case developers_can_push
         when true
           params[:push_access_levels_attributes] = [{ access_level: Gitlab::Access::DEVELOPER }]
         when false
           params[:push_access_levels_attributes] = [{ access_level: Gitlab::Access::MAINTAINER }]
         end
 
-        case @developers_can_merge
+        case developers_can_merge
         when true
           params[:merge_access_levels_attributes] = [{ access_level: Gitlab::Access::DEVELOPER }]
         when false
           params[:merge_access_levels_attributes] = [{ access_level: Gitlab::Access::MAINTAINER }]
         end
 
-        service = ProtectedBranches::UpdateService.new(@project, @current_user, @params)
+        service = ProtectedBranches::UpdateService.new(project, current_user, params)
         service.execute(protected_branch)
       end
     end
@@ -37,12 +38,12 @@ module ProtectedBranches
     private
 
     def delete_redundant_access_levels
-      unless @developers_can_merge.nil?
-        @protected_branch.merge_access_levels.destroy_all # rubocop: disable DestroyAll
+      unless developers_can_merge.nil?
+        protected_branch.merge_access_levels.destroy_all # rubocop: disable DestroyAll
       end
 
-      unless @developers_can_push.nil?
-        @protected_branch.push_access_levels.destroy_all # rubocop: disable DestroyAll
+      unless developers_can_push.nil?
+        protected_branch.push_access_levels.destroy_all # rubocop: disable DestroyAll
       end
     end
   end

lib/tasks/dev.rake

@@ -10,6 +10,7 @@ namespace :dev do
 
   desc "GitLab | Eager load application"
   task load: :environment do
+    Rails.configuration.eager_load = true
     Rails.application.eager_load!
   end
 end