Merge pull request #5527 from mulby/tweak-segment-io-endpoint

Small changes to the segment.io event handler

common/djangoapps/track/views/segmentio.py

@@ -93,14 +93,19 @@ def track_segmentio_event(request):
     # The POST body will contain the JSON encoded event
     full_segment_event = request.json
 
+    # We mostly care about the properties
+    segment_event = full_segment_event.get('properties', {})
+
     def logged_failure_response(*args, **kwargs):
         """Indicate a failure and log information about the event that will aide debugging efforts"""
         failed_response = failure_response(*args, **kwargs)
         log.warning('Unable to process event received from segment.io: %s', json.dumps(full_segment_event))
         return failed_response
 
-    # Selectively listen to particular channels
-    channel = full_segment_event.get('channel')
+    # Selectively listen to particular channels, note that the client can set the "event_source" field in the
+    # "properties" dict to override the channel provided by segment.io. This is necessary because there is a bug in some
+    # segment.io client libraries that prevented them from sending correct channel fields.
+    channel = segment_event.get('event_source')
     allowed_channels = [c.lower() for c in getattr(settings, 'TRACKING_SEGMENTIO_ALLOWED_CHANNELS', [])]
     if not channel or channel.lower() not in allowed_channels:
         return response(WARNING_IGNORED_CHANNEL, committed=False)
@@ -111,15 +116,15 @@ def track_segmentio_event(request):
     if not action or action.lower() not in allowed_actions:
         return response(WARNING_IGNORED_ACTION, committed=False)
 
-    # We mostly care about the properties
-    segment_event = full_segment_event.get('properties', {})
-
     context = {}
 
     # Start with the context provided by segment.io in the "client" field if it exists
     segment_context = full_segment_event.get('context')
     if segment_context:
         context['client'] = segment_context
+        user_agent = segment_context.get('userAgent', '')
+    else:
+        user_agent = ''
 
     # Overlay any context provided in the properties
     context.update(segment_event.get('context', {}))
@@ -136,7 +141,7 @@ def track_segmentio_event(request):
     except ValueError:
         return logged_failure_response(ERROR_INVALID_USER_ID)
     else:
-        context['user_id'] = user_id
+        context['user_id'] = user.id
 
     # course_id is expected to be provided in the context when applicable
     course_id = context.get('course_id')
@@ -173,6 +178,7 @@ def track_segmentio_event(request):
         event = {
             "username": user.username,
             "event_type": event_type,
+            "name": segment_event.get('name', ''),
             # Will be either "mobile", "browser" or "server". These names happen to be identical to the names we already
             # use so no mapping is necessary.
             "event_source": channel,
@@ -181,7 +187,7 @@ def track_segmentio_event(request):
             "context": complete_context,
             "page": segment_event.get('page'),
             "host": complete_context.get('host', ''),
-            "agent": '',
+            "agent": user_agent,
             "ip": segment_event.get('ip', ''),
             "event": segment_event.get('event', {}),
         }

common/djangoapps/track/views/tests/test_segmentio.py

@@ -40,7 +40,7 @@ class SegmentIOTrackingTestCase(TestCase):
         self.mock_tracker = patcher.start()
         self.addCleanup(patcher.stop)
 
-    def test_segmentio_tracking_get_request(self):
+    def test_get_request(self):
         request = self.request_factory.get(ENDPOINT)
         response = segmentio.track_segmentio_event(request)
         self.assertEquals(response.status_code, 405)
@@ -49,7 +49,7 @@ class SegmentIOTrackingTestCase(TestCase):
     @override_settings(
         TRACKING_SEGMENTIO_WEBHOOK_SECRET=None
     )
-    def test_segmentio_tracking_no_secret_config(self):
+    def test_no_secret_config(self):
         request = self.request_factory.post(ENDPOINT)
         response = segmentio.track_segmentio_event(request)
         self.assert_segmentio_uncommitted_response(response, segmentio.ERROR_UNAUTHORIZED, 401)
@@ -61,12 +61,12 @@ class SegmentIOTrackingTestCase(TestCase):
         self.assertEquals(parsed_content, {'committed': False, 'message': expected_message})
         self.assertFalse(self.mock_tracker.send.called)  # pylint: disable=maybe-no-member
 
-    def test_segmentio_tracking_no_secret_provided(self):
+    def test_no_secret_provided(self):
         request = self.request_factory.post(ENDPOINT)
         response = segmentio.track_segmentio_event(request)
         self.assert_segmentio_uncommitted_response(response, segmentio.ERROR_UNAUTHORIZED, 401)
 
-    def test_segmentio_tracking_secret_mismatch(self):
+    def test_secret_mismatch(self):
         request = self.create_request(key='y')
         response = segmentio.track_segmentio_event(request)
         self.assert_segmentio_uncommitted_response(response, segmentio.ERROR_UNAUTHORIZED, 401)
@@ -93,7 +93,7 @@ class SegmentIOTrackingTestCase(TestCase):
 
     @data('server', 'browser', 'Browser')
     def test_segmentio_ignore_channels(self, channel):
-        response = self.post_segmentio_event(channel=channel)
+        response = self.post_segmentio_event(event_source=channel)
         self.assert_segmentio_uncommitted_response(response, segmentio.WARNING_IGNORED_CHANNEL, 200)
 
     def create_segmentio_event(self, **kwargs):
