-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Subscriptions: use djstripe events to mail owners #9661
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
f3b962d
to
417450f
Compare
417450f
to
53558fa
Compare
self.object = context_object | ||
self.request = request | ||
self.request = request or HttpRequest() |
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.
we were requiring passing this parameter, and always using HttpRequest
when we didn't have it, so looks like a good default. Doesn't look like we are using it for making decisions.
Querysets for djstripe models. | ||
|
||
Since djstripe is a third-party app, | ||
these are injected at runtime at readthedocs/subscriptions/apps.py. |
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 can also be a collection of helpers, but using a queryset we don't need to import those and just use the model directly :)
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.
Looks good! This is pretty close to get merged.
Since we are changing how most of our email notification works, we will need to QA this closely when deploying these changes to make sure they keep continue working correctly. In particular that we are handling the Stripe webhooks correctly and also the Celery tasks.
Instead of trying to replace the existing queries, I'm making use of djstripe events/webhooks.
we use a webhook to email users the exact time the subscription changes to "canceled" (this is the moment the subs ends, not when a user cancels a subscription, since it will remain active till the end of the cicle).
and get the organization from them.
With this, we don't depend on having just one subs per-organization, and also stop relying on our models a little less.
The queries that are used to disable orgs aren't migrated yet since they query canceled subscriptions (#9652 (comment)).
📚 Documentation previews 📚
docs
): https://docs--9661.org.readthedocs.build/en/9661/dev
): https://dev--9661.org.readthedocs.build/en/9661/