Skip to content
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

Add api schemas and swagger ui support #2351

Merged
merged 27 commits into from
Jul 4, 2023
Merged

Add api schemas and swagger ui support #2351

merged 27 commits into from
Jul 4, 2023

Conversation

knolleary
Copy link
Member

@knolleary knolleary commented Jun 27, 2023

Part of #1556

Raised as a Draft to get some early feedback on the general approach.

Description

This adds the initial Swagger UI and adds schemas to the authentication, user and teams routes.

  • The Swagger doc is served from /api/json
  • The Swagger UI is browsable (without authentication) at /api/ (the trailing slash is required... may tidy that up)

This starts to rework how the forge/db/views are defined - so that we can define view schemas alongside the code that creates them. The ultimate goal will be to allow fastify do more work using the schema, but that is out of scope for this work.

I've removed app.db.views.User.shortProfile and replaced it with the existing app.db.views.User.userSummary - trying to maximise view reuse.


In terms of testing, this doesn't add any new testable functionality other than the Swagger generation. The main risk is it results in a field being stripped from a response that was previously there, and it's an edge case not covered by the existing tests. I'm doing various bits of UI click testing in addition to exercise the routes as well.

Note that the changes in #2349 will need taking into account as well once that is reviewed and merged.


API Files updated (under forge/routes/api):

@knolleary
Copy link
Member Author

Looks like I've tripped over failing to update package-log.json...

@Pezmc
Copy link
Contributor

Pezmc commented Jun 28, 2023

@knolleary Let's frame it as you tested the validation was working!

@knolleary knolleary marked this pull request as ready for review June 29, 2023 15:03
@knolleary
Copy link
Member Author

Final update - have removed the 'Try it out' button from swagger ui as the auth side is a bit funky at this stage.

Also hidden all of the non-api 'authentication' routes.

Finally, figured out how to order the tags so have done so.

@Pezmc all good to review (although you have done most of it via the child-PRs)

@knolleary knolleary requested a review from Pezmc June 29, 2023 16:20
@knolleary
Copy link
Member Author

Oh - one more thing... worked out how to document the auth schemes for the routes, but the swagger UI experience isn't ideal IMHO. So have left it in place, but commented out as it isn't entirely obvious to figure out.

@Pezmc
Copy link
Contributor

Pezmc commented Jun 30, 2023

Yes, realistically I'm not able to go through this with a fine comb, so have tested locally and given a quick skim over.

@knolleary knolleary merged commit 19223f4 into main Jul 4, 2023
@knolleary knolleary deleted the 1556-api-docs branch July 4, 2023 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants