Cleanup: test refactoring and test coverage

.coveragerc

@@ -5,3 +5,4 @@ omit =
     cadasta/*/__init__.py
     cadasta/config/*
     cadasta/manage.py
+    cadasta/core/management/commands/*

cadasta/accounts/manager.py

@@ -16,7 +16,7 @@ def get_from_username_or_email(self, identifier=None):
                 "User with username or email {} does not exist"
             ).format(identifier)
             raise self.model.DoesNotExist(error)
-        elif users_count > 1:
+        else:
             error = _(
                 "More than one user found for username or email {}"
             ).format(identifier)

cadasta/accounts/serializers.py

@@ -52,9 +52,9 @@ class Meta:
             'last_name',
             'email',
             'email_verified',
+            'last_login',
         )
         extra_kwargs = {
-            'password': {'write_only': True},
             'email': {'required': True, 'unique': True},
             'email_verified': {'read_only': True}
         }

cadasta/accounts/tests/factories.py

@@ -1,9 +1,10 @@
 import factory
 
+from core.tests.factories import ExtendedFactory
 from ..models import User
 
 
-class UserFactory(factory.django.DjangoModelFactory):
+class UserFactory(ExtendedFactory):
     class Meta:
         model = User
 

cadasta/accounts/tests/test_manager.py

@@ -9,7 +9,9 @@
 class UserManagerTest(TestCase):
     def test_get_from_usernamel(self):
         user = UserFactory.create()
-        found = User.objects.get_from_username_or_email(identifier=user.username)
+        found = User.objects.get_from_username_or_email(
+            identifier=user.username
+        )
 
         assert found == user
 
@@ -28,4 +30,6 @@ def test_mulitple_users_found(self):
         UserFactory.create(email='[email protected]')
 
         with raises(User.MultipleObjectsReturned):
-            User.objects.get_from_username_or_email(identifier='[email protected]')
+            User.objects.get_from_username_or_email(
+                identifier='[email protected]'
+            )

cadasta/accounts/tests/test_serializers.py

@@ -2,40 +2,45 @@
 from datetime import datetime
 from django.test import TestCase
 from django.utils.translation import gettext as _
+from rest_framework.test import APIRequestFactory, force_authenticate
+from rest_framework.request import Request
 
-from ..serializers import RegistrationSerializer, AccountLoginSerializer
+from ..serializers import (
+    RegistrationSerializer, UserSerializer, AccountLoginSerializer
+)
 from ..models import User
 from ..exceptions import EmailNotVerifiedError
 
 from .factories import UserFactory
 
 
+BASIC_TEST_DATA = {
+    'username': 'imagine71',
+    'email': '[email protected]',
+    'password': 'iloveyoko79',
+    'first_name': 'John',
+    'last_name': 'Lennon',
+}
+
+
 class RegistrationSerializerTest(TestCase):
     def test_field_serialization(self):
         user = UserFactory.build()
         serializer = RegistrationSerializer(user)
-        self.assertIn('email_verified', serializer.data)
-        self.assertNotIn('password', serializer.data)
+        assert 'email_verified' in serializer.data
+        assert 'password' not in serializer.data
 
     def test_create_with_valid_data(self):
-        data = {
-            'username': 'imagine71',
-            'email': '[email protected]',
-            'password': 'iloveyoko79',
-            'first_name': 'John',
-            'last_name': 'Lennon',
-        }
-
-        serializer = RegistrationSerializer(data=data)
-        self.assertTrue(serializer.is_valid())
+        serializer = RegistrationSerializer(data=BASIC_TEST_DATA)
+        assert serializer.is_valid()
 
         serializer.save()
-        self.assertEqual(User.objects.count(), 1)
+        assert User.objects.count() == 1
 
         user_obj = User.objects.first()
-        self.assertTrue(user_obj.check_password(data['password']))
-        self.assertTrue(user_obj.is_active)
-        self.assertFalse(user_obj.email_verified)
+        assert user_obj.check_password(BASIC_TEST_DATA['password'])
+        assert user_obj.is_active
+        assert not user_obj.email_verified
 
     def test_create_without_email(self):
         """Serialiser should be invalid when no email address is provided."""
@@ -49,7 +54,7 @@ def test_create_without_email(self):
         }
 
         serializer = RegistrationSerializer(data=data)
-        self.assertFalse(serializer.is_valid())
+        assert not serializer.is_valid()
 
     def test_create_with_existing_email(self):
         """Serialiser should be invalid when another user with the same email
@@ -69,11 +74,62 @@ def test_create_with_existing_email(self):
         }
 
         serializer = RegistrationSerializer(data=data)
-        self.assertFalse(serializer.is_valid())
-        self.assertIn(
-            _("Another user is already registered with this email address"),
-            serializer._errors['email'],
+        assert not serializer.is_valid()
+        assert (_("Another user is already registered with this email address")
+                in serializer._errors['email'])
+
+
+class UserSerializerTest(TestCase):
+    def test_field_serialization(self):
+        user = UserFactory.build()
+        serializer = UserSerializer(user)
+        assert 'email_verified' in serializer.data
+        assert 'password' not in serializer.data
+
+    def test_create_with_valid_data(self):
+        data = {
+            'username': 'imagine71',
+            'email': '[email protected]',
+            'password': 'iloveyoko79',
+            'first_name': 'John',
+            'last_name': 'Lennon',
+            'last_login': '2016-01-01 23:00:00'
+        }
+        serializer = UserSerializer(data=data)
+        assert serializer.is_valid()
+
+        serializer.save()
+        assert User.objects.count() == 1
+
+        user_obj = User.objects.first()
+        assert user_obj.is_active
+        assert not user_obj.email_verified
+
+    def test_update_username_fails(self):
+        serializer = UserSerializer(data=BASIC_TEST_DATA)
+        assert serializer.is_valid()
+        serializer.save()
+        user = User.objects.first()
+        other_user = UserFactory.create()
+        update_data = {'username': 'bad-update'}
+        request = APIRequestFactory().patch('/user/imagine71', update_data)
+        force_authenticate(request, user=other_user)
+        serializer2 = UserSerializer(
+            user, update_data, context={'request': Request(request)}
         )
+        assert not serializer2.is_valid()
+        assert serializer2.errors['username'] == ['Cannot update username']
+
+    def test_update_last_login_fails(self):
+        serializer = UserSerializer(data=BASIC_TEST_DATA)
+        assert serializer.is_valid()
+        user = serializer.save()
+        update_data1 = {'username': 'imagine71',
+                        'email': '[email protected]',
+                        'last_login': '2016-01-01 23:00:00'}
+        serializer2 = UserSerializer(user, data=update_data1)
+        assert not serializer2.is_valid()
+        assert serializer2.errors['last_login'] == ['Cannot update last_login']
 
 
 class AccountLoginSerializerTest(TestCase):

cadasta/accounts/tests/test_urls_api.py

@@ -10,28 +10,28 @@ def test_account_user(self):
         assert reverse(version_ns('accounts:user')) == version_url('/account/')
 
         resolved = resolve(version_url('/account/'))
-        self.assertEqual(resolved.func.__name__, api.AccountUser.__name__)
+        assert resolved.func.__name__ == api.AccountUser.__name__
 
     def test_account_register(self):
         assert (reverse(version_ns('accounts:register')) ==
                 version_url('/account/register/'))
 
         resolved = resolve(version_url('/account/register/'))
-        self.assertEqual(resolved.func.__name__, api.AccountRegister.__name__)
+        assert resolved.func.__name__ == api.AccountRegister.__name__
 
     def test_account_login(self):
         assert (reverse(version_ns('accounts:login')) ==
                 version_url('/account/login/'))
 
         resolved = resolve(version_url('/account/login/'))
-        self.assertEqual(resolved.func.__name__, api.AccountLogin.__name__)
+        assert resolved.func.__name__ == api.AccountLogin.__name__
 
     def test_account_activate(self):
         assert (reverse(version_ns('accounts:activate')) ==
                 version_url('/account/activate/'))
 
         resolved = resolve(version_url('/account/activate/'))
-        self.assertEqual(resolved.func.__name__, api.AccountVerify.__name__)
+        assert resolved.func.__name__ == api.AccountVerify.__name__
 
     def test_password_reset(self):
         self.assertEqual(
@@ -40,4 +40,4 @@ def test_password_reset(self):
         )
 
         resolved = resolve(version_url('/account/password/reset/'))
-        self.assertEqual(resolved.func.__name__, api.PasswordReset.__name__)
+        assert resolved.func.__name__ == api.PasswordReset.__name__