William Grant has proposed merging lp:~wgrant/launchpad/avoid-createPersonAndEmail into lp:launchpad.
Commit message: Avoid direct Person.createPersonAndEmail calls. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~wgrant/launchpad/avoid-createPersonAndEmail/+merge/306101 Avoid direct Person.createPersonAndEmail calls. The one non-test public callsite should have been using ensurePerson instead, and lots of tests should have been using factory.makePerson. -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/avoid-createPersonAndEmail into lp:launchpad.
=== modified file 'lib/lp/bugs/scripts/bugimport.py' --- lib/lp/bugs/scripts/bugimport.py 2015-09-30 01:51:52 +0000 +++ lib/lp/bugs/scripts/bugimport.py 2016-09-19 13:46:01 +0000 @@ -172,34 +172,18 @@ person_set = getUtility(IPersonSet) launchpad_id = self.person_id_cache.get(email) - if launchpad_id is not None: - person = person_set.get(launchpad_id) - if person is not None and person.merged is not None: - person = None - else: - person = None - - if person is None: - person = getUtility(IPersonSet).getByEmail( - email, - filter_status=False) - - if person is None: - self.logger.debug('creating person for %s' % email) - # Has the short name been taken? - if name is not None and ( - person_set.getByName(name) is not None): - # The short name is already taken, so we'll pass - # None to createPersonAndEmail(), which will take - # care of creating a unique one. - name = None - person, address = ( - person_set.createPersonAndEmail( - email=email, name=name, displayname=displayname, - rationale=PersonCreationRationale.BUGIMPORT, - comment=('when importing bugs for %s' % - self.product.displayname))) - + person = person_set.get(launchpad_id) if launchpad_id else None + if person is None or person.merged is not None: + if name is not None and person_set.getByName(name) is not None: + # The short name is already taken, so we'll pass None to + # ensurePerson(), which will take care of generating a + # unique one if a new Person is required. + name = None + person = getUtility(IPersonSet).ensurePerson( + email=email, name=name, displayname=displayname, + rationale=PersonCreationRationale.BUGIMPORT, + comment=( + 'when importing bugs for %s' % self.product.displayname)) self.person_id_cache[email] = person.id # if we are auto-verifying new accounts, make sure the person === modified file 'lib/lp/bugs/scripts/tests/test_bugimport.py' --- lib/lp/bugs/scripts/tests/test_bugimport.py 2015-07-08 16:05:11 +0000 +++ lib/lp/bugs/scripts/tests/test_bugimport.py 2016-09-19 13:46:01 +0000 @@ -41,6 +41,7 @@ from lp.registry.interfaces.product import IProductSet from lp.services.config import config from lp.services.database.sqlbase import cursor +from lp.services.identity.interfaces.account import AccountStatus from lp.services.identity.interfaces.emailaddress import IEmailAddressSet from lp.testing import ( login, @@ -209,13 +210,7 @@ def test_find_existing_person(self): # Test that getPerson() returns an existing person. - person = getUtility(IPersonSet).getByEmail('f...@example.com') - self.assertEqual(person, None) - person, email = getUtility(IPersonSet).createPersonAndEmail( - email='f...@example.com', - rationale=PersonCreationRationale.OWNER_CREATED_LAUNCHPAD) - self.assertNotEqual(person, None) - + person = self.factory.makePerson(email='f...@example.com') product = getUtility(IProductSet).getByName('netapplet') importer = bugimport.BugImporter( product, 'bugs.xml', 'bug-map.pickle') @@ -257,11 +252,9 @@ def test_verify_existing_person(self): # Test that getPerson() will validate the email of an existing # user when verify_users=True. - person, email = getUtility(IPersonSet).createPersonAndEmail( - rationale=PersonCreationRationale.OWNER_CREATED_LAUNCHPAD, - email='f...@example.com') + person = self.factory.makePerson( + email='f...@example.com', account_status=AccountStatus.NOACCOUNT) self.assertEqual(person.preferredemail, None) - product = getUtility(IProductSet).getByName('netapplet') importer = bugimport.BugImporter( product, 'bugs.xml', 'bug-map.pickle', verify_users=True) @@ -276,15 +269,9 @@ def test_verify_doesnt_clobber_preferred_email(self): # Test that getPerson() does not clobber an existing verified # email address when verify_users=True. - person, email = getUtility(IPersonSet).createPersonAndEmail( - 'f...@example.com', - PersonCreationRationale.OWNER_CREATED_LAUNCHPAD) - transaction.commit() - self.failIf(person.account is None, 'Person must have an account.') - email = getUtility(IEmailAddressSet).new( - 'f...@preferred.com', person) + person = self.factory.makePerson(email='f...@example.com') + email = getUtility(IEmailAddressSet).new('f...@preferred.com', person) person.setPreferredEmail(email) - transaction.commit() self.assertEqual(person.preferredemail.email, 'f...@preferred.com') product = getUtility(IProductSet).getByName('netapplet') === modified file 'lib/lp/registry/doc/vocabularies.txt' --- lib/lp/registry/doc/vocabularies.txt 2016-04-28 02:25:46 +0000 +++ lib/lp/registry/doc/vocabularies.txt 2016-09-19 13:46:01 +0000 @@ -564,19 +564,14 @@ A person with a single and unvalidated email address can be merged. - >>> from lp.registry.interfaces.person import PersonCreationRationale - >>> fooperson, email = person_set.createPersonAndEmail( - ... 'foo...@bar.com', PersonCreationRationale.UNKNOWN, - ... name='foobaz', displayname='foo baz') - >>> import transaction - >>> transaction.commit() + >>> from lp.services.identity.interfaces.account import AccountStatus + >>> fooperson = factory.makePerson(account_status=AccountStatus.NOACCOUNT) >>> fooperson in vocab True But any person without a single email address can't. - >>> email.destroySelf() - >>> transaction.commit() + >>> fooperson.guessedemails[0].destroySelf() >>> fooperson in vocab False @@ -887,7 +882,6 @@ ... symbolic_person, 'chat.freenode.net', '%percent') >>> irc_id =ircid_set.new( ... symbolic_person, 'irc.fnord.net', 'question?') - >>> transaction.commit() >>> for person in vocab.search('%percent'): ... print person.name === modified file 'lib/lp/registry/model/person.py' --- lib/lp/registry/model/person.py 2016-07-12 13:32:10 +0000 +++ lib/lp/registry/model/person.py 2016-09-19 13:46:01 +0000 @@ -3613,15 +3613,14 @@ return person def ensurePerson(self, email, displayname, rationale, comment=None, - registrant=None): + registrant=None, name=None): """See `IPersonSet`.""" - person = getUtility(IPersonSet).getByEmail( - email, - filter_status=False) + person = getUtility(IPersonSet).getByEmail(email, filter_status=False) if person is None: person, email_address = self.createPersonAndEmail( - email, rationale, comment=comment, displayname=displayname, - registrant=registrant, hide_email_addresses=True) + email, rationale, name=name, comment=comment, + displayname=displayname, registrant=registrant, + hide_email_addresses=True) return person def getByName(self, name, ignore_merged=True):
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp