diff options
author | Rémy Coutable <remy@rymai.me> | 2016-10-05 10:42:12 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-10-05 17:42:26 +0300 |
commit | 07602af16360329aa8af1f2091e156c9c92bdf03 (patch) | |
tree | 6d60bdfce1a847621a4efab2e81239c6af9cc6c1 /spec | |
parent | 9264816a34c45fb36c0117fe95bdf9e9af3287c8 (diff) |
Merge branch 'sh-fix-project-deletion-private-visibility' into 'master'
Fix project deletion when feature visibility is set to private
Projects that are destroyed are put in the pending_delete state.
The ProjectDestroyWorker checks whether the current user has
access, but since the ProjectFeature class uses the default scope
of the Project, it will not be able to find the right project.
This was a regression in 8.12 that caused the following stack trace:
```
NoMethodError: undefined method `team' for nil:NilClass
from app/models/project_feature.rb:62:in `get_permission'
from app/models/project_feature.rb:34:in `feature_available?'
from app/models/project.rb:21:in `feature_available?'
from app/policies/project_policy.rb:170:in `disabled_features!'
from app/policies/project_policy.rb:29:in `rules'
from app/policies/base_policy.rb:82:in `block in abilities'
from app/policies/base_policy.rb:113:in `collect_rules'
from app/policies/base_policy.rb:82:in `abilities'
from app/policies/base_policy.rb:50:in `abilities'
from app/models/ability.rb:64:in `uncached_allowed'
from app/models/ability.rb:58:in `allowed'
from app/models/ability.rb:49:in `allowed?'
from app/services/base_service.rb:11:in `can?'
from lib/gitlab/metrics/instrumentation.rb:155:in `block in can?'
from lib/gitlab/metrics/method_call.rb:23:in `measure'
from lib/gitlab/metrics/instrumentation.rb:155:in `can?'
from app/services/projects/destroy_service.rb:18:in `execute'
```
Closes #22948
See merge request !6688
Signed-off-by: Rémy Coutable <remy@rymai.me>
Diffstat (limited to 'spec')
-rw-r--r-- | spec/services/projects/destroy_service_spec.rb | 23 |
1 files changed, 22 insertions, 1 deletions
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 29341c5e57e..7dcd03496bb 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 |