Merge pull request #368 from edx/bderusha/boardman-traffic

Add IP to Segment tracking calls

ecommerce/extensions/analytics/utils.py

@@ -13,13 +13,13 @@ def is_segment_configured():
 
 
 def parse_tracking_context(user):
-    """Extract user and client IDs from a user's tracking context.
+    """Extract user ID, client ID, and IP address from a user's tracking context.
 
     Arguments:
         user (User): An instance of the User model.
 
     Returns:
-        Tuple of strings, user_tracking_id and lms_client_id
+        Tuple of strings: user_tracking_id, lms_client_id, lms_ip
     """
     tracking_context = user.tracking_context or {}
 
@@ -32,8 +32,9 @@ def parse_tracking_context(user):
         user_tracking_id = 'ecommerce-{}'.format(user.id)
 
     lms_client_id = tracking_context.get('lms_client_id')
+    lms_ip = tracking_context.get('lms_ip')
 
-    return user_tracking_id, lms_client_id
+    return user_tracking_id, lms_client_id, lms_ip
 
 
 def silence_exceptions(msg):

ecommerce/extensions/checkout/signals.py

@@ -26,7 +26,7 @@ def track_completed_order(sender, order=None, **kwargs):  # pylint: disable=unus
     if not (is_segment_configured() and order.total_excl_tax > 0):
         return
 
-    user_tracking_id, lms_client_id = parse_tracking_context(order.user)
+    user_tracking_id, lms_client_id, lms_ip = parse_tracking_context(order.user)
 
     analytics.track(
         user_tracking_id,
@@ -51,6 +51,7 @@ def track_completed_order(sender, order=None, **kwargs):  # pylint: disable=unus
             ],
         },
         context={
+            'ip': lms_ip,
             'Google Analytics': {
                 'clientId': lms_client_id
             }

ecommerce/extensions/checkout/tests/test_mixins.py

@@ -77,7 +77,7 @@ class EdxOrderPlacementMixinTests(BusinessIntelligenceMixin, RefundTestMixin, Te
         Ensure that tracking events are fired with correct content when order
         placement event handling is invoked.
         """
-        tracking_context = {'lms_user_id': 'test-user-id', 'lms_client_id': 'test-client-id'}
+        tracking_context = {'lms_user_id': 'test-user-id', 'lms_client_id': 'test-client-id', 'lms_ip': '127.0.0.1'}
         self.user.tracking_context = tracking_context
         self.user.save()
 
@@ -91,6 +91,7 @@ class EdxOrderPlacementMixinTests(BusinessIntelligenceMixin, RefundTestMixin, Te
                 self.order,
                 tracking_context['lms_user_id'],
                 tracking_context['lms_client_id'],
+                tracking_context['lms_ip'],
                 self.order.number,
                 self.order.currency,
                 self.order.total_excl_tax
@@ -131,6 +132,7 @@ class EdxOrderPlacementMixinTests(BusinessIntelligenceMixin, RefundTestMixin, Te
             self.order,
             'ecommerce-{}'.format(self.user.id),
             None,
+            None,
             self.order.number,
             self.order.currency,
             self.order.total_excl_tax

ecommerce/extensions/fulfillment/modules.py

@@ -93,17 +93,22 @@ class EnrollmentFulfillmentModule(BaseFulfillmentModule):
     Allows the enrollment of a student via purchase of a 'seat'.
     """
 
-    def _post_to_enrollment_api(self, data, client_id=None):
+    def _post_to_enrollment_api(self, data, user):
         enrollment_api_url = settings.ENROLLMENT_API_URL
         timeout = settings.ENROLLMENT_FULFILLMENT_TIMEOUT
-
         headers = {
             'Content-Type': 'application/json',
             'X-Edx-Api-Key': settings.EDX_API_KEY
         }
+
+        __, client_id, ip = parse_tracking_context(user)
+
         if client_id:
             headers['X-Edx-Ga-Client-Id'] = client_id
 
+        if ip:
+            headers['X-Forwarded-For'] = ip
+
         return requests.post(enrollment_api_url, data=json.dumps(data), headers=headers, timeout=timeout)
 
     def supports_line(self, line):
@@ -186,8 +191,7 @@ class EnrollmentFulfillmentModule(BaseFulfillmentModule):
                     }
                 )
             try:
-                __, client_id = parse_tracking_context(order.user)
-                response = self._post_to_enrollment_api(data, client_id=client_id)
+                response = self._post_to_enrollment_api(data, user=order.user)
 
                 if response.status_code == status.HTTP_200_OK:
                     line.set_status(LINE.COMPLETE)
@@ -242,8 +246,7 @@ class EnrollmentFulfillmentModule(BaseFulfillmentModule):
                 },
             }
 
-            __, client_id = parse_tracking_context(line.order.user)
-            response = self._post_to_enrollment_api(data, client_id=client_id)
+            response = self._post_to_enrollment_api(data, user=line.order.user)
 
             if response.status_code == status.HTTP_200_OK:
                 audit_log(

ecommerce/extensions/fulfillment/tests/test_modules.py

@@ -74,9 +74,11 @@ class EnrollmentFulfillmentModuleTests(CourseCatalogTestMixin, FulfillmentTestMi
         self.assertEqual(1, len(supported_lines))
 
     @httpretty.activate
-    def test_enrollment_module_fulfill(self):
+    @mock.patch('ecommerce.extensions.fulfillment.modules.parse_tracking_context')
+    def test_enrollment_module_fulfill(self, parse_tracking_context):
         """Happy path test to ensure we can properly fulfill enrollments."""
         httpretty.register_uri(httpretty.POST, settings.ENROLLMENT_API_URL, status=200, body='{}', content_type=JSON)
+        parse_tracking_context.return_value = ('user_123', 'GA-123456789', '11.22.33.44')
         # Attempt to enroll.
         with LogCapture(LOGGER_NAME) as l:
             EnrollmentFulfillmentModule().fulfill_product(self.order, list(self.order.lines.all()))
@@ -101,8 +103,11 @@ class EnrollmentFulfillmentModuleTests(CourseCatalogTestMixin, FulfillmentTestMi
 
         self.assertEqual(LINE.COMPLETE, line.status)
 
-        actual = json.loads(httpretty.last_request().body)
-        expected = {
+        last_request = httpretty.last_request()
+        actual_body = json.loads(last_request.body)
+        actual_headers = last_request.headers
+
+        expected_body = {
             'user': self.order.user.username,
             'is_active': True,
             'mode': self.certificate_type,
@@ -111,7 +116,14 @@ class EnrollmentFulfillmentModuleTests(CourseCatalogTestMixin, FulfillmentTestMi
             },
             'enrollment_attributes': []
         }
-        self.assertEqual(actual, expected)
+
+        expected_headers = {
+            'X-Edx-Ga-Client-Id': 'GA-123456789',
+            'X-Forwarded-For': '11.22.33.44',
+        }
+
+        self.assertDictContainsSubset(expected_headers, actual_headers)
+        self.assertEqual(expected_body, actual_body)
 
     @override_settings(ENROLLMENT_API_URL='')
     def test_enrollment_module_not_configured(self):
@@ -148,9 +160,11 @@ class EnrollmentFulfillmentModuleTests(CourseCatalogTestMixin, FulfillmentTestMi
         self.assertEqual(LINE.FULFILLMENT_SERVER_ERROR, self.order.lines.all()[0].status)
 
     @httpretty.activate
-    def test_revoke_product(self):
+    @mock.patch('ecommerce.extensions.fulfillment.modules.parse_tracking_context')
+    def test_revoke_product(self, parse_tracking_context):
         """ The method should call the Enrollment API to un-enroll the student, and return True. """
         httpretty.register_uri(httpretty.POST, settings.ENROLLMENT_API_URL, status=200, body='{}', content_type=JSON)
+        parse_tracking_context.return_value = ('user_123', 'GA-123456789', '11.22.33.44')
         line = self.order.lines.first()
 
         with LogCapture(LOGGER_NAME) as l:
@@ -172,8 +186,11 @@ class EnrollmentFulfillmentModuleTests(CourseCatalogTestMixin, FulfillmentTestMi
                 )
             )
 
-        actual = json.loads(httpretty.last_request().body)
-        expected = {
+        last_request = httpretty.last_request()
+        actual_body = json.loads(last_request.body)
+        actual_headers = last_request.headers
+
+        expected_body = {
             'user': self.order.user.username,
             'is_active': False,
             'mode': self.certificate_type,
@@ -181,7 +198,14 @@ class EnrollmentFulfillmentModuleTests(CourseCatalogTestMixin, FulfillmentTestMi
                 'course_id': self.course_id,
             },
         }
-        self.assertEqual(actual, expected)
+
+        expected_headers = {
+            'X-Edx-Ga-Client-Id': 'GA-123456789',
+            'X-Forwarded-For': '11.22.33.44',
+        }
+
+        self.assertDictContainsSubset(expected_headers, actual_headers)
+        self.assertEqual(expected_body, actual_body)
 
     @httpretty.activate
     def test_revoke_product_expected_error(self):
@@ -302,6 +326,10 @@ class EnrollmentFulfillmentModuleTests(CourseCatalogTestMixin, FulfillmentTestMi
             'enrollment_attributes': []
         }
 
+        # Create a dummy user and attach the tracking_context
+        user = UserFactory()
+        user.tracking_context = {'lms_user_id': '1', 'lms_client_id': '123.123', 'lms_ip': '11.22.33.44'}
+
         # Now call the enrollment api to send POST request to LMS and verify
         # that the header of the request being sent contains the analytics
         # header 'x-edx-ga-client-id'.
@@ -309,8 +337,9 @@ class EnrollmentFulfillmentModuleTests(CourseCatalogTestMixin, FulfillmentTestMi
         # not available for ecommerce tests.
         try:
             # pylint: disable=protected-access
-            EnrollmentFulfillmentModule()._post_to_enrollment_api(data=data, client_id='123.123')
+            EnrollmentFulfillmentModule()._post_to_enrollment_api(data=data, user=user)
         except ConnectionError as exp:
             # Check that the enrollment request object has the analytics header
-            # 'x-edx-ga-client-id'.
+            # 'x-edx-ga-client-id' and 'x-forwarded-for'.
             self.assertEqual(exp.request.headers.get('x-edx-ga-client-id'), '123.123')
+            self.assertEqual(exp.request.headers.get('x-forwarded-for'), '11.22.33.44')

ecommerce/extensions/refund/signals.py

@@ -16,7 +16,7 @@ def track_completed_refund(sender, refund=None, **kwargs):  # pylint: disable=un
     if not (is_segment_configured() and refund.total_credit_excl_tax > 0):
         return
 
-    user_tracking_id, lms_client_id = parse_tracking_context(refund.user)
+    user_tracking_id, lms_client_id, lms_ip = parse_tracking_context(refund.user)
 
     # Ecommerce transaction reversal, performed by emitting an event which is the inverse of an
     # order completion event emitted previously.
@@ -44,6 +44,7 @@ def track_completed_refund(sender, refund=None, **kwargs):  # pylint: disable=un
             ],
         },
         context={
+            'ip': lms_ip,
             'Google Analytics': {
                 'clientId': lms_client_id
             }

ecommerce/extensions/refund/tests/test_signals.py

@@ -21,7 +21,7 @@ class RefundTrackingTests(BusinessIntelligenceMixin, RefundTestMixin, TestCase):
 
     def test_successful_refund_tracking(self, mock_track):
         """Verify that a successfully placed refund is tracked when Segment is enabled."""
-        tracking_context = {'lms_user_id': 'test-user-id', 'lms_client_id': 'test-client-id'}
+        tracking_context = {'lms_user_id': 'test-user-id', 'lms_client_id': 'test-client-id', 'lms_ip': '127.0.0.1'}
         self.refund.user.tracking_context = tracking_context
         self.refund.user.save()
 
@@ -37,6 +37,7 @@ class RefundTrackingTests(BusinessIntelligenceMixin, RefundTestMixin, TestCase):
             self.refund,
             tracking_context['lms_user_id'],
             tracking_context['lms_client_id'],
+            tracking_context['lms_ip'],
             self.order.number,
             self.refund.currency,
             self.refund.total_credit_excl_tax
@@ -68,6 +69,7 @@ class RefundTrackingTests(BusinessIntelligenceMixin, RefundTestMixin, TestCase):
             self.refund,
             'ecommerce-{}'.format(self.user.id),
             None,
+            None,
             self.order.number,
             self.refund.currency,
             self.refund.total_credit_excl_tax

ecommerce/notifications/notifications.py

@@ -22,10 +22,10 @@ def send_notification(user, commtype_code, context):
 
     """
 
-    tracking_id, client_id = parse_tracking_context(user)
+    tracking_id, client_id, ip = parse_tracking_context(user)
 
     tracking_pixel = 'https://www.google-analytics.com/collect?v=1&t=event&ec=email&ea=open&tid={tracking_id}' \
-                     '&cid={client_id}'.format(tracking_id=tracking_id, client_id=client_id)
+                     '&cid={client_id}&uip={ip}'.format(tracking_id=tracking_id, client_id=client_id, ip=ip)
     full_name = user.get_full_name()
     context.update({
         'full_name': full_name,

ecommerce/tests/mixins.py

@@ -171,13 +171,13 @@ class BusinessIntelligenceMixin(object):
     """Provides assertions for test cases validating the emission of business intelligence events."""
 
     def assert_correct_event(
-            self, mock_track, instance, expected_user_id, expected_client_id, order_number, currency, total
+            self, mock_track, instance, expected_user_id, expected_client_id, expected_ip, order_number, currency, total
     ):
         """Check that the tracking context was correctly reflected in the emitted event."""
         (event_user_id, event_name, event_payload), kwargs = mock_track.call_args
         self.assertEqual(event_user_id, expected_user_id)
         self.assertEqual(event_name, 'Completed Order')
-        self.assertEqual(kwargs['context'], {'Google Analytics': {'clientId': expected_client_id}})
+        self.assertEqual(kwargs['context'], {'ip': expected_ip, 'Google Analytics': {'clientId': expected_client_id}})
         self.assert_correct_event_payload(instance, event_payload, order_number, currency, total)
 
     def assert_correct_event_payload(self, instance, event_payload, order_number, currency, total):