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

fix: handle paginated response #1589

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

boonware
Copy link

Issue

spinnaker/spinnaker#6768

Description

  • Current code does not handle a paginated response from GitHub API. If the user is a member of more the 30 GitHub orgs (the default page size), and the target org is not contained in the first page, then the org membership check will fail.
  • This change handles paginated response by parsing out the link to the next page in the Link header, for example: '<https://github.com/api/v3/users/1234/orgs?page=2>; rel="next", <https://github.com/api/v3/users/1234/orgs?page=3>; rel="last"'.
  • Unit test suite is included. In order to make the code testable, the private modifier was removed from the instance variable restTemplate to allow dependency mocking. A better design would inject this dependency, but this is outside the scope of the current issue.

@boonware
Copy link
Author

@dbyron-sf Could I please get a build approved? Thank you.

@mattgogerly
Copy link
Member

Hi @boonware, could you update your commit message to fix(github) and I can approve the workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants