Skip to content

Subscriptions: remove old code #10642

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 16 commits into from
Sep 5, 2023
Merged

Subscriptions: remove old code #10642

merged 16 commits into from
Sep 5, 2023

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Aug 16, 2023

We are now fully using dj-stripe for subscriptions, so we can our code for that. I'm not fully removing the app yet, so we can do a remove the old tables with zero downtime.

  • Currently, we can "lock" subscriptions, this is we never disable the organization, even if the subscription ended.
    Went ahead and moved this to be an attribute of the organization instead.
  • All querysets now use the dj-stripe models.
  • All subscriptions are canceled now when the organization is deleted, previously this happened when the subscription model was deleted, but now we aren't using those anymore.
  • A new handler for when a subscription is created was added, so in case we manually create a subscription for the user, this is reflected in the organization.
  • Other handlers were updated to take into consideration a change in subscriptions, and re-enable organizations is case the subscription is changed to active. I think we should be covered now for all cases when managing the subcriptions from stripe, no need to touch the django admin (hopefully).
  • test_daily_email was renamed to test_tasks, since we were testing that.
  • There was a problem with the CI using an old cache, our pre-commit setup depends on the testing requirements, so I added that file to the cache key.

After this is deployed, we can zeroed the migrations and remove the app.

How to deploy this change

  • Deploy web-extra
  • Run the migrations in web-extra
  • Run the following code
Organization.objects.filter(subscription__locked=True).update(never_disable=True)
  • Continue as usual

Closes #10624

@stsewd stsewd force-pushed the remove-old-subscriptions-code branch from c74d879 to 3c24454 Compare August 22, 2023 00:29
stsewd added 6 commits August 21, 2023 19:32
We are now fully using dj-stripe for subscriptions, so we can our
code for that. I'm not fully removing the app yet,
so we can do a remove the old tables with zero downtime.
@stsewd stsewd force-pushed the remove-old-subscriptions-code branch from 3c24454 to 8b896aa Compare August 22, 2023 00:32
@stsewd stsewd marked this pull request as ready for review August 22, 2023 21:14
@stsewd stsewd requested a review from a team as a code owner August 22, 2023 21:14
@stsewd stsewd requested a review from humitos August 22, 2023 21:14
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.

This looks pretty good! I wasn't able to review everything yet, tests are missing still, but the core logic is 👍🏼 . I left some feedback as well.

I'll come back with fresh eyes for the tests in the following days.

Comment on lines +65 to +67
queryset = Organization.objects.disable_soon(
days=30, exact=True
).subscription_trial_plan_ended()
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need both here, disable_soon and subscription_trial_plan_ended?

Copy link
Member Author

Choose a reason for hiding this comment

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

We are automatically disabling expired trial subscriptions only, we don't do that automatically for normal subscriptions (I think we should start doing it automatically as well)

@humitos
Copy link
Member

humitos commented Aug 25, 2023

Thanks for considering all my suggestions 👍🏼

@humitos humitos merged commit 25dc389 into main Sep 5, 2023
@humitos humitos deleted the remove-old-subscriptions-code branch September 5, 2023 07:59
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.

Subscriptions: remove old models
2 participants