Includes transferring a project from Group to User into route redirects41935-group-validation-fails-incorrectly

scenarios

- Also simplify specs by using match_array instead of count and eq
  expectations
- Removes extra validation on Namespace

5 files changed, 271 insertions, 322 deletions

diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index e61b7a24871..302c8c81c4a 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -41,7 +41,6 @@ class Namespace < ActiveRecord::Base
     namespace_path: true
 
   validate :nesting_level_allowed
-  validate :allowed_path_by_redirects
 
   delegate :name, to: :owner, allow_nil: true, prefix: true
 
@@ -254,16 +253,6 @@ class Namespace < ActiveRecord::Base
       .update_all(share_with_group_lock: true)
   end
 
-  def allowed_path_by_redirects
-    return if path.nil?
-
-    errors.add(:path, "#{path} has been taken before. Please use another one") if namespace_previously_created_with_same_path?
-  end
-
-  def namespace_previously_created_with_same_path?
-    RedirectRoute.permanent.where.not(source: self).exists?(path: path)
-  end
-
   def write_projects_repository_config
     all_projects.find_each do |project|
       project.expires_full_path_cache # we need to clear cache to validate renames correctly
diff --git a/app/models/route.rb b/app/models/route.rb
index 8b1d9035b03..86b6de618a4 100644
--- a/app/models/route.rb
+++ b/app/models/route.rb
@@ -61,21 +61,20 @@ class Route < ActiveRecord::Base
   end
 
   def redirect_with_same_path_exists?
-    permanent_redirect_routes.where.not(source_id: self_and_descendant_ids).exists?
+    base_redirect_routes.where.not(source_id: self_and_descendant_ids).exists?
   end
 
-  def permanent_redirect_routes
-    RedirectRoute
-      .permanent
-      .namespace_type
-      .matching_path_and_descendants(path)
+  def base_redirect_routes
+    RedirectRoute.permanent.matching_path_and_descendants(path)
   end
 
   def self_and_descendant_ids
-    if source.is_a?(Project) || Gitlab::Database.mysql?
+    if source.is_a?(Project)
       source.id
+    elsif Gitlab::Database.mysql?
+      [source.id] + source.projects.select(:id)
     else
-      source.self_and_descendants.select(:id)
+      source.self_and_descendants.select(:id) + source.projects.select(:id)
     end
   end
 
@@ -87,14 +86,14 @@ class Route < ActiveRecord::Base
 
   def deletable_conflicting_redirects
     if reclaiming_an_old_path?
-      RedirectRoute.matching_path_and_descendants(path)
+      base_redirect_routes
     else
       RedirectRoute.temporary.matching_path_and_descendants(path)
     end
   end
 
   def reclaiming_an_old_path?
-    permanent_redirect_routes.where(path: path).exists?
+    base_redirect_routes.namespace_type.where(path: path).exists?
   end
 
   def create_redirect_for_old_path
diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb
index 26765e5c3f3..71af8d94e7b 100644
--- a/app/services/projects/transfer_service.rb
+++ b/app/services/projects/transfer_service.rb
@@ -115,6 +115,7 @@ module Projects
 
     def rollback_side_effects
       rollback_folder_move
+      project.reload if Rails.env.test?
       update_namespace_and_visibility(@old_namespace)
       write_repository_config(@old_path)
     end
diff --git a/changelogs/unreleased/41935-group-validation-fails-incorrectly.yml b/changelogs/unreleased/41935-group-validation-fails-incorrectly.yml
new file mode 100644
index 00000000000..cd52b21308e
--- /dev/null
+++ b/changelogs/unreleased/41935-group-validation-fails-incorrectly.yml
@@ -0,0 +1,5 @@
+---
+title: Fixes route validation when reclaiming old paths
+merge_request: 16505
+author:
+type: fixed
diff --git a/spec/support/route_redirects.rb b/spec/support/route_redirects.rb
index 25fc1e5c699..edef8168e2f 100644
--- a/spec/support/route_redirects.rb
+++ b/spec/support/route_redirects.rb
@@ -3,342 +3,297 @@ shared_examples 'route redirects for groups, users and projects' do
     TestEnv.clean_test_path
   end
 
