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

octokit.pulls.listRequestedReviewers should not support pagination #33

Open
2 tasks done
nikclayton-dfinity opened this issue Dec 26, 2020 · 6 comments
Open
2 tasks done
Labels
Status: Blocked Some technical or requirement is blocking the issue Type: Bug Something isn't working as documented
Projects

Comments

@nikclayton-dfinity
Copy link

Checklist

Environment

Versions

├── @octokit/webhooks@7.21.0

and:

├── @actions/github@4.0.0
├── @actions/core@1.2.6

What happened?

The GitHub documentation notes that listRequestedReviewers can be paginated.

I don't think it can -- the response object contains users and teams properties that are arrays that contain all the requested user and team reviewers within a single response, there's no pagination that needs to happen.

So if you write this:

const reviewers = octokit.paginate(octokit.pulls.listRequestedReviewers
  {...context.repo, pull_number})

then reviewers is actually a single element array, but the Tyepscript type inference system thinks it's an object.

So then you go and write:

for (const user of reviewers.users) {
    // ...
}

and you it compiles OK, but you get an error at runtime because the users property is not defined on the response, it's actually reviewers[0].users, but if you write reviewers[0].users in the code you get type errors.

What you actually need to write is:

const {data: reviewers} = await octokit.pulls.listRequestedReviewers(
  {...context.repo, pull_number})

i.e., remove the call to paginate and destructure the response.

I don't think there's much Octokit can do here in the code, but the documentation (i.e., https://octokit.github.io/rest.js/v18#pulls-list-requested-reviewers) could be updated to reference this and note that octokit.paginate should not be used with this API call.

@gr2m
Copy link
Contributor

gr2m commented Dec 29, 2020

Thank you for reporting this problem! I created an issue with the docs team to find out if the endpoint does indeed not support any pagination. See github/docs#2433

@gr2m
Copy link
Contributor

gr2m commented Apr 18, 2021

It seems like it was filed internally, but the problem still seem to persist. I asked about the current status at github/docs#2433

@gr2m gr2m added Status: Blocked Some technical or requirement is blocking the issue Type: Bug Something isn't working as documented labels Apr 18, 2021
@ghost ghost added this to Blocked (by GitHub APIs) in JS Apr 18, 2021
@nitzanashi

This comment has been minimized.

@gr2m

This comment has been minimized.

@nitzanashi

This comment has been minimized.

@gr2m
Copy link
Contributor

gr2m commented Sep 27, 2021

@nikclayton-dfinity just a quick update: an internal issue has been back in January but remains open as of today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Blocked Some technical or requirement is blocking the issue Type: Bug Something isn't working as documented
Projects
No open projects
JS
  
Blocked
Development

No branches or pull requests

3 participants