Add button to delete all merged branches
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.
This commit is contained in:
parent
c392b0cc24
commit
1afab9eb79
14 changed files with 231 additions and 5 deletions
|
@ -142,6 +142,10 @@
|
||||||
&.btn-save {
|
&.btn-save {
|
||||||
@include btn-outline($white-light, $green-normal, $green-normal, $green-light, $white-light, $green-light);
|
@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 {
|
&.btn-gray {
|
||||||
|
|
|
@ -4,7 +4,7 @@ class Projects::BranchesController < Projects::ApplicationController
|
||||||
# Authorize
|
# Authorize
|
||||||
before_action :require_non_empty_project
|
before_action :require_non_empty_project
|
||||||
before_action :authorize_download_code!
|
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
|
def index
|
||||||
@sort = params[:sort].presence || sort_value_name
|
@sort = params[:sort].presence || sort_value_name
|
||||||
|
@ -62,6 +62,13 @@ class Projects::BranchesController < Projects::ApplicationController
|
||||||
end
|
end
|
||||||
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
|
private
|
||||||
|
|
||||||
def ref
|
def ref
|
||||||
|
|
18
app/services/delete_merged_branches_service.rb
Normal file
18
app/services/delete_merged_branches_service.rb
Normal file
|
@ -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
|
|
@ -26,6 +26,8 @@
|
||||||
= sort_title_oldest_updated
|
= sort_title_oldest_updated
|
||||||
|
|
||||||
- if can? current_user, :push_code, @project
|
- 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
|
= link_to new_namespace_project_branch_path(@project.namespace, @project), class: 'btn btn-create' do
|
||||||
New branch
|
New branch
|
||||||
|
|
||||||
|
|
20
app/workers/delete_merged_branches_worker.rb
Normal file
20
app/workers/delete_merged_branches_worker.rb
Normal file
|
@ -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
changelogs/unreleased/21076-deleted-merged-branches.yml
Normal file
4
changelogs/unreleased/21076-deleted-merged-branches.yml
Normal file
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
title: Add button to delete all merged branches
|
||||||
|
merge_request: 6449
|
||||||
|
author: Toon Claes
|
|
@ -300,6 +300,7 @@ resources :namespaces, path: '/', constraints: { id: /[a-zA-Z.0-9_\-]+/ }, only:
|
||||||
end
|
end
|
||||||
|
|
||||||
resources :branches, only: [:index, :new, :create, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex }
|
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
|
resources :tags, only: [:index, :show, :new, :create, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex } do
|
||||||
resource :release, only: [:edit, :update]
|
resource :release, only: [:edit, :update]
|
||||||
end
|
end
|
||||||
|
|
|
@ -33,6 +33,7 @@
|
||||||
- [project_service, 1]
|
- [project_service, 1]
|
||||||
- [clear_database_cache, 1]
|
- [clear_database_cache, 1]
|
||||||
- [delete_user, 1]
|
- [delete_user, 1]
|
||||||
|
- [delete_merged_branches, 1]
|
||||||
- [expire_build_instance_artifacts, 1]
|
- [expire_build_instance_artifacts, 1]
|
||||||
- [group_destroy, 1]
|
- [group_destroy, 1]
|
||||||
- [irker, 1]
|
- [irker, 1]
|
||||||
|
|
|
@ -240,3 +240,21 @@ Example response:
|
||||||
"branch_name": "newbranch"
|
"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"
|
||||||
|
```
|
||||||
|
|
|
@ -128,6 +128,18 @@ module API
|
||||||
render_api_error!(result[:message], result[:return_code])
|
render_api_error!(result[:message], result[:return_code])
|
||||||
end
|
end
|
||||||
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
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -1,13 +1,13 @@
|
||||||
require 'spec_helper'
|
require 'spec_helper'
|
||||||
|
|
||||||
describe Projects::BranchesController do
|
describe Projects::BranchesController do
|
||||||
let(:project) { create(:project) }
|
let(:project) { create(:project) }
|
||||||
let(:user) { create(:user) }
|
let(:user) { create(:user) }
|
||||||
|
let(:developer) { create(:user) }
|
||||||
|
|
||||||
before do
|
before do
|
||||||
sign_in(user)
|
|
||||||
|
|
||||||
project.team << [user, :master]
|
project.team << [user, :master]
|
||||||
|
project.team << [user, :developer]
|
||||||
|
|
||||||
allow(project).to receive(:branches).and_return(['master', 'foo/bar/baz'])
|
allow(project).to receive(:branches).and_return(['master', 'foo/bar/baz'])
|
||||||
allow(project).to receive(:tags).and_return(['v1.0.0', 'v2.0.0'])
|
allow(project).to receive(:tags).and_return(['v1.0.0', 'v2.0.0'])
|
||||||
|
@ -19,6 +19,8 @@ describe Projects::BranchesController do
|
||||||
|
|
||||||
context "on creation of a new branch" do
|
context "on creation of a new branch" do
|
||||||
before do
|
before do
|
||||||
|
sign_in(user)
|
||||||
|
|
||||||
post :create,
|
post :create,
|
||||||
namespace_id: project.namespace.to_param,
|
namespace_id: project.namespace.to_param,
|
||||||
project_id: project.to_param,
|
project_id: project.to_param,
|
||||||
|
@ -68,6 +70,10 @@ describe Projects::BranchesController do
|
||||||
let(:branch) { "1-feature-branch" }
|
let(:branch) { "1-feature-branch" }
|
||||||
let!(:issue) { create(:issue, project: project) }
|
let!(:issue) { create(:issue, project: project) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
sign_in(user)
|
||||||
|
end
|
||||||
|
|
||||||
it 'redirects' do
|
it 'redirects' do
|
||||||
post :create,
|
post :create,
|
||||||
namespace_id: project.namespace.to_param,
|
namespace_id: project.namespace.to_param,
|
||||||
|
@ -94,6 +100,10 @@ describe Projects::BranchesController do
|
||||||
describe "POST destroy with HTML format" do
|
describe "POST destroy with HTML format" do
|
||||||
render_views
|
render_views
|
||||||
|
|
||||||
|
before do
|
||||||
|
sign_in(user)
|
||||||
|
end
|
||||||
|
|
||||||
it 'returns 303' do
|
it 'returns 303' do
|
||||||
post :destroy,
|
post :destroy,
|
||||||
format: :html,
|
format: :html,
|
||||||
|
@ -109,6 +119,8 @@ describe Projects::BranchesController do
|
||||||
render_views
|
render_views
|
||||||
|
|
||||||
before do
|
before do
|
||||||
|
sign_in(user)
|
||||||
|
|
||||||
post :destroy,
|
post :destroy,
|
||||||
format: :js,
|
format: :js,
|
||||||
id: branch,
|
id: branch,
|
||||||
|
@ -139,4 +151,42 @@ describe Projects::BranchesController do
|
||||||
it { expect(response).to have_http_status(404) }
|
it { expect(response).to have_http_status(404) }
|
||||||
end
|
end
|
||||||
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
|
end
|
||||||
|
|
|
@ -299,4 +299,20 @@ describe API::API, api: true do
|
||||||
expect(json_response['message']).to eq('Cannot remove HEAD branch')
|
expect(json_response['message']).to eq('Cannot remove HEAD branch')
|
||||||
end
|
end
|
||||||
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
|
end
|
||||||
|
|
54
spec/services/delete_merged_branches_service_spec.rb
Normal file
54
spec/services/delete_merged_branches_service_spec.rb
Normal file
|
@ -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
spec/workers/delete_merged_branches_worker_spec.rb
Normal file
19
spec/workers/delete_merged_branches_worker_spec.rb
Normal file
|
@ -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
|
Loading…
Reference in a new issue