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

Documentation #44

Merged
merged 20 commits into from
Mar 2, 2022
Merged

Documentation #44

merged 20 commits into from
Mar 2, 2022

Conversation

AmmarBaki2
Copy link
Collaborator

@AmmarBaki2 AmmarBaki2 commented Feb 21, 2022

Added the documentation for all endpoints
I added the /global/all-items and /global/donators under Items and Users respectively because we still didn't determine what should we do with them, moving them to Global would be quite easy if we decide so.
closes #28

api-docs-ss

@AmmarBaki2 AmmarBaki2 added the documentation Improvements or additions to documentation label Feb 21, 2022
@AmmarBaki2 AmmarBaki2 self-assigned this Feb 21, 2022
Comment on lines 6 to 9
let { token } = req.cookies;
if (req?.headers?.swaggerToken) {
token = req.headers.swaggerToken;
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had to send the authentication token with a header because the Cookie or Set-Cookie headers are forbidden headers and the browser forbids swagger from setting them.
You can refer to this issue that explains the problem, and this MDN article for forbidden headers list.

@mehmetfatiherdem
Copy link
Collaborator

Hey @AmmarBaki2 good job with the docs. Would you be able to add the screenshot of the docs here? I need it for home page and also it may be good to see the docs ss here.

@AmmarBaki2
Copy link
Collaborator Author

Hey @AmmarBaki2 good job with the docs. Would you be able to add the screenshot of the docs here? I need it for home page and also it may be good to see the docs ss here.

Thank you @mehmetfatiherdem for the suggestion 😊
I added it 😁👍

@Shrreya Shrreya removed the request for review from mkkasem February 25, 2022 11:31
@Shrreya
Copy link
Collaborator

Shrreya commented Feb 25, 2022

@Peri7at @mehmetfatiherdem Can you do the first review for this PR today?

@Peri7at You can focus on auth routes and user routes in your review.
@mehmetfatiherdem You can focus on item routes and schemas in your review.

Let's divide and conquer!

@mehmetfatiherdem
Copy link
Collaborator

Hey @AmmarBaki2, I checked the screenshot and realized that the DELETE request for the item has the same explanation with the PUT. Did you forget it while writing :D or is the screenshot not up to date?

src/swagger.json Outdated
}
},
"delete": {
"summary": "Updates the item data with the specified ID",
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be "Deletes the item data with the specified ID"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for pointing this out 😁
It happens a lot when copying and pasting from the other endpoints 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

By the way, instead of adding a comment for each change, you can press the start a review button and once you finish the review, go back to the top and press on finish review, add a general comment, and choose if you are requesting changes or approving and then submit them as one review

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, I normally do that but for changes like grammar mistakes or wrong messages, I just leave a comment immediately :D

@mehmetfatiherdem
Copy link
Collaborator

Hey @AmmarBaki2, I tried to post an item and then update and delete them. When I do update and delete, it gives me only item owner can modify error. Are they working or am I doing something wrong?

@AmmarBaki2
Copy link
Collaborator Author

Hey @AmmarBaki2, I tried to post an item and then update and delete them. When I do update and delete, it gives me only item owner can modify error. Are they working or am I doing something wrong?

This is an error in the item controller not in the documentation, I will fix it tomorrow. It is basically taking the owner's id from the request body but it should take it from req.user instead so

Copy link
Collaborator

@mehmetfatiherdem mehmetfatiherdem left a comment

Choose a reason for hiding this comment

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

Good work @AmmarBaki2 👏

@Peri7at
Copy link
Collaborator

Peri7at commented Feb 27, 2022

Hi @Shrreya, do we have to document Bad Request as well?
image

src/swagger.json Outdated
"info": {
"version": "1.0.0",
"title": "Paying It Forward",
"description": "This is the documentation for Paying It Forward API. To access the paths that have a lock symbol next to it, you can send a signin request under Auth tab and copy the token from the `authorizationToken` response header and paste it in the `Authorize 🔒` section.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Well done, Ammar!! Everything looks good 👏👏

Just a comment regarding the main description, I guess it is enough to say "... To access the paths that have a lock symbol next to it, you need to sign in first."

image

Copy link
Collaborator

@Peri7at Peri7at Feb 27, 2022

Choose a reason for hiding this comment

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

It is not necessary to copy and paste authorization token. After signing in you can access locked endpoints automatically.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah you are right, it seems that it is using the browser's cookies
I will edit the description to what you suggested, and I will remove the swagger token from everywhere

@AmmarBaki2
Copy link
Collaborator Author

Hello @Shrreya I added all remaining endpoints and tested them
All are working fine
If you can please review it when you have free time, I will really appreciate it 😁

@Shrreya
Copy link
Collaborator

Shrreya commented Mar 2, 2022

Hi @Shrreya, do we have to document Bad Request as well? image

@Peri7at Sorry I missed seeing this comment earlier. It's okay, we don't need to document bad request as that is a standard error.

@AmmarBaki2 AmmarBaki2 merged commit da2932f into main Mar 2, 2022
@AmmarBaki2 AmmarBaki2 deleted the documentation branch March 2, 2022 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add swagger documentation
4 participants