launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-1169441 into lp:launchpad.

Commit message:
Ensure that we create the email address during account reactivation if it doesn't already exist.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1169441 in Launchpad itself: "PersonSet.getOrCreateByOpenIDIdentifier fails to set preferredemail when reactivating an account with a new address"
  https://bugs.launchpad.net/launchpad/+bug/1169441

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-1169441/+merge/159086

PersonSet.getOrCreateByOpenIDIdentifier will only create a new EmailAddress as part of a new Person. This proves problematic when we're reactivating an account using an address that isn't already on the account, as the person.setPreferredEmail(email) call does nothing. We end up with an active account with no preferredemail, which crashes.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-1169441/+merge/159086
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-1169441 into lp:launchpad.

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2013-03-12 08:18:57 +0000
+++ lib/lp/registry/model/person.py	2013-04-16 06:56:40 +0000
@@ -3370,6 +3370,9 @@
             elif person.account.status in [AccountStatus.DEACTIVATED,
                                            AccountStatus.NOACCOUNT]:
                 removeSecurityProxy(person.account).reactivate(comment)
+                if email is None:
+                    email = getUtility(IEmailAddressSet).new(
+                        email_address, person)
                 removeSecurityProxy(person).setPreferredEmail(email)
                 db_updated = True
             else:

=== modified file 'lib/lp/registry/tests/test_personset.py'
--- lib/lp/registry/tests/test_personset.py	2013-01-10 06:46:52 +0000
+++ lib/lp/registry/tests/test_personset.py	2013-04-16 06:56:40 +0000
@@ -885,6 +885,20 @@
             u'other-openid-identifier', 'foo@xxxxxxx', 'New Name',
             PersonCreationRationale.UNKNOWN, 'No Comment')
 
+    def testDeactivatedAccount(self):
+        # Logging into a deactivated account with a new email address
+        # reactivates the account, adds that email address, and sets it
+        # as preferred.
+        addr = 'not@an.address'
+        self.person.preDeactivate('I hate life.')
+        self.assertEqual(AccountStatus.DEACTIVATED, self.person.account_status)
+        self.assertIs(None, self.person.preferredemail)
+        found, updated = self.person_set.getOrCreateByOpenIDIdentifier(
+            self.identifier.identifier, addr, 'New Name',
+            PersonCreationRationale.UNKNOWN, 'No Comment')
+        self.assertEqual(AccountStatus.ACTIVE, self.person.account_status)
+        self.assertEqual(addr, self.person.preferredemail.email)
+
 
 class TestCreatePersonAndEmail(TestCase):
     """Test `IPersonSet`.createPersonAndEmail()."""