Skip to content

Conversation

@matiasperrone-exo
Copy link
Contributor

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

@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---main---oauth2companiesapicontroller branch from e780bbf to 6665d20 Compare October 14, 2025 17:54
@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---main---oauth2companiesapicontroller branch from dd0e157 to b84d0d8 Compare November 10, 2025 22:42
@matiasperrone-exo matiasperrone-exo removed the review Need reviewing from the developer label Nov 10, 2025
@matiasperrone-exo
Copy link
Contributor Author

@smarcet The security was added

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 controller references OAuth2CompaniesApiControllerAuthSchema but the security schema file defines companies_oauth2. In controller:
security: [
[
"OAuth2CompaniesApiControllerAuthSchema" => [ // Wrong name
Should be:
security: [
[
"companies_oauth2" => [ // Matches schema file

This needs to be fixed on ALL 8 protected endpoints.

Missing operationId on all endpoints:

Endpoint Suggested operationId
GET /api/v1/companies getAllCompanies
GET /api/v1/companies/{id} getCompany
POST /api/v1/companies createCompany
PUT /api/v1/companies/{id} updateCompany
DELETE /api/v1/companies/{id} deleteCompany
POST /api/v1/companies/{id}/logo addCompanyLogo
DELETE /api/v1/companies/{id}/logo deleteCompanyLogo
POST /api/v1/companies/{id}/logo/big addCompanyBigLogo
DELETE /api/v1/companies/{id}/logo/big deleteCompanyBigLogo
GET /api/public/v1/companies getAllCompaniesPublic
GET /api/public/v1/companies/{id} getCompanyPublic

In CompaniesSchemas.php, the Company schema uses anyOf incorrectly at the root level with duplicate property names:
anyOf: [
new OA\Property(property: "sponsorships", ...), // ❌ Duplicate property name
new OA\Property(property: "sponsorships", ...),
new OA\Property(property: "project_sponsorships", ...), // ❌ Duplicate property name
new OA\Property(property: "project_sponsorships", ...),
],
Should be (each property defined once with oneOf for its value):
properties: [
// ... existing properties ...
new OA\Property(
property: "sponsorships",
description: "Array of sponsorship IDs with ?relations=sponsorships. Array of full objects with ?expand=sponsorships",
oneOf: [
new OA\Schema(type: "array", items: new OA\Items(type: "integer")),
new OA\Schema(type: "array", items: new OA\Items(ref: "#/components/schemas/SummitSponsorship"))
]
),
new OA\Property(
property: "project_sponsorships",
description: "Array of project sponsorship IDs or full objects depending on expand",
oneOf: [
new OA\Schema(type: "array", items: new OA\Items(type: "integer")),
new OA\Schema(type: "array", items: new OA\Items(ref: "#/components/schemas/ProjectSponsorshipType"))
]
),
],

There's a stray class-level PHPDoc comment between two OA\Get attributes:
#[OA\Get(
path: "/api/v1/companies/{id}",
...
)]
/**

  • Class OAuth2CompaniesApiController ← This is misplaced and incorrect
  • @Package App\Http\Controllers
    */
    #[OA\Get(
    path: "/api/public/v1/companies/{id}",
    ...
    )]
    This comment should be removed as it's not relevant to the method.

@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---main---oauth2companiesapicontroller branch 2 times, most recently from 9dc5c5e to 827fa1d Compare December 3, 2025 18:26
@matiasperrone-exo matiasperrone-exo force-pushed the feature/add-openapi-documentation-to-controller---apis---protected---main---oauth2companiesapicontroller branch from 8195eeb to f0ce7ad Compare December 3, 2025 18:55
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.

LGTM. @smarcet please merge if you agree.

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 649636d into main Dec 4, 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