Ent 11903 bffcheckout price api#207
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #207 +/- ##
==========================================
+ Coverage 86.52% 87.00% +0.48%
==========================================
Files 154 154
Lines 12920 12975 +55
Branches 1233 1244 +11
==========================================
+ Hits 11179 11289 +110
+ Misses 1425 1377 -48
+ Partials 316 309 -7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
8a25aee to
1b9226b
Compare
rthota-sonata-hue
left a comment
There was a problem hiding this comment.
Looks good for merge. Approved.
| Creates a new CheckoutIntent. | ||
| """ | ||
| try: | ||
| ssp_product_slug = validated_data.pop('ssp_product', None) |
There was a problem hiding this comment.
since this is a ModelSerializer, validated_data['ssp_product'] should already be resolved to an SspProduct instance by DRF. so one line should be enough, here:
ssp_product = validated_data.pop('ssp_product', None)| 'stripe_price_id': 'price_abc123', | ||
| }, | ||
| format='json', | ||
| ) |
There was a problem hiding this comment.
Is there a test anywhere that sends an ssp_product key in the request payload?
There was a problem hiding this comment.
Updated test passing the payload
| settings_quantity_ranges = {} | ||
| for product_config in getattr(settings, 'SSP_PRODUCTS', {}).values(): | ||
| lk = product_config.get('lookup_key') | ||
| qr = product_config.get('quantity_range') | ||
| if lk and qr: | ||
| settings_quantity_ranges[lk] = qr | ||
| for ssp_product in SspProduct.objects.filter(is_active=True): | ||
| lookup_key = ssp_product.stripe_price_lookup_key |
There was a problem hiding this comment.
Ah, ok, this code represents something we're missing from SspProduct: the allowable quantity range as fields (min, max).
Currently, our stage/prod env configs don't include entries for each SspProduct, so all of the products except the base teams one will have no allowable quantity ranges available.
The cleanest path forward right now is to add a new setting like DEFAULT_SSP_QUANTITY_RANGE = [5, 50] (the same as the teams plan) and use that value directly here in this function for price_data['quantity_range']
| product, _ = SspProduct.objects.get_or_create( | ||
| slug=settings.SSP_DEFAULT_PRODUCT_SLUG, | ||
| defaults={ | ||
| 'stripe_price_lookup_key': 'teams_subscription_license_yearly', |
There was a problem hiding this comment.
This line hard-codes something that should be configuration, and also can create records with a totally random catalog query uuid. I would undo this change and remove the related migration.
| ssp_slug = None | ||
| try: | ||
| ssp_slug = product.metadata.get('ssp_product_slug') if getattr(product, 'metadata', None) else None | ||
| except Exception: # pylint: disable=broad-exception-caught | ||
| ssp_slug = None |
There was a problem hiding this comment.
this is overly defensive, we're already directly accessing product.metadata a few lines above. Replace this whole block with
ssp_slug = product.metadata.get('ssp_product_slug')There was a problem hiding this comment.
If I remove this defensive, I am getting output for price as empty object in /api/v1/bffs/checkout/context/.
| if ssp_slug: | ||
| base_data['ssp_product_slug'] = ssp_slug |
There was a problem hiding this comment.
why not always include the key and let the value be null? that is, why is the if ssp_slug check necessary here?
| try: | ||
| ssp = SspProduct.objects.filter(stripe_price_lookup_key=lookup_key).only('slug').first() | ||
| if ssp: | ||
| ssp_slug = ssp.slug | ||
| except Exception: # pylint: disable=broad-exception-caught | ||
| logger.exception('Error looking up SspProduct for lookup_key %s', lookup_key) |
There was a problem hiding this comment.
you don't need a try/except for this.
|
|
||
| user = input_data['user'] | ||
|
|
||
| # ── Resolve slug ↔ price_id BEFORE creating intent ── |
| user = input_data['user'] | ||
|
|
||
| # ── Resolve slug ↔ price_id BEFORE creating intent ── | ||
| ssp_pricing = get_ssp_product_pricing() |
There was a problem hiding this comment.
we should only do this if the caller hasn't provided an ssp_product_slug. or better yet, let's not do it at all and fetch either the requested SspProduct by slug, or the one specified by SSP_DEFAULT_PRODUCT_SLUG.
4e8510d to
2b34fb8
Compare
Jira:
https://2u-internal.atlassian.net/browse/ENT-11903
Description:
BFF context endpoint returns pricing with ssp_product_slug in every price object — the BFF context route returns pricing data organized by Stripe lookup_key. After this step, every price object returned by the BFF — including all academies and Teams products — will include the ssp_product_slug field. This gives the frontend a unified, product-centric identifier across the entire pricing response, not just a subset.
Checkout intent creation accepts ssp_product_slug — Currently, the BFF route for creating a checkout intent receives a stripe_price_id directly from the client. After this change, it will accept an ssp_product_slug from the client, look up the corresponding SspProduct record, retrieve the stripe_price_lookup_key