launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/person-in-team into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~bac/launchpad/person-in-team/+merge/55928

= Summary =

The implementation of person_is_team_admin *appears* to be inefficient
and this fix *seems* to be better.  Lots of hand waving here.

== Proposed fix ==

Get all of a users administered teams in one query and just check each
one subsequently.

== Pre-implementation notes ==

None

== Implementation details ==

As above.

== Tests ==

No test changes as test_expose seems to have good coverage and it is
just a reimplementation of existing functionality.

== Demo and Q/A ==

None

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/browser/structuralsubscription.py
-- 
https://code.launchpad.net/~bac/launchpad/person-in-team/+merge/55928
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/person-in-team into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/structuralsubscription.py'
--- lib/lp/bugs/browser/structuralsubscription.py	2011-03-30 20:19:44 +0000
+++ lib/lp/bugs/browser/structuralsubscription.py	2011-04-01 13:16:26 +0000
@@ -417,20 +417,11 @@
     IJSONRequestCache(request).objects['administratedTeams'] = info
 
 
-def person_is_team_admin(person, team):
-    answer = False
-    admins = team.adminmembers
-    for admin in admins:
-        if person.inTeam(admin):
-            answer = True
-            break
-    return answer
-
-
 def expose_user_subscriptions_to_js(user, subscriptions, request):
     """Make the user's subscriptions available to JavaScript."""
     info = {}
     api_request = IWebServiceClientRequest(request)
+    administered_teams = user.getAdministratedTeams()
     for subscription in subscriptions:
         target = subscription.target
         record = info.get(target)
@@ -444,7 +435,7 @@
         for filter in subscription.bug_filters:
             is_team = subscriber.isTeam()
             user_is_team_admin = (is_team and
-                                  person_is_team_admin(user, subscriber))
+                                  subscriber in administered_teams)
             record['filters'].append(dict(
                 filter=filter,
                 subscriber_link=absoluteURL(subscriber, api_request),