Skip to content

Add the ability to fetch user installations #577

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

Merged
merged 4 commits into from
May 4, 2017
Merged

Add the ability to fetch user installations #577

merged 4 commits into from
May 4, 2017

Conversation

bdelbasso
Copy link
Contributor

It seems that this method is missing.

@Nyholm
Copy link
Collaborator

Nyholm commented May 4, 2017

Thank you for this pr. I will close this however because this method already exists in Integrations::findInstallations

You may want to improve the documentation of this method so it easier can be found.

@Nyholm Nyholm closed this May 4, 2017
@bdelbasso
Copy link
Contributor Author

Hi @Nyholm.

I don't understand why you are saying that this already exists.

Integrations:findInstallations targets the following endpoint:

# Authorization header: JWT token of the integration
GET /integration/installations

While this PR targets:

# Authorization header: OAuth token of the user
GET /user/installations

I've tried a request on /integrations/installations with the OAuth token of a user, it returns a 401 Unauthorized "Invalid credentials".

Here is the corresponding Github doc.

@Nyholm
Copy link
Collaborator

Nyholm commented May 4, 2017

You are correct. Im sorry. I must have reviewed this too quickly.

@Nyholm Nyholm reopened this May 4, 2017
Copy link
Collaborator

@acrobat acrobat left a comment

Choose a reason for hiding this comment

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

👍 Recently introduced new endpoint!

@acrobat
Copy link
Collaborator

acrobat commented May 4, 2017

@bdelbasso Can you also add support for the repositories endpoint?

/user/installations/:installation_id/repositories. See https://developer.github.com/early-access/integrations/user-identification-authorization/#check-which-installations-resources-a-user-can-access

@Nyholm
Copy link
Collaborator

Nyholm commented May 4, 2017

We need to add a $params parameter to support paging. See CurrentUser::issues. We do also need to add some tests.

@acrobat: That is a great idea. But maybe that should be in a separate PR?

@acrobat
Copy link
Collaborator

acrobat commented May 4, 2017

I'm ok with a separate pr!

@bdelbasso
Copy link
Contributor Author

@Nyholm @acrobat PR updated! Let's do the other endpoint in a separate PR when this one is complete.

Copy link
Collaborator

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Thank you

@Nyholm Nyholm merged commit 155bc01 into KnpLabs:master May 4, 2017
@bdelbasso bdelbasso deleted the feat.userInstallations branch May 4, 2017 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants