Merge pull request #5006 from edx/will/ecom-199

ECOM-199: Auto-registration bugfix

common/djangoapps/student/tests/test_enrollment.py

@@ -29,12 +29,16 @@ class EnrollmentTest(ModuleStoreTestCase):
     """
     Test student enrollment, especially with different course modes.
     """
+    USERNAME = "Bob"
+    EMAIL = "bob@example.com"
+    PASSWORD = "edx"
+
     def setUp(self):
         """ Create a course and user, then log in. """
         super(EnrollmentTest, self).setUp()
         self.course = CourseFactory.create()
-        self.user = UserFactory.create(username="Bob", email="bob@example.com", password="edx")
-        self.client.login(username=self.user.username, password="edx")
+        self.user = UserFactory.create(username=self.USERNAME, email=self.EMAIL, password=self.PASSWORD)
+        self.client.login(username=self.USERNAME, password=self.PASSWORD)
 
         self.urls = [
             reverse('course_modes_choose', kwargs={'course_id': unicode(self.course.id)})
@@ -74,7 +78,6 @@ class EnrollmentTest(ModuleStoreTestCase):
                 course_id=self.course.id,
                 mode_slug=mode_slug,
                 mode_display_name=mode_slug,
-                expiration_datetime=datetime.now(pytz.UTC) + timedelta(days=1)
             )
 
         # Reverse the expected next URL, if one is provided
@@ -107,6 +110,74 @@ class EnrollmentTest(ModuleStoreTestCase):
             self.assertTrue(is_active)
             self.assertEqual(course_mode, enrollment_mode)
 
+    # TODO (ECOM-16): Remove once the auto-registration A/B test completes.
+    def test_enroll_from_redirect_autoreg(self):
+        # Bugfix (ECOM-199): Visitors removed from auto-enroll pool
+        # if they need to authenticate while attempting to enroll.
+        # If a user is (a) in the experimental condition of the A/B test (autoreg enabled)
+        # and (b) is not logged in when registering for a course, then
+        # the user will be redirected to the "control" URL (change_enrollment)
+        # instead of the experimental URL (change_enrollment_autoreg)
+        # We work around this by setting a flag in the session, such that
+        # if a user is *ever* in the experimental condition, they remain
+        # in the experimental condition.
+        for mode_slug in ['honor', 'audit', 'verified']:
+            CourseModeFactory.create(
+                course_id=self.course.id,
+                mode_slug=mode_slug,
+                mode_display_name=mode_slug
+            )
+
+        # Log out, so we're no longer authenticated
+        self.client.logout()
+
+        # Visit the experimental condition URL
+        resp = self._change_enrollment('enroll', auto_reg=True)
+
+        # Expect that we're denied (since we're not authenticated)
+        # and instead are redirected to the login page
+        self.assertEqual(resp.status_code, 403)
+
+        # Log the user in
+        self.client.login(username=self.USERNAME, password=self.PASSWORD)
+
+        # Go to the control URL
+        # This simulates what the JavaScript client does after getting a 403 response
+        # from the register button.
+        resp = self._change_enrollment('enroll')
+
+        # Expect that we're auto-enrolled (even though we used the control URL the second time)
+        self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id))
+
+        # Expect that the auto register flag is still set in the user's session
+        self.assertIn('auto_register', self.client.session)
+        self.assertTrue(self.client.session['auto_register'])
+
+    # TODO (ECOM-16): Remove once the auto-registration A/B test completes
+    def test_enroll_auto_registration_excluded_course(self):
+        # Create the course modes
+        for mode_slug in ['honor', 'audit', 'verified']:
+            CourseModeFactory.create(
+                course_id=self.course.id,
+                mode_slug=mode_slug,
+                mode_display_name=mode_slug,
+            )
+
+        # Visit the experimental condition URL (when the course is NOT excluded)
+        # This should place us into the experimental condition flow
+        self._change_enrollment('enroll', auto_reg=True)
+
+        # Unenroll from the course (we were registered because auto enroll was enabled)
+        self._change_enrollment('unenroll')
+
+        # Register for the course again, with the course excluded
+        # At this point, we should NOT be in the experimental condition flow
+        excluded_course_ids = [self.course.id.to_deprecated_string()]
+        with self.settings(AUTO_REGISTRATION_AB_TEST_EXCLUDE_COURSES=excluded_course_ids):
+            self._change_enrollment('enroll')
+            self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id))
+            self.assertNotIn('auto_register', self.client.session)
+
     def test_unenroll(self):
         # Enroll the student in the course
         CourseEnrollment.enroll(self.user, self.course.id, mode="honor")

common/djangoapps/student/tests/tests.py

@@ -15,6 +15,7 @@ from django.test.utils import override_settings
 from django.test.client import RequestFactory, Client
 from django.contrib.auth.models import User, AnonymousUser
 from django.core.urlresolvers import reverse
+from django.contrib.sessions.middleware import SessionMiddleware
 
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -634,8 +635,17 @@ class PaidRegistrationTest(ModuleStoreTestCase):
 
     @unittest.skipUnless(settings.FEATURES.get('ENABLE_SHOPPING_CART'), "Shopping Cart not enabled in settings")
     def test_change_enrollment_add_to_cart(self):
