🚨(pylint) make pylint work and fix issues found

Pylint was not installed and wrongly configured. After making
it work, we fix all the issues found so it can be added to our
CI requirements.

bin/pylint

@@ -27,13 +27,12 @@ do
 done
 
 if [[ -n "${diff_from}" ]]; then
-    # Run pylint only on modified files located in src/backend/joanie
+    # Run pylint only on modified files located in src/backend
     # (excluding deleted files and migration files)
     # shellcheck disable=SC2207
-    paths=($(git diff "${diff_from}" --name-only --diff-filter=d -- src/backend/joanie ':!**/migrations/*.py' | grep -E '^src/backend/joanie/.*\.py$'))
+    paths=($(git diff "${diff_from}" --name-only --diff-filter=d -- src/backend ':!**/migrations/*.py' | grep -E '^src/backend/.*\.py$'))
 fi
 
 # Fix docker vs local path when project sources are mounted as a volume
 read -ra paths <<< "$(echo "${paths[@]}" | sed "s|src/backend/||g")"
-
 _dc_run app-dev pylint "${paths[@]}" "${args[@]}"

src/backend/core/api/permissions.py

@@ -27,8 +27,8 @@ class IsSelf(IsAuthenticated):
 
 class IsOwnedOrPublic(IsAuthenticated):
     """
-    Allows access to authenticated users only for objects that are owned or not related to any user via*
+    Allows access to authenticated users only for objects that are owned or not related
-    the "owner" field.
+    to any user via the "owner" field.
     """
 
     def has_object_permission(self, request, view, obj):

src/backend/core/api/serializers.py

@@ -1,5 +1,4 @@
 """Client serializers for the People core app."""
-from drf_spectacular.utils import extend_schema_field
 from rest_framework import exceptions, serializers
 from timezone_field.rest_framework import TimeZoneSerializerField
 

src/backend/core/api/utils.py

@@ -1,10 +1,6 @@
 """
 Utils that can be useful throughout the People core app
 """
-from django.conf import settings
-from django.utils import timezone
-
-import jwt
 from rest_framework_simplejwt.tokens import RefreshToken
 
 

src/backend/core/api/viewsets.py

@@ -1,30 +1,24 @@
 """API endpoints"""
-import io
-import uuid
-from http import HTTPStatus
-
 from django.contrib.postgres.search import TrigramSimilarity
 from django.core.cache import cache
-from django.core.exceptions import ValidationError
 from django.db.models import (
-    CharField,
-    Count,
-    F,
     Func,
     OuterRef,
-    Prefetch,
     Q,
     Subquery,
-    TextField,
     Value,
-    functions,
 )
 
-from rest_framework import decorators, mixins, pagination, response, viewsets
+from rest_framework import (
-from rest_framework import permissions as drf_permissions
+    decorators,
-from rest_framework.exceptions import ValidationError as DRFValidationError
+    exceptions,
+    mixins,
+    pagination,
+    response,
+    viewsets,
+)
 
-from core import enums, models
+from core import models
 
 from . import permissions, serializers
 
@@ -109,6 +103,7 @@ class Pagination(pagination.PageNumberPagination):
     page_size_query_param = "page_size"
 
 
