launchpad-reviewers team mailing list archive

[Simone Pelosi <mp+448383@xxxxxxxxxxxxxxxxxx>]

Simone Pelosi has proposed merging ~pelpsi/launchpad:cannot-turn-lp-subteam-to-private into launchpad:master.

Commit message:
Added a status check if src_tab=="teammembership"

teammembership should be considered only if has a status different from DEACTIV>

LP: #2029487


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #2029487 in Launchpad itself: "Cannot turn LP team to Private"
  https://bugs.launchpad.net/launchpad/+bug/2029487

For more details, see:
https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/448383
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad:cannot-turn-lp-subteam-to-private into launchpad:master.

diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
index 3a7d1a5..96691ee 100644
--- a/lib/lp/registry/model/person.py
+++ b/lib/lp/registry/model/person.py
@@ -2777,11 +2777,20 @@ class Person(
         for src_tab, src_col, ref_tab, ref_col, updact, delact in references:
             if (src_tab, src_col) in skip:
                 continue
-            ref_query.append(
+            query = (
                 "SELECT '%(table)s' AS table FROM %(table)s "
                 "WHERE %(col)s = %(person_id)d"
                 % {"col": src_col, "table": src_tab, "person_id": self.id}
             )
+            if src_tab == "teammembership":
+                query += (
+                    " AND status != %(deactivated)d AND status != %(expired)d"
+                    % {
+                        "deactivated": TeamMembershipStatus.DEACTIVATED.value,
+                        "expired": TeamMembershipStatus.EXPIRED.value,
+                    }
+                )
+            ref_query.append(query)
         if ref_query:
             cur.execute(" UNION ".join(ref_query))
             for src_tab in cur.fetchall():
diff --git a/lib/lp/registry/tests/test_person.py b/lib/lp/registry/tests/test_person.py
index 3646ff7..131ceff 100644
--- a/lib/lp/registry/tests/test_person.py
+++ b/lib/lp/registry/tests/test_person.py
@@ -38,6 +38,7 @@ from lp.registry.interfaces.karma import IKarmaCacheManager
 from lp.registry.interfaces.person import ImmutableVisibilityError, IPersonSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.product import IProductSet
+from lp.registry.interfaces.teammembership import ITeamMembershipSet
 from lp.registry.model.karma import KarmaCategory, KarmaTotalCache
 from lp.registry.model.person import Person, get_recipients
 from lp.services.database.interfaces import IStore
@@ -1088,6 +1089,36 @@ class TestPersonStates(TestCaseWithFactory):
         else:
             raise AssertionError("Expected exception.")
 
+    def test_visibility_validator_subteam_public_to_private_view(self):
+        team_owner = self.factory.makePerson()
+        new_team = self.factory.makeTeam(
+            owner=team_owner, visibility=PersonVisibility.PUBLIC
+        )
+        super_team = self.factory.makeTeam(
+            owner=team_owner,
+            visibility=PersonVisibility.PUBLIC,
+            members=[new_team],
+        )
+        membershipset = getUtility(ITeamMembershipSet)
+        membershipset.deactivateActiveMemberships(
+            super_team, "gone", team_owner
+        )
+        view = create_initialized_view(
+            new_team,
+            "+edit",
+            {
+                "field.name": "newteam",
+                "field.displayname": "New Team",
+                "field.membership_policy": "RESTRICTED",
+                "field.renewal_policy": "NONE",
+                "field.visibility": "PRIVATE",
+                "field.actions.save": "Save",
+            },
+        )
+        self.assertEqual(len(view.errors), 0)
+        self.assertEqual(len(view.request.notifications), 0)
+        self.assertEqual(new_team.visibility, PersonVisibility.PRIVATE)
+
     def test_visibility_validator_team_private_to_public_view(self):
         # A PRIVATE team cannot convert to PUBLIC.
         self.otherteam.visibility = PersonVisibility.PRIVATE