diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-11-21 12:17:14 +0100 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2019-07-09 18:09:45 +0200 |
commit | 5f08830090fb6d56a14bd4aaca107b99e2b0975b (patch) | |
tree | fa1f2c2a8aa36e414bb305876a19eaf75a2c88ff | |
parent | db1b15e4245547a4468ab70d337a73a40d4fc98c (diff) | |
download | gitlab-ce-5f08830090fb6d56a14bd4aaca107b99e2b0975b.tar.gz |
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.
-rw-r--r-- | app/models/user.rb | 5 | ||||
-rw-r--r-- | changelogs/unreleased/bvl-rename-routes-after-user-rename.yml | 5 | ||||
-rw-r--r-- | spec/models/concerns/routable_spec.rb | 47 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 20 |
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 |