Move calls to create_user to earlier in the ecommerce process (#3337)

Author: jkachel

b2b/api.py

@@ -50,7 +50,6 @@
 )
 from main import constants as main_constants
 from main.utils import date_to_datetime
-from openedx.api import create_user
 from openedx.tasks import clone_courserun
 
 log = logging.getLogger(__name__)
@@ -957,9 +956,9 @@ def _prepare_basket_for_b2b_enrollment(request, product: Product) -> Basket:
     """
     from ecommerce.api import establish_basket  # noqa: PLC0415
 
-    basket = establish_basket(request)
-    # Clear the basket. For Unified Ecommerce, we may want to change this.
-    # But MITx Online only allows one item per cart and not clearing it is confusing.
+    # Establish and clear the basket. If the user doesn't have an edX username,
+    # don't wait to create one.
+    basket = establish_basket(request, no_delay=True)
     basket.basket_items.all().delete()
     basket.discounts.all().delete()
 
@@ -1024,11 +1023,6 @@ def create_b2b_enrollment(request, product: Product):
     if validation_error:
         return validation_error
 
-    # Check for an edX user, and create one if there's not one
-    if not request.user.edx_username:
-        create_user(request.user)
-        request.user.refresh_from_db()
-
     # Prepare the basket for enrollment
     basket = _prepare_basket_for_b2b_enrollment(request, product)
 

b2b/views/v0/views_test.py

@@ -337,8 +337,9 @@ def test_b2b_contract_attachment_full_contract_with_used_code(mocker):
 def test_b2b_enroll(mocker, settings, user_has_edx_user, has_price, run_is_enrollable):
     """Make sure that hitting the enroll endpoint actually results in enrollments"""
 
+    mocked_create_from_id = mocker.patch("openedx.tasks.create_user_from_id")
     mocker.patch("openedx.tasks.clone_courserun.delay")
-    mocker.patch("openedx.api._create_edx_user_request")
+    mocked_create_user_request = mocker.patch("openedx.api._create_edx_user_request")
     settings.OPENEDX_SERVICE_WORKER_API_TOKEN = "a token"  # noqa: S105
 
     contract = ContractPageFactory.create(
@@ -354,15 +355,22 @@ def test_b2b_enroll(mocker, settings, user_has_edx_user, has_price, run_is_enrol
         courserun.live = False
         courserun.save()
 
-    user = UserFactory.create()
-    user.b2b_contracts.add(contract)
+    user = UserFactory.create(no_openedx_user=(not user_has_edx_user))
 
-    if not user_has_edx_user:
-        user.openedx_users.all().delete()
+    if user_has_edx_user:
+        assert user.openedx_user
+    else:
+        assert not user.openedx_user
 
+    user.b2b_contracts.add(contract)
     user.save()
     user.refresh_from_db()
 
+    if user_has_edx_user:
+        assert user.openedx_user
+    else:
+        assert not user.openedx_user
+
     client = APIClient()
     client.force_login(user)
 
@@ -384,5 +392,10 @@ def test_b2b_enroll(mocker, settings, user_has_edx_user, has_price, run_is_enrol
 
     user.refresh_from_db()
 
+    if not user_has_edx_user:
+        mocked_create_user_request.assert_called()
+        mocked_create_from_id.assert_not_called()
+
+    del user.openedx_user
     assert user.edx_username
     assert CourseRunEnrollment.objects.filter(user=user, run=courserun).exists()

ecommerce/api.py

@@ -71,7 +71,9 @@
 )
 from main.settings import ECOMMERCE_DEFAULT_PAYMENT_GATEWAY
 from main.utils import parse_supplied_date, redirect_with_user_message
+from openedx.api import create_user
 from openedx.constants import EDX_ENROLLMENT_AUDIT_MODE, EDX_ENROLLMENT_VERIFIED_MODE
+from openedx.tasks import create_user_from_id
 
 log = logging.getLogger(__name__)
 
@@ -445,11 +447,23 @@ def process_cybersource_payment_response(request, order):
     return return_message
 
 
-def establish_basket(request):
+def establish_basket(request, *, no_delay=False):
     """
