If feature flag is enabled, replace the old login/registration pages with the…

If feature flag is enabled, replace the old login/registration pages with the new combined login/registration page.

common/djangoapps/third_party_auth/pipeline.py

@@ -111,10 +111,10 @@ AUTH_ENTRY_LOGIN = 'login'
 AUTH_ENTRY_PROFILE = 'profile'
 AUTH_ENTRY_REGISTER = 'register'
 
-# pylint: disable=fixme
-# TODO (ECOM-369): Replace `AUTH_ENTRY_LOGIN` and `AUTH_ENTRY_REGISTER`
-# with these values once the A/B test completes, then delete
-# these constants.
+# This is left-over from an A/B test
+# of the new combined login/registration page (ECOM-369)
+# We need to keep both the old and new entry points
+# until every session from before the test ended has expired.
 AUTH_ENTRY_LOGIN_2 = 'account_login'
 AUTH_ENTRY_REGISTER_2 = 'account_register'
 
@@ -129,12 +129,13 @@ AUTH_ENTRY_API = 'api'
 # to load that depend on this module.
 AUTH_DISPATCH_URLS = {
     AUTH_ENTRY_DASHBOARD: '/dashboard',
-    AUTH_ENTRY_LOGIN: '/login',
-    AUTH_ENTRY_REGISTER: '/register',
+    AUTH_ENTRY_LOGIN: '/account/login',
+    AUTH_ENTRY_REGISTER: '/account/register',
 
-    # TODO (ECOM-369): Replace the dispatch URLs
-    # for `AUTH_ENTRY_LOGIN` and `AUTH_ENTRY_REGISTER`
-    # with these values, but DO NOT DELETE THESE KEYS.
+    # This is left-over from an A/B test
+    # of the new combined login/registration page (ECOM-369)
+    # We need to keep both the old and new entry points
+    # until every session from before the test ended has expired.
     AUTH_ENTRY_LOGIN_2: '/account/login/',
     AUTH_ENTRY_REGISTER_2: '/account/register/',
 
@@ -150,11 +151,10 @@ _AUTH_ENTRY_CHOICES = frozenset([
     AUTH_ENTRY_PROFILE,
     AUTH_ENTRY_REGISTER,
 
-    # TODO (ECOM-369): For the A/B test of the combined
-    # login/registration, we needed to introduce two
-    # additional end-points.  Once the test completes,
-    # delete these constants from the choices list.
-    # pylint: disable=fixme
+    # This is left-over from an A/B test
+    # of the new combined login/registration page (ECOM-369)
+    # We need to keep both the old and new entry points
+    # until every session from before the test ended has expired.
     AUTH_ENTRY_LOGIN_2,
     AUTH_ENTRY_REGISTER_2,
 
@@ -437,31 +437,16 @@ def parse_query_params(strategy, response, *args, **kwargs):
         # Whether the auth pipeline entered from /dashboard.
         'is_dashboard': auth_entry == AUTH_ENTRY_DASHBOARD,
         # Whether the auth pipeline entered from /login.
-        'is_login': auth_entry == AUTH_ENTRY_LOGIN,
+        'is_login': auth_entry in [AUTH_ENTRY_LOGIN, AUTH_ENTRY_LOGIN_2],
         # Whether the auth pipeline entered from /register.
-        'is_register': auth_entry == AUTH_ENTRY_REGISTER,
+        'is_register': auth_entry in [AUTH_ENTRY_REGISTER, AUTH_ENTRY_REGISTER_2],
         # Whether the auth pipeline entered from /profile.
         'is_profile': auth_entry == AUTH_ENTRY_PROFILE,
         # Whether the auth pipeline entered from an API
         'is_api': auth_entry == AUTH_ENTRY_API,
-
-        # TODO (ECOM-369): Delete these once the A/B test
-        # for the combined login/registration form completes.
-        # pylint: disable=fixme
-        'is_login_2': auth_entry == AUTH_ENTRY_LOGIN_2,
-        'is_register_2': auth_entry == AUTH_ENTRY_REGISTER_2,
     }
 
 
-# TODO (ECOM-369): Once the A/B test of the combined login/registration
-# form completes, we will be able to remove the extra login/registration
-# end-points.  HOWEVER, users who used the new forms during the A/B
-# test may still have values for "is_login_2" and "is_register_2"
-# in their sessions.  For this reason, we need to continue accepting
-# these kwargs in `redirect_to_supplementary_form`, but
-# these should redirect to the same location as "is_login" and "is_register"
-# (whichever login/registration end-points win in the test).
-# pylint: disable=fixme
 @partial.partial
 def ensure_user_information(
     strategy,
@@ -497,36 +482,23 @@ def ensure_user_information(
     # invariants have been violated and future misbehavior is likely.
     user_inactive = user and not user.is_active
     user_unset = user is None
-    dispatch_to_login = is_login and (user_unset or user_inactive)
+    dispatch_to_login = (is_login or is_login_2) and (user_unset or user_inactive)
+    dispatch_to_register = (is_register or is_register_2) and user_unset
     reject_api_request = is_api and (user_unset or user_inactive)
 
     if reject_api_request:
         # Content doesn't matter; we just want to exit the pipeline
         return HttpResponseBadRequest()
 
-    # TODO (ECOM-369): Consolidate this with `dispatch_to_login`
-    # once the A/B test completes. # pylint: disable=fixme
-    dispatch_to_login_2 = is_login_2 and (user_unset or user_inactive)
-
     if is_dashboard or is_profile:
         return
 
     if dispatch_to_login:
         return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN], name='signin_user')
 
-    # TODO (ECOM-369): Consolidate this with `dispatch_to_login`
-    # once the A/B test completes. # pylint: disable=fixme
-    if dispatch_to_login_2:
-        return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_LOGIN_2])
-
-    if is_register and user_unset:
+    if dispatch_to_register:
         return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_REGISTER], name='register_user')
 
-    # TODO (ECOM-369): Consolidate this with `is_register`
-    # once the A/B test completes. # pylint: disable=fixme
-    if is_register_2 and user_unset:
-        return redirect(AUTH_DISPATCH_URLS[AUTH_ENTRY_REGISTER_2])
-
 
 @partial.partial
 def set_logged_in_cookie(backend=None, user=None, request=None, is_api=None, *args, **kwargs):

lms/urls.py

@@ -16,8 +16,6 @@ urlpatterns = ('',  # nopep8
     url(r'^request_certificate$', 'certificates.views.request_certificate'),
     url(r'^$', 'branding.views.index', name="root"),   # Main marketing page, or redirect to courseware
     url(r'^dashboard$', 'student.views.dashboard', name="dashboard"),
-    url(r'^login$', 'student.views.signin_user', name="signin_user"),
-    url(r'^register$', 'student.views.register_user', name="register_user"),
 
     url(r'^admin_dashboard$', 'dashboard.views.dashboard'),
 
@@ -31,14 +29,11 @@ urlpatterns = ('',  # nopep8
     url(r'^segmentio/event$', 'track.views.segmentio.segmentio_event'),
     url(r'^t/(?P<template>[^/]*)$', 'static_template_view.views.index'),   # TODO: Is this used anymore? What is STATIC_GRAB?
 
-    url(r'^accounts/login$', 'student.views.accounts_login', name="accounts_login"),
     url(r'^accounts/manage_user_standing', 'student.views.manage_user_standing',
         name='manage_user_standing'),
     url(r'^accounts/disable_account_ajax$', 'student.views.disable_account_ajax',
         name="disable_account_ajax"),
 
-    url(r'^login_ajax$', 'student.views.login_user', name="login"),
-    url(r'^login_ajax/(?P<error>[^/]*)$', 'student.views.login_user'),
     url(r'^logout$', 'student.views.logout_user', name='logout'),
     url(r'^create_account$', 'student.views.create_account', name='create_account'),
     url(r'^activate/(?P<key>[^/]*)$', 'student.views.activate_account', name="activate"),
@@ -78,6 +73,23 @@ urlpatterns = ('',  # nopep8
 
 )
 
+if settings.FEATURES["ENABLE_COMBINED_LOGIN_REGISTRATION"]:
+    # Backwards compatibility with old URL structure, but serve the new views
+    urlpatterns += (
+        url(r'^login$', 'student_account.views.login_and_registration_form', {'initial_mode': 'login'}, name="signin_user"),
+        url(r'^register$', 'student_account.views.login_and_registration_form', {'initial_mode': 'register'}, name="register_user"),
+        url(r'^accounts/login$', 'student_account.views.login_and_registration_form', {'initial_mode': 'login'}, name="accounts_login"),
+    )
+else:
+    # Serve the old views
+    urlpatterns += (
+        url(r'^login$', 'student.views.signin_user', name="signin_user"),
+        url(r'^register$', 'student.views.register_user', name="register_user"),
+        url(r'^accounts/login$', 'student.views.accounts_login', name="accounts_login"),
+        url(r'^login_ajax$', 'student.views.login_user', name="login"),
+        url(r'^login_ajax/(?P<error>[^/]*)$', 'student.views.login_user'),
+    )
+
 if settings.FEATURES["ENABLE_MOBILE_REST_API"]:
     urlpatterns += (
         url(r'^api/mobile/v0.5/', include('mobile_api.urls')),