launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/close-account-distinct-person into lp:launchpad.

Commit message:
Make close-account work for users with multiple email addresses.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/close-account-distinct-person/+merge/370483

The explicit order_by is needed because Person is sorted by a custom person_sort_key function by default, and that isn't interesting in this case.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/close-account-distinct-person into lp:launchpad.

=== modified file 'lib/lp/registry/scripts/closeaccount.py'
--- lib/lp/registry/scripts/closeaccount.py	2019-05-06 14:22:34 +0000
+++ lib/lp/registry/scripts/closeaccount.py	2019-07-23 12:42:58 +0000
@@ -63,7 +63,8 @@
     ).find(
         Person,
         Or(Person.name == username,
-           Lower(EmailAddress.email) == Lower(username))).one()
+           Lower(EmailAddress.email) == Lower(username))
+    ).order_by(Person.id).config(distinct=True).one()
     if person is None:
         raise LaunchpadScriptFailure("User %s does not exist" % username)
     person_name = person.name

=== modified file 'lib/lp/registry/scripts/tests/test_closeaccount.py'
--- lib/lp/registry/scripts/tests/test_closeaccount.py	2019-05-06 14:22:34 +0000
+++ lib/lp/registry/scripts/tests/test_closeaccount.py	2019-07-23 12:42:58 +0000
@@ -216,6 +216,14 @@
         self.assertRemoved(account_ids[1], person_ids[1])
         self.assertNotRemoved(account_ids[2], person_ids[2])
 
+    def test_multiple_email_addresses(self):
+        person, person_id, account_id = self.makePopulatedUser()
+        self.factory.makeEmail('%s@xxxxxxxxxxxxxxxxxxx' % person.name, person)
+        script = self.makeScript([six.ensure_str(person.name)])
+        with dbuser('launchpad'):
+            self.runScript(script)
+        self.assertRemoved(account_id, person_id)
+
     def test_unactivated(self):
         person = self.factory.makePerson(
             account_status=AccountStatus.NOACCOUNT)