-  context 'when a Group with projects is renamed' do
+  describe 'Group scenarios' do
     let!(:group_foo) { create(:group, name: 'foo', path: 'foo') }
     let!(:project_bar) { create(:project, path: 'bar', namespace: group_foo) }
 
-    it 'should create the appropriate redirect routes' do
-      group_foo.path = 'foot'
-      group_foo.save
-      expect(group_foo.redirect_routes.permanent.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.first.path).to eq('foo')
-      expect(project_bar.redirect_routes.permanent.count).to eq(1)
-      expect(project_bar.redirect_routes.permanent.first.path).to eq('foo/bar')
-    end
-  end
-
-  context 'when a Group with projects reclaims an old path' do
-    let!(:group_foo) { create(:group, name: 'foo', path: 'foo') }
-    let!(:project_bar) { create(:project, path: 'bar', namespace: group_foo) }
+    context 'when a Group with projects is renamed' do
+      it 'should create the appropriate redirect routes' do
+        group_foo.path = 'foot'
+        group_foo.save
 
-    it 'should create the appropriate redirect routes' do
-      group_foo.path = 'foot'
-      group_foo.save
-      expect(group_foo.redirect_routes.permanent.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.first.path).to eq('foo')
-      expect(project_bar.redirect_routes.permanent.count).to eq(1)
-      expect(project_bar.redirect_routes.permanent.pluck(:path)).to match_array(['foo/bar'])
-
-      # Back to foo
-      group_foo.path = 'foo'
-      group_foo.save
-      expect(group_foo.redirect_routes.permanent.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.first.path).to eq('foot')
-      expect(project_bar.redirect_routes.permanent.count).to eq(1)
-      expect(project_bar.redirect_routes.permanent.pluck(:path)).to match_array(['foot/bar'])
+        expect(group_foo.redirect_routes.temporary).to be_empty
+        expect(group_foo.redirect_routes.permanent.pluck(:path)).to match_array(['foo'])
+        expect(project_bar.redirect_routes.temporary).to be_empty
+        expect(project_bar.redirect_routes.permanent.pluck(:path)).to match_array(['foo/bar'])
+      end
     end
-  end
 
-  context 'when a Group with redirected projects is renamed' do
-    let!(:group_foo) { create(:group, name: 'foo', path: 'foo') }
-    let!(:project_bar) { create(:project, path: 'bar', namespace: group_foo) }
-
-    it 'should create the appropriate redirect routes' do
-      expect(project_bar.redirect_routes.count).to eq(0)
-
-      # Renaming project
-      project_bar.path = 'baz'
-      project_bar.save
-      expect(project_bar.redirect_routes.temporary.count).to eq(1)
-      expect(project_bar.redirect_routes.temporary.first.path).to eq('foo/bar')
-
-      # Renaming group
-      group_foo.path = 'foot'
-      group_foo.save
-      expect(group_foo.redirect_routes.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.first.path).to eq('foo')
-      expect(project_bar.redirect_routes.count).to eq(2)
-      expect(project_bar.redirect_routes.temporary.count).to eq(1)
-      expect(project_bar.redirect_routes.temporary.first.path).to eq('foo/bar')
-      expect(project_bar.redirect_routes.permanent.count).to eq(1)
-      expect(project_bar.redirect_routes.permanent.first.path).to eq('foo/baz')
+    context 'when a Group with projects reclaims an old path' do
+      it 'should create the appropriate redirect routes' do
+        group_foo.path = 'foot'
+        group_foo.save
+        expect(group_foo.redirect_routes.temporary).to be_empty
+        expect(group_foo.redirect_routes.permanent.pluck(:path)).to match_array(['foo'])
+        expect(project_bar.redirect_routes.temporary).to be_empty
+        expect(project_bar.redirect_routes.permanent.pluck(:path)).to match_array(['foo/bar'])
+
+        # Back to foo
+        group_foo.path = 'foo'
+        group_foo.save
+        expect(group_foo.redirect_routes.temporary).to be_empty
+        expect(group_foo.redirect_routes.permanent.pluck(:path)).to match_array(['foot'])
+        expect(project_bar.redirect_routes.temporary).to be_empty
+        expect(project_bar.redirect_routes.permanent.pluck(:path)).to match_array(['foot/bar'])
+      end
     end
