Merge branch 'dz-fix-group-move' into 'master'

Fix subgroup repository disappearance if group was moved

Closes #30261

See merge request !10414

3 files changed, 50 insertions, 15 deletions

diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 1d4b1f7d590..9bfa731785f 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -150,7 +150,7 @@ class Namespace < ActiveRecord::Base
   end
 
   def any_project_has_container_registry_tags?
-    projects.any?(&:has_container_registry_tags?)
+    all_projects.any?(&:has_container_registry_tags?)
   end
 
   def send_update_instructions
@@ -214,6 +214,12 @@ class Namespace < ActiveRecord::Base
     @old_repository_storage_paths ||= repository_storage_paths
   end
 
+  # Includes projects from this namespace and projects from all subgroups
+  # that belongs to this namespace
+  def all_projects
+    Project.inside_path(full_path)
+  end
+
   private
 
   def repository_storage_paths
@@ -221,7 +227,7 @@ class Namespace < ActiveRecord::Base
     # pending delete. Unscoping also get rids of the default order, which causes
     # problems with SELECT DISTINCT.
     Project.unscoped do
-      projects.select('distinct(repository_storage)').to_a.map(&:repository_storage_path)
+      all_projects.select('distinct(repository_storage)').to_a.map(&:repository_storage_path)
     end
   end
 
diff --git a/changelogs/unreleased/dz-fix-group-move.yml b/changelogs/unreleased/dz-fix-group-move.yml
new file mode 100644
index 00000000000..51fbe04fdc2
--- /dev/null
+++ b/changelogs/unreleased/dz-fix-group-move.yml
@@ -0,0 +1,4 @@
+---
+title: Fix subgroup repository disappearance if group was moved
+merge_request: 10414
+author:
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index ccaf0d7abc7..d9216112259 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -162,28 +162,44 @@ describe Namespace, models: true do
       it { expect { @namespace.move_dir }.to raise_error('Namespace cannot be moved, because at least one project has tags in container registry') }
     end
 
-    context 'renaming a sub-group' do
+    context 'with subgroups' do
       let(:parent) { create(:group, name: 'parent', path: 'parent') }
       let(:child) { create(:group, name: 'child', path: 'child', parent: parent) }
       let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child) }
-      let(:uploads_dir) { File.join(CarrierWave.root, 'uploads', 'parent') }
-      let(:pages_dir) { File.join(TestEnv.pages_path, 'parent') }
+      let(:uploads_dir) { File.join(CarrierWave.root, 'uploads') }
+      let(:pages_dir) { TestEnv.pages_path }
 
       before do
-        FileUtils.mkdir_p(File.join(uploads_dir, 'child', 'the-project'))
-        FileUtils.mkdir_p(File.join(pages_dir, 'child', 'the-project'))
+        FileUtils.mkdir_p(File.join(uploads_dir, 'parent', 'child', 'the-project'))
+        FileUtils.mkdir_p(File.join(pages_dir, 'parent', 'child', 'the-project'))
       end
 
-      it 'correctly moves the repository, uploads and pages' do
-        expected_repository_path = File.join(TestEnv.repos_path, 'parent', 'renamed', 'the-project.git')
-        expected_upload_path = File.join(uploads_dir, 'renamed', 'the-project')
-        expected_pages_path = File.join(pages_dir, 'renamed', 'the-project')
+      context 'renaming child' do
+        it 'correctly moves the repository, uploads and pages' do
+          expected_repository_path = File.join(TestEnv.repos_path, 'parent', 'renamed', 'the-project.git')
+          expected_upload_path = File.join(uploads_dir, 'parent', 'renamed', 'the-project')
+          expected_pages_path = File.join(pages_dir, 'parent', 'renamed', 'the-project')
 
-        child.update_attributes!(path: 'renamed')
+          child.update_attributes!(path: 'renamed')
 
-        expect(File.directory?(expected_repository_path)).to be(true)
-        expect(File.directory?(expected_upload_path)).to be(true)
-        expect(File.directory?(expected_pages_path)).to be(true)
+          expect(File.directory?(expected_repository_path)).to be(true)
+          expect(File.directory?(expected_upload_path)).to be(true)
+          expect(File.directory?(expected_pages_path)).to be(true)
+        end
+      end
+
+      context 'renaming parent' do
+        it 'correctly moves the repository, uploads and pages' do
+          expected_repository_path = File.join(TestEnv.repos_path, 'renamed', 'child', 'the-project.git')
+          expected_upload_path = File.join(uploads_dir, 'renamed', 'child', 'the-project')
+          expected_pages_path = File.join(pages_dir, 'renamed', 'child', 'the-project')
+
+          parent.update_attributes!(path: 'renamed')
+
+          expect(File.directory?(expected_repository_path)).to be(true)
+          expect(File.directory?(expected_upload_path)).to be(true)
+          expect(File.directory?(expected_pages_path)).to be(true)
+        end
       end
     end
   end
@@ -295,4 +311,13 @@ describe Namespace, models: true do
         to eq([namespace.owner_id])
     end
   end
+
+  describe '#all_projects' do
+    let(:group) { create(:group) }
+    let(:child) { create(:group, parent: group) }
+    let!(:project1) { create(:project_empty_repo, namespace: group) }
+    let!(:project2) { create(:project_empty_repo, namespace: child) }
+
+    it { expect(group.all_projects.to_a).to eq([project2, project1]) }
+  end
 end