From 6c00463d0aada0e3ba5e9fa7ebf2b6e8569f30a4 Mon Sep 17 00:00:00 2001 From: Wes Copeland Date: Fri, 24 Jan 2025 19:31:24 -0500 Subject: [PATCH] feat: allow users to change their names (#3071) --- .../Actions/ApproveNewDisplayNameAction.php | 28 +++ .../Controllers/UserSettingsController.php | 28 +++ .../Data/StoreUsernameChangeData.php | 23 ++ .../Data/UserSettingsPagePropsData.php | 1 + .../Requests/StoreUsernameChangeRequest.php | 18 ++ app/Community/RouteServiceProvider.php | 3 + app/Data/UserData.php | 2 + app/Data/UserPermissionsData.php | 2 + .../Resources/UserUsernameResource.php | 187 +++++++++++++++ .../UserUsernameResource/Pages/Index.php | 20 ++ app/Helpers/util/mail.php | 42 ++++ app/Http/Middleware/HandleInertiaRequests.php | 1 + app/Models/UserUsername.php | 102 ++++++++ app/Policies/UserUsernamePolicy.php | 82 +++++++ app/Support/Rules/ValidNewUsername.php | 53 +++++ ..._17_000000_update_user_usernames_table.php | 41 ++++ lang/en_US.json | 18 ++ public/request/auth/register.php | 11 +- .../components/+root/SettingsRoot.test.tsx | 60 +++++ .../components/+root/SettingsRoot.tsx | 2 + .../ChangeUsernameSectionCard.test.tsx | 219 ++++++++++++++++++ .../ChangeUsernameSectionCard.tsx | 159 +++++++++++++ .../ChangeUsernameSectionCard/index.ts | 1 + .../useChangeUsernameForm.ts | 94 ++++++++ .../DeleteAccountSectionCard.tsx | 2 +- .../SectionFormCard/SectionFormCard.test.tsx | 24 ++ .../SectionFormCard/SectionFormCard.tsx | 28 ++- .../features/settings/state/settings.atoms.ts | 3 + resources/js/pages/settings.tsx | 10 + resources/js/types/generated.d.ts | 3 + resources/js/ziggy.d.ts | 1 + .../UserSettingsControllerTest.php | 27 +++ 32 files changed, 1273 insertions(+), 22 deletions(-) create mode 100644 app/Community/Actions/ApproveNewDisplayNameAction.php create mode 100644 app/Community/Data/StoreUsernameChangeData.php create mode 100644 app/Community/Requests/StoreUsernameChangeRequest.php create mode 100644 app/Filament/Resources/UserUsernameResource.php create mode 100644 app/Filament/Resources/UserUsernameResource/Pages/Index.php create mode 100644 app/Models/UserUsername.php create mode 100644 app/Policies/UserUsernamePolicy.php create mode 100644 app/Support/Rules/ValidNewUsername.php create mode 100644 database/migrations/2025_01_17_000000_update_user_usernames_table.php create mode 100644 resources/js/features/settings/components/ChangeUsernameSectionCard/ChangeUsernameSectionCard.test.tsx create mode 100644 resources/js/features/settings/components/ChangeUsernameSectionCard/ChangeUsernameSectionCard.tsx create mode 100644 resources/js/features/settings/components/ChangeUsernameSectionCard/index.ts create mode 100644 resources/js/features/settings/components/ChangeUsernameSectionCard/useChangeUsernameForm.ts create mode 100644 resources/js/features/settings/state/settings.atoms.ts diff --git a/app/Community/Actions/ApproveNewDisplayNameAction.php b/app/Community/Actions/ApproveNewDisplayNameAction.php new file mode 100644 index 0000000000..bf13ac9b79 --- /dev/null +++ b/app/Community/Actions/ApproveNewDisplayNameAction.php @@ -0,0 +1,28 @@ +username) + ->where('id', '!=', $changeRequest->id) + ->whereNull('approved_at') + ->whereNull('denied_at') + ->update(['denied_at' => now()]); + + $changeRequest->update(['approved_at' => now()]); + + $user->display_name = $changeRequest->username; + $user->save(); + + sendDisplayNameChangeConfirmationEmail($user, $changeRequest->username); + } +} diff --git a/app/Community/Controllers/UserSettingsController.php b/app/Community/Controllers/UserSettingsController.php index d27d48a338..afd0bf827f 100644 --- a/app/Community/Controllers/UserSettingsController.php +++ b/app/Community/Controllers/UserSettingsController.php @@ -4,6 +4,7 @@ namespace App\Community\Controllers; +use App\Community\Data\StoreUsernameChangeData; use App\Community\Data\UpdateEmailData; use App\Community\Data\UpdateLocaleData; use App\Community\Data\UpdatePasswordData; @@ -13,6 +14,7 @@ use App\Community\Enums\ArticleType; use App\Community\Requests\ResetConnectApiKeyRequest; use App\Community\Requests\ResetWebApiKeyRequest; +use App\Community\Requests\StoreUsernameChangeRequest; use App\Community\Requests\UpdateEmailRequest; use App\Community\Requests\UpdateLocaleRequest; use App\Community\Requests\UpdatePasswordRequest; @@ -26,6 +28,7 @@ use App\Http\Controller; use App\Models\Role; use App\Models\User; +use App\Models\UserUsername; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Collection; @@ -56,11 +59,18 @@ public function show(): InertiaResponse ); $can = UserPermissionsData::fromUser($user)->include( + 'createUsernameChangeRequest', 'manipulateApiKeys', 'updateAvatar', 'updateMotto' ); + $requestedUsername = UserUsername::whereUserId($user->id) + ->pending() + ->latest('created_at') + ->first() + ?->username; + /** @var Collection $displayableRoles */ $displayableRoles = $user->roles; @@ -72,11 +82,29 @@ public function show(): InertiaResponse $userSettings, $can, $mappedRoles, + $requestedUsername ); return Inertia::render('settings', $props); } + public function storeUsernameChangeRequest(StoreUsernameChangeRequest $request): JsonResponse + { + $this->authorize('create', UserUsername::class); + + $data = StoreUsernameChangeData::fromRequest($request); + + /** @var User $user */ + $user = $request->user(); + + UserUsername::create([ + 'user_id' => $user->id, + 'username' => $data->newDisplayName, + ]); + + return response()->json(['success' => true]); + } + public function updatePassword(UpdatePasswordRequest $request): JsonResponse { $data = UpdatePasswordData::fromRequest($request); diff --git a/app/Community/Data/StoreUsernameChangeData.php b/app/Community/Data/StoreUsernameChangeData.php new file mode 100644 index 0000000000..9cdf1a1e0f --- /dev/null +++ b/app/Community/Data/StoreUsernameChangeData.php @@ -0,0 +1,23 @@ +newDisplayName, + ); + } +} diff --git a/app/Community/Data/UserSettingsPagePropsData.php b/app/Community/Data/UserSettingsPagePropsData.php index 4b613b1a83..dd6c7dc876 100644 --- a/app/Community/Data/UserSettingsPagePropsData.php +++ b/app/Community/Data/UserSettingsPagePropsData.php @@ -18,6 +18,7 @@ public function __construct( public UserPermissionsData $can, /** @var RoleData[] */ public array $displayableRoles, + public ?string $requestedUsername = null, ) { } } diff --git a/app/Community/Requests/StoreUsernameChangeRequest.php b/app/Community/Requests/StoreUsernameChangeRequest.php new file mode 100644 index 0000000000..604a1b8b7b --- /dev/null +++ b/app/Community/Requests/StoreUsernameChangeRequest.php @@ -0,0 +1,18 @@ + ValidNewUsername::get($this->user()), + ]; + } +} diff --git a/app/Community/RouteServiceProvider.php b/app/Community/RouteServiceProvider.php index a929371842..363e9085a7 100755 --- a/app/Community/RouteServiceProvider.php +++ b/app/Community/RouteServiceProvider.php @@ -373,6 +373,9 @@ protected function mapWebRoutes(): void Route::put('password', [UserSettingsController::class, 'updatePassword'])->name('api.settings.password.update'); Route::put('email', [UserSettingsController::class, 'updateEmail'])->name('api.settings.email.update'); + Route::post('username-change-request', [UserSettingsController::class, 'storeUsernameChangeRequest']) + ->name('api.settings.username-change-request.store'); + Route::delete('keys/web', [UserSettingsController::class, 'resetWebApiKey'])->name('api.settings.keys.web.destroy'); Route::delete('keys/connect', [UserSettingsController::class, 'resetConnectApiKey'])->name('api.settings.keys.connect.destroy'); }); diff --git a/app/Data/UserData.php b/app/Data/UserData.php index 399fd358c0..de7afa41e4 100644 --- a/app/Data/UserData.php +++ b/app/Data/UserData.php @@ -27,6 +27,7 @@ public function __construct( public Lazy|array|null $displayableRoles = null, public Lazy|string|null $emailAddress = null, public Lazy|int $id = 0, + public Lazy|bool $isEmailVerified = false, public Lazy|bool $isMuted = false, public Lazy|bool $isNew = false, public Lazy|int|null $legacyPermissions = null, @@ -78,6 +79,7 @@ public static function fromUser(User $user): self emailAddress: Lazy::create(fn () => $user->EmailAddress), mutedUntil: Lazy::create(fn () => $user->muted_until), id: Lazy::create(fn () => $user->id), + isEmailVerified: Lazy::create(fn () => $user->isEmailVerified()), isMuted: Lazy::create(fn () => $user->isMuted()), isNew: Lazy::create(fn () => $user->isNew()), legacyPermissions: Lazy::create(fn () => (int) $user->getAttribute('Permissions')), diff --git a/app/Data/UserPermissionsData.php b/app/Data/UserPermissionsData.php index 6f4c5d91e5..3e07c0ead6 100644 --- a/app/Data/UserPermissionsData.php +++ b/app/Data/UserPermissionsData.php @@ -16,6 +16,7 @@ class UserPermissionsData extends Data { public function __construct( public Lazy|bool $createTriggerTicket, + public Lazy|bool $createUsernameChangeRequest, public Lazy|bool $develop, public Lazy|bool $manageGameHashes, public Lazy|bool $manageGameSets, @@ -35,6 +36,7 @@ public static function fromUser( ? $user->can('createFor', [\App\Models\TriggerTicket::class, $triggerable]) : $user?->can('create', \App\Models\TriggerTicket::class) ?? false ), + createUsernameChangeRequest: Lazy::create(fn () => $user ? $user->can('create', \App\Models\UserUsername::class) : false), develop: Lazy::create(fn () => $user ? $user->can('develop') : false), manageGameHashes: Lazy::create(fn () => $user ? $user->can('manage', \App\Models\GameHash::class) : false), manageGameSets: Lazy::create(fn () => $user ? $user->can('manage', \App\Models\GameSet::class) : false), diff --git a/app/Filament/Resources/UserUsernameResource.php b/app/Filament/Resources/UserUsernameResource.php new file mode 100644 index 0000000000..d29be3dd9e --- /dev/null +++ b/app/Filament/Resources/UserUsernameResource.php @@ -0,0 +1,187 @@ +count(); + + return "{$count}"; + } + + public static function getNavigationBadgeColor(): ?string + { + return static::getNavigationBadge() > 0 ? 'warning' : null; + } + + public static function form(Form $form): Form + { + return $form + ->schema([ + + ]); + } + + public static function table(Table $table): Table + { + return $table + ->modifyQueryUsing(fn (Builder $query) => $query->where(function ($query) { + $query->whereNotNull('approved_at') + ->orWhereNotNull('denied_at') + ->orWhere('created_at', '>', now()->subDays(30)); + })) + ->columns([ + Tables\Columns\TextColumn::make('user.username') + ->label('Original Username') + ->url(fn (UserUsername $record) => UserResource::getUrl('view', ['record' => $record->user->display_name])) + ->extraAttributes(['class' => 'underline']) + ->openUrlInNewTab() + ->searchable() + ->sortable(), + + Tables\Columns\TextColumn::make('user.display_name') + ->label('Current Username') + ->url(fn (UserUsername $record) => UserResource::getUrl('view', ['record' => $record->user->display_name])) + ->extraAttributes(['class' => 'underline']) + ->openUrlInNewTab() + ->searchable() + ->sortable(), + + Tables\Columns\TextColumn::make('username') + ->label('Requested New Username') + ->searchable() + ->sortable(), + + Tables\Columns\TextColumn::make('created_at') + ->label('Requested At') + ->dateTime() + ->sortable(), + + Tables\Columns\TextColumn::make('status') + ->label('Status') + ->state(fn (UserUsername $record): string => match (true) { + $record->is_approved => 'Approved', + $record->is_denied => 'Denied', + default => 'Pending', + }) + ->icon(fn (UserUsername $record): string => match (true) { + $record->is_approved => 'heroicon-o-check-circle', + $record->is_denied => 'heroicon-o-x-circle', + default => 'heroicon-o-clock', + }) + ->color(fn (UserUsername $record): string => match (true) { + $record->is_approved => 'success', + $record->is_denied => 'danger', + default => 'warning', + }), + ]) + ->defaultSort('created_at', 'desc') + ->filters([ + Tables\Filters\SelectFilter::make('status') + ->options([ + 'pending' => 'Pending', + 'approved' => 'Approved', + 'denied' => 'Denied', + ]) + ->query(function ($query, $state) { + if (!isset($state['value'])) { + return $query; + } + + return match ($state['value']) { + 'pending' => $query->pending(), + 'approved' => $query->approved(), + 'denied' => $query->denied(), + default => $query, + }; + }) + ->default('pending'), + ]) + ->actions([ + Tables\Actions\Action::make('approve') + ->action(function (UserUsername $record) { + /** @var User $user */ + $user = $record->user; + $originalDisplayName = $user->display_name; + + (new ApproveNewDisplayNameAction())->execute($user, $record); + + Notification::make() + ->success() + ->title('Success') + ->body("Approved {$originalDisplayName}'s username change request.") + ->send(); + }) + ->visible(fn (UserUsername $record) => !$record->is_approved && !$record->is_denied) + ->requiresConfirmation() + ->modalDescription("Are you sure you'd like to do this? The username change will go into effect immediately.") + ->color('success') + ->icon('heroicon-o-check'), + + Tables\Actions\Action::make('deny') + ->action(function (UserUsername $record) { + $record->update(['denied_at' => now()]); + + /** @var User $user */ + $user = $record->user; + + sendDisplayNameChangeDeclineEmail($user, $record->username); + + Notification::make() + ->success() + ->title('Success') + ->body("Denied {$record->user->display_name}'s username change request.") + ->send(); + }) + ->visible(fn (UserUsername $record) => !$record->is_approved && !$record->is_denied) + ->requiresConfirmation() + ->modalDescription('Are you sure you want to deny this username change request?') + ->color('danger') + ->icon('heroicon-o-x-mark'), + ]) + ->bulkActions([ + + ]); + } + + public static function getRelations(): array + { + return [ + + ]; + } + + public static function getPages(): array + { + return [ + 'index' => Pages\Index::route('/'), + ]; + } +} diff --git a/app/Filament/Resources/UserUsernameResource/Pages/Index.php b/app/Filament/Resources/UserUsernameResource/Pages/Index.php new file mode 100644 index 0000000000..74940268ec --- /dev/null +++ b/app/Filament/Resources/UserUsernameResource/Pages/Index.php @@ -0,0 +1,20 @@ + $newDisplayName]) . "'>here"; + + $msg = "Hello,

" . + "Great news! Your username change request to {$newDisplayName} has been approved.

" . + + "You can now use your new username to log in everywhere on RetroAchievements.org.

" . + + "Check out your updated profile {$profileLink}.

" . + + "-- Your friends at RetroAchievements.org
"; + + return mail_utf8($user->EmailAddress, $emailTitle, $msg); +} + +/** + * Sends an email to a user informing them that their display name change request was declined. + */ +function sendDisplayNameChangeDeclineEmail( + User $user, + string $desiredDisplayName, +): bool { + $emailTitle = "About Your Username Change Request"; + + $msg = "Hello,

" . + "We have reviewed your request to change your username to {$desiredDisplayName}, " . + "and have decided not to approve it at this time.

" . + + "You are welcome to submit another request after a 30 day cooldown period has ended.

" . + + "-- Your friends at RetroAchievements.org
"; + + return mail_utf8($user->EmailAddress, $emailTitle, $msg); +} diff --git a/app/Http/Middleware/HandleInertiaRequests.php b/app/Http/Middleware/HandleInertiaRequests.php index 7dfb3a7641..0a5f2a9a7a 100644 --- a/app/Http/Middleware/HandleInertiaRequests.php +++ b/app/Http/Middleware/HandleInertiaRequests.php @@ -44,6 +44,7 @@ public function share(Request $request): array 'auth' => $user ? [ 'user' => UserData::fromUser($user)->include( 'id', + 'isEmailVerified', 'isMuted', 'isNew', 'legacyPermissions', diff --git a/app/Models/UserUsername.php b/app/Models/UserUsername.php new file mode 100644 index 0000000000..c68a84320b --- /dev/null +++ b/app/Models/UserUsername.php @@ -0,0 +1,102 @@ + 'datetime', + 'denied_at' => 'datetime', + ]; + + // == accessors + + public function getIsApprovedAttribute(): bool + { + return $this->approved_at !== null; + } + + public function getIsDeniedAttribute(): bool + { + return $this->denied_at !== null; + } + + public function getIsExpiredAttribute(): bool + { + if ($this->approved_at !== null || $this->denied_at !== null) { + return false; + } + + return $this->created_at->isPast() && $this->created_at->diffInDays(now()) >= 30; + } + + // == mutators + + // == relations + + /** + * @return BelongsTo + */ + public function user(): BelongsTo + { + return $this->belongsTo(User::class, 'user_id', 'ID'); + } + + // == scopes + + /** + * @param Builder $query + * @return Builder + */ + public function scopeApproved(Builder $query): Builder + { + return $query->whereNotNull('approved_at'); + } + + /** + * @param Builder $query + * @return Builder + */ + public function scopeDenied(Builder $query): Builder + { + return $query->whereNotNull('denied_at') + ->where('denied_at', '>', now()->subDays(30)); + } + + /** + * @param Builder $query + * @return Builder + */ + public function scopeExpired(Builder $query): Builder + { + return $query->whereNull('approved_at') + ->whereNull('denied_at') + ->where('created_at', '<=', now()->subDays(30)); + } + + /** + * @param Builder $query + * @return Builder + */ + public function scopePending(Builder $query): Builder + { + return $query->whereNull('approved_at') + ->whereNull('denied_at') + ->where('created_at', '>', now()->subDays(30)); + } +} diff --git a/app/Policies/UserUsernamePolicy.php b/app/Policies/UserUsernamePolicy.php new file mode 100644 index 0000000000..7d0d247305 --- /dev/null +++ b/app/Policies/UserUsernamePolicy.php @@ -0,0 +1,82 @@ +hasAnyRole([ + Role::ADMINISTRATOR, + Role::MODERATOR, + ]); + } + + public function viewAny(?User $user): bool + { + return $user->hasAnyRole([ + Role::ADMINISTRATOR, + Role::MODERATOR, + ]); + } + + public function create(User $user): bool + { + // Muted users cannot change their usernames. + if ($user->isMuted()) { + return false; + } + + $lastChanges = UserUsername::whereUserId($user->id) + ->selectRaw(<<first(); + + // If no previous requests exist, allow creation. + if (!$lastChanges || (!$lastChanges->last_request && !$lastChanges->last_approval)) { + return true; + } + + // Get the most recent action date - between request and approval. + $lastActionDate = max( + $lastChanges->last_request ? Carbon::parse($lastChanges->last_request) : Carbon::create(0), + $lastChanges->last_approval ? Carbon::parse($lastChanges->last_approval) : Carbon::create(0) + ); + + // Users get a 30-day cooldown after requests & after approval. + return !$lastActionDate->isAfter(now()->subDays(30)); + } + + public function update(User $user): bool + { + return false; + } + + public function delete(User $user): bool + { + // They'll just need to wait for 30 days to lapse. + return false; + } + + public function restore(User $user): bool + { + return false; + } + + public function forceDelete(User $user): bool + { + return false; + } +} diff --git a/app/Support/Rules/ValidNewUsername.php b/app/Support/Rules/ValidNewUsername.php new file mode 100644 index 0000000000..556e263867 --- /dev/null +++ b/app/Support/Rules/ValidNewUsername.php @@ -0,0 +1,53 @@ +getDriverName() === 'sqlite' ? 'UserAccounts' : 'mysql.UserAccounts'; + + // For new registrations, block both existing usernames and pending requests. + if (!$user) { + return array_merge($baseRules, [ + "unique:{$table},User", + "unique:{$table},display_name", + function (string $attribute, mixed $value, Closure $fail) { + // Check if this username is pending approval for anyone. + $hasPendingRequest = UserUsername::pending() + ->where('username', $value) + ->exists(); + + if ($hasPendingRequest) { + $fail('This username is currently unavailable.'); + } + }, + ]); + } + + // For username change requests, exclude the current user from + // unique checks. This allows them to revert back to their + // original username after they set a display_name. + return array_merge($baseRules, [ + Rule::unique($table, 'User')->ignore($user->id, 'ID'), + Rule::unique($table, 'display_name')->ignore($user->id, 'ID'), + ]); + } +} diff --git a/database/migrations/2025_01_17_000000_update_user_usernames_table.php b/database/migrations/2025_01_17_000000_update_user_usernames_table.php new file mode 100644 index 0000000000..400b2401e3 --- /dev/null +++ b/database/migrations/2025_01_17_000000_update_user_usernames_table.php @@ -0,0 +1,41 @@ +dropForeign(['user_id']); + $table->dropUnique(['user_id', 'username']); + + $table->foreign('user_id', 'user_usernames_user_id_foreign') + ->references('ID') + ->on('UserAccounts') + ->onDelete('set null'); + + $table->timestamp('approved_at')->nullable()->after('username'); + $table->timestamp('denied_at')->nullable()->after('approved_at'); + }); + } + + public function down(): void + { + Schema::table('user_usernames', function (Blueprint $table) { + $table->dropColumn(['approved_at', 'denied_at']); + + $table->dropForeign('user_usernames_user_id_foreign'); + + $table->unique(['user_id', 'username']); + + $table->foreign('user_id', 'user_usernames_user_id_foreign') + ->references('ID') + ->on('UserAccounts') + ->onDelete('SET NULL'); + }); + } +}; diff --git a/lang/en_US.json b/lang/en_US.json index 01d7fdb505..47e3d94589 100644 --- a/lang/en_US.json +++ b/lang/en_US.json @@ -633,6 +633,20 @@ "Beaten by same players": "Beaten by same players", "Mastered by same players": "Mastered by same players", "<1>{{achievementTitle}} from <2>{{gameTitle}}": "<1>{{achievementTitle}} from <2>{{gameTitle}}", + "New usernames must match.": "New usernames must match.", + "You can only request a new username once every 30 days, even if your new username is not approved. Are you sure you want to do this?": "You can only request a new username once every 30 days, even if your new username is not approved. Are you sure you want to do this?", + "Submitting username change request...": "Submitting username change request...", + "Submitted username change request!": "Submitted username change request!", + "Must only contain unaccented letters and numbers.": "Must only contain unaccented letters and numbers.", + "Change Username": "Change Username", + "Current Username": "Current Username", + "New Username": "New Username", + "enter your new username here...": "enter your new username here...", + "Confirm New Username": "Confirm New Username", + "confirm your new username here...": "confirm your new username here...", + "This username is already taken.": "This username is already taken.", + "Your request will either be approved or it will automatically expire 30 days from when you requested it.": "Your request will either be approved or it will automatically expire 30 days from when you requested it.", + "Requested Username": "Requested Username", "news-category.achievement-set": "Featured Set", "news-category.community": "Community", "news-category.events": "Events", @@ -642,6 +656,10 @@ "Don't ask for links to copyrighted ROMs. Don't share links to copyrighted ROMs.": "Don't ask for links to copyrighted ROMs. Don't share links to copyrighted ROMs.", "Start new topic": "Start new topic", "enter your new topic's title...": "enter your new topic's title...", + "New username must be different from current username.": "New username must be different from current username.", + "Your username change request is being reviewed.": "Your username change request is being reviewed.", + "Your username cannot be changed right now.": "Your username cannot be changed right now.", + "You can request another change after your previous request's 30-day cooldown period has ended.": "You can request another change after your previous request's 30-day cooldown period has ended.", "administrator": "Admin", "game-hash-manager": "Hash Manager", "release-manager": "Release Manager", diff --git a/public/request/auth/register.php b/public/request/auth/register.php index 961956d289..3be153d661 100644 --- a/public/request/auth/register.php +++ b/public/request/auth/register.php @@ -1,20 +1,13 @@ post()), [ - 'username' => [ - 'required', - 'unique:mysql.UserAccounts,User', - 'unique:mysql.UserAccounts,display_name', - 'min:4', - 'max:20', - new CtypeAlnum(), - ], + 'username' => ValidNewUsername::get(), 'password' => 'required|min:8|different:username', 'email' => 'required|email:filter|confirmed', 'terms' => 'accepted', diff --git a/resources/js/features/settings/components/+root/SettingsRoot.test.tsx b/resources/js/features/settings/components/+root/SettingsRoot.test.tsx index 1a446ed024..374140e0ac 100644 --- a/resources/js/features/settings/components/+root/SettingsRoot.test.tsx +++ b/resources/js/features/settings/components/+root/SettingsRoot.test.tsx @@ -48,4 +48,64 @@ describe('Component: SettingsRoot', () => { websitePrefs: 8399, }); }); + + it('given the user is muted and is email verified, does not show the change username section', async () => { + // ARRANGE + render(, { + pageProps: { + auth: { + user: createAuthenticatedUser({ + websitePrefs: 139687, + isMuted: true, + isEmailVerified: true, + }), + }, + userSettings: createUser(), + can: { updateMotto: true }, + }, + }); + + // ASSERT + expect(screen.queryByText(/change username/i)).not.toBeInTheDocument(); + }); + + it('given the user is not muted and is email verified, shows the change username section', () => { + // ARRANGE + render(, { + pageProps: { + auth: { + user: createAuthenticatedUser({ + websitePrefs: 139687, + isMuted: false, + isEmailVerified: true, + }), + }, + userSettings: createUser(), + can: { updateMotto: true }, + }, + }); + + // ASSERT + expect(screen.getByText(/change username/i)).toBeVisible(); + }); + + it('given the user is not muted and is not email verified, does not show the change username change', () => { + // ARRANGE + render(, { + pageProps: { + auth: { + user: createAuthenticatedUser({ + websitePrefs: 139687, + isMuted: false, + isEmailVerified: false, + }), + }, + userSettings: createUser(), + can: { updateMotto: true }, + }, + }); + + // ASSERT + expect(screen.queryByText(/change username/i)).not.toBeInTheDocument(); + }); }); diff --git a/resources/js/features/settings/components/+root/SettingsRoot.tsx b/resources/js/features/settings/components/+root/SettingsRoot.tsx index dc79b4e742..8769860369 100644 --- a/resources/js/features/settings/components/+root/SettingsRoot.tsx +++ b/resources/js/features/settings/components/+root/SettingsRoot.tsx @@ -5,6 +5,7 @@ import { usePageProps } from '@/common/hooks/usePageProps'; import { ChangeEmailAddressSectionCard } from '../ChangeEmailAddressSectionCard'; import { ChangePasswordSectionCard } from '../ChangePasswordSectionCard'; +import { ChangeUsernameSectionCard } from '../ChangeUsernameSectionCard'; import { DeleteAccountSectionCard } from '../DeleteAccountSectionCard'; import { KeysSectionCard } from '../KeysSectionCard'; import { LocaleSectionCard } from '../LocaleSectionCard'; @@ -47,6 +48,7 @@ export const SettingsRoot: FC = memo(() => { /> + {!auth?.user.isMuted && auth?.user.isEmailVerified ? : null} diff --git a/resources/js/features/settings/components/ChangeUsernameSectionCard/ChangeUsernameSectionCard.test.tsx b/resources/js/features/settings/components/ChangeUsernameSectionCard/ChangeUsernameSectionCard.test.tsx new file mode 100644 index 0000000000..07bbe1b474 --- /dev/null +++ b/resources/js/features/settings/components/ChangeUsernameSectionCard/ChangeUsernameSectionCard.test.tsx @@ -0,0 +1,219 @@ +import userEvent from '@testing-library/user-event'; +import axios from 'axios'; + +import { createAuthenticatedUser } from '@/common/models'; +import { render, screen, waitFor } from '@/test'; + +import { requestedUsernameAtom } from '../../state/settings.atoms'; +import { ChangeUsernameSectionCard } from './ChangeUsernameSectionCard'; + +describe('Component: ChangeUsernameSectionCard', () => { + it('renders without crashing', () => { + // ARRANGE + const { container } = render(, { + pageProps: { + auth: { user: createAuthenticatedUser({ displayName: 'test-user' }) }, + can: { createUsernameChangeRequest: true }, + }, + }); + + // ASSERT + expect(container).toBeTruthy(); + }); + + it('given the user has a pending username request, shows the pending request alert', () => { + // ARRANGE + render(, { + pageProps: { + auth: { user: createAuthenticatedUser({ displayName: 'test-user' }) }, + can: { createUsernameChangeRequest: true }, + }, + jotaiAtoms: [[requestedUsernameAtom, 'new-username']], + }); + + // ASSERT + expect(screen.getByText(/your username change request is being reviewed/i)).toBeVisible(); + expect(screen.getByText(/new-username/i)).toBeVisible(); + }); + + it('given the user cannot create a username change request, shows the wait alert', () => { + // ARRANGE + render(, { + pageProps: { + auth: { user: createAuthenticatedUser({ displayName: 'test-user' }) }, + can: { createUsernameChangeRequest: false }, + }, + }); + + // ASSERT + expect(screen.getByText(/your username cannot be changed right now/i)).toBeVisible(); + expect( + screen.getByText( + /you can request another change after your previous request's 30-day cooldown period has ended/i, + ), + ).toBeVisible(); + }); + + it('given the user can create a username change request, shows the form', () => { + // ARRANGE + render(, { + pageProps: { + auth: { user: createAuthenticatedUser({ displayName: 'test-user' }) }, + can: { createUsernameChangeRequest: true }, + }, + }); + + // ASSERT + expect(screen.getAllByLabelText(/new username/i)[0]).toBeVisible(); + expect(screen.getByLabelText(/confirm new username/i)).toBeVisible(); + }); + + it('given the user attempts to submit with non-matching usernames, does not submit', async () => { + // ARRANGE + const postSpy = vi.spyOn(axios, 'post'); + + render(, { + pageProps: { + auth: { user: createAuthenticatedUser({ displayName: 'test-user' }) }, + can: { createUsernameChangeRequest: true }, + }, + }); + + // ACT + await userEvent.type(screen.getAllByLabelText(/new username/i)[0], 'new-name'); + await userEvent.type(screen.getByLabelText(/confirm new username/i), 'different-name'); + await userEvent.click(screen.getByRole('button', { name: /update/i })); + + // ASSERT + expect(postSpy).not.toHaveBeenCalled(); + }); + + it('given the user attempts to submit usernames with invalid characters, does not submit', async () => { + // ARRANGE + const postSpy = vi.spyOn(axios, 'post'); + + render(, { + pageProps: { + auth: { user: createAuthenticatedUser({ displayName: 'TestUser' }) }, + can: { createUsernameChangeRequest: true }, + }, + }); + + // ACT + await userEvent.type(screen.getAllByLabelText(/new username/i)[0], 'new-name'); + await userEvent.type(screen.getByLabelText(/confirm new username/i), 'new-name'); + await userEvent.click(screen.getByRole('button', { name: /update/i })); + + // ASSERT + expect(postSpy).not.toHaveBeenCalled(); + expect( + screen.getAllByText(/must only contain unaccented letters and numbers./i)[0], + ).toBeVisible(); + }); + + it('given the user submits valid form data but cancels the confirmation, does not submit', async () => { + // ARRANGE + vi.spyOn(window, 'confirm').mockImplementationOnce(() => false); + const postSpy = vi.spyOn(axios, 'post'); + + render(, { + pageProps: { + auth: { user: createAuthenticatedUser({ displayName: 'test-user' }) }, + can: { createUsernameChangeRequest: true }, + }, + }); + + // ACT + await userEvent.type(screen.getAllByLabelText(/new username/i)[0], 'NewName'); + await userEvent.type(screen.getByLabelText(/confirm new username/i), 'NewName'); + await userEvent.click(screen.getByRole('button', { name: /update/i })); + + // ASSERT + expect(postSpy).not.toHaveBeenCalled(); + }); + + it('given the user submits valid form data and confirms, sends the request to the server', async () => { + // ARRANGE + vi.spyOn(window, 'confirm').mockImplementationOnce(() => true); + const postSpy = vi.spyOn(axios, 'post').mockResolvedValueOnce({ + data: { + success: true, + }, + }); + + render(, { + pageProps: { + auth: { user: createAuthenticatedUser({ displayName: 'TestUser' }) }, + can: { createUsernameChangeRequest: true }, + }, + }); + + // ACT + await userEvent.type(screen.getAllByLabelText(/new username/i)[0], 'NewName'); + await userEvent.type(screen.getByLabelText(/confirm new username/i), 'NewName'); + await userEvent.click(screen.getByRole('button', { name: /update/i })); + + // ASSERT + expect(postSpy).toHaveBeenCalledWith(route('api.settings.username-change-request.store'), { + newDisplayName: 'NewName', + }); + }); + + it('given the API returns a username taken error, shows the appropriate error message', async () => { + // ARRANGE + vi.spyOn(window, 'confirm').mockImplementationOnce(() => true); + vi.spyOn(axios, 'post').mockRejectedValueOnce({ + response: { + data: { + message: 'has already been taken', + }, + }, + }); + + render(, { + pageProps: { + auth: { user: createAuthenticatedUser({ displayName: 'TestUser' }) }, + can: { createUsernameChangeRequest: true }, + }, + }); + + // ACT + await userEvent.type(screen.getAllByLabelText(/new username/i)[0], 'NewName'); + await userEvent.type(screen.getByLabelText(/confirm new username/i), 'NewName'); + await userEvent.click(screen.getByRole('button', { name: /update/i })); + + // ASSERT + await waitFor(() => { + expect(screen.getByText(/this username is already taken/i)).toBeVisible(); + }); + }); + + it('given the API returns an unexpected error, shows a generic error message', async () => { + // ARRANGE + vi.spyOn(window, 'confirm').mockImplementationOnce(() => true); + vi.spyOn(axios, 'post').mockRejectedValueOnce({ + response: { + data: { + message: 'some other error', + }, + }, + }); + + render(, { + pageProps: { + auth: { user: createAuthenticatedUser({ displayName: 'TestUser' }) }, + can: { createUsernameChangeRequest: true }, + }, + }); + + // ACT + await userEvent.type(screen.getAllByLabelText(/new username/i)[0], 'NewName'); + await userEvent.type(screen.getByLabelText(/confirm new username/i), 'NewName'); + await userEvent.click(screen.getByRole('button', { name: /update/i })); + + // ASSERT + await waitFor(() => { + expect(screen.getByText(/something went wrong/i)).toBeVisible(); + }); + }); +}); diff --git a/resources/js/features/settings/components/ChangeUsernameSectionCard/ChangeUsernameSectionCard.tsx b/resources/js/features/settings/components/ChangeUsernameSectionCard/ChangeUsernameSectionCard.tsx new file mode 100644 index 0000000000..9c5eff2570 --- /dev/null +++ b/resources/js/features/settings/components/ChangeUsernameSectionCard/ChangeUsernameSectionCard.tsx @@ -0,0 +1,159 @@ +import { useAtom } from 'jotai'; +import { type FC, useId } from 'react'; +import { useTranslation } from 'react-i18next'; +import { LuAlertCircle } from 'react-icons/lu'; + +import { + BaseAlert, + BaseAlertDescription, + BaseAlertTitle, +} from '@/common/components/+vendor/BaseAlert'; +import { + BaseFormControl, + BaseFormField, + BaseFormItem, + BaseFormLabel, + BaseFormMessage, +} from '@/common/components/+vendor/BaseForm'; +import { BaseInput } from '@/common/components/+vendor/BaseInput'; +import { usePageProps } from '@/common/hooks/usePageProps'; + +import { requestedUsernameAtom } from '../../state/settings.atoms'; +import { SectionFormCard } from '../SectionFormCard'; +import { useChangeUsernameForm } from './useChangeUsernameForm'; + +export const ChangeUsernameSectionCard: FC = () => { + const { auth, can } = usePageProps(); + + const { t } = useTranslation(); + + const { form, mutation, onSubmit } = useChangeUsernameForm(); + + const [requestedUsername] = useAtom(requestedUsernameAtom); + + const visibleDisplayNameFieldId = useId(); + const requestedDisplayNameFieldId = useId(); + + const canShowForm = !requestedUsername && can.createUsernameChangeRequest; + + return ( + +
+
+ {requestedUsername ? : null} + {!requestedUsername && !can.createUsernameChangeRequest ? : null} + +
+ +

{auth!.user.displayName}

+
+ + {requestedUsername ? ( +
+ +

{requestedUsername}

+
+ ) : null} + + {canShowForm ? ( +
+ ( + + + {t('New Username')} + + +
+ + + + + +
+ + )} + /> + + ( + + + {t('Confirm New Username')} + + +
+ + + + + +
+ + )} + /> +
+ ) : null} +
+
+ + ); +}; + +const PendingRequestAlert: FC = () => { + const { t } = useTranslation(); + + return ( + + + {t('Your username change request is being reviewed.')} + + {t( + 'Your request will either be approved or it will automatically expire 30 days from when you requested it.', + )} + + + ); +}; + +const WaitAlert: FC = () => { + const { t } = useTranslation(); + + return ( + + + {t('Your username cannot be changed right now.')} + + {t( + "You can request another change after your previous request's 30-day cooldown period has ended.", + )} + + + ); +}; diff --git a/resources/js/features/settings/components/ChangeUsernameSectionCard/index.ts b/resources/js/features/settings/components/ChangeUsernameSectionCard/index.ts new file mode 100644 index 0000000000..914c2a62ed --- /dev/null +++ b/resources/js/features/settings/components/ChangeUsernameSectionCard/index.ts @@ -0,0 +1 @@ +export * from './ChangeUsernameSectionCard'; diff --git a/resources/js/features/settings/components/ChangeUsernameSectionCard/useChangeUsernameForm.ts b/resources/js/features/settings/components/ChangeUsernameSectionCard/useChangeUsernameForm.ts new file mode 100644 index 0000000000..394f607336 --- /dev/null +++ b/resources/js/features/settings/components/ChangeUsernameSectionCard/useChangeUsernameForm.ts @@ -0,0 +1,94 @@ +import { zodResolver } from '@hookform/resolvers/zod'; +import { useMutation } from '@tanstack/react-query'; +import axios from 'axios'; +import { useAtom } from 'jotai'; +import { useMemo } from 'react'; +import { useForm } from 'react-hook-form'; +import { useTranslation } from 'react-i18next'; +import { z } from 'zod'; + +import { toastMessage } from '@/common/components/+vendor/BaseToaster'; +import { usePageProps } from '@/common/hooks/usePageProps'; +import type { LaravelValidationError } from '@/common/models'; + +import { requestedUsernameAtom } from '../../state/settings.atoms'; + +export function useChangeUsernameForm() { + const { auth } = usePageProps(); + + const { t } = useTranslation(); + + const [requestedUsername, setRequestedUsername] = useAtom(requestedUsernameAtom); + + const usernameChangeFormSchema = useMemo( + () => + z + .object({ + newUsername: z + .string() + .min(4) + .max(20) + .regex(/^[a-zA-Z0-9]+$/, t('Must only contain unaccented letters and numbers.')), + confirmUsername: z + .string() + .min(4) + .max(20) + .regex(/^[a-zA-Z0-9]+$/, t('Must only contain unaccented letters and numbers.')), + }) + .refine((data) => data.newUsername === data.confirmUsername, { + message: t('New usernames must match.'), + path: ['confirmUsername'], + }) + .refine((data) => data.newUsername !== auth!.user.displayName, { + message: t('New username must be different from current username.'), + path: ['newUsername'], + }), + [auth, t], + ); + + type FormValues = z.infer; + + const form = useForm({ + resolver: zodResolver(usernameChangeFormSchema), + disabled: !!requestedUsername, + defaultValues: { + newUsername: '', + confirmUsername: '', + }, + }); + + const mutation = useMutation({ + mutationFn: (formValues: FormValues) => { + return axios.post(route('api.settings.username-change-request.store'), { + newDisplayName: formValues.newUsername, + }); + }, + onSuccess: (_, { newUsername }) => { + setRequestedUsername(newUsername); + }, + }); + + const onSubmit = async (formValues: FormValues) => { + const confirmationMessage = t( + 'You can only request a new username once every 30 days, even if your new username is not approved. Are you sure you want to do this?', + ); + + if (!confirm(confirmationMessage)) { + return; + } + + await toastMessage.promise(mutation.mutateAsync(formValues), { + loading: t('Submitting username change request...'), + success: t('Submitted username change request!'), + error: ({ response }: LaravelValidationError) => { + if (response.data.message.includes('already been taken')) { + return t('This username is already taken.'); + } + + return t('Something went wrong.'); + }, + }); + }; + + return { form, mutation, onSubmit }; +} diff --git a/resources/js/features/settings/components/DeleteAccountSectionCard/DeleteAccountSectionCard.tsx b/resources/js/features/settings/components/DeleteAccountSectionCard/DeleteAccountSectionCard.tsx index f13cac463e..d20ad0d415 100644 --- a/resources/js/features/settings/components/DeleteAccountSectionCard/DeleteAccountSectionCard.tsx +++ b/resources/js/features/settings/components/DeleteAccountSectionCard/DeleteAccountSectionCard.tsx @@ -67,7 +67,7 @@ export const DeleteAccountSectionCard: FC = () => {
{isDeleteAlreadyRequested ? ( - + {t("You've requested account deletion.")} {t('Your account will be permanently deleted on {{date}}', { diff --git a/resources/js/features/settings/components/SectionFormCard/SectionFormCard.test.tsx b/resources/js/features/settings/components/SectionFormCard/SectionFormCard.test.tsx index e425303dea..8d5ef15324 100644 --- a/resources/js/features/settings/components/SectionFormCard/SectionFormCard.test.tsx +++ b/resources/js/features/settings/components/SectionFormCard/SectionFormCard.test.tsx @@ -73,4 +73,28 @@ describe('Component: SectionFormCard', () => { // ASSERT expect(screen.getByRole('button', { name: /some different label/i })).toBeVisible(); }); + + it('given shouldShowFooter is false, does not render the card footer', () => { + // ARRANGE + render( + + children + , + ); + + // ASSERT + expect(screen.queryByRole('button', { name: /update/i })).not.toBeInTheDocument(); + }); + + it('given shouldShowFooter is true, renders the card footer', () => { + // ARRANGE + render( + + children + , + ); + + // ASSERT + expect(screen.getByRole('button', { name: /update/i })).toBeVisible(); + }); }); diff --git a/resources/js/features/settings/components/SectionFormCard/SectionFormCard.tsx b/resources/js/features/settings/components/SectionFormCard/SectionFormCard.tsx index 6f154d316c..e6b1cabb7f 100644 --- a/resources/js/features/settings/components/SectionFormCard/SectionFormCard.tsx +++ b/resources/js/features/settings/components/SectionFormCard/SectionFormCard.tsx @@ -24,6 +24,7 @@ export interface SectionFormCardProps { isSubmitting: boolean; buttonProps?: BaseButtonProps; + shouldShowFooter?: boolean; } export const SectionFormCard: FC = ({ @@ -33,6 +34,7 @@ export const SectionFormCard: FC = ({ isSubmitting, onSubmit, t_headingLabel, + shouldShowFooter = true, }) => { const { t } = useTranslation(); @@ -46,18 +48,20 @@ export const SectionFormCard: FC = ({
{children} - -
- - {buttonProps?.children ?? t('Update')} - -
- + {shouldShowFooter ? ( + +
+ + {buttonProps?.children ?? t('Update')} + +
+ + ) : null}
diff --git a/resources/js/features/settings/state/settings.atoms.ts b/resources/js/features/settings/state/settings.atoms.ts new file mode 100644 index 0000000000..a6336999fc --- /dev/null +++ b/resources/js/features/settings/state/settings.atoms.ts @@ -0,0 +1,3 @@ +import { atom } from 'jotai'; + +export const requestedUsernameAtom = atom(); diff --git a/resources/js/pages/settings.tsx b/resources/js/pages/settings.tsx index 86358c4758..3647209129 100644 --- a/resources/js/pages/settings.tsx +++ b/resources/js/pages/settings.tsx @@ -1,14 +1,24 @@ +import { useHydrateAtoms } from 'jotai/utils'; import { useTranslation } from 'react-i18next'; import { SEO } from '@/common/components/SEO'; +import { usePageProps } from '@/common/hooks/usePageProps'; import { AppLayout } from '@/common/layouts/AppLayout'; import type { AppPage } from '@/common/models'; import { SettingsRoot } from '@/features/settings/components/+root'; import { SettingsSidebar } from '@/features/settings/components/+sidebar'; +import { requestedUsernameAtom } from '@/features/settings/state/settings.atoms'; const Settings: AppPage = () => { + const { requestedUsername } = usePageProps(); + const { t } = useTranslation(); + useHydrateAtoms([ + [requestedUsernameAtom, requestedUsername ?? undefined], + // + ]); + return ( <> diff --git a/resources/js/types/generated.d.ts b/resources/js/types/generated.d.ts index 0232db2b11..b09137829a 100644 --- a/resources/js/types/generated.d.ts +++ b/resources/js/types/generated.d.ts @@ -130,6 +130,7 @@ declare namespace App.Community.Data { export type UserSettingsPageProps = { userSettings: App.Data.User; can: App.Data.UserPermissions; + requestedUsername: string | null; displayableRoles: Array; }; } @@ -272,6 +273,7 @@ declare namespace App.Data { displayableRoles?: Array | null; emailAddress?: string | null; id?: number; + isEmailVerified?: boolean; isMuted?: boolean; isNew?: boolean; legacyPermissions?: number | null; @@ -292,6 +294,7 @@ declare namespace App.Data { }; export type UserPermissions = { createTriggerTicket?: boolean; + createUsernameChangeRequest?: boolean; develop?: boolean; manageGameHashes?: boolean; manageGameSets?: boolean; diff --git a/resources/js/ziggy.d.ts b/resources/js/ziggy.d.ts index 65a47ac868..5b26293693 100644 --- a/resources/js/ziggy.d.ts +++ b/resources/js/ziggy.d.ts @@ -581,6 +581,7 @@ declare module 'ziggy-js' { "api.settings.preferences.update": [], "api.settings.password.update": [], "api.settings.email.update": [], + "api.settings.username-change-request.store": [], "api.settings.keys.web.destroy": [], "api.settings.keys.connect.destroy": [], "api.active-player.index": [], diff --git a/tests/Feature/Community/Controllers/UserSettingsControllerTest.php b/tests/Feature/Community/Controllers/UserSettingsControllerTest.php index a2a319570c..2cd02fcbd9 100644 --- a/tests/Feature/Community/Controllers/UserSettingsControllerTest.php +++ b/tests/Feature/Community/Controllers/UserSettingsControllerTest.php @@ -112,6 +112,33 @@ public function testUpdateEmail(): void $this->assertNull($user->email_verified_at); } + public function testUpdateUsername(): void + { + // Arrange + $this->withoutMiddleware(); + + /** @var User $user */ + $user = User::factory()->create([ + 'User' => 'Scott', + 'display_name' => 'Scott', + ]); + + // Act + $response = $this->actingAs($user) + ->postJson(route('api.settings.username-change-request.store'), [ + 'newDisplayName' => 'Scott123456712', + ]); + + // Assert + $response->assertStatus(200); + + $this->assertDatabaseHas('user_usernames', [ + 'user_id' => $user->id, + 'username' => 'Scott123456712', + 'approved_at' => null, + ]); + } + public function testUpdateProfile(): void { // Arrange