Skip to content

Commit

Permalink
fix: create class (#73)
Browse files Browse the repository at this point in the history
* fix: add new props for new user

* fix: permissions for base viewsets

* fix: assert helpers for create and update

* type hints
  • Loading branch information
SKairinos authored Feb 6, 2024
1 parent 8afb1bb commit e6a3a55
Show file tree
Hide file tree
Showing 12 changed files with 307 additions and 73 deletions.
60 changes: 57 additions & 3 deletions codeforlife/serializers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,13 @@
from rest_framework.serializers import BaseSerializer as _BaseSerializer

from ..request import Request
from ..user.models import User
from ..user.models import ( # TODO: add IndependentUser
NonSchoolTeacherUser,
SchoolTeacherUser,
StudentUser,
TeacherUser,
User,
)


# pylint: disable-next=abstract-method
Expand All @@ -28,15 +34,63 @@ def request(self):
@property
def request_user(self):
"""
The user that made the request. Assumes the user has authenticated.
The user that made the request.
Assumes the user has authenticated.
"""

return t.cast(User, self.request.user)

@property
def request_teacher_user(self):
"""
The teacher-user that made the request.
Assumes the user has authenticated.
"""

return t.cast(TeacherUser, self.request.user)

@property
def request_school_teacher_user(self):
"""
The school-teacher-user that made the request.
Assumes the user has authenticated.
"""

return t.cast(SchoolTeacherUser, self.request.user)

@property
def request_non_school_teacher_user(self):
"""
The non-school-teacher-user that made the request.
Assumes the user has authenticated.
"""

return t.cast(NonSchoolTeacherUser, self.request.user)

@property
def request_student_user(self):
"""
The student-user that made the request.
Assumes the user has authenticated.
"""

return t.cast(StudentUser, self.request.user)

# TODO: uncomment when moving to new data models.
# @property
# def request_indy_user(self):
# """
# The independent-user that made the request.
# Assumes the user has authenticated.
# """

# return t.cast(IndependentUser, self.request.user)

@property
def request_anon_user(self):
"""
The user that made the request. Assumes the user has not authenticated.
The user that made the request.
Assumes the user has not authenticated.
"""

return t.cast(AnonymousUser, self.request.user)
Expand Down
81 changes: 72 additions & 9 deletions codeforlife/tests/model_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
"""

import typing as t
from unittest.case import _AssertRaisesContext

from django.db.models import Model
from django.forms.models import model_to_dict
from django.test import TestCase
from rest_framework.serializers import ValidationError
from rest_framework.test import APIRequestFactory
Expand All @@ -26,6 +28,8 @@ class ModelSerializerTestCase(TestCase, t.Generic[AnyModel]):

request_factory = APIRequestFactory()

Data = t.Dict[str, t.Any]

@classmethod
def setUpClass(cls):
attr_name = "model_serializer_class"
Expand Down Expand Up @@ -56,23 +60,26 @@ def assert_raises_validation_error(self, code: str, *args, **kwargs):
The assert-raises context which will auto-assert the code.
"""

context = self.assertRaises(ValidationError, *args, **kwargs)

class ContextWrapper:
class Wrapper:
"""Wrap context to assert code on exit."""

def __init__(self, context):
self.context = context
def __init__(self, ctx: "_AssertRaisesContext[ValidationError]"):
self.ctx = ctx

def __enter__(self, *args, **kwargs):
return self.context.__enter__(*args, **kwargs)
return self.ctx.__enter__(*args, **kwargs)

def __exit__(self, *args, **kwargs):
value = self.context.__exit__(*args, **kwargs)
assert self.context.exception.detail[0].code == code
value = self.ctx.__exit__(*args, **kwargs)
assert (
code
== self.ctx.exception.detail[ # type: ignore[union-attr]
0 # type: ignore[index]
].code
)
return value

return ContextWrapper(context)
return Wrapper(self.assertRaises(ValidationError, *args, **kwargs))

# pylint: disable-next=too-many-arguments
def _assert_validate(
Expand Down Expand Up @@ -166,3 +173,59 @@ def get_validate(serializer: ModelSerializer[AnyModel]):
get_validate,
**kwargs,
)

def _assert_data_is_subset_of_model(self, data: Data, model):
assert isinstance(model, Model)

for field, value in data.copy().items():
# NOTE: A data value of type dict == a foreign object on the model.
if isinstance(value, dict):
self._assert_data_is_subset_of_model(
value,
getattr(model, field),
)
data.pop(field)

self.assertDictContainsSubset(data, model_to_dict(model))

def assert_create(
self,
validated_data: Data,
*args,
new_data: t.Optional[Data] = None,
**kwargs,
):
"""Assert that the data used to create the model is a subset of the
model's data.
Args:
validated_data: The data used to create the model.
new_data: Any new data that the model may have after creating.
"""

serializer = self.model_serializer_class(*args, **kwargs)
model = serializer.create(validated_data)
data = {**validated_data, **(new_data or {})}
self._assert_data_is_subset_of_model(data, model)

def assert_update(
self,
instance: AnyModel,
validated_data: Data,
*args,
new_data: t.Optional[Data] = None,
**kwargs,
):
"""Assert that the data used to update the model is a subset of the
model's data.
Args:
instance: The model instance to update.
validated_data: The data used to update the model.
new_data: Any new data that the model may have after updating.
"""

serializer = self.model_serializer_class(*args, **kwargs)
model = serializer.update(instance, validated_data)
data = {**validated_data, **(new_data or {})}
self._assert_data_is_subset_of_model(data, model)
6 changes: 3 additions & 3 deletions codeforlife/tests/model_view_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ def get_other_user(
other_user = other_users.first()
assert other_user
assert user != other_user
assert other_user.is_teacher if is_teacher else other_user.is_student
assert other_user.teacher if is_teacher else other_user.student
return other_user

def get_other_school_user(
Expand Down Expand Up @@ -844,12 +844,12 @@ def get_another_school_user(

# Cannot assert that 2 teachers are in the same class since a class
# can only have 1 teacher.
if not (user.is_teacher and other_user.is_teacher):
if not (user.teacher and other_user.teacher):
# At this point, same_class needs to be set.
assert same_class is not None, "same_class must be set."

# If one of the users is a teacher.
if user.is_teacher or is_teacher:
if user.teacher or is_teacher:
# Get the teacher.
teacher = other_user if is_teacher else user

Expand Down
8 changes: 0 additions & 8 deletions codeforlife/user/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,6 @@ def teacher(self) -> t.Optional[Teacher]:
except Teacher.DoesNotExist:
return None

@property
def is_student(self):
return self.student is not None

@property
def is_teacher(self):
return self.teacher is not None

@property
def otp_secret(self):
return self.userprofile.otp_secret
Expand Down
18 changes: 9 additions & 9 deletions codeforlife/user/tests/auth/password_validators/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
Base test case for all password validators.
"""

from unittest.case import _AssertRaisesContext

from django.core.exceptions import ValidationError
from django.test import TestCase

Expand All @@ -22,20 +24,18 @@ def assert_raises_validation_error(self, code: str, *args, **kwargs):
The assert-raises context which will auto-assert the code.
"""

context = self.assertRaises(ValidationError, *args, **kwargs)

class ContextWrapper:
class Wrapper:
"""Wrap context to assert code on exit."""

def __init__(self, context):
self.context = context
def __init__(self, ctx: "_AssertRaisesContext[ValidationError]"):
self.ctx = ctx

def __enter__(self, *args, **kwargs):
return self.context.__enter__(*args, **kwargs)
return self.ctx.__enter__(*args, **kwargs)

def __exit__(self, *args, **kwargs):
value = self.context.__exit__(*args, **kwargs)
assert self.context.exception.code == code
value = self.ctx.__exit__(*args, **kwargs)
assert self.ctx.exception.code == code
return value

return ContextWrapper(context)
return Wrapper(self.assertRaises(ValidationError, *args, **kwargs))
21 changes: 21 additions & 0 deletions codeforlife/user/tests/views/test_klass.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from ....tests import ModelViewSetTestCase
from ...models import Class
from ...permissions import InSchool, IsTeacher
from ...views import ClassViewSet


Expand Down Expand Up @@ -59,3 +60,23 @@ def test_retrieve__student__same_school__in_class(self):

# TODO: other retrieve and list tests
# TODO: replace above tests with get_queryset() tests

def test_get_permissions__list(self):
"""
Only school-teachers can list classes.
"""

self.assert_get_permissions(
permissions=[IsTeacher(), InSchool()],
action="list",
)

def test_get_permissions__retrieve(self):
"""
Anyone in a school can retrieve a class.
"""

self.assert_get_permissions(
permissions=[InSchool()],
action="retrieve",
)
28 changes: 16 additions & 12 deletions codeforlife/user/tests/views/test_school.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@

from rest_framework import status

from ....permissions import AllowNone
from ....tests import ModelViewSetTestCase
from ...models import Class, School, Student, Teacher, User, UserProfile
from ...permissions import InSchool
from ...views import SchoolViewSet


Expand Down Expand Up @@ -176,22 +178,24 @@ def test_list__indy_student(self):

self.client.list([], status.HTTP_403_FORBIDDEN)

def test_list__teacher(self):
# TODO: replace above tests with get_queryset() tests

def test_get_permissions__list(self):
"""
Teacher can list only the school they are in.
No one is allowed to list schools.
"""

user = self._login_teacher()

self.client.list([user.teacher.school])
self.assert_get_permissions(
permissions=[AllowNone()],
action="list",
)

def test_list__student(self):
def test_get_permissions__retrieve(self):
"""
Student can list only the school they are in.
Only a user in a school can retrieve a school.
"""

user = self._login_student()

self.client.list([user.student.class_field.teacher.school])

# TODO: replace above tests with get_queryset() tests
self.assert_get_permissions(
permissions=[InSchool()],
action="retrieve",
)
21 changes: 13 additions & 8 deletions codeforlife/user/views/klass.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@
Created on 24/01/2024 at 13:47:53(+00:00).
"""

import typing as t

from ...views import ModelViewSet
from ..models import Class, User
from ..permissions import InSchool
from ..models import Class
from ..permissions import InSchool, IsTeacher
from ..serializers import ClassSerializer


Expand All @@ -16,15 +14,22 @@ class ClassViewSet(ModelViewSet[Class]):
http_method_names = ["get"]
lookup_field = "access_code"
serializer_class = ClassSerializer
permission_classes = [InSchool]

def get_permissions(self):
# Only school-teachers can list classes.
if self.action == "list":
return [IsTeacher(), InSchool()]

return [InSchool()]

# pylint: disable-next=missing-function-docstring
def get_queryset(self):
user = t.cast(User, self.request.user)
if user.is_student:
user = self.request_user
if user.student:
return Class.objects.filter(students=user.student)

user = self.request_school_teacher_user
if user.teacher.is_admin:
# TODO: add school field to class object
return Class.objects.filter(teacher__school=user.teacher.school)

return Class.objects.filter(teacher=user.teacher)
Loading

0 comments on commit e6a3a55

Please sign in to comment.