diff --git a/kitsune/users/api.py b/kitsune/users/api.py index ae46e2c4c5f..b647bf8d4ba 100644 --- a/kitsune/users/api.py +++ b/kitsune/users/api.py @@ -145,8 +145,8 @@ class ProfileSerializer(serializers.ModelSerializer): display_name = serializers.WritableField(source='name', required=False) date_joined = DateTimeUTCField(source='user.date_joined', read_only=True) avatar = serializers.SerializerMethodField('get_avatar_url') - email = (PermissionMod(serializers.WritableField, permissions=[OnlySelf]) - (source='user.email', required=False)) + email = (PermissionMod(serializers.EmailField, permissions=[OnlySelf]) + (source='user.email', required=True)) settings = (PermissionMod(UserSettingSerializer, permissions=[OnlySelf]) (many=True, read_only=True)) helpfulness = serializers.Field(source='answer_helpfulness') @@ -222,22 +222,14 @@ def restore_object(self, attrs, instance=None): instance = (super(ProfileSerializer, self) .restore_object(attrs, instance)) if instance.user_id is None: - # The Profile doesn't have a user, so create one. If an email is - # specified, the user will be inactive until the email is - # confirmed. Otherwise the user can be created immediately. - if 'user.email' in attrs: - # This is a bit of cheat. The user shouldn't be saved yet, but - # ``create_inactive_user`` saves it, so their isn't much of a choice. - u = RegistrationProfile.objects.create_inactive_user( - attrs['user.username'], - attrs['user.password'], - attrs['user.email']) - instance.user_id = u.id - instance.save() - else: - u = User(username=attrs['user.username']) - u.set_password(attrs['user.password']) - instance._nested_forward_relations['user'] = u + # This is a bit of cheat. The user shouldn't be saved yet, but + # ``create_inactive_user`` saves it, so their isn't much of a choice. + u = RegistrationProfile.objects.create_inactive_user( + attrs['user.username'], + attrs['user.password'], + attrs['user.email']) + instance.user_id = u.id + instance.save() return instance def validate_username(self, attrs, source): diff --git a/kitsune/users/tests/test_api.py b/kitsune/users/tests/test_api.py index 6a67db9143c..cd31f94cbd6 100644 --- a/kitsune/users/tests/test_api.py +++ b/kitsune/users/tests/test_api.py @@ -98,16 +98,9 @@ def test_no_duplicate_emails(self): }) assert not serializer.is_valid() - def test_users_without_emails_are_active(self): - del self.data['email'] - serializer = api.ProfileSerializer(data=self.data) - serializer.is_valid() - assert serializer.is_valid() - serializer.save() - eq_(serializer.object.user.is_active, True) - def test_users_with_emails_are_inactive(self): serializer = api.ProfileSerializer(data=self.data) + serializer.is_valid() assert serializer.is_valid() serializer.save() eq_(serializer.object.user.is_active, False) @@ -347,23 +340,7 @@ def test_avatar_size(self): res = self.client.get(url, {'avatar_size': 128}) assert '?s=128' in res.data['avatar'] - def test_create_user_no_email(self): - # There is at least one user in existence due to migrations - number_users = User.objects.count() - - url = reverse('user-list') - res = self.client.post(url, { - 'username': 'kris', - 'password': 'testpass' - }) - - eq_(res.status_code, 201) - eq_(User.objects.count(), number_users + 1) - u = User.objects.order_by('-id')[0] - eq_(u.username, 'kris') - eq_(u.is_active, True) - - def test_create_user_with_email(self): + def test_create_use(self): # There is at least one user in existence due to migrations number_users = User.objects.count() @@ -381,3 +358,44 @@ def test_create_user_with_email(self): eq_(u.username, username) eq_(u.email, 'kris@example.com') eq_(u.is_active, False) + + def test_invalid_email(self): + username = 'sarah-{}'.format(random()) + url = reverse('user-list') + res = self.client.post(url, { + 'username': username, + 'password': 'testpass', + 'email': 'sarah', # invalid + }) + eq_(res.status_code, 400) + eq_(res.data, {'email': [u'Enter a valid email address.']}) + + def test_invalid_username(self): + url = reverse('user-list') + res = self.client.post(url, { + 'username': '&', # invalid + 'password': 'testpass', + 'email': 'lucy@example.com', + }) + eq_(res.status_code, 400) + eq_(res.data, {'username': [u'Usernames may only be letters, numbers, "." and "-".']}) + + def test_too_short_username(self): + url = reverse('user-list') + res = self.client.post(url, { + 'username': 'a', # too short + 'password': 'testpass', + 'email': 'lucy@example.com', + }) + eq_(res.status_code, 400) + eq_(res.data, {'username': [u'Usernames may only be letters, numbers, "." and "-".']}) + + def test_too_long_username(self): + url = reverse('user-list') + res = self.client.post(url, { + 'username': 'a' * 100, # too long + 'password': 'testpass', + 'email': 'lucy@example.com', + }) + eq_(res.status_code, 400) + eq_(res.data, {'username': [u'Usernames may only be letters, numbers, "." and "-".']})