-  end
 
-  context 'when a Group with redirected projects reclaims an old path' do
-    let!(:group_foo) { create(:group, name: 'foo', path: 'foo') }
-    let!(:project_bar) { create(:project, path: 'bar', namespace: group_foo) }
+    context 'when a Group with redirected projects is renamed' do
+      it 'should create the appropriate redirect routes' do
+        # Renaming project
+        project_bar.path = 'baz'
+        project_bar.save
+        expect(project_bar.redirect_routes.temporary.pluck(:path)).to match_array(['foo/bar'])
+        expect(project_bar.redirect_routes.permanent).to be_empty
+
+        # Renaming group
+        group_foo.path = 'foot'
+        group_foo.save
+        expect(group_foo.redirect_routes.temporary).to be_empty
+        expect(group_foo.redirect_routes.permanent.pluck(:path)).to match_array(['foo'])
+        expect(project_bar.redirect_routes.temporary.pluck(:path)).to match_array(['foo/bar'])
+        expect(project_bar.redirect_routes.permanent.pluck(:path)).to match_array(['foo/baz'])
+      end
+    end
 
-    it 'should create the appropriate redirect routes' do
-      expect(project_bar.redirect_routes.temporary.count).to eq(0)
-
-      # Renaming project
-      project_bar.path = 'baz'
-      project_bar.save
-      expect(project_bar.redirect_routes.temporary.count).to eq(1)
-      expect(project_bar.redirect_routes.temporary.first.path).to eq('foo/bar')
-
-      # Renaming group
-      group_foo.path = 'foot'
-      group_foo.save
-      expect(group_foo.redirect_routes.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.first.path).to eq('foo')
-      expect(project_bar.redirect_routes.count).to eq(2)
-      expect(project_bar.redirect_routes.temporary.count).to eq(1)
-      expect(project_bar.redirect_routes.temporary.first.path).to eq('foo/bar')
-      expect(project_bar.redirect_routes.permanent.count).to eq(1)
-      expect(project_bar.redirect_routes.permanent.first.path).to eq('foo/baz')
-
-      # Back to foo
-      group_foo.path = 'foo'
-      group_foo.save
-      expect(group_foo.redirect_routes.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.first.path).to eq('foot')
-      expect(project_bar.redirect_routes.count).to eq(1)
-      expect(project_bar.redirect_routes.permanent.count).to eq(1)
-      expect(project_bar.redirect_routes.permanent.first.path).to eq('foot/baz')
+    context 'when a Group with redirected projects reclaims an old path' do
+      it 'should create the appropriate redirect routes' do
+        # Renaming project
+        project_bar.path = 'baz'
+        project_bar.save
+        expect(project_bar.redirect_routes.temporary.pluck(:path)).to match_array(['foo/bar'])
+        expect(project_bar.redirect_routes.permanent).to be_empty
+
+        # Renaming group
+        group_foo.path = 'foot'
+        group_foo.save
+        expect(group_foo.redirect_routes.temporary).to be_empty
+        expect(group_foo.redirect_routes.permanent.pluck(:path)).to match_array(['foo'])
+        expect(project_bar.redirect_routes.temporary.pluck(:path)).to match_array(['foo/bar'])
+        expect(project_bar.redirect_routes.permanent.pluck(:path)).to match_array(['foo/baz'])
+
+        # Back to foo
+        group_foo.path = 'foo'
+        group_foo.save
+        expect(group_foo.redirect_routes.temporary).to be_empty
+        expect(group_foo.redirect_routes.permanent.pluck(:path)).to match_array(['foot'])
+        expect(project_bar.redirect_routes.temporary.pluck(:path)).to match_array(['foo/bar'])
+        expect(project_bar.redirect_routes.permanent.pluck(:path)).to match_array(['foot/baz'])
+      end
     end
-  end
 
