-
Notifications
You must be signed in to change notification settings - Fork 2
Feature | Extend Swagger Coverage for controller OAuth2UserStoriesApiController
#389
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
c5dffc3 to
19fb15c
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 Comparing this to the others, like the RSVP, I believe the expanded fields should be included as refs to the schemas (or a generic object type).
new OA\Property(property: 'event', ref: '#/components/schemas/organization'),
Please, also, take a look at LegalDocument in this PR as it looks to be a duplicate from PR 388 and might cause issues on merge.
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.
This includes invalid types for the following properties. If you look at the rendered swagger you'll see what I mean.
new OA\Property(property: 'organization', type: 'Organization'),
new OA\Property(property: 'industry', type: 'Industry'),
new OA\Property(property: 'location', type: 'Location'),
new OA\Property(property: 'image', type: 'Image'),
For schema references, you must use ref. Required Fix:
new OA\Property(
property: 'organization',
ref: '#/components/schemas/Organization',
nullable: true
),
new OA\Property(
property: 'industry',
ref: '#/components/schemas/Industry',
nullable: true
),
new OA\Property(
property: 'location',
ref: '#/components/schemas/Location',
nullable: true
),
new OA\Property(
property: 'image',
ref: '#/components/schemas/Image',
nullable: true
),
Also please look at the LegalDocument schema that's been included in this one to determine if it's a duplicate of another one that was included accidentally. Either way this PR should just include items related to OAuth2UserStoriesApiController.
"expand" description should be "description: 'Expand relationships. Available: organization, industry, location, image, tags (converts tag IDs to full tag objects)',"
And update the tags schema property to reflect both states:
new OA\Property(
property: 'tags',
type: 'array',
description: 'Array of tag IDs (use expand=tags to get full tag objects)',
items: new OA\Items(type: 'integer'),
example: [1, 2, 3]
),
61aa7e3 to
09909c6
Compare
|
TBD WIP |
09909c6 to
dfb1196
Compare
|
@caseylocker please review again as all the requested/agreed changes were incorporated. |
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 This needs an operationId parameter.
#[OA\Get(
path: '/api/public/v1/user-stories',
operationId: 'getAllUserStories', // ← Add this
summary: 'Get all user stories',
// ...
)]
That should be it. Once that's added it should be good.
- Add controller's response to OpenAPI schema
…e, and UserStoriesIndustry models
494675b to
8e66b27
Compare
|
Thanks @caseylocker for the comments. Now is ready to review again |
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.
LGTM. @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/86b6wkgpz