Separate out payment methods - #747
Conversation
3175d8a to
ef9e135
Compare
allanlasser
left a comment
allanlasser
left a comment
There was a problem hiding this comment.
There are 4 lifecycle events from Stripe that we should handle for payment methods, if we aren't already:
| def default_payment_method_obj(self): | ||
| """Return the default PaymentMethod object, or None. | ||
|
|
||
| Caches the result on the instance so multiple property | ||
| accesses in the same request only hit the DB once. The | ||
| cache is cleared automatically by ``save_payment_cache`` | ||
| and ``clear_payment_cache``. | ||
| """ | ||
| sentinel = object() | ||
| cached = getattr(self, "_default_pm_cache", sentinel) | ||
| if cached is not sentinel: | ||
| return cached | ||
| result = self.payment_methods.filter(is_default=True).first() | ||
| self._default_pm_cache = result | ||
| return result | ||
|
|
||
| def _invalidate_pm_cache(self): | ||
| try: | ||
| del self._default_pm_cache | ||
| except AttributeError: | ||
| pass | ||
|
|
||
| @property | ||
| def payment_brand(self): | ||
| pm = self.default_payment_method_obj() | ||
| return pm.brand if pm else "" | ||
|
|
||
| @property | ||
| def payment_last4(self): | ||
| pm = self.default_payment_method_obj() | ||
| return pm.last4 if pm else "" | ||
|
|
||
| @property | ||
| def payment_exp_month(self): | ||
| pm = self.default_payment_method_obj() | ||
| return pm.exp_month if pm else None | ||
|
|
||
| @property | ||
| def payment_exp_year(self): | ||
| pm = self.default_payment_method_obj() | ||
| return pm.exp_year if pm else None | ||
|
|
||
| @property | ||
| def stripe_payment_method_id(self): | ||
| pm = self.default_payment_method_obj() | ||
| return pm.stripe_id if pm else "" | ||
|
|
There was a problem hiding this comment.
I like how you preserve the current API here!
|
We don't think we need any new UI, on top of what we've done in #716. We just need to update templates/context in this PR to make sure the data flows through the same as before. |
These are being handled now |
| stripe_customer = customer_svc.retrieve(customer_id) | ||
| invoice_settings = getattr(stripe_customer, "invoice_settings", None) | ||
| default_pm_id = invoice_settings and invoice_settings.get("default_payment_method") | ||
| if default_pm_id != stripe_id: |
There was a problem hiding this comment.
I'm not sure I follow the logic here. If the payment method is being attached, what's the likelihood it's the default payment method? Even though we can support multiple payment methods, we're throwing this away and creating inconsistency between Stripe and Accounts.
There was a problem hiding this comment.
Although the data model supports saving multiple payment methods, this isn't exposed or used anywhere. We could save the data locally to keep things in sync, but don't want to increase the scope to include full multiple payment method handling support at this time.
There was a problem hiding this comment.
I added support to save non default payment methods for data syncing
2 participants
This pulls payment methods (credit cards and bank accounts) out into their own model. This helps organize the data and opens the path to supporting multiple payment methods in the future.