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

Adding params to VerifyCredentials to allow gettign emails from users if possible #376

Merged
merged 4 commits into from Aug 5, 2016

Conversation

@jquacinella
Copy link

jquacinella commented Jul 31, 2016

This is in reference to: #235


This change is Reviewable

James Quacinella added 3 commits Aug 12, 2015
…call. This includes the ability to get a users email if you have correct permissions. This also needed changes to the User class, to add a new email property.
Conflicts:
	twitter/api.py
	twitter/user.py
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jul 31, 2016

Current coverage is 69.11% (diff: 66.66%)

Merging #376 into master will decrease coverage by 0.04%

@@             master       #376   diff @@
==========================================
  Files             8          8          
  Lines          2020       2027     +7   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1397       1401     +4   
- Misses          623        626     +3   
  Partials          0          0          

Powered by Codecov. Last update 2f222c7...714cfe0

# TODO: not sure why but the twitter API needs string true, not a 1
data['include_email'] = 'true'
resp = self._RequestUrl(url, 'GET', data) # No_cache
data = self._ParseAndCheckTwitter(resp.content)

This comment has been minimized.

Copy link
@jeremylow

jeremylow Jul 31, 2016

Collaborator

need to decode the resp for python3. should be

data = self._ParseAndCheckTwitter(resp.content.decode('utf-8'))

James Quacinella
@jeremylow

This comment has been minimized.

Copy link
Collaborator

jeremylow commented Aug 2, 2016

Looks good to me. Do you know what happens when the app isn't white listed and email is requested?

@jquacinella

This comment has been minimized.

Copy link
Author

jquacinella commented Aug 2, 2016

Good question. I put in the original comment "When set to true email will be returned in the user objects as a string. If the user does not have an email address on their account, or if the email address is un-verified, null will be returned". However, if the app is not white-listed, I am not sure. My assumption would be that it'll be null as well, but I would have to set this up as a use case and see. I would only be able to do that later this week / weekend.

@jeremylow

This comment has been minimized.

Copy link
Collaborator

jeremylow commented Aug 5, 2016

I don't have a whitelisted app, so I don't know what the actual data that comes back looks like when include_email is set to True, but when your app isn't whitelisted, there's no error thrown; the email key doesn't exist in the json response.

I think this is good to merge.

@jeremylow jeremylow merged commit 5c41181 into bear:master Aug 5, 2016
1 of 4 checks passed
1 of 4 checks passed
codecov/patch 66.66% of diff hit (target 69.15%)
Details
codecov/project 69.11% (-0.05%) compared to 2f222c7
Details
code-review/reviewable 2 files, 1 discussion left (jeremylow)
Details
ci/circleci Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.