← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cprov/launchpad/strict-usernames into lp:launchpad

 

Celso Providelo has proposed merging lp:~cprov/launchpad/strict-usernames into lp:launchpad.

Commit message:
Setting stricter restrictions for LP usernames.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cprov/launchpad/strict-usernames/+merge/366985

This is part of wider effort to have cleaner usernames across all Canonical services. 

https://docs.google.com/document/d/11qu6Mc6My6AKvIn8-VnnbA5Esp7IvqHMldeFpomUVSA/edit

The current goal is to stop new users to get in with "unclean" names. SSO already forces clean names for new users, with this MP so will LP.

Next we will introduce a way to allow Store users to pick a alternative "clean" username and eventually switch to it.

-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cprov/launchpad/strict-usernames into lp:launchpad.
=== modified file 'lib/lp/app/validators/name.py'
--- lib/lp/app/validators/name.py	2012-11-29 05:52:36 +0000
+++ lib/lp/app/validators/name.py	2019-05-06 17:01:07 +0000
@@ -19,6 +19,7 @@
 valid_name_pattern = re.compile(r"^[a-z0-9][a-z0-9\+\.\-]+$")
 valid_bug_name_pattern = re.compile(r"^[a-z][a-z0-9\+\.\-]+$")
 invalid_name_pattern = re.compile(r"^[^a-z0-9]+|[^a-z0-9\\+\\.\\-]+")
+valid_username_pattern = re.compile(r"(?!^[\d-]+$)^[a-z\d](-?[a-z\d]){2,31}$")
 
 
 def sanitize_name(name):
@@ -67,6 +68,53 @@
     return False
 
 
+def valid_username(name):
+    """Return True if the username is valid, otherwise False.
+
+    Lauchpad username (`Person.name`) attribute is designed to be used
+    in URLs across products (via SSO) and thus complies with very specific
+    validation (and taste) criteria.
+
+    >>> valid_username('hello')
+    True
+    >>> valid_username('a12')
+    True
+    >>> valid_username('a-12')
+    True
+    >>> valid_username('a-1-2')
+    True
+    >>> valid_username('a' * 32)
+    True
+
+
+    >>> valid_username('a-1')
+    False
+    >>> valid_username('1-1')
+    False
+    >>> valid_username('a-a')
+    False
+    >>> valid_username('he--o')
+    False
+    >>> valid_username('-ello')
+    False
+    >>> valid_username('hell-')
+    False
+    >>> valid_username('helLo')
+    False
+    >>> valid_username('he.lo')
+    False
+    >>> valid_username('he+lo')
+    False
+    >>> valid_username('hh')
+    False
+    >>> valid_username(33 * 'h')
+    False
+    """
+    if valid_username_pattern.match(name):
+        return True
+    return False
+
+
 def name_validator(name):
     """Return True if the name is valid, or raise a
     LaunchpadValidationError.
@@ -97,3 +145,18 @@
 
         raise LaunchpadValidationError(structured(message))
     return True
+
+
+def username_validator(name):
+    """Return True if the `Person.name` is valid, or raise a
+    LaunchpadValidationError.
+    """
+    if not valid_username(name):
+        message = _(dedent("""
+            Invalid username '${name}'. Usernames must be at least two characters long
+            and start with a letter or number. All letters must be lower-case.
+            The character <samp>-</samp> is allowed after the first character."""),
+            mapping={'name': html_escape(name)})
+
+        raise LaunchpadValidationError(structured(message))
+    return True

=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_filebug.py'
--- lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2017-10-21 18:14:14 +0000
+++ lib/lp/bugs/browser/tests/test_bugtarget_filebug.py	2019-05-06 17:01:07 +0000
@@ -635,12 +635,12 @@
 
     def test_subscribers_with_name(self):
         # The extra data can add bug subscribers via Launchpad Id..
-        subscriber_1 = self.factory.makePerson(name='me')
+        subscriber_1 = self.factory.makePerson(name='you')
         subscriber_2 = self.factory.makePerson(name='him')
-        token = self.process_extra_data(command='Subscribers: me him')
+        token = self.process_extra_data(command='Subscribers: you him')
         view = self.create_initialized_view()
         view.publishTraverse(view.request, token)
-        self.assertContentEqual(['me', 'him'], view.extra_data.subscribers)
+        self.assertContentEqual(['you', 'him'], view.extra_data.subscribers)
         view.submit_bug_action.success(self.get_form())
         transaction.commit()
         bug = view.added_bug

=== modified file 'lib/lp/bugs/doc/bugsummary.txt'
--- lib/lp/bugs/doc/bugsummary.txt	2018-06-29 23:10:57 +0000
+++ lib/lp/bugs/doc/bugsummary.txt	2019-05-06 17:01:07 +0000
@@ -416,9 +416,9 @@
 For our examples, first create three people. person_z will not
 be subscribed to any bugs, so will have no access to any private bugs.
 
-    >>> person_a = factory.makePerson(name='p-a')
-    >>> person_b = factory.makePerson(name='p-b')
-    >>> person_z = factory.makePerson(name='p-z')
+    >>> person_a = factory.makePerson(name='pe-a')
+    >>> person_b = factory.makePerson(name='pe-b')
+    >>> person_z = factory.makePerson(name='pe-z')
     >>> owner = factory.makePerson(name='own')
 
 Create some teams too. team_a just has person_a as a member. team_c
@@ -470,7 +470,7 @@
     --------------------------------------------------------------------
     prod ps   dist ds   spn   tag mile status import pa gra  pol       #
     --------------------------------------------------------------------
-    x    x    di-p x    x     x   x    New    Undeci F  p-b  x         1
+    x    x    di-p x    x     x   x    New    Undeci F  pe-b x         1
     x    x    di-p x    x     x   x    New    Undeci F  own  x         3
     x    x    di-p x    x     x   x    New    Undeci F  t-a  x         1
     x    x    di-p x    x     x   x    New    Undeci F  t-c  x         1

=== modified file 'lib/lp/bugs/doc/bugtracker-person.txt'
--- lib/lp/bugs/doc/bugtracker-person.txt	2018-06-29 23:10:57 +0000
+++ lib/lp/bugs/doc/bugtracker-person.txt	2019-05-06 17:01:07 +0000
@@ -117,7 +117,7 @@
     ...     creation_comment='whilst testing ensurePersonForSelf().')
 
     >>> print(noemail_person.name)
-    no-email-person-bugzilla-checkwatches
+    no-email-bugzilla-checkwatches
 
 A BugTrackerPerson record will have been created to map
 'No-Email-Person' on our example bugtracker to
@@ -183,4 +183,3 @@
 
     >>> print(new_person.name)
     noemail-bugzilla-checkwatches-1
-

=== modified file 'lib/lp/bugs/model/bugtracker.py'
--- lib/lp/bugs/model/bugtracker.py	2019-02-23 08:15:45 +0000
+++ lib/lp/bugs/model/bugtracker.py	2019-05-06 17:01:07 +0000
@@ -645,9 +645,18 @@
         if bugtracker_person is not None:
             return bugtracker_person.person
 
-        # Generate a valid Launchpad name for the Person.
-        base_canonical_name = (
-            "%s-%s" % (sanitize_name(display_name.lower()), self.name))
+        # Generate a valid Launchpad name for the Person extracting an
+        # username from the given display_name smaller enough to be joined
+        # with the tracker name and up to 2 digits (99) for disambiguation
+        # and fit in 32 characters limit and avoid consecutive hyphens.
+        remaining = 32 - len(self.name) - 2
+        username = sanitize_name(display_name.lower())[:remaining]
+        username = username if not username.endswith('-') else username[:-1]
+
+        # XXX cprov 2019-05-06: bugtracker names may be longer than 32-chars,
+        # we need extra name-cropping if this simple strategy gets accepted.
+
+        base_canonical_name = "%s-%s" % (username, self.name)
         canonical_name = base_canonical_name
 
         person_set = getUtility(IPersonSet)

=== modified file 'lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py'
--- lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py	2018-01-26 14:38:31 +0000
+++ lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py	2019-05-06 17:01:07 +0000
@@ -54,8 +54,8 @@
 
     layer = DatabaseFunctionalLayer
 
-    name_pairs = ("A", "xa"), ("C", "xd"), ("B", "xb"), ("C", "xc")
-    name_pairs_sorted = ("A", "xa"), ("B", "xb"), ("C", "xc"), ("C", "xd")
+    name_pairs = ("A", "xaa"), ("C", "xdd"), ("B", "xbb"), ("C", "xcc")
+    name_pairs_sorted = ("A", "xaa"), ("B", "xbb"), ("C", "xcc"), ("C", "xdd")
 
     def setUp(self):
         super(TestSubscriptionRelatedSets, self).setUp()

=== modified file 'lib/lp/code/model/tests/test_branchlookup.py'
--- lib/lp/code/model/tests/test_branchlookup.py	2017-10-04 01:53:48 +0000
+++ lib/lp/code/model/tests/test_branchlookup.py	2019-05-06 17:01:07 +0000
@@ -285,7 +285,7 @@
         # XXX: JonathanLange 2009-01-13 spec=package-branches: This test is
         # bad because it assumes that the interesting branches for testing are
         # product branches.
-        owner = self.factory.makePerson(name='aa')
+        owner = self.factory.makePerson(name='aaa')
         product = self.factory.makeProduct('b')
         return self.factory.makeProductBranch(
             owner=owner, product=product, name='c')
@@ -298,14 +298,14 @@
         # Trailing slashes are stripped from the url prior to searching.
         branch = self.makeProductBranch()
         lookup = getUtility(IBranchLookup)
-        branch2 = lookup.getByUrl('http://bazaar.launchpad.dev/~aa/b/c/')
+        branch2 = lookup.getByUrl('http://bazaar.launchpad.dev/~aaa/b/c/')
         self.assertEqual(branch, branch2)
 
     def test_getByUrl_with_http(self):
         """getByUrl recognizes LP branches for http URLs."""
         branch = self.makeProductBranch()
         branch_set = getUtility(IBranchLookup)
-        branch2 = branch_set.getByUrl('http://bazaar.launchpad.dev/~aa/b/c')
+        branch2 = branch_set.getByUrl('http://bazaar.launchpad.dev/~aaa/b/c')
         self.assertEqual(branch, branch2)
 
     def test_getByUrl_with_ssh(self):
@@ -313,14 +313,14 @@
         branch = self.makeProductBranch()
         branch_set = getUtility(IBranchLookup)
         branch2 = branch_set.getByUrl(
-            'bzr+ssh://bazaar.launchpad.dev/~aa/b/c')
+            'bzr+ssh://bazaar.launchpad.dev/~aaa/b/c')
         self.assertEqual(branch, branch2)
 
     def test_getByUrl_with_sftp(self):
         """getByUrl recognizes LP branches for sftp URLs."""
         branch = self.makeProductBranch()
         branch_set = getUtility(IBranchLookup)
-        branch2 = branch_set.getByUrl('sftp://bazaar.launchpad.dev/~aa/b/c')
+        branch2 = branch_set.getByUrl('sftp://bazaar.launchpad.dev/~aaa/b/c')
         self.assertEqual(branch, branch2)
 
     def test_getByUrl_with_ftp(self):
@@ -330,15 +330,15 @@
         """
         self.makeProductBranch()
         branch_set = getUtility(IBranchLookup)
