launchpad-reviewers team mailing list archive

[Thomi Richards <thomi.richards@xxxxxxxxxxxxx>]

Thomi Richards has proposed merging lp:~thomir/launchpad/devel-fix-group-security into lp:launchpad.

Commit message:
Allow private team owners to view their team membership listing when the team contains expired or deactivated private teams.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~thomir/launchpad/devel-fix-group-security/+merge/250578

This branch allows owners of private teams to view their team listing when their team contains a private team with one one of the following membership statuses:

 * EXPIRED
 * DEACTIVATED


-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thomir/launchpad/devel-fix-group-security into lp:launchpad.

=== modified file 'lib/lp/app/tests/test_security.py'
--- lib/lp/app/tests/test_security.py	2012-08-14 23:27:07 +0000
+++ lib/lp/app/tests/test_security.py	2015-02-23 03:28:19 +0000
@@ -26,6 +26,7 @@
     )
 from lp.security import PublicOrPrivateTeamsExistence
 from lp.testing import (
+    admin_logged_in,
     person_logged_in,
     TestCase,
     TestCaseWithFactory,
@@ -218,3 +219,37 @@
             removeSecurityProxy(private_team))
         self.assertTrue(checker.checkAuthenticated(IPersonRoles(team_user)))
         self.assertFalse(checker.checkAuthenticated(IPersonRoles(other_user)))
+
+    def assertTeamOwnerCanListPrivateTeamWithTeamStatus(self, team_status):
+        main_team_owner = self.factory.makePerson()
+        main_team = self.factory.makeTeam(
+            owner=main_team_owner,
+            visibility=PersonVisibility.PRIVATE)
+        private_team_owner = self.factory.makePerson()
+        private_team = self.factory.makeTeam(
+            owner=private_team_owner,
+            visibility=PersonVisibility.PRIVATE)
+        with admin_logged_in():
+            # Cannot add a team with a non-APPROVED / PENDING status, so add
+            # it as approved and then edit the membership.
+            main_team.addMember(
+                private_team,
+                main_team_owner,
+                status=TeamMembershipStatus.APPROVED,
+                force_team_add=True)
+            main_team.setMembershipData(
+                private_team,
+                team_status,
+                main_team_owner)
+
+        checker = PublicOrPrivateTeamsExistence(removeSecurityProxy(private_team))
+        self.assertTrue(checker.checkAuthenticated(IPersonRoles(main_team_owner)))
+
+    def test_can_list_team_with_deactivated_private_team(self):
+        self.assertTeamOwnerCanListPrivateTeamWithTeamStatus(
+            TeamMembershipStatus.DEACTIVATED)
+
+    def test_can_list_team_with_expired_private_team(self):
+        self.assertTeamOwnerCanListPrivateTeamWithTeamStatus(
+            TeamMembershipStatus.EXPIRED)
+

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2015-02-11 12:09:37 +0000
+++ lib/lp/security.py	2015-02-23 03:28:19 +0000
@@ -1038,10 +1038,9 @@
                 return True
 
             # If it's not, the private team may still be a pending membership,
+            # deactivated membership, or an expired membership,
             # which still needs to be visible to team members.
             BAD_STATES = (
-                TeamMembershipStatus.DEACTIVATED.value,
-                TeamMembershipStatus.EXPIRED.value,
                 TeamMembershipStatus.DECLINED.value,
                 TeamMembershipStatus.INVITATION_DECLINED.value,
                 )