in some cases we do want that redirect url

common/djangoapps/student/tests/test_login.py

@@ -11,7 +11,7 @@ from django.test.utils import override_settings
 from django.conf import settings
 from django.core.cache import cache
 from django.core.urlresolvers import reverse, NoReverseMatch
-from django.http import HttpResponseBadRequest
+from django.http import HttpResponseBadRequest, HttpResponse
 from student.tests.factories import UserFactory, RegistrationFactory, UserProfileFactory
 from student.views import _parse_course_id_from_string, _get_course_enrollment_domain
 
@@ -209,11 +209,10 @@ class LoginTest(TestCase):
 
     def test_change_enrollment_400(self):
         """
-        Tests that a 400 in change_enrollment doesn't lead to a 400
-        and in fact just redirects the user to the dashboard
-        without incident.
+        Tests that a 400 in change_enrollment doesn't lead to a 404
+        and in fact just logs in the user without incident
         """
-        # add these post params to trigger a call to change_enrollment
+        # add this post param to trigger a call to change_enrollment
         extra_post_params = {"enrollment_action": "enroll"}
         with patch('student.views.change_enrollment') as mock_change_enrollment:
             mock_change_enrollment.return_value = HttpResponseBadRequest("I am a 400")
@@ -226,6 +225,42 @@ class LoginTest(TestCase):
         self.assertIsNone(response_content["redirect_url"])
         self._assert_response(response, success=True)
 
+    def test_change_enrollment_200_no_redirect(self):
+        """
+        Tests "redirect_url" is None if change_enrollment returns a HttpResponse
+        with no content
+        """
+        # add this post param to trigger a call to change_enrollment
+        extra_post_params = {"enrollment_action": "enroll"}
+        with patch('student.views.change_enrollment') as mock_change_enrollment:
+            mock_change_enrollment.return_value = HttpResponse()
+            response, _ = self._login_response(
+                'test@edx.org',
+                'test_password',
+                extra_post_params=extra_post_params,
+            )
+        response_content = json.loads(response.content)
+        self.assertIsNone(response_content["redirect_url"])
+        self._assert_response(response, success=True)
+
+    def test_change_enrollment_200_redirect(self):
+        """
+        Tests that "redirect_url" is the content of the HttpResponse returned
+        by change_enrollment, if there is content
+        """
+        # add this post param to trigger a call to change_enrollment
+        extra_post_params = {"enrollment_action": "enroll"}
+        with patch('student.views.change_enrollment') as mock_change_enrollment:
+            mock_change_enrollment.return_value = HttpResponse("in/nature/there/is/nothing/melancholy")
+            response, _ = self._login_response(
+                'test@edx.org',
+                'test_password',
+                extra_post_params=extra_post_params,
+            )
+        response_content = json.loads(response.content)
+        self.assertEqual(response_content["redirect_url"], "in/nature/there/is/nothing/melancholy")
+        self._assert_response(response, success=True)
+
     def _login_response(self, email, password, patched_audit_log='student.views.AUDIT_LOG', extra_post_params=None):
         ''' Post the login info '''
         post_params = {'email': email, 'password': password}

common/djangoapps/student/views.py

@@ -543,7 +543,7 @@ def dashboard(request):
 def try_change_enrollment(request):
     """
     This method calls change_enrollment if the necessary POST
-    parameters are present, but does not return anything. It
+    parameters are present, but does not return anything in most cases. It
     simply logs the result or exception. This is usually
     called after a registration or login, as secondary action.
     It should not interrupt a successful registration or login.
@@ -559,6 +559,11 @@ def try_change_enrollment(request):
                     enrollment_response.content
                 )
             )
+            # Hack: since change_enrollment delivers its redirect_url in the content
+            # of its response, we check here that only the 200 codes with content
+            # will return redirect_urls.
+            if enrollment_response.status_code == 200 and enrollment_response.content != '':
+                return enrollment_response.content
         except Exception, e:
             log.exception("Exception automatically enrolling after login: {0}".format(str(e)))