-        branch2 = branch_set.getByUrl('ftp://bazaar.launchpad.dev/~aa/b/c')
+        branch2 = branch_set.getByUrl('ftp://bazaar.launchpad.dev/~aaa/b/c')
         self.assertIs(None, branch2)
 
     def test_getByURL_with_lp_prefix(self):
         """lp: URLs for the configured prefix are supported."""
         branch_set = getUtility(IBranchLookup)
-        url = '%s~aa/b/c' % config.codehosting.bzr_lp_prefix
+        url = '%s~aaa/b/c' % config.codehosting.bzr_lp_prefix
         self.assertIs(None, branch_set.getByUrl(url))
-        owner = self.factory.makePerson(name='aa')
+        owner = self.factory.makePerson(name='aaa')
         product = self.factory.makeProduct('b')
         branch2 = branch_set.getByUrl(url)
         self.assertIs(None, branch2)
@@ -352,13 +352,13 @@
         branch_set = getUtility(IBranchLookup)
         branch = self.makeProductBranch()
         self.pushConfig('codehosting', lp_url_hosts='production,,')
-        branch2 = branch_set.getByUrl('lp://staging/~aa/b/c')
-        self.assertIs(None, branch2)
-        branch2 = branch_set.getByUrl('lp://asdf/~aa/b/c')
-        self.assertIs(None, branch2)
-        branch2 = branch_set.getByUrl('lp:~aa/b/c')
+        branch2 = branch_set.getByUrl('lp://staging/~aaa/b/c')
+        self.assertIs(None, branch2)
+        branch2 = branch_set.getByUrl('lp://asdf/~aaa/b/c')
+        self.assertIs(None, branch2)
+        branch2 = branch_set.getByUrl('lp:~aaa/b/c')
         self.assertEqual(branch, branch2)
