diff options
author | Gabriel Mazetto <brodock@gmail.com> | 2018-08-24 00:52:35 +0200 |
---|---|---|
committer | Gabriel Mazetto <brodock@gmail.com> | 2018-08-26 10:58:43 +0200 |
commit | 2e719bda0aec3970bcda06a21aa1e5eb41db323b (patch) | |
tree | 5fe03b2871724bfb6f80d5147ef914e580656654 | |
parent | a32a410fb9bfcd378840b61426a54fd56d2ebd33 (diff) | |
download | gitlab-ce-2e719bda0aec3970bcda06a21aa1e5eb41db323b.tar.gz |
don't trigger project deletion hooks twice when removing a group
-rw-r--r-- | app/models/project.rb | 5 | ||||
-rw-r--r-- | app/services/groups/destroy_service.rb | 5 | ||||
-rw-r--r-- | 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) } |