From 9603a8c6b9299bcdf4b21a949818f99365b745e0 Mon Sep 17 00:00:00 2001
From: Brian Mesick <bmesick@tcril.org>
Date: Tue, 22 Aug 2023 14:51:35 -0400
Subject: [PATCH] feat!: Do not send events for unknown users

In the past we would create a new UUID for each unknown user. Now we skip them entirely to avoid confusing the downstream database with a new id user per event.
---
 event_routing_backends/__init__.py            |  2 +-
 event_routing_backends/helpers.py             | 44 +++++++++----------
 .../tests/transformers_test_mixin.py          | 10 +++--
 event_routing_backends/tests/test_helpers.py  |  5 ++-
 4 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/event_routing_backends/__init__.py b/event_routing_backends/__init__.py
index cc729ac4..38bca024 100644
--- a/event_routing_backends/__init__.py
+++ b/event_routing_backends/__init__.py
@@ -2,4 +2,4 @@
 Various backends for receiving edX LMS events..
 """
 
-__version__ = '5.5.6'
+__version__ = '6.0.0'
diff --git a/event_routing_backends/helpers.py b/event_routing_backends/helpers.py
index 0ef3d92d..dc21f679 100644
--- a/event_routing_backends/helpers.py
+++ b/event_routing_backends/helpers.py
@@ -51,34 +51,34 @@ def get_anonymous_user_id(username_or_id, external_type):
 
     Arguments:
         username_or_id (str):     username for the learner
-        external_type  (str):     external type id e.g caliper or xapi
+        external_type  (str):     external type id e.g. caliper or xapi
 
     Returns:
         str
     """
     user = get_user(username_or_id)
     if not user:
-        logger.info('User with username "%s" does not exist. '
-                    'Cannot generate anonymous ID', username_or_id)
-
-        anonymous_id = str(uuid.uuid4())
-    else:
-        # Older versions of edx-platform do not have the XAPI or
-        # Caliper ExternalIdTypes, so we fall back to LTI here.
-        # Eventually this will be a problem when those instances
-        # upgrade and their actor id's all change, unless we
-        # eventually add a setting to force LTI here instead of the
-        # usual type.
-        try:
-            type_name = getattr(ExternalIdType, external_type)
-        except AttributeError:  # pragma: no cover
-            type_name = ExternalIdType.LTI
-
-        external_id, _ = ExternalId.add_new_user_id(user, type_name)
-        if not external_id:
-            raise ValueError("External ID type: %s does not exist" % type_name)
-
-        anonymous_id = str(external_id.external_user_id)
+        logger.warning('User with username "%s" does not exist. '
+                       'Cannot generate anonymous ID', username_or_id)
+
+        raise ValueError(f"User with username {username_or_id} does not exist.")
+
+    # Older versions of edx-platform do not have the XAPI or
+    # Caliper ExternalIdTypes, so we fall back to LTI here.
+    # Eventually this will be a problem when those instances
+    # upgrade and their actor id's all change, unless we
+    # eventually add a setting to force LTI here instead of the
+    # usual type.
+    try:
+        type_name = getattr(ExternalIdType, external_type)
+    except AttributeError:  # pragma: no cover
+        type_name = ExternalIdType.LTI
+
+    external_id, _ = ExternalId.add_new_user_id(user, type_name)
+    if not external_id:
+        raise ValueError("External ID type: %s does not exist" % type_name)
+
+    anonymous_id = str(external_id.external_user_id)
 
     return anonymous_id
 
diff --git a/event_routing_backends/processors/tests/transformers_test_mixin.py b/event_routing_backends/processors/tests/transformers_test_mixin.py
index eeb0320a..064232f2 100644
--- a/event_routing_backends/processors/tests/transformers_test_mixin.py
+++ b/event_routing_backends/processors/tests/transformers_test_mixin.py
@@ -7,6 +7,7 @@
 from unittest.mock import patch
 
 import ddt
+import pytest
 from django.contrib.auth import get_user_model
 from django.test.utils import override_settings
 
@@ -108,6 +109,9 @@ def test_event_transformer(self, event_filename, mocked_uuid):
         with open(expected_event_file_path, encoding='utf-8') as expected:
             expected_event = json.loads(expected.read())
 
-            actual_transformed_event = self.registry.get_transformer(original_event).transform()
-
-            self.compare_events(actual_transformed_event, expected_event)
+            if "anonymous" in event_filename:
+                with pytest.raises(ValueError):
+                    self.registry.get_transformer(original_event).transform()
+            else:
+                actual_transformed_event = self.registry.get_transformer(original_event).transform()
+                self.compare_events(actual_transformed_event, expected_event)
diff --git a/event_routing_backends/tests/test_helpers.py b/event_routing_backends/tests/test_helpers.py
index 3ab51a34..d2d2f06b 100644
--- a/event_routing_backends/tests/test_helpers.py
+++ b/event_routing_backends/tests/test_helpers.py
@@ -46,10 +46,13 @@ def test_get_user_email(self):
     @patch('event_routing_backends.helpers.ExternalId')
     def test_get_anonymous_user_id_with_error(self, mocked_external_id):
         mocked_external_id.add_new_user_id.return_value = (None, False)
+        # Test that a failure to add an external id raises an error
         with self.assertRaises(ValueError):
             get_anonymous_user_id('edx', 'XAPI')
 
-        self.assertIsNotNone(get_anonymous_user_id('12345678', 'XAPI'))
+        # Test that an unknown user raises this error
+        with self.assertRaises(ValueError):
+            get_anonymous_user_id('12345678', 'XAPI')
 
     def test_get_uuid5(self):
         actor = '''{