Refactor CreateBranchService into Branches::CreateService

Renamed CreateBranchService into Branches::CreateService and renamed
its usages.

Fixed related tests with new lines to make separation between the
4 stages of the test clear.

Some extra codestyle changes.

app/controllers/projects/branches_controller.rb

@@ -46,7 +46,7 @@ class Projects::BranchesController < Projects::ApplicationController
   def diverging_commit_counts
     respond_to do |format|
       format.json do
-        service = Branches::DivergingCommitCountsService.new(repository)
+        service = ::Branches::DivergingCommitCountsService.new(repository)
         branches = BranchesFinder.new(repository, params.permit(names: [])).execute
 
         Gitlab::GitalyClient.allow_n_plus_1_calls do
@@ -63,7 +63,7 @@ class Projects::BranchesController < Projects::ApplicationController
 
     redirect_to_autodeploy = project.empty_repo? && project.deployment_platform.present?
 
-    result = CreateBranchService.new(project, current_user)
+    result = ::Branches::CreateService.new(project, current_user)
         .execute(branch_name, ref)
 
     success = (result[:status] == :success)

app/services/branches/create_service.rb

+# frozen_string_literal: true
+
+module Branches
+  class CreateService < BaseService
+    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)
+
+      return result if result[:status] == :error
+
+      new_branch = repository.add_branch(current_user, branch_name, ref)
+
+      if new_branch
+        success(new_branch)
+      else
+        error("Invalid reference name: #{branch_name}")
+      end
+    rescue Gitlab::Git::PreReceiveError => ex
+      error(ex.message)
+    end
+
+    def success(branch)
+      super().merge(branch: branch)
+    end
+
+    private
+
+    def create_master_branch
+      project.repository.create_file(
+        current_user,
+        '/README.md',
+        '',
+        message: 'Add README.md',
+        branch_name: 'master'
+      )
+    end
+  end
+end

app/services/commits/commit_patch_service.rb

@@ -32,7 +32,7 @@ module Commits
     end
 
     def prepare_branch!
-      branch_result = CreateBranchService.new(project, current_user)
+      branch_result = ::Branches::CreateService.new(project, current_user)
                         .execute(@branch_name, @start_branch)
 
       if branch_result[:status] != :success

app/services/create_branch_service.rb

-# frozen_string_literal: true
-
-class CreateBranchService < BaseService
-  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)
-
-    return result if result[:status] == :error
-
-    new_branch = repository.add_branch(current_user, branch_name, ref)
-
-    if new_branch
-      success(new_branch)
-    else
-      error("Invalid reference name: #{branch_name}")
-    end
-  rescue Gitlab::Git::PreReceiveError => ex
-    error(ex.message)
-  end
-
-  def success(branch)
-    super().merge(branch: branch)
-  end
-
-  private
-
-  def create_master_branch
-    project.repository.create_file(
-      current_user,
-      '/README.md',
-      '',
-      message: 'Add README.md',
-      branch_name: 'master'
-    )
-  end
-end

app/services/merge_requests/create_from_issue_service.rb

@@ -19,7 +19,7 @@ module MergeRequests
       return error('Not allowed to create merge request') unless can_create_merge_request?
       return error('Invalid issue iid') unless @issue_iid.present? && issue.present?
 
-      result = CreateBranchService.new(target_project, current_user).execute(branch_name, ref)
+      result = ::Branches::CreateService.new(target_project, current_user).execute(branch_name, ref)
       return result if result[:status] == :error
 
       new_merge_request = create(merge_request)

ee/app/services/projects/update_mirror_service.rb

@@ -51,7 +51,7 @@ module Projects
         local_branch = local_branches[name]
 
         if local_branch.nil?
-          result = CreateBranchService.new(project, current_user).execute(name, upstream_branch.dereferenced_target.sha, create_master_if_empty: false)
+          result = ::Branches::CreateService.new(project, current_user).execute(name, upstream_branch.dereferenced_target.sha, create_master_if_empty: false)
           if result[:status] == :error
             errors << result[:message]
           end

ee/spec/services/projects/update_mirror_service_spec.rb

@@ -269,7 +269,7 @@ describe Projects::UpdateMirrorService do
             repository = project.repository
 
             allow(project).to receive(:fetch_mirror) { create_file(repository) }
-            expect(CreateBranchService).not_to receive(:create_master_branch)
+            expect(::Branches::CreateService).not_to receive(:create_master_branch)
 
             service.execute
 

lib/api/branches.rb

@@ -137,7 +137,7 @@ module API
       post ':id/repository/branches' do
         authorize_push_project
 
-        result = CreateBranchService.new(user_project, current_user)
+        result = ::Branches::CreateService.new(user_project, current_user)
                  .execute(params[:branch], params[:ref])
 
         if result[:status] == :success

spec/controllers/projects/branches_controller_spec.rb

@@ -178,7 +178,7 @@ describe Projects::BranchesController do
         it 'redirects to newly created branch' do
           result = { status: :success, branch: double(name: branch) }
 
-          expect_any_instance_of(CreateBranchService).to receive(:execute).and_return(result)
+          expect_any_instance_of(::Branches::CreateService).to receive(:execute).and_return(result)
           expect(SystemNoteService).to receive(:new_issue_branch).and_return(true)
 
           post :create,
