Skip to content
This repository has been archived by the owner. It is now read-only.

Link venmo #1857

Merged
merged 30 commits into from Jan 10, 2014

Conversation

Projects
None yet
5 participants
@thomasboyt
Copy link
Contributor

commented Jan 9, 2014

cc @simon-weber

Issue: #1839

This PR adds Venmo as a linkable account (but not an auth-able one). This is the first step towards several possible new features powered by Venmo (using Venmo as a funding source for recurring tips, using Venmo for paying out developers, and possibly using Venmo as a solution for #5).

This PR also does stopgap refactoring of several parts of elsewhere:

  • Adds the ability to iterate over types of accounts with the new platform_classes registry
  • Cuts down on copy-pasted template code using above
  • Elsewhere records can now be used to construct AccountElsewhere objects so that convenience methods can be used on existing accounts
  • Reorganized env variables file

(sorry about the gross extraneous merge commits, we can rebase this a bit if y'all want)

Thomas Boyt and others added some commits Jan 7, 2014

Merge branch 'refactor-platforms' into link-venmo
Conflicts:
	gittip/elsewhere/__init__.py
Merge branch 'master' into link-venmo
Conflicts:
	gittip/elsewhere/__init__.py
	gittip/models/participant.py
	www/%username/username.json.spt
Thomas Boyt
Merge branch 'master' into link-venmo
Conflicts:
	configure-aspen.py
	default_local.env
	default_tests.env
	gittip/models/_mixin_elsewhere.py
	gittip/wireup.py
	www/%username/index.html.spt
	www/%username/members/index.html.spt
	www/on/confirm.html.spt
	www/on/take-over.html.spt
@clone1018

This comment has been minimized.

Copy link
Contributor

commented Jan 9, 2014

All tests pass for me and linking an account works as expected.

I'll let someone who was working with Venmo release this one.

Thanks for the work guys!

@simon-weber

This comment has been minimized.

Copy link
Contributor

commented Jan 9, 2014

related: #1369. The refactoring in our PR fixes a lot of the same pain points.

@zwn

This comment has been minimized.

Copy link
Contributor

commented Jan 9, 2014

@whit537 will review this one.

branch.sql Outdated
@@ -0,0 +1,3 @@
ALTER TABLE elsewhere ADD COLUMN access_token text DEFAULT NULL;

This comment has been minimized.

Copy link
@chadwhitacre

chadwhitacre Jan 10, 2014

Contributor

Let's wrap these in a single BEGIN/END transaction block (see schema.sql for reference).

return username


class ProblemChangingUsername(Exception):

This comment has been minimized.

Copy link
@chadwhitacre

chadwhitacre Jan 10, 2014

Contributor

This duplicates what's in gittip/exceptions.py, no?

This comment has been minimized.

Copy link
@simon-weber

simon-weber Jan 10, 2014

Contributor

Good catch -- we must have missed that during a merge.

@@ -1,38 +1,53 @@
DATABASE_URL=postgres://gittip:gittip@localhost:5432/gittip

This comment has been minimized.

Copy link
@chadwhitacre

chadwhitacre Jan 10, 2014

Contributor

*.env reorg looks good, thanks. :)

This comment has been minimized.

Copy link
@clone1018

clone1018 Jan 10, 2014

Contributor

One thing to note, swaddle now displays 11 warnings in a row about skipping blank lines.

This comment has been minimized.

Copy link
@simon-weber

simon-weber Jan 10, 2014

Contributor

Ah, we must have missed that because we didn't regenerate our environments. Would running after a make clean show me these warnings?

This comment has been minimized.

Copy link
@chadwhitacre

chadwhitacre Jan 10, 2014

Contributor

Don't sweat it, we're porting away from swaddle (#468).

This comment has been minimized.

Copy link
@chadwhitacre

chadwhitacre Jan 10, 2014

Contributor

Unless @clone1018 wants to sweat it, and add line breaks under #468. :-)

This comment has been minimized.

Copy link
@clone1018

clone1018 Jan 10, 2014

Contributor

Much lazy. Lack effort.

platform_classes = OrderedDict([(platform, None) for platform in platforms_ordered])


class _RegisterPlatformMeta(type):

This comment has been minimized.

Copy link
@chadwhitacre

chadwhitacre Jan 10, 2014

Contributor

This reminds me of something I was doing on the elsewhere refactor branch. Does this relate to that?

This comment has been minimized.

Copy link
@chadwhitacre

This comment has been minimized.

Copy link
@chadwhitacre

chadwhitacre Jan 10, 2014

Contributor

This is what I was thinking of:

https://github.com/gittip/www.gittip.com/pull/1369/files#diff-7698816a9bc10d53930d35de3e4bafdfR13

There I'm using __new__ to switch which AccountElsewhere subclass we're using to hydrate the result of any given database query.

This comment has been minimized.

Copy link
@simon-weber

simon-weber Jan 10, 2014

Contributor

Yup, it accomplishes the same thing as the PlatformRegistry, but doesn't require the manual call to register.

This comment has been minimized.

Copy link
@simon-weber

simon-weber Jan 10, 2014

Contributor

Oh, I misunderstood. Yeah, I think hooking __new__ is the way to go when hydrating.

- Takes a user_id and user_info, and updates the database.
Or:
- Takes a user_id and existing_record, and constructs a "model" object out of the record

This comment has been minimized.

Copy link
@chadwhitacre

chadwhitacre Jan 10, 2014

Contributor

I wonder if the existing_record hack is doing what #1369 is doing with the subclasses of AccountElsewhere.

This comment has been minimized.

Copy link
@simon-weber

simon-weber Jan 10, 2014

Contributor

Yup, that's exactly what we were going for. We did it this way to avoid changing all the existing callsites, though hydrating with __new__ seems like the way to go in the future.

Merge upstream
Conflicts:
	gittip/wireup.py
@@ -11,6 +12,9 @@
from gittip import canonize, configure_payments
from gittip.security import authentication, csrf, x_frame_options
from gittip.utils import cache_static, timer
from gittip.elsewhere import platforms_ordered

This comment has been minimized.

Copy link
@chadwhitacre

chadwhitacre Jan 10, 2014

Contributor

This is unused in this file.

chadwhitacre added a commit that referenced this pull request Jan 10, 2014

@chadwhitacre chadwhitacre merged commit f963d20 into gratipay:master Jan 10, 2014

@chadwhitacre chadwhitacre referenced this pull request Sep 26, 2015

Closed

payout via dwolla masspay #726

0 of 1 task complete
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.