Skip to content
This repository has been archived by the owner on Sep 12, 2021. It is now read-only.

Fixed Profile URL as v1 resource is no longer available #568

Merged
merged 2 commits into from
Oct 10, 2019

Conversation

ViniciusMiana
Copy link
Contributor

Fixes

This fix updates the API url to the v2 resource and the parsing of the json response that comes from it.

Purpose

This PR allows you to user the LinkedIn provider and get the First and Last name of the logged user. Unfortunatelly the email and picture don't come by default anymore and for that reason were left out.

Background Context

It has been a while since LinkedIn has discontinued its v1 people resource. As a result, the LinkedInProvider was failing to parse the profile of the logged user.

@akkie
Copy link
Contributor

akkie commented Oct 9, 2019

@ViniciusMiana Thanks for the update 👍 Would it be possible to fetch the email and picture in an additional API request and merge it with the default data?

This documentation describes the process of getting all data: https://docs.microsoft.com/en-us/linkedin/consumer/integrations/self-serve/sign-in-with-linkedin?context=linkedin/consumer/context

It would also be nice if you can add the links to the new documentation sources and if you could adapt the tests to the new API.

@ViniciusMiana
Copy link
Contributor Author

Thanks @akkie! I was aware of the necessary calls, however, since it required greater changes, I was not sure it would be welcome. I will do that as soon as I can.

@ViniciusMiana
Copy link
Contributor Author

Hi @akkie, I made the fixes with a few hacks as the more elegant solution would require to change the interface for a SocialProfileParser and others. Let me know what you think and if any other changes are necessary.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 96.126% when pulling 0c3f1e0 on ViniciusMiana:master into dbc6b55 on mohiva:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 96.126% when pulling 0c3f1e0 on ViniciusMiana:master into dbc6b55 on mohiva:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 96.126% when pulling 0c3f1e0 on ViniciusMiana:master into dbc6b55 on mohiva:master.

@coveralls
Copy link

coveralls commented Oct 10, 2019

Coverage Status

Coverage increased (+0.07%) to 96.152% when pulling 0c3f1e0 on ViniciusMiana:master into dbc6b55 on mohiva:master.

@akkie
Copy link
Contributor

akkie commented Oct 10, 2019

Hey @ViniciusMiana, I'll see if I can change the interfaces to cover this scenario, in https://github.com/minutemen/silhouette when I update the LinkedIn provider there.

Thanks for your work 👍

@akkie akkie merged commit 49fc9c7 into mohiva:master Oct 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants