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

Support the new v2 API permissions format. #359

Merged
merged 1 commit into from Jun 10, 2014

Conversation

Projects
None yet
2 participants
@recurser
Contributor

recurser commented Jun 10, 2014

Facebook v2 API permissions are returned in a different format to v1. See v1 vs. v2 documentation.

Instead of:

{ "publish_actions": 1, ... }

... we now get:

[{ "permission": "publish_actions", "status": "granted" }, ...]

I was hesitant to dive into adding v2 support in fb_graph-mock, so the tests are a bit of a hack.

@@ -12,16 +12,16 @@ module FbGraph
ROOT_URL = 'https://graph.facebook.com'
def self.v1!
@v2 = false
::Thread.current[:fb_graph_v2] = false

This comment has been minimized.

@nov

nov Jun 10, 2014

Owner

Are you switching API version in your code repeatedly??

This comment has been minimized.

@recurser

recurser Jun 10, 2014

Contributor

Yes we are - we run a whitelabel service that has a lot of existing clients with v1 apps, and new ones starting up with v2 apps, running on the same servers. We're working through the approval process to upgrade the v1 apps but there's a lot of them, and we need to retain their ability to use :publish_actions while we're migrating.

This comment has been minimized.

@nov

nov Jun 10, 2014

Owner

ah, then putting option params for each request call is better than global setting?

This comment has been minimized.

@recurser

recurser Jun 10, 2014

Contributor

@nov yes that would be even better :) I hadn't seen fb_graph2 - will take a look!

This comment has been minimized.

@nov

nov Jun 10, 2014

Owner

thinking whether this change can break background job cases..

This comment has been minimized.

@nov

nov Jun 10, 2014

Owner

seems ok under rails default settings

This comment has been minimized.

@recurser

recurser Jun 10, 2014

Contributor

@nov deleted the Thread stuff for now - we can run a patched version if you want to have a think about it.

@nov

This comment has been minimized.

Owner

nov commented Jun 10, 2014

FYI: fb_graph2 is the newer version of fb_graph
https://github.com/nov/fb_graph2

nov added a commit that referenced this pull request Jun 10, 2014

Merge pull request #359 from recurser/v2-permissions
Support the new v2 API permissions format.

@nov nov merged commit eefd8be into nov:master Jun 10, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@nov

This comment has been minimized.

Owner

nov commented Jun 10, 2014

Thanks for your contribution.
Permission v2 support now in fb_graph v2.7.14

@recurser

This comment has been minimized.

Contributor

recurser commented Jun 10, 2014

@nov thanks for your work on fb_graph - much appreciated!

@nov

This comment has been minimized.

Owner

nov commented Jun 10, 2014

support specifying api_version per api call.
https://github.com/nov/fb_graph/blob/master/spec/fb_graph/connections/permissions_spec.rb#L65

ps.
even in that case, FbGraph.v2? returns global setting, so I modified FbGraph::Connection::Permissions like this.
https://github.com/nov/fb_graph/blob/master/lib/fb_graph/connections/permissions.rb#L6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment