-
Notifications
You must be signed in to change notification settings - Fork 951
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
Changes from 18 commits
aa30f81
2e7d5cd
fb0ca41
f3aa306
e0485ff
a1b8d7c
efb65e2
fc4ed3d
d2d7486
17291f9
65f3d00
2d94d2f
2917c49
e78968b
52c73e5
2223169
b1d8876
e1a05e8
ca15bf8
8ffedb9
a69a778
3c8911f
c422097
b9e5e61
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use |
||
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) | ||
|
@@ -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: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
The method defaults There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 This should have been fixed in #341, so if you rebase your branch off of the current master, the error will go away. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.