diff --git a/compliance/exceptions.py b/compliance/exceptions.py index 2d59fb8cf1..e9b63d6b0b 100644 --- a/compliance/exceptions.py +++ b/compliance/exceptions.py @@ -12,6 +12,12 @@ def to_error_detail(self) -> dict: class ExportComplianceError(ExportComplianceCheckError): """A user failed a CyberSource export compliance check""" + #: Opaque support-facing code appended to the learner-visible enrollment + #: error (see ``courses.exceptions.EnrollmentError.from_cause``). Lets a + #: learner quote something actionable without us disclosing the CyberSource + #: decision or reason code - those stay in the logs. + error_code = "CS_700" + def __init__(self, user, decision, reason_code, msg=None): """ Sets exception properties and adds a default message diff --git a/courses/api.py b/courses/api.py index 2e774027e4..d595dc4092 100644 --- a/courses/api.py +++ b/courses/api.py @@ -415,8 +415,9 @@ def reconcile_verified_program_enrollments( raise EnrollmentError except ExportComplianceCheckError as exc: # Don't propagate the specifics of the compliance decision to the - # client - the underlying cause is logged where it's raised. - raise EnrollmentError from exc + # client - only an opaque support code, if the cause declares one. + # The underlying cause is logged where it's raised. + raise EnrollmentError.from_cause(exc) from exc def upgrade_audit_run_enrollments_for_program_purchase(user, program): diff --git a/courses/exceptions.py b/courses/exceptions.py index 99515b4c9e..89d147acac 100644 --- a/courses/exceptions.py +++ b/courses/exceptions.py @@ -8,17 +8,40 @@ class EnrollmentError(APIException): """ Raised when an enrollment request cannot be completed. - Deliberately excludes any specifics about *why* the enrollment failed - (e.g. an export compliance decision) - historically we've just told - learners to contact support in these cases rather than surfacing that - detail to the client. The underlying cause is logged server-side by the - code that raises it (e.g. ``courses.api``). + Deliberately excludes any specifics about *why* the enrollment failed (e.g. + which export compliance decision came back) rather than surfacing that + detail to the client. The one thing that varies is an opaque support code + appended by ``from_cause`` for causes that declare one. The underlying cause + is logged server-side by the code that raises it (e.g. ``courses.api``). + + Says nothing about contacting support either - clients already pair an + enrollment failure with their own support copy, so a CTA here doubles up. """ status_code = status.HTTP_400_BAD_REQUEST - default_detail = "Unable to complete enrollment. Please contact support." + default_detail = "Unable to complete enrollment." default_code = "unable_to_complete_enrollment" + @classmethod + def from_cause(cls, exc: Exception) -> "EnrollmentError": + """ + Build an error for `exc`, appending the cause's support code when it + declares one (see ``compliance.exceptions.ExportComplianceError``). + + A cause without an ``error_code`` yields the unchanged default detail, + so a new failure mode stays opaque unless it opts in. + + Args: + exc (Exception): The underlying cause of the enrollment failure + + Returns: + EnrollmentError: The error to raise from `exc` + """ + error_code = getattr(exc, "error_code", None) + if not error_code: + return cls() + return cls(f"{cls.default_detail} Error code: {error_code}") + class EnrollmentCreationFailedError(EnrollmentError): """Error when the create_run_enrollments fails.""" diff --git a/courses/exceptions_test.py b/courses/exceptions_test.py new file mode 100644 index 0000000000..dda984af82 --- /dev/null +++ b/courses/exceptions_test.py @@ -0,0 +1,42 @@ +"""Tests for the courses API exceptions.""" + +from compliance.exceptions import ( + ExportComplianceCheckError, + ExportComplianceDataError, + ExportComplianceError, +) +from courses.exceptions import EnrollmentCreationFailedError, EnrollmentError + + +def test_from_cause_appends_the_causes_error_code(user): + """A CyberSource rejection contributes its support code to the detail.""" + exc = ExportComplianceError(user, "REJECT", "102") + + error = EnrollmentError.from_cause(exc) + + assert str(error.detail) == "Unable to complete enrollment. Error code: CS_700" + + +def test_from_cause_without_an_error_code_keeps_the_default_detail(user): + """ + Causes that declare no code stay opaque - notably the missing-profile-data + case, which never reached CyberSource and so has no CyberSource code. + """ + for exc in ( + ExportComplianceDataError(user, ["bill_to_country"]), + ExportComplianceCheckError("something else"), + ValueError("not a compliance failure at all"), + ): + error = EnrollmentError.from_cause(exc) + + assert str(error.detail) == EnrollmentError.default_detail + + +def test_from_cause_preserves_the_subclass(user): + """`cls` is honored, so a subclass keeps its own identity and detail.""" + error = EnrollmentCreationFailedError.from_cause( + ExportComplianceError(user, "REJECT", "102") + ) + + assert isinstance(error, EnrollmentCreationFailedError) + assert str(error.detail).endswith("Error code: CS_700") diff --git a/courses/serializers/v1/courses.py b/courses/serializers/v1/courses.py index c999f9b758..4bf32e0bec 100644 --- a/courses/serializers/v1/courses.py +++ b/courses/serializers/v1/courses.py @@ -200,8 +200,9 @@ def create(self, validated_data): ) except ExportComplianceCheckError as exc: # Don't propagate the specifics of the compliance decision to the - # client - the underlying cause is logged where it's raised. - raise EnrollmentError from exc + # client - only an opaque support code, if the cause declares one. + # The underlying cause is logged where it's raised. + raise EnrollmentError.from_cause(exc) from exc return successful_enrollments[0] if successful_enrollments else None diff --git a/courses/serializers/v2/courses.py b/courses/serializers/v2/courses.py index c45648762a..8f7a38c483 100644 --- a/courses/serializers/v2/courses.py +++ b/courses/serializers/v2/courses.py @@ -385,8 +385,9 @@ def create(self, validated_data): ) except ExportComplianceCheckError as exc: # Don't propagate the specifics of the compliance decision to the - # client - the underlying cause is logged where it's raised. - raise EnrollmentError from exc + # client - only an opaque support code, if the cause declares one. + # The underlying cause is logged where it's raised. + raise EnrollmentError.from_cause(exc) from exc return successful_enrollments[0] if successful_enrollments else None diff --git a/courses/serializers/v3/courses.py b/courses/serializers/v3/courses.py index 91ca376234..af15c81169 100644 --- a/courses/serializers/v3/courses.py +++ b/courses/serializers/v3/courses.py @@ -125,8 +125,9 @@ def create(self, validated_data): ) except ExportComplianceCheckError as exc: # Don't propagate the specifics of the compliance decision to the - # client - the underlying cause is logged where it's raised. - raise EnrollmentError from exc + # client - only an opaque support code, if the cause declares one. + # The underlying cause is logged where it's raised. + raise EnrollmentError.from_cause(exc) from exc if not successful_enrollments: log.error( diff --git a/courses/views/v1/views_test.py b/courses/views/v1/views_test.py index dd6617a949..d856458f39 100644 --- a/courses/views/v1/views_test.py +++ b/courses/views/v1/views_test.py @@ -558,11 +558,29 @@ def test_user_enrollments_create_export_compliance_blocked( ) assert resp.status_code == status.HTTP_400_BAD_REQUEST assert resp.json() == { - "detail": "Unable to complete enrollment. Please contact support." + "detail": "Unable to complete enrollment. Error code: CS_700" } assert not CourseRunEnrollment.objects.filter(user=user, run=run).exists() +def test_user_enrollments_create_export_compliance_missing_data( + mocker, user_drf_client, user +): + """Missing profile data is not a CyberSource rejection, so it carries no error code.""" + run = CourseRunFactory.create() + exc = ExportComplianceDataError(user, ["bill_to_country"]) + mocker.patch( + "courses.serializers.v1.courses.create_run_enrollments", + side_effect=exc, + ) + resp = user_drf_client.post( + reverse("v1:user-enrollments-api-list"), data={"run_id": run.id} + ) + assert resp.status_code == status.HTTP_400_BAD_REQUEST + assert resp.json() == {"detail": "Unable to complete enrollment."} + assert not CourseRunEnrollment.objects.filter(user=user, run=run).exists() + + def test_user_enrollments_create_b2b_run_invalid(user_drf_client, user): """Creating an enrollment for a B2B course run via the public API should be rejected.""" contract = ContractPageFactory.create() diff --git a/courses/views/v2/__init__.py b/courses/views/v2/__init__.py index bf1d368cf8..744f5a62ee 100644 --- a/courses/views/v2/__init__.py +++ b/courses/views/v2/__init__.py @@ -903,8 +903,9 @@ def _create_course_enrollment_from_program(request, courserun_id, program_enroll ) except ExportComplianceCheckError as exc: # Don't propagate the specifics of the compliance decision to the - # client - the underlying cause is logged where it's raised. - raise EnrollmentError from exc + # client - only an opaque support code, if the cause declares one. + # The underlying cause is logged where it's raised. + raise EnrollmentError.from_cause(exc) from exc if len(enrollments) == 0: raise EnrollmentCreationFailedError return _created_enrollment_response(enrollments[0]) diff --git a/courses/views/v2/views_test.py b/courses/views/v2/views_test.py index 71cc808b73..5d5f58f46a 100644 --- a/courses/views/v2/views_test.py +++ b/courses/views/v2/views_test.py @@ -36,7 +36,7 @@ ) from cms.models import CoursePage from cms.serializers import ProgramPageSerializer -from compliance.exceptions import ExportComplianceError +from compliance.exceptions import ExportComplianceDataError, ExportComplianceError from courses.constants import ENROLL_CHANGE_STATUS_UNENROLLED from courses.factories import ( CourseFactory, @@ -1258,11 +1258,29 @@ def test_user_enrollments_create_export_compliance_blocked_v2( ) assert resp.status_code == status.HTTP_400_BAD_REQUEST assert resp.json() == { - "detail": "Unable to complete enrollment. Please contact support." + "detail": "Unable to complete enrollment. Error code: CS_700" } assert not CourseRunEnrollment.objects.filter(user=user, run=run).exists() +def test_user_enrollments_create_export_compliance_missing_data_v2( + mocker, user_drf_client, user +): + """Missing profile data is not a CyberSource rejection, so it carries no error code.""" + run = CourseRunFactory.create() + exc = ExportComplianceDataError(user, ["bill_to_country"]) + mocker.patch( + "courses.serializers.v2.courses.create_run_enrollments", + side_effect=exc, + ) + resp = user_drf_client.post( + reverse("v2:user-enrollments-api-list"), data={"run_id": run.id} + ) + assert resp.status_code == status.HTTP_400_BAD_REQUEST + assert resp.json() == {"detail": "Unable to complete enrollment."} + assert not CourseRunEnrollment.objects.filter(user=user, run=run).exists() + + def test_program_filter_for_b2b_org(user, mock_course_run_clone): """Test that filtering programs by org works as expected.""" @@ -2290,7 +2308,7 @@ def test_add_verified_program_course_enrollment_export_compliance_blocked( assert resp.status_code == status.HTTP_400_BAD_REQUEST assert resp.json() == { - "detail": "Unable to complete enrollment. Please contact support." + "detail": "Unable to complete enrollment. Error code: CS_700" } assert not CourseRunEnrollment.objects.filter(user=user, run=course_run).exists() @@ -2515,7 +2533,7 @@ def test_add_nested_verified_program_course_enrollment_export_compliance_blocked assert resp.status_code == status.HTTP_400_BAD_REQUEST assert resp.json() == { - "detail": "Unable to complete enrollment. Please contact support." + "detail": "Unable to complete enrollment. Error code: CS_700" } assert not ProgramEnrollment.objects.filter(user=user, program=crogram).exists() diff --git a/courses/views/v3/__init__.py b/courses/views/v3/__init__.py index 67592abac6..921b98b54b 100644 --- a/courses/views/v3/__init__.py +++ b/courses/views/v3/__init__.py @@ -242,8 +242,9 @@ def create(self, request, *args, **kwargs): # noqa: ARG002 enrollments = create_program_enrollments(request.user, [program]) except ExportComplianceCheckError as exc: # Don't propagate the specifics of the compliance decision to the - # client - the underlying cause is logged where it's raised. - raise EnrollmentError from exc + # client - only an opaque support code, if the cause declares one. + # The underlying cause is logged where it's raised. + raise EnrollmentError.from_cause(exc) from exc if not enrollments: log.error( diff --git a/courses/views/v3/views_test.py b/courses/views/v3/views_test.py index f2ff727412..0e2eb82420 100644 --- a/courses/views/v3/views_test.py +++ b/courses/views/v3/views_test.py @@ -12,7 +12,7 @@ from rest_framework import status from rest_framework.test import APIClient -from compliance.exceptions import ExportComplianceError +from compliance.exceptions import ExportComplianceDataError, ExportComplianceError from courses.conftest import B2BCourses, UserWithEnrollmentsAndCerts from courses.constants import ( ENROLL_CHANGE_STATUS_UNENROLLED, @@ -625,11 +625,30 @@ def test_create_program_enrollment_export_compliance_blocked( assert resp.status_code == status.HTTP_400_BAD_REQUEST assert resp.json() == { - "detail": "Unable to complete enrollment. Please contact support." + "detail": "Unable to complete enrollment. Error code: CS_700" } assert not ProgramEnrollment.objects.filter(user=user, program=program).exists() +def test_create_program_enrollment_export_compliance_missing_data( + mocker, user_drf_client, user +): + """Missing profile data is not a CyberSource rejection, so it carries no error code.""" + program = ProgramFactory.create(live=True) + exc = ExportComplianceDataError(user, ["bill_to_country"]) + mocker.patch("courses.views.v3.create_program_enrollments", side_effect=exc) + + resp = user_drf_client.post( + reverse("v3:user_program_enrollments_api-list"), + data={"program_id": program.id}, + format="json", + ) + + assert resp.status_code == status.HTTP_400_BAD_REQUEST + assert resp.json() == {"detail": "Unable to complete enrollment."} + assert not ProgramEnrollment.objects.filter(user=user, program=program).exists() + + def test_create_program_enrollment_unauthenticated(): """POST without authentication returns 401 or 403.""" program = ProgramFactory.create(live=True)