diff --git a/gittip/elsewhere/__init__.py b/gittip/elsewhere/__init__.py index ad38cdcd40..96e9cf1aa8 100644 --- a/gittip/elsewhere/__init__.py +++ b/gittip/elsewhere/__init__.py @@ -1,97 +1,145 @@ -from aspen import log +import gittip +from aspen import Response from aspen.utils import typecheck -from gittip import db +from gittip.authentication import User from gittip.participant import reserve_a_random_participant_id from psycopg2 import IntegrityError +def _resolve(platform, username_key, username): + """Given three unicodes, return a participant_id. + """ + typecheck(platform, unicode, username_key, unicode, username, unicode) + rec = gittip.db.fetchone(""" + + SELECT participant_id + FROM elsewhere + WHERE platform=%s + AND user_info->%s = %s + + """, (platform, username_key, username,)) + # XXX Do we want a uniqueness constraint on $username_key? Can we do that? + + if rec is None: + raise Exception( "User %s on %s isn't known to us." + % (username, platform) + ) + return rec['participant_id'] + + class AccountElsewhere(object): - pass + platform = None # set in subclass -def upsert(platform, user_id, username, user_info): - """Given str, unicode, unicode, and dict, return unicode and boolean. + def __init__(self, user_id): + typecheck(user_id, (int, unicode)) + self.user_id = unicode(user_id) - Platform is the name of a platform that we support (ASCII blah). User_id is - an immutable unique identifier for the given user on the given platform. - Username is the user's login/username on the given platform. It is only - used here for logging. Specifically, we don't reserve their username for - them on Gittip if they're new here. We give them a random participant_id - here, and they'll have a chance to change it if/when they opt in. User_id - and username may or may not be the same. User_info is a dictionary of - profile info per the named platform. All platform dicts must have an id key - that corresponds to the primary key in the underlying table in our own db. - The return value is a tuple: (participant_id [unicode], is_claimed - [boolean], is_locked [boolean], balance [Decimal]). + def set_is_locked(self, is_locked): + gittip.db.execute(""" - """ - typecheck( platform, str - , user_id, (int, unicode) - , username, unicode - , user_info, dict - ) - user_id = unicode(user_id) + UPDATE elsewhere + SET is_locked=%s + WHERE platform=%s AND user_id=%s + """, (is_locked, self.platform, self.user_id)) - # Insert the account if needed. - # ============================= - # Do this with a transaction so that if the insert fails, the - # participant we reserved for them is rolled back as well. - try: - with db.get_transaction() as txn: - participant_id = reserve_a_random_participant_id(txn) - INSERT = """\ + def opt_in(self, participant_id, user_info): + """Given a desired participant_id, return a User object. + """ + self.set_is_locked(False) + participant_id, is_claimed, is_locked, balance = self.upsert(user_info) + user = User.from_id(participant_id) # give them a session + if not is_claimed: + user.set_as_claimed() + try: + user.change_id(participant_id) + except (Response, IntegrityError): + pass + return user - INSERT INTO elsewhere - (platform, user_id, participant_id) - VALUES (%s, %s, %s) - """ - txn.execute(INSERT, (platform, user_id, participant_id)) - except IntegrityError: - pass + def upsert(self, user_info): + """Given a dict, return a tuple. + Platform is the name of a platform that we support (ASCII blah). + User_id is an immutable unique identifier for the given user on the + given platform. Username is the user's login/username on the given + platform. It is only used here for logging. Specifically, we don't + reserve their username for them on Gittip if they're new here. We give + them a random participant_id here, and they'll have a chance to change + it if/when they opt in. User_id and username may or may not be the + same. User_info is a dictionary of profile info per the named platform. + All platform dicts must have an id key that corresponds to the primary + key in the underlying table in our own db. - # Update their user_info. - # ======================= - # Cast everything to unicode, because (I believe) hstore can take any type - # of value, but psycopg2 can't. - # - # https://postgres.heroku.com/blog/past/2012/3/14/introducing_keyvalue_data_storage_in_heroku_postgres/ - # http://initd.org/psycopg/docs/extras.html#hstore-data-type + The return value is a tuple: (participant_id [unicode], is_claimed + [boolean], is_locked [boolean], balance [Decimal]). - for k, v in user_info.items(): - user_info[k] = unicode(v) + """ + typecheck(user_info, dict) - UPDATE = """\ - UPDATE elsewhere - SET user_info=%s - WHERE platform=%s AND user_id=%s - RETURNING participant_id + # Insert the account if needed. + # ============================= + # Do this with a transaction so that if the insert fails, the + # participant we reserved for them is rolled back as well. - """ + try: + with gittip.db.get_transaction() as txn: + _participant_id = reserve_a_random_participant_id(txn) + txn.execute( "INSERT INTO elsewhere " + "(platform, user_id, participant_id) " + "VALUES (%s, %s, %s)" + , (self.platform, self.user_id, _participant_id) + ) + except IntegrityError: + pass + + + # Update their user_info. + # ======================= + # Cast everything to unicode, because (I believe) hstore can take any + # type of value, but psycopg2 can't. + # + # https://postgres.heroku.com/blog/past/2012/3/14/introducing_keyvalue_data_storage_in_heroku_postgres/ + # http://initd.org/psycopg/docs/extras.html#hstore-data-type + + for k, v in user_info.items(): + user_info[k] = unicode(v) + + + participant_id = gittip.db.fetchone(""" + + UPDATE elsewhere + SET user_info=%s + WHERE platform=%s AND user_id=%s + RETURNING participant_id + + """, (user_info, self.platform, self.user_id))['participant_id'] + + + # Get a little more info to return. + # ================================= - rec = db.fetchone(UPDATE, (user_info, platform, user_id)) - participant_id = rec['participant_id'] + rec = gittip.db.fetchone(""" + SELECT claimed_time, balance, is_locked + FROM participants + JOIN elsewhere + ON participants.id=participant_id + WHERE platform=%s + AND participants.id=%s - # Get a little more info to return. - # ================================= + """, (self.platform, participant_id)) - rec = db.fetchone( "SELECT claimed_time, balance, is_locked " - "FROM participants " - "JOIN elsewhere ON participants.id = participant_id " - "WHERE participants.id=%s" - , (participant_id,) - ) - assert rec is not None # sanity check + assert rec is not None # sanity check - return ( participant_id - , rec['claimed_time'] is not None - , rec['is_locked'] - , rec['balance'] - ) + return ( participant_id + , rec['claimed_time'] is not None + , rec['is_locked'] + , rec['balance'] + ) diff --git a/gittip/elsewhere/github.py b/gittip/elsewhere/github.py index 2a578022a9..e51344767d 100644 --- a/gittip/elsewhere/github.py +++ b/gittip/elsewhere/github.py @@ -2,15 +2,15 @@ from aspen import json, log, Response from aspen.website import Website from aspen.utils import typecheck -from gittip import db, elsewhere +from gittip.elsewhere import AccountElsewhere, _resolve -def upsert(user_info): - return elsewhere.upsert( 'github' - , user_info['id'] - , user_info['login'] - , user_info - ) +class GitHubAccount(AccountElsewhere): + platform = 'github' + + +def resolve(login): + return _resolve(u'github', u'login', login) def oauth_url(website, action, then=u""): @@ -76,20 +76,3 @@ def oauth_dance(website, qs): % (user_info['login'], user_info['id'])) return user_info - - -def resolve(login): - """Given two str, return a participant_id. - """ - FETCH = """\ - - SELECT participant_id - FROM elsewhere - WHERE platform = 'github' - AND user_info -> 'login' = %s - - """ # XXX Uniqueness constraint on login? - rec = db.fetchone(FETCH, (login,)) - if rec is None: - raise Exception("GitHub user %s has no participant." % (login)) - return rec['participant_id'] diff --git a/gittip/elsewhere/twitter.py b/gittip/elsewhere/twitter.py index f5ff2ddfd1..ed02ce4dc2 100644 --- a/gittip/elsewhere/twitter.py +++ b/gittip/elsewhere/twitter.py @@ -1,29 +1,12 @@ -from gittip import db, elsewhere +from gittip.elsewhere import AccountElsewhere, _resolve -def upsert(user_info): - return elsewhere.upsert( 'twitter' - , user_info['id'] - , user_info['screen_name'] - , user_info - ) +class TwitterAccount(AccountElsewhere): + platform = 'twitter' -def resolve(user_id): - """Given str, return a participant_id. - """ - FETCH = """\ - - SELECT participant_id - FROM elsewhere - WHERE platform='twitter' - AND user_info -> 'user_id' = %s - - """ # XXX Uniqueness constraint on screen_name? - rec = db.fetchone(FETCH, (user_id,)) - if rec is None: - raise Exception("Twitter user %s has no participant." % (user_id)) - return rec['participant_id'] +def resolve(screen_name): + return _resolve(u'twitter', u'screen_name', screen_name) def oauth_url(website, action, then=""): diff --git a/gittip/participant.py b/gittip/participant.py index c633a73f4a..b2bc14563e 100644 --- a/gittip/participant.py +++ b/gittip/participant.py @@ -708,7 +708,9 @@ def take_over(self, platform, user_id, have_confirmation=False): # Do the deal. # ============ # If other_is_not_a_stub, then other will have the account - # elsewhere taken away from them with this call. + # elsewhere taken away from them with this call. If there are other + # browsing sessions open from that account, they will stay open + # until they expire (XXX Is that okay?) txn.execute( "UPDATE elsewhere SET participant_id=%s " "WHERE platform=%s AND user_id=%s" diff --git a/gittip/testing.py b/gittip/testing.py index c1bfdb2ae7..f72df291c8 100644 --- a/gittip/testing.py +++ b/gittip/testing.py @@ -36,10 +36,11 @@ def create_schema(db): ] def populate_db_with_dummy_data(db): - from gittip.elsewhere import github + from gittip.elsewhere.github import GitHubAccount from gittip.participant import Participant for user_id, login in GITHUB_USERS: - participant_id, a,b,c = github.upsert({"id": user_id, "login": login}) + account = GitHubAccount(user_id) + participant_id, a,b,c = account.upsert({"id": user_id, "login": login}) Participant(participant_id).change_id(login) @@ -378,10 +379,13 @@ def setup_tips(*recs): elsewhere = [] for participant_id, crap in _participants.items(): (good_cc, is_suspicious, claimed, platform, user_id) = crap + username_key = "login" if platform == 'github' else "screen_name" elsewhere.append({ "platform": platform , "user_id": user_id , "participant_id": participant_id - , "user_info": {} + , "user_info": { "id": user_id + , username_key: participant_id + } }) rec = {"id": participant_id} if good_cc is not None: diff --git a/tests/test_elsewhere.py b/tests/test_elsewhere.py new file mode 100644 index 0000000000..146ca5372b --- /dev/null +++ b/tests/test_elsewhere.py @@ -0,0 +1,16 @@ +from gittip.testing import tip_graph +from gittip.elsewhere import github, twitter + + +def test_github_resolve_resolves(): + with tip_graph(('alice', 'bob', 1)): + expected = 'alice' + actual = github.resolve(u'alice') + assert actual == expected, actual + + +def test_twitter_resolve_resolves(): + with tip_graph(('alice', 'bob', 1, True, False, False, "twitter", "2345")): + expected = 'alice' + actual = twitter.resolve(u'alice') + assert actual == expected, actual diff --git a/www/on/github/%login/index.html b/www/on/github/%login/index.html index 80a03704a4..ec3b288d6e 100644 --- a/www/on/github/%login/index.html +++ b/www/on/github/%login/index.html @@ -19,15 +19,15 @@ , (path['login'],) ) if rec is not None: - userinfo = rec['user_info'] + user_info = rec['user_info'] else: url = "https://api.github.com/users/%s" - userinfo = requests.get(url % path['login']) + user_info = requests.get(url % path['login']) - if userinfo.status_code == 200: - userinfo = json.loads(userinfo.text) + if user_info.status_code == 200: + user_info = json.loads(user_info.text) else: - code = userinfo.status_code + code = user_info.status_code raise Response(500, "GitHub lookup failed with %d." % code) @@ -35,14 +35,15 @@ # ========================== # We can only tip Users, not Organizations (or whatever else type can be). -username = userinfo['login'] -name = userinfo.get('name') +username = user_info['login'] +name = user_info.get('name') if not name: name = username -usertype = userinfo.get("type", "unknown type of account").lower() +usertype = user_info.get("type", "unknown type of account").lower() if usertype == "user": - participant_id, is_claimed, is_locked, balance = github.upsert(userinfo) + account = github.GitHubAccount(user_info['id']) + participant_id, is_claimed, is_locked, balance = account.upsert(user_info) can_tip = not is_locked lock_action = "unlock" if is_locked else "lock" if is_claimed: @@ -93,7 +94,7 @@

{{ username }} has opted out of Gittip.

-

If you are {{ username }} +

If you are {{ username }} on GitHub, you can unlock your account to allow people to pledge tips to you on Gittip. We never collect any money on your behalf until you explicitly opt in.

@@ -121,11 +122,11 @@

{{ name }} has not joined Gittip.

- + - {{ userinfo['login'] }} - {% if userinfo.get('name') %}({{ userinfo.get('name') }}){% end %} + src="{{ user_info.get('avatar_url', '/assets/%s/no-avatar.png' % __version__) }}" /> + {{ user_info['login'] }} + {% if user_info.get('name') %}({{ user_info.get('name') }}){% end %}
GitHub
@@ -175,11 +176,11 @@

{{ name }} is an {{ usertype }} on GitHub.

- + - {{ userinfo['login'] }} - {% if userinfo.get('name') %}({{ userinfo.get('name') }}){% end %} + src="{{ user_info.get('avatar_url', '/assets/%s/no-avatar.png' % __version__) }}" /> + {{ user_info['login'] }} + {% if user_info.get('name') %}({{ user_info.get('name') }}){% end %}
GitHub
diff --git a/www/on/github/associate b/www/on/github/associate index d488b8ec1a..d31779ff3d 100644 --- a/www/on/github/associate +++ b/www/on/github/associate @@ -7,10 +7,7 @@ the Gittip community. """ from aspen import log, Response -from gittip import db -from gittip.authentication import User from gittip.elsewhere import github -from psycopg2 import IntegrityError # ========================== ^L @@ -32,16 +29,12 @@ if login is None: # Do something. log(u"%s wants to %s" % (login, action)) + +account = github.GitHubAccount(user_info['id']) + if action == 'opt-in': # opt in - participant_id, is_claimed, is_locked, balance = github.upsert(user_info) - user = User.from_id(participant_id) # give them a session - if not is_claimed: - user.set_as_claimed() - try: - user.change_id(login) - participant_id = login - except (Response, IntegrityError): - pass + user = account.opt_in() # setting 'user' gives them a session + participant_id = user.id else: # lock or unlock if then != login: @@ -56,14 +49,10 @@ else: # lock or unlock # participant. participant_id, is_claimed, is_locked, balance \ - = github.upsert(user_info) + = account.upsert(user_info) assert participant_id != login, login # sanity check - db.execute( "UPDATE elsewhere " - "SET is_locked=%s " - "WHERE participant_id=%s" - , (action == 'lock', participant_id) - ) + account.set_is_locked(action == 'lock') if then == u'': then = u'/%s/' % participant_id diff --git a/www/on/twitter/%screen_name/index.html b/www/on/twitter/%screen_name/index.html index da7d11d909..2f8929b047 100644 --- a/www/on/twitter/%screen_name/index.html +++ b/www/on/twitter/%screen_name/index.html @@ -21,18 +21,18 @@ , (path['screen_name'],) ) if rec is not None: - userinfo = rec['user_info'] + user_info = rec['user_info'] else: url = "https://api.twitter.com/1/users/show.json?screen_name=%s" - userinfo = requests.get(url % path['screen_name']) + user_info = requests.get(url % path['screen_name']) # Keep an eye on our Twitter usage. # ================================= - rate_limit = userinfo.headers['X-RateLimit-Limit'] - rate_limit_remaining = userinfo.headers['X-RateLimit-Remaining'] - rate_limit_reset = userinfo.headers['X-RateLimit-Reset'] + rate_limit = user_info.headers['X-RateLimit-Limit'] + rate_limit_remaining = user_info.headers['X-RateLimit-Remaining'] + rate_limit_reset = user_info.headers['X-RateLimit-Reset'] try: rate_limit = int(rate_limit) @@ -50,23 +50,24 @@ ) - if userinfo.status_code == 200: - userinfo = json.loads(userinfo.text) + if user_info.status_code == 200: + user_info = json.loads(user_info.text) else: - log("Twitter lookup failed with %d." % userinfo.status_code) + log("Twitter lookup failed with %d." % user_info.status_code) raise Response(404) # Try to load from Gittip. # ======================== -username = userinfo['screen_name'] -name = userinfo.get('name') +username = user_info['screen_name'] +name = user_info.get('name') if not name: name = username -userinfo['html_url'] = "https://twitter.com/%s" % username +user_info['html_url'] = "https://twitter.com/%s" % username -participant_id, is_claimed, is_locked, balance = twitter.upsert(userinfo) +account = twitter.TwitterAccount(user_info['id']) +participant_id, is_claimed, is_locked, balance = account.upsert(user_info) participant = Participant(participant_id) can_tip = not is_locked lock_action = "unlock" if is_locked else "lock" @@ -87,7 +88,7 @@

{{ username }} has opted out of Gittip.

-

If you are {{ username }} +

If you are {{ username }} on Twitter, you can unlock your account to allow people to pledge tips to you on Gittip. We never collect any money on your behalf until you explicitly opt in.

@@ -115,11 +116,11 @@

{{ name }} has not joined Gittip.

- + - {{ userinfo['screen_name'] }} - {% if userinfo.get('name') %}({{ userinfo.get('name') }}){% end %} + src="{{ user_info.get('profile_image_url_https', '/assets/%s/no-avatar.png' % __version__) }}" /> + {{ user_info['screen_name'] }} + {% if user_info.get('name') %}({{ user_info.get('name') }}){% end %}
Twitter
diff --git a/www/on/twitter/associate b/www/on/twitter/associate index aabf4e68a3..b7ec4cb025 100644 --- a/www/on/twitter/associate +++ b/www/on/twitter/associate @@ -9,11 +9,8 @@ the Gittip community. import requests from oauth_hook import OAuthHook from aspen import log, Response, json -from gittip import db -from gittip.authentication import User from gittip.elsewhere import twitter from urlparse import parse_qs -from psycopg2 import IntegrityError # ========================== ^L @@ -61,16 +58,12 @@ user_info['html_url'] = "https://twitter.com/" + screen_name # Do something. log(u"%s wants to %s" % (screen_name, action)) + +account = twitter.TwitterAccount(user_info['id']) + if action == 'opt-in': # opt in - participant_id, is_claimed, is_locked, balance = twitter.upsert(user_info) - user = User.from_id(participant_id) # give them a session - if not is_claimed: - user.set_as_claimed() - try: - user.change_id(screen_name) - participant_id = screen_name - except (Response, IntegrityError): - pass + user = account.opt_in() # setting 'user' gives them a session + participant_id = user.id else: # lock or unlock if then != screen_name: @@ -86,14 +79,10 @@ else: # lock or unlock # Gittip participant. participant_id, is_claimed, is_locked, balance \ - = twitter.upsert(user_info) + = account.upsert(user_info) assert participant_id != screen_name, screen_name # sanity check - db.execute( "UPDATE elsewhere " - "SET is_locked=%s " - "WHERE participant_id=%s" - , (action == 'lock', participant_id) - ) + account.set_is_locked(action == 'lock') if then == u'': then = u'/%s/' % participant_id