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

Vault new cards at Braintree #3389

Merged
merged 26 commits into from
May 12, 2015
Merged

Vault new cards at Braintree #3389

merged 26 commits into from
May 12, 2015

Conversation

rohitpaulk
Copy link
Contributor

#3377, #3287

  • Backwards compatibility for invalidation
  • Fix credit card page and verify for following cases:
    • No credit card attached
    • Card attached (balanced)
    • Card attached (braintree)
    • Updates card (from balanced to braintree)
    • Updates card (from braintree to braintree)
  • Fix receipt page

@rohitpaulk rohitpaulk added this to the Balanced shutdown milestone May 8, 2015
@rohitpaulk
Copy link
Contributor Author

https://developers.braintreepayments.com/javascript+python/guides/credit-cards#options

Looks like Braintree only requires the postal_code, and none of the other address fields.

rohitpaulk added 7 commits May 8, 2015 21:10
Initially, I was thinking of linking braintree's customer ID directly to the participant's ID in our database. Later decided against it, coz (a) We don't yet know if it'll be possible to map the custom ID when we migrate cards from Balanced to Braintree (b) We'll end up adding a DB column like `has_braintree_account` anyway for caching purposes, it's a lot easier to just reuse the logic that we had with balanced (especially in the tests)
@chadwhitacre
Copy link
Contributor

Per #3387 (comment), let's limit the scope of this PR to vaulting new cards on Braintree instead of Balanced. We want to deploy that ASAP, don't want to block it on billing.exchanges work.

This is used in many of the tests, and the settings page.
@chadwhitacre
Copy link
Contributor

Per #3387 (comment) let's please double-vault to avoid a race condition unless we can think it through again from the top to ensure there isn't one.

@chadwhitacre
Copy link
Contributor

And !m @rohitpaulk. Happy to be rustlin' dogies with you, friend. :-)

@rohitpaulk
Copy link
Contributor Author

!m @whit537 :)

@rohitpaulk
Copy link
Contributor Author

@whit537 - I haven't gotten to the Braintree tests, but this is ready for you to look at and start testing manually.

@rohitpaulk rohitpaulk changed the title Braintree \m/ Vault new cards at Braintree May 9, 2015
@chadwhitacre
Copy link
Contributor

Awesome, thank you.

Brought back the tests marked as xfail,
deleted the migration page,
reverted changes to credit card and bank account pages
@chadwhitacre
Copy link
Contributor

In terms of timing on this, I think we should wait to deploy until the old cards are migrated to Braintree. Sound right, @rohitpaulk?

@rohitpaulk
Copy link
Contributor Author

In terms of timing on this, I think we should wait to deploy until the old cards are migrated to Braintree. Sound right, @rohitpaulk?

Nope, this is ready to deploy now - it's backwards compatible with balanced cards.

@rohitpaulk
Copy link
Contributor Author

@whit537 - I'd look at this as a better alternative to #3392.

@chadwhitacre
Copy link
Contributor

Took a first read through the code. What happens if someone fills out the credit card form after Balanced initiates the export but before Braintree completes it?

@rohitpaulk
Copy link
Contributor Author

If they're filling out the cc form - the new details will be sent to Braintree and if there was an old card(Braintree or Balanced), it'll be deleted.

@chadwhitacre
Copy link
Contributor

the new details will be sent to Braintree

And then what happens when Braintree imports the old details, which will be included in the snapshot that Balanced took before the user posted the cc form?

@rohitpaulk
Copy link
Contributor Author

Once braintree is done importing, we'll only use the accounts for those who have not updated their card details, and delete the others.

@chadwhitacre
Copy link
Contributor

Interesting. I guess I need to run this code and look at the sandbox to understand how that's possible. I guess when we start saving cards now we'll be generating new Braintree customer ids, right? I guess Braintree will create a second customer record for those customers when it does the import. Presumably Balanced sent along some info that will show up in the customer records at Braintree that will allow us to connect them with participants in our database, and we can deduplicate based on that. Is that it?

@chadwhitacre
Copy link
Contributor

Two more points:

chadwhitacre added a commit that referenced this pull request May 12, 2015
Vault new cards at Braintree
@chadwhitacre chadwhitacre merged commit 87d3a68 into master May 12, 2015
@chadwhitacre chadwhitacre deleted the braintree branch May 12, 2015 02:53
@chadwhitacre
Copy link
Contributor

@chadwhitacre
Copy link
Contributor

Steps to reproduce: visit https://gratipay.com/about/me/routes/credit-card.html.

@chadwhitacre
Copy link
Contributor

P.S. Does Braintree vault for us automatically or do we have to explicitly ask for cards to be stored permanently?

@rohitpaulk
Copy link
Contributor Author

I guess when we start saving cards now we'll be generating new Braintree customer ids, right?

Yes.

I guess Braintree will create a second customer record for those customers when it does the import.
Presumably Balanced sent along some info that will show up in the customer records at Braintree that will allow us to connect them with participants in our database, and we can deduplicate based on that.

I'm assuming that we'll be provided with a mapping between balanced_customer_href's and braintree_customer_ids. We search for a participant via the balanced_customer_href and update their braintree_customer_id and card route only if they don't have a braintree card route already.

@rohitpaulk
Copy link
Contributor Author

Bringing back bank accounts opens up the hole mentioned at #3392 (comment) again.

As long as people can close their accounts (and use their bank account as the disbursement strategy), isn't that hole always open?

Should we include the braintree sandbox login in a comment so devs can inspect as needed?

Hmm, we didn't do that for Balanced. I think it'd be best to wait for someone to ask.

Does Braintree vault for us automatically or do we have to explicitly ask for cards to be stored permanently?

Automatically 💃

@chadwhitacre
Copy link
Contributor

Hmm, we didn't do that for Balanced. I think it'd be best to wait for someone to ask.

Balanced did sandboxing differently. But yeah we can wait.

[I]sn't that hole always open?

Yeah, but it's wider if people can connect new accounts.

Automatically

Cool.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants