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

Allow versioning by not forcing absolute path for graph requests #180

Merged
merged 5 commits into from
Feb 13, 2015

Conversation

frausto
Copy link
Contributor

@frausto frausto commented Feb 11, 2015

Faraday assumes leading slashes are absolute paths, this prevents you from being able to set your version based on the :site. for example when site is "https://graph.facebook.com/v2.2"

lostisland/faraday#293

@simi
Copy link
Owner

simi commented Feb 11, 2015

@frausto
Copy link
Contributor Author

frausto commented Feb 11, 2015

@simi Good catch. That was also having the same issue, I took out the leading slash but am still seeing it strip out the v2.2 when it makes the token request. I'll look into it further later on

@matteogiacobazzi
Copy link

Also in the image_url method https://github.com/mkdynamic/omniauth-facebook/blob/master/lib/omniauth/strategies/facebook.rb#L188 you can't make versioned request. What about passing the version number in client_options?

@simi
Copy link
Owner

simi commented Feb 12, 2015

@matteogiacobazzi see https://github.com/mkdynamic/omniauth-facebook#api-version. I think that should be enough, we just need to ensure it is used everywhere.

@frausto
Copy link
Contributor Author

frausto commented Feb 13, 2015

Alright I think we covered all the scenarios now where versioning was being affected

@simi
Copy link
Owner

simi commented Feb 13, 2015

Awesome @frausto! Thanks for your contribution.

simi added a commit that referenced this pull request Feb 13, 2015
Allow versioning by not forcing absolute path for graph requests
@simi simi merged commit c791e75 into simi:master Feb 13, 2015
@simi
Copy link
Owner

simi commented Feb 13, 2015

Changelog note added in 9c93c08.

@simi
Copy link
Owner

simi commented Feb 21, 2015

Released in 2.0.1!

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.

None yet

3 participants