Skip to content

Conversation

@matiasperrone-exo
Copy link
Contributor

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

@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---oauth2summittaxtypeapicontroller branch from 5103597 to 7d24b01 Compare October 14, 2025 17: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.

@matiasperrone-exo Doc generation is broken here like in pr 393.
Please verify existence of, or create
new OA\Parameter(ref: '#/components/parameters/page'),
new OA\Parameter(ref: '#/components/parameters/per_page'),

Also verify 'OAuth2' sec schema.

For 'SummitTaxType' look at the 'ticket_types' property.
Without relations=ticket_types: Field not present
With relations=ticket_types: Returns array of integers (IDs): [1, 2, 3]
With expand=ticket_types: Returns array of full objects
Should probably be something like:
new OA\Property(
property: 'ticket_types',
type: 'array',
nullable: true,
items: new OA\Items(type: 'integer'),
example: [1, 2, 3],
description: 'Array of ticket type IDs (only present when relations=ticket_types). When expanded with expand=ticket_types, contains full ticket type objects instead of IDs.'
),

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

@caseylocker regarding the mentioned field this is how it was handled:

new OA\Property(
            property: 'ticket_types',
            type: 'array',
            items: new OA\Items(oneOf: [
                new OA\Schema(type: 'integer'),
                new OA\Schema(type: 'SummitTicketType')
            ]),
            example: [1, 2, 3],
            description: 'Array of ticket type IDs or its Model (only present when relations=ticket_types), expanded when expand includes ticket_types.'
        ),

@matiasperrone-exo
Copy link
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
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
TaxTypesAuthSchema.php needs the namespace updated.
// Current (WRONG)
namespace App\Swagger\Security;

// Required (CORRECT)
namespace App\Swagger\schemas;

The epoch timestamp seems incorrect and will cause issues.
// Invalid - missing type attribute
new OA\Property(property: 'sales_start_date:datetime_epoch'),
new OA\Property(property: 'sales_end_date:datetime_epoch'),

// Correct
new OA\Property(property: 'sales_start_date', type: 'integer', format: 'int64', description: 'Epoch timestamp'),
new OA\Property(property: 'sales_end_date', type: 'integer', format: 'int64', description: 'Epoch timestamp'),

Missing operandIds. Examples below:

Endpoint Suggested operationId
GET /tax-types getAllTaxTypes
POST /tax-types createTaxType
GET /tax-types/{tax_id} getTaxType
PUT /tax-types/{tax_id} updateTaxType
DELETE /tax-types/{tax_id} deleteTaxType
PUT .../ticket-types/{ticket_type_id} addTaxToTicketType
DELETE .../ticket-types/{ticket_type_id} removeTaxFromTicketType

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.

The issue with ticket_types in app/Swagger/Models/SummitTaxTypeSchema.php is that "new OA\Schema(type: 'SummitTicketType')" is an invalid type reference.

Since we have the schema for ticket types you can use:

new OA\Property(
    property: 'ticket_types',
    type: 'array',
    items: new OA\Items(oneOf: [
        new OA\Schema(type: 'integer'),
        new OA\Schema(ref: '#/components/schemas/SummitTicketType')
    ]),
    example: [1, 2, 3],
    description: 'Array of ticket type IDs or its Model (only present when relations=ticket_types), expanded when expand includes ticket_types.'
),

@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---summit---oauth2summittaxtypeapicontroller branch from 2c3a71b to c56d65b Compare December 5, 2025 15:24
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 the 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---oauth2summittaxtypeapicontroller branch from 5b83f3e to a134fa4 Compare December 18, 2025 19:57
@matiasperrone-exo
Copy link
Contributor Author

@smarcet now it is rebased!

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 ab9ef82 into main Dec 22, 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.

5 participants