From e8d60611deaaeb4ed2a99c58b5a138db53d654dc Mon Sep 17 00:00:00 2001
From: Robert Raposa <rraposa@edx.org>
Date: Wed, 6 Mar 2019 16:24:58 -0500
Subject: [PATCH] Add user_id for username in event data.

In certain use cases, a caller may have the username and not the
user_id. In these cases, we add the user_id since events have
standardized on using the user_id as the unique id of a user.

ARCH-472
---
 common/djangoapps/track/views/__init__.py     | 18 ++++++++
 .../track/views/tests/test_views.py           | 46 ++++++++++++++++++-
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/common/djangoapps/track/views/__init__.py b/common/djangoapps/track/views/__init__.py
index b5cf9ce881..4290701ff2 100644
--- a/common/djangoapps/track/views/__init__.py
+++ b/common/djangoapps/track/views/__init__.py
@@ -4,6 +4,7 @@ import json
 import pytz
 
 from django.contrib.auth.decorators import login_required
+from django.contrib.auth.models import User
 from django.http import HttpResponse
 from django.shortcuts import redirect
 
@@ -50,6 +51,22 @@ def _get_request_value(request, value_name, default=''):
     return default
 
 
+def _add_user_id_for_username(data):
+    """
+    If data contains a username, adds the corresponding user_id to the data.
+
+    In certain use cases, the caller may have the username and not the
+    user_id. This enables us to standardize on user_id in event data,
+    even when the caller only has access to the username.
+    """
+    if data and ('username' in data) and ('user_id' not in data):
+        try:
+            user = User.objects.get(username=data.get('username'))
+            data['user_id'] = user.id
+        except User.DoesNotExist:
+            pass
+
+
 def user_track(request):
     """
     Log when POST call to "event" URL is made by a user.
@@ -68,6 +85,7 @@ def user_track(request):
     if isinstance(data, basestring) and len(data) > 0:
         try:
             data = json.loads(data)
+            _add_user_id_for_username(data)
         except ValueError:
             pass
 
diff --git a/common/djangoapps/track/views/tests/test_views.py b/common/djangoapps/track/views/tests/test_views.py
index f92ce0b03a..5416808b43 100644
--- a/common/djangoapps/track/views/tests/test_views.py
+++ b/common/djangoapps/track/views/tests/test_views.py
@@ -1,5 +1,6 @@
 # pylint: disable=missing-docstring,maybe-no-member
 
+import ddt
 from mock import patch, sentinel
 
 from django.contrib.auth.models import User
@@ -12,8 +13,18 @@ from track.tests import EventTrackingTestCase, FROZEN_TIME
 from openedx.core.lib.tests.assertions.events import assert_event_matches
 
 
+TEST_USERNAME = 'test-username'
+TEST_USER_ID = 1000
+
+
+@ddt.ddt
 class TestTrackViews(EventTrackingTestCase):
 
+    @classmethod
+    def setUpTestData(cls):
+        super(TestTrackViews, cls).setUpTestData()
+        User.objects.create(pk=TEST_USER_ID, username=TEST_USERNAME)
+
     def setUp(self):
         super(TestTrackViews, self).setUp()
 
@@ -74,8 +85,6 @@ class TestTrackViews(EventTrackingTestCase):
         }
         assert_event_matches(expected_event, actual_event)
 
-        views.user_track(request)
-
     def test_user_track_with_empty_event(self):
         request = self.request_factory.get('/event', {
             'page': self.url_with_course,
@@ -100,6 +109,39 @@ class TestTrackViews(EventTrackingTestCase):
         }
         assert_event_matches(expected_event, actual_event)
 
+    @ddt.data(
+        {
+            'event_data': u'{{"username": "{}"}}'.format(TEST_USERNAME),
+            'expected_event_data': {"username": TEST_USERNAME, "user_id": TEST_USER_ID}
+        },
+        {
+            'event_data': u'{"username": "unknown-user"}',
+            'expected_event_data': {"username": "unknown-user"},
+        }
+    )
+    @ddt.unpack
+    def test_user_track_with_username_in_data(self, event_data, expected_event_data):
+        request = self.request_factory.get('/event', {
+            'event': event_data,
+        })
+
+        views.user_track(request)
+
+        actual_event = self.get_event()
+        expected_event = {
+            'context': {
+                'course_id': '',
+                'org_id': '',
+                'event_source': 'browser',
+                'page': '',
+                'username': 'anonymous'
+            },
+            'data': expected_event_data,
+            'timestamp': FROZEN_TIME,
+            'name': 'unknown'
+        }
+        assert_event_matches(expected_event, actual_event)
+
     @override_settings(
         EVENT_TRACKING_PROCESSORS=[{'ENGINE': 'track.shim.LegacyFieldMappingProcessor'}],
     )