Conversation
app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitsEventTypesApiController.php
Outdated
Show resolved
Hide resolved
app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitsEventTypesApiController.php
Outdated
Show resolved
Hide resolved
app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitsEventTypesApiController.php
Outdated
Show resolved
Hide resolved
app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitsEventTypesApiController.php
Outdated
Show resolved
Hide resolved
smarcet
left a comment
There was a problem hiding this comment.
@andrestejerina97 please review comments
fa5e8a1 to
5842488
Compare
caseylocker
left a comment
There was a problem hiding this comment.
@matiasperrone-exo The POST in app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitsEventTypesApiController.php should be a PUT based on the route.
Incorrect
#[OA\Post(
path: "/api/v1/summits/{id}/event-types/{event_type_id}/summit-documents/{document_id}",
operationId: "addSummitDocument",
Correct
#[OA\Put(
path: "/api/v1/summits/{id}/event-types/{event_type_id}/summit-documents/{document_id}",
operationId: "addSummitDocument",
Is app/Swagger/Models/SummitSchema.php complete?
There is a TODO comment in there.
5842488 to
65bed13
Compare
|
@caseylocker the Summit schema is defined in the Summit controller. Please approve. |
caseylocker
left a comment
There was a problem hiding this comment.
getEventTypeBySummit is documented as public but the route, /api/v1/summits/{id}/event-types/{event_type_id}, requires OAuth2 authentication. Based on the scope of the PR add proper security parameter and change the tag.
security: [['summit_event_types_oauth2' => [
SummitScopes::ReadSummitData,
SummitScopes::ReadAllSummitData,
]]],
tags: ["Event Types"],
Thanks @caseylocker for the comments. Now is ready to review again |
1b361ec to
0d1777a
Compare
caseylocker
left a comment
There was a problem hiding this comment.
PUT operations document HTTP 200 but updated() actually returns a 201.
updateEventTypeBySummit, addSummitDocument, removeSummitDocument Change Response::HTTP_OK to Response::HTTP_CREATED
Missing security attribute on protected GET endpoint in getEventTypeBySummit.
Thanks for adding the public endpoint but the protected one needs documentation of the security attribute.
|
Thanks @caseylocker for the comments. Good catch! Is ready now. I fixed the issues in security and X: Summary of corrections in OAuth2SummitsEventTypesApiController
|
c6ecdd0 to
728ae67
Compare
cf3f50e to
54c18a0
Compare
Signed-off-by: Matias Perrone <github@matiasperrone.com>
Signed-off-by: Matias Perrone <github@matiasperrone.com>
5248392 to
920720d
Compare
Signed-off-by: Matias Perrone <github@matiasperrone.com>
ref: https://app.clickup.com/t/86b6wkha7