Feat | API OAuth2UserApiController routes v1#106
Conversation
e5e28a9 to
54025c8
Compare
ae79f5e to
4b5b726
Compare
28e31ae to
2c37852
Compare
martinquiroga-exo
left a comment
There was a problem hiding this comment.
LGTM
@matiasperrone-exo please add the clickup card link to this PR please
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughAdds comprehensive OpenAPI attributes to the OAuth2 users controller and introduces multiple Swagger/OpenAPI schema classes (UserFields, Create/Update requests, UpdatePic/UpdateGroups, UserInfo address/response, PaginatedUserResponse) and updates the OAuth2 security scopes/schema. ChangesUsers API + Schemas
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.54)Composer install failed: this project depends on private packages that require authentication (e.g. GitLab/GitHub, Laravel Nova, etc.). Instead, run PHPStan in a CI/CD pipeline where you can use custom packages — our pipeline remediation tool can use the PHPStan output from your CI/CD pipeline. 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 |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-106/ This page is automatically updated on each push to this PR. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Http/Controllers/Api/OAuth2/OAuth2UserApiController.php`:
- Around line 499-504: The OpenAPI response for the update operation in
OAuth2UserApiController currently uses HttpResponse::HTTP_CREATED (201); change
it to HttpResponse::HTTP_OK (200) so the OA\Response for the update (the block
creating new OA\Response in OAuth2UserApiController, around the
profile-picture/update endpoint) correctly reflects an update operation
returning 200 instead of 201.
- Around line 801-805: The OpenAPI annotation in OAuth2UserApiController
currently documents an update operation with response:
HttpResponse::HTTP_CREATED (201); change that OA\Response to use
HttpResponse::HTTP_OK (200) or HttpResponse::HTTP_NO_CONTENT (204) instead and
update the controller method that performs the user group assignment (the method
containing this OA\Response) so the actual HTTP response status it returns
matches the new code.
- Around line 397-402: The OpenAPI annotation in OAuth2UserApiController is
incorrectly using HttpResponse::HTTP_CREATED for a PUT update response; update
the OA\Response entry in the controller's update annotation to use
HttpResponse::HTTP_OK (200) so the documented status matches the update
semantics and returned User payload in the OA\JsonContent.
- Around line 450-455: The OpenAPI response annotation in
OAuth2UserApiController uses HttpResponse::HTTP_CREATED for an update endpoint;
change the response status to HttpResponse::HTTP_OK (200) to match an update
operation. Locate the OA\Response entry in the controller annotation (the block
that currently has response: HttpResponse::HTTP_CREATED and description
'Updated') and replace HttpResponse::HTTP_CREATED with HttpResponse::HTTP_OK
and, if desired, update the description to 'OK' or keep 'Updated' for clarity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9a428ab-13a3-4653-a1b3-9b59842b0f6b
📒 Files selected for processing (9)
app/Http/Controllers/Api/OAuth2/OAuth2UserApiController.phpapp/Swagger/Models/UserInfoResponseSchema.phpapp/Swagger/OAuth2UserApiControllerSchemas.phpapp/Swagger/Requests/CreateUserRequestSchema.phpapp/Swagger/Requests/UpdateUserGroupsRequestSchema.phpapp/Swagger/Requests/UpdateUserPicRequestSchema.phpapp/Swagger/Requests/UpdateUserRequestSchema.phpapp/Swagger/Requests/UserFieldsSchema.phpapp/Swagger/Security/UsersOAuth2Schema.php
smarcet
left a comment
There was a problem hiding this comment.
@matiasperrone-exo please review comments and rebase with main to get the proper preview for the swagger doc many thanks
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Swagger/Security/UsersOAuth2Schema.php`:
- Line 3: The namespace declaration in UsersOAuth2Schema.php is incorrect
(currently App\Swagger\schemas) and breaks PSR-4 autoloading; update the
namespace to match the file path (use App\Swagger\Security) so the class
UsersOAuth2Schema is declared under the App\Swagger\Security namespace, then run
a quick Composer dump-autoload to verify resolution.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9f0529e-eb78-4a4f-87f4-70c27e0a825a
📒 Files selected for processing (3)
app/Http/Controllers/Api/OAuth2/OAuth2UserApiController.phpapp/Swagger/OAuth2UserApiControllerSchemas.phpapp/Swagger/Security/UsersOAuth2Schema.php
🚧 Files skipped from review as they are similar to previous changes (1)
- app/Swagger/OAuth2UserApiControllerSchemas.php
174fd81 to
d08d047
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-106/ This page is automatically updated on each push to this PR. |
d08d047 to
b545fb6
Compare
|
@smarcet the branch was rebased |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-106/ This page is automatically updated on each push to this PR. |
|
@smarcet please review again thanks! |
smarcet
left a comment
There was a problem hiding this comment.
@matiasperrone-exo please review comments
b545fb6 to
41ffab9
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-106/ This page is automatically updated on each push to this PR. |
Task:
Ref: https://app.clickup.com/t/86b8e6k87
Endpoints:
Summary by CodeRabbit
New Features
Documentation
Style