fix: polish mypy (#110)

* fix: initial

* fix: delete unused files

* fix: make pylint happy

* fix: school 2 sessions fixture

* fix: add github actions extension

* fix: make migrations

* fix: test python code

* fix: output python version

* test

* add dummy step

* double set py version

* fix import sort

* fix: static type hints

* fix: add test clients

* merge from main

* fix: add todo

* fix: new portal version

* fix: lock file

* test new python setup

* add todo

* set back to main

.github/workflows/main.yml

@@ -10,54 +10,29 @@ on:
       - ".*"
   workflow_dispatch:
 
-env:
-  PYTHON_VERSION: 3.8
-
 jobs:
   test:
-    runs-on: ubuntu-latest
-    strategy:
-      fail-fast: false
-      matrix:
-        python-version: [3.8]
-    steps:
-      - name: 🛫 Checkout
-        uses: actions/checkout@v3
-
-      - name: 🐍 Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v4
-        with:
-          python-version: ${{ matrix.python-version }}
-
-      - name: 🛠 Install Dependencies
-        run: |
-          python -m pip install --upgrade pip
-          python -m pip install pipenv
-          pipenv install --dev
-
-      - name: 🔎 Check Code Format
-        run: if ! pipenv run black --check .; then exit 1; fi
-
-      - name: 🔎 Check Migrations
-        run: pipenv run python manage.py makemigrations --check --dry-run
-
-      # TODO: assert code coverage target.
-      - name: 🧪 Test Code Units
-        run: pipenv run pytest
+    uses: ocadotechnology/codeforlife-workspace/.github/workflows/test-python-code.yaml@main
+    with:
+      # Cannot be set with an env var. Value must match in the release job.
+      python-version: 3.8
 
   release:
     concurrency: release
     runs-on: ubuntu-latest
     needs: [test]
     if: github.ref_name == 'main'
+    env:
+      # Value must match in the test job.
+      PYTHON_VERSION: 3.8
     steps:
       - name: 🛫 Checkout
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
         with:
           token: ${{ secrets.CFL_BOT_GITHUB_TOKEN }}
           fetch-depth: 0
 
-      - name: 🐍 Set up Python
+      - name: 🐍 Set up Python ${{ env.PYTHON_VERSION }}
         uses: actions/setup-python@v4
         with:
           python-version: ${{ env.PYTHON_VERSION }}
@@ -67,10 +42,8 @@ jobs:
           python -m pip install --upgrade pip
           python -m pip install python-semantic-release~=7.33
 
-      - name: ⚙️ Configure Git
-        run: |
-          git config --local user.name cfl-bot
-          git config --local user.email [email protected]
+      - name: 🤖 Set up cfl-bot as Git User
+        uses: ocadotechnology/codeforlife-workspace/.github/actions/git/setup-bot@main
 
       - name: 🚀 Publish Semantic Release
         env:

Pipfile

@@ -17,8 +17,8 @@ importlib-metadata = "==4.13.0" # TODO: remove. needed by old portal
 django-formtools = "==2.2" # TODO: remove. needed by old portal
 django-otp = "==1.0.2" # TODO: remove. needed by old portal
 # https://pypi.org/user/codeforlife/
-cfl-common = "==6.41.5" # TODO: remove
-codeforlife-portal = "==6.41.5" # TODO: remove
+cfl-common = "==6.41.10" # TODO: remove
+codeforlife-portal = "==6.41.10" # TODO: remove
 aimmo = "==2.11.2" # TODO: remove
 rapid-router = "==5.16.21" # TODO: remove
 phonenumbers = "==8.12.12" # TODO: remove

codeforlife/models/signals/pre_save.py

@@ -48,7 +48,8 @@ def get_previous_value(field: str):
             return None
 
     else:
-        previous_instance = instance.__class__.objects.get(pk=instance.pk)
+        objects = instance.__class__.objects  # type: ignore[attr-defined]
+        previous_instance = objects.get(pk=instance.pk)
 
         def get_previous_value(field: str):
             return getattr(previous_instance, field)

codeforlife/tests/__init__.py

@@ -13,5 +13,5 @@
     ModelListSerializerTestCase,
     ModelSerializerTestCase,
 )
-from .model_view_set import ModelViewSetTestCase
-from .test import TestCase
+from .model_view_set import ModelViewSetClient, ModelViewSetTestCase
+from .test import Client, TestCase

codeforlife/tests/api.py

@@ -125,7 +125,9 @@ def _login_user_type(self, user_type: t.Type[LoginUser], **credentials):
             otp = user.totp.at(now)
             with patch.object(timezone, "now", return_value=now):
                 assert super().login(
-                    request=self.request_factory.post(user=user),
+                    request=self.request_factory.post(
+                        user=t.cast(RequestUser, user)
+                    ),
                     otp=otp,
                 ), f'Failed to login with OTP "{otp}" at {now}.'
 

codeforlife/tests/model.py

@@ -6,6 +6,7 @@
 import typing as t
 from unittest.case import _AssertRaisesContext
 
+from django.core.exceptions import ObjectDoesNotExist
 from django.db.models import Model
 from django.db.utils import IntegrityError
 
@@ -76,8 +77,9 @@ def assert_does_not_exist(self, model_or_pk: t.Union[AnyModel, t.Any]):
         """
 
         model_class = self.get_model_class()
-        with self.assertRaises(model_class.DoesNotExist):
+        with self.assertRaises(ObjectDoesNotExist):
             if isinstance(model_or_pk, Model):
                 model_or_pk.refresh_from_db()
             else:
-                model_class.objects.get(pk=model_or_pk)
+                objects = model_class.objects  # type: ignore[attr-defined]
+                objects.get(pk=model_or_pk)

codeforlife/tests/model_view_set.py

@@ -8,6 +8,7 @@
 import typing as t
 from datetime import datetime
 
+from django.core.exceptions import ObjectDoesNotExist
 from django.db.models import Model
 from django.db.models.query import QuerySet
 from django.urls import reverse
@@ -19,7 +20,7 @@
 from ..serializers import BaseSerializer
 from ..types import DataDict, JsonDict, KwArgs
 from ..user.models import AnyUser as RequestUser
-from ..user.models import User
+from ..user.models import Class, Student, User
 from ..views import ModelViewSet
 from .api import APIClient, APITestCase
 
@@ -681,8 +682,13 @@ def assert_serialized_model_equals_json_model(
         """
         # Get the logged-in user.
         try:
-            user = User.objects.get(session=self.client.session.session_key)
-        except User.DoesNotExist:
+            user = t.cast(
+                RequestUser,
+                self.get_request_user_class().objects.get(
+                    session=self.client.session.session_key
+                ),
+            )
+        except ObjectDoesNotExist:
             user = None  # NOTE: no user has logged in.
 
         # Create an instance of the model view set and serializer.
