From 161666d6b1b1288a5acf0871604899431e19e1c5 Mon Sep 17 00:00:00 2001 From: Stefan Kairinos Date: Wed, 17 Jan 2024 13:02:16 +0000 Subject: [PATCH] fix: view permissions unit tests (#57) * use new base class * quick save * in_school unit tests * tidy up * tidy up * add isort * ignore migrations * ignore all migrations * use job * move load plugins to pyproject.toml * feedback --- .github/workflows/main.yml | 10 +- .vscode/settings.json | 14 +- Pipfile | 1 + Pipfile.lock | 4 +- codeforlife/models/__init__.py | 12 +- codeforlife/permissions/__init__.py | 6 + codeforlife/tests/__init__.py | 1 + codeforlife/tests/permission.py | 99 ++++++++ codeforlife/types.py | 15 ++ codeforlife/user/fixtures/schools.json | 10 + codeforlife/user/fixtures/teachers.json | 8 + codeforlife/user/fixtures/users.json | 13 + codeforlife/user/models/klass.py | 2 +- .../user/tests/permissions/test_in_class.py | 197 ++++++++++++-- .../user/tests/permissions/test_in_school.py | 216 +++++++++++++++- .../tests/permissions/test_is_independent.py | 52 +++- .../user/tests/permissions/test_is_student.py | 92 +++++-- .../user/tests/permissions/test_is_teacher.py | 240 ++++++++++++------ pyproject.toml | 11 +- 19 files changed, 848 insertions(+), 155 deletions(-) create mode 100644 codeforlife/tests/permission.py create mode 100644 codeforlife/types.py diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2fef0c79..164d1ad9 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -15,15 +15,7 @@ env: jobs: test: - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - python-version: [3.8] - steps: - - uses: ocadotechnology/codeforlife-workspace/.github/actions/python/test@main - with: - python-version: ${{ matrix.python-version }} + uses: ocadotechnology/codeforlife-workspace/.github/workflows/test-python-code.yaml@main docs: runs-on: ubuntu-latest diff --git a/.vscode/settings.json b/.vscode/settings.json index a2abc386..73dc8602 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,4 +1,12 @@ { + "isort.path": [ + ".venv/bin/python", + "-m", + "isort" + ], + "isort.args": [ + "--settings-file=pyproject.toml" + ], "black-formatter.path": [ ".venv/bin/python", "-m", @@ -13,14 +21,16 @@ "-m", "mypy" ], + "mypy-type-checker.args": [ + "--config-file=pyproject.toml" + ], "pylint.path": [ ".venv/bin/python", "-m", "pylint" ], "pylint.args": [ - "--rcfile=pyproject.toml", - "--load-plugins=pylint_django" + "--rcfile=pyproject.toml" ], "python.testing.pytestArgs": [ "-n=auto", diff --git a/Pipfile b/Pipfile index d23a18f3..37fcaaee 100644 --- a/Pipfile +++ b/Pipfile @@ -30,6 +30,7 @@ pylint = "==3.0.2" pylint-django = "==2.5.5" pygraphviz = "==1.11" pytest-xdist = {version = "==3.5.0", extras = ["psutil"]} +isort = "==5.13.2" [requires] python_version = "3.8" diff --git a/Pipfile.lock b/Pipfile.lock index f9991054..d1899002 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "1107cd57ebbf132d4c737b2ed44b22a6d2feee479c4f6680f2dc41329c4ba84a" + "sha256": "3fc7c08f1cbe48ba6c060ba559027978806b47aebb38c03b505008bd69c04922" }, "pipfile-spec": 6, "requires": { @@ -694,7 +694,7 @@ "sha256:48fdfcb9face5d58a4f6dde2e72a1fb8dcaf8ab26f95ab49fab84c2ddefb0109", "sha256:8ca5e72a8d85860d5a3fa69b8745237f2939afe12dbf656afbcb47fe72d947a6" ], - "markers": "python_full_version >= '3.8.0'", + "index": "pypi", "version": "==5.13.2" }, "mccabe": { diff --git a/codeforlife/models/__init__.py b/codeforlife/models/__init__.py index a20c73ae..3690bfd7 100644 --- a/codeforlife/models/__init__.py +++ b/codeforlife/models/__init__.py @@ -35,7 +35,7 @@ class Meta(TypedModelMeta): class WarehouseModel(Model): """To be inherited by all models whose data is to be warehoused.""" - class QuerySet(models.QuerySet): + class QuerySet(models.QuerySet[AnyModel], t.Generic[AnyModel]): """Custom queryset to support CFL's system's operations.""" model: "WarehouseModel" # type: ignore[assignment] @@ -81,7 +81,7 @@ def get_queryset(self): A warehouse query set. """ - return WarehouseModel.QuerySet( + return WarehouseModel.QuerySet[AnyModel]( model=self.model, using=self._db, hints=self._hints, # type: ignore[attr-defined] @@ -91,7 +91,7 @@ def filter(self, *args, **kwargs): """A stub that returns our custom queryset.""" return t.cast( - WarehouseModel.QuerySet, + WarehouseModel.QuerySet[AnyModel], super().filter(*args, **kwargs), ) @@ -99,7 +99,7 @@ def exclude(self, *args, **kwargs): """A stub that returns our custom queryset.""" return t.cast( - WarehouseModel.QuerySet, + WarehouseModel.QuerySet[AnyModel], super().exclude(*args, **kwargs), ) @@ -107,7 +107,7 @@ def all(self): """A stub that returns our custom queryset.""" return t.cast( - WarehouseModel.QuerySet, + WarehouseModel.QuerySet[AnyModel], super().all(), ) @@ -115,7 +115,7 @@ def none(self): """A stub that returns our custom queryset.""" return t.cast( - WarehouseModel.QuerySet, + WarehouseModel.QuerySet[AnyModel], super().none(), ) 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..c86327a8 --- /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 _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._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._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/fixtures/schools.json b/codeforlife/user/fixtures/schools.json index 7d75eef3..2a39004c 100644 --- a/codeforlife/user/fixtures/schools.json +++ b/codeforlife/user/fixtures/schools.json @@ -8,5 +8,15 @@ "country": "GB", "uk_county": "Surrey" } + }, + { + "model": "user.School", + "pk": 2, + "fields": { + "last_saved_at": "2023-01-01 00:00:00.0+00:00", + "name": "Another Example School", + "country": "GB", + "uk_county": "Surrey" + } } ] \ No newline at end of file diff --git a/codeforlife/user/fixtures/teachers.json b/codeforlife/user/fixtures/teachers.json index 3d7cd7a2..d2456e5d 100644 --- a/codeforlife/user/fixtures/teachers.json +++ b/codeforlife/user/fixtures/teachers.json @@ -16,5 +16,13 @@ "is_admin": false, "school": 1 } + }, + { + "model": "user.Teacher", + "pk": 3, + "fields": { + "last_saved_at": "2023-01-01 00:00:00.0+00:00", + "is_admin": false + } } ] \ No newline at end of file diff --git a/codeforlife/user/fixtures/users.json b/codeforlife/user/fixtures/users.json index 9a623c60..a8ef0ff3 100644 --- a/codeforlife/user/fixtures/users.json +++ b/codeforlife/user/fixtures/users.json @@ -58,5 +58,18 @@ "email": "indiana.jones@codeforlife.com", "password": "pbkdf2_sha256$260000$a2nFLqpwD88sOeZ7wsQskW$WIJACyDluEJWKMPsO/jawrR0sHXHmYebDJoyUihKJxU=" } + }, + { + "model": "user.User", + "pk": 6, + "fields": { + "last_saved_at": "2023-01-01 00:00:00.0+00:00", + "is_active": true, + "first_name": "Albert", + "last_name": "Einstein", + "email": "albert.einstein@codeforlife.com", + "password": "pbkdf2_sha256$260000$a2nFLqpwD88sOeZ7wsQskW$WIJACyDluEJWKMPsO/jawrR0sHXHmYebDJoyUihKJxU=", + "teacher": 3 + } } ] \ No newline at end of file diff --git a/codeforlife/user/models/klass.py b/codeforlife/user/models/klass.py index c640bbbe..e20d7ab9 100644 --- a/codeforlife/user/models/klass.py +++ b/codeforlife/user/models/klass.py @@ -31,7 +31,7 @@ class Manager(WarehouseModel.Manager["Class"]): pk: str # type: ignore[assignment] students: QuerySet["_student.Student"] - id = models.CharField( # type: ignore[assignment] + id: str = models.CharField( # type: ignore[assignment] _("identifier"), primary_key=True, editable=False, diff --git a/codeforlife/user/tests/permissions/test_in_class.py b/codeforlife/user/tests/permissions/test_in_class.py index baf0e14c..e73d9f7e 100644 --- a/codeforlife/user/tests/permissions/test_in_class.py +++ b/codeforlife/user/tests/permissions/test_in_class.py @@ -3,19 +3,65 @@ Created on 14/12/2023 at 14:26:20(+00:00). """ -from rest_framework.views import APIView +from django.contrib.auth.models import AnonymousUser -from ....tests import APITestCase +from ....tests import PermissionTestCase +from ...models import Class, User from ...permissions import InClass -class TestInClass(APITestCase): +# pylint: disable-next=missing-class-docstring,too-many-instance-attributes +class TestInClass(PermissionTestCase[InClass]): + fixtures = [ + "users", + "teachers", + "schools", + "classes", + "students", + ] + + def setUp(self): + super().setUp() + + self.class_teacher_user = User.objects.get(pk=1) + assert ( + self.class_teacher_user.teacher + and self.class_teacher_user.teacher.classes.exists() + ) + self.class_teacher__classes = self.class_teacher_user.teacher.classes + self.class_teacher_request = self.request_factory.get("/") + self.class_teacher_request.user = self.class_teacher_user + + self.student_user = User.objects.get(pk=3) + assert self.student_user.student + self.student__class = self.student_user.student.klass + self.student_request = self.request_factory.get("/") + self.student_request.user = self.student_user + + self.indy_user = User.objects.get(pk=5) + assert not self.indy_user.teacher and not self.indy_user.student + self.indy_request = self.request_factory.get("/") + self.indy_request.user = self.indy_user + + self.non_class_teacher_user = User.objects.get(pk=6) + assert ( + self.non_class_teacher_user.teacher + and not self.non_class_teacher_user.teacher.classes.exists() + ) + self.non_class_teacher_request = self.request_factory.get("/") + self.non_class_teacher_request.user = self.non_class_teacher_user + + self.anon_request = self.request_factory.get("/") + self.anon_request.user = AnonymousUser() + + # pylint: disable-next=pointless-string-statement """ Naming convention: test_{class_id}__{user_type} class_id: The id of a class. Options: - - any_class: Any class. + - in_any_class: The user is in any class. + - not_in_any_class: The user is not in any class. - in_class: A specific class the user is in. - not_in_class: A specific class the user is not in. @@ -24,74 +70,169 @@ class TestInClass(APITestCase): - class_teacher: A teacher in a class. - student: A student. - indy: An independent. + - anon: An anonymous user. """ - def test_any_class__non_class_teacher(self): + def test_in_any_class__class_teacher(self): """ - Teacher without a class is not in any class. + Teacher with a class is in any class. """ - raise NotImplementedError() # TODO + self.assert_has_permission( + self.class_teacher_request, + init_kwargs={ + "class_id": None, + }, + ) - def test_any_class__class_teacher(self): + def test_in_any_class__student(self): """ - Teacher with a class is in any class. + Student is in any class. """ - raise NotImplementedError() # TODO + self.assert_has_permission( + self.student_request, + init_kwargs={ + "class_id": None, + }, + ) - def test_any_class__student(self): + def test_not_in_any_class__non_class_teacher(self): """ - Student is in any class. + Teacher without a class is not in any class. """ - raise NotImplementedError() # TODO + self.assert_not_has_permission( + self.non_class_teacher_request, + init_kwargs={ + "class_id": None, + }, + ) - def test_any_class__indy(self): + def test_not_in_any_class__indy(self): """ Independent is not in any class. """ - raise NotImplementedError() # TODO + self.assert_not_has_permission( + self.indy_request, + init_kwargs={ + "class_id": None, + }, + ) + + def test_not_in_any_class__anon(self): + """ + Anonymous user is not in any class. + """ + + self.assert_not_has_permission( + self.anon_request, + init_kwargs={ + "class_id": None, + }, + ) + + def test_in_class__class_teacher(self): + """ + Teacher with a class is in a specific class. + """ + + klass = self.class_teacher__classes.first() + assert klass is not None + + self.assert_has_permission( + self.class_teacher_request, + init_kwargs={ + "class_id": klass.id, + }, + ) + + def test_in_class__student(self): + """ + Student is in a specific class. + """ + + self.assert_has_permission( + self.student_request, + init_kwargs={ + "class_id": self.student__class.id, + }, + ) def test_not_in_class__non_class_teacher(self): """ Teacher without a class is not in a specific class. """ - raise NotImplementedError() # TODO + klass = Class.objects.first() + assert klass is not None + + self.assert_not_has_permission( + self.non_class_teacher_request, + init_kwargs={ + "class_id": klass.id, + }, + ) def test_not_in_class__class_teacher(self): """ Teacher with a class is not in a specific class. """ - raise NotImplementedError() # TODO + klass = Class.objects.difference( + self.class_teacher__classes.all() + ).first() + assert klass is not None + + self.assert_not_has_permission( + self.class_teacher_request, + init_kwargs={ + "class_id": klass.id, + }, + ) def test_not_in_class__student(self): """ Student is not in a specific class. """ - raise NotImplementedError() # TODO + klass = Class.objects.exclude(id=self.student__class.id).first() + assert klass is not None + + self.assert_not_has_permission( + self.student_request, + init_kwargs={ + "class_id": klass.id, + }, + ) def test_not_in_class__indy(self): """ Independent is not in a specific class. """ - raise NotImplementedError() # TODO + klass = Class.objects.first() + assert klass is not None - def test_in_class__class_teacher(self): - """ - Teacher with a class is in a specific class. - """ + self.assert_not_has_permission( + self.indy_request, + init_kwargs={ + "class_id": klass.id, + }, + ) - raise NotImplementedError() # TODO - - def test_in_class__student(self): + def test_not_in_class__anon(self): """ - Student is in a specific class. + Anonymous user is not in a specific class. """ - raise NotImplementedError() # TODO + klass = Class.objects.first() + assert klass is not None + + self.assert_not_has_permission( + self.anon_request, + init_kwargs={ + "class_id": klass.id, + }, + ) diff --git a/codeforlife/user/tests/permissions/test_in_school.py b/codeforlife/user/tests/permissions/test_in_school.py index 4ef30fe7..461d3de5 100644 --- a/codeforlife/user/tests/permissions/test_in_school.py +++ b/codeforlife/user/tests/permissions/test_in_school.py @@ -3,19 +3,65 @@ Created on 14/12/2023 at 14:26:20(+00:00). """ -from rest_framework.views import APIView +from django.contrib.auth.models import AnonymousUser -from ....tests import APITestCase +from ....tests import PermissionTestCase +from ...models import School, User from ...permissions import InSchool -class TestInSchool(APITestCase): +# pylint: disable-next=missing-class-docstring,too-many-instance-attributes +class TestInSchool(PermissionTestCase[InSchool]): + fixtures = [ + "users", + "teachers", + "schools", + "classes", + "students", + ] + + def setUp(self): + super().setUp() + + self.school_teacher_user = User.objects.get(pk=1) + assert ( + self.school_teacher_user.teacher + and self.school_teacher_user.teacher.school + ) + self.school_teacher__school = self.school_teacher_user.teacher.school + self.school_teacher_request = self.request_factory.get("/") + self.school_teacher_request.user = self.school_teacher_user + + self.student_user = User.objects.get(pk=3) + assert self.student_user.student + self.student__school = self.student_user.student.school + self.student_request = self.request_factory.get("/") + self.student_request.user = self.student_user + + self.indy_user = User.objects.get(pk=5) + assert not self.indy_user.teacher and not self.indy_user.student + self.indy_request = self.request_factory.get("/") + self.indy_request.user = self.indy_user + + self.non_school_teacher_user = User.objects.get(pk=6) + assert ( + self.non_school_teacher_user.teacher + and not self.non_school_teacher_user.teacher.school + ) + self.non_school_teacher_request = self.request_factory.get("/") + self.non_school_teacher_request.user = self.non_school_teacher_user + + self.anon_request = self.request_factory.get("/") + self.anon_request.user = AnonymousUser() + + # pylint: disable-next=pointless-string-statement """ Naming convention: test_{school_id}__{user_type} school_id: The id of a school. Options: - - any_school: Any school. + - in_any_school: The user is in any school. + - not_in_any_school: The user is not in any school. - in_school: A specific school the user is in. - not_in_school: A specific school the user is not in. @@ -24,6 +70,166 @@ class TestInSchool(APITestCase): - school_teacher: A teacher in a school. - student: A student. - indy: An independent. + - anon: An anonymous user. """ - # TODO: define unit tests + def test_in_any_school__school_teacher(self): + """ + Teacher with a school is in any school. + """ + + self.assert_has_permission( + self.school_teacher_request, + init_kwargs={ + "school_id": None, + }, + ) + + def test_in_any_school__student(self): + """ + Student is in any school. + """ + + self.assert_has_permission( + self.student_request, + init_kwargs={ + "school_id": None, + }, + ) + + def test_not_in_any_school__non_school_teacher(self): + """ + Teacher without a school is not in any school. + """ + + self.assert_not_has_permission( + self.non_school_teacher_request, + init_kwargs={ + "school_id": None, + }, + ) + + def test_not_in_any_school__indy(self): + """ + Independent is not in any school. + """ + + self.assert_not_has_permission( + self.indy_request, + init_kwargs={ + "school_id": None, + }, + ) + + def test_not_in_any_school__anon(self): + """ + Anonymous user is not in any school. + """ + + self.assert_not_has_permission( + self.anon_request, + init_kwargs={ + "school_id": None, + }, + ) + + def test_in_school__school_teacher(self): + """ + Teacher with a school is in a specific school. + """ + + self.assert_has_permission( + self.school_teacher_request, + init_kwargs={ + "school_id": self.school_teacher__school.id, + }, + ) + + def test_in_school__student(self): + """ + Student is in a specific school. + """ + + self.assert_has_permission( + self.student_request, + init_kwargs={ + "school_id": self.student__school.id, + }, + ) + + def test_not_in_school__non_school_teacher(self): + """ + Teacher without a school is not in a specific school. + """ + + school = School.objects.first() + assert school is not None + + self.assert_not_has_permission( + self.non_school_teacher_request, + init_kwargs={ + "school_id": school.id, + }, + ) + + def test_not_in_school__school_teacher(self): + """ + Teacher with a school is not in a specific school. + """ + + school = School.objects.exclude( + id=self.school_teacher__school.id + ).first() + assert school is not None + + self.assert_not_has_permission( + self.school_teacher_request, + init_kwargs={ + "school_id": school.id, + }, + ) + + def test_not_in_school__student(self): + """ + Student is not in a specific school. + """ + + school = School.objects.exclude(id=self.student__school.id).first() + assert school is not None + + self.assert_not_has_permission( + self.student_request, + init_kwargs={ + "school_id": school.id, + }, + ) + + def test_not_in_school__indy(self): + """ + Independent is not in a specific school. + """ + + school = School.objects.first() + assert school is not None + + self.assert_not_has_permission( + self.indy_request, + init_kwargs={ + "school_id": school.id, + }, + ) + + def test_not_in_school__anon(self): + """ + Anonymous user is not in a specific school. + """ + + school = School.objects.first() + assert school is not None + + self.assert_not_has_permission( + self.anon_request, + init_kwargs={ + "school_id": school.id, + }, + ) diff --git a/codeforlife/user/tests/permissions/test_is_independent.py b/codeforlife/user/tests/permissions/test_is_independent.py index 6270fafe..1c5b6c72 100644 --- a/codeforlife/user/tests/permissions/test_is_independent.py +++ b/codeforlife/user/tests/permissions/test_is_independent.py @@ -3,13 +3,24 @@ Created on 14/12/2023 at 14:26:20(+00:00). """ -from rest_framework.views import APIView +from django.contrib.auth.models import AnonymousUser -from ....tests import APITestCase +from ....tests import PermissionTestCase +from ...models import User from ...permissions import IsIndependent -class TestIsIndependent(APITestCase): +# pylint: disable-next=missing-class-docstring +class TestIsIndependent(PermissionTestCase[IsIndependent]): + fixtures = [ + "users", + "teachers", + "schools", + "classes", + "students", + ] + + # pylint: disable-next=pointless-string-statement """ Naming convention: test_{user_type} @@ -18,6 +29,7 @@ class TestIsIndependent(APITestCase): - teacher: A teacher. - student: A student. - indy: An independent. + - anon: An anonymous user. """ def test_teacher(self): @@ -25,18 +37,46 @@ def test_teacher(self): Teacher is not any independent. """ - raise NotImplementedError() # TODO + user = User.objects.get(pk=1) + assert user.teacher + + request = self.request_factory.get("/") + request.user = user + + self.assert_not_has_permission(request) def test_student(self): """ Student is not any independent. """ - raise NotImplementedError() # TODO + user = User.objects.get(pk=3) + assert user.student + + request = self.request_factory.get("/") + request.user = user + + self.assert_not_has_permission(request) def test_indy(self): """ Independent is any independent. """ - raise NotImplementedError() # TODO + user = User.objects.get(pk=5) + assert not user.teacher and not user.student + + request = self.request_factory.get("/") + request.user = user + + self.assert_has_permission(request) + + def test_anon(self): + """ + Anonymous user is not any independent. + """ + + request = self.request_factory.get("/") + request.user = AnonymousUser() + + self.assert_not_has_permission(request) diff --git a/codeforlife/user/tests/permissions/test_is_student.py b/codeforlife/user/tests/permissions/test_is_student.py index b2aa4583..4789bcf8 100644 --- a/codeforlife/user/tests/permissions/test_is_student.py +++ b/codeforlife/user/tests/permissions/test_is_student.py @@ -3,13 +3,44 @@ Created on 14/12/2023 at 14:26:20(+00:00). """ -from rest_framework.views import APIView +from django.contrib.auth.models import AnonymousUser -from ....tests import APITestCase +from ....tests import PermissionTestCase +from ...models import Student, User from ...permissions import IsStudent -class TestIsStudent(APITestCase): +# pylint: disable-next=missing-class-docstring +class TestIsStudent(PermissionTestCase[IsStudent]): + fixtures = [ + "users", + "teachers", + "schools", + "classes", + "students", + ] + + def setUp(self): + super().setUp() + + self.teacher_user = User.objects.get(pk=1) + assert self.teacher_user.teacher + self.teacher = self.teacher_user.teacher + self.teacher_request = self.request_factory.get("/") + self.teacher_request.user = self.teacher_user + + self.student_user = User.objects.get(pk=3) + assert self.student_user.student + self.student = self.student_user.student + self.student_request = self.request_factory.get("/") + self.student_request.user = self.student_user + + self.indy_user = User.objects.get(pk=5) + assert not self.indy_user.student and not self.indy_user.teacher + self.indy_request = self.request_factory.get("/") + self.indy_request.user = self.indy_user + + # pylint: disable-next=pointless-string-statement """ Naming convention: test_{user_type}__{student_id} @@ -18,6 +49,7 @@ class TestIsStudent(APITestCase): - teacher: A teacher. - student: A student. - indy: An independent. + - anon: An anonymous user. student_id: The ID of a student. Options: - any_student: User is any student. @@ -31,46 +63,70 @@ def test_teacher__not_any_student(self): Teacher is not any student. """ - raise NotImplementedError() # TODO - - def test_teacher__not_specific_student(self): - """ - Teacher is not a specific student. - """ - - raise NotImplementedError() # TODO + self.assert_not_has_permission( + self.teacher_request, + init_kwargs={ + "student_id": None, + }, + ) def test_indy__not_any_student(self): """ Independent is not any student. """ - raise NotImplementedError() # TODO + self.assert_not_has_permission( + self.indy_request, + init_kwargs={ + "student_id": None, + }, + ) - def test_indy__not_specific_student(self): + def test_anon__not_any_student(self): """ - Independent is not a specific student. + Anonymous user is not any student. """ - raise NotImplementedError() # TODO + request = self.request_factory.get("/") + request.user = AnonymousUser() + + self.assert_not_has_permission(request) def test_student__any_student(self): """ Student is any student. """ - raise NotImplementedError() # TODO + self.assert_has_permission( + self.student_request, + init_kwargs={ + "student_id": None, + }, + ) def test_student__specific_student(self): """ Student is a specific student. """ - raise NotImplementedError() # TODO + self.assert_has_permission( + self.student_request, + init_kwargs={ + "student_id": self.student.id, + }, + ) def test_student__not_specific_student(self): """ Student is not a specific student. """ - raise NotImplementedError() # TODO + student = Student.objects.exclude(id=self.student.id).first() + assert student is not None + + self.assert_not_has_permission( + self.student_request, + init_kwargs={ + "student_id": student.id, + }, + ) diff --git a/codeforlife/user/tests/permissions/test_is_teacher.py b/codeforlife/user/tests/permissions/test_is_teacher.py index feec8a4c..99665cbb 100644 --- a/codeforlife/user/tests/permissions/test_is_teacher.py +++ b/codeforlife/user/tests/permissions/test_is_teacher.py @@ -3,16 +3,45 @@ Created on 14/12/2023 at 14:26:20(+00:00). """ -from rest_framework.test import APIRequestFactory -from rest_framework.views import APIView +from django.contrib.auth.models import AnonymousUser -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. +# pylint: disable-next=missing-class-docstring +class TestIsTeacher(PermissionTestCase[IsTeacher]): + fixtures = [ + "users", + "teachers", + "schools", + "classes", + "students", + ] + + def setUp(self): + super().setUp() + + self.admin_teacher_user = User.objects.get(pk=1) + assert ( + self.admin_teacher_user.teacher + and self.admin_teacher_user.teacher.is_admin + ) + self.admin_teacher = self.admin_teacher_user.teacher + self.admin_teacher_request = self.request_factory.get("/") + self.admin_teacher_request.user = self.admin_teacher_user + + self.non_admin_teacher_user = User.objects.get(pk=2) + assert ( + self.non_admin_teacher_user.teacher + and not self.non_admin_teacher_user.teacher.is_admin + ) + self.non_admin_teacher = self.non_admin_teacher_user.teacher + self.non_admin_teacher_request = self.request_factory.get("/") + self.non_admin_teacher_request.user = self.non_admin_teacher_user + + # pylint: disable-next=pointless-string-statement """ Naming convention: test_{teacher_id}__{is_admin} @@ -27,111 +56,168 @@ class TestIsTeacher(APITestCase): - none: Teacher is an admin or a non-admin. """ - fixtures = [ - "users", - "teachers", - "schools", - "classes", - "students", - ] - - def setUp(self): - self.user__1 = User.objects.get(pk=1) - self.user__2 = User.objects.get(pk=2) - - assert self.user__1.teacher and self.user__1.teacher.is_admin - assert self.user__2.teacher and not self.user__2.teacher.is_admin - - 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.user = self.user__1 - self.request__2 = request_factory.get("/") - self.request__2.user = self.user__2 - 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.admin_teacher_request, + 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.admin_teacher_request, + init_kwargs={ + "teacher_id": None, + "is_admin": True, + }, + ) + + self.assert_not_has_permission( + self.non_admin_teacher_request, + 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.admin_teacher_request, + init_kwargs={ + "teacher_id": None, + "is_admin": False, + }, + ) + + self.assert_has_permission( + self.non_admin_teacher_request, + 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.admin_teacher_request, + init_kwargs={ + "teacher_id": self.admin_teacher.id, + "is_admin": None, + }, + ) + + self.assert_not_has_permission( + self.admin_teacher_request, + init_kwargs={ + "teacher_id": self.non_admin_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.admin_teacher_request, + init_kwargs={ + "teacher_id": self.admin_teacher.id, + "is_admin": True, + }, + ) + + self.assert_not_has_permission( + self.non_admin_teacher_request, + init_kwargs={ + "teacher_id": self.non_admin_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.admin_teacher_request, + init_kwargs={ + "teacher_id": self.admin_teacher.id, + "is_admin": False, + }, + ) + + self.assert_has_permission( + self.non_admin_teacher_request, + init_kwargs={ + "teacher_id": self.non_admin_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. + - anon: An anonymous 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 + + self.assert_not_has_permission(request) + + def test_anon(self): + """ + Anonymous user is not a teacher. + """ + + request = self.request_factory.get("/") + request.user = AnonymousUser() - assert IsTeacher( - teacher_id=self.user__2__teacher.id, - is_admin=False, - ).has_permission(self.request__2, APIView()) + self.assert_not_has_permission(request) diff --git a/pyproject.toml b/pyproject.toml index 9a504ad8..0d57c45b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,7 +7,7 @@ upload_to_release = true [tool.black] line-length = 80 -extend-exclude = "^/codeforlife/user/migrations/" +extend-exclude = ".*/migrations/.*py" [tool.pytest.ini_options] env = ["DJANGO_SETTINGS_MODULE=manage"] @@ -24,3 +24,12 @@ init-hook = "import os; os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'manage' [tool.pylint.format] max-line-length = 80 + +[tool.pylint.MASTER] +ignore-paths = [".*/migrations/.*py"] +load-plugins = "pylint_django" + +[tool.isort] +profile = "black" +line_length = 80 +skip_glob = ["**/migrations/*.py"]