Add api endpoint for updating username #1826
Conversation
📝 WalkthroughWalkthroughAdds a new API endpoint for updating user account username. Includes a new PUT route, request validation class with password verification, controller action that logs the username change activity, and a translation entry for the activity message. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Route
participant UpdateUsernameRequest
participant AccountController
participant UserService
participant ActivityLogger
Client->>Route: PUT /account/username
Route->>UpdateUsernameRequest: validate request
UpdateUsernameRequest->>UpdateUsernameRequest: verify password
alt Password valid
UpdateUsernameRequest-->>Route: request validated
Route->>AccountController: updateUsername()
AccountController->>UserService: update with validated data
UserService-->>AccountController: update result
rect rgb(200, 220, 240)
note right of AccountController: Username changed
AccountController->>ActivityLogger: log user:account.username-changed
ActivityLogger-->>AccountController: logged
end
AccountController-->>Client: 204 No Content
else Password invalid
UpdateUsernameRequest-->>Client: InvalidPasswordProvidedException
end
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/Http/Requests/Api/Client/Account/UpdateUsernameRequest.php (1)
22-26: Prefer Hash facade over resolving Hasher from the container.Simpler and consistent with Laravel conventions.
- use Illuminate\Container\Container; - use Illuminate\Contracts\Hashing\Hasher; + use Illuminate\Support\Facades\Hash; @@ - $hasher = Container::getInstance()->make(Hasher::class); - - // Verify password matches when changing password or email. - if (!$hasher->check($this->input('password'), $this->user()->password)) { + // Verify password matches when changing username. + if (!Hash::check($this->input('password'), $this->user()->password)) { throw new InvalidPasswordProvidedException(trans('validation.internal.invalid_password')); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/Http/Controllers/Api/Client/AccountController.php(2 hunks)app/Http/Requests/Api/Client/Account/UpdateUsernameRequest.php(1 hunks)lang/en/activity.php(1 hunks)routes/api-client.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
app/Http/Requests/Api/Client/Account/UpdateUsernameRequest.php (3)
app/Exceptions/Http/Base/InvalidPasswordProvidedException.php (1)
InvalidPasswordProvidedException(7-7)app/Http/Requests/Api/Client/ClientApiRequest.php (1)
ClientApiRequest(9-30)app/Traits/HasValidation.php (1)
getRulesForUpdate(57-80)
app/Http/Controllers/Api/Client/AccountController.php (4)
app/Http/Requests/Api/Client/Account/UpdateUsernameRequest.php (1)
UpdateUsernameRequest(11-38)app/Models/User.php (1)
username(218-223)app/Facades/Activity.php (1)
Activity(8-14)app/Services/Activity/ActivityLogService.php (3)
event(48-53)property(109-117)log(136-155)
routes/api-client.php (1)
app/Http/Controllers/Api/Client/AccountController.php (1)
AccountController(18-105)
🔇 Additional comments (1)
app/Http/Requests/Api/Client/Account/UpdateUsernameRequest.php (1)
32-37: Validation rule extraction is correct and verified.The 'username' field is properly defined in User model's validation rules at line 156 with
['required', 'between:1,255', 'unique:users,username']. The call chain User::getRulesForUpdate() → getRules() → $validationRules ensures safe array key access.
| Route::prefix('/account')->middleware(AccountSubject::class)->group(function () { | ||
| Route::get('/', [Client\AccountController::class, 'index'])->name('api:client.account'); | ||
|
|
||
| Route::put('/username', [Client\AccountController::class, 'updateUsername'])->name('api:client.account.update-username'); |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM — route mirrors existing email/password endpoints.
Add/verify API docs and tests cover this new route and name.
🏁 Script executed:
#!/bin/bash
# Find route, method, and tests/usages.
rg -nC2 $'/account/username|updateUsername|api:client.account.update-username'Length of output: 1067
I can see the route, controller method, and parallel endpoints exist. Now I need to verify if API documentation and tests cover this route.<!-- [scratchpad_update]
[task verification_3/3: api_docs_and_tests]
Web search confirms Laravel API testing best practices exist (PHPUnit, Laravel testing framework). Now need to search repository for actual test files and API documentation files related to this endpoint.
[actions]
- Search tests/ directory for test files matching updateUsername, account, or api tests
- Search for API documentation files (.md, OpenAPI specs, etc.)
[scratchpad_end] -->
🏁 Script executed:
#!/bin/bash
# Search for test files related to AccountController or updateUsername
find tests -type f -name "*.php" 2>/dev/null | xargs grep -l "updateUsername\|AccountController\|account.*update\|api:client.account" 2>/dev/null | head -20Length of output: 120
🏁 Script executed:
#!/bin/bash
# Search for API documentation files
find . -type f \( -name "*api*doc*" -o -name "*.md" -o -name "openapi*" -o -name "swagger*" \) 2>/dev/null | grep -v node_modules | head -20Length of output: 92
🏁 Script executed:
#!/bin/bash
# Look for test directories and their structure
find tests -type d | head -30Length of output: 927
🏁 Script executed:
#!/bin/bash
# Check for any test files in tests directory
ls -la tests/ 2>/dev/null || echo "tests directory not found or empty"Length of output: 587
🏁 Script executed:
#!/bin/bash
# Check AccountControllerTest.php for updateUsername test coverage
cat tests/Integration/Api/Client/AccountControllerTest.phpLength of output: 7059
🏁 Script executed:
#!/bin/bash
# Search markdown files for API documentation about this endpoint
grep -r "username\|account.*update\|/account/" readme.md contributing.md 2>/dev/null || echo "No matches in main docs"Length of output: 23
Tests are missing for the new updateUsername route.
The route is well-formed and mirrors the email/password endpoints, but AccountControllerTest.php lacks test coverage for PUT /api/client/account/username. The parallel email and password update endpoints have comprehensive tests; add similar tests for username validation, success cases, and error handling. API documentation is also not discoverable in the repository's standard documentation files.
🤖 Prompt for AI Agents
In routes/api-client.php around line 24, the new PUT
/api/client/account/username route lacks test coverage and documentation; add
tests to AccountControllerTest.php mirroring the email/password update specs:
create tests for successful username change (authenticated client, send valid
username, assert 200, database updated and response body), validation failures
(too short/invalid chars, assert 422 and validation error messages), and
uniqueness/conflict handling if applicable (assert proper error code/message);
use the same setup helpers as existing email/password tests, call the route by
name ('api:client.account.update-username'), and add the route entry to the
project's API documentation files so the endpoint is discoverable.
Followup for #1702