Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Coinbase #2052

Closed
wants to merge 19 commits into from
Closed

Coinbase #2052

wants to merge 19 commits into from

Conversation

kyungmin
Copy link

Hey there! This is the front-end code (and some back-end stuff) to add Coinbase support to Gittip through Balanced.

How it works

People will be able to donate USD gifts, but pay for them via Bitcoins in their Coinbase account.

Screenshots:

screen shot 2014-02-19 at 5 43 32 pm

Currently, everything looks the same here. We added a little note about the new feature, but you may or may not want that.

After choosing an amount and clicking 'confirm,':

screen shot 2014-02-19 at 5 44 46 pm

You have the option of paying via credit card or coinbase.

Choosing Coinbase:

screen shot 2014-02-19 at 5 45 25 pm

A popup window to OAuth to your coinbase account appears. After signing in:

screen shot 2014-02-19 at 5 46 24 pm

This says "Balanced" right now, but will say Gittip in the future. After confirming:

screen shot 2014-02-19 at 5 47 17 pm

Your gift is updated and and it's been connected. On the accounts page:

screen shot 2014-02-19 at 5 48 02 pm

You have the opportunity to see and remove or add the account.

What works in this PR

Basically, I took screenshots of all of this locally, so that all works. Hooray!

To be clear, this means that nobody is actually receiving Bitcoins, because they are instantaneously converted to USD by Coinbase. This means that individuals receiving tips don't have to opt in, they won't even know how much of their tips were sent via BTC.

What still needs to be done

The downside is.... it doesn't actually save this information in the Gittip backend. Some work needs to be done to actually save the funding instrument that comes back from Balanced, and charge from the correct account.

Balanced is obviously happy to help make this happen, but we don't know the Gittip codebase really well, so we should all pitch in with this!

This PR relies on #2036 landing before it can be merged. This PR is based on top of it, you can see all the other commits below.

Furthermore, it would be nice if #1165 could be figured out; this would allow someone to be able to tip in BTC itself, rather than tipping in a certain amount of BTC denominated in USD. We have some mocks for what that UI would look like if you're interested.

Conclusion

❤️ ❤️ ❤️

Let's hash out all the details and get this going!

@steveklabnik
Copy link

😍 😍 😍

@mahmoudimus
Copy link

Sweet! 👯

@KimlingLam
Copy link

Awesome!!!

@chadwhitacre
Copy link
Contributor

Oh my word. Here we go! 🙀

@chadwhitacre
Copy link
Contributor

Okay! I've got this running locally. First thing I notice is that the new button styles are applied globally, with some unintended (yes?) consequences:

screen shot 2014-02-19 at 9 53 16 pm

screen shot 2014-02-19 at 9 54 02 pm

Maybe the consequences were intended.

@chadwhitacre
Copy link
Contributor

First traceback! After logging in, I get:

Traceback (most recent call last):
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/algorithm.py", line 288, in run
    new_state = function(**deps.as_kwargs)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/aspen/algorithms/website.py", line 88, in get_response_for_resource
    return {'response': resource.respond(request)}
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/aspen/resources/dynamic_resource.py", line 52, in respond
    exec self.pages[1] in context
  File "/Users/whit537/personal/gittip/www.gittip.com/www/%username/index.html.spt", line 58, in 
    card = billing.BalancedCard(balanced_account_uri)
  File "/Users/whit537/personal/gittip/www.gittip.com/gittip/billing/__init__.py", line 231, in __init__
    self._customer = balanced.Customer.fetch(balanced_account_uri)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/balanced/resources.py", line 211, in fetch
    return cls.get(href)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/wac.py", line 1365, in get
    resp = cls.client.get(uri)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/wac.py", line 448, in get
    return self._op(self.interface.get, uri, **kwargs)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/wac.py", line 512, in _op
    handle_error(ex)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/wac.py", line 489, in handle_error
    raise ex
HTTPError: 403 Client Error: FORBIDDEN

@chadwhitacre
Copy link
Contributor

