diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py index b3c7e7b655d9e69e7803e02fd4aec4a57041f0a3..b836ad31d6fc0614b1d6fdd9df889972b23554f4 100644 --- a/common/djangoapps/student/tests/test_login.py +++ b/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} diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 7f411ddf5e4f130c3e2067c6307140f6ddfac4b6..ea79450b26e36afc3733d64ca127371b14209ae8 100644 --- a/common/djangoapps/student/views.py +++ b/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)))