diff options
-rw-r--r-- | app/assets/stylesheets/framework/buttons.scss | 4 | ||||
-rw-r--r-- | app/controllers/projects/branches_controller.rb | 9 | ||||
-rw-r--r-- | app/services/delete_merged_branches_service.rb | 18 | ||||
-rw-r--r-- | app/views/projects/branches/index.html.haml | 2 | ||||
-rw-r--r-- | app/workers/delete_merged_branches_worker.rb | 20 | ||||
-rw-r--r-- | changelogs/unreleased/21076-deleted-merged-branches.yml | 4 | ||||
-rw-r--r-- | config/routes/project.rb | 1 | ||||
-rw-r--r-- | config/sidekiq_queues.yml | 1 | ||||
-rw-r--r-- | doc/api/branches.md | 18 | ||||
-rw-r--r-- | lib/api/branches.rb | 12 | ||||
-rw-r--r-- | spec/controllers/projects/branches_controller_spec.rb | 58 | ||||
-rw-r--r-- | spec/requests/api/branches_spec.rb | 16 | ||||
-rw-r--r-- | spec/services/delete_merged_branches_service_spec.rb | 54 | ||||
-rw-r--r-- | spec/workers/delete_merged_branches_worker_spec.rb | 19 |
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 |