-  context 'when a Group with redirected groups is renamed' do
-    let!(:group_foo) { create(:group, name: 'foo', path: 'foo') }
-    let!(:subgroup_bar) { create(:group, name: 'bar', path: 'bar', parent: group_foo) }
-
-    it 'should create the appropriate redirecct routes' do
-      # Renaming subgroup
-      subgroup_bar.path = 'baz'
-      subgroup_bar.save
-      expect(subgroup_bar.redirect_routes.count).to eq(1)
-      expect(subgroup_bar.redirect_routes.permanent.count).to eq(1)
-      expect(subgroup_bar.redirect_routes.permanent.first.path).to eq('foo/bar')
-
-      # Renaming group
-      group_foo.path = 'qux'
-      group_foo.save
-      expect(group_foo.redirect_routes.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.first.path).to eq('foo')
-      expect(subgroup_bar.redirect_routes.count).to eq(2)
-      expect(subgroup_bar.redirect_routes.permanent.count).to eq(2)
-      expect(subgroup_bar.redirect_routes.permanent.pluck(:path)).to match_array(['foo/bar', 'foo/baz'])
+    context 'when a Group with redirected groups is renamed', :postgresql do
+      let!(:subgroup_foot) { create(:group, name: 'foot', path: 'foot', parent: group_foo) }
+
+      it 'should create the appropriate redirect routes' do
+        # Renaming subgroup
+        subgroup_foot.path = 'baz'
+        subgroup_foot.save
+        expect(subgroup_foot.redirect_routes.temporary).to be_empty
+        expect(subgroup_foot.redirect_routes.permanent.pluck(:path)).to match_array(['foo/foot'])
+
+        # Renaming group
+        group_foo.path = 'qux'
+        group_foo.save
+        expect(group_foo.redirect_routes.temporary).to be_empty
+        expect(group_foo.redirect_routes.permanent.pluck(:path)).to match_array(['foo'])
+        expect(subgroup_foot.redirect_routes.temporary).to be_empty
+        expect(subgroup_foot.redirect_routes.permanent.pluck(:path)).to match_array(['foo/foot', 'foo/baz'])
+      end
     end
-  end
 
-  context 'when a Group with redirected groups reclaims an old path' do
-    let!(:group_foo) { create(:group, name: 'foo', path: 'foo') }
-    let!(:subgroup_bar) { create(:group, name: 'bar', path: 'bar', parent: group_foo) }
-
-    it 'should create the appropriate redirect routes' do
-      # Renaming subgroup
-      subgroup_bar.path = 'baz'
-      subgroup_bar.save
-      expect(subgroup_bar.redirect_routes.count).to eq(1)
-      expect(subgroup_bar.redirect_routes.permanent.count).to eq(1)
-      expect(subgroup_bar.redirect_routes.permanent.first.path).to eq('foo/bar')
-
-      # Renaming group
-      group_foo.path = 'qux'
-      group_foo.save
-      expect(group_foo.redirect_routes.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.first.path).to eq('foo')
-      expect(subgroup_bar.redirect_routes.count).to eq(2)
-      expect(subgroup_bar.redirect_routes.permanent.count).to eq(2)
-      expect(subgroup_bar.redirect_routes.permanent.pluck(:path)).to match_array(['foo/bar', 'foo/baz'])
-
-      # Back to foo
-      group_foo.path = 'foo'
-      group_foo.save
-      expect(group_foo.redirect_routes.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.count).to eq(1)
-      expect(group_foo.redirect_routes.permanent.first.path).to eq('qux')
-      expect(subgroup_bar.redirect_routes.count).to eq(1)
-      expect(subgroup_bar.redirect_routes.permanent.count).to eq(1)
-      expect(subgroup_bar.redirect_routes.permanent.pluck(:path)).to match_array(['qux/baz'])
+    context 'when a Group with redirected groups reclaims an old path', :postgresql do
+      let!(:subgroup_foot) { create(:group, name: 'foot', path: 'foot', parent: group_foo) }
+
+      it 'should create the appropriate redirect routes' do
+        # Renaming subgroup
+        subgroup_foot.path = 'baz'
+        subgroup_foot.save
+        expect(subgroup_foot.redirect_routes.temporary).to be_empty
+        expect(subgroup_foot.redirect_routes.permanent.pluck(:path)).to match_array(['foo/foot'])
+
+        # Renaming group
+        group_foo.path = 'qux'
+        group_foo.save
+        expect(group_foo.redirect_routes.temporary).to be_empty
+        expect(group_foo.redirect_routes.permanent.pluck(:path)).to match_array(['foo'])
+        expect(subgroup_foot.redirect_routes.temporary).to be_empty
+        expect(subgroup_foot.redirect_routes.permanent.pluck(:path)).to match_array(['foo/foot', 'foo/baz'])
+
+        # Back to foo
+        group_foo.path = 'foo'
+        group_foo.save
+        expect(group_foo.redirect_routes.temporary).to be_empty
+        expect(group_foo.redirect_routes.permanent.pluck(:path)).to match_array(['qux'])
+        expect(subgroup_foot.redirect_routes.temporary).to be_empty
+        expect(subgroup_foot.redirect_routes.permanent.pluck(:path)).to match_array(['qux/baz'])
+      end
     end
   end
 
