launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/launchpad/bug-227494 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #227494 in Launchpad itself: "Person.inTeam treats team owners as members but other code does not"
  https://bugs.launchpad.net/launchpad/+bug/227494

For more details, see:
https://code.launchpad.net/~lifeless/launchpad/bug-227494/+merge/62943

Per the attached bug we currently have an awkward situation where if you ask 'is teamX.owner in the list of members of team X' and 'is teamX.owner a member of team X' you get different answers.

This exhibits itself as team owners being able to do anything the team can do even if they are not in the team and not shown as the team in the web UI.

This small patch corrects that. It leaves team owners as *administrators* of the team they own, because thats much less confusing.
-- 
https://code.launchpad.net/~lifeless/launchpad/bug-227494/+merge/62943
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/bug-227494 into lp:launchpad.

=== modified file 'lib/lp/registry/doc/teammembership-email-notification.txt'
--- lib/lp/registry/doc/teammembership-email-notification.txt	2011-05-27 19:53:20 +0000
+++ lib/lp/registry/doc/teammembership-email-notification.txt	2011-05-31 05:29:26 +0000
@@ -41,6 +41,8 @@
     >>> kamion = personset.getByName('kamion')
     >>> sampleperson = personset.getByName('name12')
     >>> ubuntu_team = personset.getByName('ubuntu-team')
+    >>> from lp.testing.sampledata import ADMIN_EMAIL
+    >>> admin_person = personset.getByEmail(ADMIN_EMAIL)
 
 
 Now Robert Collins proposes himself as a member of the Ubuntu Team. This
@@ -826,8 +828,9 @@
     ...     hwdb_admins)
     >>> print dumper_hwdb_membership.status.title
     Approved
-    >>> setStatus(dumper_hwdb_membership,
-    ...     TeamMembershipStatus.DEACTIVATED, reviewer=mark, silent=True)
+    >>> login_person(admin_person)
+    >>> setStatus(dumper_hwdb_membership, TeamMembershipStatus.DEACTIVATED,
+    ...     reviewer=admin_person, silent=True)
     >>> run_mail_jobs()
     >>> len(stub.test_emails)
     0

=== modified file 'lib/lp/registry/doc/teammembership.txt'
--- lib/lp/registry/doc/teammembership.txt	2011-04-21 05:18:26 +0000
+++ lib/lp/registry/doc/teammembership.txt	2011-05-31 05:29:26 +0000
@@ -410,7 +410,7 @@
 It's important to note that even if the owner leaves the team, which
 removes his membership, he will still be the team's owner and retain his
 rights over it. This ensures we'll never have teams which can't be
-managed.
+managed. This does not imply that the owner will be a member of the team.
 
     >>> login_person(t5.teamowner)
     >>> t5.teamowner.leave(t5)
@@ -418,8 +418,23 @@
     >>> [m.displayname for m in t5.allmembers]
     []
     >>> t5.teamowner.inTeam(t5)
-    True
-
+    False
+
+The team owner can make themselves a member again even if the team is
+restricted:
+
+    >>> t5.teamowner.join(t5, requester=t5.teamowner)
+    >>> flush_database_updates()
+    >>> t5.teamowner in t5.allmembers
+    True
+    >>> t5.teamowner.inTeam(t5)
+    True
+
+And escalate their privileges back to administrator:
+
+    >>> membership = membershipset.getByPersonAndTeam(t5.teamowner, t5)
+    >>> membership.setStatus(TeamMembershipStatus.ADMIN, t5.teamowner)
+    True
 
 Changing membership data
 ------------------------

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2011-05-18 03:36:29 +0000
+++ lib/lp/registry/interfaces/person.py	2011-05-31 05:29:26 +0000
@@ -1214,7 +1214,7 @@
     # @operation_parameters(team=copy_field(ITeamMembership['team']))
     # @export_read_operation()
     def inTeam(team):
-        """Is this person is a member or the owner of `team`?
+        """Is this person is a member of `team`?
 
         Returns `True` when you ask if an `IPerson` (or an `ITeam`,
         since it inherits from `IPerson`) is a member of himself

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2011-05-27 21:53:34 +0000
+++ lib/lp/registry/model/person.py	2011-05-31 05:29:26 +0000
@@ -1336,18 +1336,7 @@
                 pass
 
         tp = TeamParticipation.selectOneBy(team=team, person=self)
-        if tp is not None or self.id == team.teamownerID:
-            in_team = True
-        elif not team.teamowner.inTeam(team):
-            # The owner is not a member but must retain his rights over
-            # this team. This person may be a member of the owner, and in this
-            # case it'll also have rights over this team.
-            # Note that this query and the tp query above can be consolidated
-            # when we get to a finer grained level of optimisations.
-            in_team = self.inTeam(team.teamowner)
-        else:
-            in_team = False
-
+        in_team = tp is not None
         self._inTeam_cache[team.id] = in_team
         return in_team