stop using username as a fallback when display name isn't set (#10262)

2019-01-02 10:15:48 +00:00 · 2019-01-02 10:15:48 +00:00 · dd733fe76c
--- a/docs/topics/api/accounts.rst
+++ b/docs/topics/api/accounts.rst
@ -33,14 +33,14 @@ A developer is defined as a user who is listed as a developer or owner of one or
    :>json float average_addon_rating: The average rating for addons the developer has listed on the website.
    :>json string|null biography: More details about the user.
    :>json string created: The date when this user first logged in and created this account.
-    :>json boolean has_anonymous_display_name: The user has chosen neither a name nor a username.
+    :>json boolean has_anonymous_display_name: The user hasn't chosen a name.
    :>json boolean has_anonymous_username: The user hasn't chosen a username.
    :>json string|null homepage: The user's website.
    :>json int id: The numeric user id.
    :>json boolean is_addon_developer: The user has developed and listed add-ons on this website.
    :>json boolean is_artist: The user has developed and listed themes on this website.
    :>json string|null location: The location of the user.
-    :>json string name: The name chosen by the user, or the username if not set.
+    :>json string name: The name chosen by the user, or the "Firefox user {id}" if not set.
    :>json int num_addons_listed: The number of addons the developer has listed on the website.
    :>json string|null occupation: The occupation of the user.
    :>json string|null picture_type: the image type (only 'image/png' is supported) if a user photo has been uploaded, or null otherwise.
--- a/src/olympia/activity/tests/test_models.py
+++ b/src/olympia/activity/tests/test_models.py
@ -84,7 +84,8 @@ class TestActivityLog(TestCase):

    def setUp(self):
        super(TestActivityLog, self).setUp()
-        self.user = UserProfile.objects.create(username='yolo')
+        self.user = UserProfile.objects.create(
+            username='yolo', display_name='Yolo')
        self.request = Mock()
        self.request.user = self.user
        core.set_user(self.user)
@ -259,7 +260,7 @@ class TestActivityLog(TestCase):
                           au.get_role_display(), addon)
        log = ActivityLog.objects.get()

-        log_expected = ('yolo role changed to Owner for <a href="/en-US/'
+        log_expected = ('Yolo role changed to Owner for <a href="/en-US/'
                        'firefox/addon/a3615/">Delicious &lt;script src='
                        '&#34;x.js&#34;&gt;Bookmarks</a>.')
        assert log.to_string() == log_expected
--- a/src/olympia/discovery/tests/test_models.py
+++ b/src/olympia/discovery/tests/test_models.py
@ -12,7 +12,7 @@ class TestDiscoveryItem(TestCase):
        addon = addon_factory(slug=u'somé-slug', name=u'Sôme Name')
        user1 = user_factory(display_name=u'Bàr')
        addon.addonuser_set.create(user=user1, position=1)
-        user2 = user_factory(username=u'Fôo')
+        user2 = user_factory(username=u'Fôo', id=345)
        addon.addonuser_set.create(user=user2, position=2)
        user3 = user_factory(username=u'Nôpe')
        addon.addonuser_set.create(user=user3, listed=False)
@ -25,7 +25,7 @@ class TestDiscoveryItem(TestCase):
            u'Fancy Héading <span>with '
            u'<a href="http://testserver/en-US/firefox/addon/som%C3%A9-slug/'
            u'?{}">'
-            u'Sôme Name by Bàr, Fôo</a></span>').format(
+            u'Sôme Name by Bàr, Firefox user 345</a></span>').format(
                item.build_querystring())

    def test_heading_custom(self):
--- a/src/olympia/lib/akismet/tests.py
+++ b/src/olympia/lib/akismet/tests.py
@ -169,7 +169,7 @@ class TestAkismetReportsRating(BaseAkismetReportsModelTest, TestCase):
            'referrer': referrer,
            'permalink': addon.get_url_path(),
            'comment_type': 'user-review',
-            'comment_author': user.username,
+            'comment_author': user.name,
            'comment_author_email': user.email,
            'comment_author_url': user.homepage,
            'comment_content': rating.body,
@ -222,7 +222,7 @@ class TestAkismetReportsAddon(BaseAkismetReportsModelTest, TestCase):
            'user_agent': ua,
            'referrer': referrer,
            'comment_type': 'product-name',
-            'comment_author': user.username,
+            'comment_author': user.name,
            'comment_author_email': user.email,
            'comment_author_url': user.homepage,
            'comment_content': unicode(addon.name),
--- a/src/olympia/reviewers/tests/test_views.py
+++ b/src/olympia/reviewers/tests/test_views.py
@ -4298,7 +4298,7 @@ class TestReview(ReviewBase):
            doc('.user_ratings').text() ==
            u'%s on %s [10.5.6.7]\n'
            u'Rated 3 out of 5 stars\nLôrem ipsum dolor' % (
-                user.username, created_at
+                user.name, created_at
            )
        )

--- a/src/olympia/users/models.py
+++ b/src/olympia/users/models.py
@ -367,13 +367,8 @@ class UserProfile(OnChangeMixin, ModelBase, AbstractBaseUser):
    def name(self):
        if self.display_name:
            return force_text(self.display_name)
-        elif self.has_anonymous_username:
-            # L10n: {id} will be something like "13ad6a", just a random number
-            # to differentiate this user from other anonymous users.
-            return ugettext('Firefox user {id}').format(
-                id=self._anonymous_username_id())
        else:
-            return force_text(self.username)
+            return ugettext('Firefox user {id}').format(id=self.id)

    welcome_name = name

@ -381,11 +376,7 @@ class UserProfile(OnChangeMixin, ModelBase, AbstractBaseUser):
        return self.name

    def get_short_name(self):
-        return self.username
-
-    def _anonymous_username_id(self):
-        if self.has_anonymous_username:
-            return self.username.split('-')[1][:6]
+        return self.name

    def anonymize_username(self):
        """Set an anonymous username."""
@ -402,7 +393,7 @@ class UserProfile(OnChangeMixin, ModelBase, AbstractBaseUser):

    @property
    def has_anonymous_display_name(self):
-        return not self.display_name and self.has_anonymous_username
+        return not self.display_name

    @cached_property
    def ratings(self):
--- a/src/olympia/users/templatetags/jinja_helpers.py
+++ b/src/olympia/users/templatetags/jinja_helpers.py
@ -74,13 +74,14 @@ def _user_link(user, max_text_length=None):
    if isinstance(user, basestring):
        return user

-    username = user.name
    if max_text_length and len(user.name) > max_text_length:
-        username = user.name[:max_text_length].strip() + '...'
+        name = user.name[:max_text_length].strip() + '...'
+    else:
+        name = user.name

    return u'<a href="%s" title="%s">%s</a>' % (
        user.get_url_path(), jinja2.escape(user.name),
-        jinja2.escape(force_text(username)))
+        jinja2.escape(force_text(name)))


@library.filter
--- a/src/olympia/users/tests/test_helpers.py
+++ b/src/olympia/users/tests/test_helpers.py
@ -97,15 +97,16 @@ def test_users_list_truncate_display_name():

 def test_user_link_unicode():
    """make sure helper won't choke on unicode input"""
-    u = UserProfile(username=u'jmüller', display_name=u'Jürgen Müller', pk=1)
+    u = UserProfile.objects.create(
+        username=u'jmüller', display_name=u'Jürgen Müller')
    assert user_link(u) == (
        u'<a href="%s" title="%s">Jürgen Müller</a>' % (
            u.get_url_path(), u.name))

-    u = UserProfile(username='\xe5\xaf\x92\xe6\x98\x9f', pk=1)
+    u = UserProfile.objects.create(display_name=u'\xe5\xaf\x92\xe6\x98\x9f')
    assert user_link(u) == (
        u'<a href="%s" title="%s">%s</a>' % (u.get_url_path(), u.name,
-                                             u.username))
+                                             u.display_name))


 class TestAddonUsersList(TestPersonas, TestCase):
--- a/src/olympia/users/tests/test_models.py
+++ b/src/olympia/users/tests/test_models.py
@ -274,57 +274,60 @@ class TestUserProfile(TestCase):
        assert len(user.groups_list) == 2

    def test_welcome_name(self):
-        u1 = UserProfile(username='sc')
-        u2 = UserProfile(username='sc', display_name="Sarah Connor")
-        u3 = UserProfile()
-        assert u1.welcome_name == 'sc'
+        u1 = UserProfile.objects.create(username='sc')
+        u2 = UserProfile.objects.create(
+            username='sc2', display_name="Sarah Connor")
+        u3 = UserProfile.objects.create()
+        assert u1.welcome_name == 'Firefox user %s' % u1.id
        assert u2.welcome_name == 'Sarah Connor'
-        assert u3.welcome_name == ''
+        assert u3.welcome_name == 'Firefox user %s' % u3.id

    def test_welcome_name_anonymous(self):
-        user = UserProfile(
-            username='anonymous-bb4f3cbd422e504080e32f2d9bbfcee0')
-        assert user.welcome_name == 'Firefox user bb4f3c'
+        user = UserProfile.objects.create(
+            username='anonymous-bb4f3cbd422e504080e32f2d9bbfcee0', id=1234)
+        assert user.welcome_name == 'Firefox user 1234'

    def test_welcome_name_anonymous_with_display(self):
-        user = UserProfile(display_name='John Connor')
+        user = UserProfile.objects.create(display_name='John Connor')
        user.anonymize_username()
        assert user.welcome_name == 'John Connor'

    def test_has_anonymous_username_no_names(self):
