From 2e719bda0aec3970bcda06a21aa1e5eb41db323b Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Fri, 24 Aug 2018 00:52:35 +0200 Subject: [PATCH] don't trigger project deletion hooks twice when removing a group --- app/models/project.rb | 5 ++--- app/services/groups/destroy_service.rb | 5 ++++- spec/services/groups/destroy_service_spec.rb | 8 ++++++++ 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) }