Refactor DeleteBranchService into Branches::DeleteService

Renamed DeleteBranchService into Branches::DeleteService 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

@@ -102,7 +102,7 @@ class Projects::BranchesController < Projects::ApplicationController
 
   def destroy
     @branch_name = Addressable::URI.unescape(params[:id])
-    result = DeleteBranchService.new(project, current_user).execute(@branch_name)
+    result = ::Branches::DeleteService.new(project, current_user).execute(@branch_name)
 
     respond_to do |format|
       format.html do

app/services/branches/delete_service.rb

+# frozen_string_literal: true
+
+module Branches
+  class DeleteService < BaseService
+    def execute(branch_name)
+      repository = project.repository
+      branch = repository.find_branch(branch_name)
+
+      unless current_user.can?(:push_code, project)
+        return ServiceResponse.error(
+          message: 'You dont have push access to repo',
+          http_status: 405)
+      end
+
+      unless branch
+        return ServiceResponse.error(
+          message: 'No such branch',
+          http_status: 404)
+      end
+
+      if repository.rm_branch(current_user, branch_name)
+        ServiceResponse.success(message: 'Branch was deleted')
+      else
+        ServiceResponse.error(
+          message: 'Failed to remove branch',
+          http_status: 400)
+      end
+    rescue Gitlab::Git::PreReceiveError => ex
+      ServiceResponse.error(message: ex.message, http_status: 400)
+    end
+  end
+end

app/services/delete_branch_service.rb

-# frozen_string_literal: true
-
-class DeleteBranchService < BaseService
-  def execute(branch_name)
-    repository = project.repository
-    branch = repository.find_branch(branch_name)
-
-    unless current_user.can?(:push_code, project)
-      return ServiceResponse.error(
-        message: 'You dont have push access to repo',
-        http_status: 405)
-    end
-
-    unless branch
-      return ServiceResponse.error(
-        message: 'No such branch',
-        http_status: 404)
-    end
-
-    if repository.rm_branch(current_user, branch_name)
-      ServiceResponse.success(message: 'Branch was deleted')
-    else
-      ServiceResponse.error(
-        message: 'Failed to remove branch',
-        http_status: 400)
-    end
-  rescue Gitlab::Git::PreReceiveError => ex
-    ServiceResponse.error(message: ex.message, http_status: 400)
-  end
-end

app/services/delete_merged_branches_service.rb

@@ -15,7 +15,7 @@ class DeleteMergedBranchesService < BaseService
     branches = branches.reject { |branch| ProtectedBranch.protected?(project, branch) }
 
     branches.each do |branch|
-      DeleteBranchService.new(project, current_user).execute(branch)
+      ::Branches::DeleteService.new(project, current_user).execute(branch)
     end
   end
 

app/services/merge_requests/merge_service.rb

@@ -99,7 +99,7 @@ module MergeRequests
       log_info("Post merge finished on JID #{merge_jid} with state #{state}")
 
       if delete_source_branch?
-        DeleteBranchService.new(@merge_request.source_project, branch_deletion_user)
+        ::Branches::DeleteService.new(@merge_request.source_project, branch_deletion_user)
           .execute(merge_request.source_branch)
       end
     end

ee/app/services/vulnerability_feedback/create_service.rb

@@ -93,7 +93,7 @@ module VulnerabilityFeedback
       branch_name = merge_request.source_branch
 
       merge_request&.destroy &&
-        DeleteBranchService.new(project, current_user).execute(branch_name)
+        ::Branches::DeleteService.new(project, current_user).execute(branch_name)
     end
   end
 end

ee/spec/services/delete_branch_service_spec.rb → ee/spec/services/branches/delete_service_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe DeleteBranchService do
+describe Branches::DeleteService do
   let(:project) { create(:project, :repository) }
   let(:repository) { project.repository }
   let(:user) { create(:user) }

lib/api/branches.rb