-        branch2 = branch_set.getByUrl('lp://production/~aa/b/c')
+        branch2 = branch_set.getByUrl('lp://production/~aaa/b/c')
         self.assertEqual(branch, branch2)
 
     def test_getByUrl_with_alias(self):
@@ -542,13 +542,13 @@
         # product component isn't, then `NoSuchBranch` if the first two
         # components are found.
         self.assertRaises(
-            NoSuchPerson, self.branch_lookup.getByLPPath, '~aa/bb/c')
-        self.factory.makePerson(name='aa')
+            NoSuchPerson, self.branch_lookup.getByLPPath, '~aaa/bb/c')
+        self.factory.makePerson(name='aaa')
         self.assertRaises(
-            NoSuchProduct, self.branch_lookup.getByLPPath, '~aa/bb/c')
+            NoSuchProduct, self.branch_lookup.getByLPPath, '~aaa/bb/c')
         self.factory.makeProduct(name='bb')
         self.assertRaises(
-            NoSuchBranch, self.branch_lookup.getByLPPath, '~aa/bb/c')
+            NoSuchBranch, self.branch_lookup.getByLPPath, '~aaa/bb/c')
 
     def test_private_branch(self):
         # If the unique name refers to an invisible branch, getByLPPath raises
@@ -590,10 +590,10 @@
         # match an existing person, and `NoSuchBranch` if the last component
         # doesn't match an existing branch.
         self.assertRaises(
-            NoSuchPerson, self.branch_lookup.getByLPPath, '~aa/+junk/c')
-        self.factory.makePerson(name='aa')
+            NoSuchPerson, self.branch_lookup.getByLPPath, '~aaa/+junk/c')
+        self.factory.makePerson(name='aaa')
         self.assertRaises(
-            NoSuchBranch, self.branch_lookup.getByLPPath, '~aa/+junk/c')
+            NoSuchBranch, self.branch_lookup.getByLPPath, '~aaa/+junk/c')
 
     def test_resolve_personal_branch_unique_name(self):
         # getByLPPath returns the branch, no trailing path and no series if

=== modified file 'lib/lp/code/model/tests/test_gitlookup.py'
--- lib/lp/code/model/tests/test_gitlookup.py	2018-07-23 10:28:33 +0000
+++ lib/lp/code/model/tests/test_gitlookup.py	2019-05-06 17:01:07 +0000
@@ -195,7 +195,7 @@
         self.lookup = getUtility(IGitLookup)
 
     def makeProjectRepository(self):
-        owner = self.factory.makePerson(name="aa")
+        owner = self.factory.makePerson(name="aaa")
         project = self.factory.makeProduct(name="bb")
         return self.factory.makeGitRepository(
             owner=owner, target=project, name="cc")
@@ -211,47 +211,47 @@
         # Trailing slashes are stripped from the URL prior to searching.
         repository = self.makeProjectRepository()
         self.assertUrlMatches(
-            "git://git.launchpad.dev/~aa/bb/+git/cc/", repository)
+            "git://git.launchpad.dev/~aaa/bb/+git/cc/", repository)
 
     def test_getByUrl_with_trailing_segments(self):
         # URLs with trailing segments beyond the repository are rejected.
         self.makeProjectRepository()
         self.assertIsNone(
-            self.lookup.getByUrl("git://git.launchpad.dev/~aa/bb/+git/cc/foo"))
+            self.lookup.getByUrl("git://git.launchpad.dev/~aaa/bb/+git/cc/foo"))
 
     def test_getByUrl_with_git(self):
         # getByUrl recognises LP repositories for git URLs.
         repository = self.makeProjectRepository()
         self.assertUrlMatches(
-            "git://git.launchpad.dev/~aa/bb/+git/cc", repository)
+            "git://git.launchpad.dev/~aaa/bb/+git/cc", repository)
 
     def test_getByUrl_with_git_ssh(self):
         # getByUrl recognises LP repositories for git+ssh URLs.
         repository = self.makeProjectRepository()
         self.assertUrlMatches(
-            "git+ssh://git.launchpad.dev/~aa/bb/+git/cc", repository)
+            "git+ssh://git.launchpad.dev/~aaa/bb/+git/cc", repository)
 
     def test_getByUrl_with_https(self):
         # getByUrl recognises LP repositories for https URLs.
         repository = self.makeProjectRepository()
         self.assertUrlMatches(
-            "https://git.launchpad.dev/~aa/bb/+git/cc";, repository)
+            "https://git.launchpad.dev/~aaa/bb/+git/cc";, repository)
 
     def test_getByUrl_with_ssh(self):
         # getByUrl recognises LP repositories for ssh URLs.
         repository = self.makeProjectRepository()
         self.assertUrlMatches(
-            "ssh://git.launchpad.dev/~aa/bb/+git/cc", repository)
+            "ssh://git.launchpad.dev/~aaa/bb/+git/cc", repository)
 
     def test_getByUrl_with_ftp(self):
         # getByUrl does not recognise LP repositories for ftp URLs.
         self.makeProjectRepository()
         self.assertIsNone(
-            self.lookup.getByUrl("ftp://git.launchpad.dev/~aa/bb/+git/cc";))
+            self.lookup.getByUrl("ftp://git.launchpad.dev/~aaa/bb/+git/cc";))
 
     def test_getByUrl_with_lp(self):
         # getByUrl supports lp: URLs.
-        url = "lp:~aa/bb/+git/cc"
+        url = "lp:~aaa/bb/+git/cc"
         self.assertIsNone(self.lookup.getByUrl(url))
         repository = self.makeProjectRepository()
         self.assertUrlMatches(url, repository)

=== modified file 'lib/lp/registry/browser/tests/test_person.py'
--- lib/lp/registry/browser/tests/test_person.py	2018-01-02 16:10:26 +0000
+++ lib/lp/registry/browser/tests/test_person.py	2019-05-06 17:01:07 +0000
@@ -567,6 +567,25 @@
         self.ppa = self.factory.makeArchive(owner=self.person)
         self.view = create_initialized_view(self.person, '+edit')
 