@@ -104,17 +104,20 @@ class SegmentIOTrackingTestCase(TestCase):
             "event": "Did something",
             "properties": {
                 'event_type': kwargs.get('event_type', ''),
+                'event_source': kwargs.get('event_source', 'mobile'),
                 'event': kwargs.get('event', {}),
                 'context': {
                     'course_id': kwargs.get('course_id') or '',
-                }
+                },
+                'name': str(sentinel.name),
             },
             "channel": kwargs.get('channel', 'mobile'),
             "context": {
                 "library": {
                     "name": "unknown",
                     "version": "unknown"
-                }
+                },
+                'userAgent': str(sentinel.user_agent),
             },
             "receivedAt": "2014-08-27T16:33:39.100Z",
             "timestamp": "2014-08-27T16:33:39.215Z",
@@ -129,22 +132,23 @@ class SegmentIOTrackingTestCase(TestCase):
             },
             "action": action
         }
+
         return sample_event
 
     def create_segmentio_event_json(self, **kwargs):
         """Return a json string containing a fake segment.io event"""
         return json.dumps(self.create_segmentio_event(**kwargs))
 
-    def test_segmentio_tracking_no_user_for_user_id(self):
+    def test_no_user_for_user_id(self):
         response = self.post_segmentio_event(user_id=40)
         self.assert_segmentio_uncommitted_response(response, segmentio.ERROR_USER_NOT_EXIST, 400)
 
-    def test_segmentio_tracking_invalid_user_id(self):
+    def test_invalid_user_id(self):
         response = self.post_segmentio_event(user_id='foobar')
         self.assert_segmentio_uncommitted_response(response, segmentio.ERROR_INVALID_USER_ID, 400)
 
     @data('foo/bar/baz', 'course-v1:foo+bar+baz')
-    def test_segmentio_tracking(self, course_id):
+    def test_success(self, course_id):
         middleware = TrackMiddleware()
 
         request = self.create_request(
@@ -165,8 +169,9 @@ class SegmentIOTrackingTestCase(TestCase):
                 'ip': '',
                 'event_source': 'mobile',
                 'event_type': str(sentinel.event_type),
+                'name': str(sentinel.name),
                 'event': {'foo': 'bar'},
-                'agent': '',
+                'agent': str(sentinel.user_agent),
                 'page': None,
                 'time': datetime.strptime("2014-08-27T16:33:39.215Z", "%Y-%m-%dT%H:%M:%S.%fZ"),
                 'host': 'testserver',
@@ -179,7 +184,8 @@ class SegmentIOTrackingTestCase(TestCase):
                         'library': {
                             'name': 'unknown',
                             'version': 'unknown'
-                        }
+                        },
+                        'userAgent': str(sentinel.user_agent)
                     },
                     'received_at': datetime.strptime("2014-08-27T16:33:39.100Z", "%Y-%m-%dT%H:%M:%S.%fZ"),
                 },
@@ -189,7 +195,7 @@ class SegmentIOTrackingTestCase(TestCase):
 
         self.mock_tracker.send.assert_called_once_with(expected_event)  # pylint: disable=maybe-no-member
 
-    def test_segmentio_tracking_invalid_course_id(self):
+    def test_invalid_course_id(self):
         request = self.create_request(
             data=self.create_segmentio_event_json(course_id='invalid'),
             content_type='application/json'
@@ -199,9 +205,9 @@ class SegmentIOTrackingTestCase(TestCase):
         self.assertEquals(response.status_code, 200)
         self.assertTrue(self.mock_tracker.send.called)  # pylint: disable=maybe-no-member
 
-    def test_segmentio_tracking_missing_event_type(self):
+    def test_missing_event_type(self):
         sample_event_raw = self.create_segmentio_event()
-        sample_event_raw['properties'] = {}
+        del sample_event_raw['properties']['event_type']
         request = self.create_request(
             data=json.dumps(sample_event_raw),
             content_type='application/json'
@@ -211,7 +217,7 @@ class SegmentIOTrackingTestCase(TestCase):
         response = segmentio.track_segmentio_event(request)
         self.assert_segmentio_uncommitted_response(response, segmentio.ERROR_MISSING_EVENT_TYPE, 400)
 
-    def test_segmentio_tracking_missing_timestamp(self):
+    def test_missing_timestamp(self):
         sample_event_raw = self.create_event_without_fields('timestamp')
         request = self.create_request(
             data=json.dumps(sample_event_raw),
@@ -232,7 +238,7 @@ class SegmentIOTrackingTestCase(TestCase):
 
         return event
 
-    def test_segmentio_tracking_missing_received_at(self):
+    def test_missing_received_at(self):
         sample_event_raw = self.create_event_without_fields('receivedAt')
         request = self.create_request(
             data=json.dumps(sample_event_raw),
@@ -242,3 +248,18 @@ class SegmentIOTrackingTestCase(TestCase):
 
         response = segmentio.track_segmentio_event(request)
         self.assert_segmentio_uncommitted_response(response, segmentio.ERROR_MISSING_RECEIVED_AT, 400)
+
+    def test_string_user_id(self):
+        User.objects.create(pk=USER_ID, username=str(sentinel.username))
+        response = self.post_segmentio_event(user_id=str(USER_ID))
+        result = self.assert_segmentio_committed_response(response)
+        self.assertEquals(result['context']['user_id'], USER_ID)
+
+    def assert_segmentio_committed_response(self, response):
+        """Assert that an event was emitted"""
+        self.assertEquals(response.status_code, 200)
+        parsed_content = json.loads(response.content)
+        self.assertEquals(parsed_content, {'committed': True})
+        self.assertTrue(self.mock_tracker.send.called)  # pylint: disable=maybe-no-member
+        return self.mock_tracker.send.mock_calls[0][1][0]
+