Merge pull request #7394 from edx/clintonb/fix-existing-enrollment

Bypassing E-Commerce API if user is enrolled in course

lms/djangoapps/commerce/constants.py

@@ -20,3 +20,4 @@ class Messages(object):
     ORDER_COMPLETED = u'Order {order_number} was completed.'
     ORDER_INCOMPLETE_ENROLLED = u'Order {order_number} was created, but is not yet complete. User was enrolled.'
     NO_HONOR_MODE = u'Course {course_id} does not have an honor mode.'
+    ENROLLMENT_EXISTS = u'User {username} is already enrolled in {course_id}.'

lms/djangoapps/commerce/tests.py

@@ -15,7 +15,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
 
 from commerce.constants import OrderStatus, Messages
 from course_modes.models import CourseMode
-from enrollment.api import add_enrollment
+from enrollment.api import get_enrollment
 from student.models import CourseEnrollment
 from student.tests.factories import UserFactory, CourseModeFactory
 from student.tests.tests import EnrollmentEventTestMixin
@@ -166,31 +166,17 @@ class OrdersViewTests(EnrollmentEventTestMixin, ModuleStoreTestCase):
         self.assertValidEcommerceApiErrorResponse(response)
         self.assertUserNotEnrolled()
 
-    @data(True, False)
-    @httpretty.activate
-    def test_course_with_honor_seat_sku(self, user_is_active):
+    def _test_successful_ecommerce_api_call(self):
         """
-        If the course has a SKU, the view should get authorization from the E-Commerce API before enrolling
-        the user in the course. If authorization is approved, the user should be redirected to the user dashboard.
+        Verifies that the view contacts the E-Commerce API with the correct data and headers.
         """
-
-        # Set user's active flag
-        self.user.is_active = user_is_active
-        self.user.save()  # pylint: disable=no-member
-
-        def request_callback(_method, _uri, headers):
-            """ Mock the E-Commerce API's call to the enrollment API. """
-            add_enrollment(self.user.username, unicode(self.course.id), 'honor')
-            return 200, headers, ECOMMERCE_API_SUCCESSFUL_BODY
-
-        self._mock_ecommerce_api(body=request_callback)
+        self._mock_ecommerce_api(body=ECOMMERCE_API_SUCCESSFUL_BODY)
         response = self._post_to_view()
 
         # Validate the response content
         msg = Messages.ORDER_COMPLETED.format(order_number=ORDER_NUMBER)
         self.assertResponseMessage(response, msg)
-
-        self.assertUserEnrolled()
+        self.assertEqual(response.status_code, 200)
 
         # Verify the correct information was passed to the E-Commerce API
         request = httpretty.last_request()
@@ -203,6 +189,19 @@ class OrdersViewTests(EnrollmentEventTestMixin, ModuleStoreTestCase):
                                   ECOMMERCE_API_SIGNING_KEY)
         self.assertEqual(request.headers['Authorization'], 'JWT {}'.format(expected_jwt))
 
+    @data(True, False)
+    @httpretty.activate
+    def test_course_with_honor_seat_sku(self, user_is_active):
+        """
+        If the course has a SKU for honor mode, the view should get authorization from the E-Commerce API before
+        enrolling the user in the course.
+        """
+        # Set user's active flag
+        self.user.is_active = user_is_active
+        self.user.save()  # pylint: disable=no-member
+
+        self._test_successful_ecommerce_api_call()
+
     @httpretty.activate
     def test_order_not_complete(self):
         self._mock_ecommerce_api(body=json.dumps({'status': OrderStatus.OPEN, 'number': ORDER_NUMBER}))
@@ -297,3 +296,29 @@ class OrdersViewTests(EnrollmentEventTestMixin, ModuleStoreTestCase):
         the E-Commerce API is not configured.
         """
         self._test_professional_mode_only()
+
+    def test_existing_active_enrollment(self):
+        """ The view should respond with HTTP 409 if the user has an existing active enrollment for the course. """
+
+        # Enroll user in the course
+        CourseEnrollment.enroll(self.user, self.course.id)
+        self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id))
+
+        response = self._post_to_view()
+        self.assertEqual(response.status_code, 409)
+        msg = Messages.ENROLLMENT_EXISTS.format(username=self.user.username, course_id=self.course.id)
+        self.assertResponseMessage(response, msg)
+
+    @httpretty.activate
+    def test_existing_inactive_enrollment(self):
+        """
+        If the user has an inactive enrollment for the course, the view should behave as if the
+        user has no enrollment.
+        """
+        # Create an inactive enrollment
+        CourseEnrollment.enroll(self.user, self.course.id)
+        CourseEnrollment.unenroll(self.user, self.course.id, True)
+        self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id))
+        self.assertIsNotNone(get_enrollment(self.user.username, unicode(self.course.id)))
+
+        self._test_successful_ecommerce_api_call()

lms/djangoapps/commerce/views.py

@@ -9,7 +9,7 @@ from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
 import requests
 from rest_framework.permissions import IsAuthenticated
-from rest_framework.status import HTTP_406_NOT_ACCEPTABLE, HTTP_202_ACCEPTED, HTTP_200_OK
+from rest_framework.status import HTTP_406_NOT_ACCEPTABLE, HTTP_202_ACCEPTED, HTTP_200_OK, HTTP_409_CONFLICT
 from rest_framework.views import APIView
 
 from commerce.constants import OrderStatus, Messages
@@ -17,6 +17,7 @@ from commerce.http import DetailResponse, ApiErrorResponse
 from course_modes.models import CourseMode
 from courseware import courses
 from enrollment.api import add_enrollment
+from student.models import CourseEnrollment
 from util.authentication import SessionAuthenticationAllowInactiveUser
 
 
@@ -78,6 +79,17 @@ class OrdersView(APIView):
         if not valid:
             return DetailResponse(error, status=HTTP_406_NOT_ACCEPTABLE)
 
+        # Ensure that the E-Commerce API is setup properly
+        ecommerce_api_url = getattr(settings, 'ECOMMERCE_API_URL', None)
+        ecommerce_api_signing_key = getattr(settings, 'ECOMMERCE_API_SIGNING_KEY', None)
+        course_id = unicode(course_key)
+
+        # Don't do anything if an enrollment already exists
+        enrollment = CourseEnrollment.get_enrollment(user, course_key)
+        if enrollment and enrollment.is_active:
+            msg = Messages.ENROLLMENT_EXISTS.format(course_id=course_id, username=user.username)
+            return DetailResponse(msg, status=HTTP_409_CONFLICT)
+
         # Ensure that the course has an honor mode with SKU
         honor_mode = CourseMode.mode_for_course(course_key, CourseMode.HONOR)
         course_id = unicode(course_key)
@@ -95,10 +107,7 @@ class OrdersView(APIView):
             self._enroll(course_key, user)
             return DetailResponse(msg)
 
-        # Ensure that the E-Commerce API is setup properly
-        ecommerce_api_url = getattr(settings, 'ECOMMERCE_API_URL', None)
-        ecommerce_api_signing_key = getattr(settings, 'ECOMMERCE_API_SIGNING_KEY', None)
-
+        # If the API is not configured, bypass it.
         if not (ecommerce_api_url and ecommerce_api_signing_key):
             self._enroll(course_key, user)
             msg = Messages.NO_ECOM_API.format(username=user.username, course_id=course_id)