[damned-lies] Faster Teams page loading

[Stéphane Raimbault <stephaner src gnome org>]

commit 9efe1e7b00ca45740480c93b823d464ae568318a
Author: Stéphane Raimbault <stephane raimbault gmail com>
Date:   Sun May 31 23:41:02 2009 +0200

    Faster Teams page loading
    
    - add tests for Team
    - share setUp and tearDown with Vertimus
    - new all_with_coordinator function
    - replace test of presence by a try/except
---
 teams/models.py            |   69 ++++++++++++++++++++++++++++++++++++-------
 teams/tests.py             |   65 +++++++++++++++++++++++++++++++++++++++++
 teams/views.py             |    2 +-
 vertimus/tests/__init__.py |   50 ++-----------------------------
 4 files changed, 127 insertions(+), 59 deletions(-)

diff --git a/teams/models.py b/teams/models.py
index 1250986..bca98c5 100644
--- a/teams/models.py
+++ b/teams/models.py
@@ -23,20 +23,55 @@ from django.utils.translation import ugettext_lazy, ugettext as _
 from people.models import Person
 
 class TeamManager(models.Manager):
+
+    def all_with_coordinator(self):
+        """
+        Returns all teams with the coordinator already prefilled. Use that
+        function to reduce the size of extracted data and the numbers of objects
+        built or use all_with_roles() if you need informations about the other
+        roles.
+        """
+        teams = self.all()
+        roles = Role.objects.select_related("person").filter(role='coordinator')
+        role_dict = {}
+
+        for role in roles:
+            # Only one coordinator by team
+            role_dict[role.team_id] = role
+
+        for team in teams:
+            try:
+                role = role_dict[team.id]
+                team.roles = {'coordinator': [role.person]}
+            except KeyError:
+                # Abnormal because a team must have a coordinator but of no
+                # consequence
+                pass
+        return teams
+
     def all_with_roles(self):
-        """ This method prefills team.coordinator/committers/reviewers/translators to reduce subsequent database access"""
+        """
+        This method prefills team.coordinator/committer/reviewer/translator to
+        reduce subsequent database access.
+        """
         teams = self.all()
         roles = Role.objects.select_related("person").all()
         role_dict = {}
+
         for role in roles:
             if role.team_id not in role_dict:
                 role_dict[role.team_id] = [role]
             else:
                 role_dict[role.team_id].append(role)
+
         for team in teams:
-            if team.id in role_dict:
+            try:
                 for role in role_dict[team.id]:
                     team.fill_role(role.role, role.person)
+            except KeyError:
+                # Abnormal because a team must have a coordinator but of no
+                # consequence
+                pass
         return teams
 
 
@@ -69,7 +104,10 @@ class Team(models.Model):
     def fill_role(self, role, person):
         """ Used by TeamManager to prefill roles in team """
         if not self.roles:
-            self.roles = {'coordinator':[], 'committer':[], 'reviewer':[], 'translator':[]}
+            self.roles = {'coordinator': [],
+                          'committer': [],
+                          'reviewer': [],
+                          'translator': []}
         self.roles[role].append(person)
 
     def get_description(self):
@@ -85,7 +123,8 @@ class Team(models.Model):
             try:
                 # The join by role__team__id generates only one query and
                 # the same one by role__team=self two queries!
-                return Person.objects.get(role__team__id=self.id, role__role='coordinator')
+                return Person.objects.get(role__team__id=self.id,
+                                          role__role='coordinator')
             except Person.DoesNotExist:
                 return None
 
@@ -93,7 +132,8 @@ class Team(models.Model):
         try:
             return self.roles[role]
         except:
-            members = Person.objects.filter(role__team__id=self.id, role__role=role)
+            members = Person.objects.filter(role__team__id=self.id,
+                                            role__role=role)
             return members
 
     def get_committers(self):
@@ -106,8 +146,10 @@ class Team(models.Model):
         return self.get_members_by_role('translator')
 
 class FakeTeam(object):
-    """ This is a class replacing a Team object when a language
-        has no team attached """
+    """
+    This is a class replacing a Team object when a language
+    has no team attached.
+    """
     fake = 1
 
     def __init__(self, language):
@@ -136,16 +178,19 @@ ROLE_CHOICES = (
 )
 
 class Role(models.Model):
-    """ This is the intermediary class between Person and Team to attribute
-        roles to Team members. """
-
+    """
+    This is the intermediary class between Person and Team to attribute roles to
+    Team members.
+    """
     team = models.ForeignKey(Team)
     person = models.ForeignKey(Person)
-    role = models.CharField(max_length=15, choices=ROLE_CHOICES, default='translator')
+    role = models.CharField(max_length=15, choices=ROLE_CHOICES,
+                            default='translator')
 
     class Meta:
         db_table = 'role'
         unique_together = ('team', 'person')
 
     def __unicode__(self):
