Repository navigation
Consistently use component responses - #2445
Merged
Merged
Conversation
Contributor
|
🔎 A preview deployment of this pull request is available at https://utilities.philanthropydatacommons.org/pdc-service-preview-2445-9v2ck/ (and upstream https://pdc-service-preview-2445-9v2ck.ondigitalocean.app) assuming this PR is still open. 🔍 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2445 +/- ##
=======================================
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:
|
There was a problem hiding this comment.
Pull request overview
This PR standardizes OpenAPI error response documentation by moving common error responses into reusable components/responses and updating many path specs to reference them, aiming to reduce duplicated inline response definitions ahead of upcoming status-code consistency work.
Changes:
- Added reusable error response component files (
Unauthorized,NotFound,UnprocessableEntity,InputValidation,Conflict, plusInternalServerError) and registered several of them insrc/openapi/api.json. - Updated many OpenAPI path specs to use shared response components and to consistently document
401responses on authenticated endpoints. - Removed the unused
DatabaseErrorresponse and updated the changelog to reflect the documentation-only standardization.
Reviewed changes
Copilot reviewed 46 out of 46 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| src/openapi/paths/users.json | Reformats parameter refs and adds a 401 response via shared component. |
| src/openapi/paths/sources.json | Adds/standardizes 401 response to use shared component. |
| src/openapi/paths/source.json | Adds 401 response and switches 422 to shared UnprocessableEntity. |
| src/openapi/paths/proposalVersions.json | Switches 401/422 to shared components (with per-endpoint description override attempted). |
| src/openapi/paths/proposalVersion.json | Switches 401 to shared component. |
| src/openapi/paths/proposals.json | Reformats parameter refs and switches 401/409/422 to shared components (with per-endpoint description overrides attempted). |
| src/openapi/paths/proposal.json | Adds 401 response via shared component. |
| src/openapi/paths/platformProviderResponses.json | Adds 401 and switches 400/401 to shared components (with per-endpoint description override attempted). |
| src/openapi/paths/permissionGrants.json | Switches 401 and 400 to shared components (with per-endpoint description override attempted). |
| src/openapi/paths/permissionGrant.json | Switches 400/401 responses to shared components (with per-endpoint description overrides attempted). |
| src/openapi/paths/organization.json | Adds 401 response via shared component. |
| src/openapi/paths/opportunity.json | Adds 401 response via shared component. |
| src/openapi/paths/opportunities.json | Reformats parameter refs and switches 401 to shared component. |
| src/openapi/paths/funders.json | Reformats parameter refs and adds 401 response via shared component. |
| src/openapi/paths/funderCollaborativeMembers.json | Reformats parameter refs and adds 401 response via shared component. |
| src/openapi/paths/funderCollaborativeMember.json | Adds 401 responses and switches 400 to shared InputValidation. |
| src/openapi/paths/funderCollaborativeInvitationsSent.json | Reformats parameter refs and switches 401 to shared component. |
| src/openapi/paths/funder.json | Adds/switches 401 responses to shared component. |
| src/openapi/paths/dataProviders.json | Reformats parameter refs and adds 401 response via shared component. |
| src/openapi/paths/dataProvider.json | Adds/switches 401 responses to shared component. |
| src/openapi/paths/changemakers.json | Reformats parameter refs and switches 401 responses to shared component. |
| src/openapi/paths/changemakerProposals.json | Reformats parameter refs and switches 401 responses to shared component. |
| src/openapi/paths/changemakerFiscalSponsor.json | Switches 400/401/422 to shared components (with per-endpoint description overrides attempted). |
| src/openapi/paths/changemakerFieldValues.json | Switches 401/409/422 to shared components (with per-endpoint description overrides attempted). |
| src/openapi/paths/changemakerFieldValueBatches.json | Switches 401/409 to shared components (with per-endpoint description override attempted). |
| src/openapi/paths/changemakerFieldValueBatch.json | Switches 401 to shared component. |
| src/openapi/paths/changemakerFieldValue.json | Switches 401 to shared component. |
| src/openapi/paths/changemaker.json | Switches 400/401/404 to shared components (with per-endpoint description overrides attempted). |
| src/openapi/paths/bulkUploadTasks.json | Reformats parameter refs and switches 401 to shared component. |
| src/openapi/paths/baseFieldsCopyTasks.json | Reformats parameter refs and switches 401 to shared component. |
| src/openapi/paths/baseFields.json | Reformats parameter refs and adds 401 response via shared component. |
| src/openapi/paths/baseFieldLocalizations.json | Adds 401 and switches 404 to shared component (with per-endpoint description override attempted). |
| src/openapi/paths/baseFieldLocalization.json | Switches 400/401/404/409 to shared components (with per-endpoint description overrides attempted). |
| src/openapi/paths/baseField.json | Switches 401/404 to shared components (with per-endpoint description override attempted). |
| src/openapi/paths/applicationForms.json | Reformats parameter refs and switches 401/409/422 to shared components (with per-endpoint description overrides attempted). |
| src/openapi/paths/applicationFormProposalDataCsv.json | Adds 401 response via shared component. |
| src/openapi/paths/applicationForm.json | Adds 401 response via shared component. |
| src/openapi/components/responses/UnprocessableEntity.json | Adds application/json PdcError body to the shared 422 response component. |
| src/openapi/components/responses/Unauthorized.json | Adds application/json PdcError body to the shared 401 response component. |
| src/openapi/components/responses/NotFound.json | Adds application/json PdcError body to the shared 404 response component and fixes punctuation. |
| src/openapi/components/responses/InternalServerError.json | Introduces a shared 500 response component documenting a PdcError body. |
| src/openapi/components/responses/InputValidation.json | Adds application/json PdcError body to the shared 400 response component. |
| src/openapi/components/responses/DatabaseError.json | Removes the obsolete DatabaseError response component file. |
| src/openapi/components/responses/Conflict.json | Introduces a shared 409 response component documenting a PdcError body. |
| src/openapi/api.json | Registers additional reusable responses under components.responses. |
| CHANGELOG.md | Documents the OpenAPI response standardization as documentation-only. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
Copilot made 22 of the same comment (sigh) and it is also wrong -- |
hminsky2002
approved these changes
Jun 4, 2026
hminsky2002
left a comment
hminsky2002
left a comment
Contributor
There was a problem hiding this comment.
Yep looks reasonable. Lot of file changes, but just the one functional change
bickelj
approved these changes
Jun 4, 2026
bickelj
left a comment
bickelj
left a comment
Contributor
There was a problem hiding this comment.
Looks fine to me! Thank you!
Do we not use DatabaseError? I can imagine we wouldn't want to surface too gory of details but I suppose we still can within InternalServerError if we want.
Our API Specification sometimes referenced `components/responses` and sometimes had inline definitions. The responses themselves also did not have a body which resulted in redundant definitions of the body structure. This applies DRY to this attribute of our specification. Issue #2444 Avoid redundant openapi documentation
Every endpoint can potentially result in a `500` error; this should be documented in our openapi specification for completeness.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes our use of component responses more consistent in our API specification. This will help us as we make improvements to status code consistency in implementation as well.
Resolves #2444