Repository navigation
Correct OpenAPI specifications - #2453
Conversation
There were a series of cases where the OpenAPI specification did not align with our actual implementations. Issue #2451 OpenAPI spec deviates from real payload signatures
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2453 +/- ##
=======================================
Coverage 93.59% 93.59%
=======================================
Files 303 303
Lines 3839 3839
Branches 517 516 -1
=======================================
Hits 3593 3593
Misses 245 245
Partials 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
🔎 A preview deployment of this pull request is available at https://utilities.philanthropydatacommons.org/pdc-service-preview-2453-dudj4/ (and upstream https://pdc-service-preview-2453-dudj4.ondigitalocean.app) assuming this PR is still open. 🔍 |
There was a problem hiding this comment.
Pull request overview
This pull request corrects several OpenAPI component schemas so the published API specification matches the service’s actual payload shapes (per #2451), improving client generation and validation accuracy.
Changes:
- Fix incorrect/missing required fields and references across multiple schemas (e.g.,
Organization.funder,Source.required,ProposalVersion.required,Opportunity.required). - Correct schema modeling issues (e.g.,
BulkUploadTask.logsas an array withitems, recursiveBulkUploadLogDetails.cause, dynamicPresignedPost.fieldsviaadditionalProperties). - Add a changelog entry summarizing the OpenAPI-spec alignment fixes.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/openapi/components/schemas/User.json | Make keycloakUserName nullable to match actual payloads. |
| src/openapi/components/schemas/Source.json | Remove non-existent relatedEntityId from required. |
| src/openapi/components/schemas/ProposalVersion.json | Require source and createdBy in responses. |
| src/openapi/components/schemas/PresignedPost.json | Allow dynamic S3 POST form fields via additionalProperties. |
| src/openapi/components/schemas/Organization.json | Fix funder schema reference to Funder. |
| src/openapi/components/schemas/Opportunity.json | Require funderShortCode and funder in responses. |
| src/openapi/components/schemas/FunderCollaborativeInvitation.json | Require invitationStatus in responses. |
| src/openapi/components/schemas/BulkUploadTask.json | Fix logs to be an array with items. |
| src/openapi/components/schemas/BulkUploadLogDetails.json | Model recursive cause as a self-reference. |
| src/openapi/components/schemas/BulkUploadLog.json | Reference BulkUploadLogDetails directly for details. |
| src/openapi/components/schemas/BaseFieldLocalization.json | Align nullability/readOnly/required fields with actual payloads. |
| src/openapi/components/schemas/BaseField.json | Fix readOnly keyword spelling for shortCode. |
| CHANGELOG.md | Document the OpenAPI specification corrections under Unreleased. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bickelj
left a comment
bickelj
left a comment
There was a problem hiding this comment.
I quickly skimmed the changes here without trying them (for example, in the deploy preview environment) and without comparing to internal types but I see no red flags. This looks like a lot of good corrections, thank you!
I think we want a version increase so the SDK and its users will benefit.
| }, | ||
| "funder": { | ||
| "$ref": "./DataProvider.json", | ||
| "$ref": "./Funder.json", |
| "type": "string", | ||
| "type": ["string", "null"], | ||
| "description": "The full name of the user (described in OIDC 1.0 'name' standard claim) as of most recent logged-in service activity.", | ||
| "example": "John Doe" |
There was a problem hiding this comment.
I wonder if this is readOnly too.
|
|
||
| ## Unreleased | ||
|
|
||
| ### Fixed |
There was a problem hiding this comment.
If the SDK is built using the OpenAPI spec it would behoove us to update the version here.
There were a series of cases where the OpenAPI specification did not align with our actual implementations.
Some of these were identified by users, others were identified by an audit.
Resolves #2451