Skip to content

Commit

Permalink
Add button to delete all merged branches
Browse files Browse the repository at this point in the history
It adds a button to the branches page that the user can use to delete
all the branches that are already merged. This can be used to clean up
all the branches that were forgotten to delete while merging MRs.

Fixes #21076.
  • Loading branch information
To1ne committed Nov 9, 2016
1 parent c392b0c commit 1afab9e
Show file tree
Hide file tree
Showing 14 changed files with 231 additions and 5 deletions.
4 changes: 4 additions & 0 deletions app/assets/stylesheets/framework/buttons.scss
Expand Up @@ -142,6 +142,10 @@
&.btn-save {
@include btn-outline($white-light, $green-normal, $green-normal, $green-light, $white-light, $green-light);
}

&.btn-remove {
@include btn-outline($white-light, $red-normal, $red-normal, $red-light, $white-light, $red-light);
}
}

&.btn-gray {
Expand Down
9 changes: 8 additions & 1 deletion app/controllers/projects/branches_controller.rb
Expand Up @@ -4,7 +4,7 @@ class Projects::BranchesController < Projects::ApplicationController
# Authorize
before_action :require_non_empty_project
before_action :authorize_download_code!
before_action :authorize_push_code!, only: [:new, :create, :destroy]
before_action :authorize_push_code!, only: [:new, :create, :destroy, :destroy_all_merged]

def index
@sort = params[:sort].presence || sort_value_name
Expand Down Expand Up @@ -62,6 +62,13 @@ def destroy
end
end

def destroy_all_merged
DeleteMergedBranchesService.new(@project, current_user).async_execute

redirect_to namespace_project_branches_path(@project.namespace, @project),
notice: 'Merged branches are being deleted. This can take some time depending on the number of branches. Please refresh the page to see changes.'
end

private

def ref
Expand Down
18 changes: 18 additions & 0 deletions app/services/delete_merged_branches_service.rb
@@ -0,0 +1,18 @@
require_relative 'base_service'

class DeleteMergedBranchesService < BaseService
def async_execute
DeleteMergedBranchesWorker.perform_async(project.id, current_user.id)
end

def execute
raise Gitlab::Access::AccessDeniedError unless can?(current_user, :push_code, project)

branches = project.repository.branch_names
branches = branches.select { |branch| project.repository.merged_to_root_ref?(branch) }

branches.each do |branch|
DeleteBranchService.new(project, current_user).execute(branch)
end
end
end
2 changes: 2 additions & 0 deletions app/views/projects/branches/index.html.haml
Expand Up @@ -26,6 +26,8 @@
= sort_title_oldest_updated

- if can? current_user, :push_code, @project
= link_to namespace_project_merged_branches_path(@project.namespace, @project), class: 'btn btn-inverted btn-remove has-tooltip', title: "Delete all branches that are merged into '#{@project.repository.root_ref}'", method: :delete, data: { confirm: "Deleting the merged branches cannot be undone. Are you sure?", container: 'body' } do
Delete merged branches
= link_to new_namespace_project_branch_path(@project.namespace, @project), class: 'btn btn-create' do
New branch

Expand Down
20 changes: 20 additions & 0 deletions app/workers/delete_merged_branches_worker.rb
@@ -0,0 +1,20 @@
class DeleteMergedBranchesWorker
include Sidekiq::Worker
include DedicatedSidekiqQueue

def perform(project_id, user_id)
begin
project = Project.find(project_id)
rescue ActiveRecord::RecordNotFound
return
end

user = User.find(user_id)

begin
DeleteMergedBranchesService.new(project, user).execute
rescue Gitlab::Access::AccessDeniedError
return
end
end
end
4 changes: 4 additions & 0 deletions changelogs/unreleased/21076-deleted-merged-branches.yml
@@ -0,0 +1,4 @@
---
title: Add button to delete all merged branches
merge_request: 6449
author: Toon Claes
1 change: 1 addition & 0 deletions config/routes/project.rb
Expand Up @@ -300,6 +300,7 @@
end

resources :branches, only: [:index, :new, :create, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex }
delete :merged_branches, controller: 'branches', action: :destroy_all_merged
resources :tags, only: [:index, :show, :new, :create, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex } do
resource :release, only: [:edit, :update]
end
Expand Down
1 change: 1 addition & 0 deletions config/sidekiq_queues.yml
Expand Up @@ -33,6 +33,7 @@
- [project_service, 1]
- [clear_database_cache, 1]
- [delete_user, 1]
- [delete_merged_branches, 1]
- [expire_build_instance_artifacts, 1]
- [group_destroy, 1]
- [irker, 1]
Expand Down
18 changes: 18 additions & 0 deletions doc/api/branches.md
Expand Up @@ -240,3 +240,21 @@ Example response:
"branch_name": "newbranch"
}
```

## Delete merged branches

Will delete all branches that are merged into the project's default branch.

```
DELETE /projects/:id/repository/merged_branches
```

| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer | yes | The ID of a project |

It returns `200` to indicate deletion of all merged branches was started.

```bash
curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/repository/merged_branches"
```
12 changes: 12 additions & 0 deletions lib/api/branches.rb
Expand Up @@ -128,6 +128,18 @@ class Branches < Grape::API
render_api_error!(result[:message], result[:return_code])
end
end

# Delete all merged branches
#
# Parameters:
# id (required) - The ID of a project
# Example Request:
# DELETE /projects/:id/repository/branches/delete_merged
delete ":id/repository/merged_branches" do
DeleteMergedBranchesService.new(user_project, current_user).async_execute

status(200)
end
end
end
end
58 changes: 54 additions & 4 deletions spec/controllers/projects/branches_controller_spec.rb
@@ -1,13 +1,13 @@
require 'spec_helper'

describe Projects::BranchesController do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:developer) { create(:user) }

before do
sign_in(user)

project.team << [user, :master]
project.team << [user, :developer]

allow(project).to receive(:branches).and_return(['master', 'foo/bar/baz'])
allow(project).to receive(:tags).and_return(['v1.0.0', 'v2.0.0'])
Expand All @@ -19,6 +19,8 @@

context "on creation of a new branch" do
before do
sign_in(user)

post :create,
namespace_id: project.namespace.to_param,
project_id: project.to_param,
Expand Down Expand Up @@ -68,6 +70,10 @@
let(:branch) { "1-feature-branch" }
let!(:issue) { create(:issue, project: project) }

before do
sign_in(user)
end

it 'redirects' do
post :create,
namespace_id: project.namespace.to_param,
Expand All @@ -94,6 +100,10 @@
describe "POST destroy with HTML format" do
render_views

before do
sign_in(user)
end

it 'returns 303' do
post :destroy,
format: :html,
Expand All @@ -109,6 +119,8 @@
render_views

before do
sign_in(user)

post :destroy,
format: :js,
id: branch,
Expand Down Expand Up @@ -139,4 +151,42 @@
it { expect(response).to have_http_status(404) }
end
end

describe "DELETE destroy_all_merged" do
def destroy_all_merged
delete :destroy_all_merged,
namespace_id: project.namespace.to_param,
project_id: project.to_param
end

context 'when user is allowed to push' do
before do
sign_in(user)
end

it 'redirects to branches' do
destroy_all_merged

expect(response).to redirect_to namespace_project_branches_path(project.namespace, project)
end

it 'starts worker to delete merged branches' do
expect_any_instance_of(DeleteMergedBranchesService).to receive(:async_execute)

destroy_all_merged
end
end

context 'when user is not allowed to push' do
before do
sign_in(developer)
end

it 'responds with status 404' do
destroy_all_merged

expect(response).to have_http_status(404)
end
end
end
end
16 changes: 16 additions & 0 deletions spec/requests/api/branches_spec.rb
Expand Up @@ -299,4 +299,20 @@
expect(json_response['message']).to eq('Cannot remove HEAD branch')
end
end

describe "DELETE /projects/:id/repository/merged_branches" do
before do
allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true)
end

it 'returns 200' do
delete api("/projects/#{project.id}/repository/merged_branches", user)
expect(response).to have_http_status(200)
end

it 'returns a 403 error if guest' do
delete api("/projects/#{project.id}/repository/merged_branches", user2)
expect(response).to have_http_status(403)
end
end
end
54 changes: 54 additions & 0 deletions spec/services/delete_merged_branches_service_spec.rb
@@ -0,0 +1,54 @@
require 'spec_helper'

describe DeleteMergedBranchesService, services: true do
subject(:service) { described_class.new(project, project.owner) }

let(:project) { create(:project) }

context '#execute' do
context 'unprotected branches' do
before do
service.execute
end

it 'deletes a branch that was merged' do
expect(project.repository.branch_names).not_to include('improve/awesome')
end

it 'keeps branch that is unmerged' do
expect(project.repository.branch_names).to include('feature')
end

it 'keeps "master"' do
expect(project.repository.branch_names).to include('master')
end
end

context 'protected branches' do
before do
create(:protected_branch, name: 'improve/awesome', project: project)
service.execute
end

it 'keeps protected branch' do
expect(project.repository.branch_names).to include('improve/awesome')
end
end

context 'user without rights' do
let(:user) { create(:user) }

it 'cannot execute' do
expect { described_class.new(project, user).execute }.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
end

context '#async_execute' do
it 'calls DeleteMergedBranchesWorker async' do
expect(DeleteMergedBranchesWorker).to receive(:perform_async)

service.async_execute
end
end
end
19 changes: 19 additions & 0 deletions spec/workers/delete_merged_branches_worker_spec.rb
@@ -0,0 +1,19 @@
require 'spec_helper'

describe DeleteMergedBranchesWorker do
subject(:worker) { described_class.new }

let(:project) { create(:project) }

describe "#perform" do
it "calls DeleteMergedBranchesService" do
expect_any_instance_of(DeleteMergedBranchesService).to receive(:execute).and_return(true)

worker.perform(project.id, project.owner.id)
end

it "returns false when project was not found" do
expect(worker.perform('unknown', project.owner.id)).to be_falsy
end
end
end

0 comments on commit 1afab9e

Please sign in to comment.