From 3370d5820c6df396e0ece18a81001fb3a76c6649 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Mon, 15 Jan 2024 16:40:24 +0000 Subject: [PATCH] use new base class --- codeforlife/permissions/__init__.py | 6 + codeforlife/tests/__init__.py | 1 + codeforlife/tests/permission.py | 99 ++++++++ codeforlife/types.py | 15 ++ .../user/tests/permissions/test_in_class.py | 6 +- .../user/tests/permissions/test_in_school.py | 6 +- .../tests/permissions/test_is_independent.py | 6 +- .../user/tests/permissions/test_is_student.py | 6 +- .../user/tests/permissions/test_is_teacher.py | 211 ++++++++++++------ 9 files changed, 269 insertions(+), 87 deletions(-) create mode 100644 codeforlife/tests/permission.py create mode 100644 codeforlife/types.py diff --git a/codeforlife/permissions/__init__.py b/codeforlife/permissions/__init__.py index 8fb9e67c..389c3a51 100644 --- a/codeforlife/permissions/__init__.py +++ b/codeforlife/permissions/__init__.py @@ -3,5 +3,11 @@ Created on 14/12/2023 at 14:04:57(+00:00). """ +import typing as t + +from rest_framework.permissions import BasePermission + from .is_cron_request_from_google import IsCronRequestFromGoogle from .is_self import IsSelf + +AnyPermission = t.TypeVar("AnyPermission", bound=BasePermission) diff --git a/codeforlife/tests/__init__.py b/codeforlife/tests/__init__.py index daaa7e0d..10797e97 100644 --- a/codeforlife/tests/__init__.py +++ b/codeforlife/tests/__init__.py @@ -8,3 +8,4 @@ from .api import APIClient, APITestCase from .cron import CronTestCase, CronTestClient from .model import ModelTestCase +from .permission import PermissionTestCase diff --git a/codeforlife/tests/permission.py b/codeforlife/tests/permission.py new file mode 100644 index 00000000..4dda0a70 --- /dev/null +++ b/codeforlife/tests/permission.py @@ -0,0 +1,99 @@ +""" +© Ocado Group +Created on 15/01/2024 at 15:15:20(+00:00). + +Test helpers for Django Rest Framework permissions. +""" + +import typing as t + +from django.test import TestCase +from rest_framework.request import Request +from rest_framework.test import APIRequestFactory +from rest_framework.views import APIView + +from ..permissions import AnyPermission +from ..types import Args, KwArgs + + +class PermissionTestCase(TestCase, t.Generic[AnyPermission]): + """Base for all permission test cases.""" + + @classmethod + def get_permission_class(cls) -> t.Type[AnyPermission]: + """Get the permission's class. + + Returns: + The permission's class. + """ + + # pylint: disable-next=no-member + return t.get_args(cls.__orig_bases__[0])[ # type: ignore[attr-defined] + 0 + ] + + def setUp(self): + self.request_factory = APIRequestFactory() + + def _assert_has_permission( + self, + request: Request, + view: t.Optional[APIView], + init_args: t.Optional[Args], + init_kwargs: t.Optional[KwArgs], + ): + view = view or APIView() + init_args = init_args or tuple() + init_kwargs = init_kwargs or {} + + permission_class = self.get_permission_class() + permission = permission_class(*init_args, **init_kwargs) + return permission.has_permission(request, view) + + def assert_has_permission( + self, + request: Request, + view: t.Optional[APIView] = None, + init_args: t.Optional[Args] = None, + init_kwargs: t.Optional[KwArgs] = None, + ): + """Assert that the request does have permission. + + Args: + request: The request being sent to the view. + view: The view that is being requested. + init_args: The arguments used to initialize the permission. + init_kwargs: The keyword arguments used to initialize the + permission. + """ + + assert self._assert_has_permission( + request, + view, + init_args, + init_kwargs, + ) + + def assert_not_has_permission( + self, + request: Request, + view: t.Optional[APIView] = None, + init_args: t.Optional[Args] = None, + init_kwargs: t.Optional[KwArgs] = None, + ): + """Assert that the request does not have permission. + + Args: + request: The request being sent to the view. + view: The view that is being requested. + init_args: The arguments used to initialize the permission. + init_kwargs: The keyword arguments used to initialize the + permission. + """ + + assert not self._assert_has_permission( + request, + view, + init_args, + init_kwargs, + ) diff --git a/codeforlife/types.py b/codeforlife/types.py new file mode 100644 index 00000000..a51526d8 --- /dev/null +++ b/codeforlife/types.py @@ -0,0 +1,15 @@ +""" +© Ocado Group +Created on 15/01/2024 at 15:32:54(+00:00). + +Reusable type hints. +""" + +import typing as t + +Args = t.Tuple[t.Any, ...] +KwArgs = t.Dict[str, t.Any] + +JsonList = t.List["JsonValue"] +JsonDict = t.Dict[str, "JsonValue"] +JsonValue = t.Union[int, str, bool, JsonList, JsonDict] diff --git a/codeforlife/user/tests/permissions/test_in_class.py b/codeforlife/user/tests/permissions/test_in_class.py index baf0e14c..c420e61a 100644 --- a/codeforlife/user/tests/permissions/test_in_class.py +++ b/codeforlife/user/tests/permissions/test_in_class.py @@ -3,13 +3,11 @@ Created on 14/12/2023 at 14:26:20(+00:00). """ -from rest_framework.views import APIView - -from ....tests import APITestCase +from ....tests import PermissionTestCase from ...permissions import InClass -class TestInClass(APITestCase): +class TestInClass(PermissionTestCase[InClass]): """ Naming convention: test_{class_id}__{user_type} diff --git a/codeforlife/user/tests/permissions/test_in_school.py b/codeforlife/user/tests/permissions/test_in_school.py index 4ef30fe7..a98f9f11 100644 --- a/codeforlife/user/tests/permissions/test_in_school.py +++ b/codeforlife/user/tests/permissions/test_in_school.py @@ -3,13 +3,11 @@ Created on 14/12/2023 at 14:26:20(+00:00). """ -from rest_framework.views import APIView - -from ....tests import APITestCase +from ....tests import PermissionTestCase from ...permissions import InSchool -class TestInSchool(APITestCase): +class TestInSchool(PermissionTestCase[InSchool]): """ Naming convention: test_{school_id}__{user_type} diff --git a/codeforlife/user/tests/permissions/test_is_independent.py b/codeforlife/user/tests/permissions/test_is_independent.py index 6270fafe..b278a214 100644 --- a/codeforlife/user/tests/permissions/test_is_independent.py +++ b/codeforlife/user/tests/permissions/test_is_independent.py @@ -3,13 +3,11 @@ Created on 14/12/2023 at 14:26:20(+00:00). """ -from rest_framework.views import APIView - -from ....tests import APITestCase +from ....tests import PermissionTestCase from ...permissions import IsIndependent -class TestIsIndependent(APITestCase): +class TestIsIndependent(PermissionTestCase[IsIndependent]): """ Naming convention: test_{user_type} diff --git a/codeforlife/user/tests/permissions/test_is_student.py b/codeforlife/user/tests/permissions/test_is_student.py index b2aa4583..992f8ed2 100644 --- a/codeforlife/user/tests/permissions/test_is_student.py +++ b/codeforlife/user/tests/permissions/test_is_student.py @@ -3,13 +3,11 @@ Created on 14/12/2023 at 14:26:20(+00:00). """ -from rest_framework.views import APIView - -from ....tests import APITestCase +from ....tests import PermissionTestCase from ...permissions import IsStudent -class TestIsStudent(APITestCase): +class TestIsStudent(PermissionTestCase[IsStudent]): """ Naming convention: test_{user_type}__{student_id} diff --git a/codeforlife/user/tests/permissions/test_is_teacher.py b/codeforlife/user/tests/permissions/test_is_teacher.py index feec8a4c..e5ac4d7d 100644 --- a/codeforlife/user/tests/permissions/test_is_teacher.py +++ b/codeforlife/user/tests/permissions/test_is_teacher.py @@ -3,30 +3,13 @@ Created on 14/12/2023 at 14:26:20(+00:00). """ -from rest_framework.test import APIRequestFactory -from rest_framework.views import APIView - -from ....tests import APITestCase +from ....tests import PermissionTestCase from ...models import User from ...permissions import IsTeacher -class TestIsTeacher(APITestCase): - # TODO: test that students and independents do not get permission. - """ - Naming convention: - test_{teacher_id}__{is_admin} - - teacher_id: The id of a teacher. Options: - - id: A specific teacher. - - none: Any teacher. - - is_admin: A flag for if the teacher is an admin. Options: - - true: Teacher is an admin. - - false: Teacher is a non-admin. - - none: Teacher is an admin or a non-admin. - """ - +# pylint: disable-next=missing-class-docstring +class TestIsTeacher(PermissionTestCase[IsTeacher]): fixtures = [ "users", "teachers", @@ -36,6 +19,8 @@ class TestIsTeacher(APITestCase): ] def setUp(self): + super().setUp() + self.user__1 = User.objects.get(pk=1) self.user__2 = User.objects.get(pk=2) @@ -45,93 +30,177 @@ def setUp(self): self.user__1__teacher = self.user__1.teacher self.user__2__teacher = self.user__2.teacher - request_factory = APIRequestFactory() - self.request__1 = request_factory.get("/") + self.request__1 = self.request_factory.get("/") self.request__1.user = self.user__1 - self.request__2 = request_factory.get("/") + self.request__2 = self.request_factory.get("/") self.request__2.user = self.user__2 + # pylint: disable-next=pointless-string-statement + """ + Naming convention: + test_{teacher_id}__{is_admin} + + teacher_id: The id of a teacher. Options: + - id: A specific teacher. + - none: Any teacher. + + is_admin: A flag for if the teacher is an admin. Options: + - true: Teacher is an admin. + - false: Teacher is a non-admin. + - none: Teacher is an admin or a non-admin. + """ + def test_none__none(self): """ Is any teacher. """ - assert IsTeacher( - teacher_id=None, - is_admin=None, - ).has_permission(self.request__1, APIView()) + self.assert_has_permission( + self.request__1, + init_kwargs={ + "teacher_id": None, + "is_admin": None, + }, + ) def test_none__true(self): """ Is any admin teacher. """ - assert IsTeacher( - teacher_id=None, - is_admin=True, - ).has_permission(self.request__1, APIView()) - - assert not IsTeacher( - teacher_id=None, - is_admin=True, - ).has_permission(self.request__2, APIView()) + self.assert_has_permission( + self.request__1, + init_kwargs={ + "teacher_id": None, + "is_admin": True, + }, + ) + + self.assert_not_has_permission( + self.request__2, + init_kwargs={ + "teacher_id": None, + "is_admin": True, + }, + ) def test_none__false(self): """ Is any non-admin teacher. """ - assert not IsTeacher( - teacher_id=None, - is_admin=False, - ).has_permission(self.request__1, APIView()) - - assert IsTeacher( - teacher_id=None, - is_admin=False, - ).has_permission(self.request__2, APIView()) + self.assert_not_has_permission( + self.request__1, + init_kwargs={ + "teacher_id": None, + "is_admin": False, + }, + ) + + self.assert_has_permission( + self.request__2, + init_kwargs={ + "teacher_id": None, + "is_admin": False, + }, + ) def test_id__none(self): """ Is a specific teacher. """ - assert IsTeacher( - teacher_id=self.user__1__teacher.id, - is_admin=None, - ).has_permission(self.request__1, APIView()) - - assert not IsTeacher( - teacher_id=self.user__2__teacher.id, - is_admin=None, - ).has_permission(self.request__1, APIView()) + self.assert_has_permission( + self.request__1, + init_kwargs={ + "teacher_id": self.user__1__teacher.id, + "is_admin": None, + }, + ) + + self.assert_not_has_permission( + self.request__1, + init_kwargs={ + "teacher_id": self.user__2__teacher.id, + "is_admin": None, + }, + ) def test_id__true(self): """ Is a specific admin teacher. """ - assert IsTeacher( - teacher_id=self.user__1__teacher.id, - is_admin=True, - ).has_permission(self.request__1, APIView()) - - assert not IsTeacher( - teacher_id=self.user__2__teacher.id, - is_admin=True, - ).has_permission(self.request__2, APIView()) + self.assert_has_permission( + self.request__1, + init_kwargs={ + "teacher_id": self.user__1__teacher.id, + "is_admin": True, + }, + ) + + self.assert_not_has_permission( + self.request__2, + init_kwargs={ + "teacher_id": self.user__2__teacher.id, + "is_admin": True, + }, + ) def test_id__false(self): """ Is a specific non-admin teacher. """ - assert not IsTeacher( - teacher_id=self.user__1__teacher.id, - is_admin=False, - ).has_permission(self.request__1, APIView()) + self.assert_not_has_permission( + self.request__1, + init_kwargs={ + "teacher_id": self.user__1__teacher.id, + "is_admin": False, + }, + ) + + self.assert_has_permission( + self.request__2, + init_kwargs={ + "teacher_id": self.user__2__teacher.id, + "is_admin": False, + }, + ) + + # pylint: disable-next=pointless-string-statement + """ + Naming convention: + test_{user_type} + + user_type: The type of user making the request. Options: + - student: A student user. + - indy: An independent user. + """ + + def test_student(self): + """ + Student is not a teacher. + """ + + user = User.objects.get(pk=3) + assert user.student is not None + + request = self.request_factory.get("/") + request.user = user + + self.assert_not_has_permission(request) + + def test_indy(self): + """ + Independent is not a teacher. + """ + + user = User.objects.get(pk=5) + assert user.student is None + assert user.teacher is None + + request = self.request_factory.get("/") + request.user = user - assert IsTeacher( - teacher_id=self.user__2__teacher.id, - is_admin=False, - ).has_permission(self.request__2, APIView()) + self.assert_not_has_permission(request)