diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 4ebaed3d08..ac818a23bc 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -340,6 +340,8 @@ def login_user(request): """ AJAX request to log in the user. """ + _parse_analytics_param_for_course_id(request) + third_party_auth_requested = third_party_auth.is_enabled() and pipeline.running(request) first_party_auth_requested = bool(request.POST.get('email')) or bool(request.POST.get('password')) is_user_third_party_authenticated = False @@ -478,6 +480,32 @@ class LoginSessionView(APIView): return super(LoginSessionView, self).dispatch(request, *args, **kwargs) +def _parse_analytics_param_for_course_id(request): + """ If analytics request param is found, parse and add course id as a new request param. """ + # Make a copy of the current POST request to modify. + modified_request = request.POST.copy() + if isinstance(request, HttpRequest): + # Works for an HttpRequest but not a rest_framework.request.Request. + request.POST = modified_request + else: + # The request must be a rest_framework.request.Request. + request._data = modified_request # pylint: disable=protected-access + # Include the course ID if it's specified in the analytics info + # so it can be included in analytics events. + if "analytics" in modified_request: + try: + analytics = json.loads(modified_request["analytics"]) + if "enroll_course_id" in analytics: + modified_request["course_id"] = analytics.get("enroll_course_id") + except (ValueError, TypeError): + set_custom_metric('shim_analytics_course_id', 'parse-error') + log.error( + u"Could not parse analytics object sent to user API: {analytics}".format( + analytics=analytics + ) + ) + + def shim_student_view(view_func, check_logged_in=False): """Create a "shim" view for a view function from the student Django app. @@ -501,7 +529,8 @@ def shim_student_view(view_func, check_logged_in=False): Keyword Args: check_logged_in (boolean): If true, check whether the user successfully - authenticated and if not set the status to 403. + authenticated and if not set the status to 403. This argument is + used to test the shim by skipping this check. Returns: function @@ -509,45 +538,6 @@ def shim_student_view(view_func, check_logged_in=False): """ @wraps(view_func) def _inner(request): # pylint: disable=missing-docstring - # Make a copy of the current POST request to modify. - modified_request = request.POST.copy() - if isinstance(request, HttpRequest): - # Works for an HttpRequest but not a rest_framework.request.Request. - set_custom_metric('shim_request_type', 'traditional') - request.POST = modified_request - else: - set_custom_metric('shim_request_type', 'drf') - # The request must be a rest_framework.request.Request. - request._data = modified_request # pylint: disable=protected-access - - # The login and registration handlers in student view try to change - # the user's enrollment status if these parameters are present. - # Since we want the JavaScript client to communicate directly with - # the enrollment API, we want to prevent the student views from - # updating enrollments. - if "enrollment_action" in modified_request: - set_custom_metric('shim_del_enrollment_action', modified_request["enrollment_action"]) - del modified_request["enrollment_action"] - if "course_id" in modified_request: - set_custom_metric('shim_del_course_id', modified_request["course_id"]) - del modified_request["course_id"] - - # Include the course ID if it's specified in the analytics info - # so it can be included in analytics events. - if "analytics" in modified_request: - try: - analytics = json.loads(modified_request["analytics"]) - if "enroll_course_id" in analytics: - set_custom_metric('shim_analytics_course_id', analytics.get("enroll_course_id")) - modified_request["course_id"] = analytics.get("enroll_course_id") - except (ValueError, TypeError): - set_custom_metric('shim_analytics_course_id', 'parse-error') - log.error( - u"Could not parse analytics object sent to user API: {analytics}".format( - analytics=analytics - ) - ) - # Call the original view to generate a response. # We can safely modify the status code or content # of the response, but to be safe we won't mess diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index 9245c0a3c9..3a6c1b3bf0 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -714,15 +714,26 @@ class LoginSessionViewTest(ApiTestCase): }, ]) - def test_login(self): + @ddt.data(True, False) + @patch('openedx.core.djangoapps.user_authn.views.login.segment') + def test_login(self, include_analytics, mock_segment): # Create a test user UserFactory.create(username=self.USERNAME, email=self.EMAIL, password=self.PASSWORD) - # Login - response = self.client.post(self.url, { + data = { "email": self.EMAIL, "password": self.PASSWORD, - }) + } + if include_analytics: + track_label = "edX/DemoX/Fall" + data.update({ + "analytics": json.dumps({"enroll_course_id": track_label}) + }) + else: + track_label = None + + # Login + response = self.client.post(self.url, data) self.assertHttpOK(response) # Verify that we logged in successfully by accessing @@ -730,6 +741,19 @@ class LoginSessionViewTest(ApiTestCase): response = self.client.get(reverse("dashboard")) self.assertHttpOK(response) + # Verify events are called + expected_user_id = 1 + mock_segment.identify.assert_called_once_with( + expected_user_id, + {'username': self.USERNAME, 'email': self.EMAIL}, + {'MailChimp': False} + ) + mock_segment.track.assert_called_once_with( + expected_user_id, + 'edx.bi.user.account.authenticated', + {'category': 'conversion', 'provider': None, 'label': track_label} + ) + def test_session_cookie_expiry(self): # Create a test user UserFactory.create(username=self.USERNAME, email=self.EMAIL, password=self.PASSWORD) @@ -793,29 +817,6 @@ class StudentViewShimTest(TestCase): super(StudentViewShimTest, self).setUp() self.captured_request = None - def test_strip_enrollment_action(self): - view = self._shimmed_view(HttpResponse()) - request = HttpRequest() - request.POST["enrollment_action"] = "enroll" - request.POST["course_id"] = "edx/101/demo" - view(request) - - # Expect that the enrollment action and course ID - # were stripped out before reaching the wrapped view. - self.assertNotIn("enrollment_action", self.captured_request.POST) - self.assertNotIn("course_id", self.captured_request.POST) - - def test_include_analytics_info(self): - view = self._shimmed_view(HttpResponse()) - request = HttpRequest() - request.POST["analytics"] = json.dumps({ - "enroll_course_id": "edX/DemoX/Fall" - }) - view(request) - - # Expect that the analytics course ID was passed to the view - self.assertEqual(self.captured_request.POST.get("course_id"), "edX/DemoX/Fall") - def test_third_party_auth_login_failure(self): view = self._shimmed_view( HttpResponse(status=403),