fix: Users.CreateUser... vague error into optional name parameter. #10033
Conversation
…oints Signed-off-by: Abinand P <abinand0911@gmail.com>
WalkthroughThe changes update the user creation logic by making the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Controller
participant Database
Client->>API: POST /v1/users (with/without name)
API->>Controller: createUser(..., ?string $name, ...)
Controller->>Controller: if $name is null, set to ""
Controller->>Database: Store user with $name (may be "")
Database-->>Controller: Confirmation
Controller-->>API: User created response
API-->>Client: Success/failure response
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/controllers/api/users.php (1)
85-87: Improve the null check to be more specific.Using
empty($name)checks for multiple "empty" values (null, false, 0, '0', '', []), but for a nullable string parameter, you likely want to check specifically for null.Consider using a more specific null check:
- if (empty($name)) { - $name = ''; - } + $name = $name ?? '';Or if you want to be explicit:
- if (empty($name)) { - $name = ''; - } + if ($name === null) { + $name = ''; + }This prevents unexpected behavior if someone passes other "empty" values like
0orfalse.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
app/controllers/api/users.php(11 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: scan
🔇 Additional comments (10)
app/controllers/api/users.php (10)
62-62: Function signature change looks good.The change from
string $nameto?string $namecorrectly makes the name parameter optional and nullable, which aligns with the PR objective.
222-222: API endpoint signature update is consistent.The change from
string $nameto?string $namein the main user creation endpoint is consistent with thecreateUserfunction update.
256-256: Bcrypt endpoint signature update is consistent.The parameter signature change aligns with the overall modification to make name optional.
291-291: MD5 endpoint signature update is consistent.The parameter signature change aligns with the overall modification to make name optional.
326-326: Argon2 endpoint signature update is consistent.The parameter signature change aligns with the overall modification to make name optional.
362-362: SHA endpoint signature update is consistent.The parameter signature change aligns with the overall modification to make name optional.
403-403: PHPass endpoint signature update is consistent.The parameter signature change aligns with the overall modification to make name optional.
443-443: Scrypt endpoint signature update is consistent.The parameter signature change aligns with the overall modification to make name optional.
489-489: Scrypt-modified endpoint signature update is consistent.The parameter signature change aligns with the overall modification to make name optional.
526-526: Target creation endpoint signature update is consistent.The parameter signature change aligns with the overall modification to make name optional across user-related endpoints.
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
What does this PR do?
In the api/users.php the function signature requires string for name parameter even though its an optional one and displays a vague message over it.
I have modified the signature and added a null check and made the name as empty string if the parameter is given as null.
Test Plan
After updating the code, I have created a user with the null values using the api call using
Thunder client VS code extension(similar to postman)Related PRs and Issues
Checklist
Summary by CodeRabbit
New Features
Bug Fixes