Polish pylint (#321)

* fix: linting errors

* fix linting

* add todo to fix broken test

* fix bulk create endpoint

* add github actions extension

* install new cfl py package

.devcontainer.json

@@ -18,7 +18,8 @@
         "ms-python.black-formatter",
         "qwtel.sqlite-viewer",
         "njpwerner.autodocstring",
-        "tamasfe.even-better-toml"
+        "tamasfe.even-better-toml",
+        "github.vscode-github-actions"
       ]
     }
   },

backend/Pipfile

@@ -22,7 +22,7 @@ name = "pypi"
 # 5. Run `pipenv install --dev` in your terminal.
 
 [packages]
-codeforlife = {ref = "v0.16.0", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"}
+codeforlife = {ref = "v0.16.1", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"}
 # 🚫 Don't add [packages] below that are inhertited from the CFL package.
 # TODO: check if we need the below packages
 whitenoise = "==6.5.0"
@@ -47,7 +47,7 @@ google-cloud-container = "==2.3.0"
 # "django-anymail[amazon_ses]" = "==7.0.*"
 
 [dev-packages]
-codeforlife = {ref = "v0.16.0", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]}
+codeforlife = {ref = "v0.16.1", 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 inhertited from the CFL package.
 # TODO: check if we need the below packages

backend/api/apps.py

@@ -6,6 +6,7 @@
 from django.apps import AppConfig
 
 
+# pylint: disable-next=missing-class-docstring
 class ApiConfig(AppConfig):
     default_auto_field = "django.db.models.BigAutoField"
     name = "api"

backend/api/serializers/school_teacher_invitation.py

@@ -49,6 +49,7 @@ def create(self, validated_data):
         validated_data["expiry"] = timezone.now() + timedelta(days=30)
 
         invitation = super().create(validated_data)
+        # pylint: disable-next=protected-access
         invitation._token = token
         return invitation
 

backend/api/serializers/student_test.py

@@ -29,6 +29,7 @@
 )
 
 # pylint: disable=missing-class-docstring
+# pylint: disable=too-many-ancestors
 
 
 # ------------------------------------------------------------------------------

backend/api/serializers/user.py

@@ -115,10 +115,12 @@ class Meta(_UserSerializer.Meta):
 
     def create(self, validated_data):
         add_to_newsletter: bool = validated_data.pop("add_to_newsletter")
+        # pylint: disable-next=unused-variable
         date_of_birth: date = validated_data.pop("date_of_birth")
 
         # TODO: Use date of birth in post email save signal to send
         #  appropriate verification email depending on age, cf
+        # pylint: disable-next=line-too-long
         #  https://github.com/ocadotechnology/codeforlife-portal/blob/master/portal/views/home.py#L192
 
         independent_user = IndependentUser.objects.create_user(**validated_data)
@@ -307,6 +309,7 @@ def create(self, validated_data: DataDict):
         user: User = validated_data["email"]
 
         # Generate reset-password url for the frontend.
+        # pylint: disable-next=unused-variable
         reset_password_url = "/".join(
             [
                 settings.SERVICE_BASE_URL,

backend/api/signals/school_teacher_invitation.py

@@ -18,5 +18,5 @@ def school_teacher_invitation__post_save(
 ):
     """Send invitation email to invited teacher."""
 
-    instance._token  # TODO: send email to invited teacher with differing
+    # instance._token  TODO: send email to invited teacher with differing
     # content based on whether the email is already linked to an account or not.

backend/api/signals/school_teacher_invitation_test.py

@@ -6,6 +6,7 @@
 from django.test import TestCase
 
 
+# pylint: disable-next=missing-class-docstring
 class TestSchoolTeacherInvitation(TestCase):
     def test_post_save(self):
         """Creating a teacher invitation sends a verification email."""

backend/api/signals/user_test.py

@@ -7,6 +7,7 @@
 from django.test import TestCase
 
 
+# pylint: disable-next=missing-class-docstring
 class TestUser(TestCase):
     def test_pre_save__otp_secret(self):
         """

backend/api/views/cron/test_user.py

@@ -1,3 +1,5 @@
+# pylint: skip-file
+
 # from datetime import timedelta
 # from unittest.mock import patch, Mock, ANY
 

backend/api/views/klass_test.py

@@ -121,7 +121,8 @@ def test_create__self(self):
 
     def test_create__other(self):
         """
-        Admin-teacher can create a class with another teacher as the class owner.
+        Admin-teacher can create a class with another teacher as the class
+        owner.
         """
         user = self.admin_school_teacher_user
         teacher = (

backend/api/views/otp_bypass_token.py

@@ -25,18 +25,16 @@ def get_permissions(self):
 
         return [IsTeacher()]
 
+    # TODO: replace this custom action with bulk create and list serializer.
     @action(detail=False, methods=["post"])
     def generate(self, request: Request):
         """Generates some OTP bypass tokens for a user."""
-
-        user = request.auth_user
-
-        OtpBypassToken.objects.filter(user=user).delete()
-
-        tokens = OtpBypassToken.generate_tokens()
-
-        OtpBypassToken.objects.bulk_create(
-            [OtpBypassToken(user=user, token=token) for token in tokens]
+        otp_bypass_tokens = OtpBypassToken.objects.bulk_create(
+            request.auth_user
         )
 
-        return Response(tokens, status.HTTP_201_CREATED)
+        return Response(
+            # pylint: disable-next=protected-access
+            [otp_bypass_token._token for otp_bypass_token in otp_bypass_tokens],
+            status.HTTP_201_CREATED,
+        )

backend/api/views/otp_bypass_token_test.py

@@ -3,7 +3,7 @@
 Created on 24/01/2024 at 09:47:04(+00:00).
 """
 
-from unittest.mock import patch
+from unittest.mock import call, patch
 
 from codeforlife.tests import ModelViewSetTestCase
 from codeforlife.user.models import OtpBypassToken, User
@@ -12,32 +12,24 @@
 from .otp_bypass_token import OtpBypassTokenViewSet
 
 
-# pylint: disable-next=missing-class-docstring
+# pylint: disable-next=missing-class-docstring,too-many-ancestors
 class TestOtpBypassTokenViewSet(ModelViewSetTestCase[User, OtpBypassToken]):
     basename = "otp-bypass-token"
     model_view_set_class = OtpBypassTokenViewSet
+    fixtures = ["school_2"]
 
     def setUp(self):
-        self.user = User.objects.get(email="[email protected]")
-        assert not self.user.otp_bypass_tokens.exists()
-
-        self.otp_bypass_tokens = OtpBypassToken.objects.bulk_create(
-            [
-                OtpBypassToken(user=self.user, token=token)
-                for token in OtpBypassToken.generate_tokens()
-            ]
-        )
+        user = User.objects.filter(otp_bypass_tokens__isnull=False).first()
+        assert user
+        self.user = user
 
     def test_generate(self):
-        """
-        Generate max number of OTP bypass tokens.
-        """
-
-        user = self.client.login_teacher(
-            email="[email protected]",
-            password="Password1",
+        """Generate max number of OTP bypass tokens."""
+        otp_bypass_token_pks = list(
+            self.user.otp_bypass_tokens.values_list("pk", flat=True)
         )
-        assert user == self.user
+
+        self.client.login(email=self.user.email, password="password")
 
         tokens = {
             "aaaaaaaa",
@@ -51,34 +43,36 @@ def test_generate(self):
             "iiiiiiii",
             "jjjjjjjj",
         }
-        assert len(tokens) == OtpBypassToken.max_count
 
-        with patch.object(
-            OtpBypassToken, "generate_tokens", return_value=tokens
-        ) as generate_tokens:
+        with patch(
+            "codeforlife.user.models.otp_bypass_token.get_random_string",
+            side_effect=list(tokens),
+        ) as get_random_string:
             response = self.client.post(
                 self.reverse_action("generate"),
                 status_code_assertion=status.HTTP_201_CREATED,
             )
 
-            generate_tokens.assert_called_once()
+            get_random_string.assert_has_calls(
+                [
+                    call(
+                        OtpBypassToken.length,
+                        OtpBypassToken.allowed_chars,
+                    )
+                    for _ in range(len(tokens))
+                ]
+            )
 
         # We received the expected tokens.
         assert set(response.json()) == tokens
 
         # The user's pre-existing tokens were deleted.
-        assert (
-            OtpBypassToken.objects.filter(
-                pk__in=[
-                    otp_bypass_token.pk
-                    for otp_bypass_token in self.otp_bypass_tokens
-                ]
-            ).count()
-            == 0
-        )
+        assert not OtpBypassToken.objects.filter(
+            pk__in=otp_bypass_token_pks
+        ).exists()
 
         # The new tokens all check out.
-        for otp_bypass_token in OtpBypassToken.objects.filter(user=user):
+        for otp_bypass_token in self.user.otp_bypass_tokens.all():
             found_token = False
             for token in tokens.copy():
                 found_token = otp_bypass_token.check_token(token)

backend/api/views/school_teacher_invitation.py

@@ -54,7 +54,7 @@ def get_queryset(self):
     @action(
         detail=True, methods=["get", "post"], url_path="accept/(?P<token>.+)"
     )
-    def accept(self, request: Request, pk: str, token: str):
+    def accept(self, request: Request, token: str, **kwargs: str):
         """
         Handles accepting a teacher's invitation to join their school. On GET,
         checks validity of the invitation token. On PATCH, rechecks this

backend/api/views/school_teacher_invitation_test.py

@@ -13,7 +13,7 @@
 from .school_teacher_invitation import SchoolTeacherInvitationViewSet
 
 
-# pylint: disable-next=missing-class-docstring
+# pylint: disable-next=missing-class-docstring,too-many-ancestors
 class TestSchoolTeacherInvitationViewSet(
     ModelViewSetTestCase[User, SchoolTeacherInvitation]
 ):

backend/api/views/school_test.py

@@ -11,7 +11,7 @@
 from .school import SchoolViewSet
 
 
-# pylint: disable-next=missing-class-docstring
+# pylint: disable-next=missing-class-docstring,too-many-ancestors
 class TestSchoolViewSet(ModelViewSetTestCase[User, School]):
     basename = "school"
     model_view_set_class = SchoolViewSet

backend/api/views/user.py

@@ -81,20 +81,7 @@ def get_queryset(self, user_class=User):
         return queryset
 
     def destroy(self, request, *args, **kwargs):
-        user = self.get_object()
-        user.first_name = ""
-        user.last_name = ""
-        user.email = ""
-        user.is_active = False
-        user.save(
-            update_fields=[
-                "first_name",
-                "last_name",
-                "email",
-                "username",
-                "is_active",
-            ]
-        )
+        self.get_object().anonymize()
 
         return Response(status=status.HTTP_204_NO_CONTENT)
 

backend/api/views/user_test.py

@@ -349,7 +349,9 @@ def test_partial_update__teacher(self):
 
         other_school_teacher_user = (
             SchoolTeacherUser.objects.filter(
-                new_teacher__school=self.admin_school_1_teacher_user.teacher.school
+                new_teacher__school=(
+                    self.admin_school_1_teacher_user.teacher.school
+                )
             )
             .exclude(pk=self.admin_school_1_teacher_user.pk)
             .first()

backend/main.py

@@ -1,3 +1,9 @@
+"""
+© Ocado Group
+Created on 11/04/2024 at 16:51:45(+01:00).
+"""
+
 # This is the entrypoint to our app.
 # https://cloud.google.com/appengine/docs/standard/python3/runtime#application_startup
+# pylint: disable-next=unused-import
 from service.wsgi import application as app

backend/manage.py

@@ -8,6 +8,7 @@ def main():
     """Run administrative tasks."""
     os.environ.setdefault("DJANGO_SETTINGS_MODULE", "service.settings")
     try:
+        # pylint: disable-next=import-outside-toplevel
         from django.core.management import execute_from_command_line
     except ImportError as exc:
         raise ImportError(

backend/pyproject.toml

@@ -20,12 +20,13 @@ django_settings_module = "service.settings"
 
 [tool.pylint.main]
 init-hook = "import os, sys; sys.path.append(os.getcwd())"
+disable = ["fixme"]
 
 [tool.pylint.format]
 max-line-length = 80
 
 [tool.pylint.MASTER]
-ignore-paths = [".*/migrations/.*py"]
+ignore-paths = [".*/migrations/.*.py"]
 load-plugins = "pylint_django"
 django-settings-module = "service.settings"