-
Notifications
You must be signed in to change notification settings - Fork 2
Feature | Extend Swagger Coverage for controller OAuth2SummitAttendeeNotesApiController #417
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
c86fe47 to
ec8716a
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.
Wrong namespace as per project standards:
App\Swagger\security should be App\Swagger\schemas
As per project standards tag "AttendeeNotes" should be "Attendee Notes" - Title Case.
The anyOf is incorrectly used for expandable properties.
// Current (invalid)
anyOf: [
new OA\Property(property: 'author_id', type: 'integer', ...),
new OA\Property(property: 'author', type: 'AdminMember', ...),
...
]
// Fix - Document in properties
properties: [
new OA\Property(property: 'id', type: 'integer'),
new OA\Property(property: 'created', type: 'integer', description: 'Unix timestamp'),
new OA\Property(property: 'last_edited', type: 'integer', description: 'Unix timestamp'),
new OA\Property(property: 'content', type: 'string', description: 'Note content'),
new OA\Property(property: 'author_id', type: 'integer', description: 'Author member ID. Replaced by author object when using ?expand=author'),
new OA\Property(property: 'owner_id', type: 'integer', description: 'Owner attendee ID. Replaced by owner object when using ?expand=owner'),
new OA\Property(property: 'ticket_id', type: 'integer', nullable: true, description: 'Ticket ID. Replaced by ticket object when using ?expand=ticket'),
]
|
Thanks @caseylocker for the comments. Now is ready to review again |
ec8716a to
4a1b1a0
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.
Let's make it easy. Here's the new block of code for app/Swagger/SummitAttendeeNoteSchemas.php.
#[OA\Schema(
schema: 'SummitAttendeeNote',
type: 'object',
description: 'Note attached to an attendee (admin view with email)',
required: ['id', 'created', 'last_edited', 'content'],
properties: [
new OA\Property(property: 'id', type: 'integer'),
new OA\Property(property: 'created', type: 'integer', description: 'Unix timestamp'),
new OA\Property(property: 'last_edited', type: 'integer', description: 'Unix timestamp'),
new OA\Property(property: 'content', type: 'string', description: 'Note content'),
new OA\Property(property: 'author_id', type: 'integer', example: 123, description: 'Author member ID. Replaced by author object when using ?expand=author'),
new OA\Property(property: 'owner_id', type: 'integer', example: 456, description: 'Owner attendee ID. Replaced by owner object when using ?expand=owner'),
new OA\Property(property: 'ticket_id', type: 'integer', example: 789, nullable: true, description: 'Ticket ID. Replaced by ticket object when using ?expand=ticket'),
]
)]
class SummitAttendeeNoteSchema {}
|
Sorry @caseylocker this one shouldn't been moved as it was not ready yet. Please review it again. thanks. |
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 approved. @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
ref: https://app.clickup.com/t/86b6wkh99