-        return "%s is %s in %s team" % (self.person.name, self.role, self.team.description)
+        return "%s is %s in %s team" % (self.person.name, self.role,
+                                        self.team.description)
diff --git a/teams/tests.py b/teams/tests.py
new file mode 100644
index 0000000..56b2681
--- /dev/null
+++ b/teams/tests.py
@@ -0,0 +1,65 @@
+from django.test import TestCase
+from people.models import Person
+from teams.models import Team, Role
+
+class TeamTest(TestCase):
+
+    def setUp(self):
+        self.pn = Person(first_name='John', last_name='Nothing',
+            email='jn devnull com', username= 'jn')
+        self.pn.save()
+
+        self.pt = Person(first_name='John', last_name='Translator',
+            email='jt tf1 com', username= 'jt')
+        self.pt.save()
+
+        self.pr = Person(first_name='John', last_name='Reviewer',
+            email='jr csa com', username= 'jr')
+        self.pr.save()
+
+        self.pc = Person(first_name='John', last_name='Committer',
+            email='jc alinsudesonpleingre fr', username= 'jc')
+        self.pc.save()
+
+        self.pcoo = Person(first_name='John', last_name='Coordinator',
+            email='jcoo imthebigboss fr', username= 'jcoo')
+        self.pcoo.save()
+
+        self.t = Team(name='fr', description='French')
+        self.t.save()
+
+        self.role = Role(team=self.t, person=self.pt)
+        self.role.save()
+
+        self.role = Role(team=self.t, person=self.pr, role='reviewer')
+        self.role.save()
+
+        self.role = Role(team=self.t, person=self.pc, role='committer')
+        self.role.save()
+
+        self.role = Role(team=self.t, person=self.pcoo, role='coordinator')
+        self.role.save()
+
+    def tearDown(self):
+        for role in Role.objects.all():
+            role.delete()
+        self.pcoo.delete()
+        self.pc.delete()
+        self.pr.delete()
+        self.pt.delete()
+        self.pn.delete()
+        self.t.delete()
+
+    def test_roles(self):
+        """
+        Tests the hierarchy of roles
+        """
+        people = self.t.get_coordinator()
+        self.assertEqual(people, self.pcoo)
+
+        team = Team.objects.all_with_coordinator()[0]
+        pcoo = team.get_coordinator()
+        self.assertEqual(pcoo, self.pcoo)
+
+        list_pc = team.get_committers()
+        self.assertEqual(list_pc[0], self.pc)
diff --git a/teams/views.py b/teams/views.py
index 23ced02..c1fcc5d 100644
--- a/teams/views.py
+++ b/teams/views.py
@@ -28,7 +28,7 @@ from teams.forms import EditMemberRoleForm
 from languages.models import Language
 
 def teams(request):
-    teams = Team.objects.all_with_roles()
+    teams = Team.objects.all_with_coordinator()
 
     context = {
         'pageSection': 'teams',
diff --git a/vertimus/tests/__init__.py b/vertimus/tests/__init__.py
index cfb2f73..e95eca0 100644
--- a/vertimus/tests/__init__.py
+++ b/vertimus/tests/__init__.py
@@ -27,50 +27,15 @@ from django.http import QueryDict
 from django.utils.datastructures import MultiValueDict
 from django.conf import settings
 
-from people.models import Person
-from teams.models import Team, Role
-from languages.models import Language
+from teams.tests import TeamTest
 from stats.models import Module, Branch, Release, Category, Domain
 from vertimus.models import *
 from vertimus.forms import ActionForm
 
-class VertimusTest(TestCase):
+class VertimusTest(TeamTest):
 
     def setUp(self):
-        self.pn = Person(first_name='John', last_name='Nothing',
-            email='jn devnull com', username= 'jn')
-        self.pn.save()
-
-        self.pt = Person(first_name='John', last_name='Translator',
-            email='jt tf1 com', username= 'jt')
-        self.pt.save()
-
-        self.pr = Person(first_name='John', last_name='Reviewer',
-            email='jr csa com', username= 'jr')
-        self.pr.save()
-
-        self.pc = Person(first_name='John', last_name='Committer',
-            email='jc alinsudesonpleingre fr', username= 'jc')
-        self.pc.save()
-
-        self.pcoo = Person(first_name='John', last_name='Coordinator',
-            email='jcoo imthebigboss fr', username= 'jcoo')
-        self.pcoo.save()
-
-        self.t = Team(name='fr', description='French')
-        self.t.save()
-
-        self.role = Role(team=self.t, person=self.pt)
-        self.role.save()
-
-        self.role = Role(team=self.t, person=self.pr, role='reviewer')
-        self.role.save()
-
-        self.role = Role(team=self.t, person=self.pc, role='committer')
-        self.role.save()
-
-        self.role = Role(team=self.t, person=self.pcoo, role='coordinator')
-        self.role.save()
+        super(VertimusTest, self).setUp()
 
         self.l = Language(name='french', locale='fr', team=self.t)
         self.l.save()
@@ -107,14 +72,7 @@ class VertimusTest(TestCase):
         self.b.delete()
         self.m.delete()
         self.l.delete()
-        for role in Role.objects.all():
-            role.delete()
-        self.t.delete()
-        self.pn.delete()
-        self.pt.delete()
-        self.pr.delete()
-        self.pc.delete()
-        self.pcoo.delete()
+        super(VertimusTest, self).tearDown()
 
     def test_state_none(self):
         sdb = StateDb(branch=self.b, domain=self.d, language=self.l)