fix: various fixes for bulk logic and type hints (#65)

* only include read_only fields

* user serializers correctly for remaining models

* fix: extra_kwargs

* fix: extra_kwargs, user

* fix: extra_kwargs again

* fix: custom base serializer

* use string lib

* add type hints

* add type hint

* custom data assertion

codeforlife/serializers/__init__.py

@@ -3,4 +3,5 @@
 Created on 20/01/2024 at 11:19:12(+00:00).
 """
 
+from .base import *
 from .model import *

codeforlife/serializers/base.py

@@ -0,0 +1,41 @@
+"""
+© Ocado Group
+Created on 29/01/2024 at 14:27:09(+00:00).
+
+Base serializer.
+"""
+
+import typing as t
+
+from django.contrib.auth.models import AnonymousUser
+from rest_framework.serializers import BaseSerializer as _BaseSerializer
+
+from ..request import Request
+from ..user.models import User
+
+
+# pylint: disable-next=abstract-method
+class BaseSerializer(_BaseSerializer):
+    """Base serializer to be inherited by all other serializers."""
+
+    @property
+    def request(self):
+        """The HTTP request that triggered the view."""
+
+        return t.cast(Request, self.context["request"])
+
+    @property
+    def request_user(self):
+        """
+        The user that made the request. Assumes the user has authenticated.
+        """
+
+        return t.cast(User, self.request.user)
+
+    @property
+    def request_anon_user(self):
+        """
+        The user that made the request. Assumes the user has not authenticated.
+        """
+
+        return t.cast(AnonymousUser, self.request.user)

codeforlife/serializers/model.py

@@ -12,10 +12,16 @@
 from rest_framework.serializers import ModelSerializer as _ModelSerializer
 from rest_framework.serializers import ValidationError as _ValidationError
 
+from .base import BaseSerializer
+
 AnyModel = t.TypeVar("AnyModel", bound=Model)
 
 
-class ModelSerializer(_ModelSerializer[AnyModel], t.Generic[AnyModel]):
+class ModelSerializer(
+    BaseSerializer,
+    _ModelSerializer[AnyModel],
+    t.Generic[AnyModel],
+):
     """Base model serializer for all model serializers."""
 
     # pylint: disable-next=useless-parent-delegation
@@ -26,8 +32,12 @@ def update(self, instance, validated_data: t.Dict[str, t.Any]):
     def create(self, validated_data: t.Dict[str, t.Any]):
         return super().create(validated_data)
 
+    def validate(self, attrs: t.Dict[str, t.Any]):
+        return attrs
+
 
 class ModelListSerializer(
+    BaseSerializer,
     t.Generic[AnyModel],
     _ListSerializer[t.List[AnyModel]],
 ):
@@ -48,6 +58,7 @@ class Meta:
             list_serializer_class = UserListSerializer
     """
 
+    instance: t.List[AnyModel]
     batch_size: t.Optional[int] = None
 
     @classmethod
@@ -81,7 +92,11 @@ def create(self, validated_data: t.List[t.Dict[str, t.Any]]):
             batch_size=self.batch_size,
         )
 
-    def update(self, instance, validated_data: t.List[t.Dict[str, t.Any]]):
+    def update(
+        self,
+        instance: t.List[AnyModel],
+        validated_data: t.List[t.Dict[str, t.Any]],
+    ):
         """Bulk update many instances of a model.
 
         https://www.django-rest-framework.org/api-guide/serializers/#customizing-multiple-update
@@ -122,3 +137,7 @@ def validate(self, attrs: t.List[t.Dict[str, t.Any]]):
                 raise _ValidationError("Some models do not exist.")
 
         return attrs
+
+    # pylint: disable-next=useless-parent-delegation,arguments-renamed
+    def to_representation(self, instance: t.List[AnyModel]):
+        return super().to_representation(instance)

codeforlife/tests/model_view_set.py

@@ -101,6 +101,14 @@ def _make_assertions():
 
         self._assert_response(response, _make_assertions)
 
+    def _assert_data_contains_subset(self, request: Data, response: Data):
+        for key, value in request.items():
+            response_value = response[key]
+            if isinstance(value, dict):
+                self._assert_data_contains_subset(value, response_value)
+            else:
+                assert value == response_value
+
     @staticmethod
     def status_code_is_ok(status_code: int):
         """Check if the status code is greater than or equal to 200 and less
@@ -252,17 +260,21 @@ def create(
         self,
         data: Data,
         status_code_assertion: StatusCodeAssertion = status.HTTP_201_CREATED,
+        assert_data_contains_subset: bool = True,
         **kwargs,
     ):
+        # pylint: disable=line-too-long
         """Create a model.
 
         Args:
             data: The values for each field.
             status_code_assertion: The expected status code.
+            assert_data_contains_subset: Assert if the request model is a subset of the response model.
 
         Returns:
             The HTTP response.
         """
+        # pylint: enable=line-too-long
 
         response: Response = self.post(
             self.reverse("list"),
@@ -272,31 +284,35 @@ def create(
             **kwargs,
         )
 
-        self._assert_response_json(
-            response,
-            make_assertions=lambda actual_data: (
-                # pylint: disable-next=no-member
-                self._test_case.assertDictContainsSubset(data, actual_data)
-            ),
-        )
+        if assert_data_contains_subset:
+            self._assert_response_json(
+                response,
+                make_assertions=lambda actual_data: (
+                    self._assert_data_contains_subset(data, actual_data)
+                ),
+            )
 
         return response
 
     def bulk_create(
         self,
         data: t.List[Data],
         status_code_assertion: StatusCodeAssertion = status.HTTP_201_CREATED,
+        assert_data_contains_subset: bool = True,
         **kwargs,
     ):
+        # pylint: disable=line-too-long
         """Bulk create many instances of a model.
 
         Args:
             data: The values for each field, for each model.
             status_code_assertion: The expected status code.
+            assert_data_contains_subset: Assert if the request models are a subset of the response models.
 
         Returns:
             The HTTP response.
         """
+        # pylint: enable=line-too-long
 
         response: Response = self.post(
             self.reverse("bulk"),
@@ -306,12 +322,13 @@ def bulk_create(
             **kwargs,
         )
 
-        def make_assertions(actual_data: t.List[self.Data]):
-            for model, actual_model in zip(data, actual_data):
-                # pylint: disable-next=no-member
-                self._test_case.assertDictContainsSubset(model, actual_model)
+        if assert_data_contains_subset:
 
-        self._assert_response_json_bulk(response, make_assertions, data)
+            def make_assertions(actual_data: t.List[ModelViewSetClient.Data]):
+                for model, actual_model in zip(data, actual_data):
+                    self._assert_data_contains_subset(model, actual_model)
+
+            self._assert_response_json_bulk(response, make_assertions, data)
 
         return response
 
@@ -390,7 +407,7 @@ def list(
             **kwargs,
         )
 
-        def _make_assertions(actual_data: self.Data):
+        def _make_assertions(actual_data: ModelViewSetClient.Data):
             for data, model in zip(actual_data["data"], models):
                 self.assert_data_equals_model(
                     data,
@@ -434,7 +451,7 @@ def partial_update(
             **kwargs,
         )
 
-        def _make_assertions(actual_data: self.Data):
+        def _make_assertions(actual_data: ModelViewSetClient.Data):
             model.refresh_from_db()
             self.assert_data_equals_model(
                 actual_data,
@@ -479,7 +496,7 @@ def bulk_partial_update(
             **kwargs,
         )
 
-        def make_assertions(actual_data: t.List[self.Data]):
+        def make_assertions(actual_data: t.List[ModelViewSetClient.Data]):
             models.sort(key=lambda model: getattr(model, self._lookup_field))
 
             for data, model in zip(actual_data, models):

codeforlife/user/models/otp_bypass_token.py

@@ -1,3 +1,9 @@
+"""
+© Ocado Group
+Created on 29/01/2024 at 16:46:24(+00:00).
+"""
+
+import string
 import typing as t
 from itertools import groupby
 
@@ -12,7 +18,7 @@
 
 class OtpBypassToken(models.Model):
     length = 8
-    allowed_chars = "abcdefghijklmnopqrstuvwxyz"
+    allowed_chars = string.ascii_lowercase
     max_count = 10
     max_count_validation_error = ValidationError(
         f"Exceeded max count of {max_count}"

codeforlife/user/models/user.py

@@ -50,6 +50,8 @@
 
 
 class User(_User):
+    _password: t.Optional[str]
+
     id: int
     auth_factors: QuerySet["auth_factor.AuthFactor"]
     otp_bypass_tokens: QuerySet["otp_bypass_token.OtpBypassToken"]

codeforlife/user/serializers/klass.py

@@ -3,33 +3,50 @@
 Created on 20/01/2024 at 11:28:29(+00:00).
 """
 
+from rest_framework import serializers
+
 from ...serializers import ModelSerializer
 from ..models import Class
 
 
 # pylint: disable-next=missing-class-docstring
 class ClassSerializer(ModelSerializer[Class]):
+    id = serializers.CharField(
+        source="access_code",
+        read_only=True,
+    )
+
+    read_classmates_data = serializers.BooleanField(
+        source="classmates_data_viewable",
+        read_only=True,
+    )
+
+    receive_requests_until = serializers.DateTimeField(
+        source="accept_requests_until",
+        read_only=True,
+    )
+
+    teacher = serializers.IntegerField(
+        source="teacher.id",
+        read_only=True,
+    )
+
+    school = serializers.IntegerField(
+        source="teacher.school.id",
+        read_only=True,
+    )
+
     # pylint: disable-next=missing-class-docstring,too-few-public-methods
     class Meta:
         model = Class
         fields = [
+            "name",
             "id",
             "teacher",
             "school",
-            "name",
             "read_classmates_data",
             "receive_requests_until",
         ]
         extra_kwargs = {
-            "id": {"read_only": True},
-        }
-
-    def to_representation(self, instance):
-        return {
-            "id": instance.access_code,
-            "name": instance.name,
-            "read_classmates_data": instance.classmates_data_viewable,
-            "receive_requests_until": instance.accept_requests_until,
-            "teacher": instance.teacher.pk,
-            "school": instance.teacher.school.pk,
+            "name": {"read_only": True},
         }

codeforlife/user/serializers/school.py

@@ -3,12 +3,16 @@
 Created on 20/01/2024 at 11:28:19(+00:00).
 """
 
+from rest_framework import serializers
+
 from ...serializers import ModelSerializer
 from ..models import School
 
 
 # pylint: disable-next=missing-class-docstring
 class SchoolSerializer(ModelSerializer[School]):
+    uk_county = serializers.CharField(source="county", read_only=True)
+
     # pylint: disable-next=missing-class-docstring,too-few-public-methods
     class Meta:
         model = School
@@ -20,12 +24,6 @@ class Meta:
         ]
         extra_kwargs = {
             "id": {"read_only": True},
-        }
-
-    def to_representation(self, instance):
-        return {
-            "id": instance.id,
-            "name": instance.name,
-            "country": str(instance.country),
-            "uk_county": instance.county,
+            "name": {"read_only": True},
+            "country": {"read_only": True},
         }

codeforlife/user/serializers/student.py

@@ -3,12 +3,24 @@
 Created on 20/01/2024 at 11:27:56(+00:00).
 """
 
+from rest_framework import serializers
+
 from ...serializers import ModelSerializer
 from ..models import Student
 
 
 # pylint: disable-next=missing-class-docstring
 class StudentSerializer(ModelSerializer[Student]):
+    klass = serializers.CharField(
+        source="class_field.access_code",
+        read_only=True,
+    )
+
+    school = serializers.IntegerField(
+        source="class_field.teacher.school.id",
+        read_only=True,
+    )
+
     # pylint: disable-next=missing-class-docstring,too-few-public-methods
     class Meta:
         model = Student
@@ -20,10 +32,3 @@ class Meta:
         extra_kwargs = {
             "id": {"read_only": True},
         }
-
-    def to_representation(self, instance):
-        return {
-            "id": instance.id,
-            "klass": instance.class_field.access_code,
-            "school": instance.class_field.teacher.school.pk,
-        }

codeforlife/user/serializers/teacher.py

@@ -19,4 +19,6 @@ class Meta:
         ]
         extra_kwargs = {
             "id": {"read_only": True},
+            "school": {"read_only": True},
+            "is_admin": {"read_only": True},
         }