-
Notifications
You must be signed in to change notification settings - Fork 10
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
API v1: all endpoints conform to OpenAPI spec #65
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
… valid API key is present
TODO: try to fix CORS issue from Discord (will do this after initial review) |
@@ -0,0 +1,246 @@ | |||
import prisma from "../../prisma" |
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.
(fyi, useful for changes like these to know if you just moved it to a new file or if there are any changes, it's tricky to notice small changes otherwise)
.optional(), | ||
}), | ||
) | ||
.output(QuestionSchema) |
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.
@jonnyspicer This QuestionSchema
doesn't include questions.forecasts
, question.options
, sharedWithLists
, comments
, tags
, etc. The same is true for getQuestionsApiProcedure
below, and I think other cases where we use the zod generated schemas as outputs in this file.
This is a moderate issue, because:
-
The schema in the openAPI JSON (as shown on
/api-setup
doesn't include these properties, which are important (e.g., most usecases of getQuestion for MCQs will want to see the options, and in general people will want to see the forecasts on the question). -
Additionally, it looks like the Swagger UI only shows parts of the output that conform to the output spec in the web UI when you click execute, so it omits forecasts, options, tags, etc. However, when I test the endpoint using curl I do see the full output.
Do you have a suggestion for how to fix the schema not including prisma-include
d relations?
I think a temporary fix to allow us to merge this would be to set the schema as .output(z.any())
, though I agree that it would be nice to have the actual schema shown in the UI setup page without needing to run an example.
Pull Request: API v1 - all endpoints conform to OpenAPI spec
Related Issue
[Asana ref](API v1: all endpoints conform to OpenAPI spec)
Changes Made
question_router.tsx
into their own filesTesting
Wrote basic unit tests for public-facing APIs, tested all endpoints in Preview with Postman