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

type(docs): Update GraphQL document (support 3.11.0) #4791

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

suyeon-jung-dev
Copy link
Contributor

@suyeon-jung-dev suyeon-jung-dev commented Jul 25, 2024

Proposed changes

This PR adds:

  • Replacing the GraphQL docs dociql with spectaql
    • It was created using spectaQL tool, customized similarly to a previous version of the GraphQL documents.
  • Updating GraphQL docs v3.9.x and higher (3.9.x, 3.10.x, 3.11.x)
  • Adding common error response information
  • Update the latest version of GraphQL docs URL (v3.11.x) on Litmus Documentation Page (/litmus/experiments/api/contents)

Solved #4771

GraphQL API docs demo v3.9.0: https://suyeon-jung-dev.github.io/litmus/graphql/v3.9.0/api.html
GraphQL API docs demo v3.10.0: https://suyeon-jung-dev.github.io/litmus/graphql/v3.10.0/api.html
GraphQL API docs demo v3.11.0: https://suyeon-jung-dev.github.io/litmus/graphql/v3.11.0/api.html

image

Types of changes

What types of changes does your code introduce to Litmus? Put an x in the boxes that apply

  • New feature (non-breaking change which adds functionality)
  • Bugfix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices applies)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING doc
  • I have signed the commit for DCO to be passed.
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added necessary documentation (if appropriate)

Dependency

  • Please add the links to the dependent PR need to be merged before this (if any).

Special notes for your reviewer:

@suyeon-jung-dev suyeon-jung-dev changed the title Feature/update api docs 3.9.1 type(docs): Updated Auth API docs to v3.9.1 Jul 25, 2024
@namkyu1999 namkyu1999 changed the title type(docs): Updated Auth API docs to v3.9.1 type(docs): Update Auth & GraphQL API docs to v3.9.1 Jul 26, 2024
@suyeon-jung-dev suyeon-jung-dev changed the title type(docs): Update Auth & GraphQL API docs to v3.9.1 type(docs): Update GraphQL API docs to v3.9.2 Aug 8, 2024
@suyeon-jung-dev suyeon-jung-dev changed the title type(docs): Update GraphQL API docs to v3.9.2 type(docs): Update GraphQL document (support 3.9.2) Aug 8, 2024
@suyeon-jung-dev
Copy link
Contributor Author

suyeon-jung-dev commented Aug 8, 2024

Actually there is a thing I forgot to tell you guys.

To create with the spactaQL tool, we need to change one of Query/Mutation/Subscriptions to parent type.
You can see this commit.
The reason why we need to do this, I think the graphql/validation/validate.js library in spectaql couldn't generate with no-parent type of Query/Mutation/Subscription SDL files.

Do you guys think this method seems fine?

@neelanjan00
Copy link
Member

neelanjan00 commented Aug 19, 2024

@suyeon-jung-dev Here are a couple of things we should add:

  • A window to try the API for a given server URL (similar to Swagger).
  • Provide examples of success and failure cases while documenting them.

LMK if this sounds good?

@suyeon-jung-dev
Copy link
Contributor Author

suyeon-jung-dev commented Aug 19, 2024

Hi, @neelanjan00 . If we need to add these two things, I'll look for other tools. Because SpectaQL doesn't support them.

  • A window to try the API for a given server URL (similar to Swagger).
    SpectaQL supports only api end point paths like below.
Screenshot 2024-08-19 at 6 30 52 PM
  • Provide examples of success and failure cases while documenting them
    SpectaQL supports only success case.
Screenshot 2024-08-19 at 6 43 42 PM

Thank you for your feedback :)

@neelanjan00
Copy link
Member

Can you please check if you can find any tool that can help us achieve this? This will be very useful! Thanks!

@suyeon-jung-dev
Copy link
Contributor Author

suyeon-jung-dev commented Sep 8, 2024

@suyeon-jung-dev Here are a couple of things we should add:

  • A window to try the API for a given server URL (similar to Swagger).
  • Provide examples of success and failure cases while documenting them.

LMK if this sounds good?

Hi @neelanjan00

I looked into the two things you asked about earlier, but I couldn't find any tools that have all the features we need. 🥲

The tool we previously used, dociql, is no longer being updated — its last update was 2 years ago. Additionally, dociql can only be installed in a Python 2 environment. Even if we were to continue using it, we would still need to create a config.yaml file manually, which requires typing out every line.

Regarding the feature "A window to try the API for a given server URL (similar to Swagger)," I have some concerns about potential security issues. We can also use GraphQL Playground for this purpose instead.

