Merge lp:~dholbach/ubuntu-teamreports/small-fixes into lp:ubuntu-teamreports

[Daniel Holbach]

[Christian Zambrano (czambran)]

You seem to be missing a '%' in the following line:

return "%s - %s" (self.name, self.who)

review: Needs Fixing

[Daniel Holbach (dholbach)]

That line actually looks fine to me. Do you get an error in any of the admin pages?

[Daniel Holbach (dholbach)]

Sorry, that must have been jetlag, you are right. :-)

[Christian Zambrano (czambran)]

Daniel,

Why are you hard-coding the team "ubuntu-council-teams"?

Why are you saving after get_or_create? The object returned, whether new or old, has been persisted.

The following try-except seems unnecessary:

try:
    t = models.Team.objects.get(lpid=lp_team.name)
except:
    t = None

I am still trying to get the hang of this, please forgive me if I am pointing out useless stuff

review: Needs Information

[Daniel Holbach (dholbach)]

> Why are you hard-coding the team "ubuntu-council-teams"?

If you have a look at https://launchpad.net/~ubuntu-council-teams/+members you can see that it lists all the current Ubuntu Governance Boards. If any replacements happen, we will get that data from Launchpad "for free".

> Why are you saving after get_or_create? The object returned, whether new or
> old, has been persisted.

I was under the impression that I still needed to save when a new object was created. I removed it now.

> The following try-except seems unnecessary:
>
> try:
> t = models.Team.objects.get(lpid=lp_team.name)
> except:
> t = None

Oops, yes, that's legacy code I forgot to remove. Thanks for pointing out.

> I am still trying to get the hang of this, please forgive me if I am pointing
> out useless stuff

Thanks a lot for your thorough review.

[Christian Zambrano (czambran)]

Daniel,

You still have a few instances of calling get_or_create and save right after that.

review: Needs Fixing

[Christian Zambrano (czambran)]

To the best of my ability, this looks good.

review: Approve

[Daniel Holbach (dholbach)]

Thanks a lot for this great review.

=== modified file 'teamreports/reports/admin.py'
--- teamreports/reports/admin.py	2010-04-19 08:41:43 +0000
+++ teamreports/reports/admin.py	2010-11-02 15:51:07 +0000
@@ -1,25 +1,8 @@
 from django.contrib import admin
 from reports.models import Team, Person, TeamProposal, ReportEntry, TeamCategory
 
-class TeamAdmin(admin.ModelAdmin):
-    search_fields = ('lpid',)
-
-class PersonAdmin(admin.ModelAdmin):
-    search_fields = ('lpid',)
-    list_display = ('lpid',)
-
-class TeamProposalAdmin(admin.ModelAdmin):
-    search_fields = ('team',)
-
-class ReportEntryAdmin(admin.ModelAdmin):
-    search_fields = ('date', 'team', )
-
-class TeamCategoryAdmin(admin.ModelAdmin):
-    search_fields = ('name', )
-    list_display = ('name', )
-
-admin.site.register(Team, TeamAdmin)
-admin.site.register(Person, PersonAdmin)
-admin.site.register(TeamProposal, TeamProposalAdmin)
-admin.site.register(ReportEntry, ReportEntryAdmin)
-admin.site.register(TeamCategory, TeamCategoryAdmin)
+admin.site.register(Team)
+admin.site.register(Person)
+admin.site.register(TeamProposal)
+admin.site.register(ReportEntry)
+admin.site.register(TeamCategory)

=== modified file 'teamreports/reports/management/commands/import-translations.py'
--- teamreports/reports/management/commands/import-translations.py	2010-04-13 16:29:38 +0000
+++ teamreports/reports/management/commands/import-translations.py	2010-11-02 15:51:07 +0000
@@ -26,7 +26,6 @@
        not os.path.isdir(project_locale_path):
         pwd = os.getcwd()
         os.chdir(settings.PROJECT_PATH)
