Skip to content

Conversation

@andrestejerina97
Copy link
Contributor

@andrestejerina97 andrestejerina97 force-pushed the feature/add-openapi-documentation-to-controller-oauthsummitrsvpttemplatepicontroller branch from e45382e to 8ff0971 Compare October 23, 2025 18:48
@andrestejerina97 andrestejerina97 self-assigned this Oct 31, 2025
@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller-oauthsummitrsvpttemplatepicontroller branch from 0050df6 to 3adbb6a Compare December 8, 2025 18:46
@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller-oauthsummitrsvpttemplatepicontroller branch from f0ae2ad to e7f96e9 Compare December 8, 2025 18:49
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.

Path Mismatch (Line 375) in app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitRSVPTemplatesApiController.php
Documented: /api/v1/summits/{id}/rsvp-templates/metadata
Actual Route: /api/v1/summits/{id}/rsvp-templates/questions/metadata The route is nested under the questions prefix (api_v1.php:540-541).

Namespace is missing from app/Swagger/SummitRSVPTemplate.php
namespace App\Swagger\schemas;

Inconsistent Security Scope (Line 443-446)
deleteRSVPTemplate uses ReadAllSummitData but other DELETE operations correctly use:

SummitScopes::WriteSummitData,
SummitScopes::WriteRSVPTemplateData,

This should be consistent with other delete operations.

@matiasperrone-exo matiasperrone-exo added the documentation Improvements or additions to documentation label Dec 9, 2025
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.

Please replace the raw response codes with the equivalent constants.
response: 200 should be Response::HTTP_OK
response: 201 should be Response::HTTP_CREATED
response: 204 should be Response::HTTP_NO_CONTENT

@matiasperrone-exo
Copy link
Contributor

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.

Approved

@smarcet smarcet force-pushed the main branch 4 times, most recently from c6ecdd0 to 728ae67 Compare December 17, 2025 00:43
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