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

Coinbase #2053

Closed
wants to merge 39 commits into from
Closed

Coinbase #2053

wants to merge 39 commits into from

Conversation

chadwhitacre
Copy link
Contributor

This is a redo of #2052 to bring the commits into the main repo so we can all hack on it.


Hey there! This is the front-end code (and some back-end stuff) to add Coinbase support to Gratipay 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!

## To Do - [x] Fix bitcoin address styling - #2065

@chadwhitacre chadwhitacre mentioned this pull request Feb 20, 2014
I suspect bitcoin is the more popular one-off method of the two, in
addition to being alphabetically first.
@chadwhitacre
Copy link
Contributor Author

I rearranged the accounts elsewhere listing, per #2052 (comment).

screen shot 2014-02-19 at 11 53 12 pm

The "One-off Receiving Options" changes to "One-off Giving Options" when viewing someone else's profile.

@chadwhitacre
Copy link
Contributor Author

I'm seeing serious brokenness (along the lines of #2052 (comment)) for the group elsewhere pages (e.g.):

screen shot 2014-02-19 at 11 55 08 pm

Reflowing, minor verbiage.
@chadwhitacre
Copy link
Contributor Author

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

Per IRC, Balanced will control a Coinbase OAuth app in our name. They'll store the secret.

@chadwhitacre
Copy link
Contributor Author

We added a little note about the new feature, but you may or may not want that.

Right, thanks. I think we'll take that out. :-)

@steveklabnik
Copy link

Per IRC, Balanced will control a Coinbase OAuth app in our name. They'll store the secret.

Correct. The idea is that we manage all the shenanigans of connecting to Coinbase for you, it's the same as tokenizing a card. In this case, the OAuth secret is like the card number.

@chadwhitacre
Copy link
Contributor Author

I'm seeing the "Edit" buttons replaced with links:

screen shot 2014-02-20 at 12 32 12 am

vs.

screen shot 2014-02-20 at 12 33 24 am

Does that belong in this PR?

@steveklabnik
Copy link

/cc @kyungmin , though ultimately this is a product decision, really.

@kyungmin
Copy link

@whit537 The latest commit should fix the unintentional button size changes. I agree that the "edit" change is irrelevant to this PR.

@patcon
Copy link
Contributor

patcon commented Feb 20, 2014

Found visual styling defect, but reported in #2065 since it's tangential

@chadwhitacre
Copy link
Contributor Author

Thanks @kyungmin. I'm still seeing problems related to #2053 (comment), however.

@ESWAT ESWAT removed their assignment Mar 13, 2014
This was referenced Mar 14, 2014
@chadwhitacre
Copy link
Contributor Author

We've deployed some of the Connected Accounts style changes. Here's a screenshot:

screen shot 2014-03-14 at 5 01 53 pm

@chadwhitacre
Copy link
Contributor Author

It's time to start chipping away at the Coinbase integration itself if we can. The most significant change involved here is that we are moving from one way to pay in, to two ways, which is basically infinite ways according to the 0, 1, infinity rule. The approach I'm seeing on this PR is to try charging a credit card, and then fall back to charging an "external_account":

            try:
                source = customer.cards.one()
            except balanced.exc.wac.NoResultFound:
                source = customer.external_accounts.one()

What concerns me about this approach is that Gittip's funding flow already confuses our users, and adding this implicit rule about cards and external accounts is only going to further muddy the water. See:

  • credit card statement should only be for the difference #1486, where a user who is both giving and receiving is confused about the amount they are being charged.
  • Fix for issue #757 #771, where people think that by adding a bank account, they'll be able to use that to add funds (ACH debit; we only use bank accounts for ACH credit right now).
  • Similar confusion exists around Venmo and Bitcoin—We allow connecting these funding accounts but don't actually use them anywhere.

I suppose that the easiest way to address this confusion without losing ourselves down a rabbit hole (#1063) is the one we've already identified: to group accounts in the UI and label them appropriately. That should be our next PR.

After that we can bust up the monolithic "Balanced Payments" item in the connected accounts listing into two: Credit Card (under "Adding Money"), and Bank Account (under "Withdrawing Money").

At that point we can look at adding Coinbase as an additional item under Adding Money.

@chadwhitacre
Copy link
Contributor Author

@ESWAT Wanna take a crack at grouping/labeling the Connected Accounts? Here's what I think we want:

  • Moving Money Every Week - Balanced Payments
  • Other {Giving,Receiving} Options - Bitcoin, Venmo
  • Social Profiles—Bitbucket, Bountysource, GitHub, OpenStreetMap, Twitter

I took a pass at this in #2124 so you can scavenge for parts there if you like. Iirc you'll want to change <table id="accounts"> to <table class="accounts"> and have several of them. You can access website.platforms.venmo and skip that one in the loop for Social Profiles, or whatevs. Also, "Other Giving Options" should be "Other Receiving Options" when a member is viewing their own profile.

@ESWAT
Copy link

ESWAT commented Mar 14, 2014

@whit537 Sure thing. Is the grouping task higher priority over restyling the account avatars we’ve talked about in #2103 (comment)?

@chadwhitacre
Copy link
Contributor Author

Is the grouping task higher priority over restyling the account avatars we’ve talked about

@ESWAT Yes, thanks for asking. This #2053 PR is the biggest thing on our plate right now. We need to carve up this elephant and eat it all ASAP.

@ESWAT
Copy link

ESWAT commented Mar 14, 2014

Cool. Going to create a new Issue to deal with the grouping then.

@chadwhitacre
Copy link
Contributor Author

thx :)

@duckinator
Copy link
Contributor

Hey, would someone mind explaining what parts of this still need to be pulled out into separate PRs, just so everyone has some idea of how it's going?

Thanks!

@seanlinsley
Copy link
Contributor

This weekend I was planning on working on using CC holds to not hit people's cards when they'd otherwise have enough money incoming, but Coinbase really is more important. Towards that end, the first thing I'll be working on is #1063. Time permitting, I'll also work on Coinbase integration itself. If anyone has time this weekend to pair on this, please let me know.

@chadwhitacre
Copy link
Contributor Author

IRC

From above:

After that we can bust up the monolithic "Balanced Payments" item in the connected accounts listing into two: Credit Card (under "Adding Money"), and Bank Account (under "Withdrawing Money").

At that point we can look at adding Coinbase as an additional item under Adding Money.

@ESWAT
Copy link

ESWAT commented Apr 18, 2014

I can work on splitting the current Balanced Payments section to the one in coinbase, to make room for the coinbase account being displayed.

@chadwhitacre
Copy link
Contributor Author

Now that the adding/withdrawing money sections are split up (#2297), let's look at adding Coinbase as an additional way to add money. If we don't get to #1063 first we should do like @kyungmin & @matthewfl did and simply try Coinbase if the CC doesn't work.

@chadwhitacre
Copy link
Contributor Author

Sounds like @Aaron1011 might take this on. IRC

@chadwhitacre chadwhitacre force-pushed the master branch 2 times, most recently from cf99c0f to 11fcaf3 Compare October 20, 2014 17:37
@rohitpaulk rohitpaulk mentioned this pull request Oct 25, 2014
8 tasks
@chadwhitacre chadwhitacre force-pushed the master branch 3 times, most recently from bb509d8 to 0d20225 Compare November 3, 2014 21:49
@Changaco
Copy link
Contributor

Changaco commented Jan 3, 2015

This is obsolete, closing in favor of #2841.

@Changaco Changaco closed this Jan 3, 2015
@Changaco Changaco deleted the coinbase branch January 3, 2015 09:59
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.