+    def test_legacy_usernames_do_not_block_edit(self):
+        # Users with legacy usernames (less restrictive) are not forced
+        # to update it along with other details of their account.
+        removeSecurityProxy(self.person).name = 'legacy+name'
+
+        form = {
+            'field.display_name': 'Even Nicer DisplayName',
+            'field.actions.save': 'Save Changes',
+            }
+        view = create_initialized_view(self.person, '+edit', form=form)
+
+        notifications = view.request.response.notifications
+        self.assertEqual(1, len(notifications))
+        self.assertEqual(
+            'The changes to your personal details have been saved.',
+            notifications[0].message)
+        self.assertEqual('Even Nicer DisplayName', self.person.displayname)
+        self.assertEqual('legacy+name', self.person.name)
+
     def createAddEmailView(self, email_address):
         """Test helper to create +editemails view."""
         form = {

=== modified file 'lib/lp/registry/browser/tests/test_team.py'
--- lib/lp/registry/browser/tests/test_team.py	2015-10-26 14:54:43 +0000
+++ lib/lp/registry/browser/tests/test_team.py	2019-05-06 17:01:07 +0000
@@ -523,7 +523,8 @@
         team = self.factory.makeTeam(
             membership_policy=TeamMembershipPolicy.MODERATED)
         self.factory.grantCommercialSubscription(team)
-        team_name = self.factory.getUniqueString()
+        # Pass a prefix to keep team name way under 32-char limit.
+        team_name = self.factory.getUniqueString("simple-team")
         form = {
             'field.name': team_name,
             'field.display_name': 'New Team',
@@ -575,7 +576,8 @@
 
     def test_create_team(self):
         personset = getUtility(IPersonSet)
-        team_name = self.factory.getUniqueString()
+        # Pass a prefix to keep team name way under 32-char limit.
+        team_name = self.factory.getUniqueString("simple-team")
         form = {
             'field.name': team_name,
             'field.display_name': 'New Team',

=== modified file 'lib/lp/registry/doc/person.txt'
--- lib/lp/registry/doc/person.txt	2016-04-14 06:14:59 +0000
+++ lib/lp/registry/doc/person.txt	2019-05-06 17:01:07 +0000
@@ -1346,12 +1346,12 @@
 
     >>> foo_bar = getUtility(IPersonSet).getByEmail('foo.bar@xxxxxxxxxxxxx')
     >>> new_person = person_set.createPersonWithoutEmail(
-    ...     'ix', PersonCreationRationale.BUGIMPORT,
+    ...     'ixx', PersonCreationRationale.BUGIMPORT,
     ...     comment="when importing bugs", displayname="Ford Prefect",
     ...     registrant=foo_bar)
 
     >>> print new_person.name
-    ix
+    ixx
 
     >>> print new_person.displayname
     Ford Prefect

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2018-08-02 16:12:57 +0000
+++ lib/lp/registry/interfaces/person.py	2019-05-06 17:01:07 +0000
@@ -101,7 +101,10 @@
     )
 from lp.app.validators import LaunchpadValidationError
 from lp.app.validators.email import email_validator
-from lp.app.validators.name import name_validator
+from lp.app.validators.name import (
+    name_validator,
+    username_validator,
+    )
 from lp.blueprints.interfaces.specificationtarget import IHasSpecifications
 from lp.bugs.interfaces.bugtarget import IHasBugs
 from lp.code.interfaces.hasbranches import (
@@ -662,11 +665,11 @@
     name = exported(
         PersonNameField(
             title=_('Name'), required=True, readonly=False,
-            constraint=name_validator,
+            constraint=username_validator,
             description=_(
                 "A short unique name, beginning with a lower-case "
                 "letter or number, and containing only letters, "
-                "numbers, dots, hyphens, or plus signs.")))
+                "numbers and non-consecutive hyphens.")))
     display_name = exported(
         StrippedTextLine(
             title=_('Display Name'), required=True, readonly=False,

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2018-11-13 03:48:13 +0000
+++ lib/lp/registry/model/person.py	2019-05-06 17:01:07 +0000
@@ -118,7 +118,7 @@
 from lp.app.validators.email import valid_email
 from lp.app.validators.name import (
     sanitize_name,
-    valid_name,
+    valid_username,
     )
 from lp.blueprints.enums import SpecificationFilter
 from lp.blueprints.model.specification import (
@@ -3478,7 +3478,7 @@
         """See `IPersonSet`."""
         if user != getUtility(ILaunchpadCelebrities).ubuntu_sso:
             raise Unauthorized()
-        self._validateName(name)
+        self._validateUsername(name)
         try:
             account = getUtility(IAccountSet).getByOpenIDIdentifier(
                 openid_identifier)
@@ -3618,8 +3618,8 @@
             rationale=PersonCreationRationale.USERNAME_PLACEHOLDER,
             comment="when setting a username in SSO", account=account)
 
-    def _validateName(self, name):
-        if not valid_name(name):
+    def _validateUsername(self, name):
+        if not valid_username(name):
             raise InvalidName(
                 "%s is not a valid name for a person." % name)
         else:
@@ -3632,7 +3632,7 @@
     def _newPerson(self, name, displayname, hide_email_addresses,
                    rationale, comment=None, registrant=None, account=None):
         """Create and return a new Person with the given attributes."""
-        self._validateName(name)
+        self._validateUsername(name)
 
         if not displayname:
             displayname = name.capitalize()
@@ -4312,12 +4312,12 @@
             "%s is not a valid email address" % email_addr)
 
     user = re.match("^(\S+)@(?:\S+)$", email_addr).groups()[0]
-    user = user.replace(".", "-").replace("_", "-")
+    user = user.replace(".", "-").replace("_", "-").replace("+", "-")
 
     person_set = PersonSet()
 
     def _valid_nick(nick):
-        if not valid_name(nick):
+        if not valid_username(nick):
             return False
         elif is_registered(nick):
             return False

=== modified file 'lib/lp/registry/tests/test_nickname.py'
--- lib/lp/registry/tests/test_nickname.py	2015-10-26 14:54:43 +0000
+++ lib/lp/registry/tests/test_nickname.py	2019-05-06 17:01:07 +0000
@@ -36,14 +36,14 @@
         # valid nick that doesn't start with symbols.
         parts = ['---bar', 'foo.bar', 'foo-bar', 'foo+bar']
         nicks = [generate_nick("%s@xxxxxxxxxxx" % part) for part in parts]
-        self.assertEqual(['bar', 'foo-bar', 'foo-bar', 'foo+bar'], nicks)
+        self.assertEqual(['bar', 'foo-bar', 'foo-bar', 'foo-bar'], nicks)
 
     def test_enforces_minimum_length(self):
         # Nicks must be a minimum length. generate_nick enforces this by
         # adding random suffixes to the required length.
-        self.assertIs(None, getUtility(IPersonSet).getByName('i'))
-        nick = generate_nick('i@xxxxxxxxxxx')
-        self.assertEqual('i-b', nick)
+        self.assertIs(None, getUtility(IPersonSet).getByName('hi'))
+        nick = generate_nick('hi@xxxxxxxxxxx')
+        self.assertEqual('hi-5', nick)
 
     def test_can_create_noncolliding_nicknames(self):
         # Given the same email address, generate_nick doesn't recreate the

=== modified file 'lib/lp/registry/tests/test_notification.py'
--- lib/lp/registry/tests/test_notification.py	2018-02-02 10:06:24 +0000
+++ lib/lp/registry/tests/test_notification.py	2019-05-06 17:01:07 +0000
@@ -21,7 +21,7 @@
     layer = DatabaseFunctionalLayer
 
     def test_send_message(self):
-        self.factory.makePerson(email='me@xxxxxx', name='me')
+        self.factory.makePerson(email='you@xxxxxx', name='you')
         user = self.factory.makePerson(email='him@xxxxxx', name='him')
         subject = 'test subject'
         body = 'test body'
@@ -29,11 +29,11 @@
         recipients_set.add(user, 'test reason', 'test rationale')
         pop_notifications()
         send_direct_contact_email(
-            'me@xxxxxx', recipients_set, user, subject, body)
+            'you@xxxxxx', recipients_set, user, subject, body)
         notifications = pop_notifications()
         notification = notifications[0]
         self.assertEqual(1, len(notifications))
