From 96d87a9391b4cee85ac72e30aab4feac1d81148e Mon Sep 17 00:00:00 2001
From: Andrew Williamson <awilliamson@mozilla.com>
Date: Wed, 9 Jan 2019 19:18:03 +0800
Subject: [PATCH] drop username editing (#10329)

---
 docs/topics/api/accounts.rst                |  3 +-
 src/olympia/accounts/serializers.py         | 32 ++--------------
 src/olympia/accounts/tests/test_views.py    | 22 +++--------
 src/olympia/users/forms.py                  | 41 ++-------------------
 src/olympia/users/templates/users/edit.html |  5 ---
 src/olympia/users/tests/test_forms.py       | 30 +++------------
 6 files changed, 19 insertions(+), 114 deletions(-)

diff --git a/docs/topics/api/accounts.rst b/docs/topics/api/accounts.rst
index 99e6ca8f4d..05a634e018 100644
--- a/docs/topics/api/accounts.rst
+++ b/docs/topics/api/accounts.rst
@@ -45,7 +45,7 @@ A developer is defined as a user who is listed as a developer or owner of one or
     :>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.
     :>json string|null picture_url: URL to a photo of the user, or null if no photo has been uploaded.
-    :>json string username: username chosen by the user, used in the account url. If not set will be a randomly generated string.
+    :>json string username: deprecated property still included for backwards compatability.  Previous chosen by the user, used in the account url. If not previously set will be a randomly generated string.
 
 
 
@@ -137,7 +137,6 @@ but not listed below are not editable and will be ignored in the patch request.
     :<json string|null homepage: The user's website.
     :<json string|null location: The location of the user.
     :<json string|null occupation: The occupation of the user.
-    :<json string|null username: username to be used in the account url.  The username can only contain letters, numbers, underscores or hyphens. All-number usernames are prohibited as they conflict with user-ids.
 
 
 -------------------
diff --git a/src/olympia/accounts/serializers.py b/src/olympia/accounts/serializers.py
index 10acce94dd..f147fa12c3 100644
--- a/src/olympia/accounts/serializers.py
+++ b/src/olympia/accounts/serializers.py
@@ -13,8 +13,8 @@ from olympia.access import acl
 from olympia.access.models import Group
 from olympia.amo.templatetags.jinja_helpers import absolutify
 from olympia.amo.utils import (
-    ImageCheck, clean_nl, has_links, slug_validator, subscribe_newsletter,
-    unsubscribe_newsletter, urlparams)
+    clean_nl, has_links, ImageCheck,
+    subscribe_newsletter, unsubscribe_newsletter, urlparams)
 from olympia.api.utils import is_gate_active
 from olympia.api.validators import OneOrMorePrintableCharacterValidator
 from olympia.users import notifications
@@ -83,11 +83,11 @@ class UserProfileSerializer(PublicUserProfileSerializer):
         fields = PublicUserProfileSerializer.Meta.fields + (
             'display_name', 'email', 'deleted', 'last_login', 'picture_upload',
             'last_login_ip', 'read_dev_agreement', 'permissions',
-            'fxa_edit_email_url',
+            'fxa_edit_email_url', 'username',
         )
         writeable_fields = (
             'biography', 'display_name', 'homepage', 'location', 'occupation',
-            'picture_upload', 'username',
+            'picture_upload',
         )
         read_only_fields = tuple(set(fields) - set(writeable_fields))
 
@@ -119,30 +119,6 @@ class UserProfileSerializer(PublicUserProfileSerializer):
             )
         return value
 
-    def validate_username(self, value):
-        # All-digits usernames are disallowed since they can be confused for
-        # user IDs in URLs.
-        if value.isdigit():
-            raise serializers.ValidationError(
-                ugettext(u'Usernames cannot contain only digits.'))
-
-        slug_validator(
-            value, lower=False,
-            message=ugettext(u'Enter a valid username consisting of letters, '
-                             u'numbers, underscores or hyphens.'))
-        if DeniedName.blocked(value):
-            raise serializers.ValidationError(
-                ugettext(u'This username cannot be used.'))
-
-        # Bug 858452. Remove this check when collation of the username
-        # column is changed to case insensitive.
-        if (UserProfile.objects.exclude(id=self.instance.id)
-                       .filter(username__iexact=value).exists()):
-            raise serializers.ValidationError(
-                ugettext(u'This username is already in use.'))
-
-        return value
-
     def validate_picture_upload(self, value):
         image_check = ImageCheck(value)
 
