feat: ENT-11468 PR-A Updated Models layer (data + migration)#174
feat: ENT-11468 PR-A Updated Models layer (data + migration)#174shravani-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 model layer and migrations to support Academy-based checkout intents alongside existing Teams flows, including a renamed Academy catalog identifier and new CheckoutIntent retrieval/uniqueness behavior.
Changes:
- Renamed
EnterpriseAcademy.enterprise_catalog_uuidtocatalog_query_idvia data-preserving migrations and updated tests. - Updated
CheckoutIntentto support multiple intents per user by switchinguserto aForeignKey, addingstripe_product_id, and introducing partial unique constraints. - Adjusted checkout BFF tests to use
CheckoutIntent.for_userand added model tests for the new constraints and retrieval logic.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| enterprise_access/apps/customer_billing/tests/test_models.py | Updates Academy field rename assertions; adds tests for new CheckoutIntent constraints and for_user filtering behavior. |
| enterprise_access/apps/customer_billing/models.py | Renames Academy model field; adds stripe_product_id, changes intent ownership cardinality, introduces partial unique constraints, and updates for_user query logic. |
| enterprise_access/apps/customer_billing/migrations/0032_remove_enterpriseacademy_enterprise_catalog_uuid_and_more.py | Renames Academy field in both main and historical models; adds CheckoutIntent fields and DB constraints. |
| enterprise_access/apps/bffs/tests/test_checkout_handlers.py | Updates handler tests to mock CheckoutIntent.for_user instead of queryset filtering. |
| enterprise_access/apps/api/v1/tests/test_checkout_bff_views.py | Updates BFF context endpoint test to patch CheckoutIntent.for_user. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #174 +/- ##
==========================================
+ Coverage 86.08% 86.14% +0.05%
==========================================
Files 149 151 +2
Lines 12500 12557 +57
Branches 1194 1199 +5
==========================================
+ Hits 10761 10817 +56
- Misses 1424 1425 +1
Partials 315 315 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
vshaikismail-sonata
left a comment
There was a problem hiding this comment.
LGTM.....!!!!
|
Neither the PR title nor the commit message is descriptive enough. Please see https://open-edx-proposals.readthedocs.io/en/latest/best-practices/oep-0051-bp-conventional-commits.html
|
70d55fe to
4482d43
Compare
| models.UniqueConstraint( | ||
| fields=["user", "enterprise_slug", "stripe_product_id"], | ||
| condition=models.Q( | ||
| stripe_product_id__isnull=False, | ||
| state__in=["created", "paid"], | ||
| ), | ||
| name="unique_active_checkout_intent_with_product", | ||
| ), | ||
| models.UniqueConstraint( | ||
| fields=["user", "enterprise_slug"], | ||
| condition=models.Q( | ||
| stripe_product_id__isnull=True, | ||
| state__in=["created", "paid"], | ||
| ), | ||
| name="unique_active_checkout_intent_without_product", |
There was a problem hiding this comment.
❌ Limiting uniqueness constraints to just created and paid states would result in a change in behavior for Teams. Right now we do not want to allow a user to create multiple trials. However, as written these constraints would allow trial A to be fulfilled while trial B can progress from created -> paid -> fulfilled without encountering any database errors.
❌ enterprise_slug cannot be included in the constraints because it would result in a single user being able to provision multiple trials (using different slugs).
| stripe_product_id = models.CharField( | ||
| max_length=255, | ||
| null=False, | ||
| blank=True, |
| user = models.ForeignKey( | ||
| User, | ||
| on_delete=models.CASCADE, | ||
| ) |
| # Alter type from UUIDField -> IntegerField | ||
| migrations.AlterField( | ||
| model_name="enterpriseacademy", | ||
| name="catalog_query_id", | ||
| field=models.IntegerField( | ||
| blank=True, | ||
| db_index=True, | ||
| help_text="Catalog query integer ID for this Academy.", | ||
| null=True, | ||
| ), | ||
| ), | ||
| migrations.AlterField( | ||
| model_name="historicalenterpriseacademy", | ||
| name="catalog_query_id", | ||
| field=models.IntegerField( | ||
| blank=True, | ||
| db_index=True, | ||
| help_text="Catalog query integer ID for this Academy.", |
| def test_unique_active_for_blank_stripe_product_id(self): | ||
| """Two active intents with the same user+enterprise_slug and blank stripe_product_id should conflict.""" | ||
| CheckoutIntent.objects.create( | ||
| user=self.user, | ||
| enterprise_slug='test-enterprise', |
| stripe_product_id = models.CharField( | ||
| max_length=255, | ||
| null=True, | ||
| blank=True, | ||
| db_index=True, | ||
| help_text="Stripe Product ID for paid/academy intents. NULL for Teams intents.", | ||
| ) |
There was a problem hiding this comment.
❌ Please keep stripe decoupled from the CheckoutIntent domain. An alternative option would be to associate the CheckoutIntent with an SSP product:
| stripe_product_id = models.CharField( | |
| max_length=255, | |
| null=True, | |
| blank=True, | |
| db_index=True, | |
| help_text="Stripe Product ID for paid/academy intents. NULL for Teams intents.", | |
| ) | |
| ssp_product_selection = models.ForeignKey( | |
| SspProduct, | |
| help_text="The selected Self-Service Purchasing product.", | |
| ) |
Then you could rename EnterpriseAcademy -> SspProduct so that it covers both essentials AND teams, and also centralizes the place to map the product selection to the license-manager product ID used for provisioning:
# Delete EnterpriseAcademy model, replace with:
class SspProduct(TimeStampedModel):
slug = models.SlugField(
unique=True,
null=True,
help_text='Use this as the unique, cross-service identifier for the product.',
)
academy_uuid = models.UUIDField(
null=True,
help_text="UUID synced from the Academy table in enterprise-catalog IFF this is an Essentials product.",
)
academy_title = models.CharField(
null=True,
blank=True,
help_text='Title synced from the Academy table in enterprise-catalog IFF this is an Essentials product.',
)
stripe_lookup_key = models.CharField(
null=True,
blank=False,
help_text="Stripe Price lookup_key, to help quickly determine the price for this product.",
)
trial_subscription_product_id = models.IntegerField(
null=True,
help_text="License-manager product ID representing the trial period.",
)
paid_subscription_product_id = models.IntegerField(
null=True,
help_text="License-manager product ID representing a paid period.",
)The sync logic in #171 could be updated to just upsert academy_uuid and academy_title, leaving all the other field untouched so that an operator can manually configure them.
There was a problem hiding this comment.
@pwnage101 Is this the only comment on this ticket? Or will there be other items? I am hoping that we can capture all current feedback in one review. Just double checking to make sure there are not additional items.
There was a problem hiding this comment.
Please refer to https://2u-internal.atlassian.net/wiki/spaces/SOL/pages/3740565518/Troy+s+ADR+for+SspProduct for a more complete proposal.

Description :

In this PR we included the database and model layer changes required to support Academy-based checkout intents alongside existing Teams checkout flows.
It updates the
CheckoutIntentmodel to allow multiple intents per user, addsstripe_product_idfor product-specific intent handling, and implements partial unique constraints to prevent duplicate active intents.The PR also renames
enterprise_catalog_uuidtocatalog_query_idinEnterpriseAcademywhile preserving existing data through migrations.Additionally, it improves intent retrieval logic with deterministic filtering and adds new Stripe metadata constants for cross-service checkout tracking.