diff --git a/CHANGELOG b/CHANGELOG index e2e3033615ffa8351cf75ce5d63d479aeafdaf40..9635309b052dec0b54806d8a704e5ca20c62e575 100644 --- a/CHANGELOG +++ b/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) diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index 8c9534c3565f95b80cb6a0b12ef47e40468c4949..530f7d5a30e6ad182e1aa1a0f68dfafc40c40c99 100644 --- a/app/models/project_feature.rb +++ b/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 diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 29341c5e57e129e4c023a0d656237586caa56b72..7dcd03496bbcae7df6be43437d632d175e260041 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/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