diff --git a/codeforlife/tests/model_view_set.py b/codeforlife/tests/model_view_set.py index cb9290c2..fd1dfd3c 100644 --- a/codeforlife/tests/model_view_set.py +++ b/codeforlife/tests/model_view_set.py @@ -12,7 +12,7 @@ from django.db.models import Model from django.db.models.query import QuerySet -from django.urls import reverse as _reverse +from django.urls import reverse from django.utils import timezone from django.utils.http import urlencode from pyotp import TOTP @@ -22,7 +22,14 @@ from ..permissions import Permission from ..serializers import ModelSerializer -from ..user.models import AuthFactor, User +from ..user.models import ( + AuthFactor, + NonSchoolTeacherUser, + SchoolTeacherUser, + StudentUser, + TeacherUser, + User, +) from ..views import ModelViewSet AnyModel = t.TypeVar("AnyModel", bound=Model) @@ -181,39 +188,6 @@ def parse_data(data): "Data does not equal serialized model.", ) - def reverse( - self, - action: str, - model: t.Optional[AnyModel] = None, - **kwargs, - ): - """Get the reverse URL for the model view set's action. - - Args: - action: The name of the action. - model: The model to look up. - - Returns: - The reversed URL. - """ - - reverse_kwargs = kwargs.pop("kwargs", {}) - if model is not None: - reverse_kwargs[self._model_view_set_class.lookup_field] = getattr( - model, - self._model_view_set_class.lookup_field, - ) - - return _reverse( - viewname=kwargs.pop( - "viewname", - # pylint: disable-next=no-member - f"{self._test_case.basename}-{action}", - ), - kwargs=reverse_kwargs, - **kwargs, - ) - # pylint: disable-next=too-many-arguments def generic( self, @@ -278,7 +252,8 @@ def create( # pylint: enable=line-too-long response: Response = self.post( - self.reverse("list"), + # pylint: disable-next=no-member + self._test_case.reverse_action("list"), data=json.dumps(data), content_type="application/json", status_code_assertion=status_code_assertion, @@ -316,7 +291,8 @@ def bulk_create( # pylint: enable=line-too-long response: Response = self.post( - self.reverse("bulk"), + # pylint: disable-next=no-member + self._test_case.reverse_action("bulk"), data=json.dumps(data), content_type="application/json", status_code_assertion=status_code_assertion, @@ -356,7 +332,8 @@ def retrieve( # pylint: enable=line-too-long response: Response = self.get( - self.reverse("detail", model), + # pylint: disable-next=no-member + self._test_case.reverse_action("detail", model), status_code_assertion=status_code_assertion, **kwargs, ) @@ -403,7 +380,11 @@ def list( ).exists(), "List must exclude some models for a valid test." response: Response = self.get( - f"{self.reverse('list')}?{urlencode(filters or {})}", + ( + # pylint: disable-next=no-member + self._test_case.reverse_action("list") + + f"?{urlencode(filters or {})}" + ), status_code_assertion=status_code_assertion, **kwargs, ) @@ -445,7 +426,8 @@ def partial_update( # pylint: enable=line-too-long response: Response = self.patch( - self.reverse("detail", model), + # pylint: disable-next=no-member + self._test_case.reverse_action("detail", model), data=json.dumps(data), content_type="application/json", status_code_assertion=status_code_assertion, @@ -490,7 +472,8 @@ def bulk_partial_update( # pylint: enable=line-too-long response: Response = self.patch( - self.reverse("bulk"), + # pylint: disable-next=no-member + self._test_case.reverse_action("bulk"), data=json.dumps(data), content_type="application/json", status_code_assertion=status_code_assertion, @@ -532,7 +515,8 @@ def destroy( """ response: Response = self.delete( - self.reverse("detail", model), + # pylint: disable-next=no-member + self._test_case.reverse_action("detail", model), status_code_assertion=status_code_assertion, **kwargs, ) @@ -569,7 +553,8 @@ def bulk_destroy( """ response: Response = self.delete( - self.reverse("bulk"), + # pylint: disable-next=no-member + self._test_case.reverse_action("bulk"), data=json.dumps(lookup_values), content_type="application/json", status_code_assertion=status_code_assertion, @@ -612,12 +597,16 @@ def login(self, **credentials): return user - def login_teacher(self, is_admin: t.Optional[bool] = None, **credentials): + def login_teacher( + self, + is_admin: t.Optional[bool] = None, + **credentials, + ) -> TeacherUser: # pylint: disable=line-too-long """Log in a user and assert they are a teacher. Args: - is_admin: Whether or not the teacher is an admin. Set none if a teacher can be either or. + is_admin: Whether or not the teacher is an admin. Set to None if the teacher can be either or. Returns: The teacher-user. @@ -626,12 +615,43 @@ def login_teacher(self, is_admin: t.Optional[bool] = None, **credentials): user = self.login(**credentials) assert user.teacher - assert user.teacher.school if is_admin is not None: assert is_admin == user.teacher.is_admin + return user - def login_student(self, **credentials): + def login_school_teacher( + self, + is_admin: t.Optional[bool] = None, + **credentials, + ): + # pylint: disable=line-too-long + """Log in a user and assert they are a school-teacher. + + Args: + is_admin: Whether or not the teacher is an admin. Set to None if the teacher can be either or. + + Returns: + The school-teacher-user. + """ + # pylint: enable=line-too-long + + user = self.login_teacher(is_admin, **credentials) + assert user.teacher.school + return t.cast(SchoolTeacherUser, user) + + def login_non_school_teacher(self, **credentials): + """Log in a user and assert they are a non-school-teacher. + + Returns: + The non-school-teacher-user. + """ + + user = self.login_teacher(is_admin=None, **credentials) + assert not user.teacher.school + return t.cast(NonSchoolTeacherUser, user) + + def login_student(self, **credentials) -> StudentUser: """Log in a user and assert they are a student. Returns: @@ -643,7 +663,8 @@ def login_student(self, **credentials): assert user.student.class_field.teacher.school return user - def login_indy(self, **credentials): + # TODO: set return type to IndependentUser when we use the new data models. + def login_indy(self, **credentials) -> StudentUser: """Log in an independent and assert they are a student. Returns: @@ -690,6 +711,33 @@ def setUpClass(cls): return super().setUpClass() + def reverse_action( + self, + name: str, + model: t.Optional[AnyModel] = None, + **kwargs, + ): + """Get the reverse URL for the model view set's action. + + Args: + name: The name of the action. + model: The model to look up. + + Returns: + The reversed URL for the model view set's action. + """ + + reverse_kwargs = kwargs.pop("kwargs", {}) + if model is not None: + lookup_field = self.model_view_set_class.lookup_field + reverse_kwargs[lookup_field] = getattr(model, lookup_field) + + return reverse( + viewname=kwargs.pop("viewname", f"{self.basename}-{name}"), + kwargs=reverse_kwargs, + **kwargs, + ) + def assert_get_permissions( self, permissions: t.List[Permission], diff --git a/codeforlife/user/migrations/0002_nonschoolteacher_nonschoolteacheruser_schoolteacher_schoolteacheruser_studentuser_teacheruser.py b/codeforlife/user/migrations/0002_nonschoolteacher_nonschoolteacheruser_schoolteacher_schoolteacheruser_studentuser_teacheruser.py new file mode 100644 index 00000000..561c58f3 --- /dev/null +++ b/codeforlife/user/migrations/0002_nonschoolteacher_nonschoolteacheruser_schoolteacher_schoolteacheruser_studentuser_teacheruser.py @@ -0,0 +1,93 @@ +# Generated by Django 3.2.20 on 2024-02-05 10:04 + +import django.contrib.auth.models +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('common', '0045_otp'), + ('user', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='NonSchoolTeacher', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('common.teacher',), + ), + migrations.CreateModel( + name='NonSchoolTeacherUser', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('user.user',), + managers=[ + ('objects', django.contrib.auth.models.UserManager()), + ], + ), + migrations.CreateModel( + name='SchoolTeacher', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('common.teacher',), + ), + migrations.CreateModel( + name='SchoolTeacherUser', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('user.user',), + managers=[ + ('objects', django.contrib.auth.models.UserManager()), + ], + ), + migrations.CreateModel( + name='StudentUser', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('user.user',), + managers=[ + ('objects', django.contrib.auth.models.UserManager()), + ], + ), + migrations.CreateModel( + name='TeacherUser', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('user.user',), + managers=[ + ('objects', django.contrib.auth.models.UserManager()), + ], + ), + ] diff --git a/codeforlife/user/models/__init__.py b/codeforlife/user/models/__init__.py index b61114af..fdf82727 100644 --- a/codeforlife/user/models/__init__.py +++ b/codeforlife/user/models/__init__.py @@ -8,5 +8,12 @@ from .session import Session from .session_auth_factor import SessionAuthFactor from .student import Student -from .teacher import Teacher -from .user import User, UserProfile # TODO: remove UserProfile +from .teacher import NonSchoolTeacher, SchoolTeacher, Teacher +from .user import ( # TODO: remove UserProfile + NonSchoolTeacherUser, + SchoolTeacherUser, + StudentUser, + TeacherUser, + User, + UserProfile, +) diff --git a/codeforlife/user/models/teacher.py b/codeforlife/user/models/teacher.py index d53ab4f1..4c8dea44 100644 --- a/codeforlife/user/models/teacher.py +++ b/codeforlife/user/models/teacher.py @@ -1,68 +1,27 @@ -# from django.db import models +""" +© Ocado Group +Created on 05/02/2024 at 09:49:56(+00:00). +""" -# from .user import User -# from .school import School - - -# class TeacherModelManager(models.Manager): -# def factory(self, first_name, last_name, email, password): -# user = User.objects.create_user( -# username=email, -# email=email, -# password=password, -# first_name=first_name, -# last_name=last_name, -# ) - -# return Teacher.objects.create(user=user) +from common.models import Teacher +from django_stubs_ext.db.models import TypedModelMeta -# # Filter out non active teachers by default -# def get_queryset(self): -# return super().get_queryset().filter(user__is_active=True) +from .school import School -# class Teacher(models.Model): -# user = models.OneToOneField( -# User, -# related_name="teacher", -# null=True, -# blank=True, -# on_delete=models.CASCADE, -# ) -# school = models.ForeignKey( -# School, -# related_name="school_teacher", -# null=True, -# blank=True, -# on_delete=models.SET_NULL, -# ) -# is_admin = models.BooleanField(default=False) -# blocked_time = models.DateTimeField(null=True, blank=True) -# invited_by = models.ForeignKey( -# "self", -# related_name="invited_teachers", -# null=True, -# blank=True, -# on_delete=models.SET_NULL, -# ) +class SchoolTeacher(Teacher): + """A teacher that is in a school.""" -# objects = TeacherModelManager() + school: School -# def teaches(self, userprofile): -# if hasattr(userprofile, "student"): -# student = userprofile.student -# return ( -# not student.is_independent() -# and student.class_field.teacher == self -# ) + class Meta(TypedModelMeta): + proxy = True -# def has_school(self): -# return self.school is not (None or "") -# def has_class(self): -# return self.class_teacher.exists() +class NonSchoolTeacher(Teacher): + """A teacher that is not in a school.""" -# def __str__(self): -# return f"{self.user.first_name} {self.user.last_name}" + school: None -from common.models import Teacher + class Meta(TypedModelMeta): + proxy = True diff --git a/codeforlife/user/models/user.py b/codeforlife/user/models/user.py index 79b24999..7df30615 100644 --- a/codeforlife/user/models/user.py +++ b/codeforlife/user/models/user.py @@ -1,64 +1,35 @@ -# from datetime import timedelta -# from enum import Enum - -# from django.contrib.auth.models import AbstractUser -# from django.contrib.auth.models import UserManager as AbstractUserManager -# from django.db import models -# from django.utils import timezone - - -# class UserManager(AbstractUserManager): -# def create_user(self, username, email=None, password=None, **extra_fields): -# return super().create_user(username, email, password, **extra_fields) - -# def create_superuser( -# self, username, email=None, password=None, **extra_fields -# ): -# return super().create_superuser( -# username, email, password, **extra_fields -# ) - - -# class User(AbstractUser): -# class Type(str, Enum): -# TEACHER = "teacher" -# DEP_STUDENT = "dependent-student" -# INDEP_STUDENT = "independent-student" - -# developer = models.BooleanField(default=False) -# is_verified = models.BooleanField(default=False) - -# objects: UserManager = UserManager() - -# def __str__(self): -# return self.get_full_name() - -# @property -# def joined_recently(self): -# return timezone.now() - timedelta(days=7) <= self.date_joined +""" +© Ocado Group +Created on 05/02/2024 at 09:50:04(+00:00). +""" import typing as t from common.models import UserProfile + +# pylint: disable-next=imported-auth-user from django.contrib.auth.models import User as _User from django.db.models.query import QuerySet from django.utils.translation import gettext_lazy as _ +from django_stubs_ext.db.models import TypedModelMeta from . import auth_factor, otp_bypass_token, session from .student import Student -from .teacher import Teacher +from .teacher import NonSchoolTeacher, SchoolTeacher, Teacher class User(_User): _password: t.Optional[str] - id: int - auth_factors: QuerySet["auth_factor.AuthFactor"] - otp_bypass_tokens: QuerySet["otp_bypass_token.OtpBypassToken"] - session: "session.Session" + id: int # type: ignore[assignment] + auth_factors: QuerySet["auth_factor.AuthFactor"] # type: ignore[assignment] + otp_bypass_tokens: QuerySet[ # type: ignore[assignment] + "otp_bypass_token.OtpBypassToken" + ] + session: "session.Session" # type: ignore[assignment] userprofile: UserProfile - class Meta: + class Meta(TypedModelMeta): proxy = True @property @@ -109,3 +80,52 @@ def is_verified(self): @property def aimmo_badges(self): return self.userprofile.aimmo_badges + + +class TeacherUser(User): + """A user that is a teacher.""" + + teacher: Teacher + student: None + + class Meta(TypedModelMeta): + proxy = True + + +class SchoolTeacherUser(User): + """A user that is a teacher in a school.""" + + teacher: SchoolTeacher + student: None + + class Meta(TypedModelMeta): + proxy = True + + +class NonSchoolTeacherUser(User): + """A user that is a teacher not in a school.""" + + teacher: NonSchoolTeacher + student: None + + class Meta(TypedModelMeta): + proxy = True + + +class StudentUser(User): + """A user that is a student.""" + + teacher: None + student: Student + + class Meta(TypedModelMeta): + proxy = True + + +# TODO: uncomment this when using new models. +# class IndependentUser(User): +# teacher: None +# student: None + +# class Meta(TypedModelMeta): +# proxy = True diff --git a/codeforlife/user/tests/views/test_user.py b/codeforlife/user/tests/views/test_user.py index cacb475a..50f56fcb 100644 --- a/codeforlife/user/tests/views/test_user.py +++ b/codeforlife/user/tests/views/test_user.py @@ -67,15 +67,15 @@ def setUp(self): new_user=user, ) - def _login_teacher(self): - return self.client.login_teacher( + def _login_school_teacher(self): + return self.client.login_school_teacher( email="maxplanck@codeforlife.com", password="Password1", is_admin=False, ) - def _login_admin_teacher(self): - return self.client.login_teacher( + def _login_admin_school_teacher(self): + return self.client.login_school_teacher( email="alberteinstein@codeforlife.com", password="Password1", is_admin=True, @@ -123,7 +123,7 @@ def test_retrieve__teacher__self(self): Teacher can retrieve their own user data. """ - user = self._login_teacher() + user = self._login_school_teacher() self.client.retrieve(user) @@ -150,7 +150,7 @@ def test_retrieve__teacher__teacher__same_school(self): Teacher can retrieve another teacher from the same school. """ - user = self._login_teacher() + user = self._login_school_teacher() other_user = self.get_another_school_user( user, @@ -168,7 +168,7 @@ def test_retrieve__teacher__student__same_school__same_class(self): Teacher can retrieve a student from the same school and class. """ - user = self._login_teacher() + user = self._login_school_teacher() other_user = self.get_another_school_user( user, @@ -189,7 +189,7 @@ def test_retrieve__teacher__student__same_school__not_same_class(self): class. """ - user = self._login_teacher() + user = self._login_school_teacher() other_user = self.get_another_school_user( user, @@ -208,7 +208,7 @@ def test_retrieve__admin_teacher__student__same_school__same_class(self): Admin teacher can retrieve a student from the same school and class. """ - user = self._login_admin_teacher() + user = self._login_admin_school_teacher() other_user = self.get_another_school_user( user, @@ -231,7 +231,7 @@ def test_retrieve__admin_teacher__student__same_school__not_same_class( different class. """ - user = self._login_admin_teacher() + user = self._login_admin_school_teacher() other_user = self.get_another_school_user( user, @@ -332,7 +332,7 @@ def test_retrieve__teacher__teacher__not_same_school(self): Teacher cannot retrieve another teacher from another school. """ - user = self._login_teacher() + user = self._login_school_teacher() other_user = self.get_another_school_user( user, @@ -350,7 +350,7 @@ def test_retrieve__teacher__student__not_same_school(self): Teacher cannot retrieve a student from another school. """ - user = self._login_teacher() + user = self._login_school_teacher() other_user = self.get_another_school_user( user, @@ -449,7 +449,7 @@ def test_list__teacher(self): Teacher can list all the users in the same class. """ - user = self._login_teacher() + user = self._login_school_teacher() self.client.list( User.objects.filter(new_teacher__school=user.teacher.school) @@ -464,7 +464,7 @@ def test_list__teacher__students_in_class(self): Teacher can list all the users in a class they own. """ - user = self._login_teacher() + user = self._login_school_teacher() klass = user.teacher.class_teacher.first() assert klass