-  context 'when a User is renamed' do
+  describe 'User scenarios' do
     let!(:user_foo) { create(:user, username: 'foo') }
 
-    it 'should create the appropriate redirect routes' do
-      user_foo.username = 'foot'
-      user_foo.save
+    context 'when a User is renamed' do
+      it 'should create the appropriate redirect routes' do
+        user_foo.username = 'foot'
+        user_foo.save
 
-      expect(user_foo.namespace.redirect_routes.count).to eq(1)
-      expect(user_foo.namespace.redirect_routes.permanent.count).to eq(1)
-      expect(user_foo.namespace.redirect_routes.permanent.first.path).to eq('foo')
+        expect(user_foo.namespace.redirect_routes.temporary).to be_empty
+        expect(user_foo.namespace.redirect_routes.permanent.pluck(:path)).to match_array(['foo'])
+      end
     end
-  end
 
-  context 'when a User with projects is renamed' do
-    let!(:user_foo) { create(:user, username: 'foo') }
-    let!(:project) { create(:project, path: 'bar', namespace: user_foo.namespace) }
-
-    it 'should create the appropriate redirect routes' do
-      # Renames user
-      user_foo.username = 'foot'
-      user_foo.save
-      expect(user_foo.namespace.redirect_routes.count).to eq(1)
-      expect(user_foo.namespace.redirect_routes.permanent.count).to eq(1)
-      expect(user_foo.namespace.redirect_routes.permanent.first.path).to eq('foo')
-      expect(project.redirect_routes.count).to eq(1)
-      expect(project.redirect_routes.permanent.count).to eq(1)
-      expect(project.redirect_routes.permanent.first.path).to eq('foo/bar')
+    context 'when a User with projects is renamed' do
+      let!(:project) { create(:project, path: 'bar', namespace: user_foo.namespace) }
+
+      it 'should create the appropriate redirect routes' do
+        # Renames user
+        user_foo.username = 'foot'
+        user_foo.save
+        expect(user_foo.namespace.redirect_routes.temporary).to be_empty
+        expect(user_foo.namespace.redirect_routes.permanent.pluck(:path)).to match_array(['foo'])
+        expect(project.redirect_routes.temporary).to be_empty
+        expect(project.redirect_routes.permanent.pluck(:path)).to match_array(['foo/bar'])
+      end
     end
-  end
 
