feat: Capture stripe cancellation in SelfServiceSubscriptionRenewal records#95
feat: Capture stripe cancellation in SelfServiceSubscriptionRenewal records#95marlonkeating merged 27 commits intomainfrom
Conversation
marlonkeating
force-pushed
the
mkeating/ENT-11578
branch
from
509185c to
29c99d3
Compare
|
@copilot review now |
|
@marlonkeating I've opened a new pull request, #96, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
Adds first-class tracking of Stripe subscription cancellation state on SelfServiceSubscriptionRenewal so the API/front-end can surface whether a self-serve subscription is currently canceled, including backfill for historical events.
Changes:
- Add
is_canceledboolean field toSelfServiceSubscriptionRenewal(and historical model) + update Stripe webhook handling to maintain it. - Extend
get_stripe_subscription_plan_infoAPI response (+ serializer/tests) to returnis_canceledandrenewed_subscription_plan_uuid. - Add a management command (and tests) to backfill
is_canceledfrom historical Stripe events; enhance admin list display/filtering.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| enterprise_access/apps/customer_billing/models.py | Adds is_canceled field to renewal model. |
| enterprise_access/apps/customer_billing/migrations/0027_historicalselfservicesubscriptionrenewal_is_canceled_and_more.py | Migrates is_canceled onto current + historical tables. |
| enterprise_access/apps/customer_billing/stripe_event_handlers.py | Updates webhook handlers to set/clear cancellation state on related renewals. |
| enterprise_access/apps/customer_billing/management/commands/backfill_subscription_renewal_cancellations.py | New backfill command to derive is_canceled from StripeEventSummary history. |
| enterprise_access/apps/customer_billing/management/commands/tests/test_backfill_cancellations.py | Tests for backfill behavior (dry-run, idempotency, restore events). |
| enterprise_access/apps/customer_billing/admin.py | Surfaces is_canceled in admin list display/filter. |
| enterprise_access/apps/api/v1/views/customer_billing.py | API now returns is_canceled + renewed_subscription_plan_uuid. |
| enterprise_access/apps/api/serializers/customer_billing.py | Serializer updated for new response fields. |
| enterprise_access/apps/api/v1/tests/test_stripe_event_summary_views.py | Tests response includes new fields and reflects cancellation state. |
| enterprise_access/apps/customer_billing/tests/test_stripe_event_handlers.py | Tests webhook handling sets/clears is_canceled. |
| enterprise_access/apps/customer_billing/tests/test_models.py | Confirms is_canceled default is False. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Adds first-class cancellation tracking on SelfServiceSubscriptionRenewal so the system (and admin-portal-facing API responses) can surface whether a self-service subscription has been canceled, with backfill support for historical Stripe events.
Changes:
- Add
is_canceledboolean field toSelfServiceSubscriptionRenewal(and historical model via migration) and expose it via admin. - Update Stripe webhook handling to flip renewal cancellation state on subscription create/update/delete events.
- Extend
get-stripe-subscription-plan-infoAPI response to includeis_canceledandrenewed_subscription_plan_uuid, and add backfill command + tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| enterprise_access/apps/customer_billing/models.py | Adds is_canceled field to renewal model. |
| enterprise_access/apps/customer_billing/migrations/0027_historicalselfservicesubscriptionrenewal_is_canceled_and_more.py | Migrates new is_canceled field onto current + historical tables. |
| enterprise_access/apps/customer_billing/stripe_event_handlers.py | Introduces renewal cancellation-state updater and wires it into relevant Stripe events. |
| enterprise_access/apps/customer_billing/admin.py | Displays/filters renewal cancellation state in Django admin. |
| enterprise_access/apps/customer_billing/management/commands/backfill_subscription_renewal_cancellations.py | New management command to backfill is_canceled from historical Stripe events. |
| enterprise_access/apps/customer_billing/management/commands/tests/test_backfill_cancellations.py | Adds coverage for backfill command behavior (dry-run, idempotency, restore handling). |
| enterprise_access/apps/api/v1/views/customer_billing.py | Extends subscription-plan-info endpoint to return is_canceled + renewed_subscription_plan_uuid. |
| enterprise_access/apps/api/serializers/customer_billing.py | Extends response serializer with the new fields. |
| enterprise_access/apps/api/v1/tests/test_stripe_event_summary_views.py | Adds/updates API tests asserting the new response fields. |
| enterprise_access/apps/customer_billing/tests/test_stripe_event_handlers.py | Adds webhook tests verifying renewals get marked canceled/uncanceled. |
| enterprise_access/apps/customer_billing/tests/test_models.py | Adds model default test for is_canceled=False. |
You can also share your feedback on Copilot code review. Take the survey.
| self.stdout.write( | ||
| self.style.SUCCESS( | ||
| f'Backfill complete. Updated renewals: {updated_count}. Unchanged events: {unchanged_count}.' | ||
| ) | ||
| ) | ||
|
|
| updated = checkout_intent.renewals.update(is_canceled=is_canceled) | ||
| logger.info( | ||
| 'Updated %d renewal(s) for CheckoutIntent %s: is_canceled=%s', | ||
| updated, checkout_intent.id, is_canceled, | ||
| ) |
There was a problem hiding this comment.
Pull request overview
This PR adds first-class cancellation tracking to self-service subscription renewals so the frontend can surface “canceled vs active” state and the renewed plan UUID more easily.
Changes:
- Add
is_canceledtoSelfServiceSubscriptionRenewal(and historical model) + update webhook handlers to flip the flag on delete/restore. - Extend
get-stripe-subscription-plan-infoAPI response + serializer to includeis_canceledandrenewed_subscription_plan_uuid. - Introduce a backfill management command with dedicated test coverage; improve admin list display/filtering.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| enterprise_access/apps/customer_billing/tests/test_stripe_event_handlers.py | Adds tests asserting renewals are marked canceled/uncanceled based on Stripe subscription events. |
| enterprise_access/apps/customer_billing/tests/test_models.py | Adds model default test for is_canceled=False. |
| enterprise_access/apps/customer_billing/stripe_event_handlers.py | Introduces helper to update renewal cancellation state; hooks it into subscription created/updated/deleted handlers. |
| enterprise_access/apps/customer_billing/models.py | Adds is_canceled BooleanField to SelfServiceSubscriptionRenewal. |
| enterprise_access/apps/customer_billing/migrations/0027_historicalselfservicesubscriptionrenewal_is_canceled_and_more.py | Migration to add is_canceled to both current and historical tables. |
| enterprise_access/apps/customer_billing/management/commands/backfill_subscription_renewal_cancellations.py | New command to backfill is_canceled from historical Stripe delete/restore events. |
| enterprise_access/apps/customer_billing/management/commands/tests/test_backfill_cancellations.py | Comprehensive tests for the backfill command behavior and output. |
| enterprise_access/apps/customer_billing/admin.py | Adds admin list display/filtering for is_canceled and optimizes queryset. |
| enterprise_access/apps/api/v1/views/customer_billing.py | Adds is_canceled and renewed_subscription_plan_uuid to subscription plan info response. |
| enterprise_access/apps/api/v1/tests/test_stripe_event_summary_views.py | Updates/extends API tests for the new response fields. |
| enterprise_access/apps/api/serializers/customer_billing.py | Extends response serializer with is_canceled and renewed_subscription_plan_uuid. |
Comments suppressed due to low confidence (1)
enterprise_access/apps/customer_billing/stripe_event_handlers.py:702
subscription_deletedcallscheckout_intent.previous_summary(...)and then immediately dereferencesprevious_summary.subscription_statuswithout a None-check. If there is no prior summary (or prior summaries havestripe_event_created_at=NULL, whichprevious_summaryfilters out), this will raise and abort handling the deletion event. Add an explicitif not previous_summary: ... return(or otherwise handle the missing-summary case) before accessingsubscription_status.
_update_renewal_cancellation_state(checkout_intent, is_canceled=True)
previous_summary = checkout_intent.previous_summary(event, stripe_object_type='subscription')
if previous_summary.subscription_status == StripeSubscriptionStatus.ACTIVE:
# https://docs.stripe.com/api/subscriptions/object#subscription_object-ended_at
You can also share your feedback on Copilot code review. Take the survey.
| restore_events = StripeEventSummary.objects.filter( | ||
| checkout_intent=deleted_event.checkout_intent, | ||
| event_type__in=['customer.subscription.created', 'customer.subscription.updated'], | ||
| ) | ||
|
|
||
| if deleted_event.stripe_event_created_at: | ||
| restore_events = restore_events.filter( | ||
| stripe_event_created_at__gt=deleted_event.stripe_event_created_at, | ||
| ) | ||
| else: | ||
| restore_events = restore_events.filter(created__gt=deleted_event.created) | ||
|
|
| SelfServiceSubscriptionRenewalFactory, | ||
| StripeEventDataFactory | ||
| ) | ||
|
|
||
|
|
marlonkeating
force-pushed
the
mkeating/ENT-11578
branch
from
4227762 to
969a20f
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #95 +/- ##
==========================================
+ Coverage 84.23% 84.24% +0.01%
==========================================
Files 144 144
Lines 12173 12195 +22
Branches 1159 1161 +2
==========================================
+ Hits 10254 10274 +20
- Misses 1599 1600 +1
- Partials 320 321 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| is_canceled = serializers.BooleanField( | ||
| default=False, | ||
| required=False, | ||
| help_text=( |
There was a problem hiding this comment.
Everything looks good, though copilot did make some good comments, but I was wondering if there's any value in making this a timestamp that is null when it's not canceled?
| subscription.id, | ||
| checkout_intent.id, | ||
| ) | ||
| _update_renewal_cancellation_state(checkout_intent, is_canceled=True) |
There was a problem hiding this comment.
Just to be completely clear, this event handler for customer.subscription.deleted is NOT executed when the admin clicks the cancel button. Our Stripe subscriptions are configured to "cancel_at_period_end", which means only the customer.subscription.updated event is emitted. More info: https://2u-internal.atlassian.net/wiki/spaces/SOL/pages/2202107978/ENT-10715+SSP+Cancellation#User-initiated-cancellation
Was it intentional to NOT update renewals on clicking the cancel button?
There was a problem hiding this comment.
Pull request overview
Adds cancellation state tracking to self-service subscription renewals so API/admin surfaces whether a Stripe subscription has been canceled, and provides a backfill command + tests to populate historical data.
Changes:
- Add
is_canceledtoSelfServiceSubscriptionRenewal(+ historical model) and expose it via the subscription-plan-info API response. - Update Stripe webhook handlers to flip
is_canceledbased on subscription lifecycle events. - Add a management command (and tests) to backfill cancellation state from historical Stripe events; enhance admin list display/filtering.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| enterprise_access/apps/customer_billing/tests/test_stripe_event_handlers.py | Adds tests asserting renewals get marked canceled/uncanceled from Stripe events. |
| enterprise_access/apps/customer_billing/tests/test_models.py | Adds default-value test for is_canceled. |
| enterprise_access/apps/customer_billing/stripe_event_handlers.py | Introduces renewal cancellation-state update helper and calls it from subscription handlers. |
| enterprise_access/apps/customer_billing/models.py | Adds is_canceled field to renewal model. |
| enterprise_access/apps/customer_billing/migrations/0029_historicalselfservicesubscriptionrenewal_is_canceled_and_more.py | Migration adding is_canceled to current + historical renewal tables. |
| enterprise_access/apps/customer_billing/management/commands/backfill_subscription_renewal_cancellations.py | New command to backfill is_canceled from deletion/restore events. |
| enterprise_access/apps/customer_billing/management/commands/tests/test_backfill_cancellations.py | New test suite for the backfill command behavior and output. |
| enterprise_access/apps/customer_billing/admin.py | Admin list display/filter additions for renewal cancellation state. |
| enterprise_access/apps/api/v1/views/customer_billing.py | API endpoint now returns is_canceled and renewed_subscription_plan_uuid. |
| enterprise_access/apps/api/v1/tests/test_stripe_event_summary_views.py | Tests for new API fields and cancellation toggling. |
| enterprise_access/apps/api/serializers/customer_billing.py | Response serializer extended with is_canceled and renewed_subscription_plan_uuid. |
| first_related_renewal = SelfServiceSubscriptionRenewal.objects.filter( | ||
| Q(prior_subscription_plan_uuid=subscription_plan_uuid) | | ||
| Q(renewed_subscription_plan_uuid=subscription_plan_uuid) | ||
| ).select_related('checkout_intent').order_by('created').first() | ||
| if first_related_renewal: | ||
| checkout_intent_uuid = first_related_renewal.checkout_intent.uuid | ||
| is_canceled = first_related_renewal.is_canceled | ||
| renewed_subscription_plan_uuid = first_related_renewal.renewed_subscription_plan_uuid |
There was a problem hiding this comment.
The SelfServiceSubscriptionRenewal lookup can match multiple rows when a plan UUID appears as renewed_subscription_plan_uuid in an earlier renewal and later as prior_subscription_plan_uuid in a subsequent renewal (common for intermediate paid plans). Because the query ORs both fields and then orders by created ascending, it may select the wrong renewal and return an incorrect renewed_subscription_plan_uuid / is_canceled. Consider disambiguating (e.g., prefer a renewal where prior_subscription_plan_uuid matches the query, otherwise fall back to a match on renewed_subscription_plan_uuid), or otherwise make the selection deterministic and aligned with the endpoint’s intent.
| subscription_cancel_at = serializers.DateTimeField( | ||
| allow_null=True, | ||
| required=False, | ||
| help_text=( | ||
| 'Timestamp when the subscription is scheduled to be canceled. ' | ||
| 'None if no cancellation is scheduled or if the subscription has already been deleted.' | ||
| ), | ||
| ) | ||
|
|
There was a problem hiding this comment.
This serializer now has both canceled_date and subscription_cancel_at, but the view only supplies canceled_date. If subscription_cancel_at is intended to be part of the API contract, ensure the view populates it (and clarify how it differs from canceled_date); otherwise, remove it to avoid an advertised-but-never-returned field.
| subscription_cancel_at = serializers.DateTimeField( | |
| allow_null=True, | |
| required=False, | |
| help_text=( | |
| 'Timestamp when the subscription is scheduled to be canceled. ' | |
| 'None if no cancellation is scheduled or if the subscription has already been deleted.' | |
| ), | |
| ) |
| 'checkout_intent_uuid': str(self.checkout_intent.uuid), | ||
| 'is_canceled': False, | ||
| 'renewed_subscription_plan_uuid': str(self.renewed_subscription_plan_uuid), | ||
| 'subscription_cancel_at': None, |
There was a problem hiding this comment.
These assertions require a subscription_cancel_at key in the response, but get_stripe_subscription_plan_info currently returns canceled_date (and does not populate subscription_cancel_at), so this test will fail. Either update the view to return subscription_cancel_at or adjust the expected response to match the actual response contract.
| 'subscription_cancel_at': None, |
| event_data = StripeEventData.objects.create( | ||
| event_id='evt_test_is_canceled_default', | ||
| event_type='customer.subscription.updated', | ||
| checkout_intent=self.checkout_intent, | ||
| data={'test': 'data'} | ||
| ) |
There was a problem hiding this comment.
These tests create StripeEventData with data={'test': 'data'}. The post_save signal tries to build a StripeEventSummary via populate_with_summary_data(), which expects data['data']['object']... and will raise/log an error for this payload. Use StripeEventDataFactory (or a minimally valid Stripe payload structure) here to avoid noisy error logs and to better match real event data.
| first_related_renewal = SelfServiceSubscriptionRenewal.objects.filter( | ||
| Q(prior_subscription_plan_uuid=subscription_plan_uuid) | | ||
| Q(renewed_subscription_plan_uuid=subscription_plan_uuid) | ||
| ).select_related('checkout_intent').order_by('created').first() | ||
| if first_related_renewal: | ||
| checkout_intent_uuid = first_related_renewal.checkout_intent.uuid | ||
| is_canceled = first_related_renewal.is_canceled | ||
| renewed_subscription_plan_uuid = first_related_renewal.renewed_subscription_plan_uuid | ||
| canceled_date = first_related_renewal.subscription_cancel_at |
There was a problem hiding this comment.
first_related_renewal is fetched with order_by('created').first(), which returns the oldest renewal. Since SelfServiceSubscriptionRenewal is ordered by -created (and multiple renewals per checkout intent are possible), this can return stale is_canceled / renewed_subscription_plan_uuid values. Prefer selecting the latest renewal (e.g., rely on model ordering or order by -created).
| response_serializer = serializers.StripeSubscriptionPlanInfoResponseSerializer( | ||
| data={ | ||
| 'upcoming_invoice_amount_due': upcoming_invoice_amount_due, | ||
| 'currency': currency, | ||
| 'canceled_date': canceled_date, | ||
| 'checkout_intent_uuid': checkout_intent_uuid | ||
| 'checkout_intent_uuid': checkout_intent_uuid, | ||
| 'is_canceled': is_canceled, | ||
| 'renewed_subscription_plan_uuid': renewed_subscription_plan_uuid, | ||
| }, |
There was a problem hiding this comment.
StripeSubscriptionPlanInfoResponseSerializer now defines subscription_cancel_at, and the tests expect it in the response, but the view never includes subscription_cancel_at in the serializer input dict. As a result, DRF will omit the field from response_serializer.data, and the response schema won’t match the tests/client expectations. Populate subscription_cancel_at explicitly (likely from the renewal) or remove the field/tests if it’s not intended to be returned.
Co-authored-by: Marlon Keating <mkeating@2u.com>
marlonkeating
force-pushed
the
mkeating/ENT-11578
branch
from
13ff058 to
be1b70d
Compare
We want to start tracking stripe cancellations within the
SelfServiceSubscriptionRenewalobject in order to surface the information easier on the front-end.Jira:
ENT-11578
The main additions are an
is_canceledfield to relevant models and serializers, API support for reporting cancellation state, a management command for backfilling historical data, and improved admin and test coverage.Testing Instructions:
is_canceled,renewed_subscription_plan_uuidmatches value onSelfServiceSubscriptionRenewalrecordMerge checklist:
./manage.py makemigrationshas been runPost merge: