don't trigger project deletion hooks twice when removing a group

3 files changed, 14 insertions, 4 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index 37231df979e..e042c2e6dd6 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -85,8 +85,7 @@ class Project < ActiveRecord::Base
   after_create :create_project_feature, unless: :project_feature
 
   after_create -> { SiteStatistic.track(STATISTICS_ATTRIBUTE) }
-  before_destroy ->(project) { project.project_feature } # keep reference so we can untrack later
-  after_destroy :untrack_site_statistics
+  before_destroy :untrack_site_statistics
 
   after_create :create_ci_cd_settings,
     unless: :ci_cd_settings,
@@ -2095,7 +2094,7 @@ class Project < ActiveRecord::Base
 
   def untrack_site_statistics
     SiteStatistic.untrack(STATISTICS_ATTRIBUTE)
-    SiteStatistic.project_feature.untrack_statistics_for_deletion!
+    self.project_feature.untrack_statistics_for_deletion!
   end
 
   def execute_rename_repository_hooks!(full_path_before)
diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb
index 12aeba4af71..93d84bd8a9c 100644
--- a/app/services/groups/destroy_service.rb
+++ b/app/services/groups/destroy_service.rb
@@ -12,12 +12,15 @@ module Groups
     def execute
       group.prepare_for_destroy
 
-      group.projects.each do |project|
+      group.projects.includes(:project_feature).each do |project|
         # Execute the destruction of the models immediately to ensure atomic cleanup.
         success = ::Projects::DestroyService.new(project, current_user).execute
         raise DestroyError, "Project #{project.id} can't be deleted" unless success
       end
 
+      # reload the relation to prevent triggering destroy hooks on the projects again
+      group.projects.reload
+
       group.children.each do |group|
         # This needs to be synchronous since the namespace gets destroyed below
         DestroyService.new(group, current_user).execute
diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb
index d80d0f5a8a8..97a88b5d697 100644
--- a/spec/services/groups/destroy_service_spec.rb
+++ b/spec/services/groups/destroy_service_spec.rb
@@ -35,6 +35,14 @@ describe Groups::DestroyService do
       it { expect(NotificationSetting.unscoped.all).not_to include(notification_setting) }
     end
 
+    context 'site statistics' do
+      it 'doesnt trigger project deletion hooks twice' do
+        expect_any_instance_of(Project).to receive(:untrack_site_statistics).once
+
+        destroy_group(group, user, async)
+      end
+    end
+
     context 'mattermost team' do
       let!(:chat_team) { create(:chat_team, namespace: group) }