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
pagination capability for PRs API #120
Conversation
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.
Checks are failing
Please do npm run lint-fix
to fix the errors and commit again
fixed the build errors |
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.
Please pass a default value of 1 in function call
Otherwise if the user simply calls pullrequests/open
without page
as query, a blank response will be returned.
fixed the page number to be 1 by default |
const { data } = await githubService.fetchOpenPRs() | ||
let pageNumber = 1 | ||
|
||
if (req.query.page > 1) { | ||
pageNumber = req.query.page | ||
} | ||
const { data } = await githubService.fetchOpenPRs(pageNumber) |
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.
There is a better and short way to pass default arguments
Please have a look here:
website-backend/models/users.js
Line 60 in d17ea62
.offset((parseInt(query.size) || 100) * (parseInt(query.page) || 0)) |
Also, please make sure to convert the argument to integer using parseInt
as done in above function.
did the changes do check |
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.
Just one final, simple change and we'll be able to merge it!
@@ -107,7 +107,8 @@ const getStalePRs = async (req, res) => { | |||
*/ | |||
const getOpenPRs = async (req, res) => { | |||
try { | |||
const { data } = await githubService.fetchOpenPRs() | |||
const pageNumber = ((req.query.page) || 1) | |||
const { data } = await githubService.fetchOpenPRs(pageNumber) |
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.
Can we simply pass it to the function than saving it in variable and then passing it?
fetchOpenPRs(req.query.page || 1)
would be short and sweet
did the changes do check |
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.
Great job!
Thank you! 🎉
I did the changes for getting the selected page value from user