add proper response codes to attach endpoint (#3084)

Author: gumaerc

b2b/views/v0/__init__.py

@@ -117,6 +117,9 @@ def post(self, request, enrollment_code: str, format=None):  # noqa: A002, ARG00
         If the user is already in the contract, then we skip it.
 
         Returns:
+        - 201: Code successfully redeemed and user attached to new contract(s)
+        - 200: Code valid but user already attached to all associated contracts
+        - 404: Invalid or expired enrollment code
         - list of ContractPageSerializer - the contracts for the user
         """
 
@@ -141,9 +144,8 @@ def get_active_user_contracts(user):
             )
         except Discount.DoesNotExist:
             return Response(
-                ContractPageSerializer(
-                    get_active_user_contracts(request.user), many=True
-                ).data
+                {"detail": "Invalid or expired enrollment code."},
+                status=status.HTTP_404_NOT_FOUND,
             )
 
         contract_ids = list(code.b2b_contracts().values_list("id", flat=True))
@@ -154,6 +156,7 @@ def get_active_user_contracts(user):
             .all()
         )
 
+        contracts_attached = False
         for contract in contracts:
             if contract.is_full():
                 continue
@@ -165,11 +168,17 @@ def get_active_user_contracts(user):
             DiscountContractAttachmentRedemption.objects.create(
                 user=request.user, discount=code, contract=contract
             )
+            contracts_attached = True
 
         request.user.save()
 
+        response_status = (
+            status.HTTP_201_CREATED if contracts_attached else status.HTTP_200_OK
+        )
+
         return Response(
             ContractPageSerializer(
                 get_active_user_contracts(request.user), many=True
-            ).data
+            ).data,
+            status=response_status,
         )

b2b/views/v0/views_test.py

@@ -35,7 +35,8 @@ def test_b2b_contract_attachment_bad_code(user):
     url = reverse("b2b:attach-user", kwargs={"enrollment_code": "not a code"})
     resp = client.post(url)
 
-    assert resp.status_code == 200
+    assert resp.status_code == 404
+    assert resp.json()["detail"] == "Invalid or expired enrollment code."
     assert user.b2b_contracts.count() == 0
 
 
@@ -90,18 +91,21 @@ def test_b2b_contract_attachment(mocker, max_learners, code_used):
     )
     resp = client.post(url)
 
-    assert resp.status_code == 200
-
     user.refresh_from_db()
 
     if code_used and max_learners:
+        # Code already used for attachment and is not unlimited - should return 404
+        assert resp.status_code == 404
+        assert resp.json()["detail"] == "Invalid or expired enrollment code."
         assert not user.b2b_organizations.filter(pk=contract.organization.id).exists()
         assert not user.b2b_contracts.filter(pk=contract.id).exists()
 
         assert not DiscountContractAttachmentRedemption.objects.filter(
             contract=contract, user=user, discount=contract_codes[0]
         ).exists()
     else:
+        # Successfully attached to contract - should return 201
+        assert resp.status_code == 201
         mocked_attach_user.assert_called()
 
         assert user.b2b_organizations.filter(pk=contract.organization.id).exists()
@@ -157,7 +161,9 @@ def test_b2b_contract_attachment_invalid_code_dates(user, bad_start_or_end):
     with freeze_time(slightly_future_time):
         resp = client.post(url)
 
-    assert resp.status_code == 200
+    # Code is expired/not yet active - should return 404
+    assert resp.status_code == 404
+    assert resp.json()["detail"] == "Invalid or expired enrollment code."
 
     user.refresh_from_db()
     assert not user.b2b_contracts.filter(pk=contract.id).exists()
@@ -211,6 +217,7 @@ def test_b2b_contract_attachment_invalid_contract_dates(user, bad_start_or_end):
     with freeze_time(slightly_future_time):
         resp = client.post(url)
 
+    # Contract dates are invalid - code is valid but no contracts to attach - should return 200
     assert resp.status_code == 200
 
     user.refresh_from_db()
@@ -248,7 +255,8 @@ def test_b2b_contract_attachment_full_contract(mocker):
     )
     resp = client.post(url)
 
-    assert resp.status_code == 200
+    # Successfully attached - should return 201
+    assert resp.status_code == 201
     mocked_attach_user.assert_called()
 
     user.refresh_from_db()
@@ -265,7 +273,9 @@ def test_b2b_contract_attachment_full_contract(mocker):
     )
     resp = client.post(url)
 
-    assert resp.status_code == 200
+    # Code already used for attachment and contract has max_learners=1 - should return 404
+    assert resp.status_code == 404
+    assert resp.json()["detail"] == "Invalid or expired enrollment code."
     mocked_attach_user.assert_not_called()
 
     user.refresh_from_db()

courses/conftest.py

@@ -60,7 +60,11 @@ def course_catalog_data(course_catalog_program_count, course_catalog_course_coun
         course_catalog_course_count(int): number of courses to generate.
         course_catalog_program_count(int): number of programs to generate.
     """
-    # Use a local Random instance with fixed seed for deterministic test data
+    import random
+
+    # Seed both local and global random for full determinism
+    # Factory Boy's FuzzyText uses the global random module
+    random.seed(42)
     rng = Random(42)  # noqa: S311
     programs = []
     courses = []
@@ -99,13 +103,22 @@ def _create_program(courses, rng):
         elective_flag=True,
     )
     if len(courses) > 3:  # noqa: PLR2004
-        for c in rng.sample(courses, 3):
+        # Use deterministic indices instead of random sampling
+        # This ensures the same courses are selected regardless of test execution order
+        indices = list(range(len(courses)))
+        rng.shuffle(indices)
+
+        # Select 3 courses for required
+        for idx in indices[:3]:
             required_courses_node.add_child(
-                node_type=ProgramRequirementNodeType.COURSE, course=c
+                node_type=ProgramRequirementNodeType.COURSE, course=courses[idx]
             )
-        for c in rng.sample(courses, 3):
+
+        # Select 3 courses for electives (reuse the shuffled indices)
+        elective_indices = indices[3:6] if len(indices) >= 6 else indices[:3]  # noqa: PLR2004
+        for idx in elective_indices:
             elective_courses_node.add_child(
-                node_type=ProgramRequirementNodeType.COURSE, course=c
+                node_type=ProgramRequirementNodeType.COURSE, course=courses[idx]
             )
     else:
         required_courses_node.add_child(

openapi/specs/v0.yaml

@@ -117,6 +117,9 @@ paths:
         If the user is already in the contract, then we skip it.
 
         Returns:
+        - 201: Code successfully redeemed and user attached to new contract(s)
+        - 200: Code valid but user already attached to all associated contracts
+        - 404: Invalid or expired enrollment code
         - list of ContractPageSerializer - the contracts for the user
       parameters:
       - in: path

openapi/specs/v1.yaml

@@ -117,6 +117,9 @@ paths:
         If the user is already in the contract, then we skip it.
 
         Returns:
+        - 201: Code successfully redeemed and user attached to new contract(s)
+        - 200: Code valid but user already attached to all associated contracts
+        - 404: Invalid or expired enrollment code
         - list of ContractPageSerializer - the contracts for the user
       parameters:
       - in: path

openapi/specs/v2.yaml

@@ -117,6 +117,9 @@ paths:
         If the user is already in the contract, then we skip it.
 
         Returns:
+        - 201: Code successfully redeemed and user attached to new contract(s)
+        - 200: Code valid but user already attached to all associated contracts
+        - 404: Invalid or expired enrollment code
         - list of ContractPageSerializer - the contracts for the user
       parameters:
       - in: path