Skip to content

Conversation

@matiasperrone-exo
Copy link
Contributor

@matiasperrone-exo matiasperrone-exo self-assigned this Oct 1, 2025
@smarcet smarcet force-pushed the main branch 5 times, most recently from c94fc68 to 9a8387b Compare October 2, 2025 17:58
@matiasperrone-exo matiasperrone-exo added the documentation Improvements or additions to documentation label Oct 7, 2025
@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---main---oauth2paymentgatewayprofileapicontroller branch 2 times, most recently from 89f24ad to 1b2743b Compare October 8, 2025 21:40
@matiasperrone-exo matiasperrone-exo changed the title Feature | Extend Swagger Coverage for controller Apis/Protected/Summit/OAuth2PaymentGatewayProfileApiController.php Feature | Extend Swagger Coverage for controller OAuth2PaymentGatewayProfileApiController.php Oct 8, 2025
@matiasperrone-exo matiasperrone-exo changed the title Feature | Extend Swagger Coverage for controller OAuth2PaymentGatewayProfileApiController.php Feature | Extend Swagger Coverage for controller OAuth2PaymentGatewayProfileApiController Oct 14, 2025
@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---main---oauth2paymentgatewayprofileapicontroller branch from 20d40b7 to 44e5dd7 Compare October 14, 2025 17:15
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 Looking at this I don't see "oauth2_security_scope" defined anywhere.

@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---main---oauth2paymentgatewayprofileapicontroller branch from 0c2cca6 to cc60f7c Compare October 31, 2025 14:56
@matiasperrone-exo
Copy link
Contributor Author

@matiasperrone-exo Looking at this I don't see "oauth2_security_scope" defined anywhere.

@caseylocker thanks! I added a new security schema and replaced with the correct scopes

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.

@matiasperrone-exo please review comments

@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---main---oauth2paymentgatewayprofileapicontroller branch from ecdc3d9 to f179b81 Compare November 7, 2025 18:24
@matiasperrone-exo matiasperrone-exo added question Further information is requested invalid This doesn't seem right review Need reviewing from the developer and removed question Further information is requested labels Nov 7, 2025
@matiasperrone-exo matiasperrone-exo removed the review Need reviewing from the developer label Nov 18, 2025
@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---main---oauth2paymentgatewayprofileapicontroller branch from 627f001 to 752af36 Compare November 24, 2025 14:19
@matiasperrone-exo matiasperrone-exo removed the invalid This doesn't seem right label Nov 24, 2025
@matiasperrone-exo
Copy link
Contributor Author

@caseylocker Please review again. All the conflicts were resolved, the security schema created and also the related security roles were included.
The serializer was review to avoid missing fields.

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 The format of the security definitions is incorrect. Missing inner array brackets around the scopes:

// Current (INVALID)
security: [['summit_payment_gateway_oauth2' =>
    SummitScopes::ReadAllSummitData,
    SummitScopes::ReadPaymentProfiles
]],

// Required (CORRECT)
security: [['summit_payment_gateway_oauth2' => [
    SummitScopes::ReadAllSummitData,
    SummitScopes::ReadPaymentProfiles
]]],

We're also missing the operationId on all endpoints. Endpoints and suggestions...

GET /api/v1/summits/{id}/payment-gateway-profiles getAllPaymentGatewayProfiles
GET /api/v1/summits/{id}/payment-gateway-profiles/{payment_profile_id} getPaymentGatewayProfile
POST /api/v1/summits/{id}/payment-gateway-profiles createPaymentGatewayProfile
PUT /api/v1/summits/{id}/payment-gateway-profiles/{payment_profile_id} updatePaymentGatewayProfile
DELETE /api/v1/summits/{id}/payment-gateway-profiles/{payment_profile_id} deletePaymentGatewayProfile

@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---main---oauth2paymentgatewayprofileapicontroller branch 2 times, most recently from 0250dff to 0426fe9 Compare December 5, 2025 14:20
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 double check your security scopes syntax. 4 are missing array brackets.

// WRONG
'summit_payment_gateway_oauth2' =>
    SummitScopes::ReadAllSummitData,
SummitScopes::ReadPaymentProfiles
// Correct
security: [
    [
        'summit_payment_gateway_oauth2' => [
            SummitScopes::ReadAllSummitData,
            SummitScopes::ReadPaymentProfiles
        ]
    ]
],

@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---main---oauth2paymentgatewayprofileapicontroller branch from 0426fe9 to e33a3c4 Compare December 11, 2025 20:27
@matiasperrone-exo
Copy link
Contributor Author

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

I fixed the missing brackets and the rebase conflicts

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

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 7d1b6aa into main Dec 16, 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