Merge pull request #372 from nelc/and/support_numeric_usernames

fix: support numeric usernames on get_user method

CHANGELOG.rst

@@ -14,6 +14,11 @@ Change Log
 Unreleased
 ~~~~~~~~~~
 
+[7.2.0]
+~~~~~~~
+
+* Add support for usernames that are numeric.
+
 [7.1.0]
 ~~~~~~~
 

event_routing_backends/__init__.py

@@ -2,4 +2,4 @@
 Various backends for receiving edX LMS events..
 """
 
-__version__ = '7.1.0'
+__version__ = '7.2.0'

event_routing_backends/helpers.py

@@ -93,17 +93,16 @@ def get_user(username_or_id):
     Returns:
         user object
     """
-    user = user_id = username = None
-    if username_or_id:
-        try:
-            user_id = int(username_or_id)
-        except ValueError:
-            username = username_or_id
+    user = username = None
+
+    if not username_or_id:
+        return None
 
-    if username:
+    try:
+        user = User.objects.get(id=int(username_or_id))
+    except (User.DoesNotExist, ValueError):
+        username = username_or_id
         user = User.objects.filter(username=username).first()
-    elif user_id:
-        user = User.objects.filter(id=user_id).first()
 
     if username and not user:
         try:

event_routing_backends/tests/test_helpers.py

@@ -3,25 +3,30 @@
 """
 from unittest.mock import patch
 
+from ddt import data, ddt
 from django.test import TestCase
 
 from event_routing_backends.helpers import (
     get_anonymous_user_id,
     get_block_id_from_event_referrer,
     get_course_from_id,
+    get_user,
     get_user_email,
     get_uuid5,
 )
 from event_routing_backends.tests.factories import UserFactory
 
 
+@ddt
 class TestHelpers(TestCase):
     """
     Test the helper methods.
     """
+
     def setUp(self):
         super().setUp()
-        UserFactory.create(username='edx', email='[email protected]')
+        self.edx_user = UserFactory.create(username='edx', email='[email protected]')
+        UserFactory.create(username='10228945687', email='[email protected]')
 
     def test_get_block_id_from_event_referrer_with_error(self):
         sample_event = {
@@ -83,3 +88,40 @@ def test_get_course_from_id_unknown_course(self, mock_get_course_overviews):
         mock_get_course_overviews.return_value = []
         with self.assertRaises(ValueError):
             get_course_from_id("foo")
+
+    @data("edx", "10228945687")
+    def test_get_user_by_username(self, username):
+        """Test that the method get_user returns the right user based on given username parameter.
+
+        Expected behavior:
+            - Returned user corresponds to the username.
+        """
+        user = get_user(username)
+
+        self.assertEqual(username, user.username)
+
+    def test_get_user_by_id(self):
+        """ Test that the method get_user returns the right user based on the user id.
+
+        Expected behavior:
+            - Returned user is the edx_user
+        """
+        user = get_user(self.edx_user.id)
+
+        self.assertEqual(self.edx_user, user)
+
+    def test_get_user_priority(self):
+        """Since the method can find users by id or username this checks that the user
+        found by id will be returned instead of the user found by username when a user's
+        username is the same as the id of another user.
+
+        Expected behavior:
+            - Returned user corresponds to the id.
+        """
+        right_user = UserFactory.create(username='testing', email='[email protected]')
+        # Create user with the previous user id as username.
+        UserFactory.create(username=right_user.id, email='[email protected]')
+
+        user = get_user(str(right_user.id))
+
+        self.assertEqual(right_user, user)

setup.cfg

@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 7.1.0
+current_version = 7.2.0
 commit = False
 tag = False