@@ -844,7 +850,9 @@ def get_another_school_user(
         school = (
             user.teacher.school
             if user.teacher
-            else user.student.class_field.teacher.school
+            else t.cast(
+                Class, t.cast(Student, user.student).class_field
+            ).teacher.school
         )
         assert school
 
@@ -882,12 +890,14 @@ def get_another_school_user(
                     )
                 # Else, both users are students.
                 else:
+                    klass = t.cast(
+                        Class, t.cast(Student, user.student).class_field
+                    )
+
                     assert (
-                        user.student.class_field
-                        == other_user.student.class_field
+                        klass == other_user.student.class_field
                         if same_class
-                        else user.student.class_field
-                        != other_user.student.class_field
+                        else klass != other_user.student.class_field
                     )
         else:
             assert school != other_school

codeforlife/tests/test.py

@@ -3,12 +3,40 @@
 Created on 10/04/2024 at 13:03:00(+01:00).
 """
 
+import typing as t
 from unittest.case import _AssertRaisesContext
 
 from django.core.exceptions import ValidationError
+from django.http import HttpResponse
+from django.test import Client as _Client
 from django.test import TestCase as _TestCase
 
 
+class Client(_Client):
+    """A Django client with type hints."""
+
+    def generic(self, *args, **kwargs):
+        return t.cast(HttpResponse, super().generic(*args, **kwargs))
+
+    def get(self, *args, **kwargs):
+        return t.cast(HttpResponse, super().get(*args, **kwargs))
+
+    def post(self, *args, **kwargs):
+        return t.cast(HttpResponse, super().post(*args, **kwargs))
+
+    def put(self, *args, **kwargs):
+        return t.cast(HttpResponse, super().put(*args, **kwargs))
+
+    def patch(self, *args, **kwargs):
+        return t.cast(HttpResponse, super().patch(*args, **kwargs))
+
+    def delete(self, *args, **kwargs):
+        return t.cast(HttpResponse, super().delete(*args, **kwargs))
+
+    def options(self, *args, **kwargs):
+        return t.cast(HttpResponse, super().options(*args, **kwargs))
+
+
 class TestCase(_TestCase):
     """Base test case for all tests to inherit."""
 

codeforlife/urls.py

@@ -3,11 +3,13 @@
 Created on 12/04/2024 at 14:42:20(+01:00).
 """
 
+import typing as t
+
 from django.contrib import admin
 from django.contrib.auth.views import LogoutView
 from django.http import HttpResponse
 from django.shortcuts import render
-from django.urls import include, path, re_path
+from django.urls import URLPattern, URLResolver, include, path, re_path
 from rest_framework import status
 
 from .settings import SERVICE_IS_ROOT, SERVICE_NAME
@@ -31,7 +33,7 @@ def service_urlpatterns(
     """
 
     # Specific url patterns.
-    urlpatterns = [
+    urlpatterns: t.List[t.Union[URLResolver, URLPattern]] = [
         path(
             "admin/",
             admin.site.urls,

codeforlife/user/auth/backends/user_id_and_login_id.py

@@ -5,7 +5,12 @@
 
 import typing as t
 
-from common.helpers.generators import get_hashed_login_id
+# isort: off
+from common.helpers.generators import (  # type: ignore[import-untyped]
+    get_hashed_login_id,
+)
+
+# isort: on
 
 from ....request import HttpRequest
 from ...models import Student, StudentUser

codeforlife/user/auth/password_validators/__init__.py

@@ -3,6 +3,6 @@
 Created on 30/01/2024 at 12:28:00(+00:00).
 """
 
-from .student import StudentPasswordValidator
 from .independent import IndependentPasswordValidator
+from .student import StudentPasswordValidator
 from .teacher import TeacherPasswordValidator

codeforlife/user/filters/user.py

@@ -3,7 +3,9 @@
 Created on 03/04/2024 at 16:37:44(+01:00).
 """
 
-from django_filters import rest_framework as filters
+from django_filters import (  # type: ignore[import-untyped] # isort: skip
+    rest_framework as filters,
+)
 
 from ..models import User
 

codeforlife/user/models/klass.py

@@ -4,4 +4,4 @@
 """
 
 # pylint: disable-next=unused-import
-from common.models import Class
+from common.models import Class  # type: ignore[import-untyped]

codeforlife/user/models/school.py

@@ -4,4 +4,4 @@
 """
 
 # pylint: disable-next=unused-import
-from common.models import School
+from common.models import School  # type: ignore[import-untyped]

codeforlife/user/models/student.py

@@ -1,3 +1,5 @@
+# TODO: remove this in new system
+# mypy: disable-error-code="import-untyped"
 """
 © Ocado Group
 Created on 14/02/2024 at 17:16:44(+00:00).

codeforlife/user/models/teacher.py

@@ -1,3 +1,5 @@
+# TODO: remove this in new system
+# mypy: disable-error-code="import-untyped"
 """
 © Ocado Group
 Created on 05/02/2024 at 09:49:56(+00:00).

codeforlife/user/models/user.py

@@ -1,3 +1,5 @@
+# TODO: remove this in new system
+# mypy: disable-error-code="import-untyped"
 """
 © Ocado Group
 Created on 05/02/2024 at 09:50:04(+00:00).

setup.py

@@ -88,6 +88,7 @@ def parse_requirements(packages: t.Dict[str, t.Dict[str, t.Any]]):
     long_description=long_description,
     long_description_content_type="text/markdown",
     url="https://github.com/ocadotechnology/codeforlife-package-python",
+    # TODO: exclude test files
     packages=find_packages(exclude=["tests", "tests.*"]),
     include_package_data=True,
     data_files=[get_data_files(DATA_DIR), get_data_files(USER_FIXTURES_DIR)],