-    Gets or creates the user's basket. (This may get some more logic later.)
+    Gets or creates the user's basket. Queue creation of their edX user if necessary.
+
+    Kwargs:
+        no_delay (bool): don't queue the edX user creation
     """
     user = request.user
+
+    if not user.edx_username:
+        if no_delay:
+            create_user(user)
+            # The openedx_user is a cached property, so delete the cached value
+            del user.openedx_user
+        else:
+            create_user_from_id.delay(user.id)
+
     (basket, is_new) = Basket.objects.get_or_create(user=user)
 
     if is_new:

ecommerce/api_test.py

@@ -29,6 +29,7 @@
     check_for_duplicate_discount_redemptions,
     create_verified_program_course_run_enrollment,
     create_verified_program_discount,
+    establish_basket,
     get_auto_apply_discounts_for_basket,
     process_cybersource_payment_response,
     refund_order,
@@ -69,6 +70,7 @@
 from flexiblepricing.constants import FlexiblePriceStatus
 from flexiblepricing.factories import FlexiblePriceFactory, FlexiblePriceTierFactory
 from openedx.constants import EDX_ENROLLMENT_VERIFIED_MODE
+from openedx.factories import OpenEdxUserFactory
 from users.factories import UserFactory
 
 pytestmark = [pytest.mark.django_db]
@@ -1139,3 +1141,46 @@ def test_get_auto_apply_discounts_respects_dates(user):
 
     discounts = get_auto_apply_discounts_for_basket(basket.id)
     assert discounts.count() == 0
+
+
+@pytest.mark.parametrize(
+    "no_delay",
+    [
+        True,
+        False,
+    ],
+)
+def test_establish_basket_calls_create_user(mocker, no_delay):
+    """Test that establish_basket calls create_user if there's no edX username."""
+
+    if no_delay:
+        expected_run_mock = mocker.patch("ecommerce.api.create_user")
+        expected_skip_mock = mocker.patch("openedx.tasks.create_user_from_id.delay")
+    else:
+        expected_run_mock = mocker.patch("openedx.tasks.create_user_from_id.delay")
+        expected_skip_mock = mocker.patch("ecommerce.api.create_user")
+
+    user = UserFactory.create(no_openedx_api_auth=True, no_openedx_user=True)
+    request = RequestFactory().get("/")
+    request.user = user
+
+    basket = establish_basket(request, no_delay=no_delay)
+
+    expected_run_mock.assert_called_once()
+    expected_run_mock.reset_mock()
+    expected_skip_mock.assert_not_called()
+
+    OpenEdxUserFactory.create(user=user)
+    if not no_delay:
+        del user.openedx_user
+
+    basket.delete()
+
+    user.refresh_from_db()
+    request.user = user
+
+    assert user.edx_username
+
+    establish_basket(request)
+
+    assert not expected_run_mock.called

ecommerce/hooks/process_transaction_line.py

@@ -5,7 +5,6 @@
 import pluggy
 
 from courses.models import CourseRun, PaidCourseRun, PaidProgram, Program
-from openedx.api import create_user
 from openedx.constants import EDX_ENROLLMENT_VERIFIED_MODE
 
 hookimpl = pluggy.HookimplMarker("mitxonline")
@@ -26,11 +25,6 @@ def _create_courserun_enrollment(line) -> str | None:
         log.debug("Item purchased %s is not a course run, skipping", purchased_run)
         return None
 
-    # Check for an edX user, and create one if there's not one
-    if not line.order.purchaser.edx_username:
-        create_user(line.order.purchaser)
-        line.order.purchaser.refresh_from_db()
-
     create_run_enrollments(
         line.order.purchaser,
         [purchased_run],

ecommerce/mail_api_test.py

@@ -1,6 +1,7 @@
 from decimal import Decimal
 
 import pytest
+import responses
 import reversion
 
 from ecommerce.factories import ProductFactory
@@ -38,6 +39,7 @@ def test_mail_api_task_called(  # noqa: PLR0913
 
 
 @pytest.mark.skip_nplusone_check
+@responses.activate
 def test_mail_api_receipt_generation(  # noqa: PLR0913
     settings, mocker, user, products, user_client, django_capture_on_commit_callbacks
 ):
@@ -48,6 +50,7 @@ def test_mail_api_receipt_generation(  # noqa: PLR0913
     """
     settings.OPENEDX_SERVICE_WORKER_API_TOKEN = "mock_api_token"  # noqa: S105
     mock_send_message = mocker.patch("mitol.mail.api.send_message")
+    responses.get("https://fonts.googleapis.com/css")
 
     with django_capture_on_commit_callbacks(execute=True):
         order = create_order_receipt(mocker, user, products, user_client)
@@ -64,6 +67,7 @@ def test_mail_api_receipt_generation(  # noqa: PLR0913
 
 
 @pytest.mark.skip_nplusone_check
+@responses.activate
 def test_mail_api_refund_email_generation(
     settings, mocker, user, products, user_client
 ):
@@ -73,6 +77,7 @@ def test_mail_api_refund_email_generation(
     """
     settings.OPENEDX_SERVICE_WORKER_API_TOKEN = "mock_api_token"  # noqa: S105
     order = create_order_receipt(mocker, user, products, user_client)
+    responses.get("https://fonts.googleapis.com/css")
 
     mock_send_message = mocker.patch("mitol.mail.api.send_message")
 

ecommerce/views/legacy/views_test.py

@@ -1180,7 +1180,7 @@ def test_start_checkout_and_ensure_edx_username_created(mocker, settings, produc
     Check that checking out with a user that doesn't have an edx username
     creates them an edx username
     """
-    mocked_create_user = mocker.patch("openedx.api.create_edx_user")
+    mocked_create_user = mocker.patch("openedx.tasks.create_user_from_id.delay")
     mocker.patch("openedx.api.create_edx_auth_token")
     user = UserFactory.create()
     user.openedx_users.all().delete()
@@ -1204,7 +1204,7 @@ def test_start_checkout_and_ensure_edx_username_created(mocker, settings, produc
             "run": order.lines.first().purchased_object.course.title,
         }
     )
-    mocked_create_user.assert_called_once()
+    mocked_create_user.assert_called()
 
 
 @pytest.mark.parametrize("use_redemption_type_flags", [True, False])