From 68b5ac7f185b9b30cd862eacf806db726d8ce6e4 Mon Sep 17 00:00:00 2001 From: Vinnie Okada <vokada@mrvinn.com> Date: Tue, 7 Oct 2014 14:55:15 -0500 Subject: [PATCH] Add option to keep repo on project delete Update the project API controller to use `Projects::DestroyService` instead of calling `Project#destroy` directly. Also add an optional parameter, `:keep_repo`, that allows a project to be deleted without deleting the repository, wiki, and satellite from disk. --- app/controllers/projects_controller.rb | 3 ++- app/services/projects/destroy_service.rb | 13 ++++++++----- lib/api/projects.rb | 8 +++++++- spec/features/projects_spec.rb | 7 ++++++- spec/requests/api/projects_spec.rb | 15 +++++++++++++++ 5 files changed, 38 insertions(+), 8 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index b3380a6ff2..c881c921ce 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -100,7 +100,8 @@ class ProjectsController < ApplicationController def destroy return access_denied! unless can?(current_user, :remove_project, project) - ::Projects::DestroyService.new(@project, current_user, {}).execute + ::Projects::DestroyService.new(@project, current_user, + keep_repo: params[:keep_repo]).execute respond_to do |format| format.html do diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 7e1d753b02..7c7892a0b1 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -6,7 +6,10 @@ module Projects project.team.truncate project.repository.expire_cache unless project.empty_repo? - if project.destroy + result = project.destroy + return false unless result + + unless params[:keep_repo] GitlabShellWorker.perform_async( :remove_repository, project.path_with_namespace @@ -18,11 +21,11 @@ module Projects ) project.satellite.destroy - - log_info("Project \"#{project.name}\" was removed") - system_hook_service.execute_hooks_for(project, :destroy) - true end + + log_info("Project \"#{project.name}\" was removed") + system_hook_service.execute_hooks_for(project, :destroy) + result end end end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 7f7d2f8e9a..e70548d1e8 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -174,11 +174,17 @@ module API # # Parameters: # id (required) - The ID of a project + # keep_repo (optional) - If true, then delete the project from the + # database but keep the repo, wiki, and satellite on disk. # Example Request: # DELETE /projects/:id delete ":id" do authorize! :remove_project, user_project - user_project.destroy + ::Projects::DestroyService.new( + user_project, + current_user, + keep_repo: params[:keep_repo] + ).execute end # Mark this project as forked from another diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index 524c4d5fa2..369db56d49 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -10,7 +10,12 @@ describe "Projects", feature: true do visit edit_project_path(@project) end - it "should be correct path" do + it 'should delete the project from the database and disk' do + expect(GitlabShellWorker).to( + receive(:perform_async).with(:remove_repository, + /#{@project.path_with_namespace}/) + ).twice + expect { click_link "Remove project" }.to change {Project.count}.by(-1) end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index aa1437c71a..6de37cff0a 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -632,10 +632,25 @@ describe API::API, api: true do describe "DELETE /projects/:id" do context "when authenticated as user" do it "should remove project" do + expect(GitlabShellWorker).to( + receive(:perform_async).with(:remove_repository, + /#{project.path_with_namespace}/) + ).twice + delete api("/projects/#{project.id}", user) response.status.should == 200 end + it 'should keep repo when "keep_repo" param is true' do + expect(GitlabShellWorker).not_to( + receive(:perform_async).with(:remove_repository, + /#{project.path_with_namespace}/) + ) + + delete api("/projects/#{project.id}?keep_repo=true", user) + response.status.should == 200 + end + it "should not remove a project if not an owner" do user3 = create(:user) project.team << [user3, :developer] -- 2.30.9