feat: make CheckoutIntent.ssp_product non-nullable#195
Open
shravani-sonata-gottapu wants to merge 2 commits into
Open
feat: make CheckoutIntent.ssp_product non-nullable#195shravani-sonata-gottapu wants to merge 2 commits into
shravani-sonata-gottapu wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the customer billing checkout flow to make CheckoutIntent.ssp_product required, adjust the CheckoutIntent.user relationship, and enforce uniqueness per (user, ssp_product) so checkout intents are product-scoped.
Changes:
- Adds a guard + schema migration to make
ssp_productnon-nullable, convertusertoForeignKey, and add a unique constraint on(user, ssp_product). - Updates intent creation logic and checkout-session creation to resolve/default an
SspProduct(notablyteams-yearly) when callers don’t provide one. - Updates/extends test helpers/factories and adjusts selected tests for the new non-null behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| enterprise_access/apps/customer_billing/tests/test_models.py | Updates model tests around ssp_product handling/defaulting. |
| enterprise_access/apps/customer_billing/tests/test_migrations.py | Adjusts migration tests for rollback/forward re-runs. |
| enterprise_access/apps/customer_billing/tests/factories.py | Ensures CheckoutIntentFactory always provides an ssp_product. |
| enterprise_access/apps/customer_billing/models.py | Makes ssp_product non-null, changes user field type, adds uniqueness metadata, and updates intent lookup/defaulting logic. |
| enterprise_access/apps/customer_billing/migrations/0037_make_ssp_product_non_nullable_and_user_fk.py | Adds guard + alters fields + adds DB uniqueness constraint. |
| enterprise_access/apps/customer_billing/api.py | Resolves an SspProduct for free-trial checkout session creation. |
| enterprise_access/apps/api/serializers/customer_billing.py | Defaults ssp_product during checkout intent creation via API serializer. |
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (93.06%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #195 +/- ##
==========================================
+ Coverage 86.17% 86.43% +0.25%
==========================================
Files 152 153 +1
Lines 12585 12742 +157
Branches 1200 1227 +27
==========================================
+ Hits 10845 11013 +168
+ Misses 1425 1408 -17
- Partials 315 321 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Comment on lines
+685
to
+690
| # Prefer an existing intent for the same user and SSP product when provided, | ||
| # otherwise fall back to any existing intent for the user (legacy behaviour). | ||
| if ssp_product: | ||
| existing_intent = cls.objects.filter(user=user, ssp_product=ssp_product).order_by('-created').first() | ||
| else: | ||
| existing_intent = cls.objects.filter(user=user).order_by('-created').first() |
Comment on lines
568
to
+572
| # Check if user already has a non-expired intent | ||
| existing_intent = CheckoutIntent.objects.filter( | ||
| existing_qs = CheckoutIntent.objects.filter( | ||
| user=self.user, | ||
| state__in=self.NON_EXPIRED_STATES | ||
| ).first() | ||
| state__in=self.NON_EXPIRED_STATES, | ||
| ) |
Comment on lines
+797
to
+801
| qs = cls.objects.filter(user=user).order_by('-created') | ||
| try: | ||
| return qs[0] | ||
| except IndexError: | ||
| return None |
Comment on lines
+703
to
+707
| ) | ||
| else: | ||
| existing_intent = ( | ||
| cls.objects.filter(user=user).order_by('-created').first() | ||
| ) |
Comment on lines
+304
to
+315
| def save(self, *args, **kwargs): | ||
| """Ensure a valid ``ssp_product`` before persisting.""" | ||
| if not getattr(self, 'ssp_product', None): | ||
| SspProductModel = apps.get_model('customer_billing', 'SspProduct') | ||
| try: | ||
| self.ssp_product = SspProductModel.objects.get(slug='teams-yearly') | ||
| except SspProductModel.DoesNotExist as exc: | ||
| raise ValidationError({'ssp_product': 'Default ssp_product teams-yearly was not found.'}) from exc | ||
| except SspProductModel.MultipleObjectsReturned as exc: | ||
| raise ValidationError({'ssp_product': 'Multiple teams-yearly ssp_product records found.'}) from exc | ||
|
|
||
| return super().save(*args, **kwargs) |
Comment on lines
+839
to
+843
| qs = StripeEventSummary.objects.filter( | ||
| checkout_intent=self, | ||
| stripe_event_created_at__lt=event_timestamp, | ||
| **filter_kwargs, | ||
| ).order_by('-stripe_event_created_at').first() | ||
| ).order_by('-stripe_event_created_at') |
Comment on lines
117
to
121
| updated_count = CheckoutIntent.cleanup_expired() | ||
|
|
||
| # Verify it returns the correct count of updated intents | ||
| self.assertEqual(updated_count, 1, "Should have updated exactly one intent") | ||
|
|
Comment on lines
+125
to
+139
| SspProductModel = apps.get_model('customer_billing', 'SspProduct') | ||
| original_get = SspProductModel.objects.get | ||
|
|
||
| def _raise(*args, **kwargs): | ||
| raise SspProductModel.MultipleObjectsReturned() | ||
|
|
||
| SspProductModel.objects.get = _raise | ||
| try: | ||
| with self.assertRaises(RuntimeError): | ||
| CheckoutIntent.create_intent( | ||
| user=cast(AbstractUser, self.user1), | ||
| quantity=1, | ||
| ) | ||
| finally: | ||
| SspProductModel.objects.get = original_get |
Comment on lines
+196
to
+200
| extra_kwargs = { | ||
| 'ssp_product': { | ||
| 'required': False, | ||
| 'allow_null': False, | ||
| }, |
vshaikismail-sonata
force-pushed
the
shravani/ENT-11902
branch
from
f37ccf0 to
ac8c765
Compare
Comment on lines
+304
to
+307
| def save(self, *args, **kwargs): | ||
| """Ensure a valid ``ssp_product`` before persisting.""" | ||
| if not getattr(self, 'ssp_product', None): | ||
| SspProductModel = apps.get_model('customer_billing', 'SspProduct') |
Comment on lines
+704
to
+707
| else: | ||
| existing_intent = ( | ||
| cls.objects.filter(user=user).order_by('-created').first() | ||
| ) |
Comment on lines
+843
to
+847
| ).order_by('-stripe_event_created_at') | ||
| try: | ||
| return qs[0] | ||
| except IndexError: | ||
| return None |
Comment on lines
119
to
121
| # Verify it returns the correct count of updated intents | ||
| self.assertEqual(updated_count, 1, "Should have updated exactly one intent") | ||
|
|
Comment on lines
311
to
312
| self.assertEqual(context.checkout_intent, context.checkout_intent or {} | mock_intent_data) | ||
| mock_filter.assert_called_once_with(user=self.user) |
Comment on lines
+82
to
+91
| @staticmethod | ||
| def _create_teams_yearly_product(): | ||
| return SspProduct.objects.get_or_create( | ||
| slug='teams-yearly', | ||
| defaults={ | ||
| 'stripe_price_lookup_key': 'teams_yearly_price', | ||
| 'catalog_query_uuid': '00000000-0000-0000-0000-000000000000', | ||
| 'is_active': True, | ||
| }, | ||
| )[0] |
Comment on lines
193
to
+201
| class Meta: | ||
| model = CheckoutIntent | ||
| fields = '__all__' | ||
| extra_kwargs = { | ||
| 'ssp_product': { | ||
| 'required': False, | ||
| 'allow_null': False, | ||
| }, | ||
| } |
Comment on lines
+449
to
+464
| if not ssp_product: | ||
| ssp_product = SspProduct.objects.filter(slug='teams-yearly').first() | ||
| if not ssp_product: | ||
| error_code, developer_message = CHECKOUT_SESSION_ERROR_CODES['stripe_price_id']['DOES_NOT_EXIST'] | ||
| raise CreateCheckoutSessionValidationError( | ||
| validation_errors_by_field={ | ||
| 'stripe_price_id': { | ||
| 'error_code': error_code, | ||
| 'developer_message': ( | ||
| f'{developer_message} No SspProduct configured for ' | ||
| f'stripe_price_id={input_data.get("stripe_price_id")}. ' | ||
| 'Ensure stripe_price_lookup_key maps to an existing SspProduct.' | ||
| ), | ||
| } | ||
| } | ||
| ) |
vshaikismail-sonata
force-pushed
the
shravani/ENT-11902
branch
from
6e53da6 to
29a3c49
Compare
Comment on lines
116
to
120
| @@ -106,17 +119,67 @@ def test_cleanup_expired_without_mocks(self): | |||
| # Verify it returns the correct count of updated intents | |||
| self.assertEqual(updated_count, 1, "Should have updated exactly one intent") | |||
Comment on lines
+125
to
+139
| SspProductModel = apps.get_model('customer_billing', 'SspProduct') | ||
| original_get = SspProductModel.objects.get | ||
|
|
||
| def _raise(*args, **kwargs): | ||
| raise SspProductModel.MultipleObjectsReturned() | ||
|
|
||
| SspProductModel.objects.get = _raise | ||
| try: | ||
| with self.assertRaises(RuntimeError): | ||
| CheckoutIntent.create_intent( | ||
| user=cast(AbstractUser, self.user1), | ||
| quantity=1, | ||
| ) | ||
| finally: | ||
| SspProductModel.objects.get = original_get |
Comment on lines
+304
to
+315
| def save(self, *args, **kwargs): | ||
| """Ensure a valid ``ssp_product`` before persisting.""" | ||
| if not getattr(self, 'ssp_product', None): | ||
| SspProductModel = apps.get_model('customer_billing', 'SspProduct') | ||
| try: | ||
| self.ssp_product = SspProductModel.objects.get(slug='teams-yearly') | ||
| except SspProductModel.DoesNotExist as exc: | ||
| raise ValidationError({'ssp_product': 'Default ssp_product teams-yearly was not found.'}) from exc | ||
| except SspProductModel.MultipleObjectsReturned as exc: | ||
| raise ValidationError({'ssp_product': 'Multiple teams-yearly ssp_product records found.'}) from exc | ||
|
|
||
| return super().save(*args, **kwargs) |
Comment on lines
+694
to
+707
| else: | ||
| SspProductModel = apps.get_model('customer_billing', 'SspProduct') | ||
| teams_yearly = SspProductModel.objects.filter(slug='teams-yearly').first() | ||
| if teams_yearly: | ||
| existing_intent = ( | ||
| cls.objects.filter( | ||
| user=user, | ||
| ssp_product=teams_yearly, | ||
| ).order_by('-created').first() | ||
| ) | ||
| else: | ||
| existing_intent = ( | ||
| cls.objects.filter(user=user).order_by('-created').first() | ||
| ) |
Comment on lines
+196
to
+201
| extra_kwargs = { | ||
| 'ssp_product': { | ||
| 'required': False, | ||
| 'allow_null': False, | ||
| }, | ||
| } |
Comment on lines
+82
to
+91
| @staticmethod | ||
| def _create_teams_yearly_product(): | ||
| return SspProduct.objects.get_or_create( | ||
| slug='teams-yearly', | ||
| defaults={ | ||
| 'stripe_price_lookup_key': 'teams_yearly_price', | ||
| 'catalog_query_uuid': '00000000-0000-0000-0000-000000000000', | ||
| 'is_active': True, | ||
| }, | ||
| )[0] |
Comment on lines
+694
to
+707
| else: | ||
| SspProductModel = apps.get_model('customer_billing', 'SspProduct') | ||
| teams_yearly = SspProductModel.objects.filter(slug='teams-yearly').first() | ||
| if teams_yearly: | ||
| existing_intent = ( | ||
| cls.objects.filter( | ||
| user=user, | ||
| ssp_product=teams_yearly, | ||
| ).order_by('-created').first() | ||
| ) | ||
| else: | ||
| existing_intent = ( | ||
| cls.objects.filter(user=user).order_by('-created').first() | ||
| ) |
Comment on lines
116
to
120
| @@ -106,17 +119,67 @@ def test_cleanup_expired_without_mocks(self): | |||
| # Verify it returns the correct count of updated intents | |||
| self.assertEqual(updated_count, 1, "Should have updated exactly one intent") | |||
Comment on lines
+196
to
+201
| extra_kwargs = { | ||
| 'ssp_product': { | ||
| 'required': False, | ||
| 'allow_null': False, | ||
| }, | ||
| } |
shravani-sonata-gottapu
force-pushed
the
shravani/ENT-11902
branch
2 times, most recently
from
8f0837e to
a99d404
Compare
shravani-sonata-gottapu
force-pushed
the
shravani/ENT-11902
branch
from
a99d404 to
a115fac
Compare
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
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.
Ticket - ENT-11902 https://2u-internal.atlassian.net/browse/ENT-11902
Description :