Maybe that's a mismatch with the dev marketplace I'm still using?

@chadwhitacre
Copy link
Contributor

Yeah, sorry. I was using a production backup against a test marketplace. With an empty dev db I'm able to proceed.

var balanced_js = "https://js.balancedpayments.com/1.dev/balanced.js";
jQuery.getScript(balanced_js, function() {
balanced.init(balanced_uri);
balanced.externalAccount.create('coinbase', Gittip.payments.cb.handleResponse);
Copy link
Contributor

Choose a reason for hiding this comment

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

This. Is. Wild. You're aweome @kyungmin!

Do we have any ability to pass through the meta params from the Coinbase API docs?
https://coinbase.com/docs/api/permissions#additional-parameters

This part of the Balanced API isn't doc'd yet, so maybe this isn't possible yet.

Choose a reason for hiding this comment

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

Currently, no, we don't have that kind of thing. Maybe we should? This is one great reason to do this kind of thing in public. 😍

@chadwhitacre
Copy link
Contributor

So a few first impressions:

  • Thanks! This is great! !m @kyungmin @matthewfl @balanced 💃
  • I like the updated treatment of "accounts elsewhere" (as we call them), with the dividing line running horizontally instead of vertically, more prominent "add" button, and the platform name in small caps.
  • I appreciate the sentiment to start grouping accounts elsewhere. Let's work out some issues:
    • Only four of the six under "user logins" are actually supported to login to Gittip (Bitbucket, GitHub, OpenStreetMap, Twitter).
    • "Send Money" and "Receive Money" aren't quite right, because you don't need "exchange" instruments attached in order to send and receive money (our terminology is surely non-standard, but we use the word "transfer" to mean moving money around inside of Gittip, and "exchange" to mean moving money between Gittip and the outside world).
    • The bitcoin address field is actually mostly useful for advertising an alternate funding method for one-off payments that are out-of-band with respect to Gittip's weekly cycle. In that sense it's almost better classified with Venmo.
    • I notice that the "Send Money" and "Receive Money" accounts are now private. This addresses Make it possible to hide your bank account/credit card status from profile #653, but note that the bitcoin address should actually be public.

Given all that, how about this grouping?

  • Social Profiles - Twitter, GitHub, Bitbucket, Bountysource, OpenStreetMap
  • One-off Giving Options - Venmo, Bitcoin
  • Adding Money - Credit Card, Coinbase
  • Withdrawing Money - Bank Account

@chadwhitacre
Copy link
Contributor

When I try to attached a Coinbase account, the pop-up gets stuck here:

screen shot 2014-02-19 at 10 58 27 pm

I'm not seeing anything in the js console.

@steveklabnik
Copy link

Bummertown. @kyungmin , any idea how that'd happen?

@chadwhitacre
Copy link
Contributor

Furthermore, it would be nice if #1165 could be figured out; this would allow someone to be able to tip in BTC itself, rather than tipping in a certain amount of BTC denominated in USD. We have some mocks for what that UI would look like if you're interested.

Thanks, but I think we should punt on that. #1165 is going to be a fair bit of work, and this PR is enough to chew on as it is. When we first started talking about bitcoin we actually had people adamantly wanting to not receive BTC unless it was transparently converted to USD, so this behavior fits with our original vision for bitcoin integration, and adds a further wrinkle to the multicurrency problem for us. We'll get there, but let's leave it off this PR. :-)

@steveklabnik
Copy link

Great! Just wanted to make sure the behavior is crystal clear.

Payments are hard. So many damn edge cases!

@chadwhitacre
Copy link
Contributor

@steveklabnik @kyungmin I should say that stuckness is after I go through the Coinbase oauth flow.

@chadwhitacre
Copy link
Contributor

Closing in favor of #2053, which is in a branch on the main repo so we can all hack on it. I've added @kyungmin and @matthewfl as collaborators so they should have access.

chadwhitacre added a commit that referenced this pull request Feb 20, 2014
This was referenced Feb 20, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants