Skip to content

Conversation

@matiasperrone-exo
Copy link
Contributor

@matiasperrone-exo matiasperrone-exo self-assigned this Oct 15, 2025
@matiasperrone-exo matiasperrone-exo added the documentation Improvements or additions to documentation label Oct 15, 2025
@matiasperrone-exo matiasperrone-exo added the review Need reviewing from the developer label Nov 10, 2025
@matiasperrone-exo matiasperrone-exo removed the review Need reviewing from the developer label Nov 24, 2025
@matiasperrone-exo matiasperrone-exo force-pushed the feature/openapi-documentation--oauth2summitmetricsapicontroller branch 4 times, most recently from 7e87cef to 14414cc Compare November 26, 2025 21:57
Copy link

@caseylocker caseylocker left a comment

Choose a reason for hiding this comment

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

@matiasperrone-exo

Wrong namespace in security schema:
// Current (line 3)
namespace App\Swagger\Security;

// Should be
namespace App\Swagger\schemas;

Wrong x: parameter key (3 times):
x: [
"authz_groups" => [IGroup::SummitAccessControl]
],

// Should be
x: [
"required-groups" => [IGroup::SummitAccessControl]
],

Wrong paths for member schedule metrics endpoints: Per routes/api_v1.php, the actual routes are:
PUT {summit_id}/members/{member_id}/schedule/{event_id}/enter
POST {summit_id}/members/{member_id}/schedule/{event_id}/leave
// Current (line 214) - extra "/metrics/"
path: "/api/v1/summits/{id}/members/{member_id}/schedule/{event_id}/metrics/enter"

// Should be
path: "/api/v1/summits/{id}/members/{member_id}/schedule/{event_id}/enter"

// Current (line 285) - extra "/metrics/"
path: "/api/v1/summits/{id}/members/{member_id}/schedule/{event_id}/metrics/leave"

// Should be
path: "/api/v1/summits/{id}/members/{member_id}/schedule/{event_id}/leave"

@matiasperrone-exo matiasperrone-exo force-pushed the feature/openapi-documentation--oauth2summitmetricsapicontroller branch from cfafaf8 to d09e4a5 Compare December 3, 2025 20:42
@matiasperrone-exo
Copy link
Contributor Author

Thanks @caseylocker for the comments. Now is ready to review again

Copy link

@caseylocker caseylocker left a 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

Copy link
Collaborator

@smarcet smarcet left a comment

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 b18eede into main Dec 3, 2025
3 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.

5 participants