-
Notifications
You must be signed in to change notification settings - Fork 2
Feature | Extend Swagger Coverage for controller OAuth2OrganizationsApiController
#370
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.
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
Conversation
c94fc68 to
9a8387b
Compare
ee73837 to
c678bdb
Compare
OAuth2OrganizationsApiController
4991df1 to
25001af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is at least one breaking error here. Description follows:
Line 56-61: Invalid Response References
Error: References #/components/responses/400, etc. which don't exist in the OpenAPI spec
Fix: Replace with inline descriptions:
new OA\Response(response: 400, description: "Bad Request"),
new OA\Response(response: 401, description: "Unauthorized"),
new OA\Response(response: 403, description: "Forbidden"),
new OA\Response(response: 412, description: "Precondition Failed - Validation Error"),
new OA\Response(response: 422, description: "Unprocessable Entity"),
new OA\Response(response: 500, description: "Server Error"),
Line 45: Inconsistent Tag Naming. This is the same issue that we've seen before. Just make it Title Case.
Current: tags: ['organizations'] (lowercase)
Expected: tags: ['Organizations'] (Title Case - matches line 88 and project convention)
Line 44: I don't find 'oauth2_security_scope' defined anywhere.
security: [['oauth2_security_scope' => ...]]
25001af to
1808543
Compare
9cb7b1a to
8ec4fad
Compare
|
@caseylocker @smarcet the security scope was updated with the right value. |
4b8e02d to
fe755b2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matiasperrone
IGNORE THIS - Name space issue: namespace App\Swagger\Schemas; needs to be namespace App\Swagger\Security;
I didn't take into account pr 385
In OAuth2OrganizationsApiController.php: the POST endpoint is missing operationId (but GET has it). It's also missing a description.
In app/Swagger/schemas.php you missing the type: 'object' declaration in the allOf for PaginatedOrganizationsResponse.
7d5c9df to
70777d4
Compare
caseylocker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matiasperrone-exo there's a capitalization issue with the namespace in OrganizationsAuthSchema.php
// Current (WRONG)
namespace App\Swagger\Schemas; // Capital 'S'
// Required (CORRECT)
namespace App\Swagger\schemas; // Lowercase 's'
Also needs and operationId.
#[OA\Post(
path: '/api/v1/organizations',
operationId: 'createOrganization', // ← ADD THIS
summary: 'Creates a new organization',
// ...
)]
Once those 2 items are fixed it should be good to go.
- Add controller's response to OpenAPI schema
43ecd3a to
1176042
Compare
caseylocker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go.
smarcet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Task:
Ref: https://app.clickup.com/t/86b6wkgp2