Skip to content

Use djstripe models for organization subscriptions #9486

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

Merged
merged 3 commits into from
Aug 24, 2022
Merged

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Aug 10, 2022

This is to achieve #9312.

Basically just adding relationship to the Customer model from djstripe, we would need to trigger a resync for .com (since the last time there were some errors due to stripe bug with the logo), after that we can migrate the existing organizations to set the stripe customer on their models, this is done automatically for users that visit the subscription page and for new organizations. I'm not doing this as a data migration, since we would be hitting the stripe API in the process, we can execute this script to migrate them from web extra:

from readthedocs.organizations.models import Organization
from readthedocs.subscriptions.utils import get_or_create_stripe_customer

import logging

log = logging.get_logger(__name__)


def migrate(limit=500):
    queryset = Organization.objects.filter(stripe_customer=None).all()
    if limit:
        queryset = queryset[:limit]
    else:
        queryset = queryset.iterator()
    for organization in queryset:
        try:
            get_or_create_stripe_customer(organization)
        except Exception:
            log.exception(
                "Failed to create stripe customer for %{organization}s",
                organization=organization.slug,
            )

I have tested this locally with stripe's test mode.


📚 Documentation preview 📚: https://docs--9486.org.readthedocs.build/en/9486/

@stsewd stsewd force-pushed the use-djstripe branch 6 times, most recently from 5c8d6df to d54e766 Compare August 10, 2022 23:28
@stsewd stsewd marked this pull request as ready for review August 10, 2022 23:59
@stsewd stsewd requested a review from a team as a code owner August 10, 2022 23:59
@stsewd stsewd requested a review from humitos August 10, 2022 23:59
@stsewd
Copy link
Member Author

stsewd commented Aug 16, 2022

we would need to trigger a resync for .com (since the last time there were some errors due to stripe bug with the logo)

This was already done, and all responses from our webhook are all green!

@ericholscher ericholscher self-requested a review August 16, 2022 20:55
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great first step. Excited about this transition!

@@ -784,6 +784,10 @@ def DOCKER_LIMITS(self):
DJSTRIPE_FOREIGN_KEY_TO_FIELD = "id"
DJSTRIPE_USE_NATIVE_JSONFIELD = True # We recommend setting to True for new installations

# We are managing the subscriber relationship by ourselves,
# since we have subscriptions attached to an organization or gold user.
DJSTRIPE_SUBSCRIBER_CUSTOMER_KEY = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there more information here? I don't quite understand this comment -- is it disabling some feature of djstripe? Are we going to turn this back on in the future?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's disabling the feature, yes (well, partially, since stripe still creates a relationship to the user object), and we won't be using this in the future. I'll expand the comment.

@stsewd stsewd merged commit ac1dc7c into main Aug 24, 2022
@stsewd stsewd deleted the use-djstripe branch August 24, 2022 17:52
Copy link
Member

@humitos humitos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I like how the code gets simplified and easier to follow with this package 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants