From c2b927d112b7b1cf4c55aeeaa4c3895b7e22761f Mon Sep 17 00:00:00 2001 From: SKairinos Date: Tue, 5 Nov 2024 11:54:25 +0000 Subject: [PATCH 01/21] fix: settings --- settings.py | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/settings.py b/settings.py index 36db269..c27923f 100644 --- a/settings.py +++ b/settings.py @@ -16,24 +16,42 @@ import os from pathlib import Path +# Build paths inside the project like this: BASE_DIR / 'subdir'. +BASE_DIR = Path(__file__).resolve().parent + # NOTE: Must come before importing CFL settings. os.environ["SERVICE_NAME"] = "contributor" -# pylint: disable-next=wildcard-import,unused-wildcard-import,wrong-import-position -from codeforlife.settings import * +# GitHub -# Github GH_ORG = "ocadotechnology" GH_REPO = "codeforlife-workspace" GH_FILE = "CONTRIBUTING.md" GH_CLIENT_ID = os.getenv("GH_CLIENT_ID", "Ov23liBErSabQFqROeMg") GH_CLIENT_SECRET = os.getenv("GH_CLIENT_SECRET", "replace-me") +# pylint: disable-next=wildcard-import,unused-wildcard-import,wrong-import-position +from codeforlife.settings import * + +# Installed Apps +# https://docs.djangoproject.com/en/3.2/ref/settings/#installed-apps + +INSTALLED_APPS.remove("codeforlife.user") +INSTALLED_APPS.remove("game") +INSTALLED_APPS.remove("portal") +INSTALLED_APPS.remove("common") + +# Authentication backends +# https://docs.djangoproject.com/en/3.2/ref/settings/#authentication-backends + AUTHENTICATION_BACKENDS = ["api.auth.backends.GitHubBackend"] +# Sessions +# https://docs.djangoproject.com/en/3.2/topics/http/sessions/ + SESSION_ENGINE = "api.models.session" -# Build paths inside the project like this: BASE_DIR / 'subdir'. -BASE_DIR = Path(__file__).resolve().parent +# Static files (CSS, JavaScript, Images) +# https://docs.djangoproject.com/en/3.2/howto/static-files/ STATIC_ROOT = get_static_root(BASE_DIR) From e81503e8d1edbbc5a44a8182c4033c7b2d674f94 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Tue, 5 Nov 2024 12:29:02 +0000 Subject: [PATCH 02/21] fix settings --- Pipfile | 4 +- Pipfile.lock | 6 +-- api/migrations/0001_initial.py | 69 ++++++++++------------------------ api/models/contributor.py | 9 +++-- settings.py | 5 +++ 5 files changed, 35 insertions(+), 58 deletions(-) diff --git a/Pipfile b/Pipfile index 223d550..e3febec 100644 --- a/Pipfile +++ b/Pipfile @@ -22,11 +22,11 @@ name = "pypi" # 5. Run `pipenv install --dev` in your terminal. [packages] -codeforlife = {ref = "v0.20.0", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"} +codeforlife = {ref = "contributor-backend-21", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"} # 🚫 Don't add [packages] below that are inherited from the CFL package. [dev-packages] -codeforlife = {ref = "v0.20.0", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]} +codeforlife = {ref = "contributor-backend-21", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]} # codeforlife = {file = "../codeforlife-package-python", editable = true, extras = ["dev"]} # 🚫 Don't add [dev-packages] below that are inherited from the CFL package. diff --git a/Pipfile.lock b/Pipfile.lock index 5b00517..82c78f1 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "c01201a1d4f0def8095ffa1b24ed278e4b8b1d40d9599ffd1dde77751cf256fa" + "sha256": "f90d8b2e3f7f6dfe78ee5846bbb16f47d92f1fbfa5aa4bef4964873f8b55acad" }, "pipfile-spec": 6, "requires": { @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "ecc5db6b0b45e64488d60eea18915ab6bb224393" + "ref": "c3312e6b2f30db23a3f3221323fd0ef0f46f5470" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "ecc5db6b0b45e64488d60eea18915ab6bb224393" + "ref": "c3312e6b2f30db23a3f3221323fd0ef0f46f5470" }, "codeforlife-portal": { "hashes": [ diff --git a/api/migrations/0001_initial.py b/api/migrations/0001_initial.py index 0a295a2..accff7b 100644 --- a/api/migrations/0001_initial.py +++ b/api/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.25 on 2024-09-10 15:33 +# Generated by Django 3.2.25 on 2024-11-05 12:23 import django.core.validators from django.db import migrations, models @@ -6,6 +6,7 @@ class Migration(migrations.Migration): + initial = True dependencies = [ @@ -15,16 +16,12 @@ class Migration(migrations.Migration): migrations.CreateModel( name='Contributor', fields=[ - ('id', models.IntegerField( - help_text="The contributor's GitHub user-ID.", - primary_key=True, serialize=False)), + ('last_login', models.DateTimeField(blank=True, null=True, verbose_name='last login')), + ('id', models.IntegerField(help_text="The contributor's GitHub user-ID.", primary_key=True, serialize=False)), ('name', models.TextField(verbose_name='name')), - ('location', - models.TextField(null=True, verbose_name='location')), + ('location', models.TextField(null=True, verbose_name='location')), ('html_url', models.TextField(verbose_name='html url')), ('avatar_url', models.TextField(verbose_name='avatar url')), - ('last_login', models.DateTimeField(blank=True, null=True, - verbose_name='last login')), ], options={ 'verbose_name': 'contributor', @@ -34,15 +31,10 @@ class Migration(migrations.Migration): migrations.CreateModel( name='Session', fields=[ - ('session_key', - models.CharField(max_length=40, primary_key=True, - serialize=False, verbose_name='session key')), + ('session_key', models.CharField(max_length=40, primary_key=True, serialize=False, verbose_name='session key')), ('session_data', models.TextField(verbose_name='session data')), - ('expire_date', models.DateTimeField(db_index=True, - verbose_name='expire date')), - ('contributor', models.OneToOneField(blank=True, null=True, - on_delete=django.db.models.deletion.CASCADE, - to='api.contributor')), + ('expire_date', models.DateTimeField(db_index=True, verbose_name='expire date')), + ('contributor', models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='api.contributor')), ], options={ 'verbose_name': 'session', @@ -53,16 +45,11 @@ class Migration(migrations.Migration): migrations.CreateModel( name='ContributorEmail', fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, - serialize=False, verbose_name='ID')), - ('email', models.EmailField(max_length=254, unique=True, - verbose_name='email')), + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('email', models.EmailField(max_length=254, unique=True, verbose_name='email')), ('is_primary', models.BooleanField(verbose_name='is primary')), ('is_public', models.BooleanField(verbose_name='is public')), - ('contributor', - models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, - related_name='emails', - to='api.contributor')), + ('contributor', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='emails', to='api.contributor')), ], options={ 'verbose_name': 'contributor email', @@ -72,18 +59,10 @@ class Migration(migrations.Migration): migrations.CreateModel( name='AgreementSignature', fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, - serialize=False, verbose_name='ID')), - ('agreement_id', models.CharField( - help_text='Commit ID of the contribution agreement in workspace.', - max_length=40, - validators=[django.core.validators.MinLengthValidator(40)], - verbose_name='agreement id')), + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('agreement_id', models.CharField(help_text='Commit ID of the contribution agreement in workspace.', max_length=40, validators=[django.core.validators.MinLengthValidator(40)], verbose_name='agreement id')), ('signed_at', models.DateTimeField(verbose_name='signed at')), - ('contributor', - models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, - related_name='agreement_signatures', - to='api.contributor')), + ('contributor', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='agreement_signatures', to='api.contributor')), ], options={ 'verbose_name': 'agreement signature', @@ -93,18 +72,10 @@ class Migration(migrations.Migration): migrations.CreateModel( name='Repository', fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, - serialize=False, verbose_name='ID')), - ('gh_id', models.IntegerField( - help_text='Github ID of the repo a contributor has contributed to.', - verbose_name='GitHub ID')), - ('points', models.IntegerField(default=0, - help_text='Story points the contributor closed for this repository.', - verbose_name='points')), - ('contributor', - models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, - related_name='repositories', - to='api.contributor')), + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('gh_id', models.IntegerField(help_text='Github ID of the repo a contributor has contributed to.', verbose_name='GitHub ID')), + ('points', models.IntegerField(default=0, help_text='Story points the contributor closed for this repository.', verbose_name='points')), + ('contributor', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='repositories', to='api.contributor')), ], options={ 'verbose_name': 'repository', @@ -114,9 +85,7 @@ class Migration(migrations.Migration): ), migrations.AddConstraint( model_name='contributoremail', - constraint=models.UniqueConstraint( - condition=models.Q(('is_primary', True)), - fields=('contributor',), name='contributor__is_primary'), + constraint=models.UniqueConstraint(condition=models.Q(('is_primary', True)), fields=('contributor',), name='contributor__is_primary'), ), migrations.AlterUniqueTogether( name='agreementsignature', diff --git a/api/models/contributor.py b/api/models/contributor.py index c78051a..f295b78 100644 --- a/api/models/contributor.py +++ b/api/models/contributor.py @@ -7,6 +7,7 @@ import requests from codeforlife.types import JsonDict +from django.contrib.auth.models import AbstractBaseUser from django.db import models from django.db.models import QuerySet from django.utils import timezone @@ -24,15 +25,18 @@ TypedModelMeta = object -class Contributor(models.Model): +class Contributor(AbstractBaseUser): """A contributor that contributes to a repo""" + USERNAME_FIELD = "id" + agreement_signatures: QuerySet["AgreementSignature"] emails: QuerySet["ContributorEmail"] repositories: QuerySet["Repository"] session: "Session" - is_active = True + # Contributors log in with their GitHub account. + password = None # type: ignore[assignment] pk: int id = models.IntegerField( @@ -42,7 +46,6 @@ class Contributor(models.Model): location = models.TextField(_("location"), null=True) html_url = models.TextField(_("html url")) avatar_url = models.TextField(_("avatar url")) - last_login = models.DateTimeField(_("last login"), blank=True, null=True) class Meta(TypedModelMeta): verbose_name = _("contributor") diff --git a/settings.py b/settings.py index c27923f..ba8f443 100644 --- a/settings.py +++ b/settings.py @@ -41,6 +41,11 @@ INSTALLED_APPS.remove("portal") INSTALLED_APPS.remove("common") +# Auth user model +# https://docs.djangoproject.com/en/3.2/topics/auth/customizing/#substituting-a-custom-user-model + +AUTH_USER_MODEL = "api.contributor" + # Authentication backends # https://docs.djangoproject.com/en/3.2/ref/settings/#authentication-backends From af319bf58f3400c521c094863a38bd350f5f9003 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Tue, 5 Nov 2024 13:06:39 +0000 Subject: [PATCH 03/21] fix: initial migration --- api/migrations/0001_initial.py | 2 +- api/models/contributor.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/api/migrations/0001_initial.py b/api/migrations/0001_initial.py index accff7b..6d6363d 100644 --- a/api/migrations/0001_initial.py +++ b/api/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.25 on 2024-11-05 12:23 +# Generated by Django 3.2.25 on 2024-11-05 12:59 import django.core.validators from django.db import migrations, models diff --git a/api/models/contributor.py b/api/models/contributor.py index f295b78..9db0fff 100644 --- a/api/models/contributor.py +++ b/api/models/contributor.py @@ -3,6 +3,7 @@ Created on 05/07/2024 at 16:18:48(+01:00). """ +import sys import typing as t import requests @@ -60,6 +61,14 @@ def is_authenticated(self): # pylint: disable-next=import-outside-toplevel from .session import Session + # Avoid initial migration error where session table is not created yet + if ( + sys.argv + and "manage.py" in sys.argv[0] + and "runserver" not in sys.argv + ): + return True + return Session.objects.filter( contributor=self, expire_date__gt=timezone.now(), From 0068f9ce18e6bc04f5f0626bd8b0a05a7b4604a4 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Tue, 5 Nov 2024 13:19:28 +0000 Subject: [PATCH 04/21] add todos --- settings.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/settings.py b/settings.py index ba8f443..28f2fd3 100644 --- a/settings.py +++ b/settings.py @@ -37,9 +37,9 @@ # https://docs.djangoproject.com/en/3.2/ref/settings/#installed-apps INSTALLED_APPS.remove("codeforlife.user") -INSTALLED_APPS.remove("game") -INSTALLED_APPS.remove("portal") -INSTALLED_APPS.remove("common") +INSTALLED_APPS.remove("game") # TODO: remove after restructure +INSTALLED_APPS.remove("portal") # TODO: remove after restructure +INSTALLED_APPS.remove("common") # TODO: remove after restructure # Auth user model # https://docs.djangoproject.com/en/3.2/topics/auth/customizing/#substituting-a-custom-user-model From 705504918479b0a7e82d8ba376bf34f1f5c24582 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Tue, 5 Nov 2024 14:05:45 +0000 Subject: [PATCH 05/21] delete request --- Pipfile.lock | 4 ++-- api/common/__init__.py | 1 - api/common/api_request_factory.py | 18 +++++++++--------- api/common/model_serializer.py | 5 +++-- api/common/model_view_set.py | 7 ++++--- api/common/request.py | 28 ---------------------------- 6 files changed, 18 insertions(+), 45 deletions(-) delete mode 100644 api/common/request.py diff --git a/Pipfile.lock b/Pipfile.lock index 82c78f1..06e93d0 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "c3312e6b2f30db23a3f3221323fd0ef0f46f5470" + "ref": "67e6b17810d65b0971fdf840d4ec724b6a5eb5b6" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "c3312e6b2f30db23a3f3221323fd0ef0f46f5470" + "ref": "67e6b17810d65b0971fdf840d4ec724b6a5eb5b6" }, "codeforlife-portal": { "hashes": [ diff --git a/api/common/__init__.py b/api/common/__init__.py index 36cd86e..86ae357 100644 --- a/api/common/__init__.py +++ b/api/common/__init__.py @@ -7,4 +7,3 @@ from .model_serializer_test_case import ModelSerializerTestCase from .model_view_set import ModelViewSet from .model_view_set_test_case import ModelViewSetTestCase -from .request import Request diff --git a/api/common/api_request_factory.py b/api/common/api_request_factory.py index 3218771..ab70866 100644 --- a/api/common/api_request_factory.py +++ b/api/common/api_request_factory.py @@ -5,6 +5,7 @@ import typing as t +from codeforlife.request import BaseRequest from django.core.handlers.wsgi import WSGIRequest from rest_framework.parsers import ( FileUploadParser, @@ -15,7 +16,6 @@ from rest_framework.test import APIRequestFactory as _APIRequestFactory from ..models import Contributor -from .request import Request class APIRequestFactory(_APIRequestFactory): @@ -24,7 +24,7 @@ class APIRequestFactory(_APIRequestFactory): def request(self, user: t.Optional[Contributor] = None, **kwargs): wsgi_request = t.cast(WSGIRequest, super().request(**kwargs)) - request = Request( + request = BaseRequest[Contributor]( wsgi_request, parsers=[ JSONParser(), @@ -52,7 +52,7 @@ def generic( **extra ): return t.cast( - Request, + BaseRequest[Contributor], super().generic( method, path or "/", @@ -72,7 +72,7 @@ def get( # type: ignore[override] **extra ): return t.cast( - Request, + BaseRequest[Contributor], super().get( path or "/", data, @@ -96,7 +96,7 @@ def post( # type: ignore[override] format = "json" return t.cast( - Request, + BaseRequest[Contributor], super().post( path or "/", data, @@ -122,7 +122,7 @@ def put( # type: ignore[override] format = "json" return t.cast( - Request, + BaseRequest[Contributor], super().put( path or "/", data, @@ -148,7 +148,7 @@ def patch( # type: ignore[override] format = "json" return t.cast( - Request, + BaseRequest[Contributor], super().patch( path or "/", data, @@ -174,7 +174,7 @@ def delete( # type: ignore[override] format = "json" return t.cast( - Request, + BaseRequest[Contributor], super().delete( path or "/", data, @@ -200,7 +200,7 @@ def options( # type: ignore[override] format = "json" return t.cast( - Request, + BaseRequest[Contributor], super().options( path or "/", data or {}, diff --git a/api/common/model_serializer.py b/api/common/model_serializer.py index 282aaf1..e509030 100644 --- a/api/common/model_serializer.py +++ b/api/common/model_serializer.py @@ -5,11 +5,12 @@ import typing as t +from codeforlife.request import BaseRequest from codeforlife.types import DataDict from django.db.models import Model from rest_framework.serializers import ModelSerializer as _ModelSerializer -from .request import Request +from ..models import Contributor AnyModel = t.TypeVar("AnyModel", bound=Model) @@ -21,7 +22,7 @@ class ModelSerializer(_ModelSerializer[AnyModel], t.Generic[AnyModel]): def request(self): """The HTTP request that triggered the view.""" - return t.cast(Request, self.context["request"]) + return t.cast(BaseRequest[Contributor], self.context["request"]) @property def view(self): diff --git a/api/common/model_view_set.py b/api/common/model_view_set.py index 031a20b..2fb4fa8 100644 --- a/api/common/model_view_set.py +++ b/api/common/model_view_set.py @@ -5,10 +5,11 @@ import typing as t +from codeforlife.request import BaseRequest from django.db.models import Model from rest_framework.viewsets import ModelViewSet as _ModelViewSet -from .request import Request +from ..models import Contributor AnyModel = t.TypeVar("AnyModel", bound=Model) @@ -30,14 +31,14 @@ class __ModelViewSet(_ModelViewSet, t.Generic[AnyModel]): class ModelViewSet(__ModelViewSet[AnyModel], t.Generic[AnyModel]): """Base model view set.""" - request: Request + request: BaseRequest[Contributor] serializer_class: t.Optional[t.Type["ModelSerializer[AnyModel]"]] def initialize_request(self, request, *args, **kwargs): # NOTE: Call to super has side effects and is required. super().initialize_request(request, *args, **kwargs) - return Request( + return BaseRequest[Contributor]( request=request, parsers=self.get_parsers(), authenticators=self.get_authenticators(), diff --git a/api/common/request.py b/api/common/request.py deleted file mode 100644 index 7024231..0000000 --- a/api/common/request.py +++ /dev/null @@ -1,28 +0,0 @@ -""" -© Ocado Group -Created on 13/09/2024 at 12:01:04(+03:00). -""" - -import typing as t - -from django.contrib.auth.models import AnonymousUser -from rest_framework.request import Request as _Request - -from ..models import Contributor - - -# pylint: disable-next=abstract-method -class Request(_Request): - """A HTTP request.""" - - user: t.Union[Contributor, AnonymousUser] # type: ignore[assignment] - - @property - def anon_user(self): - """The anonymous user that made the request.""" - return t.cast(AnonymousUser, self.user) - - @property - def contributor(self): - """The contributor that made the request.""" - return t.cast(Contributor, self.user) From 108bfdcff5f1bb9cafa43c93f200b2ceb49999f6 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Tue, 5 Nov 2024 16:49:12 +0000 Subject: [PATCH 06/21] fix request --- Pipfile.lock | 4 ++-- api/auth/backends/github.py | 5 +++-- api/auth/backends/github_test.py | 5 +++-- api/views/session.py | 6 ++++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 06e93d0..00b671f 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "67e6b17810d65b0971fdf840d4ec724b6a5eb5b6" + "ref": "a3fc0c7df30a981ba98fc42ea6c398a5437ff38c" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "67e6b17810d65b0971fdf840d4ec724b6a5eb5b6" + "ref": "a3fc0c7df30a981ba98fc42ea6c398a5437ff38c" }, "codeforlife-portal": { "hashes": [ diff --git a/api/auth/backends/github.py b/api/auth/backends/github.py index 7ab9bf8..70c8e1f 100644 --- a/api/auth/backends/github.py +++ b/api/auth/backends/github.py @@ -6,12 +6,13 @@ import typing as t import requests -from codeforlife.request import HttpRequest +from codeforlife.request import BaseHttpRequest from codeforlife.types import JsonDict from django.conf import settings from django.contrib.auth.backends import BaseBackend from ...models import Contributor +from ...models.session import SessionStore class GitHubBackend(BaseBackend): @@ -19,7 +20,7 @@ class GitHubBackend(BaseBackend): def authenticate( # type: ignore[override] self, - request: t.Optional[HttpRequest], + request: t.Optional[BaseHttpRequest[SessionStore, Contributor]], code: t.Optional[str] = None, **kwargs, ): diff --git a/api/auth/backends/github_test.py b/api/auth/backends/github_test.py index 295c9c5..0dbb51a 100644 --- a/api/auth/backends/github_test.py +++ b/api/auth/backends/github_test.py @@ -7,12 +7,13 @@ from unittest.mock import Mock, patch import requests -from codeforlife.request import HttpRequest +from codeforlife.request import BaseHttpRequest from codeforlife.tests import TestCase from django.conf import settings from rest_framework import status from ...models import Contributor +from ...models.session import SessionStore from .github import GitHubBackend @@ -24,7 +25,7 @@ def setUp(self): # Set up initial test data self.contributor1 = Contributor.objects.get(id=1) self.backend = GitHubBackend() - self.request = HttpRequest() + self.request = BaseHttpRequest[SessionStore, Contributor]() self.gh_access_token_response = requests.Response() self.gh_access_token_response.status_code = status.HTTP_200_OK diff --git a/api/views/session.py b/api/views/session.py index 2ba2bde..3abd669 100644 --- a/api/views/session.py +++ b/api/views/session.py @@ -7,7 +7,7 @@ import typing as t from urllib.parse import quote_plus -from codeforlife.request import HttpRequest +from codeforlife.request import BaseHttpRequest from django.conf import settings from django.contrib.auth import login from django.contrib.auth.views import LoginView as _LoginView @@ -15,12 +15,14 @@ from rest_framework import status from ..forms import GitHubLoginForm +from ..models import Contributor +from ..models.session import SessionStore class LoginView(_LoginView): """Login users with their existing github accounts.""" - request: HttpRequest + request: BaseHttpRequest[SessionStore, Contributor] def get_form_class(self): return GitHubLoginForm From fb76ca517ed688525381b3387912ab1c07971f73 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Tue, 5 Nov 2024 16:53:02 +0000 Subject: [PATCH 07/21] fix base request --- api/common/api_request_factory.py | 17 +++++++++-------- api/common/model_serializer.py | 5 ++++- api/common/model_view_set.py | 5 +++-- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/api/common/api_request_factory.py b/api/common/api_request_factory.py index ab70866..0e2cffb 100644 --- a/api/common/api_request_factory.py +++ b/api/common/api_request_factory.py @@ -16,6 +16,7 @@ from rest_framework.test import APIRequestFactory as _APIRequestFactory from ..models import Contributor +from ..models.session import SessionStore class APIRequestFactory(_APIRequestFactory): @@ -24,7 +25,7 @@ class APIRequestFactory(_APIRequestFactory): def request(self, user: t.Optional[Contributor] = None, **kwargs): wsgi_request = t.cast(WSGIRequest, super().request(**kwargs)) - request = BaseRequest[Contributor]( + request = BaseRequest[SessionStore, Contributor]( wsgi_request, parsers=[ JSONParser(), @@ -52,7 +53,7 @@ def generic( **extra ): return t.cast( - BaseRequest[Contributor], + BaseRequest[SessionStore, Contributor], super().generic( method, path or "/", @@ -72,7 +73,7 @@ def get( # type: ignore[override] **extra ): return t.cast( - BaseRequest[Contributor], + BaseRequest[SessionStore, Contributor], super().get( path or "/", data, @@ -96,7 +97,7 @@ def post( # type: ignore[override] format = "json" return t.cast( - BaseRequest[Contributor], + BaseRequest[SessionStore, Contributor], super().post( path or "/", data, @@ -122,7 +123,7 @@ def put( # type: ignore[override] format = "json" return t.cast( - BaseRequest[Contributor], + BaseRequest[SessionStore, Contributor], super().put( path or "/", data, @@ -148,7 +149,7 @@ def patch( # type: ignore[override] format = "json" return t.cast( - BaseRequest[Contributor], + BaseRequest[SessionStore, Contributor], super().patch( path or "/", data, @@ -174,7 +175,7 @@ def delete( # type: ignore[override] format = "json" return t.cast( - BaseRequest[Contributor], + BaseRequest[SessionStore, Contributor], super().delete( path or "/", data, @@ -200,7 +201,7 @@ def options( # type: ignore[override] format = "json" return t.cast( - BaseRequest[Contributor], + BaseRequest[SessionStore, Contributor], super().options( path or "/", data or {}, diff --git a/api/common/model_serializer.py b/api/common/model_serializer.py index e509030..aae7893 100644 --- a/api/common/model_serializer.py +++ b/api/common/model_serializer.py @@ -11,6 +11,7 @@ from rest_framework.serializers import ModelSerializer as _ModelSerializer from ..models import Contributor +from ..models.session import SessionStore AnyModel = t.TypeVar("AnyModel", bound=Model) @@ -22,7 +23,9 @@ class ModelSerializer(_ModelSerializer[AnyModel], t.Generic[AnyModel]): def request(self): """The HTTP request that triggered the view.""" - return t.cast(BaseRequest[Contributor], self.context["request"]) + return t.cast( + BaseRequest[SessionStore, Contributor], self.context["request"] + ) @property def view(self): diff --git a/api/common/model_view_set.py b/api/common/model_view_set.py index 2fb4fa8..676de48 100644 --- a/api/common/model_view_set.py +++ b/api/common/model_view_set.py @@ -10,6 +10,7 @@ from rest_framework.viewsets import ModelViewSet as _ModelViewSet from ..models import Contributor +from ..models.session import SessionStore AnyModel = t.TypeVar("AnyModel", bound=Model) @@ -31,14 +32,14 @@ class __ModelViewSet(_ModelViewSet, t.Generic[AnyModel]): class ModelViewSet(__ModelViewSet[AnyModel], t.Generic[AnyModel]): """Base model view set.""" - request: BaseRequest[Contributor] + request: BaseRequest[SessionStore, Contributor] serializer_class: t.Optional[t.Type["ModelSerializer[AnyModel]"]] def initialize_request(self, request, *args, **kwargs): # NOTE: Call to super has side effects and is required. super().initialize_request(request, *args, **kwargs) - return BaseRequest[Contributor]( + return BaseRequest[SessionStore, Contributor]( request=request, parsers=self.get_parsers(), authenticators=self.get_authenticators(), From 45dae2097d6594325f36d93cf9dc10c0ccbc44ba Mon Sep 17 00:00:00 2001 From: SKairinos Date: Tue, 5 Nov 2024 16:57:28 +0000 Subject: [PATCH 08/21] fix property refs --- api/serializers/agreement_signature.py | 2 +- api/views/agreement_signature.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/serializers/agreement_signature.py b/api/serializers/agreement_signature.py index 0a60be6..1bfc02c 100644 --- a/api/serializers/agreement_signature.py +++ b/api/serializers/agreement_signature.py @@ -38,5 +38,5 @@ def validate_signed_at(self, value: datetime): return value def create(self, validated_data): - validated_data["contributor"] = self.request.contributor + validated_data["contributor"] = self.request.auth_user return super().create(validated_data) diff --git a/api/views/agreement_signature.py b/api/views/agreement_signature.py index da84870..82f9dd3 100644 --- a/api/views/agreement_signature.py +++ b/api/views/agreement_signature.py @@ -18,7 +18,7 @@ class AgreementSignatureViewSet(ModelViewSet[AgreementSignature]): def get_queryset(self): return AgreementSignature.objects.filter( - contributor=self.request.contributor + contributor=self.request.auth_user ).order_by("signed_at") @action(detail=False, methods=["get"]) From 87391a5d2d499dc753707a363cbe2b192f3f357f Mon Sep 17 00:00:00 2001 From: SKairinos Date: Tue, 5 Nov 2024 17:48:33 +0000 Subject: [PATCH 09/21] delete api request factory --- Pipfile.lock | 4 +- api/common/api_request_factory.py | 213 ----------------------- api/common/model_serializer_test_case.py | 12 +- api/common/model_view_set_test_case.py | 8 +- 4 files changed, 17 insertions(+), 220 deletions(-) delete mode 100644 api/common/api_request_factory.py diff --git a/Pipfile.lock b/Pipfile.lock index 00b671f..615a133 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "a3fc0c7df30a981ba98fc42ea6c398a5437ff38c" + "ref": "fb58a80778d99aadde76e0ddaa784f57a126f571" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "a3fc0c7df30a981ba98fc42ea6c398a5437ff38c" + "ref": "fb58a80778d99aadde76e0ddaa784f57a126f571" }, "codeforlife-portal": { "hashes": [ diff --git a/api/common/api_request_factory.py b/api/common/api_request_factory.py deleted file mode 100644 index 0e2cffb..0000000 --- a/api/common/api_request_factory.py +++ /dev/null @@ -1,213 +0,0 @@ -""" -© Ocado Group -Created on 13/09/2024 at 12:00:25(+03:00). -""" - -import typing as t - -from codeforlife.request import BaseRequest -from django.core.handlers.wsgi import WSGIRequest -from rest_framework.parsers import ( - FileUploadParser, - FormParser, - JSONParser, - MultiPartParser, -) -from rest_framework.test import APIRequestFactory as _APIRequestFactory - -from ..models import Contributor -from ..models.session import SessionStore - - -class APIRequestFactory(_APIRequestFactory): - """Custom API request factory that returns DRF's Request object.""" - - def request(self, user: t.Optional[Contributor] = None, **kwargs): - wsgi_request = t.cast(WSGIRequest, super().request(**kwargs)) - - request = BaseRequest[SessionStore, Contributor]( - wsgi_request, - parsers=[ - JSONParser(), - FormParser(), - MultiPartParser(), - FileUploadParser(), - ], - ) - - if user: - # pylint: disable-next=attribute-defined-outside-init - request.user = user - - return request - - # pylint: disable-next=too-many-arguments - def generic( - self, - method: str, - path: t.Optional[str] = None, - data: t.Optional[str] = None, - content_type: t.Optional[str] = None, - secure: bool = True, - user: t.Optional[Contributor] = None, - **extra - ): - return t.cast( - BaseRequest[SessionStore, Contributor], - super().generic( - method, - path or "/", - data or "", - content_type or "application/json", - secure, - user=user, - **extra, - ), - ) - - def get( # type: ignore[override] - self, - path: t.Optional[str] = None, - data: t.Any = None, - user: t.Optional[Contributor] = None, - **extra - ): - return t.cast( - BaseRequest[SessionStore, Contributor], - super().get( - path or "/", - data, - user=user, - **extra, - ), - ) - - # pylint: disable-next=too-many-arguments - def post( # type: ignore[override] - self, - path: t.Optional[str] = None, - data: t.Any = None, - # pylint: disable-next=redefined-builtin - format: t.Optional[str] = None, - content_type: t.Optional[str] = None, - user: t.Optional[Contributor] = None, - **extra - ): - if format is None and content_type is None: - format = "json" - - return t.cast( - BaseRequest[SessionStore, Contributor], - super().post( - path or "/", - data, - format, - content_type, - user=user, - **extra, - ), - ) - - # pylint: disable-next=too-many-arguments - def put( # type: ignore[override] - self, - path: t.Optional[str] = None, - data: t.Any = None, - # pylint: disable-next=redefined-builtin - format: t.Optional[str] = None, - content_type: t.Optional[str] = None, - user: t.Optional[Contributor] = None, - **extra - ): - if format is None and content_type is None: - format = "json" - - return t.cast( - BaseRequest[SessionStore, Contributor], - super().put( - path or "/", - data, - format, - content_type, - user=user, - **extra, - ), - ) - - # pylint: disable-next=too-many-arguments - def patch( # type: ignore[override] - self, - path: t.Optional[str] = None, - data: t.Any = None, - # pylint: disable-next=redefined-builtin - format: t.Optional[str] = None, - content_type: t.Optional[str] = None, - user: t.Optional[Contributor] = None, - **extra - ): - if format is None and content_type is None: - format = "json" - - return t.cast( - BaseRequest[SessionStore, Contributor], - super().patch( - path or "/", - data, - format, - content_type, - user=user, - **extra, - ), - ) - - # pylint: disable-next=too-many-arguments - def delete( # type: ignore[override] - self, - path: t.Optional[str] = None, - data: t.Any = None, - # pylint: disable-next=redefined-builtin - format: t.Optional[str] = None, - content_type: t.Optional[str] = None, - user: t.Optional[Contributor] = None, - **extra - ): - if format is None and content_type is None: - format = "json" - - return t.cast( - BaseRequest[SessionStore, Contributor], - super().delete( - path or "/", - data, - format, - content_type, - user=user, - **extra, - ), - ) - - # pylint: disable-next=too-many-arguments - def options( # type: ignore[override] - self, - path: t.Optional[str] = None, - data: t.Optional[t.Union[t.Dict[str, str], str]] = None, - # pylint: disable-next=redefined-builtin - format: t.Optional[str] = None, - content_type: t.Optional[str] = None, - user: t.Optional[Contributor] = None, - **extra - ): - if format is None and content_type is None: - format = "json" - - return t.cast( - BaseRequest[SessionStore, Contributor], - super().options( - path or "/", - data or {}, - format, - content_type, - user=user, - **extra, - ), - ) diff --git a/api/common/model_serializer_test_case.py b/api/common/model_serializer_test_case.py index 26c3933..79d030f 100644 --- a/api/common/model_serializer_test_case.py +++ b/api/common/model_serializer_test_case.py @@ -5,13 +5,15 @@ import typing as t +from codeforlife.request import BaseRequest +from codeforlife.tests import BaseAPIRequestFactory from codeforlife.tests import ( ModelSerializerTestCase as _ModelSerializerTestCase, ) from django.db.models import Model from ..models import Contributor -from .api_request_factory import APIRequestFactory +from ..models.session import SessionStore from .model_serializer import ModelSerializer AnyModel = t.TypeVar("AnyModel", bound=Model) @@ -24,13 +26,17 @@ class ModelSerializerTestCase(_ModelSerializerTestCase, t.Generic[AnyModel]): ModelSerializer[AnyModel] ] - request_factory: APIRequestFactory # type: ignore[assignment] + request_factory: BaseAPIRequestFactory[ # type: ignore[assignment] + BaseRequest[SessionStore, Contributor], Contributor + ] @classmethod def setUpClass(cls): result = super().setUpClass() - cls.request_factory = APIRequestFactory() + cls.request_factory = BaseAPIRequestFactory[ + BaseRequest[SessionStore, Contributor], Contributor + ]() return result diff --git a/api/common/model_view_set_test_case.py b/api/common/model_view_set_test_case.py index 2d54daa..9ea49b9 100644 --- a/api/common/model_view_set_test_case.py +++ b/api/common/model_view_set_test_case.py @@ -10,6 +10,8 @@ from unittest.mock import ANY, call, patch import requests +from codeforlife.request import BaseRequest +from codeforlife.tests import BaseAPIRequestFactory from codeforlife.tests import ModelViewSetClient as _ModelViewSetClient from codeforlife.tests import ModelViewSetTestCase as _ModelViewSetTestCase from django.conf import settings @@ -18,7 +20,7 @@ from rest_framework.test import APIClient from ..models import Contributor -from .api_request_factory import APIRequestFactory +from ..models.session import SessionStore from .model_view_set import ModelViewSet AnyModel = t.TypeVar("AnyModel", bound=Model) @@ -31,7 +33,9 @@ class ModelViewSetClient(_ModelViewSetClient, t.Generic[AnyModel]): def __init__(self, enforce_csrf_checks: bool = False, **defaults): super().__init__(enforce_csrf_checks, **defaults) - self.request_factory = APIRequestFactory( # type: ignore[assignment] + self.request_factory = BaseAPIRequestFactory[ + BaseRequest[SessionStore, Contributor], Contributor + ]( # type: ignore[assignment] enforce_csrf_checks, **defaults ) From 8b18a944aa0fa176a743b670680f496cf92ca6c9 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Wed, 6 Nov 2024 12:22:16 +0000 Subject: [PATCH 10/21] use base model serializer and view set --- Pipfile.lock | 4 +-- api/common/model_serializer.py | 45 ++++--------------------------- api/common/model_view_set.py | 31 +++++---------------- api/models/agreement_signature.py | 2 +- 4 files changed, 14 insertions(+), 68 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 615a133..4963da4 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "fb58a80778d99aadde76e0ddaa784f57a126f571" + "ref": "b805632573a12224713fc504e290b9475ef2073b" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "fb58a80778d99aadde76e0ddaa784f57a126f571" + "ref": "b805632573a12224713fc504e290b9475ef2073b" }, "codeforlife-portal": { "hashes": [ diff --git a/api/common/model_serializer.py b/api/common/model_serializer.py index aae7893..e51628c 100644 --- a/api/common/model_serializer.py +++ b/api/common/model_serializer.py @@ -6,9 +6,8 @@ import typing as t from codeforlife.request import BaseRequest -from codeforlife.types import DataDict +from codeforlife.serializers import BaseModelSerializer from django.db.models import Model -from rest_framework.serializers import ModelSerializer as _ModelSerializer from ..models import Contributor from ..models.session import SessionStore @@ -16,42 +15,8 @@ AnyModel = t.TypeVar("AnyModel", bound=Model) -class ModelSerializer(_ModelSerializer[AnyModel], t.Generic[AnyModel]): +class ModelSerializer( + BaseModelSerializer[BaseRequest[SessionStore, Contributor], AnyModel], + t.Generic[AnyModel], +): """Base model serializer.""" - - @property - def request(self): - """The HTTP request that triggered the view.""" - - return t.cast( - BaseRequest[SessionStore, Contributor], self.context["request"] - ) - - @property - def view(self): - """The view that instantiated this serializer.""" - # NOTE: import outside top-level to avoid circular imports. - # pylint: disable-next=import-outside-toplevel - from .model_view_set import ModelViewSet - - return t.cast(ModelViewSet[AnyModel], self.context["view"]) - - @property - def non_none_instance(self): - """Casts the instance to not None.""" - return t.cast(AnyModel, self.instance) - - # pylint: disable-next=useless-parent-delegation - def update(self, instance: AnyModel, validated_data: DataDict) -> AnyModel: - return super().update(instance, validated_data) - - # pylint: disable-next=useless-parent-delegation - def create(self, validated_data: DataDict) -> AnyModel: - return super().create(validated_data) - - def validate(self, attrs: DataDict): - return attrs - - # pylint: disable-next=useless-parent-delegation - def to_representation(self, instance: AnyModel) -> DataDict: - return super().to_representation(instance) diff --git a/api/common/model_view_set.py b/api/common/model_view_set.py index 676de48..1587b6d 100644 --- a/api/common/model_view_set.py +++ b/api/common/model_view_set.py @@ -6,8 +6,8 @@ import typing as t from codeforlife.request import BaseRequest +from codeforlife.views import BaseModelViewSet from django.db.models import Model -from rest_framework.viewsets import ModelViewSet as _ModelViewSet from ..models import Contributor from ..models.session import SessionStore @@ -17,32 +17,13 @@ if t.TYPE_CHECKING: # pragma: no cover from .model_serializer import ModelSerializer - # NOTE: This raises an error during runtime. - # pylint: disable-next=too-few-public-methods,invalid-name - class __ModelViewSet(_ModelViewSet[AnyModel], t.Generic[AnyModel]): - pass - -else: - # pylint: disable-next=too-many-ancestors,invalid-name - class __ModelViewSet(_ModelViewSet, t.Generic[AnyModel]): - pass - # pylint: disable-next=too-many-ancestors -class ModelViewSet(__ModelViewSet[AnyModel], t.Generic[AnyModel]): +class ModelViewSet( + BaseModelViewSet[BaseRequest[SessionStore, Contributor], AnyModel], + t.Generic[AnyModel], +): """Base model view set.""" - request: BaseRequest[SessionStore, Contributor] + request_class = BaseRequest[SessionStore, Contributor] serializer_class: t.Optional[t.Type["ModelSerializer[AnyModel]"]] - - def initialize_request(self, request, *args, **kwargs): - # NOTE: Call to super has side effects and is required. - super().initialize_request(request, *args, **kwargs) - - return BaseRequest[SessionStore, Contributor]( - request=request, - parsers=self.get_parsers(), - authenticators=self.get_authenticators(), - negotiator=self.get_content_negotiator(), - parser_context=self.get_parser_context(request), - ) diff --git a/api/models/agreement_signature.py b/api/models/agreement_signature.py index f2ba685..c06e25e 100644 --- a/api/models/agreement_signature.py +++ b/api/models/agreement_signature.py @@ -45,7 +45,7 @@ class Meta(TypedModelMeta): verbose_name_plural = _("agreement signatures") def __str__(self): - cont = f"Contributor {self.contributor.pk} signed" + cont = f"Contributor {self.contributor_id} signed" repo = f"{self.agreement_id[:7]} at {self.signed_at}" return f"{cont} {repo}" From 895a82b4139ee2eea14774c68698a8c4ac888e26 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Wed, 6 Nov 2024 13:20:46 +0000 Subject: [PATCH 11/21] abstract model serializers --- Pipfile.lock | 4 +- api/common/model_serializer.py | 9 +++- api/common/model_serializer_test_case.py | 68 ++++++++++-------------- 3 files changed, 39 insertions(+), 42 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 4963da4..3656a56 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "b805632573a12224713fc504e290b9475ef2073b" + "ref": "27df3eeebbc3e486d49276906c2efa4c7702fc3b" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "b805632573a12224713fc504e290b9475ef2073b" + "ref": "27df3eeebbc3e486d49276906c2efa4c7702fc3b" }, "codeforlife-portal": { "hashes": [ diff --git a/api/common/model_serializer.py b/api/common/model_serializer.py index e51628c..4462c46 100644 --- a/api/common/model_serializer.py +++ b/api/common/model_serializer.py @@ -6,7 +6,7 @@ import typing as t from codeforlife.request import BaseRequest -from codeforlife.serializers import BaseModelSerializer +from codeforlife.serializers import BaseModelListSerializer, BaseModelSerializer from django.db.models import Model from ..models import Contributor @@ -20,3 +20,10 @@ class ModelSerializer( t.Generic[AnyModel], ): """Base model serializer.""" + + +class ModelListSerializer( + BaseModelListSerializer[BaseRequest[SessionStore, Contributor], AnyModel], + t.Generic[AnyModel], +): + """Base model list serializer.""" diff --git a/api/common/model_serializer_test_case.py b/api/common/model_serializer_test_case.py index 79d030f..addcddb 100644 --- a/api/common/model_serializer_test_case.py +++ b/api/common/model_serializer_test_case.py @@ -6,57 +6,47 @@ import typing as t from codeforlife.request import BaseRequest -from codeforlife.tests import BaseAPIRequestFactory from codeforlife.tests import ( - ModelSerializerTestCase as _ModelSerializerTestCase, + BaseAPIRequestFactory, + BaseModelListSerializerTestCase, + BaseModelSerializerTestCase, ) from django.db.models import Model from ..models import Contributor from ..models.session import SessionStore -from .model_serializer import ModelSerializer +from .model_serializer import ModelListSerializer, ModelSerializer AnyModel = t.TypeVar("AnyModel", bound=Model) -class ModelSerializerTestCase(_ModelSerializerTestCase, t.Generic[AnyModel]): +class ModelSerializerTestCase( + BaseModelSerializerTestCase[ + ModelSerializer[AnyModel], + BaseAPIRequestFactory[ + BaseRequest[SessionStore, Contributor], Contributor + ], + AnyModel, + ], + t.Generic[AnyModel], +): """Base model serializer test case.""" - model_serializer_class: t.Type[ # type: ignore[assignment] - ModelSerializer[AnyModel] - ] - - request_factory: BaseAPIRequestFactory[ # type: ignore[assignment] - BaseRequest[SessionStore, Contributor], Contributor - ] + request_factory_class = BaseAPIRequestFactory - @classmethod - def setUpClass(cls): - result = super().setUpClass() - cls.request_factory = BaseAPIRequestFactory[ +# pylint: disable-next=too-many-ancestors +class ModelListSerializerTestCase( + BaseModelListSerializerTestCase[ + ModelListSerializer[AnyModel], + ModelSerializer[AnyModel], + BaseAPIRequestFactory[ BaseRequest[SessionStore, Contributor], Contributor - ]() - - return result - - @classmethod - def get_request_user_class(cls) -> t.Type[AnyModel]: - """Get the model view set's class. - - Returns: - The model view set's class. - """ - return Contributor # type: ignore[return-value] - - @classmethod - def get_model_class(cls) -> t.Type[AnyModel]: - """Get the model view set's class. - - Returns: - The model view set's class. - """ - # pylint: disable-next=no-member - return t.get_args(cls.__orig_bases__[0])[ # type: ignore[attr-defined] - 0 - ] + ], + AnyModel, + ], + t.Generic[AnyModel], +): + """Base model serializer test case.""" + + request_factory_class = BaseAPIRequestFactory From ce1b4d318467972725f0688584558ef2bb98dffa Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 12:22:09 +0000 Subject: [PATCH 12/21] use abstract user and session models --- Pipfile.lock | 4 +-- api/migrations/0001_initial.py | 4 +-- api/models/contributor.py | 3 +- api/models/session.py | 61 ++-------------------------------- 4 files changed, 8 insertions(+), 64 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 3656a56..c45acda 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "27df3eeebbc3e486d49276906c2efa4c7702fc3b" + "ref": "b633a24ec881a51988de5e4bac3b37df320f8082" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "27df3eeebbc3e486d49276906c2efa4c7702fc3b" + "ref": "b633a24ec881a51988de5e4bac3b37df320f8082" }, "codeforlife-portal": { "hashes": [ diff --git a/api/migrations/0001_initial.py b/api/migrations/0001_initial.py index 6d6363d..8ebf9c8 100644 --- a/api/migrations/0001_initial.py +++ b/api/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.25 on 2024-11-05 12:59 +# Generated by Django 3.2.25 on 2024-11-07 11:40 import django.core.validators from django.db import migrations, models @@ -34,7 +34,7 @@ class Migration(migrations.Migration): ('session_key', models.CharField(max_length=40, primary_key=True, serialize=False, verbose_name='session key')), ('session_data', models.TextField(verbose_name='session data')), ('expire_date', models.DateTimeField(db_index=True, verbose_name='expire date')), - ('contributor', models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='api.contributor')), + ('user', models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='api.contributor')), ], options={ 'verbose_name': 'session', diff --git a/api/models/contributor.py b/api/models/contributor.py index 9db0fff..50a3b3f 100644 --- a/api/models/contributor.py +++ b/api/models/contributor.py @@ -7,8 +7,8 @@ import typing as t import requests +from codeforlife.models import AbstractBaseUser from codeforlife.types import JsonDict -from django.contrib.auth.models import AbstractBaseUser from django.db import models from django.db.models import QuerySet from django.utils import timezone @@ -39,7 +39,6 @@ class Contributor(AbstractBaseUser): # Contributors log in with their GitHub account. password = None # type: ignore[assignment] - pk: int id = models.IntegerField( primary_key=True, help_text=_("The contributor's GitHub user-ID.") ) diff --git a/api/models/session.py b/api/models/session.py index 91b6976..78da8b3 100644 --- a/api/models/session.py +++ b/api/models/session.py @@ -3,13 +3,7 @@ Created on 06/08/2024 at 14:07:24(+01:00). """ -import typing as t - -from django.contrib.auth import SESSION_KEY -from django.contrib.sessions.backends.db import SessionStore as DBStore -from django.contrib.sessions.base_session import AbstractBaseSession -from django.db import models -from django.utils import timezone +from codeforlife.models import AbstractBaseSession, BaseSessionStore from .contributor import Contributor @@ -20,66 +14,17 @@ class Session(AbstractBaseSession): https://docs.djangoproject.com/en/3.2/topics/http/sessions/#example """ - contributor_id: int - contributor = models.OneToOneField( - Contributor, - null=True, - blank=True, - on_delete=models.CASCADE, - ) - - @property - def is_expired(self): - """Whether or not this session has expired.""" - return self.expire_date < timezone.now() - - @property - def store(self): - """A store instance for this session.""" - return self.get_session_store_class()(self.session_key) + user = AbstractBaseSession.init_user_field(Contributor) @classmethod def get_session_store_class(cls): return SessionStore -class SessionStore(DBStore): +class SessionStore(BaseSessionStore[Session, Contributor]): """ A custom session store interface to support: 1. creating only one session per contributor; 2. clearing a contributor's expired sessions. https://docs.djangoproject.com/en/3.2/topics/http/sessions/#example """ - - @classmethod - def get_model_class(cls): - return Session - - def create_model_instance(self, data): - try: - contributor_id = int(data.get(SESSION_KEY)) - except (ValueError, TypeError): - # Create an anon session. - return super().create_model_instance(data) - - model_class = self.get_model_class() - - try: - session = model_class.objects.get(contributor_id=contributor_id) - except model_class.DoesNotExist: - # Associate session to contributor. - session = model_class.objects.get(session_key=self.session_key) - session.contributor = Contributor.objects.get(id=contributor_id) - - session.session_data = self.encode(data) - - return session - - @classmethod - def clear_expired(cls, contributor_id: t.Optional[int] = None): - session_query = cls.get_model_class().objects.filter( - expire_date__lt=timezone.now() - ) - if contributor_id: - session_query = session_query.filter(contributor_id=contributor_id) - session_query.delete() From 676b35453c6c73c4a38a28a77962eebe7df20630 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 12:25:42 +0000 Subject: [PATCH 13/21] fix query --- api/models/contributor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/models/contributor.py b/api/models/contributor.py index 50a3b3f..92137de 100644 --- a/api/models/contributor.py +++ b/api/models/contributor.py @@ -69,7 +69,7 @@ def is_authenticated(self): return True return Session.objects.filter( - contributor=self, + user=self, expire_date__gt=timezone.now(), ).exists() From 134bb13f23f23054c936961c5e1f6a06c291c024 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 13:17:11 +0000 Subject: [PATCH 14/21] new cfl package --- Pipfile.lock | 4 ++-- api/models/contributor.py | 19 ------------------- api/views/session.py | 2 +- 3 files changed, 3 insertions(+), 22 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index c45acda..6617529 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "b633a24ec881a51988de5e4bac3b37df320f8082" + "ref": "62266099e0375e2a2d1989bf9f72d0a76fc5bc58" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "b633a24ec881a51988de5e4bac3b37df320f8082" + "ref": "62266099e0375e2a2d1989bf9f72d0a76fc5bc58" }, "codeforlife-portal": { "hashes": [ diff --git a/api/models/contributor.py b/api/models/contributor.py index 92137de..88e33fc 100644 --- a/api/models/contributor.py +++ b/api/models/contributor.py @@ -54,25 +54,6 @@ class Meta(TypedModelMeta): def __str__(self): return f"{self.name} <{self.primary_email}>" - @property - def is_authenticated(self): - """A flag designating if this contributor has authenticated.""" - # pylint: disable-next=import-outside-toplevel - from .session import Session - - # Avoid initial migration error where session table is not created yet - if ( - sys.argv - and "manage.py" in sys.argv[0] - and "runserver" not in sys.argv - ): - return True - - return Session.objects.filter( - user=self, - expire_date__gt=timezone.now(), - ).exists() - @property def primary_email(self): """The primary email of this contributor, if they have one.""" diff --git a/api/views/session.py b/api/views/session.py index 3abd669..b8aef56 100644 --- a/api/views/session.py +++ b/api/views/session.py @@ -38,7 +38,7 @@ def form_valid(self, form: GitHubLoginForm): # type: ignore # pylint: disable-next=line-too-long self.request.session.clear_expired( - contributor_id=contributor.pk + user_id=contributor.pk ) # type: ignore login(self.request, contributor) # type: ignore From 8c881fb972f78f1dc09cf11ad364dfbb123d2153 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 13:23:08 +0000 Subject: [PATCH 15/21] test --- Pipfile.lock | 4 ++-- api/models/contributor.py | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 6617529..65ef6c2 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "62266099e0375e2a2d1989bf9f72d0a76fc5bc58" + "ref": "691d3721e8e278312e82eebcfe19f31a4ebe6c35" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "62266099e0375e2a2d1989bf9f72d0a76fc5bc58" + "ref": "691d3721e8e278312e82eebcfe19f31a4ebe6c35" }, "codeforlife-portal": { "hashes": [ diff --git a/api/models/contributor.py b/api/models/contributor.py index 88e33fc..3ea9f77 100644 --- a/api/models/contributor.py +++ b/api/models/contributor.py @@ -3,7 +3,6 @@ Created on 05/07/2024 at 16:18:48(+01:00). """ -import sys import typing as t import requests @@ -11,7 +10,6 @@ from codeforlife.types import JsonDict from django.db import models from django.db.models import QuerySet -from django.utils import timezone from django.utils.translation import gettext_lazy as _ from requests.exceptions import RequestException From daa17c07091eccce61de3f998b6d1d3b455ed613 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 13:56:39 +0000 Subject: [PATCH 16/21] abstract model view set test case and client --- Pipfile.lock | 4 +- api/common/model_view_set_test_case.py | 60 +++++++++++--------------- api/views/agreement_signature_test.py | 10 ++--- 3 files changed, 31 insertions(+), 43 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 65ef6c2..3cfa68e 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "691d3721e8e278312e82eebcfe19f31a4ebe6c35" + "ref": "687d58797d7e46c6fb039c02172b90fada557771" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "691d3721e8e278312e82eebcfe19f31a4ebe6c35" + "ref": "687d58797d7e46c6fb039c02172b90fada557771" }, "codeforlife-portal": { "hashes": [ diff --git a/api/common/model_view_set_test_case.py b/api/common/model_view_set_test_case.py index 9ea49b9..ab44eb8 100644 --- a/api/common/model_view_set_test_case.py +++ b/api/common/model_view_set_test_case.py @@ -11,9 +11,11 @@ import requests from codeforlife.request import BaseRequest -from codeforlife.tests import BaseAPIRequestFactory -from codeforlife.tests import ModelViewSetClient as _ModelViewSetClient -from codeforlife.tests import ModelViewSetTestCase as _ModelViewSetTestCase +from codeforlife.tests import ( + BaseAPIRequestFactory, + BaseModelViewSetClient, + BaseModelViewSetTestCase, +) from django.conf import settings from django.db.models import Model from rest_framework import status @@ -27,17 +29,18 @@ # pylint: disable-next=too-many-ancestors,arguments-differ -class ModelViewSetClient(_ModelViewSetClient, t.Generic[AnyModel]): +class ModelViewSetClient( + BaseModelViewSetClient[ + "ModelViewSetTestCase[AnyModel]", + BaseAPIRequestFactory[ + BaseRequest[SessionStore, Contributor], Contributor + ], + ], + t.Generic[AnyModel], +): """Client used to make test requests.""" - def __init__(self, enforce_csrf_checks: bool = False, **defaults): - super().__init__(enforce_csrf_checks, **defaults) - - self.request_factory = BaseAPIRequestFactory[ - BaseRequest[SessionStore, Contributor], Contributor - ]( # type: ignore[assignment] - enforce_csrf_checks, **defaults - ) + request_factory_class = BaseAPIRequestFactory # pylint: disable-next=arguments-differ def login( # type: ignore[override] @@ -144,20 +147,19 @@ def login( # type: ignore[override] return Contributor.objects.get(session=self.session.session_key) - # pylint: disable-next=arguments-differ - def login_as(self, contributor: Contributor): # type: ignore[override] - return self.login(contributor) - # pylint: disable-next=too-many-ancestors -class ModelViewSetTestCase(_ModelViewSetTestCase, t.Generic[AnyModel]): +class ModelViewSetTestCase( + BaseModelViewSetTestCase[ + ModelViewSet[AnyModel], + ModelViewSetClient[AnyModel], + AnyModel, + ], + t.Generic[AnyModel], +): """Base model view set test case.""" - model_view_set_class: t.Type[ # type: ignore[assignment] - ModelViewSet[AnyModel] - ] - client: ModelViewSetClient[AnyModel] - client_class: t.Type[ModelViewSetClient[AnyModel]] = ModelViewSetClient + client_class = ModelViewSetClient @classmethod def get_model_class(cls) -> t.Type[AnyModel]: @@ -170,17 +172,3 @@ def get_model_class(cls) -> t.Type[AnyModel]: return t.get_args(cls.__orig_bases__[0])[ # type: ignore[attr-defined] 0 ] - - @classmethod - def get_request_user_class(cls): - return Contributor - - def _get_client_class(self): - # TODO: unpack type args in index after moving to python 3.11 - # pylint: disable-next=too-few-public-methods - class _Client( - self.client_class[self.get_model_class()] # type: ignore[misc] - ): - _test_case = self - - return _Client diff --git a/api/views/agreement_signature_test.py b/api/views/agreement_signature_test.py index eda6a68..8ee7873 100644 --- a/api/views/agreement_signature_test.py +++ b/api/views/agreement_signature_test.py @@ -74,7 +74,7 @@ def test_retrieve(self): agreement_signature = contributor.agreement_signatures.first() assert agreement_signature - self.client.login_as(contributor) + self.client.login(contributor) self.client.retrieve(model=agreement_signature) def test_list(self): @@ -85,7 +85,7 @@ def test_list(self): ) assert agreement_signatures - self.client.login_as(contributor) + self.client.login(contributor) self.client.list(models=agreement_signatures) def test_create(self): @@ -99,7 +99,7 @@ def test_create(self): # pylint: disable-next=protected-access response._content = json.dumps([{"sha": agreement_id}]).encode("utf-8") - self.client.login_as(contributor) + self.client.login(contributor) with patch.object( requests, "get", return_value=response @@ -132,7 +132,7 @@ def _test_check_signed_latest( "utf-8" ) - self.client.login_as(contributor) + self.client.login(contributor) with patch.object( requests, "get", return_value=response @@ -167,7 +167,7 @@ def test_check_signed_latest__github_api_error(self): response = requests.Response() response.status_code = status.HTTP_500_INTERNAL_SERVER_ERROR - self.client.login_as(self.contributor3) + self.client.login(self.contributor3) with patch.object( requests, "get", return_value=response From 7b96de8b741a8862737ae92998bdf2ad0d526a87 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 14:16:13 +0000 Subject: [PATCH 17/21] relocate files --- api/common/__init__.py | 9 ---- .../_model_list_serializer.py} | 9 +--- .../_model_list_serializer_test_case.py} | 19 +-------- api/serializers/_model_serializer.py | 22 ++++++++++ .../_model_serializer_test_case.py | 31 ++++++++++++++ api/serializers/agreement_signature.py | 2 +- api/serializers/agreement_signature_test.py | 2 +- api/serializers/contributor.py | 2 +- api/serializers/contributor_test.py | 2 +- .../_model_view_set.py} | 2 +- .../_model_view_set_client.py} | 36 ++-------------- api/views/_model_view_set_test_case.py | 42 +++++++++++++++++++ api/views/agreement_signature.py | 2 +- api/views/agreement_signature_test.py | 2 +- api/views/contributor.py | 2 +- api/views/contributor_test.py | 2 +- 16 files changed, 111 insertions(+), 75 deletions(-) delete mode 100644 api/common/__init__.py rename api/{common/model_serializer.py => serializers/_model_list_serializer.py} (65%) rename api/{common/model_serializer_test_case.py => serializers/_model_list_serializer_test_case.py} (64%) create mode 100644 api/serializers/_model_serializer.py create mode 100644 api/serializers/_model_serializer_test_case.py rename api/{common/model_view_set.py => views/_model_view_set.py} (91%) rename api/{common/model_view_set_test_case.py => views/_model_view_set_client.py} (86%) create mode 100644 api/views/_model_view_set_test_case.py diff --git a/api/common/__init__.py b/api/common/__init__.py deleted file mode 100644 index 86ae357..0000000 --- a/api/common/__init__.py +++ /dev/null @@ -1,9 +0,0 @@ -""" -© Ocado Group -Created on 13/09/2024 at 12:00:14(+03:00). -""" - -from .model_serializer import ModelSerializer -from .model_serializer_test_case import ModelSerializerTestCase -from .model_view_set import ModelViewSet -from .model_view_set_test_case import ModelViewSetTestCase diff --git a/api/common/model_serializer.py b/api/serializers/_model_list_serializer.py similarity index 65% rename from api/common/model_serializer.py rename to api/serializers/_model_list_serializer.py index 4462c46..d7bdcec 100644 --- a/api/common/model_serializer.py +++ b/api/serializers/_model_list_serializer.py @@ -6,7 +6,7 @@ import typing as t from codeforlife.request import BaseRequest -from codeforlife.serializers import BaseModelListSerializer, BaseModelSerializer +from codeforlife.serializers import BaseModelListSerializer from django.db.models import Model from ..models import Contributor @@ -15,13 +15,6 @@ AnyModel = t.TypeVar("AnyModel", bound=Model) -class ModelSerializer( - BaseModelSerializer[BaseRequest[SessionStore, Contributor], AnyModel], - t.Generic[AnyModel], -): - """Base model serializer.""" - - class ModelListSerializer( BaseModelListSerializer[BaseRequest[SessionStore, Contributor], AnyModel], t.Generic[AnyModel], diff --git a/api/common/model_serializer_test_case.py b/api/serializers/_model_list_serializer_test_case.py similarity index 64% rename from api/common/model_serializer_test_case.py rename to api/serializers/_model_list_serializer_test_case.py index addcddb..2d007dc 100644 --- a/api/common/model_serializer_test_case.py +++ b/api/serializers/_model_list_serializer_test_case.py @@ -9,32 +9,17 @@ from codeforlife.tests import ( BaseAPIRequestFactory, BaseModelListSerializerTestCase, - BaseModelSerializerTestCase, ) from django.db.models import Model from ..models import Contributor from ..models.session import SessionStore -from .model_serializer import ModelListSerializer, ModelSerializer +from ._model_list_serializer import ModelListSerializer +from ._model_serializer import ModelSerializer AnyModel = t.TypeVar("AnyModel", bound=Model) -class ModelSerializerTestCase( - BaseModelSerializerTestCase[ - ModelSerializer[AnyModel], - BaseAPIRequestFactory[ - BaseRequest[SessionStore, Contributor], Contributor - ], - AnyModel, - ], - t.Generic[AnyModel], -): - """Base model serializer test case.""" - - request_factory_class = BaseAPIRequestFactory - - # pylint: disable-next=too-many-ancestors class ModelListSerializerTestCase( BaseModelListSerializerTestCase[ diff --git a/api/serializers/_model_serializer.py b/api/serializers/_model_serializer.py new file mode 100644 index 0000000..e51628c --- /dev/null +++ b/api/serializers/_model_serializer.py @@ -0,0 +1,22 @@ +""" +© Ocado Group +Created on 13/09/2024 at 12:00:41(+03:00). +""" + +import typing as t + +from codeforlife.request import BaseRequest +from codeforlife.serializers import BaseModelSerializer +from django.db.models import Model + +from ..models import Contributor +from ..models.session import SessionStore + +AnyModel = t.TypeVar("AnyModel", bound=Model) + + +class ModelSerializer( + BaseModelSerializer[BaseRequest[SessionStore, Contributor], AnyModel], + t.Generic[AnyModel], +): + """Base model serializer.""" diff --git a/api/serializers/_model_serializer_test_case.py b/api/serializers/_model_serializer_test_case.py new file mode 100644 index 0000000..0e2362f --- /dev/null +++ b/api/serializers/_model_serializer_test_case.py @@ -0,0 +1,31 @@ +""" +© Ocado Group +Created on 13/09/2024 at 12:00:34(+03:00). +""" + +import typing as t + +from codeforlife.request import BaseRequest +from codeforlife.tests import BaseAPIRequestFactory, BaseModelSerializerTestCase +from django.db.models import Model + +from ..models import Contributor +from ..models.session import SessionStore +from ._model_serializer import ModelSerializer + +AnyModel = t.TypeVar("AnyModel", bound=Model) + + +class ModelSerializerTestCase( + BaseModelSerializerTestCase[ + ModelSerializer[AnyModel], + BaseAPIRequestFactory[ + BaseRequest[SessionStore, Contributor], Contributor + ], + AnyModel, + ], + t.Generic[AnyModel], +): + """Base model serializer test case.""" + + request_factory_class = BaseAPIRequestFactory diff --git a/api/serializers/agreement_signature.py b/api/serializers/agreement_signature.py index 1bfc02c..746b0ad 100644 --- a/api/serializers/agreement_signature.py +++ b/api/serializers/agreement_signature.py @@ -8,8 +8,8 @@ from django.utils import timezone from rest_framework import serializers -from ..common import ModelSerializer from ..models import AgreementSignature +from ._model_serializer import ModelSerializer # pylint: disable=missing-class-docstring # pylint: disable=missing-function-docstring diff --git a/api/serializers/agreement_signature_test.py b/api/serializers/agreement_signature_test.py index 34f9fca..cdedc58 100644 --- a/api/serializers/agreement_signature_test.py +++ b/api/serializers/agreement_signature_test.py @@ -12,8 +12,8 @@ from django.utils import timezone from rest_framework import status -from ..common import ModelSerializerTestCase from ..models import AgreementSignature, Contributor +from ._model_serializer_test_case import ModelSerializerTestCase from .agreement_signature import AgreementSignatureSerializer diff --git a/api/serializers/contributor.py b/api/serializers/contributor.py index 01a6b16..0740603 100644 --- a/api/serializers/contributor.py +++ b/api/serializers/contributor.py @@ -5,8 +5,8 @@ import typing as t -from ..common import ModelSerializer from ..models import Contributor +from ._model_serializer import ModelSerializer # pylint: disable-next=missing-class-docstring,too-many-ancestors diff --git a/api/serializers/contributor_test.py b/api/serializers/contributor_test.py index 13475e1..07377e8 100644 --- a/api/serializers/contributor_test.py +++ b/api/serializers/contributor_test.py @@ -3,8 +3,8 @@ Created on 12/07/2024 at 11:36:23(+01:00). """ -from ..common import ModelSerializerTestCase from ..models import Contributor +from ._model_serializer_test_case import ModelSerializerTestCase from .contributor import ContributorSerializer diff --git a/api/common/model_view_set.py b/api/views/_model_view_set.py similarity index 91% rename from api/common/model_view_set.py rename to api/views/_model_view_set.py index 1587b6d..0ff7d47 100644 --- a/api/common/model_view_set.py +++ b/api/views/_model_view_set.py @@ -15,7 +15,7 @@ AnyModel = t.TypeVar("AnyModel", bound=Model) if t.TYPE_CHECKING: # pragma: no cover - from .model_serializer import ModelSerializer + from ..serializers._model_serializer import ModelSerializer # pylint: disable-next=too-many-ancestors diff --git a/api/common/model_view_set_test_case.py b/api/views/_model_view_set_client.py similarity index 86% rename from api/common/model_view_set_test_case.py rename to api/views/_model_view_set_client.py index ab44eb8..e2d78aa 100644 --- a/api/common/model_view_set_test_case.py +++ b/api/views/_model_view_set_client.py @@ -11,11 +11,7 @@ import requests from codeforlife.request import BaseRequest -from codeforlife.tests import ( - BaseAPIRequestFactory, - BaseModelViewSetClient, - BaseModelViewSetTestCase, -) +from codeforlife.tests import BaseAPIRequestFactory, BaseModelViewSetClient from django.conf import settings from django.db.models import Model from rest_framework import status @@ -23,10 +19,12 @@ from ..models import Contributor from ..models.session import SessionStore -from .model_view_set import ModelViewSet AnyModel = t.TypeVar("AnyModel", bound=Model) +if t.TYPE_CHECKING: # pragma: no cover + from ._model_view_set_test_case import ModelViewSetTestCase + # pylint: disable-next=too-many-ancestors,arguments-differ class ModelViewSetClient( @@ -146,29 +144,3 @@ def login( # type: ignore[override] ) return Contributor.objects.get(session=self.session.session_key) - - -# pylint: disable-next=too-many-ancestors -class ModelViewSetTestCase( - BaseModelViewSetTestCase[ - ModelViewSet[AnyModel], - ModelViewSetClient[AnyModel], - AnyModel, - ], - t.Generic[AnyModel], -): - """Base model view set test case.""" - - client_class = ModelViewSetClient - - @classmethod - def get_model_class(cls) -> t.Type[AnyModel]: - """Get the model view set's class. - - Returns: - The model view set's class. - """ - # pylint: disable-next=no-member - return t.get_args(cls.__orig_bases__[0])[ # type: ignore[attr-defined] - 0 - ] diff --git a/api/views/_model_view_set_test_case.py b/api/views/_model_view_set_test_case.py new file mode 100644 index 0000000..375a8b2 --- /dev/null +++ b/api/views/_model_view_set_test_case.py @@ -0,0 +1,42 @@ +""" +© Ocado Group +Created on 13/09/2024 at 12:00:50(+03:00). +""" + +# pylint: disable=duplicate-code + +import typing as t + +from codeforlife.tests import BaseModelViewSetTestCase +from django.db.models import Model + +from ._model_view_set import ModelViewSet +from ._model_view_set_client import ModelViewSetClient + +AnyModel = t.TypeVar("AnyModel", bound=Model) + + +# pylint: disable-next=too-many-ancestors +class ModelViewSetTestCase( + BaseModelViewSetTestCase[ + ModelViewSet[AnyModel], + ModelViewSetClient[AnyModel], + AnyModel, + ], + t.Generic[AnyModel], +): + """Base model view set test case.""" + + client_class = ModelViewSetClient + + @classmethod + def get_model_class(cls) -> t.Type[AnyModel]: + """Get the model view set's class. + + Returns: + The model view set's class. + """ + # pylint: disable-next=no-member + return t.get_args(cls.__orig_bases__[0])[ # type: ignore[attr-defined] + 0 + ] diff --git a/api/views/agreement_signature.py b/api/views/agreement_signature.py index 82f9dd3..7378b72 100644 --- a/api/views/agreement_signature.py +++ b/api/views/agreement_signature.py @@ -6,9 +6,9 @@ from codeforlife.response import Response from codeforlife.views import action -from ..common import ModelViewSet from ..models import AgreementSignature from ..serializers import AgreementSignatureSerializer +from ._model_view_set import ModelViewSet # pylint: disable-next=missing-class-docstring,too-many-ancestors diff --git a/api/views/agreement_signature_test.py b/api/views/agreement_signature_test.py index 8ee7873..2317da6 100644 --- a/api/views/agreement_signature_test.py +++ b/api/views/agreement_signature_test.py @@ -11,8 +11,8 @@ from django.utils import timezone from rest_framework import status -from ..common import ModelViewSetTestCase from ..models import AgreementSignature, Contributor +from ._model_view_set_test_case import ModelViewSetTestCase from .agreement_signature import AgreementSignatureViewSet diff --git a/api/views/contributor.py b/api/views/contributor.py index 9baf444..d8d1203 100644 --- a/api/views/contributor.py +++ b/api/views/contributor.py @@ -5,9 +5,9 @@ from codeforlife.permissions import AllowAny -from ..common import ModelViewSet from ..models import Contributor from ..serializers import ContributorSerializer +from ._model_view_set import ModelViewSet # pylint: disable-next=missing-class-docstring,too-many-ancestors diff --git a/api/views/contributor_test.py b/api/views/contributor_test.py index 99f0b35..a745051 100644 --- a/api/views/contributor_test.py +++ b/api/views/contributor_test.py @@ -3,8 +3,8 @@ Created on 16/07/2024 at 14:54:09(+01:00). """ -from ..common import ModelViewSetTestCase from ..models import Contributor +from ._model_view_set_test_case import ModelViewSetTestCase from .contributor import ContributorViewSet From f9cdb29e5dfaefc5b1385c50bc7c14b15e5ebb84 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 14:51:50 +0000 Subject: [PATCH 18/21] new cfl package --- Pipfile.lock | 4 ++-- api/serializers/_model_list_serializer.py | 9 ++++++++- api/serializers/_model_list_serializer_test_case.py | 2 ++ api/serializers/_model_serializer.py | 9 ++++++++- api/serializers/_model_serializer_test_case.py | 2 ++ api/views/_model_view_set.py | 9 ++++++--- 6 files changed, 28 insertions(+), 7 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 3cfa68e..3a99635 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "687d58797d7e46c6fb039c02172b90fada557771" + "ref": "623b1a2a334a5f60cd0f516fe7633fc2a28d3345" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "687d58797d7e46c6fb039c02172b90fada557771" + "ref": "623b1a2a334a5f60cd0f516fe7633fc2a28d3345" }, "codeforlife-portal": { "hashes": [ diff --git a/api/serializers/_model_list_serializer.py b/api/serializers/_model_list_serializer.py index d7bdcec..09c1bef 100644 --- a/api/serializers/_model_list_serializer.py +++ b/api/serializers/_model_list_serializer.py @@ -14,9 +14,16 @@ AnyModel = t.TypeVar("AnyModel", bound=Model) +if t.TYPE_CHECKING: # pragma: no cover + from ..views._model_view_set import ModelViewSet + class ModelListSerializer( - BaseModelListSerializer[BaseRequest[SessionStore, Contributor], AnyModel], + BaseModelListSerializer[ + BaseRequest[SessionStore, Contributor], + "ModelViewSet[AnyModel]", + AnyModel, + ], t.Generic[AnyModel], ): """Base model list serializer.""" diff --git a/api/serializers/_model_list_serializer_test_case.py b/api/serializers/_model_list_serializer_test_case.py index 2d007dc..3a36ea4 100644 --- a/api/serializers/_model_list_serializer_test_case.py +++ b/api/serializers/_model_list_serializer_test_case.py @@ -22,6 +22,7 @@ # pylint: disable-next=too-many-ancestors class ModelListSerializerTestCase( + # pylint: disable=duplicate-code BaseModelListSerializerTestCase[ ModelListSerializer[AnyModel], ModelSerializer[AnyModel], @@ -31,6 +32,7 @@ class ModelListSerializerTestCase( AnyModel, ], t.Generic[AnyModel], + # pylint: enable=duplicate-code ): """Base model serializer test case.""" diff --git a/api/serializers/_model_serializer.py b/api/serializers/_model_serializer.py index e51628c..e122653 100644 --- a/api/serializers/_model_serializer.py +++ b/api/serializers/_model_serializer.py @@ -14,9 +14,16 @@ AnyModel = t.TypeVar("AnyModel", bound=Model) +if t.TYPE_CHECKING: # pragma: no cover + from ..views._model_view_set import ModelViewSet + class ModelSerializer( - BaseModelSerializer[BaseRequest[SessionStore, Contributor], AnyModel], + BaseModelSerializer[ + BaseRequest[SessionStore, Contributor], + "ModelViewSet[AnyModel]", + AnyModel, + ], t.Generic[AnyModel], ): """Base model serializer.""" diff --git a/api/serializers/_model_serializer_test_case.py b/api/serializers/_model_serializer_test_case.py index 0e2362f..0fe905e 100644 --- a/api/serializers/_model_serializer_test_case.py +++ b/api/serializers/_model_serializer_test_case.py @@ -17,6 +17,7 @@ class ModelSerializerTestCase( + # pylint: disable=duplicate-code BaseModelSerializerTestCase[ ModelSerializer[AnyModel], BaseAPIRequestFactory[ @@ -25,6 +26,7 @@ class ModelSerializerTestCase( AnyModel, ], t.Generic[AnyModel], + # pylint: enable=duplicate-code ): """Base model serializer test case.""" diff --git a/api/views/_model_view_set.py b/api/views/_model_view_set.py index 0ff7d47..24bbdab 100644 --- a/api/views/_model_view_set.py +++ b/api/views/_model_view_set.py @@ -20,10 +20,13 @@ # pylint: disable-next=too-many-ancestors class ModelViewSet( - BaseModelViewSet[BaseRequest[SessionStore, Contributor], AnyModel], + BaseModelViewSet[ + BaseRequest[SessionStore, Contributor], + "ModelSerializer[AnyModel]", + AnyModel, + ], t.Generic[AnyModel], ): """Base model view set.""" - request_class = BaseRequest[SessionStore, Contributor] - serializer_class: t.Optional[t.Type["ModelSerializer[AnyModel]"]] + request_class = BaseRequest From 0c8bd76aa15432f4cf68088ec7298d5209898e9b Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 15:30:06 +0000 Subject: [PATCH 19/21] abstract login view and form --- Pipfile.lock | 4 +-- api/forms/session.py | 50 +++----------------------------- api/views/session.py | 68 ++++---------------------------------------- 3 files changed, 12 insertions(+), 110 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index 3a99635..c97a45b 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "623b1a2a334a5f60cd0f516fe7633fc2a28d3345" + "ref": "9fc68d3c8a699f7edae8ae3e88b480e54ac5787f" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "623b1a2a334a5f60cd0f516fe7633fc2a28d3345" + "ref": "9fc68d3c8a699f7edae8ae3e88b480e54ac5787f" }, "codeforlife-portal": { "hashes": [ diff --git a/api/forms/session.py b/api/forms/session.py index 1486aa4..abeb2ac 100644 --- a/api/forms/session.py +++ b/api/forms/session.py @@ -3,58 +3,16 @@ Created on 05/08/2024 at 15:54:51(+01:00). """ +from codeforlife.forms import BaseLoginForm from django import forms -from django.contrib.auth import authenticate -from django.core.exceptions import ValidationError -from django.core.handlers.wsgi import WSGIRequest from ..models import Contributor -class GitHubLoginForm(forms.Form): +class GitHubLoginForm(BaseLoginForm[Contributor]): """Login with an existing github account.""" - contributor: Contributor - code = forms.CharField(required=True) - def __init__(self, request: WSGIRequest, *args, **kwargs): - self.request = request - super().__init__(*args, **kwargs) - - def clean(self): - """Authenticates a contributor. - - Raises: - ValidationError: If there are form errors. - ValidationError: If the contributor's credentials were incorrect. - ValidationError: If the contributor's instance is incorrect. - - Returns: - The cleaned form data. - """ - if self.errors: - raise ValidationError( - "Found form errors. Skipping authentication.", - code="form_errors", - ) - - # Needs mocking. Must return a valid Contributor - contributor = authenticate( - self.request, - **{key: self.cleaned_data[key] for key in self.fields.keys()} - ) - if contributor is None: - raise ValidationError( - "The code returned was invalid or expired.", - code="invalid_login", - ) - - if not isinstance(contributor, Contributor): - raise ValidationError( - "Incorrect contributor class.", - code="incorrect_contributor_class", - ) - - self.contributor = contributor - return self.cleaned_data + def get_invalid_login_error_message(self): + return "The code returned was invalid or expired." diff --git a/api/views/session.py b/api/views/session.py index b8aef56..c2fbd19 100644 --- a/api/views/session.py +++ b/api/views/session.py @@ -3,77 +3,21 @@ Created on 05/08/2024 at 17:23:01(+01:00). """ -import json -import typing as t -from urllib.parse import quote_plus - from codeforlife.request import BaseHttpRequest -from django.conf import settings -from django.contrib.auth import login -from django.contrib.auth.views import LoginView as _LoginView -from django.http import JsonResponse -from rest_framework import status +from codeforlife.views import BaseLoginView from ..forms import GitHubLoginForm from ..models import Contributor from ..models.session import SessionStore -class LoginView(_LoginView): +class LoginView( + BaseLoginView[BaseHttpRequest[SessionStore, Contributor], Contributor] +): """Login users with their existing github accounts.""" - request: BaseHttpRequest[SessionStore, Contributor] - def get_form_class(self): return GitHubLoginForm - def get_form_kwargs(self): - form_kwargs = super().get_form_kwargs() - form_kwargs["data"] = json.loads(self.request.body) - - return form_kwargs - - def form_valid(self, form: GitHubLoginForm): # type: ignore - contributor = form.contributor - - # pylint: disable-next=line-too-long - self.request.session.clear_expired( - user_id=contributor.pk - ) # type: ignore - - login(self.request, contributor) # type: ignore - - self.request.session.save() - - # Get session metadata. - session_metadata = {"contributor_id": contributor.id} - - # Return session metadata in response and a non-HTTP-only cookie. - response = JsonResponse(session_metadata) - response.set_cookie( - key=settings.SESSION_METADATA_COOKIE_NAME, - value=quote_plus( - json.dumps( - session_metadata, - separators=(",", ":"), - indent=None, - ) - ), - max_age=( - None - if settings.SESSION_EXPIRE_AT_BROWSER_CLOSE - else settings.SESSION_COOKIE_AGE - ), - secure=settings.SESSION_COOKIE_SECURE, - samesite=t.cast( - t.Optional[t.Literal["Lax", "Strict", "None", False]], - settings.SESSION_COOKIE_SAMESITE, - ), - domain=settings.SESSION_COOKIE_DOMAIN, - httponly=False, - ) - - return response - - def form_invalid(self, form: GitHubLoginForm): # type: ignore - return JsonResponse(form.errors, status=status.HTTP_400_BAD_REQUEST) + def get_session_metadata(self, user): + return {"contributor_id": user.id} From 54aebf72007b2c0467cf18ce99a16508251c825b Mon Sep 17 00:00:00 2001 From: SKairinos Date: Thu, 7 Nov 2024 17:31:58 +0000 Subject: [PATCH 20/21] new cfl package --- Pipfile.lock | 4 ++-- api/views/_model_view_set_test_case.py | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Pipfile.lock b/Pipfile.lock index c97a45b..2639ae2 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "9fc68d3c8a699f7edae8ae3e88b480e54ac5787f" + "ref": "655604e6c749ace4866e12abd76d97962f017e66" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "9fc68d3c8a699f7edae8ae3e88b480e54ac5787f" + "ref": "655604e6c749ace4866e12abd76d97962f017e66" }, "codeforlife-portal": { "hashes": [ diff --git a/api/views/_model_view_set_test_case.py b/api/views/_model_view_set_test_case.py index 375a8b2..423e2a5 100644 --- a/api/views/_model_view_set_test_case.py +++ b/api/views/_model_view_set_test_case.py @@ -8,6 +8,7 @@ import typing as t from codeforlife.tests import BaseModelViewSetTestCase +from codeforlife.types import get_arg from django.db.models import Model from ._model_view_set import ModelViewSet @@ -36,7 +37,4 @@ def get_model_class(cls) -> t.Type[AnyModel]: Returns: The model view set's class. """ - # pylint: disable-next=no-member - return t.get_args(cls.__orig_bases__[0])[ # type: ignore[attr-defined] - 0 - ] + return get_arg(cls, 0) From e03ffbc9be888583e9d63d106b5c418ae8680014 Mon Sep 17 00:00:00 2001 From: SKairinos Date: Wed, 13 Nov 2024 12:02:19 +0000 Subject: [PATCH 21/21] new cfl package --- Pipfile | 4 ++-- Pipfile.lock | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Pipfile b/Pipfile index e3febec..f1be819 100644 --- a/Pipfile +++ b/Pipfile @@ -22,11 +22,11 @@ name = "pypi" # 5. Run `pipenv install --dev` in your terminal. [packages] -codeforlife = {ref = "contributor-backend-21", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"} +codeforlife = {ref = "v0.21.0", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"} # 🚫 Don't add [packages] below that are inherited from the CFL package. [dev-packages] -codeforlife = {ref = "contributor-backend-21", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]} +codeforlife = {ref = "v0.21.0", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]} # codeforlife = {file = "../codeforlife-package-python", editable = true, extras = ["dev"]} # 🚫 Don't add [dev-packages] below that are inherited from the CFL package. diff --git a/Pipfile.lock b/Pipfile.lock index 2639ae2..8d19176 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "f90d8b2e3f7f6dfe78ee5846bbb16f47d92f1fbfa5aa4bef4964873f8b55acad" + "sha256": "288e98756c46420cdb42e60bd7f01373cae1806ba2f47635bcdd49a05d2f08d2" }, "pipfile-spec": 6, "requires": { @@ -160,7 +160,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "655604e6c749ace4866e12abd76d97962f017e66" + "ref": "8ccf3d86cf3ea7826567dc654120a4f27a1f10b1" }, "codeforlife-portal": { "hashes": [ @@ -1095,7 +1095,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "655604e6c749ace4866e12abd76d97962f017e66" + "ref": "8ccf3d86cf3ea7826567dc654120a4f27a1f10b1" }, "codeforlife-portal": { "hashes": [