+# pylint: disable=too-many-ancestors
 class ContactViewSet(
     mixins.CreateModelMixin,
     mixins.DestroyModelMixin,
@@ -159,7 +154,6 @@ class ContactViewSet(
         key_base = f"throttle-contact-list-{user.id!s}"
         key_minute = f"{key_base:s}-minute"
         key_hour = f"{key_base:s}-hour"
-        key_day = f"{key_base:s}-day"
 
         try:
             count_minute = cache.incr(key_minute)
@@ -173,14 +167,8 @@ class ContactViewSet(
             cache.set(key_hour, 1, 3600)
             count_hour = 1
 
-        try:
+        if count_minute > 20 or count_hour > 150:
-            count_day = cache.incr(key_day)
+            raise exceptions.Throttled()
-        except ValueError:
-            cache.set(key_day, 1, 86400)
-            count_day = 1
-
-        if count_minute > 20 or count_hour > 150 or count_day > 500:
-            raise drf_exceptions.Throttled()
 
         serializer = self.get_serializer(queryset, many=True)
         return response.Response(serializer.data)
@@ -333,7 +321,7 @@ class TeamAccessViewSet(
 
         # Check if the access being deleted is the last owner access for the team
         if instance.role == "owner" and team.accesses.filter(role="owner").count() == 1:
-            return Response(
+            return response.Response(
                 {"detail": "Cannot delete the last owner access for the team."},
                 status=400,
             )
@@ -355,10 +343,7 @@ class TeamAccessViewSet(
                 instance.role == models.RoleChoices.OWNER
                 and team.accesses.filter(role=models.RoleChoices.OWNER).count() == 1
             ):
-                raise serializers.ValidationError(
+                message = "Cannot change the role to a non-owner role for the last owner access."
-                    {
+                raise exceptions.ValidationError({"role": message})
-                        "role": "Cannot change the role to a non-owner role for the last owner access."
-                    }
-                )
 
         serializer.save()

src/backend/core/authentication.py

@@ -1,16 +1,13 @@
 """Authentication for the People core app."""
 from django.conf import settings
-from django.contrib.auth import get_user_model
 from django.utils.functional import SimpleLazyObject
 from django.utils.module_loading import import_string
-from django.utils.translation import get_supported_language_variant
 from django.utils.translation import gettext_lazy as _
 
 from drf_spectacular.authentication import SessionScheme, TokenScheme
 from drf_spectacular.plumbing import build_bearer_security_scheme_object
 from rest_framework import authentication
 from rest_framework_simplejwt.authentication import JWTAuthentication
-from rest_framework_simplejwt.exceptions import InvalidToken
 
 
 class DelegatedJWTAuthentication(JWTAuthentication):

src/backend/core/factories.py

@@ -2,19 +2,13 @@
 """
 Core application factories
 """
-import hashlib
-import json
-import random
-from datetime import datetime, timedelta, timezone
-
 from django.conf import settings
 from django.contrib.auth.hashers import make_password
-from django.utils import timezone as django_timezone
 
 import factory.fuzzy
 from faker import Faker
 
-from core import enums, models
+from core import models
 
 fake = Faker()
 

src/backend/core/models.py

@@ -4,30 +4,23 @@ Declare and configure the models for the People core application
 import json
 import os
 import uuid
-from datetime import timedelta
-from zoneinfo import ZoneInfo
 
 from django.conf import settings
 from django.contrib.auth import models as auth_models
 from django.contrib.auth.base_user import AbstractBaseUser
-from django.contrib.postgres.fields import ArrayField
-from django.contrib.postgres.indexes import GinIndex
 from django.core import exceptions, mail, validators
 from django.db import models
-from django.db.models import F, Q
-from django.utils import timezone as timezone_util
 from django.utils.functional import lazy
-from django.utils.text import capfirst, slugify
 from django.utils.translation import gettext_lazy as _
 
 import jsonschema
-from dateutil.relativedelta import relativedelta
+from rest_framework_simplejwt.exceptions import InvalidToken
 from rest_framework_simplejwt.settings import api_settings
 from timezone_field import TimeZoneField
 
 current_dir = os.path.dirname(os.path.abspath(__file__))
 contact_schema_path = os.path.join(current_dir, "jsonschema", "contact_data.json")
-with open(contact_schema_path, "r") as contact_schema_file:
+with open(contact_schema_path, "r", encoding="utf-8") as contact_schema_file:
     contact_schema = json.load(contact_schema_file)
 
 
@@ -140,7 +133,7 @@ class Contact(BaseModel):
         # Check if the contact points to a base contact that itself points to another base contact
         if self.base_id and self.base.base_id:
             raise exceptions.ValidationError(
-                "A contact cannot point to a base contact that itself points to another base contact."
+                "A contact cannot point to a base contact that itself points to a base contact."
             )
 
         # Validate the content of the "data" field against our jsonschema definition
@@ -159,6 +152,7 @@ class UserManager(auth_models.UserManager):
     """
 
     def get_queryset(self):
+        """Always select the related contact when doing a query on users."""
         return super().get_queryset().select_related("profile_contact")
 
 
@@ -467,8 +461,8 @@ def oidc_user_getter(validated_token):
             **{api_settings.USER_ID_FIELD: user_id}, profile_contact=contact
         )
 
-    # If the identity in the token is seen for the first time, make it the main email. Otherwise, update the
+    # If the identity in the token is seen for the first time, make it the main email.
-    # email and respect the main identity set by the user
+    # Otherwise, update the email and respect the main identity set by the user
     if email := validated_token["email"]:
         Identity.objects.update_or_create(
             user=user, email=email, create_defaults={"is_main": True}

src/backend/core/tests/swagger/test_openapi_schema.py

@@ -2,7 +2,9 @@
 Test suite for generated openapi schema.
 """
 import json
+from io import StringIO
 
+from django.core.management import call_command
 from django.test import Client
 
 import pytest

src/backend/core/tests/test_api_contacts.py

@@ -1,14 +1,10 @@
 """
 Test contacts API endpoints in People's core app.
 """
-import random
-from unittest import mock
-
 from django.test.utils import override_settings
 
 import pytest
 from rest_framework.test import APIClient
-from rest_framework_simplejwt.tokens import AccessToken
 
 from core import factories, models
 from core.api import serializers
@@ -120,7 +116,7 @@ def test_api_contacts_list_authenticated_by_full_name():
     dave = factories.BaseContactFactory(full_name="David Bowman")
     nicole = factories.BaseContactFactory(full_name="Nicole Foole")
     frank = factories.BaseContactFactory(full_name="Frank Poole")
-    heywood = factories.BaseContactFactory(full_name="Heywood Floyd")
+    factories.BaseContactFactory(full_name="Heywood Floyd")
 
     # Full query should work
     response = APIClient().get(
@@ -433,7 +429,7 @@ def test_api_contacts_create_authenticated_existing_override():
     jwt_token = OIDCToken.for_user(user)
 
     base_contact = factories.BaseContactFactory()
-    contact = factories.ContactFactory(base=base_contact, owner=user)
+    factories.ContactFactory(base=base_contact, owner=user)
 
     response = APIClient().post(
         "/api/v1.0/contacts/",

src/backend/core/tests/test_api_team_accesses.py

@@ -6,7 +6,6 @@ from uuid import uuid4
 
 import pytest
 from rest_framework.test import APIClient
-from rest_framework_simplejwt.tokens import AccessToken
 
 from core import factories, models
 from core.api import serializers
@@ -38,7 +37,7 @@ def test_api_team_accesses_list_authenticated_unrelated():
     jwt_token = OIDCToken.for_user(user)
 
     team = factories.TeamFactory()
-    accesses = factories.TeamAccessFactory.create_batch(3, team=team)
+    factories.TeamAccessFactory.create_batch(3, team=team)
 
     # Accesses for other teams to which the user is related should not be listed either
     other_access = factories.TeamAccessFactory(user=user)
@@ -82,8 +81,7 @@ def test_api_team_accesses_list_authenticated_related():
     assert response.status_code == 200
     content = response.json()
     assert len(content["results"]) == 3
-    id_sorter = lambda x: x["id"]
+    assert sorted(content["results"], key=lambda x: x["id"]) == sorted(
-    assert sorted(content["results"], key=id_sorter) == sorted(
         [
             {
                 "id": str(user_access.id),
@@ -104,7 +102,7 @@ def test_api_team_accesses_list_authenticated_related():
                 "abilities": access2.get_abilities(user),
             },
         ],
-        key=id_sorter,
+        key=lambda x: x["id"],
     )
 
 
@@ -576,7 +574,7 @@ def test_api_team_accesses_update_owner_except_owner():
     jwt_token = OIDCToken.for_user(user)
 
     team = factories.TeamFactory(users=[(user, "owner")])
-    other_user = factories.UserFactory()
+    factories.UserFactory()
     access = factories.TeamAccessFactory(
         team=team,
         role=random.choice(["administrator", "member"]),

src/backend/core/tests/test_api_users.py

@@ -1,12 +1,6 @@
 """
 Test users API endpoints in the People core app.
 """
-import random
-from unittest import mock
-from zoneinfo import ZoneInfo
-
-from django.test.utils import override_settings
-
 import pytest
 from rest_framework.test import APIClient
 
@@ -151,6 +145,8 @@ def test_api_users_create_authenticated():
             "language": "fr-fr",
             "password": "mypassword",
         },
+        format="json",
+        HTTP_AUTHORIZATION=f"Bearer {jwt_token}",
     )
     assert response.status_code == 404
     assert "Not Found" in response.content.decode("utf-8")
@@ -161,7 +157,7 @@ def test_api_users_update_anonymous():
     """Anonymous users should not be able to update users via the API."""
     user = factories.UserFactory()
 
-    old_user_values = serializers.UserSerializer(instance=user).data
+    old_user_values = dict(serializers.UserSerializer(instance=user).data)
     new_user_values = serializers.UserSerializer(instance=factories.UserFactory()).data
 
     response = APIClient().put(
@@ -176,21 +172,24 @@ def test_api_users_update_anonymous():
     }
 
     user.refresh_from_db()
-    user_values = serializers.UserSerializer(instance=user).data
+    user_values = dict(serializers.UserSerializer(instance=user).data)
     for key, value in user_values.items():
         assert value == old_user_values[key]
 
 
 def test_api_users_update_authenticated_self():
     """
-    Authenticated users should be able to update their own user but only "language" and "timezone" fields.
+    Authenticated users should be able to update their own user but only "language"
+    and "timezone" fields.
     """
     identity = factories.IdentityFactory()
     user = identity.user
     jwt_token = OIDCToken.for_user(user)
 
-    old_user_values = serializers.UserSerializer(instance=user).data
+    old_user_values = dict(serializers.UserSerializer(instance=user).data)
-    new_user_values = serializers.UserSerializer(instance=factories.UserFactory()).data
+    new_user_values = dict(
+        serializers.UserSerializer(instance=factories.UserFactory()).data
+    )
 
     response = APIClient().put(
         f"/api/v1.0/users/{user.id!s}/",
@@ -201,7 +200,7 @@ def test_api_users_update_authenticated_self():
 
     assert response.status_code == 200
     user.refresh_from_db()
-    user_values = serializers.UserSerializer(instance=user).data
+    user_values = dict(serializers.UserSerializer(instance=user).data)
     for key, value in user_values.items():
         if key in ["language", "timezone"]:
             assert value == new_user_values[key]
@@ -215,7 +214,7 @@ def test_api_users_update_authenticated_other():
     jwt_token = OIDCToken.for_user(identity.user)
 
     user = factories.UserFactory()
-    old_user_values = serializers.UserSerializer(instance=user).data
+    old_user_values = dict(serializers.UserSerializer(instance=user).data)
     new_user_values = serializers.UserSerializer(instance=factories.UserFactory()).data
 
     response = APIClient().put(
@@ -227,7 +226,7 @@ def test_api_users_update_authenticated_other():
 
     assert response.status_code == 403
     user.refresh_from_db()
-    user_values = serializers.UserSerializer(instance=user).data
+    user_values = dict(serializers.UserSerializer(instance=user).data)
     for key, value in user_values.items():
         assert value == old_user_values[key]
 
@@ -236,8 +235,10 @@ def test_api_users_patch_anonymous():
     """Anonymous users should not be able to patch users via the API."""
     user = factories.UserFactory()
 
-    old_user_values = serializers.UserSerializer(instance=user).data
+    old_user_values = dict(serializers.UserSerializer(instance=user).data)
-    new_user_values = serializers.UserSerializer(instance=factories.UserFactory()).data
+    new_user_values = dict(
+        serializers.UserSerializer(instance=factories.UserFactory()).data
+    )
 
     for key, new_value in new_user_values.items():
         response = APIClient().patch(
@@ -251,21 +252,24 @@ def test_api_users_patch_anonymous():
         }
 
     user.refresh_from_db()
-    user_values = serializers.UserSerializer(instance=user).data
+    user_values = dict(serializers.UserSerializer(instance=user).data)
     for key, value in user_values.items():
         assert value == old_user_values[key]
 
 
 def test_api_users_patch_authenticated_self():
     """
-    Authenticated users should be able to patch their own user but only "language" and "timezone" fields.
+    Authenticated users should be able to patch their own user but only "language"
+    and "timezone" fields.
     """
     identity = factories.IdentityFactory()
     user = identity.user
     jwt_token = OIDCToken.for_user(user)
 
-    old_user_values = serializers.UserSerializer(instance=user).data
+    old_user_values = dict(serializers.UserSerializer(instance=user).data)
-    new_user_values = serializers.UserSerializer(instance=factories.UserFactory()).data
+    new_user_values = dict(
+        serializers.UserSerializer(instance=factories.UserFactory()).data
+    )
 
     for key, new_value in new_user_values.items():
         response = APIClient().patch(
@@ -277,7 +281,7 @@ def test_api_users_patch_authenticated_self():
         assert response.status_code == 200
 
     user.refresh_from_db()
-    user_values = serializers.UserSerializer(instance=user).data
+    user_values = dict(serializers.UserSerializer(instance=user).data)
     for key, value in user_values.items():
         if key in ["language", "timezone"]:
             assert value == new_user_values[key]
@@ -291,8 +295,10 @@ def test_api_users_patch_authenticated_other():
     jwt_token = OIDCToken.for_user(identity.user)
 
     user = factories.UserFactory()
-    old_user_values = serializers.UserSerializer(instance=user).data
+    old_user_values = dict(serializers.UserSerializer(instance=user).data)
-    new_user_values = serializers.UserSerializer(instance=factories.UserFactory()).data
+    new_user_values = dict(
+        serializers.UserSerializer(instance=factories.UserFactory()).data
+    )
 
     for key, new_value in new_user_values.items():
         response = APIClient().put(
@@ -304,7 +310,7 @@ def test_api_users_patch_authenticated_other():
         assert response.status_code == 403
 
     user.refresh_from_db()
-    user_values = serializers.UserSerializer(instance=user).data
+    user_values = dict(serializers.UserSerializer(instance=user).data)
     for key, value in user_values.items():
         assert value == old_user_values[key]
 

src/backend/core/tests/test_models_contacts.py

@@ -5,7 +5,7 @@ from django.core.exceptions import ValidationError
 
 import pytest
 
-from core import factories, models
+from core import factories
 
 pytestmark = pytest.mark.django_db
 
@@ -31,7 +31,7 @@ def test_models_contacts_base_self():
         contact.save()
 
     error_message = (
-        "{'__all__': ['A contact cannot point to a base contact that itself points to another "
+        "{'__all__': ['A contact cannot point to a base contact that itself points to a "
         "base contact.', 'A contact cannot be based on itself.']}"
     )
     assert str(excinfo.value) == error_message
@@ -45,7 +45,7 @@ def test_models_contacts_base_to_base():
         factories.ContactFactory(base=contact)
 
     error_message = (
-        "{'__all__': ['A contact cannot point to a base contact that itself points to another "
+        "{'__all__': ['A contact cannot point to a base contact that itself points to a "
         "base contact.']}"
     )
     assert str(excinfo.value) == error_message
@@ -103,7 +103,7 @@ def test_models_contacts_profile_owned_by_other():
 
 def test_models_contacts_data_valid():
     """Contact information matching the jsonschema definition should be valid"""
-    contact = factories.ContactFactory(
+    factories.ContactFactory(
         data={
             "emails": [
                 {"type": "Work", "value": "john.doe@work.com"},
@@ -157,7 +157,7 @@ def test_models_contacts_data_invalid():
             }
         )
 
-    assert (
+    assert str(excinfo.value) == (
-        str(excinfo.value)
+        "{'data': [\"Validation error in 'emails.0.type': 'invalid type' is not one of ['Work', "
-        == "{'data': [\"Validation error in 'emails.0.type': 'invalid type' is not one of ['Work', 'Home', 'Other']\"]}"
+        "'Home', 'Other']\"]}"
     )

src/backend/core/tests/test_models_identities.py

@@ -37,7 +37,7 @@ def test_models_identities_is_main_automatic():
 def test_models_identities_is_main_exists():
     """A user should always keep one and only one of its identities as main."""
     user = factories.UserFactory()
-    main_identity, secondary_identity = factories.IdentityFactory.create_batch(
+    main_identity, _secondary_identity = factories.IdentityFactory.create_batch(
         2, user=user
     )
 
@@ -127,8 +127,8 @@ def test_models_identities_sub_null():
         models.Identity.objects.create(user=user, sub=None)
 
 
-def test_models_identities_sub_null():
+def test_models_identities_sub_blank():
-    """The "sub" field should not be null."""
+    """The "sub" field should not be blank."""
     user = factories.UserFactory()
     with pytest.raises(ValidationError, match="This field cannot be blank."):
         models.Identity.objects.create(user=user, email="david@example.com", sub="")
@@ -165,9 +165,9 @@ def test_models_identities_sub_spaces():
     with pytest.raises(ValidationError) as excinfo:
         factories.IdentityFactory(sub="a b")
 
-    assert (
+    assert str(excinfo.value) == (
-        str(excinfo.value)
+        "{'sub': ['Enter a valid sub. This value may contain only letters, numbers, "
-        == "{'sub': ['Enter a valid sub. This value may contain only letters, numbers, and @/./+/-/_ characters.']}"
+        "and @/./+/-/_ characters.']}"
     )
 
 

src/backend/core/tests/test_models_team_accesses.py

@@ -6,7 +6,7 @@ from django.core.exceptions import ValidationError
 
 import pytest
 
-from core import factories, models
+from core import factories
 
 pytestmark = pytest.mark.django_db
 
@@ -72,7 +72,9 @@ def test_models_team_access_get_abilities_authenticated():
 
 
 def test_models_team_access_get_abilities_for_owner_of_self_allowed():
-    """Check abilities of self access for the owner of a team when there is more than one user left."""
+    """
+    Check abilities of self access for the owner of a team when there is more than one user left.
+    """
     access = factories.TeamAccessFactory(role="owner")
     factories.TeamAccessFactory(team=access.team, role="owner")
     abilities = access.get_abilities(access.user)

src/backend/core/tests/test_models_teams.py

@@ -41,7 +41,7 @@ def test_models_teams_name_max_length():
     factories.TeamFactory(name="a " * 50)
     with pytest.raises(
         ValidationError,
-        match="Ensure this value has at most 100 characters \(it has 102\).",
+        match=r"Ensure this value has at most 100 characters \(it has 102\)\.",
     ):
         factories.TeamFactory(name="a " * 51)
 

src/backend/core/tests/test_models_users.py

@@ -4,7 +4,6 @@ Unit tests for the User model
 from unittest import mock
 
 from django.core.exceptions import ValidationError
-from django.test.utils import override_settings
 
 import pytest
 

src/backend/people/urls.py

@@ -4,7 +4,7 @@ from django.conf import settings
 from django.conf.urls.static import static
 from django.contrib import admin
 from django.contrib.staticfiles.urls import staticfiles_urlpatterns
-from django.urls import include, path, re_path
+from django.urls import path, re_path
 
 from drf_spectacular.views import (
     SpectacularJSONAPIView,

src/backend/pyproject.toml

@@ -65,6 +65,8 @@ dev = [
     "ipdb==0.13.13",
     "ipython==8.18.1",
     "pyfakefs==5.3.2",
+    "pylint-django==2.5.5",
+    "pylint==3.0.3",
     "pytest-cov==4.1.0",
     "pytest-django==4.7.0",
     "pytest==7.4.3",