diff --git a/src/olympia/accounts/tests/test_views.py b/src/olympia/accounts/tests/test_views.py
index 4db33e79d1..05bcd688f9 100644
--- a/src/olympia/accounts/tests/test_views.py
+++ b/src/olympia/accounts/tests/test_views.py
@@ -1037,7 +1037,6 @@ class TestAccountViewSetUpdate(TestCase):
         'homepage': 'http://bob-loblaw-law-web.blog',
         'location': 'law office',
         'occupation': 'lawyer',
-        'username': 'bob',
     }
 
     def setUp(self):
@@ -1087,15 +1086,19 @@ class TestAccountViewSetUpdate(TestCase):
 
     def test_read_only_fields(self):
         self.client.login_api(self.user)
+        existing_username = self.user.username
         original = self.client.get(self.url).content
         # Try to patch a field that can't be patched.
-        response = self.patch(data={'last_login_ip': '666.666.666.666'})
+        response = self.patch(data={
+            'last_login_ip': '666.666.666.666', 'username': 'new_username'})
         assert response.status_code == 200
         assert response.content == original
         self.user = self.user.reload()
         # Confirm field hasn't been updated.
         assert json.loads(response.content)['last_login_ip'] == ''
         assert self.user.last_login_ip == ''
+        assert json.loads(response.content)['username'] == existing_username
+        assert self.user.username == existing_username
 
     def test_biography_no_links(self):
         self.client.login_api(self.user)
@@ -1105,21 +1108,6 @@ class TestAccountViewSetUpdate(TestCase):
         assert json.loads(response.content) == {
             'biography': ['No links are allowed.']}
 
-    def test_username_valid(self):
-        self.client.login_api(self.user)
-        response = self.patch(
-            data={'username': '123456'})
-        assert response.status_code == 400
-        assert json.loads(response.content) == {
-            'username': ['Usernames cannot contain only digits.']}
-
-        response = self.patch(
-            data={'username': u'£^@'})
-        assert response.status_code == 400
-        assert json.loads(response.content) == {
-            'username': [u'Enter a valid username consisting of letters, '
-                         u'numbers, underscores or hyphens.']}
-
     def test_display_name_validation(self):
         self.client.login_api(self.user)
         response = self.patch(
diff --git a/src/olympia/users/forms.py b/src/olympia/users/forms.py
index d610661bb4..918345e27d 100644
--- a/src/olympia/users/forms.py
+++ b/src/olympia/users/forms.py
@@ -15,8 +15,9 @@ from olympia.accounts.views import fxa_error_message
 from olympia.activity.models import ActivityLog
 from olympia.amo.fields import HttpHttpsOnlyURLField
 from olympia.amo.utils import (
-    ImageCheck, clean_nl, fetch_subscribed_newsletters, has_links,
-    slug_validator, subscribe_newsletter, unsubscribe_newsletter)
+    clean_nl, has_links, ImageCheck,
+    fetch_subscribed_newsletters, subscribe_newsletter,
+    unsubscribe_newsletter)
 from olympia.users import notifications
 
 from . import tasks
