Skip to content

Conversation

@matiasperrone-exo
Copy link
Contributor

@matiasperrone-exo matiasperrone-exo self-assigned this Oct 13, 2025
@matiasperrone-exo matiasperrone-exo added the documentation Improvements or additions to documentation label Oct 13, 2025
@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---summit---oauth2summitmediauploadtypeapicontroller branch 3 times, most recently from a6557c2 to a38ef4e Compare October 14, 2025 17:53
@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---oauth2summitmediauploadtypeapicontroller branch from af04bd8 to 3eb31b7 Compare November 10, 2025 21:08
@matiasperrone-exo matiasperrone-exo removed the review Need reviewing from the developer label Nov 18, 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.

@matiasperrone-exo Parameter mismatch on all 8 endpoints.
// Path uses {id}
path: "/api/v1/summits/{id}/media-upload-types"

// But parameter says 'summit_id' in 'query' - WRONG!
new OA\Parameter(
name: 'summit_id', // Should be 'id'
in: 'query', // Should be 'path'
required: false, // Should be true
...
),

// Correct definition:
new OA\Parameter(
name: 'id',
in: 'path',
required: true,
description: 'Summit ID',
schema: new OA\Schema(type: 'integer')
),

Add operanIds to all the endpoint definitions

Endpoint Suggested operationId
GET /media-upload-types getAllMediaUploadTypes
GET /media-upload-types/{id} getMediaUploadType
POST /media-upload-types createMediaUploadType
PUT /media-upload-types/{id} updateMediaUploadType
DELETE /media-upload-types/{id} deleteMediaUploadType
PUT .../presentation-types/{id} addMediaUploadTypeToPresentationType
DELETE .../presentation-types/{id} removeMediaUploadTypeFromPresentationType
POST .../all/clone/{to_summit_id} cloneMediaUploadTypes

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.

I may not have been clear enough regarding the 'id' in 'query' being wrong and that it's actually in the 'path' for all 8 endpoints.

Example:

new OA\Parameter(
    name: 'id',
    in: 'query',      //  WRONG - should be 'path'
    required: false,  //  WRONG - path params must be required: true
    schema: new OA\Schema(type: 'integer'),
    description: 'The summit ID'
), 

Should be:

new OA\Parameter(
    name: 'id',
    in: 'path',       //  Correct
    required: true,   //  Path parameters MUST be required
    schema: new OA\Schema(type: 'integer'),
    description: 'The summit ID'
),

Affected lines:
Lines 94-100 (getAllMediaUploadTypes)
Lines 187-193 (getMediaUploadType)
Lines 253-259 (createMediaUploadType)
Lines 304-310 (updateMediaUploadType)
Lines 362-368 (deleteMediaUploadType)
Lines 531-537 (addMediaUploadTypeToPresentationType)
Lines 610-616 (removeMediaUploadTypeFromPresentationType)
Lines 688-694 (cloneMediaUploadTypes)

@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---summit---oauth2summitmediauploadtypeapicontroller branch from 69ff1e9 to 3a57467 Compare December 4, 2025 18:22
@matiasperrone-exo
Copy link
Contributor Author

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

@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---summit---oauth2summitmediauploadtypeapicontroller branch from 32fa05f to 7dd6e2d Compare December 5, 2025 14:39
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
Copy link
Collaborator

smarcet commented Dec 16, 2025

@matiasperrone please rebase against main and fix conflicts many thanks

@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---apis---protected---summit---oauth2summitmediauploadtypeapicontroller branch from 7dd6e2d to cb17dad Compare December 29, 2025 20:03
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