@@ -162,7 +162,7 @@ module API
         commit = user_project.repository.commit(branch.dereferenced_target)
 
         destroy_conditionally!(commit, last_updated: commit.authored_date) do
-          result = DeleteBranchService.new(user_project, current_user)
+          result = ::Branches::DeleteService.new(user_project, current_user)
                     .execute(params[:branch])
 
           if result.error?

spec/features/merge_request/user_sees_deleted_target_branch_spec.rb

@@ -9,7 +9,7 @@ describe 'Merge request > User sees deleted target branch', :js do
 
   before do
     project.add_maintainer(user)
-    DeleteBranchService.new(project, user).execute('feature')
+    ::Branches::DeleteService.new(project, user).execute('feature')
     sign_in(user)
     visit project_merge_request_path(project, merge_request)
   end

spec/services/delete_branch_service_spec.rb → spec/services/branches/delete_service_spec.rb

@@ -2,11 +2,11 @@
 
 require 'spec_helper'
 
-describe DeleteBranchService do
+describe Branches::DeleteService do
   let(:project) { create(:project, :repository) }
   let(:repository) { project.repository }
   let(:user) { create(:user) }
-  let(:service) { described_class.new(project, user) }
+  subject(:service) { described_class.new(project, user) }
 
   shared_examples 'a deleted branch' do |branch_name|
     it 'removes the branch' do

spec/services/merge_requests/merge_service_spec.rb

@@ -211,7 +211,8 @@ describe MergeRequests::MergeService do
         end
 
         it 'does not delete the source branch' do
-          expect(DeleteBranchService).not_to receive(:new)
+          expect(::Branches::DeleteService).not_to receive(:new)
+
           service.execute(merge_request)
         end
       end
@@ -226,7 +227,7 @@ describe MergeRequests::MergeService do
         end
 
         it 'does not delete the source branch' do
-          expect(DeleteBranchService).not_to receive(:new)
+          expect(::Branches::DeleteService).not_to receive(:new)
           service.execute(merge_request)
         end
       end
@@ -238,7 +239,7 @@ describe MergeRequests::MergeService do
           end
 
           it 'removes the source branch using the author user' do
-            expect(DeleteBranchService).to receive(:new)
+            expect(::Branches::DeleteService).to receive(:new)
               .with(merge_request.source_project, merge_request.author)
               .and_call_original
             service.execute(merge_request)
@@ -248,7 +249,7 @@ describe MergeRequests::MergeService do
             let(:service) { described_class.new(project, user, merge_params.merge('should_remove_source_branch' => false)) }
 
             it 'does not delete the source branch' do
-              expect(DeleteBranchService).not_to receive(:new)
+              expect(::Branches::DeleteService).not_to receive(:new)
               service.execute(merge_request)
             end
           end
@@ -260,7 +261,7 @@ describe MergeRequests::MergeService do
           end
 
           it 'removes the source branch using the current user' do
-            expect(DeleteBranchService).to receive(:new)
+            expect(::Branches::DeleteService).to receive(:new)
               .with(merge_request.source_project, user)
               .and_call_original
             service.execute(merge_request)

spec/services/merge_requests/merge_to_ref_service_spec.rb

@@ -61,7 +61,7 @@ describe MergeRequests::MergeToRefService do
     end
 
     it 'does not delete the source branch' do
-      expect(DeleteBranchService).not_to receive(:new)
+      expect(::Branches::DeleteService).not_to receive(:new)
 
       process_merge_to_ref
     end

spec/services/merge_requests/refresh_service_spec.rb

@@ -113,7 +113,7 @@ describe MergeRequests::RefreshService do
 
       context 'when source branch ref does not exists' do
         before do
-          DeleteBranchService.new(@project, @user).execute(@merge_request.source_branch)
+          ::Branches::DeleteService.new(@project, @user).execute(@merge_request.source_branch)
         end
 
         it 'closes MRs without source branch ref' do