-        self.assertEqual('Me <me@xxxxxx>', notification['From'])
+        self.assertEqual('You <you@xxxxxx>', notification['From'])
         self.assertEqual('Him <him@xxxxxx>', notification['To'])
         self.assertEqual(subject, notification['Subject'])
         self.assertEqual(
@@ -46,7 +46,7 @@
                 '%s' % body,
                 '-- ',
                 'This message was sent from Launchpad by',
-                'Me (http://launchpad.dev/~me)',
+                'You (http://launchpad.dev/~you)',
                 'test reason.',
                 'For more information see',
                 'https://help.launchpad.net/YourAccount/ContactingPeople']),
@@ -54,30 +54,30 @@
 
     def test_quota_reached_error(self):
         # An error is raised if the user has reached the daily quota.
-        self.factory.makePerson(email='me@xxxxxx', name='me')
+        self.factory.makePerson(email='you@xxxxxx', name='you')
         user = self.factory.makePerson(email='him@xxxxxx', name='him')
         recipients_set = NotificationRecipientSet()
-        old_message = self.factory.makeSignedMessage(email_address='me@xxxxxx')
+        old_message = self.factory.makeSignedMessage(email_address='you@xxxxxx')
         authorization = IDirectEmailAuthorization(user)
         for action in range(authorization.message_quota):
             authorization.record(old_message)
         self.assertRaises(
             QuotaReachedError, send_direct_contact_email,
-            'me@xxxxxx', recipients_set, user, 'subject', 'body')
+            'you@xxxxxx', recipients_set, user, 'subject', 'body')
 
     def test_empty_recipient_set(self):
         # The recipient set can be empty. No messages are sent and the
         # action does not count toward the daily quota.
