Fix project deletion when feature visibility is set to private

CHANGELOG

@@ -53,6 +53,7 @@ v 8.13.0 (unreleased)
 v 8.12.4 (unreleased)
   - Fix type mismatch bug when closing Jira issue
   - Skip wiki creation when GitHub project has wiki enabled
+  - Fix failed project deletion when feature visibility set to private
   - Fix issues importing services via Import/Export
   - Restrict failed login attempts for users with 2FA enabled
   - Fix "Copy to clipboard" tooltip to say "Copied!" when clipboard button is clicked. (lukehowell)

app/models/project_feature.rb

@@ -20,7 +20,10 @@ class ProjectFeature < ActiveRecord::Base
 
   FEATURES = %i(issues merge_requests wiki snippets builds)
 
-  belongs_to :project
+  # Default scopes force us to unscope here since a service may need to check
+  # permissions for a project in pending_delete
+  # http://stackoverflow.com/questions/1540645/how-to-disable-default-scope-for-a-belongs-to
+  belongs_to :project, -> { unscope(where: :pending_delete) }
 
   default_value_for :builds_access_level,         value: ENABLED, allows_nil: false
   default_value_for :issues_access_level,         value: ENABLED, allows_nil: false

spec/services/projects/destroy_service_spec.rb

@@ -5,6 +5,7 @@ describe Projects::DestroyService, services: true do
   let!(:project) { create(:project, namespace: user.namespace) }
   let!(:path) { project.repository.path_to_repo }
   let!(:remove_path) { path.sub(/\.git\Z/, "+#{project.id}+deleted.git") }
+  let!(:async) { false } # execute or async_execute
 
   context 'Sidekiq inline' do
     before do
@@ -28,6 +29,22 @@ describe Projects::DestroyService, services: true do
     it { expect(Dir.exist?(remove_path)).to be_truthy }
   end
 
+  context 'async delete of project with private issue visibility' do
+    let!(:async) { true }
+
+    before do
+      project.project_feature.update_attribute("issues_access_level", ProjectFeature::PRIVATE)
+      # Run sidekiq immediately to check that renamed repository will be removed
+      Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
+    end
+
+    it 'deletes the project' do
+      expect(Project.all).not_to include(project)
+      expect(Dir.exist?(path)).to be_falsey
+      expect(Dir.exist?(remove_path)).to be_falsey
+    end
+  end
+
   context 'container registry' do
     before do
       stub_container_registry_config(enabled: true)
@@ -52,6 +69,10 @@ describe Projects::DestroyService, services: true do
   end
 
   def destroy_project(project, user, params)
-    Projects::DestroyService.new(project, user, params).execute
+    if async
+      Projects::DestroyService.new(project, user, params).async_execute
+    else
+      Projects::DestroyService.new(project, user, params).execute
+    end
   end
 end