-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Subscriptions: attach stripe subscription to organizations #9751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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.
This is good to me. I'd like to see more comments in the code only, but 👍🏼
# TODO: remove this once we don't depend on our Subscription models. | ||
from readthedocs.subscriptions.models import Subscription | ||
|
||
subscription = Subscription.objects.get_or_create_default_subscription(self) | ||
if not subscription: | ||
# This only happens during development. | ||
return None | ||
|
||
active_subscription = self.stripe_customer.subscriptions.filter( |
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.
active_subscription = self.stripe_customer.subscriptions.filter( | |
# Always return the active subscription first if the customer has multiple subscriptions | |
active_subscription = self.stripe_customer.subscriptions.filter( |
if not subscription: | ||
# This only happens during development. |
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 not subscription: | |
# This only happens during development. | |
if not subscription and settings.DEBUG: | |
# This only happens during development. |
Make sure that this only happens on development. Otherwise, we should log an error/warning here.
if not stripe_subscription: | ||
log.info("Stripe subscription not found.") | ||
return |
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.
What is the case where this can happen? I'm not sure to follow it. If Stripe Checkout is the one that creates the Subscription, why we would not find it after that?
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.
just being defensive, the creation of the subs object could have failed or could have been deleted.
After deploying this code, we need to attach the proper stripe subscription object to organizations
Closes #9652
Closes https://github.com/readthedocs/readthedocs-corporate/issues/1508
Ref #9313 (comment)