Prevent project team from being truncated too early during project destructionsh-fix-project-team-truncation-in-destroy

There are two issues with truncating the project team early:

1. `Projects::UnlinkForkService` may not close merge requests properly since
   permissions may be revoked early.

2. If an error is encountered during flushing of caches, then the user will
   lose all privileges, possibly causing an issue on deletion on retry.

2 files changed, 20 insertions, 2 deletions

diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index a08c6fcd94b..9716a1780a9 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -17,8 +17,6 @@ module Projects
     def execute
       return false unless can?(current_user, :remove_project, project)
 
-      project.team.truncate
-
       repo_path = project.path_with_namespace
       wiki_path = repo_path + '.wiki'
 
@@ -30,6 +28,7 @@ module Projects
       Projects::UnlinkForkService.new(project, current_user).execute
 
       Project.transaction do
+        project.team.truncate
         project.destroy!
 
         unless remove_registry_tags
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index 3faa88c00a1..74bfba44dfd 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -50,6 +50,25 @@ describe Projects::DestroyService, services: true do
     it { expect(Dir.exist?(remove_path)).to be_truthy }
   end
 
+  context 'when flushing caches fail' do
+    before do
+      new_user = create(:user)
+      project.team.add_user(new_user, Gitlab::Access::DEVELOPER)
+      allow_any_instance_of(Projects::DestroyService).to receive(:flush_caches).and_raise(Redis::CannotConnectError)
+    end
+
+    it 'keeps project team intact upon an error' do
+      Sidekiq::Testing.inline! do
+        begin
+          destroy_project(project, user, {})
+        rescue Redis::CannotConnectError
+        end
+      end
+
+      expect(project.team.members.count).to eq 1
+    end
+  end
+
   context 'with async_execute' do
     let(:async) { true }