@@ -200,7 +200,7 @@ describe Projects::BranchesController do
           it 'redirects to autodeploy setup page' do
             result = { status: :success, branch: double(name: branch) }
 
-            expect_any_instance_of(CreateBranchService).to receive(:execute).and_return(result)
+            expect_any_instance_of(::Branches::CreateService).to receive(:execute).and_return(result)
             expect(SystemNoteService).to receive(:new_issue_branch).and_return(true)
 
             post :create,
@@ -221,7 +221,7 @@ describe Projects::BranchesController do
 
           create(:cluster, :provided_by_gcp, projects: [project])
 
-          expect_any_instance_of(CreateBranchService).to receive(:execute).and_return(result)
+          expect_any_instance_of(::Branches::CreateService).to receive(:execute).and_return(result)
           expect(SystemNoteService).to receive(:new_issue_branch).and_return(true)
 
           post :create,

spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb

@@ -178,10 +178,11 @@ describe 'Merge request > User selects branches for new MR', :js do
   end
 
   context 'with special characters in branch names' do
+    let(:create_branch_service) { ::Branches::CreateService.new(project, user) }
+
     it 'escapes quotes in branch names' do
       special_branch_name = '"with-quotes"'
-      CreateBranchService.new(project, user)
-        .execute(special_branch_name, 'add-pdf-file')
+      create_branch_service.execute(special_branch_name, 'add-pdf-file')
 
       visit project_new_merge_request_path(project)
       select_source_branch(special_branch_name)
@@ -192,8 +193,7 @@ describe 'Merge request > User selects branches for new MR', :js do
 
     it 'does not escape unicode in branch names' do
       special_branch_name = 'ʕ•ᴥ•ʔ'
-      CreateBranchService.new(project, user)
-        .execute(special_branch_name, 'add-pdf-file')
+      create_branch_service.execute(special_branch_name, 'add-pdf-file')
 
       visit project_new_merge_request_path(project)
       select_source_branch(special_branch_name)

spec/requests/api/branches_spec.rb

@@ -131,7 +131,7 @@ describe API::Branches do
         end
 
         new_branch_name = 'protected-branch'
-        CreateBranchService.new(project, current_user).execute(new_branch_name, 'master')
+        ::Branches::CreateService.new(project, current_user).execute(new_branch_name, 'master')
         create(:protected_branch, name: new_branch_name, project: project)
 
         expect do

spec/services/create_branch_service_spec.rb → spec/services/branches/create_service_spec.rb

@@ -2,9 +2,9 @@
 
 require 'spec_helper'
 
-describe CreateBranchService do
+describe Branches::CreateService do
   let(:user) { create(:user) }
-  let(:service) { described_class.new(project, user) }
+  subject(:service) { described_class.new(project, user) }
 
   describe '#execute' do
     context 'when repository is empty' do
@@ -30,7 +30,7 @@ describe CreateBranchService do
         allow(project.repository).to receive(:add_branch).and_return(false)
       end
 
-      it 'retruns an error with the branch name' do
+      it 'returns an error with the branch name' do
         result = service.execute('my-feature', 'master')
 
         expect(result[:status]).to eq(:error)

spec/support/helpers/position_tracer_helpers.rb

@@ -50,7 +50,7 @@ module PositionTracerHelpers
   end
 
   def create_branch(new_name, branch_name)
-    CreateBranchService.new(project, current_user).execute(new_name, branch_name)
+    ::Branches::CreateService.new(project, current_user).execute(new_name, branch_name)
   end
 
   def create_file(branch_name, file_name, content)

spec/support/shared_examples/features/creatable_merge_request_shared_examples.rb

@@ -8,11 +8,14 @@ RSpec.shared_examples 'a creatable merge request' do
     page.within '.dropdown-menu-user' do
       click_link user2.name
     end
+
     expect(find('input[name="merge_request[assignee_ids][]"]', visible: false).value).to match(user2.id.to_s)
     page.within '.js-assignee-search' do
       expect(page).to have_content user2.name
     end
+
     click_link 'Assign to me'
+
     expect(find('input[name="merge_request[assignee_ids][]"]', visible: false).value).to match(user.id.to_s)
     page.within '.js-assignee-search' do
       expect(page).to have_content user.name
@@ -22,6 +25,7 @@ RSpec.shared_examples 'a creatable merge request' do
     page.within '.issue-milestone' do
       click_link milestone.title
     end
+
     expect(find('input[name="merge_request[milestone_id]"]', visible: false).value).to match(milestone.id.to_s)
     page.within '.js-milestone-select' do
       expect(page).to have_content milestone.title
@@ -32,6 +36,7 @@ RSpec.shared_examples 'a creatable merge request' do
       click_link label.title
       click_link label2.title
     end
+
     page.within '.js-label-select' do
       expect(page).to have_content label.title
     end
@@ -58,8 +63,9 @@ RSpec.shared_examples 'a creatable merge request' do
 
   it 'updates the branches when selecting a new target project', :js do
     target_project_member = target_project.owner
-    CreateBranchService.new(target_project, target_project_member)
-    .execute('a-brand-new-branch-to-test', 'master')
+    ::Branches::CreateService.new(target_project, target_project_member)
+      .execute('a-brand-new-branch-to-test', 'master')
+
     visit project_new_merge_request_path(source_project)
 
     first('.js-target-project').click