-        request = self.req_factory.post(reverse('change_enrollment'), {'course_id': self.course.id.to_deprecated_string(),
-                                                                       'enrollment_action': 'add_to_cart'})
+        request = self.req_factory.post(
+            reverse('change_enrollment'), {
+                'course_id': self.course.id.to_deprecated_string(),
+                'enrollment_action': 'add_to_cart'
+            }
+        )
+
+        # Add a session to the request
+        SessionMiddleware().process_request(request)
+        request.session.save()
+
         request.user = self.user
         response = change_enrollment(request)
         self.assertEqual(response.status_code, 200)

common/djangoapps/student/views.py

@@ -634,6 +634,25 @@ def change_enrollment(request, auto_register=False):
         ))
         return HttpResponseBadRequest(_("Invalid course id"))
 
+    # TODO (ECOM-16): Remove this once the auto-registration A/B test completes
+    # If a user is in the experimental condition (auto-registration enabled),
+    # immediately set a session flag so they stay in the experimental condition.
+    # We keep them in the experimental condition even if later on the user
+    # tries to register using the control URL (e.g. because of a redirect from the login page,
+    # which is hard-coded to use the control URL).
+    if auto_register:
+        request.session['auto_register'] = True
+    if request.session.get('auto_register') and not auto_register:
+        auto_register = True
+
+    # TODO (ECOM-16): Remove this once the auto-registration A/B test completes
+    # We've agreed to exclude certain courses from the A/B test.  If we find ourselves
+    # registering for one of these courses, immediately switch to the control.
+    if unicode(course_id) in getattr(settings, 'AUTO_REGISTRATION_AB_TEST_EXCLUDE_COURSES', []):
+        auto_register = False
+        if 'auto_register' in request.session:
+            del request.session['auto_register']
+
     if not user.is_authenticated():
         return HttpResponseForbidden()
 
@@ -671,11 +690,6 @@ def change_enrollment(request, auto_register=False):
         # TODO (ECOM-16): Once the auto-registration AB-test is complete, delete
         # one of these two conditions and remove the `auto_register` flag.
         if auto_register:
-            # TODO (ECOM-16): This stores a flag in the session so downstream
-            # views will recognize that the user is in the "auto-registration"
-            # experimental condition.  We can remove this once the AB test completes.
-            request.session['auto_register'] = True
-
             available_modes = CourseMode.modes_for_course_dict(course_id)
 
             # Handle professional ed as a special case.
@@ -710,12 +724,6 @@ def change_enrollment(request, auto_register=False):
         # before sending them to the "choose your track" page.
         # This is the control for the auto-registration AB-test.
         else:
-            # TODO (ECOM-16): If the user is NOT in the experimental condition,
-            # make sure their session reflects this.  We can remove this
-            # once the AB test completes.
-            if 'auto_register' in request.session:
-                del request.session['auto_register']
-
             # If this course is available in multiple modes, redirect them to a page
             # where they can choose which mode they want.
             available_modes = CourseMode.modes_for_course(course_id)

lms/envs/aws.py

@@ -445,3 +445,6 @@ OPTIMIZELY_PROJECT_ID = AUTH_TOKENS.get('OPTIMIZELY_PROJECT_ID', OPTIMIZELY_PROJ
 
 #### Course Registration Code length ####
 REGISTRATION_CODE_LENGTH = ENV_TOKENS.get('REGISTRATION_CODE_LENGTH', 8)
+
+# TODO (ECOM-16): Remove once the A/B test of auto-registration completes
+AUTO_REGISTRATION_AB_TEST_EXCLUDE_COURSES = set(ENV_TOKENS.get('AUTO_REGISTRATION_AB_TEST_EXCLUDE_COURSES', AUTO_REGISTRATION_AB_TEST_EXCLUDE_COURSES))

lms/envs/common.py

@@ -1735,3 +1735,21 @@ OPENID_DOMAIN_PREFIX = 'openid:'
 ANALYTICS_DATA_URL = ""
 ANALYTICS_DATA_TOKEN = ""
 ANALYTICS_DASHBOARD_URL = ""
+
+# TODO (ECOM-16): Remove once the A/B test of auto-registration completes
+AUTO_REGISTRATION_AB_TEST_EXCLUDE_COURSES = set([
+    "HarvardX/SW12.2x/1T2014",
+    "HarvardX/SW12.3x/1T2014",
+    "HarvardX/SW12.4x/1T2014",
+    "HarvardX/SW12.5x/2T2014",
+    "HarvardX/SW12.6x/2T2014",
+    "HarvardX/HUM2.1x/3T2014",
+    "HarvardX/SW12x/2013_SOND",
+    "LinuxFoundationX/LFS101x/2T2014",
+    "HarvardX/CS50x/2014_T1",
+    "HarvardX/AmPoX.1/2014_T3",
+    "HarvardX/SW12.7x/3T2014",
+    "HarvardX/SW12.10x/1T2015",
+    "HarvardX/SW12.9x/3T2014",
+    "HarvardX/SW12.8x/3T2014",
+])