Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorToon Claes <toon@iotcl.com>2016-09-21 17:15:12 +0300
committerToon Claes <toon@iotcl.com>2016-11-09 23:04:03 +0300
commit1afab9eb79c87f32c7b899e58bc9a0ea8a113594 (patch)
tree33068aabffdb6b76c0489246429a3a71c2b748a4
parentc392b0cc24ba40e3fed920c6c693cb24665193af (diff)
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.
-rw-r--r--app/assets/stylesheets/framework/buttons.scss4
-rw-r--r--app/controllers/projects/branches_controller.rb9
-rw-r--r--app/services/delete_merged_branches_service.rb18
-rw-r--r--app/views/projects/branches/index.html.haml2
-rw-r--r--app/workers/delete_merged_branches_worker.rb20
-rw-r--r--changelogs/unreleased/21076-deleted-merged-branches.yml4
-rw-r--r--config/routes/project.rb1
-rw-r--r--config/sidekiq_queues.yml1
-rw-r--r--doc/api/branches.md18
-rw-r--r--lib/api/branches.rb12
-rw-r--r--spec/controllers/projects/branches_controller_spec.rb58
-rw-r--r--spec/requests/api/branches_spec.rb16
-rw-r--r--spec/services/delete_merged_branches_service_spec.rb54
-rw-r--r--spec/workers/delete_merged_branches_worker_spec.rb19
14 files changed, 231 insertions, 5 deletions
diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss
index ed21ad83a1c..8da8933e8a4 100644
--- a/app/assets/stylesheets/framework/buttons.scss
+++ b/app/assets/stylesheets/framework/buttons.scss
@@ -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 {
diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb
index 2de8ada3e29..6b9f37983c4 100644
--- a/app/controllers/projects/branches_controller.rb
+++ b/app/controllers/projects/branches_controller.rb
@@ -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
@@ -62,6 +62,13 @@ class Projects::BranchesController < Projects::ApplicationController
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
diff --git a/app/services/delete_merged_branches_service.rb b/app/services/delete_merged_branches_service.rb
new file mode 100644
index 00000000000..8b8deafedb7
--- /dev/null
+++ b/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
diff --git a/app/views/projects/branches/index.html.haml b/app/views/projects/branches/index.html.haml
index 84f38575e84..2246316b540 100644
--- a/app/views/projects/branches/index.html.haml
+++ b/app/views/projects/branches/index.html.haml
@@ -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
diff --git a/app/workers/delete_merged_branches_worker.rb b/app/workers/delete_merged_branches_worker.rb
new file mode 100644
index 00000000000..f870da4ecfd
--- /dev/null
+++ b/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
diff --git a/changelogs/unreleased/21076-deleted-merged-branches.yml b/changelogs/unreleased/21076-deleted-merged-branches.yml
new file mode 100644
index 00000000000..b7fa7f14384
--- /dev/null
+++ b/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
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 8142e231621..b4ee3f68e14 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -300,6 +300,7 @@ resources :namespaces, path: '/', constraints: { id: /[a-zA-Z.0-9_\-]+/ }, only:
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
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index f36fe893fd0..d0e6eb93fb2 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -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]
diff --git a/doc/api/branches.md b/doc/api/branches.md
index 0b5f7778fc7..f68eeb9f86b 100644
--- a/doc/api/branches.md
+++ b/doc/api/branches.md
@@ -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"
+```
diff --git a/lib/api/branches.rb b/lib/api/branches.rb
index 21a106387f0..73aed624ea7 100644
--- a/lib/api/branches.rb
+++ b/lib/api/branches.rb
@@ -128,6 +128,18 @@ module 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
diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb
index 644de308c64..f7cf006efd6 100644
--- a/spec/controllers/projects/branches_controller_spec.rb
+++ b/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'])
@@ -19,6 +19,8 @@ describe Projects::BranchesController do
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,
@@ -68,6 +70,10 @@ describe Projects::BranchesController do
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,
@@ -94,6 +100,10 @@ describe Projects::BranchesController do
describe "POST destroy with HTML format" do
render_views
+ before do
+ sign_in(user)
+ end
+
it 'returns 303' do
post :destroy,
format: :html,
@@ -109,6 +119,8 @@ describe Projects::BranchesController do
render_views
before do
+ sign_in(user)
+
post :destroy,
format: :js,
id: branch,
@@ -139,4 +151,42 @@ describe Projects::BranchesController do
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
diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb
index 1711096f4bd..8f605757186 100644
--- a/spec/requests/api/branches_spec.rb
+++ b/spec/requests/api/branches_spec.rb
@@ -299,4 +299,20 @@ describe API::API, api: true do
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
diff --git a/spec/services/delete_merged_branches_service_spec.rb b/spec/services/delete_merged_branches_service_spec.rb
new file mode 100644
index 00000000000..181488e89c7
--- /dev/null
+++ b/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
diff --git a/spec/workers/delete_merged_branches_worker_spec.rb b/spec/workers/delete_merged_branches_worker_spec.rb
new file mode 100644
index 00000000000..d9497bd486c
--- /dev/null
+++ b/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