Skip to content

Allow username capitalization change #12438

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
godkama wants to merge 2 commits into
base: master
Choose a base branch
from
+18 −5
Open

Allow username capitalization change #12438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8bd40b4
Update ChangeUsername.php
godkama Sep 30, 2025
7830ba2
Update UsersController.php
godkama Sep 30, 2025
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
6 changes: 5 additions & 1 deletion app/Http/Controllers/UsersController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,11 @@ public function checkUsernameAvailability()

$available = $errors->isEmpty();
$message = $available ? "Username '".e($username)."' is available!" : $errors->toSentence();
$cost = $available ? Auth::user()->usernameChangeCost() : 0;
$isCapitalizationOnly = strcasecmp($username, Auth::user()->username) === 0 && $username !== Auth::user()->username;
Copy link
Copy Markdown
Contributor

@LiquidPL LiquidPL Feb 14, 2026

Choose a reason for hiding this comment

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

this check should probably be put into helpers.php and used both here and in ChangeUsername

$cost = 0;
if ($available) {
$cost = $isCapitalizationOnly ? 0 : Auth::user()->usernameChangeCost();
}
Comment on lines +136 to +139
Copy link
Copy Markdown
Contributor

@LiquidPL LiquidPL Feb 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

you could do it this way instead and it'd be easier to read, especially since we don't have to care about the availability checks if the username has the same capitalization

Suggested change
$cost = 0;
if ($available) {
$cost = $isCapitalizationOnly ? 0 : Auth::user()->usernameChangeCost();
}
if ($isCapitalizationOnly) {
$cost = 0;
else {
$cost = $available ? Auth::user()->usernameChangeCost() : 0;
}


return [
'username' => Request::input('username'),
Expand Down
17 changes: 13 additions & 4 deletions app/Libraries/ChangeUsername.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ class ChangeUsername

protected $type;

/** @var User */
protected $user;
Copy link
Copy Markdown
Contributor

@LiquidPL LiquidPL Feb 14, 2026

Choose a reason for hiding this comment

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

why was this removed? tbh with modern php it could just be:

Suggested change
protected $user;
protected User $user;


protected $username;
Expand All @@ -41,6 +40,7 @@ public function __construct(User $user, string $newUsername, string $type = 'pai
public function validate(): ValidationErrors
{
$this->validationErrors()->reset();

if ($this->user->user_id <= 1) {
return $this->validationErrors()->addTranslated('user_id', 'This user cannot be renamed');
}
Expand All @@ -53,7 +53,8 @@ public function validate(): ValidationErrors
return $this->validationErrors()->addTranslated('username', static::requireSupportedMessage());
}

if (User::cleanUsername($this->username) === $this->user->username_clean) {
// Block if new username is exactly the same as current (case-sensitive match)
if (strcasecmp($this->username, $this->user->username) === 0 && $this->username === $this->user->username) {
Copy link
Copy Markdown

@sylphi sylphi Oct 7, 2025

Choose a reason for hiding this comment

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

I wonder why not just compare them. If It blocks when they are exactly the same, there doesn't seem to be any need to do a case-insensitive comparison?

Copy link
Copy Markdown
Author

@godkama godkama Oct 16, 2025

Choose a reason for hiding this comment

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

the cleanUsername strips all possibilities of case comparison

Copy link
Copy Markdown
Contributor

@LiquidPL LiquidPL Feb 14, 2026

Choose a reason for hiding this comment

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

wouldn't a regular string comparison (aka the second check) be sufficient in this case?

return $this->validationErrors()->add('username', '.change_username.username_is_same');
}

Expand All @@ -72,9 +73,17 @@ public function validationErrorsTranslationPrefix(): string
{
return 'user';
}

private function hasExtraValidations()
private function hasExtraValidations(): bool
{
if ($this->isCapitalizationOnlyChange()) {
return false;
}

Comment on lines +78 to +81
Copy link
Copy Markdown
Contributor

@LiquidPL LiquidPL Feb 14, 2026

Choose a reason for hiding this comment

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

Putting this here will allow restricted users to change the capitalization on their usernames, which shouldn't be the case. isCapitalizationOnlyChange should only bypass the supporter check if anything.

return !in_array($this->type, static::LESS_VALIDATION_TYPES, true);
}
private function isCapitalizationOnlyChange(): bool
{
return strcasecmp($this->username, $this->user->username) === 0
&& $this->username !== $this->user->username;
}
}