Merge pull request #43 from Pearson-Advance/vue/PADV-1574

PADV-1574: Improve LtiProfile name and username property
Pearson-Advance · Sep 10, 2024 · 2cd5e90 · 2cd5e90
2 parents 07245e5 + d62ea75
commit 2cd5e90
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 17 deletions.
diff --git a/openedx_lti_tool_plugin/models.py b/openedx_lti_tool_plugin/models.py
@@ -1,5 +1,6 @@
 """Django Model."""
 import json
+import re
 import uuid
 from typing import TypeVar
 
@@ -110,29 +111,27 @@ def family_name(self) -> str:
     @property
     def name(self) -> str:
         """str: Name."""
-        if self.given_name and self.middle_name and self.family_name:
-            return f'{self.given_name} {self.middle_name} {self.family_name}'
-
-        if self.given_name and self.family_name:
-            return f'{self.given_name} {self.family_name}'
-
-        return self.given_name
+        # Return the name from pii if available
+        if name := self.pii.get('name', ''):
+            return name
+        # Create list with available name parts.
+        parts = [self.given_name, self.middle_name, self.family_name]
+        # Construct the name based on list of available name parts.
+        return ' '.join(part for part in parts if part)
 
     @property
     def username(self) -> str:
         """str: Username."""
         # Get username from user.
         if getattr(self, 'user', None):
             return self.user.username
-        # Generate username string without name.
+        # Generate username with short UUID.
         if not self.given_name:
             return f'{self.short_uuid}'
-        # Generate username string with name.
-        return (
-            f'{self.given_name.lower()[:30]}'
-            f'{self.family_name.lower()[:1]}'
-            f'.{self.short_uuid}'
-        )
+        # Get cleaned unicode name.
+        name = re.sub(r'\W+', '', f'{self.given_name[:30]}{self.family_name[:1]}')
+        # Generate username with lowercase name and short UUID.
+        return f'{name.lower()}.{self.short_uuid}'
 
     @property
     def email(self) -> str:

diff --git a/openedx_lti_tool_plugin/tests/test_models.py b/openedx_lti_tool_plugin/tests/test_models.py
@@ -1,5 +1,6 @@
 """Test models module."""
 import random
+import re
 import string
 import uuid
 from unittest.mock import MagicMock, PropertyMock, call, patch
@@ -22,7 +23,9 @@
 GIVEN_NAME = 'random-given-name'
 MIDDLE_NAME = 'random-middle-name'
 FAMILY_NAME = 'random-family-name'
-GIVEN_NAME_LARGER = ''.join(random.choices(string.ascii_lowercase, k=300))
+GIVEN_NAME_LARGER = ''.join(random.choices([string.ascii_lowercase, string.punctuation], k=300))
+UNICODE_USERNAME_GIVEN_NAME = re.sub(r'\W+', '', f'{GIVEN_NAME_LARGER[:30]}')
+UNICODE_USERNAME_NAME = re.sub(r'\W+', '', f'{GIVEN_NAME_LARGER[:30]}{FAMILY_NAME[:1]}')
 
 
 @ddt.ddt
@@ -200,6 +203,12 @@ def test_family_name_property(self):
         self.assertEqual(self.lti_profile.family_name, FAMILY_NAME)
 
     @ddt.data(
+        (
+            {
+                'name': NAME,
+            },
+            NAME,
+        ),
         (
             {
                 'given_name': GIVEN_NAME,
@@ -231,15 +240,15 @@ def test_name_property(self, name_data: dict, name_return: str):
             {
                 'given_name': GIVEN_NAME_LARGER,
             },
-            f'{GIVEN_NAME_LARGER.lower()[:30]}.',
+            f'{UNICODE_USERNAME_GIVEN_NAME.lower()}.',
         ),
         (
             False,
             {
                 'given_name': GIVEN_NAME_LARGER,
                 'family_name': FAMILY_NAME,
             },
-            f'{GIVEN_NAME_LARGER.lower()[:30]}{FAMILY_NAME.lower()[:1]}.',
+            f'{UNICODE_USERNAME_NAME.lower()}.',
         ),
         (False, {}, ''),
         (True, {}, ''),