Skip to content

AO3-6359 Fix Possible to delete the pseud that corresponds with your username if the capitalization or diacritics differ #5534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Dobe-Time wants to merge 10 commits into
base: master
Choose a base branch
from
+66 −16
Open

AO3-6359 Fix Possible to delete the pseud that corresponds with your username if the capitalization or diacritics differ #5534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
34456be
AO3-6359 Possible to delete the pseud that corresponds with your user…
Dobe-Time Dec 30, 2025
b8db8aa
Merge branch 'AO3-6359' of https://github.com/Dobe-Time/otwarchive in…
Dobe-Time Dec 30, 2025
1bfcfbe
AO3-6359 fixing extra new lines in user_spec.rb
Dobe-Time Jan 3, 2026
1a7c62d
AO3-6359 Fixing style issue for Rubocop
Dobe-Time Jan 3, 2026
1d044cc
AO3-6359 Fixing conditional statement style issue for Rubocop
Dobe-Time Jan 3, 2026
6688ebc
Merge branch 'otwcode:master' into AO3-6359
Dobe-Time Jan 18, 2026
b6ec0e4
Merge branch 'otwcode:master' into AO3-6359
Dobe-Time Jan 26, 2026
074d320
Merge branch 'otwcode:master' into AO3-6359
Dobe-Time Feb 3, 2026
cad132d
AO3-6359: Additional pseud testing and user.rb fixes
TobyS-CS Feb 3, 2026
1d6da05
AO3-6359: RuboCop fixes for extra space and array creation.
TobyS-CS Feb 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 14 additions & 16 deletions app/models/user.rb
View file
Open in desktop
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

@Bilka2 Bilka2 Jan 27, 2026

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

@Dobe-Time Dobe-Time Feb 3, 2026

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

@Bilka2 Bilka2 Feb 3, 2026

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

@Dobe-Time Dobe-Time Feb 4, 2026

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
View file
Open in desktop
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