launchpad-reviewers team mailing list archive

[Colin Watson <mp+445009@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:close-account-skip-self-team-participation into launchpad:master.

Commit message:
Skip reflexive TeamParticipation row in close-account

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/445009

I happened to notice that `cronscripts/check-teamparticipation.py` had some work to do on dogfood, and thought that was odd.  This turns out to be because `close-account` removes all `TeamParticipation` rows for the closed account, including the self-participation row created by the `you_are_your_own_member` database trigger for all `Person` rows.  I don't believe this was intentional; it should only remove rows resulting from actual team memberships.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:close-account-skip-self-team-participation into launchpad:master.

diff --git a/lib/lp/registry/scripts/closeaccount.py b/lib/lp/registry/scripts/closeaccount.py
index 73510a0..970e3e1 100644
--- a/lib/lp/registry/scripts/closeaccount.py
+++ b/lib/lp/registry/scripts/closeaccount.py
@@ -331,7 +331,6 @@ def close_account(username, log):
         ("KarmaTotalCache", "person"),
         # Team memberships
         ("TeamMembership", "person"),
-        ("TeamParticipation", "person"),
         # Contacts
         ("AnswerContact", "person"),
         # Pending items in queues
@@ -361,6 +360,19 @@ def close_account(username, log):
             (person.id,),
         )
 
+    # Remove all team participation records for the person, except for the
+    # self-participation record that every person has.
+    table_notification("TeamParticipation")
+    store.execute(
+        """
+        DELETE FROM TeamParticipation
+        WHERE person = ? AND team != ?
+        """,
+        (person.id, person.id),
+    )
+    skip.add(("teamparticipation", "person"))
+    skip.add(("teamparticipation", "team"))
+
     # Trash Sprint Attendance records in the future.
     table_notification("SprintAttendance")
     store.execute(
diff --git a/lib/lp/registry/scripts/tests/test_closeaccount.py b/lib/lp/registry/scripts/tests/test_closeaccount.py
index 9177ddc..7f5e1bf 100644
--- a/lib/lp/registry/scripts/tests/test_closeaccount.py
+++ b/lib/lp/registry/scripts/tests/test_closeaccount.py
@@ -31,6 +31,7 @@ from lp.code.tests.helpers import GitHostingFixture
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.teammembership import ITeamMembershipSet
 from lp.registry.model.productrelease import ProductRelease
+from lp.registry.model.teammembership import TeamParticipation
 from lp.registry.personmerge import merge_people
 from lp.registry.scripts.closeaccount import CloseAccountScript
 from lp.scripts.garbo import PopulateLatestPersonSourcePackageReleaseCache
@@ -920,6 +921,31 @@ class TestCloseAccount(TestCaseWithFactory):
             self.runScript(script)
         self.assertRemoved(account_id, person_id)
 
+    def test_handles_teamparticipation_except_self(self):
+        person = self.factory.makePerson()
+        team = self.factory.makeTeam(members=[person])
+        store = Store.of(person)
+        person_id = person.id
+        account_id = person.account.id
+        self.assertContentEqual(
+            [person_id, team.id],
+            store.find(
+                TeamParticipation.teamID,
+                TeamParticipation.personID == person_id,
+            ),
+        )
+        script = self.makeScript([person.name])
+        with dbuser("launchpad"):
+            self.runScript(script)
+        self.assertRemoved(account_id, person_id)
+        self.assertContentEqual(
+            [person_id],
+            store.find(
+                TeamParticipation.teamID,
+                TeamParticipation.personID == person_id,
+            ),
+        )
+
     def test_skips_teamowner_merged(self):
         person = self.factory.makePerson()
         merged_person = self.factory.makePerson()