Skip to content

Feature: Extend Swagger Coverage for controller OAuth2SummitsEventTypesApiController - #425

Merged
romanetar merged 8 commits into
mainfrom
feature/add-openapi-documentation-to-controller-oauthsummiteventtypecontroller
Jan 28, 2026
Merged

Feature: Extend Swagger Coverage for controller OAuth2SummitsEventTypesApiController#425
romanetar merged 8 commits into
mainfrom
feature/add-openapi-documentation-to-controller-oauthsummiteventtypecontroller

Conversation

@andrestejerina97

Copy link
Copy Markdown
Contributor

@andrestejerina97 andrestejerina97 self-assigned this Oct 31, 2025

@smarcet smarcet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrestejerina97 please review comments

@andrestejerina97
andrestejerina97 force-pushed the feature/add-openapi-documentation-to-controller-oauthsummiteventtypecontroller branch from fa5e8a1 to 5842488 Compare November 20, 2025 16:49

@caseylocker caseylocker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

@matiasperrone-exo
matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller-oauthsummiteventtypecontroller branch from 5842488 to 65bed13 Compare December 5, 2025 15:44
@matiasperrone-exo

Copy link
Copy Markdown
Contributor

@caseylocker the Summit schema is defined in the Summit controller. Please approve.

@caseylocker caseylocker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"], 

@matiasperrone-exo

Copy link
Copy Markdown
Contributor

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.
Both protected and public are pointing to the same method, I added the missing public endpoint to have every endpoint documented.

Now is ready to review again

@matiasperrone-exo
matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller-oauthsummiteventtypecontroller branch from 1b361ec to 0d1777a Compare December 10, 2025 22:36

@caseylocker caseylocker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@matiasperrone-exo

Copy link
Copy Markdown
Contributor

Thanks @caseylocker for the comments. Good catch! Is ready now.

I fixed the issues in security and X:

Summary of corrections in OAuth2SummitsEventTypesApiController

Method Change made
getEventTypeBySummit Added security with ReadSummitData, ReadAllSummitData
addEventTypeBySummit Fixed scope order (WriteEventTypeData, WriteSummitData) and required-groups order
updateEventTypeBySummit Fixed scope order and required-groups order
deleteEventTypeBySummit Fixed scope order and required-groups order
seedDefaultEventTypesBySummit Fixed scope order and required-groups order
addSummitDocument Fixed scope order and required-groups order
removeSummitDocument Fixed scope order and required-groups order

@caseylocker caseylocker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved

@smarcet
smarcet force-pushed the main branch 4 times, most recently from c6ecdd0 to 728ae67 Compare December 17, 2025 00:43
@matiasperrone-exo
matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller-oauthsummiteventtypecontroller branch from cf3f50e to 54c18a0 Compare December 29, 2025 21:08
@matiasperrone-exo
matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller-oauthsummiteventtypecontroller branch from 5248392 to 920720d Compare January 27, 2026 16:33
Signed-off-by: Matias Perrone <github@matiasperrone.com>
@romanetar
romanetar merged commit ceab4d8 into main Jan 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants