Merge pull request #9415 from edx/christina/insensitive-check

Do a case insensitive check for conflicts to improve performance.

common/djangoapps/util/model_utils.py

@@ -165,30 +165,3 @@ def slugify(value):
     value = unicodedata.normalize('NFKD', value).encode('ascii', 'ignore').decode('ascii')
     value = re.sub(r'[^\w\s-]', '', value).strip().lower()
     return mark_safe(re.sub(r'[-\s]+', '-', value))
-
-
-def generate_unique_readable_id(name, queryset, lookup_field):
-    """Generates a unique readable id from name by appending a numeric suffix.
-
-    Args:
-        name (string): Name to generate the id from. May include spaces.
-        queryset (QuerySet): QuerySet to check for uniqueness within.
-        lookup_field (string): Field name on the model that corresponds to the
-            unique identifier.
-
-    Returns:
-        string: generated unique identifier
-    """
-    candidate = slugify(name)
-    conflicts = queryset.filter(**{lookup_field + '__startswith': candidate}).values_list(lookup_field, flat=True)
-
-    if conflicts and candidate in conflicts:
-        suffix = 2
-        while True:
-            new_id = candidate + '-' + str(suffix)
-            if new_id not in conflicts:
-                candidate = new_id
-                break
-            suffix += 1
-
-    return candidate

lms/djangoapps/teams/models.py

@@ -10,7 +10,7 @@ from django.utils.translation import ugettext_lazy
 from django_countries.fields import CountryField
 
 from xmodule_django.models import CourseKeyField
-from util.model_utils import generate_unique_readable_id
+from util.model_utils import slugify
 from student.models import LanguageField, CourseEnrollment
 from .errors import AlreadyOnTeamInCourse, NotEnrolledInCourseForTeam
 
@@ -51,9 +51,9 @@ class CourseTeam(models.Model):
               team uses, as ISO 639-1 code.
 
         """
-
-        team_id = generate_unique_readable_id(name, cls.objects.all(), 'team_id')
-        discussion_topic_id = uuid4().hex
+        unique_id = uuid4().hex
+        team_id = slugify(name)[0:20] + '-' + unique_id
+        discussion_topic_id = unique_id
 
         course_team = cls(
             team_id=team_id,

lms/djangoapps/teams/tests/test_views.py

@@ -533,20 +533,28 @@ class TestCreateTeamAPI(TeamAPITestCase):
     def test_access(self, user, status):
         team = self.post_create_team(status, self.build_team_data(name="New Team"), user=user)
         if status == 200:
-            self.assertEqual(team['id'], 'new-team')
-            self.assertIn('discussion_topic_id', team)
+            self.verify_expected_team_id(team, 'new-team')
             teams = self.get_teams_list(user=user)
             self.assertIn("New Team", [team['name'] for team in teams['results']])
 
+    def verify_expected_team_id(self, team, expected_prefix):
+        """ Verifies that the team id starts with the specified prefix and ends with the discussion_topic_id """
+        self.assertIn('id', team)
+        self.assertIn('discussion_topic_id', team)
+        self.assertEqual(team['id'], expected_prefix + '-' + team['discussion_topic_id'])
+
     def test_naming(self):
         new_teams = [
             self.post_create_team(data=self.build_team_data(name=name), user=self.create_and_enroll_student())
-            for name in ["The Best Team", "The Best Team", "The Best Team", "The Best Team 2"]
+            for name in ["The Best Team", "The Best Team", "A really long team name"]
         ]
-        self.assertEquals(
-            [team['id'] for team in new_teams],
-            ['the-best-team', 'the-best-team-2', 'the-best-team-3', 'the-best-team-2-2']
-        )
+        # Check that teams with the same name have unique IDs.
+        self.verify_expected_team_id(new_teams[0], 'the-best-team')
+        self.verify_expected_team_id(new_teams[1], 'the-best-team')
+        self.assertNotEqual(new_teams[0]['id'], new_teams[1]['id'])
+
+        # Verify expected truncation behavior with names > 20 characters.
+        self.verify_expected_team_id(new_teams[2], 'a-really-long-team-n')
 
     @ddt.data((400, {
         'name': 'Bad Course ID',
@@ -616,6 +624,10 @@ class TestCreateTeamAPI(TeamAPITestCase):
             language='fr'
         ), user=creator)
 
+        # Verify the id (it ends with a unique hash, which is the same as the discussion_id).
+        self.verify_expected_team_id(team, 'fully-specified-team')
+        del team['id']
+
         # Remove date_created and discussion_topic_id because they change between test runs
         del team['date_created']
         del team['discussion_topic_id']
@@ -643,7 +655,6 @@ class TestCreateTeamAPI(TeamAPITestCase):
             'is_active': True,
             'topic_id': 'great-topic',
             'course_id': str(self.test_course_1.id),
-            'id': 'fully-specified-team',
             'description': 'Another fantastic team'
         })