Merge branch 'fix-project-destroy-skip-repo' into 'master'

Fix skip_repo parameter being ignored when destroying a namespace

When destroying a namespace, the `skip_repo` parameter is supposed to prevent the repository directory from being destroyed and allow
the namespace after_destroy hook to run. If the namespace fails to be deleted for some reason, we could be left with repositories that are deleted with existing projects.

See merge request !5654

CHANGELOG

@@ -52,6 +52,7 @@ v 8.11.0 (unreleased)
   - Add commit stats in commit api. !5517 (dixpac)
   - Add CI configuration button on project page
   - Make error pages responsive (Takuya Noguchi)
+  - Fix skip_repo parameter being ignored when destroying a namespace
   - Change requests_profiles resource constraint to catch virtually any file
   - Reduce number of queries made for merge_requests/:id/diffs
   - Sensible state specific default sort order for issues and merge requests !5453 (tomb0y)

app/workers/project_destroy_worker.rb

@@ -12,6 +12,6 @@ class ProjectDestroyWorker
 
     user = User.find(user_id)
 
-    ::Projects::DestroyService.new(project, user, params).execute
+    ::Projects::DestroyService.new(project, user, params.symbolize_keys).execute
   end
 end

spec/workers/project_destroy_worker_spec.rb

+require 'spec_helper'
+
+describe ProjectDestroyWorker do
+  let(:project) { create(:project) }
+  let(:path) { project.repository.path_to_repo }
+
+  subject { ProjectDestroyWorker.new }
+
+  describe "#perform" do
+    it "deletes the project" do
+      subject.perform(project.id, project.owner, {})
+
+      expect(Project.all).not_to include(project)
+      expect(Dir.exist?(path)).to be_falsey
+    end
+
+    it "deletes the project but skips repo deletion" do
+      subject.perform(project.id, project.owner, { "skip_repo" => true })
+
+      expect(Project.all).not_to include(project)
+      expect(Dir.exist?(path)).to be_truthy
+    end
+  end
+end