Merge branch 'delete-records-from-deleting-projects' into 'master'

Allow destruction of records in archived pending_delete Projects

See merge request gitlab-org/gitlab!73863

app/policies/project_policy.rb

@@ -47,6 +47,9 @@ class ProjectPolicy < BasePolicy
   desc "Project is archived"
   condition(:archived, scope: :subject, score: 0) { project.archived? }
 
+  desc "Project is in the process of being deleted"
+  condition(:pending_delete) { project.pending_delete? }
+
   condition(:default_issues_tracker, scope: :subject) { project.default_issues_tracker? }
 
   desc "Container registry is disabled"
@@ -457,7 +460,13 @@ class ProjectPolicy < BasePolicy
     prevent(*readonly_abilities)
 
     readonly_features.each do |feature|
-      prevent(*create_update_admin_destroy(feature))
+      prevent(*create_update_admin(feature))
+    end
+  end
+
+  rule { archived & ~pending_delete }.policy do
+    readonly_features.each do |feature|
+      prevent(:"destroy_#{feature}")
     end
   end
 

spec/policies/project_policy_spec.rb

@@ -104,29 +104,71 @@ RSpec.describe ProjectPolicy do
   end
 
   context 'pipeline feature' do
-    let(:project) { private_project }
+    let(:project)      { private_project }
+    let(:current_user) { developer }
+    let(:pipeline)     { create(:ci_pipeline, project: project) }
 
-    before do
-      private_project.add_developer(current_user)
+    describe 'for confirmed user' do
+      it 'allows modify pipelines' do
+        expect_allowed(:create_pipeline)
+        expect_allowed(:update_pipeline)
+        expect_allowed(:create_pipeline_schedule)
+      end
     end
 
     describe 'for unconfirmed user' do
-      let(:current_user) { create(:user, confirmed_at: nil) }
+      let(:current_user) { project.owner.tap { |u| u.update!(confirmed_at: nil) } }
 
       it 'disallows to modify pipelines' do
         expect_disallowed(:create_pipeline)
         expect_disallowed(:update_pipeline)
+        expect_disallowed(:destroy_pipeline)
         expect_disallowed(:create_pipeline_schedule)
       end
     end
 
-    describe 'for confirmed user' do
-      let(:current_user) { developer }
+    describe 'destroy permission' do
+      describe 'for developers' do
+        it 'prevents :destroy_pipeline' do
+          expect(current_user.can?(:destroy_pipeline, pipeline)).to be_falsey
+        end
+      end
 
-      it 'allows modify pipelines' do
-        expect_allowed(:create_pipeline)
-        expect_allowed(:update_pipeline)
-        expect_allowed(:create_pipeline_schedule)
+      describe 'for maintainers' do
+        let(:current_user) { maintainer }
+
+        it 'prevents :destroy_pipeline' do
+          project.add_maintainer(maintainer)
+          expect(current_user.can?(:destroy_pipeline, pipeline)).to be_falsey
+        end
+      end
+
+      describe 'for project owner' do
+        let(:current_user) { project.owner }
+
+        it 'allows :destroy_pipeline' do
+          expect(current_user.can?(:destroy_pipeline, pipeline)).to be_truthy
+        end
+
+        context 'on archived projects' do
+          before do
+            project.update!(archived: true)
+          end
+
+          it 'prevents :destroy_pipeline' do
+            expect(current_user.can?(:destroy_pipeline, pipeline)).to be_falsey
+          end
+        end
+
+        context 'on archived pending_delete projects' do
+          before do
+            project.update!(archived: true, pending_delete: true)
+          end
+
+          it 'allows :destroy_pipeline' do
+            expect(current_user.can?(:destroy_pipeline, pipeline)).to be_truthy
+          end
+        end
       end
     end
   end

spec/services/projects/destroy_service_spec.rb

@@ -331,6 +331,14 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
         end
       end
     end
+
+    context 'for an archived project' do
+      before do
+        project.update!(archived: true)
+      end
+
+      it_behaves_like 'deleting the project with pipeline and build'
+    end
   end
 
   describe 'container registry' do