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

feat(api): API documentation with Swagger UI #49791

Merged
merged 5 commits into from
Mar 24, 2023

Conversation

Nirajn2311
Copy link
Member

Checklist:

Closes #49712

To view the documentation, start the server and visit http://localhost:3000/documentation/. One thing to note is even though if you set the session ID the page can't send the request with the cookie for security reasons regarding modifying request headers in browser(More details here swagger-api/swagger-js#1163).

Also do we need the documentation page for production?

@Nirajn2311 Nirajn2311 requested a review from a team as a code owner March 21, 2023 20:34
@github-actions github-actions bot added the platform: api Server application that needs familiarity with Express, Loopback, MongoDB etc. label Mar 21, 2023
@ghost
Copy link

ghost commented Mar 21, 2023

👀 Review this PR in a CodeSee Review Map

View the CodeSee Map of this change

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

Copy link
Member

@ShaunSHamilton ShaunSHamilton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one, Niraj

@ShaunSHamilton
Copy link
Member

Also do we need the documentation page for production?

I do not think it matters much, but, if not, then a nice-to-have would be on .dev

Copy link
Contributor

@ojeytonwilliams ojeytonwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we want to enable this in production? I thought it was just a tool to help us develop the api.

@ShaunSHamilton
Copy link
Member

Why would we want to enable this in production? I thought it was just a tool to help us develop the api.

We are hoping to provide a REST API for other tools to use. So, our API should be documented somewhere.

@Nirajn2311
Copy link
Member Author

I thought it was just a tool to help us develop the api.

To be clear, irrespective of whether we use the UI in production or development, we can't execute any of the endpoints coz of the restriction I mentioned in the description. It will just be like an API reference page.

api/index.ts Outdated Show resolved Hide resolved
Copy link
Member

@ShaunSHamilton ShaunSHamilton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, Niraj

@raisedadead raisedadead enabled auto-merge (squash) March 24, 2023 06:59
@raisedadead raisedadead added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Mar 24, 2023
Copy link
Contributor

@ojeytonwilliams ojeytonwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reservations about the approach we're taking to env vars, but that's broader than this PR.

Putting them aside for now, this LGTM. Nice work @Nirajn2311 🚀

@raisedadead raisedadead merged commit 1dcba78 into freeCodeCamp:main Mar 24, 2023
@Nirajn2311 Nirajn2311 deleted the feat/swagger-api-docs branch March 27, 2023 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: api Server application that needs familiarity with Express, Loopback, MongoDB etc. status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setup OpenAPI 3.0 documentation with Swagger
4 participants