Skip to content

allow username change, avoid new collisions on case insensitive match #2061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions users/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class UserProfileForm(ModelForm):
class Meta:
model = User
fields = [
'username',
'first_name',
'last_name',
'email',
Expand All @@ -22,6 +23,17 @@ class Meta:
'email_privacy': forms.RadioSelect,
}

def clean_username(self):
try:
user = User.objects.get_by_natural_key(self.cleaned_data.get('username'))
except User.MultipleObjectsReturned:
raise forms.ValidationError('A user with that username already exists.')
except User.DoesNotExist:
return self.cleaned_data.get('username')
if user == self.instance:
return self.cleaned_data.get('username')
raise forms.ValidationError('A user with that username already exists.')

def clean_email(self):
email = self.cleaned_data.get('email')
user = User.objects.filter(email=email).exclude(pk=self.instance.pk)
Expand Down
10 changes: 8 additions & 2 deletions users/models.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import datetime

from django.conf import settings
from django.contrib.auth.models import AbstractUser
from django.contrib.auth.models import AbstractUser, UserManager
from django.urls import reverse
from django.db import models
from django.utils import timezone
Expand All @@ -15,6 +15,12 @@
DEFAULT_MARKUP_TYPE = getattr(settings, 'DEFAULT_MARKUP_TYPE', 'markdown')


class CustomUserManager(UserManager):
def get_by_natural_key(self, username):
case_insensitive_username_field = '{}__iexact'.format(self.model.USERNAME_FIELD)
return self.get(**{case_insensitive_username_field: username})


class User(AbstractUser):
bio = MarkupField(blank=True, default_markup_type=DEFAULT_MARKUP_TYPE, escape_html=True)

Expand All @@ -38,7 +44,7 @@ class User(AbstractUser):

public_profile = models.BooleanField('Make my profile public', default=True)

objects = UserManager()
objects = CustomUserManager()

def get_absolute_url(self):
return reverse('users:user_detail', kwargs={'slug': self.username})
Expand Down
17 changes: 17 additions & 0 deletions users/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ def test_unique_email(self):
User.objects.create_user('test42', '[email protected]', 'testpass')

form = UserProfileForm({
'username': 'stanne',
'email': '[email protected]',
'search_visibility': 0,
'email_privacy': 0,
Expand All @@ -134,3 +135,19 @@ def test_unique_email(self):
form.errors,
{'email': ['Please use a unique email address.']}
)

def test_case_insensitive_unique_username(self):
User.objects.create_user('stanne', '[email protected]', 'testpass')
User.objects.create_user('test42', '[email protected]', 'testpass')

form = UserProfileForm({
'username': 'Test42',
'email': '[email protected]',
'search_visibility': 0,
'email_privacy': 0,
}, instance=User.objects.get(username='stanne'))
self.assertFalse(form.is_valid())
self.assertEqual(
form.errors,
{'username': ['A user with that username already exists.']}
)
7 changes: 4 additions & 3 deletions users/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def test_membership_update(self):
self.assertEqual(response.status_code, 302) # Requires login now

self.assertTrue(self.user2.has_membership)
self.client.login(username=self.user2, password='password')
self.client.login(username=self.user2.username, password='password')
response = self.client.get(url)
self.assertEqual(response.status_code, 200)

Expand All @@ -105,7 +105,7 @@ def test_membership_update(self):
def test_membership_update_404(self):
url = reverse('users:user_membership_edit')
self.assertFalse(self.user.has_membership)
self.client.login(username=self.user, password='password')
self.client.login(username=self.user.username, password='password')
response = self.client.get(url)
self.assertEqual(response.status_code, 404)

Expand All @@ -114,7 +114,7 @@ def test_user_has_already_have_membership(self):
# has membership.
url = reverse('users:user_membership_create')
self.assertTrue(self.user2.has_membership)
self.client.login(username=self.user2, password='password')
self.client.login(username=self.user2.username, password='password')
response = self.client.get(url)
self.assertRedirects(response, reverse('users:user_membership_edit'))

Expand All @@ -139,6 +139,7 @@ def test_user_update_redirect(self):

# should return 200 if the user does want to see their user profile
post_data = {
'username': 'username',
'search_visibility': 0,
'email_privacy': 1,
'public_profile': False,
Expand Down