Merge branch...

Merge branch '224674-fix-update_routes_for_lost_and_found_group_and_orphaned_projects-conflict-with-ghost-route' into 'master'

Fix conflict on the migration that adds routes for orphaned projects

See merge request gitlab-org/gitlab!35425

changelogs/unreleased/224674-fix-update_routes_for_lost_and_found_group_and_orphaned_projects-c.yml

+---
+title: Fix path conflict for Ghost on UpdateRoutesForLostAndFoundGroupAndOrphanedProjects
+merge_request: 35425
+author:
+type: fixed

db/post_migrate/20200602143020_update_routes_for_lost_and_found_group_and_orphaned_projects.rb

@@ -136,6 +136,7 @@ class UpdateRoutesForLostAndFoundGroupAndOrphanedProjects < ActiveRecord::Migrat
     # to ensure the Active Record's knowledge of the table structure is current
     Namespace.reset_column_information
     Route.reset_column_information
+    User.reset_column_information
 
     # Find the ghost user, its namespace and the "lost and found" group
     ghost_user = User.ghost
@@ -158,6 +159,15 @@ class UpdateRoutesForLostAndFoundGroupAndOrphanedProjects < ActiveRecord::Migrat
       'More info: gitlab.com/gitlab-org/gitlab/-/issues/198603'
     lost_and_found_group.save!
 
+    # make sure that the ghost namespace has a unique path
+    ghost_namespace.generate_unique_path
+
+    if ghost_namespace.path_changed?
+      ghost_namespace.save!
+      # If the path changed, also update the Ghost User's username to match the new path.
+      ghost_user.update!(username: ghost_namespace.path)
+    end
+
     # Update the routes for the Ghost user, the "lost and found" group
     # and all the orphaned projects
     ghost_namespace.ensure_route!

spec/migrations/update_routes_for_lost_and_found_group_and_orphaned_projects_spec.rb

@@ -62,8 +62,8 @@ RSpec.describe UpdateRoutesForLostAndFoundGroupAndOrphanedProjects, :migration d
       source_type: 'Project',
       path: 'orphaned_project',
       name: 'orphaned_project',
-      created_at: Time.zone.now,
-      updated_at: Time.zone.now
+      created_at: Time.current,
+      updated_at: Time.current
     )
 
     # Create another user named ghost which is not the Ghost User
@@ -90,10 +90,10 @@ RSpec.describe UpdateRoutesForLostAndFoundGroupAndOrphanedProjects, :migration d
     routes.create!(
       source_id: fake_ghost_user_namespace.id,
       source_type: 'Namespace',
-      path: 'Ghost User',
-      name: 'ghost1',
-      created_at: Time.zone.now,
-      updated_at: Time.zone.now
+      path: 'ghost1',
+      name: 'Ghost User',
+      created_at: Time.current,
+      updated_at: Time.current
     )
 
     fake_lost_and_found_group = namespaces.create!(
@@ -109,8 +109,8 @@ RSpec.describe UpdateRoutesForLostAndFoundGroupAndOrphanedProjects, :migration d
       source_type: 'Namespace',
       path: described_class::User::LOST_AND_FOUND_GROUP, # same path as the lost-and-found group
       name: 'Lost and Found',
-      created_at: Time.zone.now,
-      updated_at: Time.zone.now
+      created_at: Time.current,
+      updated_at: Time.current
     )
 
     members.create!(
@@ -135,17 +135,58 @@ RSpec.describe UpdateRoutesForLostAndFoundGroupAndOrphanedProjects, :migration d
       source_type: 'Project',
       path: "#{described_class::User::LOST_AND_FOUND_GROUP}/normal_project",
       name: 'Lost and Found / normal_project',
-      created_at: Time.zone.now,
-      updated_at: Time.zone.now
+      created_at: Time.current,
+      updated_at: Time.current
     )
+
+    # Add a project whose route conflicts with the ghost username
+    # and should force the data migration to pick a new Ghost username and path
+    ghost_project = projects.create!(
+      name: 'Ghost Project',
+      path: 'ghost',
+      visibility_level: 20,
+      archived: false,
+      namespace_id: fake_lost_and_found_group.id
+    )
+
+    routes.create!(
+      source_id: ghost_project.id,
+      source_type: 'Project',
+      path: 'ghost',
+      name: 'Ghost Project',
+      created_at: Time.current,
+      updated_at: Time.current
+    )
+  end
+
+  it 'fixes the ghost user username and namespace path' do
+    ghost_user = users.find_by(user_type: described_class::User::USER_TYPE_GHOST)
+    ghost_namespace = namespaces.find_by(owner_id: ghost_user.id)
+
+    expect(ghost_user.username).to eq('ghost')
+    expect(ghost_namespace.path).to eq('ghost')
+
+    disable_migrations_output { migrate! }
+
+    ghost_user = users.find_by(user_type: described_class::User::USER_TYPE_GHOST)
+    ghost_namespace = namespaces.find_by(owner_id: ghost_user.id)
+    ghost_namespace_route = routes.find_by(source_id: ghost_namespace.id, source_type: 'Namespace')
+
+    expect(ghost_user.username).to eq('ghost2')
+    expect(ghost_namespace.path).to eq('ghost2')
+    expect(ghost_namespace_route.path).to eq('ghost2')
   end
 
   it 'creates the route for the ghost user namespace' do
-    expect(routes.where(path: 'ghost').count).to eq(0)
+    expect(routes.where(path: 'ghost').count).to eq(1)
+    expect(routes.where(path: 'ghost1').count).to eq(1)
+    expect(routes.where(path: 'ghost2').count).to eq(0)
 
     disable_migrations_output { migrate! }
 
     expect(routes.where(path: 'ghost').count).to eq(1)
+    expect(routes.where(path: 'ghost1').count).to eq(1)
+    expect(routes.where(path: 'ghost2').count).to eq(1)
   end
 
   it 'fixes the path for the lost-and-found group by generating a unique one' do