Merge lp:~edwin-grubbs/launchpad/bug-248518-setPreferredEmail into lp:launchpad

[Edwin Grubbs]

Summary
-------

Person.setPreferredEmail() no longer activates the account automatically.

Implementation details
----------------------

Changed setPreferredEmail and fixed tests.
    lib/lp/registry/model/person.py
    lib/lp/registry/doc/person.txt
    lib/lp/testing/factory.py

Fixed lint errors.
    lib/lp/bugs/browser/bugalsoaffects.py
    lib/lp/bugs/browser/tests/bugtask-adding-views.txt

Tests
-----

./bin/test -vv -t 'doc/person.txt|bugtask-adding-views.txt'

Demo and Q/A
------------

This has no effect on the UI.

[Curtis Hovey (sinzui)]

I am surprised there is not comparable test for account.activate() in doc/account. I think your test is the only documentation we have. My initial though was to move your test, but I think your decision was right. I think your tests tells other launchpad developer how to get a person's account activated.

I like your graceful solution in the factory.

review: Approve (code)

=== modified file 'lib/lp/bugs/browser/bugalsoaffects.py'
--- lib/lp/bugs/browser/bugalsoaffects.py	2010-04-23 20:24:36 +0000
+++ lib/lp/bugs/browser/bugalsoaffects.py	2010-04-23 20:24:38 +0000
@@ -593,7 +593,7 @@
         elif link_upstream_how == LinkUpstreamHowOptions.EMAIL_UPSTREAM_DONE:
             # Ensure there's a bug tracker for this email address.
             bug_url = 'mailto:' + data['upstream_email_address_done']
-            bug_tracker = getUtility(IBugTrackerSet).ensureBugTracker(
+            getUtility(IBugTrackerSet).ensureBugTracker(
                 bug_url, self.user, BugTrackerType.EMAILADDRESS)
             data['bug_url'] = bug_url
 

=== modified file 'lib/lp/bugs/browser/tests/bugtask-adding-views.txt'
--- lib/lp/bugs/browser/tests/bugtask-adding-views.txt	2010-04-23 20:24:36 +0000
+++ lib/lp/bugs/browser/tests/bugtask-adding-views.txt	2010-04-23 20:24:38 +0000
@@ -12,7 +12,6 @@
     >>> len(bug_four.bugtasks)
     1
     >>> firefox_task = bug_four.bugtasks[0]
-    >>> from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 
 The views registered at +choose-affected-product and +distrotask are in
 fact meta views responsible for calling other views in order to guide the

=== modified file 'lib/lp/registry/doc/person.txt'
--- lib/lp/registry/doc/person.txt	2010-04-23 20:24:36 +0000
+++ lib/lp/registry/doc/person.txt	2010-04-23 20:24:38 +0000
@@ -119,11 +119,9 @@
 
 By default, newly created Person entries will have
 AccountStatus.NOACCOUNT as their account_status. This is only changed
-if/when we turn that entry into an actual user account. (Currently, we
-assume that a Person entry is turned into a user account when its first
-email address is confirmed) Note that both the Person and the
-EmailAddress have accounts when they are created using the
-createPersonAndEmail() method.
+if/when we turn that entry into an actual user account.  Note that both
+the Person and the EmailAddress have accounts when they are created
+using the createPersonAndEmail() method.
 
     >>> p.account_status
     <DBItem AccountStatus.NOACCOUNT...
@@ -136,6 +134,16 @@
     <DBItem EmailAddressStatus.PREFERRED...
 
     >>> p.account_status
+    <DBItem AccountStatus.NOACCOUNT...
+
+    >>> from canonical.launchpad.interfaces.lpstorm import IMasterStore
+    >>> from canonical.launchpad.database.account import Account
+    >>> account = IMasterStore(Account).get(Account, p.accountID)
+    >>> account.activate(
+    ...     "Activated by doc test.",
+    ...     password=p.password,
+    ...     preferred_email=email)
+    >>> p.account_status
     <DBItem AccountStatus.ACTIVE...
 
 The user can add additional email addresses. The

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2010-04-23 14:22:30 +0000
+++ lib/lp/registry/model/person.py	2010-04-23 20:24:38 +0000
@@ -2091,17 +2091,6 @@
         if email is None:
             self._unsetPreferredEmail()
             return
-        if (self.preferredemail is None
-            and self.account_status != AccountStatus.ACTIVE):
-            # XXX sinzui 2008-07-14 bug=248518:
-            # This is a hack to preserve this function's behaviour before
-            # Account was split from Person. This can be removed when
-            # all the callsites ensure that the account is ACTIVE first.
-            account = IMasterStore(Account).get(Account, self.accountID)
-            account.activate(
-                "Activated when the preferred email was set.",
-                password=self.password,
-                preferred_email=email)
         self._setPreferredEmail(email)
 
     def _setPreferredEmail(self, email):

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-04-23 02:35:47 +0000
+++ lib/lp/testing/factory.py	2010-04-23 20:24:38 +0000
@@ -400,10 +400,10 @@
         # To make the person someone valid in Launchpad, validate the
         # email.
         if email_address_status == EmailAddressStatus.PREFERRED:
-            person.validateAndEnsurePreferredEmail(email)
             account = IMasterStore(Account).get(
                 Account, person.accountID)
             account.status = AccountStatus.ACTIVE
+            person.validateAndEnsurePreferredEmail(email)
 
         removeSecurityProxy(email).status = email_address_status
 
@@ -448,6 +448,13 @@
             PersonCreationRationale.OWNER_CREATED_LAUNCHPAD,
             name=variable_name, displayname=full_name)
         if set_preferred_email:
+            # setPreferredEmail no longer activates the account
+            # automatically.
+            account = IMasterStore(Account).get(Account, person.accountID)
+            account.activate(
+                "Activated by factory.makePersonByName",
+                password='foo',
+                preferred_email=email)
             person.setPreferredEmail(email)
 
         if not use_default_autosubscribe_policy:
@@ -1358,7 +1365,7 @@
     def makePackageCodeImport(self, sourcepackage=None, **kwargs):
         """Make a code import targetting a sourcepackage."""
         if sourcepackage is None:
-           sourcepackage = self.makeSourcePackage()
+            sourcepackage = self.makeSourcePackage()
         target = IBranchTarget(sourcepackage)
         return self.makeCodeImport(target=target, **kwargs)