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

Add function for retrieving user permissions #264

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
aa30f81
retrieve permissions object from API
seawolf42 Jan 12, 2016
2e7d5cd
transform result into dictionary for convenient lookups
seawolf42 Jan 12, 2016
fb0ca41
retrieve the token from the environment once and make all tests depen…
seawolf42 Jan 12, 2016
f3aa306
move test to bottom of file
seawolf42 Jan 12, 2016
e0485ff
flake8 compliant
seawolf42 Jan 13, 2016
a1b8d7c
compliance with python 2.6
seawolf42 Jan 13, 2016
efb65e2
use unittest2 if older python to support skipIf decorator
seawolf42 Jan 13, 2016
fc4ed3d
Merge branch 'master' of github.com:mobolic/facebook-sdk
seawolf42 Aug 14, 2016
d2d7486
use user ID for any arbitrary user rather than just the current user
seawolf42 Aug 15, 2016
17291f9
if user does not exist or has not authorized the app, raise GraphAPIE…
seawolf42 Aug 15, 2016
65f3d00
lint clean
seawolf42 Aug 15, 2016
2d94d2f
documentation for get_permissions method
seawolf42 Aug 15, 2016
2917c49
better grammar
seawolf42 Aug 15, 2016
e78968b
remove file accidentally checked in
seawolf42 Aug 18, 2016
52c73e5
Merge remote-tracking branch 'upstream/master'
seawolf42 Nov 24, 2016
2223169
remove support for python 2.6
seawolf42 Nov 24, 2016
b1d8876
flake8 compliance
seawolf42 Nov 26, 2016
e1a05e8
bug fix
seawolf42 Nov 26, 2016
ca15bf8
remove **args parameter
seawolf42 Jan 6, 2017
8ffedb9
change to string format instead of concatenation
seawolf42 Jan 6, 2017
a69a778
Merge remote-tracking branch 'upstream/master'
seawolf42 Jan 6, 2017
3c8911f
create test users rather than using hard-coded user ID
seawolf42 Jan 7, 2017
c422097
add args to request
seawolf42 Jan 7, 2017
b9e5e61
working unit tests
seawolf42 Jan 7, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ env:
global:
- FACEBOOK_APP_ID=198798870326423
- FACEBOOK_SECRET=2db4d76fe8a336cf292470c20a5a5684
- FACEBOOK_USER_ID=12345
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juhopaak added methods to create test users programmatically (fdc36cc). I think using them is a better solution than using a hardcoded user ID.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, though this feature wasn't there when we first talked about this :) ... happy to figure out how to make that change.

19 changes: 19 additions & 0 deletions docs/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -271,3 +271,22 @@ Deletes the object with the given ID from the graph.
.. code-block:: python

graph.delete_object(id='post_id')

get_permissions
^^^^^^^^^^^^^

https://developers.facebook.com/docs/graph-api/reference/user/permissions/

Returns the permissions granted to the app by the user with the given ID as a
``dict``.

**Parameters**

* ``user_id`` - A ``string`` or an ``int`` that is a unique ID for a user.

**Example**

.. code-block:: python

permissions = graph.get_permissions(user_id=12345)
print(permissions["public_profile"])
10 changes: 9 additions & 1 deletion facebook/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,14 @@ def __init__(self, access_token=None, timeout=None, version=None,
else:
self.version = "v" + default_version

def get_permissions(self, user_id, **args):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

args doesn't seem to be used in this method. Is there a reason that it has been included?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had used it at one point, I think, and forgot to remove it. thanks!

"""Fetches the permissions object from the graph."""
response = self.request(
self.version + "/" + str(user_id) + "/permissions")["data"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use .format here instead of manually concatenating strings?

result = dict((x["permission"], x["status"] == "granted")
for x in response)
return result

def get_object(self, id, **args):
"""Fetches the given object from the graph."""
return self.request("{0}/{1}".format(self.version, id), args)
Expand Down Expand Up @@ -235,7 +243,7 @@ def request(
# or it does not need `access_token`.
if post_args and "access_token" not in post_args:
post_args["access_token"] = self.access_token
elif "access_token" not in args:
elif args and "access_token" not in args:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this related to user permissions, or is it a fix for a different bug?

Copy link
Contributor Author

@seawolf42 seawolf42 Jan 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably a different bug... I had a test that was failing before, though when I revert this now that same test isn't failing, so I'm not sure what changed. Your call, do you want this reverted or included? If reverted, should I submit a different PR for it?

Copy link
Contributor Author

@seawolf42 seawolf42 Jan 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take that back, that is necessary for one of my unit tests to pass. Otherwise I get:

Traceback (most recent call last):
  File "/Users/seawolf/repos/facebook-sdk-jke/test/test_facebook.py", line 223, in test_get_user_permissions_nonexistant_user
    facebook.GraphAPI(token).get_permissions(1)
  File "facebook/__init__.py", line 108, in get_permissions
    "{0}/{1}/permissions".format(self.version, user_id)
  File "facebook/__init__.py", line 247, in request
    elif "access_token" not in args:
TypeError: argument of type 'NoneType' is not iterable

The method defaults args to None, but if it is unset then line 246 throws the exception.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the Contributing Guidelines, a pull request should only contain commits that aren't relevant to its core feature. If it doesn't have to do with user permissions, it should be in a different pull request.

The purpose of that code is to ensure that the arguments passed to Facebook include an access token. With your code, it is necessary to set args, otherwise this will not happen.

This should have been fixed in #341, so if you rebase your branch off of the current master, the error will go away.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally makes sense, removed the change to the request method and added an empty args dict to the call.

args["access_token"] = self.access_token

try:
Expand Down
30 changes: 28 additions & 2 deletions test/test_facebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@ def setUp(self):
try:
self.app_id = os.environ["FACEBOOK_APP_ID"]
self.secret = os.environ["FACEBOOK_SECRET"]
self.user_id = os.environ["FACEBOOK_USER_ID"]
except KeyError:
raise Exception("FACEBOOK_APP_ID and FACEBOOK_SECRET "
"must be set as environmental variables.")
raise Exception("FACEBOOK_APP_ID, FACEBOOK_SECRET, and "
"FACEBOOK_USER_ID must be set as "
"environmental variables.")

def assert_raises_multi_regex(
self, expected_exception, expected_regexp, callable_obj=None,
Expand Down Expand Up @@ -197,5 +199,29 @@ def test_parse_signed_request_when_correct(self):
self.assertTrue('algorithm' in result)


class TestGetUserPermissions(FacebookTestCase):
"""
Test if user permissions are retrieved correctly.

Note that this only tests if the returned JSON object exists and is
structured as expected, not whether any specific scope is included
(other than the default `public_profile` scope).

"""
@unittest.skip('test cannot be run without a valid user ID')
def test_get_user_permissions_node(self):
token = facebook.GraphAPI().get_app_access_token(
self.app_id, self.secret)
permissions = facebook.GraphAPI(token).get_permissions(self.user_id)
assert permissions is not None
assert permissions["public_profile"] is True

def test_get_user_permissions_nonexistant_user(self):
token = facebook.GraphAPI().get_app_access_token(
self.app_id, self.secret)
with self.assertRaises(facebook.GraphAPIError):
facebook.GraphAPI(token).get_permissions(1)


if __name__ == '__main__':
unittest.main()