Considering all of this, I think SpectaQL is the best option.

@neelanjan00
Copy link
Member

@suyeon-jung-dev Here are a couple of things we should add:

  • A window to try the API for a given server URL (similar to Swagger).
  • Provide examples of success and failure cases while documenting them.

LMK if this sounds good?

Hi @neelanjan00

I looked into the two things you asked about earlier, but I couldn't find any tools that have all the features we need. 🥲

The tool we previously used, dociql, is no longer being updated — its last update was 2 years ago. Additionally, dociql can only be installed in a Python 2 environment. Even if we were to continue using it, we would still need to create a config.yaml file manually, which requires typing out every line.

Regarding the feature "A window to try the API for a given server URL (similar to Swagger)," I have some concerns about potential security issues. We can also use GraphQL Playground for this purpose instead.

Considering all of this, I think SpectaQL is the best option.

Okay sure, I guess we can just document the various success and failure cases for each API. We can skip the Swagger-like interface in favour of the in-built GraphQL playground.

@namkyu1999
Copy link
Member

namkyu1999 commented Sep 18, 2024

Okay sure, I guess we can just document the various success and failure cases for each API. We can skip the Swagger-like interface in favour of the in-built GraphQL playground.

Hi @neelanjan00 , I tried to find many alternatives, but it seems unnecessary to create an error/success cases for every single request. (we already have success cases)

  1. All open source that auto-generates GraphQL docs (DociQL, MagiDoc, SpectaQL...) make docs based on 'schema'.

In the case of an error, it depends on implementing the resolver function (not schema), so we have to check it manually to add it to the API docs for the error case. (It's time time-consuming job and hard to maintain)

  1. Unlike REST API, GraphQL returns 200 OKs even in case of an error

All GraphQL queries and mutations return errors in a structure like List. In our codebase, we only return a single error. It also differs only in the message part of the error and does not return any other metadata or data.


IMO, it would be much more efficient to add a page for common errors. This is because most errors coming from GraphQL servers are similar(authentication failure, MongoDB CRUD failure, etc.). SpectaQL supports adding pages using Markdown. So how about using this feature?

@neelanjan00
Copy link
Member

Okay sure, I guess we can just document the various success and failure cases for each API. We can skip the Swagger-like interface in favour of the in-built GraphQL playground.

Hi @neelanjan00 , I tried to find many alternatives, but it seems unnecessary to create an error/success cases for every single request. (we already have success cases)

  1. All open source that auto-generates GraphQL docs (DociQL, MagiDoc, SpectaQL...) make docs based on 'schema'.

In the case of an error, it depends on implementing the resolver function (not schema), so we have to check it manually to add it to the API docs for the error case. (It's time time-consuming job and hard to maintain)

  1. Unlike REST API, GraphQL returns 200 OKs even in case of an error

All GraphQL queries and mutations return errors in a structure like List. In our codebase, we only return a single error. It also differs only in the message part of the error and does not return any other metadata or data.

IMO, it would be much more efficient to add a page for common errors. This is because most errors coming from GraphQL servers are similar(authentication failure, MongoDB CRUD failure, etc.). SpectaQL supports adding pages using Markdown. So how about using this feature?

Sounds good, this is what I meant as well actually, sorry for the confusion if any. Yes, having some sort of documentation around the success and most common failure messages for at least the most common mutations and queries will be a good way forward.

@suyeon-jung-dev suyeon-jung-dev marked this pull request as draft September 23, 2024 15:19
@suyeon-jung-dev
Copy link
Contributor Author

suyeon-jung-dev commented Sep 25, 2024

I deployed a demo using the branch feature/update-api-docs-3.9.1-deploy-test on my repository.

You can check 3.9.x, 3.10.x, 3.11.x version of GraphQL docs demo with those links.

Finally, the changes I made include:

  1. Replacing the GraphQL docs dociql with spectaql
  2. Updating GraphQL docs v3.9.x and higher (3.9.x, 3.10.x, 3.11.x)
  3. Adding common error response information
  4. Update the latest version of GraphQL docs URL (v3.11.x) on Litmus Documentation Page (/litmus/experiments/api/contents)

[Common Error Response page demo]
image

@suyeon-jung-dev suyeon-jung-dev marked this pull request as ready for review September 25, 2024 14:21
@namkyu1999 namkyu1999 changed the title type(docs): Update GraphQL document (support 3.9.2) type(docs): Update GraphQL document (support 3.11.0) Sep 25, 2024
Copy link
Member

@namkyu1999 namkyu1999 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

4 participants