Skip to content

Commit 8774d9b

Browse files
committed
feat: change create_intent() to allow blank slug/name
This change is needed to support the use case of creating an intent WITHOUT an enterprise slug/name, such as during submission of the PlanDetails page (first page) of the checkout stepper. ENT-10756
1 parent e87874e commit 8774d9b

File tree

7 files changed

+199
-60
lines changed

7 files changed

+199
-60
lines changed

enterprise_access/apps/api/serializers/customer_billing.py

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,26 @@
33
"""
44
from django_countries.serializers import CountryFieldMixin
55
from rest_framework import serializers
6+
from rest_framework.exceptions import APIException
67

78
from enterprise_access.apps.customer_billing.constants import ALLOWED_CHECKOUT_INTENT_STATE_TRANSITIONS
8-
from enterprise_access.apps.customer_billing.models import CheckoutIntent
9+
from enterprise_access.apps.customer_billing.models import (
10+
CheckoutIntent,
11+
SlugReservationConflict,
12+
TerminalCheckoutIntentConflict
13+
)
14+
15+
16+
class RecordConflictError(APIException):
17+
"""
18+
Raise this to trigger HTTP 422, as an alternative to ValidationError (HTTP 400).
19+
20+
422 more accurately describes a conflicting database record, whereas 400 means there's an issue
21+
with the query structure or values.
22+
"""
23+
status_code = 422
24+
default_detail = 'Encountered a conflicting record.'
25+
default_code = 'record_conflict_error'
926

1027

1128
# pylint: disable=abstract-method
@@ -176,15 +193,33 @@ def validate_terms_metadata(self, value):
176193
)
177194
return value
178195

196+
def validate(self, attrs):
197+
"""
198+
Perform any cross-field validation.
199+
"""
200+
if attrs.get('enterprise_slug') and not attrs.get('enterprise_name'):
201+
raise serializers.ValidationError(
202+
{'enterprise_name': 'enterprise_name is required when enterprise_slug is provided.'}
203+
)
204+
if attrs.get('enterprise_name') and not attrs.get('enterprise_slug'):
205+
raise serializers.ValidationError(
206+
{'enterprise_slug': 'enterprise_slug is required when enterprise_name is provided.'}
207+
)
208+
return attrs
209+
179210
def create(self, validated_data):
180211
"""
181212
Creates a new CheckoutIntent.
182213
"""
183-
return CheckoutIntent.create_intent(
184-
user=self.context['request'].user,
185-
slug=validated_data['enterprise_slug'],
186-
name=validated_data['enterprise_name'],
187-
quantity=validated_data['quantity'],
188-
country=validated_data.get('country'),
189-
terms_metadata=validated_data.get('terms_metadata'),
190-
)
214+
try:
215+
return CheckoutIntent.create_intent(
216+
user=self.context['request'].user,
217+
slug=validated_data['enterprise_slug'],
218+
name=validated_data['enterprise_name'],
219+
quantity=validated_data['quantity'],
220+
country=validated_data.get('country'),
221+
terms_metadata=validated_data.get('terms_metadata'),
222+
)
223+
except (SlugReservationConflict, TerminalCheckoutIntentConflict) as exc:
224+
# RecordConflictError is configured to cause a 422 response.
225+
raise RecordConflictError() from exc

enterprise_access/apps/api/v1/tests/test_checkout_intent_views.py

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -362,43 +362,37 @@ def test_create_or_update_checkout_intent_success(self):
362362
self.assertEqual(self.checkout_intent_1.terms_metadata, {'version': '2.0', 'updated': True})
363363

364364
@ddt.data(
365-
{'quantity': -1},
366-
{'quantity': 0},
367-
{'quantity': 'invalid'},
368-
{'enterprise_slug': ''},
369-
{'enterprise_name': ''},
365+
# Invalid quantity cases:
366+
{'quantity': -1, 'enterprise_slug': 'valid', 'enterprise_name': 'Valid'},
367+
{'quantity': 0, 'enterprise_slug': 'valid', 'enterprise_name': 'Valid'},
368+
{'quantity': 'invalid', 'enterprise_slug': 'valid', 'enterprise_name': 'Valid'},
369+
# Missing slug/name when the other is provided.
370+
{'quantity': 10, 'enterprise_slug': '', 'enterprise_name': 'Valid'},
371+
{'quantity': 10, 'enterprise_name': 'Valid'},
372+
{'quantity': 10, 'enterprise_slug': 'valid', 'enterprise_name': ''},
373+
{'quantity': 10, 'enterprise_slug': 'valid'},
370374
)
371375
@ddt.unpack
372-
def test_create_checkout_intent_invalid_field_values(self, **invalid_field):
376+
def test_create_checkout_intent_invalid_field_values(self, **invalid_payload):
373377
"""Test creation fails with invalid field values."""
374378
other_user = UserFactory()
375379
self.set_jwt_cookie([{
376380
'system_wide_role': SYSTEM_ENTERPRISE_LEARNER_ROLE,
377381
'context': str(uuid.uuid4()),
378382
}], user=other_user)
379383

380-
request_data = {
381-
'enterprise_slug': 'test-enterprise',
382-
'enterprise_name': 'Test Enterprise',
383-
'quantity': 10,
384-
}
385-
request_data.update(invalid_field)
386-
387384
response = self.client.post(
388385
self.list_url,
389-
request_data,
386+
invalid_payload,
390387
format='json'
391388
)
392389

393390
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
394-
# The field name should be in the error response
395-
invalid_field_name = list(invalid_field.keys())[0]
396-
self.assertIn(invalid_field_name, response.data)
397391

398392
@ddt.data(
399-
{'enterprise_name': 'hello', 'quantity': 10},
400-
{'enterprise_slug': 'hello', 'quantity': 10},
401-
{'enterprise_name': 'hello', 'enterprise_slug': 'foo'},
393+
{}, # Missing quantity.
394+
{'enterprise_slug': 'hello', 'quantity': 10}, # Missing enterprise_name.
395+
{'enterprise_name': 'Hello', 'quantity': 10}, # Missing enterprise_slug.
402396
)
403397
@ddt.unpack
404398
def test_create_checkout_intent_missing_required_fields(self, **payload):

enterprise_access/apps/customer_billing/api.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,18 @@
88

99
import stripe
1010
from django.contrib.auth import get_user_model
11+
from django.contrib.auth.models import AbstractUser
1112
from django.core.exceptions import ValidationError
1213
from django.core.validators import validate_email, validate_slug
1314
from requests.exceptions import HTTPError
1415

1516
from enterprise_access.apps.api_client.lms_client import LmsApiClient
1617
from enterprise_access.apps.customer_billing.constants import CHECKOUT_SESSION_ERROR_CODES
17-
from enterprise_access.apps.customer_billing.models import CheckoutIntent
18+
from enterprise_access.apps.customer_billing.models import (
19+
CheckoutIntent,
20+
SlugReservationConflict,
21+
TerminalCheckoutIntentConflict
22+
)
1823
from enterprise_access.apps.customer_billing.pricing_api import get_ssp_product_pricing
1924
from enterprise_access.apps.customer_billing.stripe_api import create_subscription_checkout_session
2025

@@ -26,7 +31,7 @@ class CheckoutSessionInputValidatorData(TypedDict, total=False):
2631
"""
2732
`total=False` so that validation does not require all fields to be provided.
2833
"""
29-
user: User | None
34+
user: AbstractUser | None
3035
admin_email: str
3136
full_name: str
3237
enterprise_slug: str
@@ -38,7 +43,7 @@ class CheckoutSessionInputData(TypedDict, total=True):
3843
"""
3944
Input parameters for checkout session creation.
4045
"""
41-
user: User | None
46+
user: AbstractUser | None
4247
admin_email: str
4348
enterprise_slug: str
4449
company_name: str
@@ -402,11 +407,11 @@ def create_free_trial_checkout_session(
402407
try:
403408
intent = CheckoutIntent.create_intent(
404409
user=user,
410+
quantity=input_data.get('quantity'),
405411
slug=input_data.get('enterprise_slug'),
406412
name=input_data.get('company_name'),
407-
quantity=input_data.get('quantity'),
408413
)
409-
except ValidationError as exc:
414+
except (SlugReservationConflict, TerminalCheckoutIntentConflict) as exc:
410415
raise CreateCheckoutSessionValidationError(
411416
validation_errors_by_field={
412417
'enterprise_slug': {
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Generated by Django 4.2.24 on 2025-10-02 00:58
2+
3+
import django.core.validators
4+
from django.db import migrations, models
5+
import re
6+
7+
8+
class Migration(migrations.Migration):
9+
10+
dependencies = [
11+
('customer_billing', '0006_customer_uuid_stripe_id'),
12+
]
13+
14+
operations = [
15+
migrations.AlterField(
16+
model_name='checkoutintent',
17+
name='enterprise_name',
18+
field=models.CharField(blank=True, help_text='Checkout intent enterprise customer name', max_length=255, null=True),
19+
),
20+
migrations.AlterField(
21+
model_name='checkoutintent',
22+
name='enterprise_slug',
23+
field=models.SlugField(blank=True, help_text='Checkout intent enterprise customer slug', max_length=255, null=True, validators=[django.core.validators.RegexValidator(re.compile('^[-a-zA-Z0-9_]+\\Z'), 'Enter a valid “slug” consisting of letters, numbers, underscores or hyphens.', 'invalid')]),
24+
),
25+
migrations.AlterField(
26+
model_name='historicalcheckoutintent',
27+
name='enterprise_name',
28+
field=models.CharField(blank=True, help_text='Checkout intent enterprise customer name', max_length=255, null=True),
29+
),
30+
migrations.AlterField(
31+
model_name='historicalcheckoutintent',
32+
name='enterprise_slug',
33+
field=models.SlugField(blank=True, help_text='Checkout intent enterprise customer slug', max_length=255, null=True, validators=[django.core.validators.RegexValidator(re.compile('^[-a-zA-Z0-9_]+\\Z'), 'Enter a valid “slug” consisting of letters, numbers, underscores or hyphens.', 'invalid')]),
34+
),
35+
]

enterprise_access/apps/customer_billing/models.py

Lines changed: 72 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@
2525
User = get_user_model()
2626

2727

28+
class TerminalCheckoutIntentConflict(Exception):
29+
pass
30+
31+
32+
class SlugReservationConflict(Exception):
33+
pass
34+
35+
2836
class CheckoutIntent(TimeStampedModel):
2937
"""
3038
Tracks the complete lifecycle of a self-service checkout process:
@@ -101,10 +109,14 @@ class StateChoices(models.TextChoices):
101109
max_length=255,
102110
)
103111
enterprise_name = models.CharField(
112+
null=True,
113+
blank=True,
104114
max_length=255,
105115
help_text="Checkout intent enterprise customer name",
106116
)
107117
enterprise_slug = models.SlugField(
118+
null=True,
119+
blank=True,
108120
max_length=255,
109121
validators=[validate_slug],
110122
help_text="Checkout intent enterprise customer slug"
@@ -386,9 +398,9 @@ def can_reserve(cls, slug=None, name=None, exclude_user=None):
386398
def create_intent(
387399
cls,
388400
user: AbstractUser,
389-
slug: str,
390-
name: str,
391401
quantity: int,
402+
slug: str | None = None,
403+
name: str | None = None,
392404
country: str | None = None,
393405
terms_metadata: dict | None = None
394406
) -> Self:
@@ -399,22 +411,23 @@ def create_intent(
399411
not already in use by another checkout flow. If the user already has an intent:
400412
- If it's in a success state (PAID, FULFILLED), the existing intent is returned unchanged
401413
- If it's in a failure state (ERRORED_*), a ValueError is raised
402-
- If it's in CREATED state, it's updated with the new details
414+
- If it's in CREATED state, it's updated with the new details (more like PATCH, not PUT).
403415
404416
The method also cleans up any expired intents first to free up reserved slugs/names.
405417
406418
Args:
407419
user (User): The Django User who will own this intent
408-
slug (str): The enterprise slug to reserve
409-
name (str): The enterprise name to reserve
410420
quantity (int): Number of licenses to create
421+
slug (str, Optional): The enterprise slug to reserve
422+
name (str, Optional): The enterprise name to reserve
411423
412424
Returns:
413425
CheckoutIntent: The created or updated intent object
414426
415427
Raises:
416-
ValueError: If the slug or name is already reserved by another user
417-
ValueError: If the user already has an intent that failed
428+
ValueError: If only one of [slug, name] were given, but not the other.
429+
SlugReservationConflict: If the slug or name is already reserved by another user
430+
TerminalCheckoutIntentConflict: If the user already has an intent that failed
418431
419432
Note:
420433
This operation is atomic - either the entire reservation succeeds or fails.
@@ -425,28 +438,70 @@ def create_intent(
425438
with transaction.atomic():
426439
cls.cleanup_expired()
427440

428-
if not cls.can_reserve(slug, name, exclude_user=user):
429-
raise ValueError(f"Slug '{slug}' or name '{name}' is already reserved")
441+
if bool(slug) != bool(name):
442+
raise ValueError("slug and name must either both be given or neither be given.")
430443

431444
existing_intent = cls.objects.filter(user=user).first()
432445

433-
expires_at = timezone.now() + timedelta(minutes=cls.get_reservation_duration())
434-
446+
# If an existing intent has already reached a terminal state, exit fast.
435447
if existing_intent:
436448
if existing_intent.state in cls.SUCCESS_STATES:
437449
return existing_intent
438450

439451
if existing_intent.state in cls.FAILURE_STATES:
440-
raise ValueError("Failed checkout record already exists")
452+
raise TerminalCheckoutIntentConflict("Failed checkout record already exists")
453+
454+
# Establish whether or not a new slug needs to be reserved. This logic is really only an
455+
# optimization to avoid unnecessary DB lookups to search for reservation conflicts (via
456+
# can_reserve()) in cases where we know the reservation is not changing.
457+
#
458+
# | Slug | Intent | A Slug | Requested Slug | Requested Slug
459+
# Case | Requested? | Existed? | Already Reserved? | Is Different? | Needs To Be Reserved
460+
# ------+------------+----------+-------------------+----------------+---------------------
461+
# 1 | no | no | N/A | N/A | no
462+
# 2 | no | yes | no | N/A | no
463+
# 3 | no | yes | yes | N/A | no
464+
# 4 | yes | no | N/A | N/A | yes
465+
# 5 | yes | yes | no | N/A | yes
466+
# 6 | yes | yes | yes | no | no
467+
# 7 | yes | yes | yes | yes | yes
468+
if slug:
469+
if existing_intent:
470+
slug_changing = slug != existing_intent.enterprise_slug
471+
name_changing = name != existing_intent.enterprise_name
472+
should_reserve_new_slug = slug_changing or name_changing # Cases 5, 6, 7
473+
else:
474+
should_reserve_new_slug = True # Case 4
475+
else:
476+
should_reserve_new_slug = False # Cases 1, 2, 3
477+
478+
# If we are reserving a new slug, then gate this whole view on it not already being reserved.
479+
if should_reserve_new_slug:
480+
if not cls.can_reserve(slug, name, exclude_user=user):
481+
raise SlugReservationConflict(f"Slug '{slug}' or name '{name}' is already reserved")
482+
483+
expires_at = timezone.now() + timedelta(minutes=cls.get_reservation_duration())
484+
485+
# The remaining code essentially performs an update_or_create(), but we're not
486+
# using update_or_create() because we already have the existing intent and
487+
# don't need to spend a DB query looking it up again. Also, wouldn't be able
488+
# to do the terms_metadata merging logic which could come in handy in case
489+
# this ever gets called multiple times and we have terms on multiple pages.
441490

442-
# Update the existing CREATED or EXPIRED intent
491+
if existing_intent:
492+
# Found an existing CREATED or EXPIRED intent, so update it.
493+
494+
# Force update certain fields.
443495
existing_intent.state = CheckoutIntentState.CREATED
444-
existing_intent.enterprise_slug = slug
445-
existing_intent.enterprise_name = name
446496
existing_intent.quantity = quantity
447497
existing_intent.expires_at = expires_at
448-
existing_intent.country = country
449-
existing_intent.terms_metadata = terms_metadata
498+
499+
# Any of the following could be None since they're optional, so lets only update them if supplied.
500+
existing_intent.enterprise_slug = slug or existing_intent.enterprise_slug
501+
existing_intent.enterprise_name = name or existing_intent.enterprise_name
502+
existing_intent.country = country or existing_intent.country
503+
existing_intent.terms_metadata = (existing_intent.terms_metadata or {}) | (terms_metadata or {})
504+
450505
existing_intent.save()
451506
return existing_intent
452507

0 commit comments

Comments
 (0)