Step 1b — the Subscription / SubscriptionItem data model - #809
Draft
mitchelljkotler wants to merge 4 commits into
Draft
Step 1b — the Subscription / SubscriptionItem data model#809mitchelljkotler wants to merge 4 commits into
mitchelljkotler wants to merge 4 commits into
Conversation
Follows the model split by moving every call site onto the right half of the pair, and fixes the behaviour that the split had silently changed. Adding a plan now joins the organization's existing subscription when the billing shape matches, so an org receives one invoice instead of several. SubscriptionItemQuerySet.start() picks the subscription by interval and collection method, creates it only when nothing matches, and otherwise pushes the new line onto the live Stripe subscription. Fixes carried in with it: - Subscription.free read item.free, which is a Plan attribute - Subscription.cancel() called send_slack_notification, which is on the line; it now announces every line it cancelled - SubscriptionItem had no cancel(), so removing one plan cancelled the whole subscription. The last line cancels the subscription at period end; any other is dropped from Stripe with proration suppressed - subscription_cancelled() and the invoice metadata assumed one plan per subscription - the admin's plan filter used to_attr on a nested prefetch path, so plan_subscriptions was always None - the Mailchimp journey and "started" Slack notification were lost in the split; both are back on the line, which is what names a plan - webhook handlers looked up Stripe subscription ids on the line - a bulk rename had mangled user-facing strings into "SubscriptionItem Payment", "SubscriptionItem cancelled." and similar SubscriptionItem.organization is a read-only property reaching through the parent, so the column stays on Subscription and a line can never disagree with the subscription it bills on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The pack entitlement tests were added on pricing-models, after the commit that renamed SubscriptionFactory across the test suite, so their usages were never converted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacks on #781. Split out of #784 so the structural change and the behaviour change can be read separately.
What this is
The rename and the restructure, and nothing that changes what the system does. The question to read it with is "does this still mean the same thing?" — almost every hunk is the same expression reached a different way.
Subscription→SubscriptionItem, and a newSubscriptionparent that owns the Stripe subscription. Stripe requires every item on a subscription to share a billing interval and collection method, so an organization needs one subscription per combination it holds — that's what the uniqueness constraint encodes.What it deliberately isn't
Per-item cancellation, the Stripe sync logic, the billing pages, the renewal date moving off the API, the reconciliation commands — all in #784, which is 25 files and where every review finding so far has landed.
Migrations
0083and0084land here in draft form; #784 finalises them (Squash the split migrations and make them reversible) and adds0085. Since this branch never reaches master without #784 merged into it, the intermediate migrations are never applied anywhere — but it does mean the migration files you see here are not the ones that ship. Read them in #784.QA
There should be nothing to test behaviourally — that's the claim this PR is making. Worth confirming on the preview deploy that the organization payment page, the plan list and the admin all render as they do on master.
🤖 Generated with Claude Code
https://claude.ai/code/session_0151rwxtvLg1vKEiz7XsJfzf