diff --git a/codeforlife/models/__init__.py b/codeforlife/models/__init__.py index c704f3cf..430adb51 100644 --- a/codeforlife/models/__init__.py +++ b/codeforlife/models/__init__.py @@ -2,7 +2,7 @@ © Ocado Group Created on 04/12/2023 at 14:36:56(+00:00). -Base models. +Base models. Tests at: codeforlife.user.tests.models.test_abstract """ import typing as t diff --git a/codeforlife/tests/__init__.py b/codeforlife/tests/__init__.py index 80ebc8c4..daaa7e0d 100644 --- a/codeforlife/tests/__init__.py +++ b/codeforlife/tests/__init__.py @@ -1,2 +1,10 @@ -from .api import APITestCase, APIClient +""" +© Ocado Group +Created on 08/12/2023 at 18:05:20(+00:00). + +All test helpers. +""" + +from .api import APIClient, APITestCase from .cron import CronTestCase, CronTestClient +from .model import ModelTestCase diff --git a/codeforlife/tests/model.py b/codeforlife/tests/model.py new file mode 100644 index 00000000..7cf831a5 --- /dev/null +++ b/codeforlife/tests/model.py @@ -0,0 +1,40 @@ +""" +© Ocado Group +Created on 08/12/2023 at 18:05:47(+00:00). + +Test helpers for Django models. +""" + +import typing as t + +from django.db.models import Model +from django.db.utils import IntegrityError +from django.test import TestCase + +AnyModel = t.TypeVar("AnyModel", bound=Model) + + +class ModelTestCase(TestCase, t.Generic[AnyModel]): + """Base for all model test cases.""" + + @classmethod + def get_model_class(cls) -> t.Type[AnyModel]: + """Get the model's class. + + Returns: + The model's class. + """ + + # pylint: disable-next=no-member + return t.get_args(cls.__orig_bases__[0])[ # type: ignore[attr-defined] + 0 + ] + + def assert_raises_integrity_error(self, *args, **kwargs): + """Assert the code block raises an integrity error. + + Returns: + Error catcher that will assert if an integrity error is raised. + """ + + return self.assertRaises(IntegrityError, *args, **kwargs) diff --git a/codeforlife/user/migrations/0001_initial.py b/codeforlife/user/migrations/0001_initial.py index 42075775..8ebb846b 100644 --- a/codeforlife/user/migrations/0001_initial.py +++ b/codeforlife/user/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.20 on 2023-12-08 15:41 +# Generated by Django 3.2.20 on 2023-12-08 16:55 from django.conf import settings import django.core.validators @@ -30,7 +30,7 @@ class Migration(migrations.Migration): ('email', models.EmailField(blank=True, max_length=254, null=True, unique=True, verbose_name='email address')), ('is_staff', models.BooleanField(default=False, help_text='Designates whether the user can log into this admin site.', verbose_name='staff status')), ('is_active', models.BooleanField(default=False, help_text='Designates whether this user should be treated as active. Unselect this instead of deleting accounts.', verbose_name='active')), - ('date_joined', models.DateTimeField(default=django.utils.timezone.now, verbose_name='date joined')), + ('date_joined', models.DateTimeField(default=django.utils.timezone.now, editable=False, verbose_name='date joined')), ('otp_secret', models.CharField(editable=False, help_text='Secret used to generate a OTP.', max_length=40, null=True, verbose_name='OTP secret')), ('last_otp_for_time', models.DateTimeField(editable=False, help_text='Used to prevent replay attacks, where the same OTP is used for different times.', null=True, verbose_name='last OTP for-time')), ], @@ -205,6 +205,10 @@ class Migration(migrations.Migration): name='authfactor', unique_together={('user', 'type')}, ), + migrations.AddConstraint( + model_name='user', + constraint=models.CheckConstraint(check=models.Q(models.Q(('student__isnull', False), ('teacher__isnull', True)), models.Q(('student__isnull', True), ('teacher__isnull', False)), models.Q(('student__isnull', True), ('teacher__isnull', True)), _connector='OR'), name='user__profile'), + ), migrations.AddConstraint( model_name='user', constraint=models.CheckConstraint(check=models.Q(models.Q(('email__isnull', False), ('teacher__isnull', False)), models.Q(('email__isnull', True), ('student__isnull', False)), models.Q(('email__isnull', False), ('student__isnull', True), ('teacher__isnull', True)), _connector='OR'), name='user__email'), diff --git a/codeforlife/user/models/user.py b/codeforlife/user/models/user.py index ddeec1c9..1dc97c20 100644 --- a/codeforlife/user/models/user.py +++ b/codeforlife/user/models/user.py @@ -16,6 +16,7 @@ from django.db import models from django.db.models import Q from django.db.models.query import QuerySet +from django.db.utils import IntegrityError from django.utils import timezone from django.utils.translation import gettext_lazy as _ from django_stubs_ext.db.models import TypedModelMeta @@ -40,6 +41,11 @@ class Manager(BaseUserManager["User"]): Custom user manager for custom user model. """ + def create(self, **kwargs): + """Prevent calling create to maintain data integrity.""" + + raise IntegrityError("Must call create_user instead.") + def _create_user( self, password: str, @@ -57,19 +63,14 @@ def _create_user( user.save(using=self._db) return user - def create_user( - self, - password: str, - email: t.Optional[str] = None, - **fields, - ): + def create_user(self, password: str, first_name: str, **fields): """Create a user. https://github.com/django/django/blob/19bc11f636ca2b5b80c3d9ad5b489e43abad52bb/django/contrib/auth/models.py#L149C9-L149C20 Args: password: The user's non-hashed password. - email: The user's email address. + first_name: The user's first name. Returns: A user instance. @@ -77,21 +78,16 @@ def create_user( fields.setdefault("is_staff", False) fields.setdefault("is_superuser", False) - return self._create_user(password, email, **fields) + return self._create_user(password, first_name=first_name, **fields) - def create_superuser( - self, - password: str, - email: t.Optional[str] = None, - **fields, - ): + def create_superuser(self, password: str, first_name: str, **fields): """Create a super user. https://github.com/django/django/blob/19bc11f636ca2b5b80c3d9ad5b489e43abad52bb/django/contrib/auth/models.py#L154C9-L154C25 Args: password: The user's non-hashed password. - email: The user's email address. + first_name: The user's first name. Raises: ValueError: If is_staff is not True. @@ -109,7 +105,7 @@ def create_superuser( if fields.get("is_superuser") is not True: raise ValueError("Superuser must have is_superuser=True.") - return self._create_user(password, email, **fields) + return self._create_user(password, first_name=first_name, **fields) objects: Manager = Manager.from_queryset( # type: ignore[misc] AbstractModel.QuerySet @@ -125,7 +121,7 @@ def create_superuser( blank=True, ) - # QUES: is last name required for teachers? + # TODO: is last name required for teachers? last_name = models.CharField( _("last name"), max_length=150, @@ -160,6 +156,7 @@ def create_superuser( date_joined = models.DateTimeField( _("date joined"), default=timezone.now, + editable=False, ) otp_secret = models.CharField( @@ -202,6 +199,24 @@ class Meta(TypedModelMeta): verbose_name = _("user") verbose_name_plural = _("users") constraints = [ + models.CheckConstraint( + check=( + # pylint: disable-next=unsupported-binary-operation + Q( + teacher__isnull=True, + student__isnull=False, + ) + | Q( + teacher__isnull=False, + student__isnull=True, + ) + | Q( + teacher__isnull=True, + student__isnull=True, + ) + ), + name="user__profile", + ), models.CheckConstraint( check=( # pylint: disable-next=unsupported-binary-operation diff --git a/codeforlife/user/tests/models/test_abstract.py b/codeforlife/user/tests/models/test_abstract.py new file mode 100644 index 00000000..e2254c64 --- /dev/null +++ b/codeforlife/user/tests/models/test_abstract.py @@ -0,0 +1,107 @@ +""" +© Ocado Group +Created on 08/12/2023 at 15:48:38(+00:00). +""" + +from unittest.mock import patch + +from django.test import TestCase +from django.utils import timezone + +from ...models import User + + +class TestAbstract(TestCase): + """ + Tests the abstract model inherited by other models. + + Abstract model path: codeforlife.models + """ + + fixtures = [ + "users", + "teachers", + ] + + def setUp(self): + self.john_doe = User.objects.get(pk=1) + self.jane_doe = User.objects.get(pk=2) + + def test_delete(self): + """ + Deleting a model instance sets its deletion schedule. + """ + + now = timezone.now() + with patch.object(timezone, "now", return_value=now) as timezone_now: + self.john_doe.delete() + + assert timezone_now.call_count == 2 + assert self.john_doe.delete_after == now + User.delete_wait + assert self.john_doe.last_saved_at == now + + def test_objects__delete(self): + """ + Deleting a set of models in a query sets their deletion schedule. + """ + + now = timezone.now() + with patch.object(timezone, "now", return_value=now) as timezone_now: + User.objects.filter( + pk__in=[ + self.john_doe.pk, + self.jane_doe.pk, + ] + ).delete() + + assert timezone_now.call_count == 2 + + self.john_doe.refresh_from_db() + assert self.john_doe.delete_after == now + User.delete_wait + assert self.john_doe.last_saved_at == now + + self.jane_doe.refresh_from_db() + assert self.jane_doe.delete_after == now + User.delete_wait + assert self.jane_doe.last_saved_at == now + + def test_objects__create(self): + """ + Creating a model records when it was first saved. + """ + + now = timezone.now() + with patch.object(timezone, "now", return_value=now) as timezone_now: + user = User.objects.create_user( + password="password", + first_name="first_name", + last_name="last_name", + email="example@email.com", + ) + + assert timezone_now.call_count == 1 + assert user.last_saved_at == now + + def test_objects__bulk_create(self): + """ + Bulk creating models records when they were first saved. + """ + + now = timezone.now() + with patch.object(timezone, "now", return_value=now) as timezone_now: + users = User.objects.bulk_create( + [ + User( + first_name="first_name_1", + last_name="last_name_1", + email="example_1@email.com", + ), + User( + first_name="first_name_2", + last_name="last_name_2", + email="example_2@email.com", + ), + ] + ) + + assert timezone_now.call_count == 2 + assert all(user.last_saved_at == now for user in users) diff --git a/codeforlife/user/tests/models/test_klass.py b/codeforlife/user/tests/models/test_klass.py new file mode 100644 index 00000000..18d3fee0 --- /dev/null +++ b/codeforlife/user/tests/models/test_klass.py @@ -0,0 +1,18 @@ +""" +© Ocado Group +Created on 08/12/2023 at 17:43:11(+00:00). +""" + +from ....tests import ModelTestCase +from ...models import Class + + +class TestClass(ModelTestCase[Class]): + """Tests the Class model.""" + + def test_id__validators__regex(self): + """ + Check the regex validation of a class' ID. + """ + + raise NotImplementedError() # TODO diff --git a/codeforlife/user/tests/models/test_school.py b/codeforlife/user/tests/models/test_school.py new file mode 100644 index 00000000..a570714a --- /dev/null +++ b/codeforlife/user/tests/models/test_school.py @@ -0,0 +1,18 @@ +""" +© Ocado Group +Created on 08/12/2023 at 17:43:11(+00:00). +""" + +from ....tests import ModelTestCase +from ...models import School + + +class TestSchool(ModelTestCase[School]): + """Tests the School model.""" + + def test_constraints__no_uk_county_if_country_not_uk(self): + """ + Cannot have set a UK county if the country is not set to UK. + """ + + raise NotImplementedError() # TODO diff --git a/codeforlife/user/tests/models/test_student.py b/codeforlife/user/tests/models/test_student.py new file mode 100644 index 00000000..101e673e --- /dev/null +++ b/codeforlife/user/tests/models/test_student.py @@ -0,0 +1,46 @@ +""" +© Ocado Group +Created on 08/12/2023 at 17:43:11(+00:00). +""" + +from ....tests import ModelTestCase +from ...models import Student + + +class TestStudent(ModelTestCase[Student]): + """Tests the Student model.""" + + def test_objects__create(self): + """ + Create a student. + """ + + raise NotImplementedError() # TODO + + def test_objects__bulk_create(self): + """ + Bulk create many students. + """ + + raise NotImplementedError() # TODO + + def test_objects__create_user(self): + """ + Create a user with a student profile. + """ + + raise NotImplementedError() # TODO + + def test_objects__bulk_create_users(self): + """ + Bulk create many users with a student profile. + """ + + raise NotImplementedError() # TODO + + def test_teacher(self): + """ + Get student's teacher. + """ + + raise NotImplementedError() # TODO diff --git a/codeforlife/user/tests/models/test_teacher.py b/codeforlife/user/tests/models/test_teacher.py new file mode 100644 index 00000000..7b852bac --- /dev/null +++ b/codeforlife/user/tests/models/test_teacher.py @@ -0,0 +1,41 @@ +""" +© Ocado Group +Created on 08/12/2023 at 17:43:11(+00:00). +""" + +from ....tests import ModelTestCase +from ...models import Teacher + + +class TestTeacher(ModelTestCase[Teacher]): + """Tests the Teacher model.""" + + def test_objects__create_user(self): + """ + Create a user with a teacher profile. + """ + + teacher_fields = {"is_admin": True} + user_fields = { + "first_name": "first_name", + "email": "example@codeforlife.com", + "password": "password", + } + + user = Teacher.objects.create_user( + teacher=teacher_fields, + **user_fields, + ) + + assert user.first_name == user_fields["first_name"] + assert user.email == user_fields["email"] + assert user.password != user_fields["password"] + assert user.check_password(user_fields["password"]) + assert user.teacher.is_admin == teacher_fields["is_admin"] + + def test_students(self): + """ + Get all students from all classes. + """ + + raise NotImplementedError() # TODO diff --git a/codeforlife/user/tests/models/test_user.py b/codeforlife/user/tests/models/test_user.py index e8cb91ae..f07a879d 100644 --- a/codeforlife/user/tests/models/test_user.py +++ b/codeforlife/user/tests/models/test_user.py @@ -1,89 +1,128 @@ -from unittest.mock import patch +""" +© Ocado Group +Created on 08/12/2023 at 17:37:30(+00:00). +""" -from django.test import TestCase -from django.utils import timezone +from ....tests import ModelTestCase +from ...models import Student, Teacher, User -from ...models import Teacher, User - -class TestUser(TestCase): +class TestUser(ModelTestCase[User]): """Tests the User model.""" - fixtures = [ - "users.json", - "teachers.json", - ] - - def setUp(self): - self.john_doe = User.objects.get(pk=1) - self.jane_doe = User.objects.get(pk=2) - - def test_delete(self): - now = timezone.now() - with patch.object(timezone, "now", return_value=now) as timezone_now: - self.john_doe.delete() + def test_constraints__profile(self): + """ + Independents must have an email. + """ - assert timezone_now.call_count == 2 - assert self.john_doe.delete_after == now + User.delete_wait - assert self.john_doe.last_saved_at == now + with self.assert_raises_integrity_error(): + User.objects.create( + first_name="student_and_teacher", + teacher=Teacher.objects.create(), + student=Student.objects.create(auto_gen_password="password"), + ) - def test_objects__delete(self): - now = timezone.now() - with patch.object(timezone, "now", return_value=now) as timezone_now: - User.objects.filter( - pk__in=[ - self.john_doe.pk, - self.jane_doe.pk, - ] - ).delete() + def test_constraints__email__teacher(self): + """ + Teachers must have an email. + """ - assert timezone_now.call_count == 2 + with self.assert_raises_integrity_error(): + User.objects.create( + first_name="teacher", + teacher=Teacher.objects.create(), + ) - self.john_doe.refresh_from_db() - assert self.john_doe.delete_after == now + User.delete_wait - assert self.john_doe.last_saved_at == now + def test_constraints__email__student(self): + """ + Student cannot have an email. + """ - self.jane_doe.refresh_from_db() - assert self.jane_doe.delete_after == now + User.delete_wait - assert self.jane_doe.last_saved_at == now + with self.assert_raises_integrity_error(): + User.objects.create( + first_name="student", + student=Student.objects.create(auto_gen_password="password"), + email="student@codeforlife.com", + ) - def test_objects__create(self): - teacher = Teacher.objects.create() + def test_constraints__email__indy(self): + """ + Independents must have an email. + """ - now = timezone.now() - with patch.object(timezone, "now", return_value=now) as timezone_now: - user = User.objects.create( + with self.assert_raises_integrity_error(): + User.objects.create( first_name="first_name", - last_name="last_name", - email="example@email.com", - teacher=teacher, ) - assert timezone_now.call_count == 1 - assert user.last_saved_at == now - - def test_objects__bulk_create(self): - teacher_1 = Teacher.objects.create() - teacher_2 = Teacher.objects.create() - - now = timezone.now() - with patch.object(timezone, "now", return_value=now) as timezone_now: - users = User.objects.bulk_create( - [ - User( - first_name="first_name_1", - last_name="last_name_1", - email="example_1@email.com", - teacher=teacher_1, - ), - User( - first_name="first_name_2", - last_name="last_name_2", - email="example_2@email.com", - teacher=teacher_2, - ), - ] - ) - - assert timezone_now.call_count == 2 - assert all(user.last_saved_at == now for user in users) + def test_objects__create(self): + """ + Cannot call objects.create. + """ + + with self.assert_raises_integrity_error(): + User.objects.create() + + def test_objects__create_user__teacher(self): + """ + Create a teacher user. + """ + + raise NotImplementedError() # TODO + + def test_objects__create_user__student(self): + """ + Create a student user. + """ + + raise NotImplementedError() # TODO + + def test_objects__create_user__indy(self): + """ + Create an independent user. + """ + + user_fields = { + "first_name": "first_name", + "email": "example@codeforlife.com", + "password": "password", + } + + user = User.objects.create_user(**user_fields) + assert user.first_name == user_fields["first_name"] + assert user.email == user_fields["email"] + assert user.password != user_fields["password"] + assert user.check_password(user_fields["password"]) + + def test_objects__create_superuser__teacher(self): + """ + Create a teacher super user. + """ + + raise NotImplementedError() # TODO + + def test_objects__create_superuser__student(self): + """ + Create a student super user. + """ + + raise NotImplementedError() # TODO + + def test_objects__create_superuser__indy(self): + """ + Create an independent super user. + """ + + user_fields = { + "first_name": "first_name", + "email": "example@codeforlife.com", + "password": "password", + } + + user = User.objects.create_superuser(**user_fields) + assert user.first_name == user_fields["first_name"] + assert user.email == user_fields["email"] + assert user.password != user_fields["password"] + assert user.check_password(user_fields["password"]) + assert user.is_staff + assert user.is_superuser