-  context 'when a User with projects reclaims an old path' do
-    let!(:user_foo) { create(:user, username: 'foo') }
-    let!(:project) { create(:project, path: 'bar', namespace: user_foo.namespace) }
-
-    it 'should create the appropriate redirect routes' do
-      # Renames user
-      user_foo.username = 'foot'
-      user_foo.save
-      expect(user_foo.namespace.redirect_routes.count).to eq(1)
-      expect(user_foo.namespace.redirect_routes.permanent.count).to eq(1)
-      expect(user_foo.namespace.redirect_routes.permanent.first.path).to eq('foo')
-      expect(project.redirect_routes.count).to eq(1)
-      expect(project.redirect_routes.permanent.count).to eq(1)
-      expect(project.redirect_routes.permanent.first.path).to eq('foo/bar')
-
-      # Back to foo
-      user_foo.username = 'foo'
-      user_foo.save
-      expect(user_foo.namespace.redirect_routes.count).to eq(1)
-      expect(user_foo.namespace.redirect_routes.permanent.first.path).to eq('foot')
-      expect(project.redirect_routes.count).to eq(1)
-      expect(project.redirect_routes.permanent.count).to eq(1)
-      expect(project.redirect_routes.permanent.first.path).to eq('foot/bar')
+    context 'when a User with projects reclaims an old path' do
+      let!(:project) { create(:project, path: 'bar', namespace: user_foo.namespace) }
+
+      it 'should create the appropriate redirect routes' do
+        # Renames user
+        user_foo.username = 'foot'
+        user_foo.save
+        expect(user_foo.namespace.redirect_routes.temporary).to be_empty
+        expect(user_foo.namespace.redirect_routes.permanent.pluck(:path)).to match_array(['foo'])
+        expect(project.redirect_routes.temporary).to be_empty
+        expect(project.redirect_routes.permanent.pluck(:path)).to match_array(['foo/bar'])
+
+        # Back to foo
+        user_foo.username = 'foo'
+        user_foo.save
+        expect(user_foo.namespace.redirect_routes.temporary).to be_empty
+        expect(user_foo.namespace.redirect_routes.permanent.pluck(:path)).to match_array(['foot'])
+        expect(project.redirect_routes.temporary).to be_empty
+        expect(project.redirect_routes.permanent.pluck(:path)).to match_array(['foot/bar'])
+      end
     end
-  end
 
-  context 'when a User with redirected projects is renamed' do
-    let!(:user_foo) { create(:user, username: 'foo') }
-    let!(:project) { create(:project, path: 'bar', namespace: user_foo.namespace) }
-
-    it 'should create the appropriate redirect routes' do
-      expect(project.redirect_routes.temporary.count).to eq(0)
-
-      # Renaming project
-      project.path = 'baz'
-      project.save
-      expect(project.redirect_routes.count).to eq(1)
-      expect(project.redirect_routes.temporary.count).to eq(1)
-      expect(project.redirect_routes.temporary.first.path).to eq('foo/bar')
-
-      # Renaming user
-      user_foo.username = 'foot'
-      user_foo.save
-      expect(user_foo.namespace.redirect_routes.count).to eq(1)
-      expect(user_foo.namespace.redirect_routes.permanent.count).to eq(1)
-      expect(user_foo.namespace.redirect_routes.permanent.first.path).to eq('foo')
-      expect(project.redirect_routes.count).to eq(2)
-      expect(project.redirect_routes.temporary.count).to eq(1)
-      expect(project.redirect_routes.temporary.first.path).to eq('foo/bar')
-      expect(project.redirect_routes.permanent.count).to eq(1)
-      expect(project.redirect_routes.permanent.first.path).to eq('foo/baz')
+    context 'when a User with redirected projects is renamed' do
+      let!(:project) { create(:project, path: 'bar', namespace: user_foo.namespace) }
+
+      it 'should create the appropriate redirect routes' do
+        # Renaming project
+        project.path = 'baz'
+        project.save
+        expect(project.redirect_routes.temporary.pluck(:path)).to match_array(['foo/bar'])
+        expect(project.redirect_routes.permanent).to be_empty
+
+        # Renaming user
+        user_foo.username = 'foot'
+        user_foo.save
+        expect(user_foo.namespace.redirect_routes.temporary).to be_empty
+        expect(user_foo.namespace.redirect_routes.permanent.pluck(:path)).to match_array(['foo'])
+        expect(project.redirect_routes.temporary.pluck(:path)).to match_array(['foo/bar'])
+        expect(project.redirect_routes.permanent.pluck(:path)).to match_array(['foo/baz'])
+      end
     end
-  end
 
