Skip to content

Feature | Extend Swagger Coverage for controller OAuth2SummitBadgesApiController - #380

Merged
smarcet merged 17 commits into
mainfrom
feature/add-openapi-documentation-to-controller---apis---protected---summit---oauth2summitbadgesapicontroller
Dec 8, 2025
Merged

Feature | Extend Swagger Coverage for controller OAuth2SummitBadgesApiController#380
smarcet merged 17 commits into
mainfrom
feature/add-openapi-documentation-to-controller---apis---protected---summit---oauth2summitbadgesapicontroller

Conversation

@matiasperrone-exo

@matiasperrone-exo matiasperrone-exo commented Oct 2, 2025

Copy link
Copy Markdown
Contributor

@matiasperrone-exo matiasperrone-exo self-assigned this Oct 2, 2025
@matiasperrone-exo matiasperrone-exo added the documentation Improvements or additions to documentation label Oct 7, 2025
@matiasperrone-exo matiasperrone-exo changed the title Feature | Extend Swagger Coverage for controller Apis/Protected/Summit/OAuth2SummitBadgesApiController.php Feature | Extend Swagger Coverage for controller OAuth2SummitBadgesApiController.php Oct 7, 2025
@matiasperrone-exo
matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---summit---oauth2summitbadgesapicontroller branch 3 times, most recently from 104b8ac to 60766b8 Compare October 8, 2025 22:16
@matiasperrone-exo matiasperrone-exo changed the title Feature | Extend Swagger Coverage for controller OAuth2SummitBadgesApiController.php Feature | Extend Swagger Coverage for controller OAuth2SummitBadgesApiController Oct 13, 2025
@matiasperrone-exo
matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---summit---oauth2summitbadgesapicontroller branch from a73e642 to 0f536ac Compare October 14, 2025 17:26

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

'oauth2_security_scope' issue again. Needs to be defined in schemas.php.
response description of "not found" needs to be title case in the controller, "Not Found":
new OA\Response(response: Response::HTTP_NOT_FOUND, description: "Not Found"),

@matiasperrone-exo
matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---summit---oauth2summitbadgesapicontroller branch from d1f67e8 to e0522af Compare October 31, 2025 18:42
@matiasperrone-exo

Copy link
Copy Markdown
Contributor Author

'oauth2_security_scope' issue again. Needs to be defined in schemas.php. response description of "not found" needs to be title case in the controller, "Not Found": new OA\Response(response: Response::HTTP_NOT_FOUND, description: "Not Found"),

@caseylocker the suggested Title Case is now implemented

@matiasperrone-exo matiasperrone-exo added the review Need reviewing from the developer label Nov 10, 2025
@matiasperrone-exo
matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---summit---oauth2summitbadgesapicontroller branch from 6f7be97 to 1f3f9aa Compare November 13, 2025 20:00
@matiasperrone-exo

Copy link
Copy Markdown
Contributor Author

@caseylocker The security schema for the controller was moved to its own file

@matiasperrone-exo matiasperrone-exo removed the review Need reviewing from the developer label Nov 18, 2025

@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
Please include a unique operationId to endpoints. It's generally not required but is a good standard to follow for automated tooling.

Fix invalid Object references. You can test this in the generate docs. These are invalid types:
new OA\Property(property: 'ticket', type: 'Ticket'),
new OA\Property(property: 'type', type: 'BadgeType'),
Use ref for these since we're going 1 level deep for nesting here or just use the id declarations and and mark the expandable ones as nullable.

Fix the title casing in the controller error response for:
new OA\Response(response: Response::HTTP_NOT_FOUND, description: "not found"),
It should be title case = "Not Found"

Fix the confusing owner_email filter example:
items: new OA\Items(type: 'string', example: 'owner_email=@example.com')
I know what you're saying here but "=@" means "starts with".
Use "contains" instead if you want to keep the format so: owner_email@@example.com"
Or "starts with": "owner_email=@john"

@matiasperrone-exo

Copy link
Copy Markdown
Contributor Author

@caseylocker all the requested changes were introduced. Please review it again.

@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 We're on a roll. Looks good. @smarcet if you agree please merge.

@smarcet

smarcet commented Dec 3, 2025

Copy link
Copy Markdown
Collaborator

@matiasperrone please review conlficts

@matiasperrone-exo
matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---summit---oauth2summitbadgesapicontroller branch from 75b02ef to 744ca2e Compare December 5, 2025 14:26

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

LGTM

@smarcet
smarcet merged commit 1727cb9 into main Dec 8, 2025
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.

4 participants