hexgnu / linkedin

Ruby wrapper for the LinkedIn API
http://rdoc.info/gems/linkedin
MIT License
756 stars 407 forks source link

Update LinkedIn::Api::ShareAndSocialStream#add_share for Api v2 #274

Open dsandstrom opened 5 years ago

dsandstrom commented 5 years ago

Along with sending over the comment, you must send the user's "urn"

Example (updated):

client = LinkedIn::Client.new
client.authorize_from_access(token)
client.v2_add_share(urn, { comment: 'hi' })

https://docs.microsoft.com/en-us/linkedin/consumer/integrations/self-serve/share-on-linkedin?context=linkedin/consumer/context

hexgnu commented 5 years ago

@dsandstrom if you want to write a test for this I can review this and merge it and also cut a new gem.

Thanks!

dsandstrom commented 5 years ago

Well the problem is this change breaks all other endpoints as they have not been updated for LinkedIn's API v2. I just wanted to show other people what I got working.

hexgnu commented 5 years ago

We could do something like Linkedin::V2.add_share too. That way it wouldn't break for others

dsandstrom commented 5 years ago

Not sure how you want to keep v2 methods separate, but this allows v1 endpoints to still work. You said to do "something like Linkedin::V2.add_share", but that doesn't really make sense since we run the methods on the LinkedIn::Client. A possibility is to add a LinkedIn::V2::Client, then include the v2 methods into that class.

hexgnu commented 5 years ago

nice work! I just put a few comments, mostly nitpicks and question type things.

I'm wondering if we should do something besides the prefixing of the methods for v2_* requests.

I think it's ok, it just feels a bit weird is all since v1 is mostly deprecated at this point.

dsandstrom commented 5 years ago

Prefixing with v2 was just temporary. A V2 module needs to be set up. See https://github.com/hexgnu/linkedin/pull/274#issuecomment-472236744

hexgnu commented 5 years ago

Yea Linkedin::V2::Client makes sense. if you want to do that.

addbrick commented 5 years ago

@hexgnu @dsandstrom Just a heads up I added the email endpoint in a fork and opened a PR on @dsandstrom fork. https://github.com/dsandstrom/linkedin/pull/1. My company needs it for our auth flow.

addbrick commented 5 years ago

@hexgnu @dsandstrom I ended up adding a little more to my pr it now includes the additions:

https://github.com/dsandstrom/linkedin/pull/1

Meant to say this the first time, but I did not address any of the outstanding pr comments.

hexgnu commented 5 years ago

If you all want to combine forces feel free to do so :) will gladly review and merge in.

malissa commented 4 years ago

@dsandstrom @addbrick - great work! Thank you for doing this! If you all can get this in I can't wait to use it :)

hexgnu commented 4 years ago

Again happy to help merge this stuff in I just don't work with the linkedin API anymore so I'd be flying blind here :smile:.