-  context 'when a User with redirected projects reclaims an old path' do
-    let!(:user_foo) { create(:user, username: 'foo') }
-    let!(:project) { create(:project, path: 'bar', namespace: user_foo.namespace) }
-
-    it 'should create the appropriate redirect routes' do
-      expect(project.redirect_routes.temporary.count).to eq(0)
-
-      # Renaming project
-      project.path = 'baz'
-      project.save
-      expect(project.redirect_routes.count).to eq(1)
-      expect(project.redirect_routes.temporary.count).to eq(1)
-      expect(project.redirect_routes.temporary.first.path).to eq('foo/bar')
-
-      # Renaming user
-      user_foo.username = 'foot'
-      user_foo.save
-      expect(user_foo.namespace.redirect_routes.count).to eq(1)
-      expect(user_foo.namespace.redirect_routes.permanent.count).to eq(1)
-      expect(user_foo.namespace.redirect_routes.permanent.first.path).to eq('foo')
-      expect(project.redirect_routes.count).to eq(2)
-      expect(project.redirect_routes.temporary.count).to eq(1)
-      expect(project.redirect_routes.temporary.first.path).to eq('foo/bar')
-      expect(project.redirect_routes.permanent.count).to eq(1)
-      expect(project.redirect_routes.permanent.first.path).to eq('foo/baz')
-
-      # Back to foo
-      user_foo.username = 'foo'
-      user_foo.save
-      expect(user_foo.namespace.redirect_routes.count).to eq(1)
-      expect(user_foo.namespace.redirect_routes.permanent.count).to eq(1)
-      expect(user_foo.namespace.redirect_routes.permanent.first.path).to eq('foot')
-      expect(project.redirect_routes.count).to eq(1)
-      expect(project.redirect_routes.permanent.count).to eq(1)
-      expect(project.redirect_routes.permanent.first.path).to eq('foot/baz')
+    context 'when a User with redirected projects reclaims an old path' do
+      let!(:project) { create(:project, path: 'bar', namespace: user_foo.namespace) }
+
+      it 'should create the appropriate redirect routes' do
+        # Renaming project
+        project.path = 'baz'
+        project.save
+        expect(project.redirect_routes.temporary.pluck(:path)).to match_array(['foo/bar'])
+        expect(project.redirect_routes.permanent).to be_empty
+
+        # Renaming user
+        user_foo.username = 'foot'
+        user_foo.save
+        expect(user_foo.namespace.redirect_routes.temporary).to be_empty
+        expect(user_foo.namespace.redirect_routes.permanent.pluck(:path)).to match_array(['foo'])
+        expect(project.redirect_routes.temporary.pluck(:path)).to match_array(['foo/bar'])
+        expect(project.redirect_routes.permanent.pluck(:path)).to match_array(['foo/baz'])
+
+        # Back to foo
+        user_foo.username = 'foo'
+        user_foo.save
+        expect(user_foo.namespace.redirect_routes.temporary).to be_empty
+        expect(user_foo.namespace.redirect_routes.permanent.pluck(:path)).to match_array(['foot'])
+        expect(project.redirect_routes.temporary.pluck(:path)).to match_array(['foo/bar'])
+        expect(project.redirect_routes.permanent.pluck(:path)).to match_array(['foot/baz'])
+      end
     end
   end
 
-  context 'when a Project is renamed' do
-    let(:project_bar) { create(:project, path: 'bar') }
-
-    it 'should create the appropriate redirect routes' do
-      expect(project_bar.redirect_routes.count).to eq(0)
+  describe 'Project scenarios' do
+    let!(:project_bar) { create(:project, path: 'bar') }
+
+    context 'when a Project is renamed' do
+      it 'should create the appropriate redirect routes' do
+        # Renaming project
+        project_bar.path = 'baz'
+        project_bar.save
+        expect(project_bar.redirect_routes.temporary.pluck(:path)).to match_array(["#{project_bar.namespace.name}/bar"])
+        expect(project_bar.redirect_routes.permanent).to be_empty
+      end
+    end
 
-      # Renaming project
-      project_bar.path = 'baz'
-      project_bar.save
-      expect(project_bar.redirect_routes.count).to eq(1)
-      expect(project_bar.redirect_routes.temporary.count).to eq(1)
-      expect(project_bar.redirect_routes.temporary.first.path).to eq("#{project_bar.namespace.name}/bar")
+    context 'when a Project is renamed multiple times' do
+      let(:namespace) { project_bar.namespace }
+
+      it 'should create the appropriate redirect routes' do
+        # Renaming project
+        project_bar.path = 'baz'
+        project_bar.save
+        expect(project_bar.redirect_routes.temporary.pluck(:path)).to match_array(["#{namespace.name}/bar"])
+        expect(project_bar.redirect_routes.permanent).to be_empty
+
+        project_bar.path = 'bax'
+        project_bar.save
+        expect(project_bar.redirect_routes.temporary.pluck(:path)).to match_array(["#{namespace.name}/bar", "#{namespace.name}/baz"])
+        expect(project_bar.redirect_routes.permanent).to be_empty
+      end
     end
