Skip to content

Conversation

@matiasperrone-exo
Copy link
Contributor

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

@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---summit---oauth2summitpresentationactionapicontroller branch from 93df54d to aac6838 Compare October 9, 2025 15:17
@matiasperrone-exo matiasperrone-exo self-assigned this Oct 9, 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---oauth2summitpresentationactionapicontroller branch from 4af6cc1 to aa008ca Compare October 14, 2025 17:45
@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---summit---oauth2summitpresentationactionapicontroller branch from d8c91c9 to 51bc32c Compare October 14, 2025 17:46
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.

'oauth2_security_scope' nonexistant issue again. Please resubmit when fixed/added.

@matiasperrone-exo matiasperrone-exo added the review Need reviewing from the developer label Nov 10, 2025
@matiasperrone-exo
Copy link
Contributor Author

@caseylocker The security schema for the controller was created and placed into its own file

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

Namespace issue.
namespace App\Swagger\schemas; // ← Correct (not App\Swagger\Security)

Missing operationId on Both Endpoints
operationId: 'completePresentationAction', // ← Add this
operationId: 'incompletePresentationAction', // ← Add this

Invalid anyOf Usage for Expandable Properties in SummitPresentationSchemas.php
anyOf means 'value must match exactly ONE of these schemas' (for polymorphism). Expandable properties are not polymorphic - they're conditionally included based on query parameters. The following example with also fix the invalid type references.
// Example of fix
properties: [
new OA\Property(property: 'id', type: 'integer', example: 1),
new OA\Property(property: 'is_completed', type: 'boolean', example: true),
new OA\Property(property: 'created', type: 'integer', example: 1633024800, description: 'Unix timestamp when created'),
new OA\Property(property: 'last_edited', type: 'integer', example: 1633111200, description: 'Unix timestamp when last updated'),
new OA\Property(
property: 'presentation_id',
type: 'integer',
example: 10,
description: 'Presentation ID. Use ?expand=presentation to get full object'
),
new OA\Property(
property: 'type_id',
type: 'integer',
example: 5,
description: 'Action type ID. Use ?expand=type to get full object'
),
new OA\Property(
property: 'created_by_id',
type: 'integer',
nullable: true,
example: 42,
description: 'Created by user ID. Use ?expand=created_by to get full object'
),
new OA\Property(
property: 'updated_by_id',
type: 'integer',
nullable: true,
example: 42,
description: 'Updated by user ID. Use ?expand=updated_by to get full object'
),
],

@matiasperrone-exo
Copy link
Contributor Author

matiasperrone-exo commented Nov 24, 2025

@caseylocker please review this PR again. The related "expand" and "relations" where introduced as mentioned.

The missing, and not included Models are: Presentation, SummitEventType. These two are huge. Let me know if you need to include these too.

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 Looks good, approved. @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 ad60772 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