launchpad-reviewers team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

Gary Poster has proposed merging lp:~gary/launchpad/owner_administrator into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #768366 in Launchpad itself: "teams owned but not joined should not be included in administrated teams for structural subscriptions overlay"
  https://bugs.launchpad.net/launchpad/+bug/768366

For more details, see:
https://code.launchpad.net/~gary/launchpad/owner_administrator/+merge/58683

This small branch changes the structural subscription overlay to not allow the current user to subscribe teams that they own but are not members of.  An existing constraint that I'd rather not revisit right now is that only subscribers can modify their subscriptions.  Allowing people to create a subscription but not edit it does not match our UI model.  Therefore, I made this change.

lint is happy.

Thank you
-- 
https://code.launchpad.net/~gary/launchpad/owner_administrator/+merge/58683
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/owner_administrator into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/structuralsubscription.py'
--- lib/lp/bugs/browser/structuralsubscription.py	2011-04-13 01:19:03 +0000
+++ lib/lp/bugs/browser/structuralsubscription.py	2011-04-21 14:26:09 +0000
@@ -428,6 +428,7 @@
     api_request = IWebServiceClientRequest(request)
     is_distro = IDistribution.providedBy(context)
     if user is not None:
+        membership = list(user.teams_participated_in)
         for team in user.getAdministratedTeams():
             # If the context is a distro AND a bug supervisor is set AND
             # the admininistered team is not a member of the bug supervisor
@@ -435,6 +436,12 @@
             if (is_distro and context.bug_supervisor is not None and
                 not team.inTeam(context.bug_supervisor)):
                 continue
+            # If the user is not a member of the team itself, then skip it,
+            # because structural subscriptions and their filters can only be
+            # edited by the subscriber.
+            # This can happen if the user is an owner but not a member.
+            if not team in membership:
+                continue
             info.append({
                 'link': absoluteURL(team, api_request),
                 'title': team.title,

=== modified file 'lib/lp/bugs/browser/tests/test_expose.py'
--- lib/lp/bugs/browser/tests/test_expose.py	2011-04-06 15:03:54 +0000
+++ lib/lp/bugs/browser/tests/test_expose.py	2011-04-21 14:26:09 +0000
@@ -144,6 +144,17 @@
         self.assertThat(team_info[0]['link'],
             Equals('http://example.com/BugSupervisorSubTeam'))
 
+    def test_teams_owned_but_not_joined_are_not_included(self):
+        context = self.factory.makeProduct(owner=self.user)
+        team = self.factory.makeTeam(
+            name='bug-supervisor-team', owner=self.user)
+        with person_logged_in(self.user):
+            self.user.leave(team)
+        expose_user_administered_teams_to_js(self.request, self.user, context,
+            absoluteURL=fake_absoluteURL)
+        team_info = self._sort(self.request.objects['administratedTeams'])
+        self.assertEquals(len(team_info), 0)
+
     def test_teams_for_distro_with_bug_super(self):
         self._setup_teams(self.user)
         context = self.factory.makeDistribution(