diff --git a/common/djangoapps/course_modes/tests/test_views.py b/common/djangoapps/course_modes/tests/test_views.py index 74e3b4b1b5..30b242671b 100644 --- a/common/djangoapps/course_modes/tests/test_views.py +++ b/common/djangoapps/course_modes/tests/test_views.py @@ -32,44 +32,22 @@ class CourseModeViewTest(ModuleStoreTestCase): self.client.login(username=self.user.username, password="edx") @ddt.data( - # is_active?, enrollment_mode, upgrade?, redirect?, auto_register? - (True, 'verified', True, True, False), # User is already verified - (True, 'verified', False, True, False), # User is already verified - (True, 'honor', True, False, False), # User isn't trying to upgrade - (True, 'honor', False, True, False), # User is trying to upgrade - (True, 'audit', True, False, False), # User isn't trying to upgrade - (True, 'audit', False, True, False), # User is trying to upgrade - (False, 'verified', True, False, False), # User isn't active - (False, 'verified', False, False, False), # User isn't active - (False, 'honor', True, False, False), # User isn't active - (False, 'honor', False, False, False), # User isn't active - (False, 'audit', True, False, False), # User isn't active - (False, 'audit', False, False, False), # User isn't active - - # When auto-registration is enabled, users may already be - # registered when they reach the "choose your track" - # page. In this case, we do NOT want to redirect them - # to the dashboard, because we want to give them the option - # to enter the verification/payment track. - # TODO (ECOM-16): based on the outcome of the auto-registration AB test, - # either keep these tests or remove them. In either case, - # remove the "auto_register" flag from this test case. - (True, 'verified', True, False, True), - (True, 'verified', False, True, True), - (True, 'honor', True, False, True), - (True, 'honor', False, False, True), - (True, 'audit', True, False, True), - (True, 'audit', False, False, True), + # is_active?, enrollment_mode, upgrade?, redirect? + (True, 'verified', True, False), # User has an active verified enrollment and is trying to upgrade + (True, 'verified', False, True), # User has an active verified enrollment and is not trying to upgrade + (True, 'honor', True, False), # User has an active honor enrollment and is trying to upgrade + (True, 'honor', False, False), # User has an active honor enrollment and is not trying to upgrade + (True, 'audit', True, False), # User has an active audit enrollment and is trying to upgrade + (True, 'audit', False, False), # User has an active audit enrollment and is not trying to upgrade + (False, 'verified', True, True), # User has an inactive verified enrollment and is trying to upgrade + (False, 'verified', False, True), # User has an inactive verified enrollment and is not trying to upgrade + (False, 'honor', True, True), # User has an inactive honor enrollment and is trying to upgrade + (False, 'honor', False, True), # User has an inactive honor enrollment and is not trying to upgrade + (False, 'audit', True, True), # User has an inactive audit enrollment and is trying to upgrade + (False, 'audit', False, True), # User has an inactive audit enrollment and is not trying to upgrade ) @ddt.unpack - def test_redirect_to_dashboard(self, is_active, enrollment_mode, upgrade, redirect, auto_register): - - # TODO (ECOM-16): Remove once we complete the auto-reg AB test. - if auto_register: - session = self.client.session - session['auto_register'] = True - session.save() - + def test_redirect_to_dashboard(self, is_active, enrollment_mode, upgrade, redirect): # Create the course modes for mode in ('audit', 'honor', 'verified'): CourseModeFactory(mode_slug=mode, course_id=self.course.id) @@ -96,6 +74,17 @@ class CourseModeViewTest(ModuleStoreTestCase): else: self.assertEquals(response.status_code, 200) + def test_redirect_to_dashboard_no_enrollment(self): + # Create the course modes + for mode in ('audit', 'honor', 'verified'): + CourseModeFactory(mode_slug=mode, course_id=self.course.id) + + # User visits the track selection page directly without ever enrolling + url = reverse('course_modes_choose', args=[unicode(self.course.id)]) + response = self.client.get(url) + + self.assertRedirects(response, reverse('dashboard')) + @ddt.data( '', '1,,2', @@ -114,6 +103,14 @@ class CourseModeViewTest(ModuleStoreTestCase): suggested_prices=price_list ) + # Enroll the user in the test course to emulate + # automatic enrollment + CourseEnrollmentFactory( + is_active=True, + course_id=self.course.id, + user=self.user + ) + # Verify that the prices render correctly response = self.client.get( reverse('course_modes_choose', args=[unicode(self.course.id)]), @@ -124,16 +121,7 @@ class CourseModeViewTest(ModuleStoreTestCase): # TODO: Fix it so that response.templates works w/ mako templates, and then assert # that the right template rendered - # TODO (ECOM-16): Remove the auto-registration flag once the AB test is complete - # and we choose the winner as the default - @ddt.data(True, False) - def test_professional_registration(self, auto_register): - - # TODO (ECOM-16): Remove once we complete the auto-reg AB test. - if auto_register: - self.client.session['auto_register'] = True - self.client.session.save() - + def test_professional_registration(self): # The only course mode is professional ed CourseModeFactory(mode_slug='professional', course_id=self.course.id) @@ -167,22 +155,12 @@ class CourseModeViewTest(ModuleStoreTestCase): 'unsupported': {'unsupported_mode': True}, } - # TODO (ECOM-16): Remove the auto-register flag once the AB-test completes - # and we default it to enabled or disabled. @ddt.data( - (False, 'honor', 'dashboard'), - (False, 'verified', 'show_requirements'), - (True, 'honor', 'dashboard'), - (True, 'verified', 'show_requirements'), + ('honor', 'dashboard'), + ('verified', 'show_requirements'), ) @ddt.unpack - def test_choose_mode_redirect(self, auto_register, course_mode, expected_redirect): - - # TODO (ECOM-16): Remove once we complete the auto-reg AB test. - if auto_register: - self.client.session['auto_register'] = True - self.client.session.save() - + def test_choose_mode_redirect(self, course_mode, expected_redirect): # Create the course modes for mode in ('audit', 'honor', 'verified'): CourseModeFactory(mode_slug=mode, course_id=self.course.id) @@ -221,23 +199,24 @@ class CourseModeViewTest(ModuleStoreTestCase): expected_amount = decimal.Decimal(self.POST_PARAMS_FOR_COURSE_MODE['verified']['contribution']) self.assertEqual(actual_amount, expected_amount) - # TODO (ECOM-16): Remove auto-register booleans once the AB-test completes - @ddt.data(False, True) - def test_successful_honor_enrollment(self, auto_register): - # TODO (ECOM-16): Remove once we complete the auto-reg AB test. - if auto_register: - self.client.session['auto_register'] = True - self.client.session.save() - + def test_successful_honor_enrollment(self): # Create the course modes for mode in ('honor', 'verified'): CourseModeFactory(mode_slug=mode, course_id=self.course.id) - # Choose the mode (POST request) + # Enroll the user in the default mode (honor) to emulate + # automatic enrollment + params = { + 'enrollment_action': 'enroll', + 'course_id': unicode(self.course.id) + } + self.client.post(reverse('change_enrollment'), params) + + # Explicitly select the honor mode (POST request) choose_track_url = reverse('course_modes_choose', args=[unicode(self.course.id)]) self.client.post(choose_track_url, self.POST_PARAMS_FOR_COURSE_MODE['honor']) - # Verify the enrollment + # Verify that the user's enrollment remains unchanged mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) self.assertEqual(mode, 'honor') self.assertEqual(is_active, True) diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index 531069b2f6..b70e9e3028 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -49,32 +49,17 @@ class ChooseModeView(View): """ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - enrollment_mode, is_active = CourseEnrollment.enrollment_mode_for_user(request.user, course_key) + upgrade = request.GET.get('upgrade', False) request.session['attempting_upgrade'] = upgrade - # TODO (ECOM-16): Remove once the AB-test of auto-registration completes - auto_register = request.session.get('auto_register', False) - - # Inactive users always need to re-register - # Verified and professional users do not need to register or upgrade - # Registered users who are not trying to upgrade do not need to re-register - if not auto_register: - go_to_dashboard = ( - is_active and - (not upgrade or enrollment_mode in ['verified', 'professional']) - ) - - # If auto-registration is enabled, then students might already be registered, + # Students will already have an active course enrollment at this stage, # but we should still show them the "choose your track" page so they have # the option to enter the verification/payment flow. - # TODO (ECOM-16): Based on the results of the AB-test, set the default behavior to - # either enable or disable auto-registration. - else: - go_to_dashboard = ( - not upgrade and enrollment_mode in ['verified', 'professional'] - ) + go_to_dashboard = ( + not upgrade and enrollment_mode in ['verified', 'professional'] + ) if go_to_dashboard: return redirect(reverse('dashboard')) @@ -92,6 +77,16 @@ class ChooseModeView(View): ) ) + # If a user's course enrollment is inactive at this stage, the track + # selection page may have been visited directly, so we should redirect + # the user to their dashboard. By the time the user gets here during the + # normal registration process, they will already have an activated enrollment; + # the button appearing on the track selection page only redirects the user to + # the dashboard, and we don't want the user to be confused when they click the + # honor button and are taken to their dashboard without being enrolled. + if not is_active: + return redirect(reverse('dashboard')) + donation_for_course = request.session.get("donation_for_course", {}) chosen_price = donation_for_course.get(unicode(course_key), None) @@ -106,7 +101,6 @@ class ChooseModeView(View): "error": error, "upgrade": upgrade, "can_audit": "audit" in modes, - "autoreg": auto_register } if "verified" in modes: context["suggested_prices"] = [ @@ -155,10 +149,10 @@ class ChooseModeView(View): if requested_mode not in allowed_modes: return HttpResponseBadRequest(_("Enrollment mode not supported")) - # TODO (ECOM-16): Remove if the experimental variant wins. Functionally, - # it doesn't matter, but it will avoid hitting the database. if requested_mode == 'honor': - CourseEnrollment.enroll(user, course_key, requested_mode) + # The user will have already been enrolled in the honor mode at this + # point, so we just redirect them to the dashboard, thereby avoiding + # hitting the database a second time attempting to enroll them. return redirect(reverse('dashboard')) mode_info = allowed_modes[requested_mode] diff --git a/common/djangoapps/external_auth/tests/test_shib.py b/common/djangoapps/external_auth/tests/test_shib.py index 3b9668aedc..fa09501e41 100644 --- a/common/djangoapps/external_auth/tests/test_shib.py +++ b/common/djangoapps/external_auth/tests/test_shib.py @@ -14,7 +14,6 @@ from django.test.client import RequestFactory, Client as DjangoTestClient from django.test.utils import override_settings from django.core.urlresolvers import reverse from django.contrib.auth.models import AnonymousUser, User -from django.contrib.sessions.middleware import SessionMiddleware from django.utils.importlib import import_module from xmodule.modulestore.tests.factories import CourseFactory @@ -511,10 +510,6 @@ class ShibSPTest(ModuleStoreTestCase): for student in [shib_student, other_ext_student, int_student]: request = self.request_factory.post('/change_enrollment') - # Add a session to the request - SessionMiddleware().process_request(request) - request.session.save() - request.POST.update({'enrollment_action': 'enroll', 'course_id': course.id.to_deprecated_string()}) request.user = student diff --git a/common/djangoapps/student/tests/test_enrollment.py b/common/djangoapps/student/tests/test_enrollment.py index 58a32dd312..818c9bd9b6 100644 --- a/common/djangoapps/student/tests/test_enrollment.py +++ b/common/djangoapps/student/tests/test_enrollment.py @@ -45,34 +45,26 @@ class EnrollmentTest(ModuleStoreTestCase): reverse('course_modes_choose', kwargs={'course_id': unicode(self.course.id)}) ] - # TODO (ECOM-16): We need separate test cases for both conditions in the auto-registration - # AB-test. Once we get the results of that test, we should - # remove the losing condition from this test. @ddt.data( # Default (no course modes in the database) # Expect that we're redirected to the dashboard # and automatically enrolled as "honor" - ([], '', 'honor', False), - ([], '', 'honor', True), + ([], '', 'honor'), # Audit / Verified / Honor - # We should always go to the "choose your course" page, - # If auto-registration is enabled, we should also be registered - # as "honor" by default. - (['honor', 'verified', 'audit'], 'course_modes_choose', None, False), - (['honor', 'verified', 'audit'], 'course_modes_choose', 'honor', True), + # We should always go to the "choose your course" page. + # We should also be enrolled as "honor" by default. + (['honor', 'verified', 'audit'], 'course_modes_choose', 'honor'), # Professional ed # Expect that we're sent to the "choose your track" page # (which will, in turn, redirect us to a page where we can verify/pay) - # Even if auto registration is enabled, we should NOT be auto-registered, - # because that would be giving away an expensive course for free :) - (['professional'], 'course_modes_choose', None, False), - (['professional'], 'course_modes_choose', None, True), - + # We should NOT be auto-enrolled, because that would be giving + # away an expensive course for free :) + (['professional'], 'course_modes_choose', None), ) @ddt.unpack - def test_enroll(self, course_modes, next_url, enrollment_mode, auto_reg): + def test_enroll(self, course_modes, next_url, enrollment_mode): # Create the course modes (if any) required for this test case for mode_slug in course_modes: CourseModeFactory.create( @@ -90,16 +82,10 @@ class EnrollmentTest(ModuleStoreTestCase): ) # Enroll in the course and verify the URL we get sent to - resp = self._change_enrollment('enroll', auto_reg=auto_reg) + resp = self._change_enrollment('enroll') self.assertEqual(resp.status_code, 200) self.assertEqual(resp.content, full_url) - # TODO (ECOM-16): If auto-registration is enabled, check that we're - # storing the auto-reg flag in the user's session - if auto_reg: - self.assertIn('auto_register', self.client.session) - self.assertTrue(self.client.session['auto_register']) - # If we're not expecting to be enrolled, verify that this is the case if enrollment_mode is None: self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) @@ -111,53 +97,10 @@ 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']) - - def test_enroll_from_redirect_autoreg_third_party(self): + def test_enroll_from_third_party_redirect(self): """ Test that, when a user visits the registration page *after* visiting a course, - if they go on to register and/or log in via third-party auth, they'll be registered + if they go on to register and/or log in via third-party auth, they'll be enrolled in that course. The testing here is a bit hackish, since we just ping the registration page, then @@ -173,31 +116,6 @@ class EnrollmentTest(ModuleStoreTestCase): change_enrollment_third_party(is_register=True, strategy=strategy, user=self.user) self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id)) - # 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") @@ -238,9 +156,8 @@ class EnrollmentTest(ModuleStoreTestCase): resp = self._change_enrollment('unenroll', course_id="edx/") self.assertEqual(resp.status_code, 400) - def _change_enrollment(self, action, course_id=None, auto_reg=False): - """ - Change the student's enrollment status in a course. + def _change_enrollment(self, action, course_id=None): + """Change the student's enrollment status in a course. Args: action (string): The action to perform (either "enroll" or "unenroll") @@ -249,9 +166,6 @@ class EnrollmentTest(ModuleStoreTestCase): course_id (unicode): If provided, use this course ID. Otherwise, use the course ID created in the setup for this test. - auto_reg (boolean): Whether to use the auto-registration hook. - TODO (ECOM-16): remove this once we complete the AB test for auto-registration. - Returns: Response @@ -259,13 +173,8 @@ class EnrollmentTest(ModuleStoreTestCase): if course_id is None: course_id = unicode(self.course.id) - url = ( - reverse('change_enrollment') - if not auto_reg - else reverse('change_enrollment_autoreg') - ) params = { 'enrollment_action': action, 'course_id': course_id } - return self.client.post(url, params) + return self.client.post(reverse('change_enrollment'), params) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index ba94f01d5f..14be87b300 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -726,7 +726,7 @@ def try_change_enrollment(request): @require_POST @commit_on_success_with_read_committed -def change_enrollment(request, auto_register=False, check_access=True): +def change_enrollment(request, check_access=True): """ Modify the enrollment status for the logged-in user. @@ -742,37 +742,21 @@ def change_enrollment(request, auto_register=False, check_access=True): happens. This function should only be called from an AJAX request or as a post-login/registration helper, so the error messages in the responses should never actually be user-visible. - The original version of the change enrollment handler, - which does NOT perform auto-registration. - - TODO (ECOM-16): We created a second variation of this handler that performs - auto-registration for an AB-test. Depending on the results of that test, - we should make the winning implementation the default. Args: request (`Request`): The Django request object Keyword Args: - auto_register (boolean): If True, auto-register the user - for a default course mode when they first enroll - before sending them to the "choose your track" page + check_access (boolean): If True, we check that an accessible course actually + exists for the given course_key before we enroll the student. + The default is set to False to avoid breaking legacy code or + code with non-standard flows (ex. beta tester invitations), but + for any standard enrollment flow you probably want this to be True. Returns: Response """ - # Sets the auto_register flag, if that's desired - # 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 - # Get the user user = request.user @@ -797,15 +781,6 @@ def change_enrollment(request, auto_register=False, check_access=True): ) return HttpResponseBadRequest(_("Invalid course id")) - # Don't execute auto-register for the set of courses excluded from auto-registration - # 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 action == "enroll": # Make sure the course exists # We don't do this check on unenroll, or a bad course id can't be unenrolled from @@ -814,73 +789,38 @@ def change_enrollment(request, auto_register=False, check_access=True): .format(user.username, course_id)) return HttpResponseBadRequest(_("Course id is invalid")) - # We use this flag to determine which condition of an AB-test - # for auto-registration we're currently in. - # (We have two URLs that both point to this view, but vary the - # value of `auto_register`) - # In the auto-registration case, we automatically register the student - # as "honor" before allowing them to choose a track. - # 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: - available_modes = CourseMode.modes_for_course_dict(course_id) - - # Handle professional ed as a special case. - # If professional ed is included in the list of available modes, - # then do NOT automatically enroll the student (we want them to pay first!) - # By convention, professional ed should be the *only* available course mode, - # if it's included at all -- anything else is a misconfiguration. But if someone - # messes up and adds an additional course mode, we err on the side of NOT - # accidentally giving away free courses. - if "professional" not in available_modes: - # Enroll the user using the default mode (honor) - # We're assuming that users of the course enrollment table - # will NOT try to look up the course enrollment model - # by its slug. If they do, it's possible (based on the state of the database) - # for no such model to exist, even though we've set the enrollment type - # to "honor". - try: - CourseEnrollment.enroll(user, course_id, check_access=check_access) - except Exception: - return HttpResponseBadRequest(_("Could not enroll")) - - # If we have more than one course mode or professional ed is enabled, - # then send the user to the choose your track page. - # (In the case of professional ed, this will redirect to a page that - # funnels users directly into the verification / payment flow) - if len(available_modes) > 1 or "professional" in available_modes: - return HttpResponse( - reverse("course_modes_choose", kwargs={'course_id': unicode(course_id)}) - ) - - # Otherwise, there is only one mode available (the default) - return HttpResponse() - - # If auto-registration is disabled, do NOT register the student - # before sending them to the "choose your track" page. - # This is the control for the auto-registration AB-test. - else: - # 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) - if len(available_modes) > 1: - return HttpResponse( - reverse("course_modes_choose", kwargs={'course_id': unicode(course_id)}) - ) - - current_mode = available_modes[0] - # only automatically enroll people if the only mode is 'honor' - if current_mode.slug != 'honor': - return HttpResponse( - reverse("course_modes_choose", kwargs={'course_id': unicode(course_id)}) - ) + available_modes = CourseMode.modes_for_course_dict(course_id) + # Handle professional ed as a special case. + # If professional ed is included in the list of available modes, + # then do NOT automatically enroll the student (we want them to pay first!) + # By convention, professional ed should be the *only* available course mode, + # if it's included at all -- anything else is a misconfiguration. But if someone + # messes up and adds an additional course mode, we err on the side of NOT + # accidentally giving away free courses. + if "professional" not in available_modes: + # Enroll the user using the default mode (honor) + # We're assuming that users of the course enrollment table + # will NOT try to look up the course enrollment model + # by its slug. If they do, it's possible (based on the state of the database) + # for no such model to exist, even though we've set the enrollment type + # to "honor". try: - CourseEnrollment.enroll(user, course_id, mode=current_mode.slug, check_access=check_access) + CourseEnrollment.enroll(user, course_id, check_access=check_access) except Exception: return HttpResponseBadRequest(_("Could not enroll")) - return HttpResponse() + # If we have more than one course mode or professional ed is enabled, + # then send the user to the choose your track page. + # (In the case of professional ed, this will redirect to a page that + # funnels users directly into the verification / payment flow) + if len(available_modes) > 1 or "professional" in available_modes: + return HttpResponse( + reverse("course_modes_choose", kwargs={'course_id': unicode(course_id)}) + ) + + # Otherwise, there is only one mode available (the default) + return HttpResponse() elif action == "add_to_cart": # Pass the request handling to shoppingcart.views diff --git a/common/templates/course_modes/choose.html b/common/templates/course_modes/choose.html index 1d28a4d972..49d4559d0f 100644 --- a/common/templates/course_modes/choose.html +++ b/common/templates/course_modes/choose.html @@ -5,272 +5,145 @@ <%block name="bodyclass">register verification-process step-select-track ${'is-upgrading' if upgrade else ''} <%block name="pagetitle"> - %if upgrade: - ${_("Upgrade Your Registration for {} | Choose Your Track").format(course_name)} - %else: - ${_("Register for {} | Choose Your Track").format(course_name)} + % if upgrade: + ${_("Upgrade Your Registration for {} | Choose Your Track").format(course_name)} + % else: + ${_("Register for {} | Choose Your Track").format(course_name)} %endif <%block name="js_extra"> - + }); + <%block name="content"> - -%if error: -
-
- -
-

${_("Sorry, there was an error when trying to register you")}

-
-

${error}

-
-
-
-
-%endif - -
-
- - <%include file="/verify_student/_verification_header.html" args="course_name=course_name" /> - -## TODO (ECOM-16): This is part of an AB-test of auto-registration. -## Once the test completes, we can make the winning configuration the default -## and remove this flag. - %if not autoreg: - -
-
- - %if not upgrade: -

${_("Select your track:")}

- %endif - -
- - % if "verified" in modes: -
-
- - %if verified_name == "Verified Certificate": -

${_("Certificate of Achievement (ID Verified)")}

- %else: -

${verified_name}

- %endif - - %if verified_name == "Verified Certificate": - %if upgrade: + % if error: +
+
+ +
+

${_("Sorry, there was an error when trying to register you")}

-

${_("Upgrade and work toward a verified Certificate of Achievement.")}

+

${error}

- %else: -
-

${_("Sign up and work toward a verified Certificate of Achievement.")}

-
- %endif - %else: -
-

${verified_description}

-
- %endif -
- -
-
${_("Select your contribution for this course (min. $")} ${min_price} ${currency}${_("):")}
- - %if error: -
-
-

${error}

-
- %endif - - <%include file="_contribution.html" args="suggested_prices=suggested_prices, currency=currency, chosen_price=chosen_price, min_price=min_price"/>
- -
    -
  • - %if upgrade: - - %else: - - %endif - -
    • -
-
- -
-

${_("Verified Registration Requirements")}

- - %if upgrade: -
-

${_("To upgrade your registration and work towards a Verified Certificate of Achievement, you will need a webcam, a credit or debit card, and an ID.")}

-
- %else: -
-

${_("To register for a Verified Certificate of Achievement option, you will need a webcam, a credit or debit card, and an ID.")}

-
- %endif - -

${_("What is an ID Verified Certificate?")}

-
-

${_("An ID Verified Certificate requires proof of your identity through your photo and ID and is checked throughout the course to verify that it is you who earned the passing grade.")}

-
-
- % endif - - %if not upgrade: - - % if "honor" in modes: - - ${_("or")} - -
-
- -

${_("Audit This Course")}

-
-

${_("Audit this course for free and have complete access to all the course material, activities, tests, and forums. If your work is satisfactory and you abide by the Honor Code, you'll receive a personalized Honor Code Certificate to showcase your achievement.")}

-
-
- -
    -
  • - -
    • -
-
- % endif - - %endif - - - -
-
- - %else: - -
-
- - %if not upgrade: -

${_("Now choose your course track:")}

- %endif - -
- - % if "verified" in modes: -
-
- -

${_("Pursue a Verified Certificate")}

- - %if upgrade: -
-

${_("Plan to use your completed coursework for job applications, career advancement, or school applications? Upgrade to work toward a Verified Certificate of Achievement to document your accomplishment. A minimum fee applies.")}

-
- %else: -
-

${_("Plan to use your completed coursework for job applications, career advancement, or school applications? Then work toward a Verified Certificate of Achievement to document your accomplishment. A minimum fee applies.")}

-
- %endif -
- -
-
${_("Select your contribution for this course (min. $")} ${min_price} ${currency}${_("):")}
- - %if error: -
-
-

${error}

-
-
- %endif - - <%include file="_contribution.html" args="suggested_prices=suggested_prices, currency=currency, chosen_price=chosen_price, min_price=min_price"/> - -
    -
  • - %if upgrade: - - %else: - - %endif - -
    • -
-
-
- % endif - - %if not upgrade: - % if "honor" in modes: - - ${_("or")} - -
-
- -

${_("Audit This Course")}

-
-

${_("Audit this course for free and have complete access to all the course material, activities, tests, and forums. If your work is satisfactory and you abide by the Honor Code, you'll receive a personalized Honor Code Certificate to showcase your achievement.")}

-
-
- -
    -
  • - -
    • -
-
- - % endif - - %endif - - -
-
-
+
%endif - <%include file="/verify_student/_verification_support.html" /> - - +
+
+ <%include file="/verify_student/_verification_header.html" args="course_name=course_name" /> + +
+
+ % if not upgrade: +

${_("Now choose your course track:")}

+ % endif + +
+ % if "verified" in modes: +
+
+ +

${_("Pursue a Verified Certificate")}

+ + % if upgrade: +
+

${_("Plan to use your completed coursework for job applications, career advancement, or school applications? Upgrade to work toward a Verified Certificate of Achievement to document your accomplishment. A minimum fee applies.")}

+
+ % else: +
+

${_("Plan to use your completed coursework for job applications, career advancement, or school applications? Then work toward a Verified Certificate of Achievement to document your accomplishment. A minimum fee applies.")}

+
+ % endif +
+ +
+
${_("Select your contribution for this course (min. $")} ${min_price} ${currency}${_("):")}
+ + % if error: +
+
+

${error}

+
+
+ % endif + + <%include file="_contribution.html" args="suggested_prices=suggested_prices, currency=currency, chosen_price=chosen_price, min_price=min_price"/> + +
    +
  • + % if upgrade: + + % else: + + % endif +
    • +
+
+
+ % endif + + % if not upgrade: + % if "honor" in modes: + + ${_("or")} + + +
+
+ +

${_("Audit This Course")}

+
+

${_("Audit this course for free and have complete access to all the course material, activities, tests, and forums. If your work is satisfactory and you abide by the Honor Code, you'll receive a personalized Honor Code Certificate to showcase your achievement.")}

+
+
+ +
    +
  • + +
    • +
+
+ % endif + % endif + + +
+
+
+ + <%include file="/verify_student/_verification_support.html" /> +
+
diff --git a/lms/djangoapps/courseware/features/certificates.py b/lms/djangoapps/courseware/features/certificates.py index 6890270a23..5e5d0051cc 100644 --- a/lms/djangoapps/courseware/features/certificates.py +++ b/lms/djangoapps/courseware/features/certificates.py @@ -61,7 +61,7 @@ def select_contribution(amount=32): def click_verified_track_button(): world.wait_for_ajax_complete() - btn_css = 'input[value="Select Certificate"]' + btn_css = 'input[value="Pursue a Verified Certificate"]' world.css_click(btn_css) diff --git a/lms/djangoapps/verify_student/tests/test_integration.py b/lms/djangoapps/verify_student/tests/test_integration.py index 5c69b911d0..9d47222cfe 100644 --- a/lms/djangoapps/verify_student/tests/test_integration.py +++ b/lms/djangoapps/verify_student/tests/test_integration.py @@ -99,12 +99,7 @@ class TestProfEdVerification(ModuleStoreTestCase): # On the verified page, expect that there's a link to payment page self.assertContains(resp, '/shoppingcart/payment_fake') - def test_do_not_auto_register(self): - # TODO (ECOM-16): Remove once we complete the AB-test of auto-registration. - session = self.client.session - session['auto_register'] = True - session.save() - + def test_do_not_auto_enroll(self): # Go to the course mode page, expecting a redirect # to the show requirements page. resp = self.client.get(self.urls['course_modes_choose'], follow=True) diff --git a/lms/djangoapps/verify_student/tests/test_views.py b/lms/djangoapps/verify_student/tests/test_views.py index 6ce29b5d08..c19ccf80ea 100644 --- a/lms/djangoapps/verify_student/tests/test_views.py +++ b/lms/djangoapps/verify_student/tests/test_views.py @@ -191,7 +191,7 @@ class TestVerifyView(ModuleStoreTestCase): kwargs={"course_id": unicode(self.course_key)}) response = self.client.get(url) - self.assertIn("You are registering for", response.content) + self.assertIn("You are now registered to audit", response.content) def test_valid_course_upgrade_text(self): url = reverse('verify_student_verify', diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 04e1d4c809..8deb474079 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -113,9 +113,6 @@ class VerifyView(View): "upgrade": upgrade == u'True', "can_audit": CourseMode.mode_for_course(course_id, 'audit') is not None, "modes_dict": CourseMode.modes_for_course_dict(course_id), - - # TODO (ECOM-16): Remove once the AB test completes - "autoreg": request.session.get('auto_register', False), "retake": request.GET.get('retake', False), } @@ -166,9 +163,6 @@ class VerifiedView(View): "upgrade": upgrade == u'True', "can_audit": "audit" in modes_dict, "modes_dict": modes_dict, - - # TODO (ECOM-16): Remove once the AB test completes - "autoreg": request.session.get('auto_register', False), } return render_to_response('verify_student/verified.html', context) @@ -358,9 +352,6 @@ def show_requirements(request, course_id): "is_not_active": not request.user.is_active, "upgrade": upgrade == u'True', "modes_dict": modes_dict, - - # TODO (ECOM-16): Remove once the AB test completes - "autoreg": request.session.get('auto_register', False), } return render_to_response("verify_student/show_requirements.html", context) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 990803b41a..8f72b7ae0b 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -467,9 +467,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)) - # REGISTRATION CODES DISPLAY INFORMATION INVOICE_CORP_ADDRESS = ENV_TOKENS.get('INVOICE_CORP_ADDRESS', INVOICE_CORP_ADDRESS) INVOICE_PAYMENT_INSTRUCTIONS = ENV_TOKENS.get('INVOICE_PAYMENT_INSTRUCTIONS', INVOICE_PAYMENT_INSTRUCTIONS) diff --git a/lms/envs/common.py b/lms/envs/common.py index c5dea04781..695b0808b8 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1817,24 +1817,6 @@ ANALYTICS_DATA_TOKEN = "" ANALYTICS_DASHBOARD_URL = "" ANALYTICS_DASHBOARD_NAME = PLATFORM_NAME + " Insights" -# 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", -]) - # REGISTRATION CODES DISPLAY INFORMATION SUBTITUTIONS IN THE INVOICE ATTACHMENT INVOICE_CORP_ADDRESS = "Please place your corporate address\nin this configuration" INVOICE_PAYMENT_INSTRUCTIONS = "This is where you can\nput directions on how people\nbuying registration codes" diff --git a/lms/static/sass/views/_verification.scss b/lms/static/sass/views/_verification.scss index 504f7b5565..3945876888 100644 --- a/lms/static/sass/views/_verification.scss +++ b/lms/static/sass/views/_verification.scss @@ -355,12 +355,13 @@ // UI: page header .page-header { width: flex-grid(12,12); - margin: 0 0 ($baseline/2) 0; - border-bottom: ($baseline/4) solid $m-gray-l4; + margin-bottom: 0; + border-bottom: none; .title { @include clearfix(); width: flex-grid(12,12); + margin: 0; .sts-course, .sts-track { display: inline-block; @@ -393,96 +394,51 @@ } } - .sts-label { - @extend %t-title7; + .sts-label, .sts-course-org, .sts-course-number, .sts-course-name { + @extend %t-title5; @extend %t-weight4; - display: block; - margin-bottom: ($baseline/2); - border-bottom: ($baseline/10) solid $m-gray-l4; - padding-bottom: ($baseline/2); - color: $m-gray-d1; - } - - .sts-course { - @extend %t-title; - width: flex-grid(9,12); + @include font-size(14); + @include line-height(14); + display: inline-block; + color: $gray; text-transform: none; } - .sts-course-org, .sts-course-number { - @extend %t-title5; - @extend %t-weight4; - display: inline-block; + .sts-label { + margin: 0; + border: none; + padding: 0; + } + + .sts-course { + width: initial; } .sts-course-org { - margin-right: ($baseline/4); + margin-right: 0; } - .sts-course-name { - @include font-size(28); - @include line-height(28); - @extend %t-weight4; - display: block; - } - } - } - - // CASE: page header - experiment variant A overrides - .page-header.exp-variant-A { - margin-bottom: 0; - border-bottom: none; - - .title { - margin: 0; - } - - - .sts-label { - display: inline-block; - margin: 0; - border: none; - padding: 0; - text-transform: none; - } - - .sts-course-org { - margin-right: 0; - } - - .sts-label, .sts-course-org, .sts-course-number, .sts-course-name { - @extend %t-title5; - @extend %t-weight4; - @include font-size(14); - @include line-height(14); - display: inline-block; - color: $gray; - text-transform: none; - } - - .wrapper-sts { - display: inline-block; - width: flex-grid(9,12); - margin-bottom: ($baseline/4); - } - - .sts-course { - width: initial; - } - - .title .sts-track { - display: inline-block; - - .sts-track-value { - background: $verified-color-lvl3; + .wrapper-sts { + display: inline-block; + width: flex-grid(9,12); + margin-bottom: ($baseline/4); } - &.professional-ed { + .title .sts-track { + display: inline-block; .sts-track-value { - background-color: $professional-color-lvl1; + background: $verified-color-lvl3; + } + + &.professional-ed { + + .sts-track-value { + background-color: $professional-color-lvl1; + } } } + } } @@ -1150,10 +1106,6 @@ // ==================== // UI: main content - .wrapper-content-main { - - } - .content-main { width: flex-grid(12,12); @@ -1188,20 +1140,16 @@ margin-right: flex-gutter(); &:last-child { - margin-right: 0 + margin-right: 0; + } + + &.help-item-technical { + width: flex-grid(8,12); } } } } - // CASE: supplemental content - experiment variant A overrides - .wrapper-content-supplementary.exp-variant-A { - - .help-item-technical { - width: flex-grid(8,12); - } - } - // ==================== // VIEW: select a track @@ -1217,7 +1165,7 @@ margin: ($baseline*2) 0; .deco-divider { - width: flex-grid(8,12); + width: flex-grid(12,12); float: left; } } @@ -1227,7 +1175,7 @@ } .register-choice { - width: flex-grid(8,12); + width: flex-grid(12,12); margin: 0 flex-gutter() $baseline 0; border-top: ($baseline/4) solid $m-gray-d4; padding: $baseline ($baseline*1.5); @@ -1287,7 +1235,7 @@ .list-actions { width: flex-grid(8,8); - margin: ($baseline/3) 0; + margin: ($baseline) 0; } .action-select input { @@ -1310,7 +1258,10 @@ } .list-actions { - margin: ($baseline/3) 0; + margin: ($baseline/4) 0; + border-top: none; + width: flex-grid(4,12); + float: right; } .action-intro, .action-select { @@ -1325,7 +1276,7 @@ } .action-select { - width: flex-grid(5,8); + width: initial; } .action-select input { @@ -1382,7 +1333,13 @@ .contribution-options { @include clearfix(); - margin: $baseline 0; + margin: 0; + width: flex-grid(8,12); + + &:after{ + clear: none; + display: none; + } .field { float: left; @@ -1425,39 +1382,6 @@ } } - // CASE: select a track - experiment variant A overrides - .wrapper-register-choose.exp-variant-A { - - .register-choice { - width: flex-grid(12,12); - } - - .deco-divider{ - width: flex-grid(12,12); - } - - .contribution-options { - width: flex-grid(8,12); - margin: 0; - - &:after{ - clear: none; - display: none; - } - } - - .register-choice-certificate .list-actions { - border-top: none; - width: flex-grid(4,12); - float: right; - margin: ($baseline/4) 0; - - .action-select { - width: initial; - } - } - } - // VIEW: requirements &.step-requirements { diff --git a/lms/templates/verify_student/_verification_header.html b/lms/templates/verify_student/_verification_header.html index 2dd626aa37..82671dd77f 100644 --- a/lms/templates/verify_student/_verification_header.html +++ b/lms/templates/verify_student/_verification_header.html @@ -1,89 +1,40 @@ <%! from django.utils.translation import ugettext as _ %> -## TODO (ECOM-16): This is part of an AB-test of auto-registration. -## Once the test completes, we can make the winning configuration the default -## and remove this flag. -%if not autoreg: -
-

- %if upgrade: - ${_("You are upgrading your registration for")} - %elif reverify: - ${_("You are re-verifying for")} - %elif modes_dict and "professional" in modes_dict: - ${_("You are registering for")} - %else: - ${_("You are registering for")} - %endif - - - - ${course_org} - ${course_num} - ${course_name} - - - %if modes_dict and "professional" in modes_dict: - - - ${_("Professional Education")} +

+ + % if upgrade: + ${_("You are upgrading your registration for")} + % elif reverify: + ${_("You are re-verifying for")} + % elif modes_dict and "professional" in modes_dict: + ${_("You are registering for")} + % else: + ${_("Congrats! You are now registered to audit")} + % endif + ${course_org}'s + ${course_num} + ${course_name} - - %else: - - - %if upgrade: - ${_("Upgrading to:")} ${_("Verified")} - %elif reverify: - ${_("Re-verifying for:")} ${_("Verified")} - %else: - ${_("Registering as: ")} ${_("Verified")} - %endif - - - %endif - -

-

-%else: - -
-

- - %if upgrade: - ${_("You are upgrading your registration for")} - %elif reverify: - ${_("You are re-verifying for")} - %elif modes_dict and "professional" in modes_dict: - ${_("You are registering for")} - %else: - ${_("Congrats! You are now registered to audit")} - %endif - ${course_org}'s - ${course_num} - ${course_name}. - - - %if modes_dict and "professional" in modes_dict: - - - ${_("Professional Education")} - - - %else: - - - %if upgrade: - ${_("Upgrading to:")} ${_("Verified")} - %elif reverify: - ${_("Re-verifying for:")} ${_("Verified")} - %else: - ${_("Registering as: ")} ${_("Verified")} - %endif - - %endif -

+ % if modes_dict and "professional" in modes_dict: + + + ${_("Professional Education")} + + + % else: + + + % if upgrade: + ${_("Upgrading to:")} ${_("Verified")} + % elif reverify: + ${_("Re-verifying for:")} ${_("Verified")} + % else: + ${_("Registering as: ")} ${_("Verified")} + % endif + + + % endif +
-%endif diff --git a/lms/templates/verify_student/_verification_support.html b/lms/templates/verify_student/_verification_support.html index d5f5b5a3da..0da3cd8aa9 100644 --- a/lms/templates/verify_student/_verification_support.html +++ b/lms/templates/verify_student/_verification_support.html @@ -1,65 +1,21 @@ <%! from django.utils.translation import ugettext as _ %> -## TODO (ECOM-16): This is part of an AB-test of auto-registration. -## Once the test completes, we can make the winning configuration the default -## and remove this flag. -%if not autoreg: -
-
- -%else: - -
- -
-%endif diff --git a/lms/urls.py b/lms/urls.py index 4f746e6fe6..d4f0bf7dc6 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -241,17 +241,6 @@ if settings.COURSEWARE_ENABLED: 'student.views.change_enrollment', name="change_enrollment"), url(r'^change_email_settings$', 'student.views.change_email_settings', name="change_email_settings"), - # Used for an AB-test of auto-registration - # TODO (ECOM-16): Based on the AB-test, update the default behavior and change - # this URL to point to the original view. Eventually, this URL - # should be removed, but not the AB test completes. - url( - r'^change_enrollment_autoreg$', - 'student.views.change_enrollment', - {'auto_register': True}, - name="change_enrollment_autoreg", - ), - #About the course url(r'^courses/{}/about$'.format(settings.COURSE_ID_PATTERN), 'courseware.views.course_about', name="about_course"),