-        self.factory.makePerson(email='me@xxxxxx', name='me')
+        self.factory.makePerson(email='you@xxxxxx', name='you')
         user = self.factory.makePerson(email='him@xxxxxx', name='him')
         recipients_set = NotificationRecipientSet()
-        old_message = self.factory.makeSignedMessage(email_address='me@xxxxxx')
+        old_message = self.factory.makeSignedMessage(email_address='you@xxxxxx')
         authorization = IDirectEmailAuthorization(user)
         for action in range(authorization.message_quota - 1):
             authorization.record(old_message)
         pop_notifications()
         send_direct_contact_email(
-            'me@xxxxxx', recipients_set, user, 'subject', 'body')
+            'you@xxxxxx', recipients_set, user, 'subject', 'body')
         notifications = pop_notifications()
         self.assertEqual(0, len(notifications))
         self.assertTrue(authorization.is_allowed)

=== modified file 'lib/lp/services/database/tests/test_transaction_policy.py'
--- lib/lp/services/database/tests/test_transaction_policy.py	2013-06-20 05:50:00 +0000
+++ lib/lp/services/database/tests/test_transaction_policy.py	2019-05-06 17:01:07 +0000
@@ -37,7 +37,8 @@
 
         :return: A token that `hasDatabaseBeenWrittenTo` can look for.
         """
-        name = self.factory.getUniqueString()
+        # Pass a prefix to keep team name way under 32-char limit.
+        name = self.factory.getUniqueString('test-transaction')
         self.factory.makePerson(name=name)
         return name
 

=== modified file 'lib/lp/translations/stories/standalone/xx-person-activity.txt'
--- lib/lp/translations/stories/standalone/xx-person-activity.txt	2018-06-02 22:39:54 +0000
+++ lib/lp/translations/stories/standalone/xx-person-activity.txt	2019-05-06 17:01:07 +0000
@@ -51,13 +51,18 @@
 URL-escaped user names
 ----------------------
 
-Since the user's name is included in the URL, and user names can contain
-some slightly weird characters, it is escaped especially for this usage.
+Since the user's name is included in the URL, and legacy user names can
+contain some slightly weird characters, it is escaped especially for this
+usage.
 
-For instance, here's a user called a+b.
+For instance, here's a legacy user called "a+b".
 
     >>> login('carlos@xxxxxxxxxxxxx')
-    >>> ab = factory.makePerson(name='a+b')
+    >>> ab = factory.makePerson()
+    >>> from zope.security.proxy import removeSecurityProxy
+    >>> naked_person = removeSecurityProxy(ab)
+    >>> naked_person.name = 'a+b'
+    >>> naked_person.display_name = 'A+b'
     >>> sr_pofile = factory.makePOFile('sr')
     >>> message = factory.makeCurrentTranslationMessage(
     ...     pofile=sr_pofile, translator=ab)


Follow ups