Commit 374b8e95 authored by Douwe Maan's avatar Douwe Maan

Merge branch '21076-deleted-merged-branches' into 'master'

Add button to delete all merged branches

## What does this MR do?

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.

**Note**  
~~This MR is WIP until MR !6408 is merged.~~

## Are there points in the code the reviewer needs to double check?

The UX of the actual "Delete merged branches" button.

## Why was this MR needed?

Fixes #21076

## Screenshots
![Branches page without "Delete all merged" button](/uploads/3a2936a83c3547a0fce92a74af880a2d/Screen_Shot_2016-10-17_at_20.06.30.png)

Before:
![Screen_Shot_2016-10-17_at_20.07.11](/uploads/55efcebf4e0a45dbfc70ba4a11ca152c/Screen_Shot_2016-10-17_at_20.07.11.png)


After:

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

## What are the relevant issue numbers?

Closes #21076

See merge request !6449
parents fdd7e3f6 1afab9eb
...@@ -141,6 +141,10 @@ ...@@ -141,6 +141,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 ...@@ -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 ...@@ -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
......
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 @@ ...@@ -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
......
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
---
title: Add button to delete all merged branches
merge_request: 6449
author: Toon Claes
...@@ -125,6 +125,7 @@ resources :namespaces, path: '/', constraints: { id: /[a-zA-Z.0-9_\-]+/ }, only: ...@@ -125,6 +125,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
......
...@@ -34,6 +34,7 @@ ...@@ -34,6 +34,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: ...@@ -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 ...@@ -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
...@@ -3,11 +3,11 @@ require 'spec_helper' ...@@ -3,11 +3,11 @@ 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 ...@@ -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 ...@@ -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 ...@@ -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 ...@@ -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 ...@@ -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 ...@@ -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
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
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment