-
Notifications
You must be signed in to change notification settings - Fork 2
Feature | Extend Swagger Coverage for controller OAuth2SummitMediaFileTypeApiController
#375
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
71ab1a0 to
2c50dc4
Compare
OAuth2SummitMediaFileTypeApiController
b303941 to
6aefbeb
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 looks like this one also relies on 'summit_rsvp_oauth2' being a defined security schema but it doesn't exist in the Pr in app/Swagger/schemas.php
It looks like once that's actually defined it will fix at least a few of these PRs.
6aefbeb to
48aa91c
Compare
@caseylocker thanks the issue is now fixed and the docs are compiling now. |
6a650e4 to
f29c0b9
Compare
|
@caseylocker @smarcet All the scopes are now the right ones, please review. |
4d93845 to
a6fbac7
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.
Change the "tags" for the GET all to match the other ones - tags: ['Summit Media File Types'],
None of the 5 endpoints have operationId defined. Add for API client generation:
GET all: operationId: 'getAllSummitMediaFileTypes'
GET one: operationId: 'getSummitMediaFileType'
POST: operationId: 'createSummitMediaFileType'
PUT: operationId: 'updateSummitMediaFileType'
DELETE: operationId: 'deleteSummitMediaFileType'
Missing type: 'object' in Paginated Schema
Should be:
new OA\Schema(
type: 'object',
properties: [
…uth2SummitMediaFileTypeApiController.php
62f2a0c to
8953b63
Compare
@caseylocker this PR is ready to be reviewed, I've included all the requested changes |
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 Right on. Looks good. @smarcet if you agree please merge.
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/86b6wkh0a