diff options
-rw-r--r-- | app/models/namespace.rb | 12 | ||||
-rw-r--r-- | app/models/project.rb | 2 | ||||
-rw-r--r-- | app/services/groups/destroy_service.rb | 4 | ||||
-rw-r--r-- | spec/services/groups/destroy_service_spec.rb | 19 |
4 files changed, 31 insertions, 6 deletions
diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 6de4d08fc28..bd0336c984a 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -42,7 +42,7 @@ class Namespace < ActiveRecord::Base after_commit :refresh_access_of_projects_invited_groups, on: :update, if: -> { previous_changes.key?('share_with_group_lock') } # Save the storage paths before the projects are destroyed to use them on after destroy - before_destroy(prepend: true) { @old_repository_storage_paths = repository_storage_paths } + before_destroy(prepend: true) { prepare_for_destroy } after_destroy :rm_dir scope :root, -> { where('type IS NULL') } @@ -211,6 +211,14 @@ class Namespace < ActiveRecord::Base parent_id_changed? end + def prepare_for_destroy + old_repository_storage_paths + end + + def old_repository_storage_paths + @old_repository_storage_paths ||= repository_storage_paths + end + private def repository_storage_paths @@ -224,7 +232,7 @@ class Namespace < ActiveRecord::Base def rm_dir # Remove the namespace directory in all storages paths used by member projects - @old_repository_storage_paths.each do |repository_storage_path| + old_repository_storage_paths.each do |repository_storage_path| # Move namespace directory into trash. # We will remove it later async new_path = "#{path}+#{id}+deleted" diff --git a/app/models/project.rb b/app/models/project.rb index ed43fc2e575..fc5b1a66910 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -214,6 +214,8 @@ class Project < ActiveRecord::Base # Scopes default_scope { where(pending_delete: false) } + scope :with_deleted, -> { unscope(where: :pending_delete) } + scope :sorted_by_activity, -> { reorder(last_activity_at: :desc) } scope :sorted_by_stars, -> { reorder('projects.star_count DESC') } diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index 7f2d28086f5..2e2d7f884ac 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -8,7 +8,9 @@ module Groups end def execute - group.projects.each do |project| + group.prepare_for_destroy + + group.projects.with_deleted.each do |project| # Execute the destruction of the models immediately to ensure atomic cleanup. # Skip repository removal because we remove directory with namespace # that contain all these repositories diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index f86189b68e9..32c2ed8cae7 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -9,14 +9,18 @@ describe Groups::DestroyService, services: true do let!(:gitlab_shell) { Gitlab::Shell.new } let!(:remove_path) { group.path + "+#{group.id}+deleted" } + before do + group.add_user(user, Gitlab::Access::OWNER) + end + shared_examples 'group destruction' do |async| context 'database records' do before do destroy_group(group, user, async) end - it { expect(Group.all).not_to include(group) } - it { expect(Project.all).not_to include(project) } + it { expect(Group.unscoped.all).not_to include(group) } + it { expect(Project.unscoped.all).not_to include(project) } end context 'file system' do @@ -32,7 +36,7 @@ describe Groups::DestroyService, services: true do context 'Sidekiq fake' do before do - # Dont run sidekiq to check if renamed repository exists + # Don't run sidekiq to check if renamed repository exists Sidekiq::Testing.fake! { destroy_group(group, user, async) } end @@ -95,4 +99,13 @@ describe Groups::DestroyService, services: true do describe 'synchronous delete' do it_behaves_like 'group destruction', false end + + context 'projects in pending_delete' do + before do + project.pending_delete = true + project.save + end + + it_behaves_like 'group destruction', false + end end |