Skip to content
Open
30 changes: 14 additions & 16 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -548,25 +548,23 @@ def self.fetch_orphan_account

def update_pseud_name
return unless saved_change_to_login? && login_before_last_save.present?

old_pseud = pseuds.where(name: login_before_last_save).first
if login.downcase == login_before_last_save.downcase
old_pseud.name = login
old_pseud.save!
else

pseud_to_update = pseuds.where(name: login_before_last_save).first
# If the new login is (case insensitive) different from the old login
if login.downcase != login_before_last_save.downcase
new_pseud = pseuds.where(name: login).first
# do nothing if they already have the matching pseud
if new_pseud.blank?
if old_pseud.present?
# change the old pseud to match
old_pseud.name = login
old_pseud.save!(validate: false)
else
# shouldn't be able to get here, but just in case
Pseud.create!(name: login, user_id: id)
end
# If the user does have an existing pseud for the new login
if new_pseud.present?
pseud_to_update = new_pseud
# If the pseud for the old login doesn't exist
elsif pseud_to_update.blank?
# shouldn't be able to get here, but just in case
Pseud.create!(name: login, user_id: id)
return
end
end
pseud_to_update.name = login
pseud_to_update.save!(validate: justification_enabled?)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can see the Justifiable module already disables its validation is justification is disabled, so we shouldn't need this special case here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is disabled here: https://github.com/otwcode/otwarchive/blob/master/app/models/user.rb#L538-L540
Without justification_enabled the test cases where an Admin updates the user name fails for

     Failure/Error: pseud_to_update.save!

     ActiveRecord::RecordInvalid:
       Validation failed: Ticket ID can't be blank, Ticket ID may begin with an # and otherwise contain only numbers.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which test case fails?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 3

rspec ./spec/models/user_spec.rb:226 # User#update when logged in as an admin when username is changed only sets admin_renamed_at
rspec ./spec/models/user_spec.rb:233 # User#update when logged in as an admin when username is changed creates an admin log item
rspec ./spec/models/user_spec.rb:249 # User#update when logged in as an admin username was recently changed does not prevent changing the username

All for this reason

  3) User#update when logged in as an admin username was recently changed does not prevent changing the username
     Failure/Error: pseud_to_update.save!

     ActiveRecord::RecordInvalid:
       Validation failed: Ticket ID can't be blank, Ticket ID may begin with an # and otherwise contain only numbers.

end

def reindex_user_creations_after_rename
Expand Down
52 changes: 52 additions & 0 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,58 @@
expect(log_item.note).to eq("Change made by #{admin.login}")
end
end

context "username was changed to match an existing pseud's name except downcased" do
let(:new_pseud) { build(:pseud, name: "New_Usernamé") }

before do
existing_user.pseuds << new_pseud
existing_user.update!(login: "new_username")
existing_user.reload
end

it "pseud's capitalization and diacritics were changed to match the new username's" do
expect(existing_user.pseuds.size).to eq(2)
expect(existing_user.pseuds.second.name).to eq(existing_user.login)
expect(existing_user.pseuds.second.name).to eq("new_username")
expect(existing_user.login).to eq("new_username")
end
end

context "username was changed to match an existing pseud's name with mixed capitalization and diacritics" do
let(:new_pseud) { build(:pseud, name: "New_Usernamé") }

before do
existing_user.pseuds << new_pseud
existing_user.update!(login: "new_UsernaMe")
existing_user.reload
end

it "pseud's capitalization and diacritics were changed to match the new username's" do
expect(existing_user.pseuds.size).to eq(2)
expect(existing_user.pseuds.second.name).to eq(existing_user.login)
expect(existing_user.pseuds.second.name).to eq("new_UsernaMe")
expect(existing_user.login).to eq("new_UsernaMe")
end
end

context "username was changed to not match an existing pseud's name, and the users has no psueds" do
let(:new_pseud) { build(:pseud, name: "New_Usernamé") }

before do
existing_user.pseuds = []

existing_user.update!(login: "new_UsernaMe")
existing_user.reload
end

it "creates a new pseud for the users new login" do
expect(existing_user.pseuds.size).to eq(1)
expect(existing_user.pseuds.first.name).to eq(existing_user.login)
expect(existing_user.pseuds.first.name).to eq("new_UsernaMe")
expect(existing_user.login).to eq("new_UsernaMe")
end
end
end

describe ".search_multiple_by_email" do
Expand Down
Loading