@@ -63,7 +64,6 @@ LOGIN_HELP_URL = (
 
 
 class UserEditForm(forms.ModelForm):
-    username = forms.CharField(max_length=50, required=False)
     display_name = forms.CharField(label=_(u'Display Name'), max_length=50,
                                    required=False)
     location = forms.CharField(label=_(u'Location'), max_length=100,
@@ -148,44 +148,11 @@ class UserEditForm(forms.ModelForm):
     class Meta:
         model = UserProfile
         fields = (
-            'username', 'email', 'display_name', 'location', 'occupation',
+            'email', 'display_name', 'location', 'occupation',
             'homepage', 'photo', 'biography', 'display_collections',
             'notifications',
         )
 
-    def clean_username(self):
-        name = self.cleaned_data['username']
-
-        if not name:
-            if self.instance.has_anonymous_username:
-                name = self.instance.username
-            else:
-                name = self.instance.anonymize_username()
-
-        # All-digits usernames are disallowed since they can be
-        # confused for user IDs in URLs. (See bug 862121.)
-        if name.isdigit():
-            raise forms.ValidationError(
-                ugettext('Usernames cannot contain only digits.'))
-
-        slug_validator(
-            name, lower=False,
-            message=ugettext(
-                'Enter a valid username consisting of letters, numbers, '
-                'underscores or hyphens.'))
-        if DeniedName.blocked(name):
-            raise forms.ValidationError(
-                ugettext('This username cannot be used.'))
-
-        # FIXME: Bug 858452. Remove this check when collation of the username
-        # column is changed to case insensitive.
-        if (UserProfile.objects.exclude(id=self.instance.id)
-                       .filter(username__iexact=name).exists()):
-            raise forms.ValidationError(
-                ugettext('This username is already in use.'))
-
-        return name
-
     def clean_display_name(self):
         name = self.cleaned_data['display_name']
         if DeniedName.blocked(name):
diff --git a/src/olympia/users/templates/users/edit.html b/src/olympia/users/templates/users/edit.html
index a06efd660a..22440a9ed2 100644
--- a/src/olympia/users/templates/users/edit.html
+++ b/src/olympia/users/templates/users/edit.html
@@ -34,11 +34,6 @@ data-default-locale="{{ request.LANG|lower }}"
             {%- endtrans %}
           </p>
           <ul class="formfields">
-            <li{% if form.username.errors %} class="error"{% endif %}>
-              <label for="id_username">{{ _('Username') }}</label>
-              {{ form.username }}
-              {{ form.username.errors }}
-            </li>
             <li>
               <label for="id_email">
                 {{ _('Email Address') }}
diff --git a/src/olympia/users/tests/test_forms.py b/src/olympia/users/tests/test_forms.py
index 54057f202e..aaade82c43 100644
--- a/src/olympia/users/tests/test_forms.py
+++ b/src/olympia/users/tests/test_forms.py
@@ -66,26 +66,9 @@ class TestUserEditForm(UserFormBase):
         self.client.login(email='jbalogh@mozilla.com')
         self.url = reverse('users.edit')
 
-    def test_no_username_or_display_name(self):
-        assert not self.user.has_anonymous_username
-        data = {'username': '',
-                'email': 'jbalogh@mozilla.com'}
-        response = self.client.post(self.url, data)
-        self.assertNoFormErrors(response)
-        assert self.user.reload().has_anonymous_username
-
-    def test_change_username(self):
-        assert self.user.username != 'new-username'
-        data = {'username': 'new-username',
-                'email': 'jbalogh@mozilla.com'}
-        response = self.client.post(self.url, data)
-        self.assertNoFormErrors(response)
-        assert self.user.reload().username == 'new-username'
-
     def test_cannot_change_display_name_to_denied_ones(self):
         assert self.user.display_name != 'Mozilla'
-        data = {'username': 'new-username',
-                'display_name': 'IE6Fan',
+        data = {'display_name': 'IE6Fan',
                 'email': 'jbalogh@mozilla.com'}
         response = self.client.post(self.url, data)
         msg = "This display name cannot be used."
@@ -93,8 +76,7 @@ class TestUserEditForm(UserFormBase):
         assert self.user.reload().display_name != 'IE6Fan'
 
     def test_display_name_length(self):
-        data = {'username': 'new-username',
-                'display_name': 'a' * 51,
+        data = {'display_name': 'a' * 51,
                 'email': 'jbalogh@mozilla.com'}
         response = self.client.post(self.url, data)
         msg = 'Ensure this value has at most 50 characters (it has 51).'
@@ -117,8 +99,7 @@ class TestUserEditForm(UserFormBase):
         the auto-generated value does not change."""
         username = self.user.anonymize_username()
         self.user.save()
-        data = {'username': '',
-                'email': 'jbalogh@mozilla.com'}
+        data = {'email': 'jbalogh@mozilla.com'}
         response = self.client.post(self.url, data)
         self.assertNoFormErrors(response)
         assert self.user.reload().username == username
@@ -139,9 +120,8 @@ class TestUserEditForm(UserFormBase):
         self.assertContains(r, 'Profile Updated')
 
     def test_long_data(self):
-        data = {'username': 'jbalogh',
-                'email': 'jbalogh@mozilla.com'}
-        for field, length in (('username', 50), ('display_name', 50),
+        data = {'email': 'jbalogh@mozilla.com'}
+        for field, length in (('display_name', 50),
                               ('location', 100), ('occupation', 100)):
             data[field] = 'x' * (length + 1)
             r = self.client.post(self.url, data, follow=True)