-
Notifications
You must be signed in to change notification settings - Fork 2
Feature | Extend Swagger Coverage for controller OAuth2CompaniesApiController
#399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
e780bbf to
6665d20
Compare
dd0e157 to
b84d0d8
Compare
|
@smarcet The security was added |
caseylocker
left a comment
There was a problem hiding this 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.
9dc5c5e to
827fa1d
Compare
8195eeb to
f0ce7ad
Compare
caseylocker
left a comment
There was a problem hiding this 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.
smarcet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Task
Ref: https://app.clickup.com/t/86b6wkh6m