-
Notifications
You must be signed in to change notification settings - Fork 308
Conversation
I don't see that we had tests for |
@whit537 - I'll take this over, you could focus on using the Balanced -> Braintree migration data. Sound good? |
@rohitpaulk Roger that. |
It looks like customer id is an editable field. The id for customers brought over from Balanced is their Balanced customer id. |
In other words, we don't need a separate Participant ID in |
Moving to #3391 ... |
Good look. |
Looks like we can't capture card holds immediately like with Balanced. |
Yeah, Balanced floated us the money (i.e., they made it available to us in our escrow immediately, even though they didn't actually get the money from the acquiring bank until the next day). This came up in my meeting with Citizens last week. It's not a big deal for now since we have enough in escrow to float the different for a day. As we gain more experience with Braintree and as we reduce escrow (#1383) we'll have to maybe rethink our timing of card captures vs. payouts. |
The main point is that card captures are almost certain to settle without incident, so there's little risk in floating for a day if we can make it work cash-flow-wise. Once we have a hold/authorization, we can count on the funds landing in our bank account sooner or later. |
Cool, so I'll proceed with the assumption that a transaction that is |
Yup. We'll keep an eye out, and if we end up getting burned by this assumption down the road then we'll adjust. |
(so that create_card_hold can pick it up)
@rohitpaulk One test failure, ya? |
@whit537 - Has to do with how we cleanup stuff in the Braintree test sandbox, I'm working on a fix. |
Since I was in a hurry, I neglected changing to Braintree's nomenclature (and I probably left a few |
log(msg + "succeeded.") | ||
error = "" | ||
result = braintree.Transaction.sale({ | ||
'amount': str(cents/100.0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we in danger of float errors here? Why type is cents
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cents
is an int
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You positive? I thought it was a Decimal
here. Regardless, if Braintree accepts dollars we should just pass dollars. We only convert to cents in _prep_hit
because Balanced wanted cents (though does ach_credit
also use _prep_hit
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm reading correctly, cents
comes from _prep_hit
- and https://github.com/gratipay/gratipay.com/blob/master/gratipay/billing/exchanges.py#L288
We only convert to cents in _prep_hit because Balanced wanted cents
True. I wanted to minimize the code changes here, that's why I reused the function.
though does ach_credit also use _prep_hit
No, it doesn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to minimize the code changes here, that's why I reused the function.
Yeah, we can clean that up later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reticketed as #3500.
!m @rohitpaulk |
Closes #3287.