Set the name of a user-namespace to the user name

Instead of setting the name of the namespace to the user's username,
set it to the user's name.

This is more consistent with how we name the routes:
The route-name of a namespace is the human name of the routable. In
the case of a user-namespace, this is the owner's name.

When we change a user's name (both on create and update), we now also
update the namespace-name to the user's name. This will make sure that
if we also correctly update all the nested routes.

4 files changed, 57 insertions, 20 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index 26be197209a..02637b70f03 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1117,9 +1117,10 @@ class User < ApplicationRecord
 
   def ensure_namespace_correct
     if namespace
-      namespace.path = namespace.name = username if username_changed?
+      namespace.path = username if username_changed?
+      namespace.name = name if name_changed?
     else
-      build_namespace(path: username, name: username)
+      build_namespace(path: username, name: name)
     end
   end
 
diff --git a/changelogs/unreleased/bvl-rename-routes-after-user-rename.yml b/changelogs/unreleased/bvl-rename-routes-after-user-rename.yml
new file mode 100644
index 00000000000..8b0a01dea53
--- /dev/null
+++ b/changelogs/unreleased/bvl-rename-routes-after-user-rename.yml
@@ -0,0 +1,5 @@
+---
+title: Update a user's routes after updating their name
+merge_request: 23272
+author:
+type: fixed
diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb
index 1fb0dd5030c..31163a5bb5c 100644
--- a/spec/models/concerns/routable_spec.rb
+++ b/spec/models/concerns/routable_spec.rb
@@ -15,23 +15,46 @@ describe Group, 'Routable' do
   end
 
   describe 'Callbacks' do
-    it 'creates route record on create' do
-      expect(group.route.path).to eq(group.path)
-      expect(group.route.name).to eq(group.name)
-    end
+    context 'for a group' do
+      it 'creates route record on create' do
+        expect(group.route.path).to eq(group.path)
+        expect(group.route.name).to eq(group.name)
+      end
+
+      it 'updates route record on path change' do
+        group.update(path: 'wow', name: 'much')
 
-    it 'updates route record on path change' do
-      group.update(path: 'wow', name: 'much')
+        expect(group.route.path).to eq('wow')
+        expect(group.route.name).to eq('much')
+      end
+
+      it 'ensure route path uniqueness across different objects' do
+        create(:group, parent: group, path: 'xyz')
+        duplicate = build(:project, namespace: group, path: 'xyz')
 
-      expect(group.route.path).to eq('wow')
-      expect(group.route.name).to eq('much')
+        expect { duplicate.save! }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Path has already been taken')
+      end
     end
 
-    it 'ensure route path uniqueness across different objects' do
-      create(:group, parent: group, path: 'xyz')
-      duplicate = build(:project, namespace: group, path: 'xyz')
+    context 'for a user' do
+      let(:user) { create(:user, username: 'jane', name: "Jane Doe") }
 
-      expect { duplicate.save! }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Path has already been taken')
+      it 'creates the route for a record on create' do
+        expect(user.namespace.name).to eq('Jane Doe')
+        expect(user.namespace.path).to eq('jane')
+      end
+
+      it 'updates routes and nested routes on name change' do
+        project = create(:project, path: 'work-stuff', name: 'Work stuff', namespace: user.namespace)
+
+        user.update!(username: 'jaen', name: 'Jaen Did')
+        project.reload
+
+        expect(user.namespace.name).to eq('Jaen Did')
+        expect(user.namespace.path).to eq('jaen')
+        expect(project.full_name).to eq('Jaen Did / Work stuff')
+        expect(project.full_path).to eq('jaen/work-stuff')
+      end
     end
   end
 
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index a4d177da0be..e4f84172215 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -2931,7 +2931,7 @@ describe User do
       let(:user) { create(:user, username: username) }
 
       context 'when the user is updated' do
-        context 'when the username is changed' do
+        context 'when the username or name is changed' do
           let(:new_username) { 'bar' }
 
           it 'changes the namespace (just to compare to when username is not changed)' do
@@ -2942,16 +2942,24 @@ describe User do
             end.to change { user.namespace.updated_at }
           end
 
-          it 'updates the namespace name' do
+          it 'updates the namespace path when the username was changed' do
             user.update!(username: new_username)
 
-            expect(user.namespace.name).to eq(new_username)
+            expect(user.namespace.path).to eq(new_username)
           end
 
-          it 'updates the namespace path' do
-            user.update!(username: new_username)
+          it 'updates the namespace name if the name was changed' do
+            user.update!(name: 'New name')
 
-            expect(user.namespace.path).to eq(new_username)
+            expect(user.namespace.name).to eq('New name')
+          end
+
+          it 'updates nested routes for the namespace if the name was changed' do
+            project = create(:project, namespace: user.namespace)
+
+            user.update!(name: 'New name')
+
+            expect(project.route.reload.name).to include('New name')
           end
 
           context 'when there is a validation error (namespace name taken) while updating namespace' do