launchpad-reviewers team mailing list archive

[Clinton Fung <mp+436973@xxxxxxxxxxxxxxxxxx>]

Clinton Fung has proposed merging ~clinton-fung/launchpad:account-deletion-person-merge-skip01 into launchpad:master.

Commit message:
Add a skip for person.merged (and also personnotification.person) to allow account closure to proceed.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~clinton-fung/launchpad/+git/launchpad/+merge/436973

When merging two accounts, the source (from) account has a field set that links it to the target (to) account, for audit purposes. This link prevents account deletion. Add a skip to allow deletion to complete in this scenario.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~clinton-fung/launchpad:account-deletion-person-merge-skip01 into launchpad:master.

diff --git a/lib/lp/registry/scripts/closeaccount.py b/lib/lp/registry/scripts/closeaccount.py
index f673a02..cb2bf4f 100644
--- a/lib/lp/registry/scripts/closeaccount.py
+++ b/lib/lp/registry/scripts/closeaccount.py
@@ -140,7 +140,9 @@ def close_account(username, log):
         ("packagecopyrequest", "requester"),
         ("packagediff", "requester"),
         ("packageupload", "signing_key_owner"),
+        ("person", "merged"),
         ("personlocation", "last_modified_by"),
+        ("personnotification", "person"),
         ("persontransferjob", "major_person"),
         ("persontransferjob", "minor_person"),
         ("poexportrequest", "person"),
diff --git a/lib/lp/registry/scripts/tests/test_closeaccount.py b/lib/lp/registry/scripts/tests/test_closeaccount.py
index 41ca7e5..d6b0778 100644
--- a/lib/lp/registry/scripts/tests/test_closeaccount.py
+++ b/lib/lp/registry/scripts/tests/test_closeaccount.py
@@ -32,15 +32,20 @@ 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.personmerge import merge_people
 from lp.registry.scripts.closeaccount import CloseAccountScript
 from lp.scripts.garbo import PopulateLatestPersonSourcePackageReleaseCache
+from lp.services.config import config
 from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import (
     flush_database_caches,
     get_transaction_timestamp,
 )
 from lp.services.identity.interfaces.account import AccountStatus, IAccountSet
-from lp.services.identity.interfaces.emailaddress import IEmailAddressSet
+from lp.services.identity.interfaces.emailaddress import (
+    EmailAddressStatus,
+    IEmailAddressSet,
+)
 from lp.services.job.interfaces.job import JobType
 from lp.services.job.model.job import Job
 from lp.services.log.logger import BufferLogger, DevNullLogger
@@ -51,7 +56,11 @@ from lp.soyuz.enums import ArchiveSubscriberStatus, PackagePublishingStatus
 from lp.soyuz.model.archive import Archive
 from lp.soyuz.model.archiveauthtoken import ArchiveAuthToken
 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
-from lp.testing import TestCaseWithFactory, login_celebrity
+from lp.testing import (
+    TestCaseWithFactory,
+    celebrity_logged_in,
+    login_celebrity,
+)
 from lp.testing.dbuser import dbuser
 from lp.testing.layers import LaunchpadZopelessLayer
 from lp.translations.interfaces.pofiletranslator import IPOFileTranslatorSet
@@ -1074,6 +1083,28 @@ class TestCloseAccount(TestCaseWithFactory):
 
         self.assertRemoved(account_id, person_id)
 
+    def test_skips_merged_and_personnotification_references(self):
+        from_person = self.factory.makePerson()
+        to_person = self.factory.makePerson()
+
+        # See TestMergePeople._do_premerge()
+        with celebrity_logged_in("admin"):
+            email = from_person.preferredemail
+            email.status = EmailAddressStatus.NEW
+            email.person = to_person
+        transaction.commit()
+
+        # See TestMergePeople._do_merge()
+        with dbuser(config.IPersonMergeJobSource.dbuser):
+            merge_people(from_person, to_person, None)
+
+        account_id = to_person.account.id
+        person_id = to_person.id
+        script = self.makeScript([to_person.name])
+        with dbuser("launchpad"):
+            self.runScript(script)
+        self.assertRemoved(account_id, person_id)
+
     def test_non_product_announcements_are_not_skipped(self):
         person = self.factory.makePerson()
         person_id = person.id