-        print settings.PROJECT_PATH
         subprocess.call(["./manage.py", "makemessages", "-l", locale[0]])
         os.chdir(pwd)
     subprocess.call(["cp", pofile, 

=== modified file 'teamreports/reports/management/commands/init-reports.py'
--- teamreports/reports/management/commands/init-reports.py	2010-04-16 10:53:05 +0000
+++ teamreports/reports/management/commands/init-reports.py	2010-11-02 15:51:07 +0000
@@ -17,7 +17,6 @@
         print " * Checking if ubuntu-teamreports-dev is in Auth teams:", 
         (lc, new_object) = Group.objects.get_or_create(name="ubuntu-teamreports-dev")
         if new_object:
-            lc.save()
             print "added."
         else:
             print "yes."

=== modified file 'teamreports/reports/management/commands/lpupdate.py'
--- teamreports/reports/management/commands/lpupdate.py	2010-04-16 13:52:42 +0000
+++ teamreports/reports/management/commands/lpupdate.py	2010-11-02 15:51:07 +0000
@@ -4,50 +4,25 @@
 
 from common import launchpad
 from reports import models
-from django.contrib.auth.models import Group
-
-from datetime import datetime
-
-lp = launchpad.lp_login()
-
-def find_team(team_name):
-    try:
-        lp_team = lp.people[team_name]
-    except KeyError:
-        lp_team = None
-    if lp_team and lp_team.is_team:
-        return lp_team
-    return None
 
 class Command(NoArgsCommand):
     help = "Pull information about team from Launchpad and add to the DB."
 
     def handle_noargs(self, **options):
-        
-        teams = models.Team.objects.all()
-        for t in teams:
-            lp_team = find_team(t.lpid)
-            if not lp_team:
-                # Team does not exist.
-                t.delete()
-            else:
-                t.name = lp_team.display_name
-                owner, created = models.Person.objects.get_or_create(lpid=lp_team.team_owner.name, 
-                                                                     name=lp_team.team_owner.display_name)
-                if created:
-                    owner.save()
-                t.owner = owner
-                for lp_admin in lp_team.getMembersByStatus(status='Administrator'):
-                    admin, created = models.Person.objects.get_or_create(lpid=lp_admin.name,
-                                                                         name=lp_admin.display_name)
-                    if created:
-                        admin.save()
-                    t.admins.add(admin)
-                    t.members.add(admin)
-                for lp_member in lp_team.getMembersByStatus(status='Approved'):
-                    member, created = models.Person.objects.get_or_create(lpid=lp_member.name,
-                                                                          name=lp_member.display_name)
-                    if created:
-                        member.save()
-                    t.members.add(member)
-                t.save()
+        lp = launchpad.lp_login()
+        for lp_team in lp.people["ubuntu-council-teams"].members:
+            t, created = models.Team.objects.get_or_create(lpid=lp_team.name)
+            t.name = lp_team.display_name
+            owner, created = models.Person.objects.get_or_create(lpid=lp_team.team_owner.name, 
+                                                                 name=lp_team.team_owner.display_name)
+            t.owner = owner
+            for lp_admin in lp_team.getMembersByStatus(status='Administrator'):
+                admin, created = models.Person.objects.get_or_create(lpid=lp_admin.name,
+                                                                     name=lp_admin.display_name)
+                t.admins.add(admin)
+                t.members.add(admin)
+            for lp_member in lp_team.getMembersByStatus(status='Approved'):
+                member, created = models.Person.objects.get_or_create(lpid=lp_member.name,
+                                                                      name=lp_member.display_name)
+                t.members.add(member)
+            t.save()

=== modified file 'teamreports/reports/management/commands/process-proposals.py'
--- teamreports/reports/management/commands/process-proposals.py	2010-04-19 08:41:43 +0000
+++ teamreports/reports/management/commands/process-proposals.py	2010-11-02 15:51:07 +0000
@@ -11,6 +11,4 @@
         proposals = models.TeamProposal.objects.all()
         for proposal in proposals.filter(approved=True):
             p, created = models.Team.objects.get_or_create(lpid=proposal.team)
-            if created:
-                p.save()
             proposal.delete()

=== modified file 'teamreports/reports/models.py'
--- teamreports/reports/models.py	2010-04-19 08:41:43 +0000
+++ teamreports/reports/models.py	2010-11-02 15:51:07 +0000
@@ -5,16 +5,26 @@
     rank = models.IntegerField(_("Rank"), default=1000)
     name = models.CharField(_("Category"), blank=False, max_length=30)
 
+    def __unicode__(self):
+        return self.name
+
 class TeamProposal(models.Model):
     date = models.DateField(_("Date"), blank=False)
     who = models.CharField(_("Launchpad Person ID"), max_length=50, blank=False)
     team = models.CharField(_("Launchpad Team ID"), max_length=50, blank=False)
     approved = models.BooleanField(_("Approved"), default=False)
+    
+    def __unicode__(self):
+        return "%s - %s" % (self.name, self.who)
 
 class Person(models.Model):
     lpid = models.SlugField(_("Launchpad Person ID"), max_length=50, blank=False)
     name = models.CharField(_("Team Name"), max_length=180)
 
+    def __unicode__(self):
+        return self.lpid
+
+
 class Team(models.Model):
     lpid = models.SlugField(_("Launchpad Team ID"), max_length=50)
     name = models.CharField(_("Team Name"), max_length=180)
@@ -23,9 +33,15 @@
     responsibles = models.ManyToManyField(Person, related_name='responsibles')
     members = models.ManyToManyField(Person, related_name='members')
     category = models.ForeignKey(TeamCategory, blank=True, null=True)
+    
+    def __unicode__(self):
+        return self.lpid
 
 class ReportEntry(models.Model):
     date = models.DateField(_("Date"), blank=False)
     team = models.ForeignKey(Team, blank=False)
     entry = models.CharField(_("Entry"), max_length=700, blank=False)
     who = models.ForeignKey(Person, blank=False)
+    
+    def __unicode__(self):
+        return "%s - %s" % (self.team, self.date)