-        user = UserProfile(display_name=None)
+        user = UserProfile.objects.create(display_name=None)
        user.anonymize_username()
        assert user.has_anonymous_username

    def test_has_anonymous_username_username_set(self):
-        user = UserProfile(username='bob', display_name=None)
+        user = UserProfile.objects.create(username='bob', display_name=None)
        assert not user.has_anonymous_username

    def test_has_anonymous_username_display_name_set(self):
-        user = UserProfile(display_name='Bob Bobbertson')
+        user = UserProfile.objects.create(display_name='Bob Bobbertson')
        user.anonymize_username()
        assert user.has_anonymous_username

    def test_has_anonymous_username_both_names_set(self):
-        user = UserProfile(username='bob', display_name='Bob Bobbertson')
+        user = UserProfile.objects.create(
+            username='bob', display_name='Bob Bobbertson')
        assert not user.has_anonymous_username

    def test_has_anonymous_display_name_no_names(self):
-        user = UserProfile(display_name=None)
+        user = UserProfile.objects.create(display_name=None)
        user.anonymize_username()
        assert user.has_anonymous_display_name

    def test_has_anonymous_display_name_username_set(self):
-        user = UserProfile(username='bob', display_name=None)
-        assert not user.has_anonymous_display_name
+        user = UserProfile.objects.create(username='bob', display_name=None)
+        assert user.has_anonymous_display_name

    def test_has_anonymous_display_name_display_name_set(self):
-        user = UserProfile(display_name='Bob Bobbertson')
+        user = UserProfile.objects.create(display_name='Bob Bobbertson')
        user.anonymize_username()
        assert not user.has_anonymous_display_name

    def test_has_anonymous_display_name_both_names_set(self):
-        user = UserProfile(username='bob', display_name='Bob Bobbertson')
+        user = UserProfile.objects.create(
+            username='bob', display_name='Bob Bobbertson')
        assert not user.has_anonymous_display_name

    def test_superuser(self):
@ -362,15 +365,18 @@ class TestUserProfile(TestCase):
        """
        Test for a preview URL if image is set, or default image otherwise.
        """
-        u = UserProfile(id=1234, picture_type='image/png',
-                        modified=date.today())
+        u = UserProfile.objects.create(
+            id=1234, picture_type='image/png', modified=date.today(),
+            username='a')
        u.picture_url.index('/userpics/0/1/1234.png?modified=')

-        u = UserProfile(id=1234567890, picture_type='image/png',
-                        modified=date.today())
+        u = UserProfile.objects.create(
+            id=1234567890, picture_type='image/png', modified=date.today(),
+            username='b')
        u.picture_url.index('/userpics/1234/1234567/1234567890.png?modified=')

-        u = UserProfile(id=1234, picture_type=None)
+        u = UserProfile.objects.create(
+            id=123456, picture_type=None, username='c')
        assert u.picture_url.endswith('/anon_user.png')

    def test_review_replies(self):
@ -441,10 +447,10 @@ class TestUserProfile(TestCase):
        assert c.slug == 'favorites'

    def test_get_url_path(self):
-        assert UserProfile(id=1).get_url_path() == (
-            '/en-US/firefox/user/1/')
-        assert UserProfile(username='yolo', id=1).get_url_path() == (
+        assert UserProfile.objects.create(id=1).get_url_path() == (
            '/en-US/firefox/user/1/')
+        assert UserProfile.objects.create(
+            username='yolo', id=2).get_url_path() == ('/en-US/firefox/user/2/')

    def test_mobile_addons(self):
        user = UserProfile.objects.get(id='4043307')
@ -498,16 +504,20 @@ class TestUserProfile(TestCase):
        before_change = (
            datetime(2018, 1, 1) - timedelta(days=42))

-        assert not UserProfile().has_read_developer_agreement()
-        assert not UserProfile(
-            read_dev_agreement=None).has_read_developer_agreement()
-        assert not UserProfile(
-            read_dev_agreement=before_change).has_read_developer_agreement()
+        assert not UserProfile.objects.create(
+            username='a').has_read_developer_agreement()
+        assert not UserProfile.objects.create(
+            username='b', read_dev_agreement=None
+        ).has_read_developer_agreement()
+        assert not UserProfile.objects.create(
+            username='c', read_dev_agreement=before_change
+        ).has_read_developer_agreement()

        # User has read the agreement after it was modified for
        # post-review: it should return True.
-        assert UserProfile(
-            read_dev_agreement=after_change).has_read_developer_agreement()
+        assert UserProfile.objects.create(
+            username='d', read_dev_agreement=after_change
+        ).has_read_developer_agreement()

    def test_is_public(self):
        user = UserProfile.objects.get(id=4043307)