gocialite icon indicating copy to clipboard operation
gocialite copied to clipboard

Fix linkedin

Open gadelkareem opened this issue 5 years ago • 9 comments

Upgrade Linkedin API and retrieve email address correctly since V1 disallows email address retrieval.

gadelkareem avatar May 03 '20 19:05 gadelkareem

@danilopolani any updates?

gadelkareem avatar May 19 '20 10:05 gadelkareem

Ehy, sorry I'm a loot busy in this time due to my work. Anyway I was looking at the code, I remember that to retrieve the profile picture URL was a bit tricky in V2, did you test it?

Ref: https://docs.microsoft.com/en-us/linkedin/shared/references/v2/profile/profile-picture?context=linkedin/consumer/context

danilopolani avatar May 19 '20 10:05 danilopolani

Ehy, sorry I'm a loot busy in this time due to my work. Anyway I was looking at the code, I remember that to retrieve the profile picture URL was a bit tricky in V2, did you test it?

Ref: https://docs.microsoft.com/en-us/linkedin/shared/references/v2/profile/profile-picture?context=linkedin/consumer/context

Yeah I tried to implemented but it looked complicated and will require extra API calls so I dropped it.

gadelkareem avatar May 19 '20 14:05 gadelkareem

Ok, but then we can't merge this because we would not guarantee the same output as before. If someone uses the avatar field to store the picture URL and updates the app with this PR, his app would break for sure

danilopolani avatar May 19 '20 15:05 danilopolani

@danilopolani it is already there https://github.com/danilopolani/gocialite/pull/16/files#diff-7af7b035f06b4444f6c2e0b4bba1a6a3R31 just empty field

gadelkareem avatar May 20 '20 09:05 gadelkareem

Yeah, but it can't be an empty field if before it was not, otherwise it's a breaking change

danilopolani avatar May 20 '20 09:05 danilopolani

Yeah, but it can't be an empty field if before it was not, otherwise it's a breaking change

If the field still exists then it is not a breaking change because it will not panic. It could also be a user without an avatar so it could already be empty.

gadelkareem avatar May 20 '20 09:05 gadelkareem

Ok, but imagine this flow:

My app implements a "Sign in with LinkedIn" feature; when a user clicks on the button and I receive the callback, I always update his avatar to have it up-to-date. I update Gocialite with this PR. Users keep logging in with LinkedIn, but their profile pictures start disappearing. What's happening?

Maybe it would not be a "breaking" for the panic itself, but for a regular flow and the data consistency. I hope you got the point from the example above

danilopolani avatar May 20 '20 09:05 danilopolani

Ok, but imagine this flow:

My app implements a "Sign in with LinkedIn" feature; when a user clicks on the button and I receive the callback, I always update his avatar to have it up-to-date. I update Gocialite with this PR. Users keep logging in with LinkedIn, but their profile pictures start disappearing. What's happening?

Maybe it would not be a "breaking" for the panic itself, but for a regular flow and the data consistency. I hope you got the point from the example above

Well currently the user does not get an email because of the new restriction linkedin added to the retired API so it is already broken in this sense.

gadelkareem avatar May 20 '20 09:05 gadelkareem