-  end
 
-  context 'when a Project is renamed multiple times' do
-    let(:project_bar) { create(:project, path: 'bar') }
-    let(:namespace) { project_bar.namespace }
-
-    it 'should create the appropriate redirect routes' do
-      expect(project_bar.redirect_routes.count).to eq(0)
-
-      # Renaming project
-      project_bar.path = 'baz'
-      project_bar.save
-      expect(project_bar.redirect_routes.count).to eq(1)
-      expect(project_bar.redirect_routes.temporary.count).to eq(1)
-      expect(project_bar.redirect_routes.temporary.first.path).to eq("#{namespace.name}/bar")
-
-      project_bar.path = 'bax'
-      project_bar.save
-      expect(project_bar.redirect_routes.count).to eq(2)
-      expect(project_bar.redirect_routes.temporary.count).to eq(2)
-      expect(project_bar.redirect_routes.temporary.pluck(:path)).to match_array(["#{namespace.name}/bar", "#{namespace.name}/baz"])
+    context 'when transferring a nested Project to another Group' do
+      let!(:group_foo) { create(:group, name: 'foo', path: 'foo') }
+      let!(:project_bar) { create(:project, :repository, path: 'bar', namespace: group_foo) }
+      let!(:group_foot) { create(:group, name: 'foot', path: 'foot') }
+      let!(:user) { create(:user) }
+
+      it 'should create the appropriate redirect routes' do
+        create(:group_member, :owner, group: group_foo, user: user)
+        create(:group_member, :owner, group: group_foot, user: user)
+
+        # Transferring project
+        Projects::TransferService.new(project_bar, user).execute(group_foot)
+        expect(project_bar.namespace).to eq(group_foot)
+        expect(project_bar.redirect_routes.temporary.pluck(:path)).to match_array(['foo/bar'])
+        expect(project_bar.redirect_routes.permanent).to be_empty
+        expect(group_foo.redirect_routes).to be_empty
+        expect(group_foot.redirect_routes).to be_empty
+      end
     end
-  end
 
-  context 'when transferring a nested Project to another group' do
-    let!(:group_foo) { create(:group, name: 'foo', path: 'foo') }
-    let!(:project_bar) { create(:project, :repository, path: 'bar', namespace: group_foo) }
-    let!(:group_foot) { create(:group, name: 'foot', path: 'foot') }
-    let!(:user) { create(:user) }
-
-    it 'should create the appropriate redirect routes' do
-      create(:group_member, :owner, group: group_foo, user: user)
-      create(:group_member, :owner, group: group_foot, user: user)
-      expect(project_bar.redirect_routes.count).to eq(0)
-
-      # Transferring project
-      Projects::TransferService.new(project_bar, user).execute(group_foot)
-      expect(project_bar.namespace).to eq(group_foot)
-      expect(project_bar.redirect_routes.count).to eq(1)
-      expect(project_bar.redirect_routes.temporary.count).to eq(1)
-      expect(project_bar.redirect_routes.temporary.first.path).to eq('foo/bar')
-      expect(group_foo.redirect_routes.count).to eq(0)
-      expect(group_foot.redirect_routes.count).to eq(0)
+    context 'when transferring a nested Project to a User' do
+      let!(:group_foo) { create(:group, name: 'foo', path: 'foo') }
+      let!(:project_bar) { create(:project, :repository, path: 'bar', namespace: group_foo) }
+      let!(:user_baz) { create(:user, username: 'baz') }
+
+      it 'should create the appropriate redirect routes' do
+        create(:group_member, :owner, group: group_foo, user: user_baz)
+
+        # Transferring project
+        Projects::TransferService.new(project_bar, user_baz).execute(user_baz.namespace)
+        expect(project_bar.namespace).to eq(user_baz.namespace)
+        expect(project_bar.redirect_routes.temporary.pluck(:path)).to match_array(['foo/bar'])
+        expect(project_bar.redirect_routes.permanent).to be_empty
+        expect(group_foo.redirect_routes).to be_empty
+      end
     end
   end
 end