Fix type errors (#81)

This brings down the number of type errors from 118 down to 92. I think most of the remaining errors are due to the */models.py files where I'm not sure what to do about it. I run the type checker in docker via `just typecheck`.

The change in 0af3fbcac4 requires this change upstream: https://github.com/valberg/django-registries/pull/28 we can omit that commit for now.

Reviewed-on: https://git.data.coop/data.coop/membersystem/pulls/81
Reviewed-by: benjaoming <benjaoming@data.coop>
Co-authored-by: Reynir Björnsson <reynir@reynir.dk>
Co-committed-by: Reynir Björnsson <reynir@reynir.dk>

src/accounting/admin.py

@@ -33,12 +33,11 @@ class OrderAdminForm(forms.ModelForm):
     def clean(self) -> None:
         """Clean the order."""
         cd = super().clean()
-        if not cd["account"] and cd["member"]:
+        if cd and not cd["account"] and cd["member"]:
             try:
                 cd["account"] = models.Account.objects.get_or_create(owner=cd["member"])[0]
             except models.Account.MultipleObjectsReturned:
                 cd["account"] = models.Account.objects.filter(owner=cd["member"]).first()
-        return cd
 
 
 @admin.register(models.Order)

src/accounting/views.py

@@ -85,6 +85,7 @@ def order_pay(request: HttpRequest, order_id: int) -> HttpResponse:
 
     # TODO(benjaoming): Redirect with status=303
     #  https://git.data.coop/data.coop/membersystem/issues/63
+    # TODO(reynir): url is None if session is not active
     return redirect(checkout_session.url)
 
 

src/membership/emails.py

@@ -20,6 +20,7 @@ from django.utils.translation import gettext_lazy as _
 if TYPE_CHECKING:
     from accounting.models import Order
     from django.http import HttpRequest
+    from django_stubs_ext import StrOrPromise
 
     from .models import Membership
 
@@ -35,7 +36,7 @@ class BaseEmail(EmailMessage):
     template = "membership/email/base.txt"
     # Optional: Set to a template path for subject
     template_subject = None
-    default_subject = "SET SUBJECT HERE"
+    default_subject : StrOrPromise = "SET SUBJECT HERE"
 
     def __init__(self, request: HttpRequest, *args, **kwargs) -> None:  # noqa: ANN002, ANN003
         self.context = kwargs.pop("context", {})
@@ -90,7 +91,7 @@ class BaseEmail(EmailMessage):
 
     def send_with_feedback(self, *, success_msg: str | None = None, no_message: bool = False) -> None:
         """Send email, possibly adding feedback via django.contrib.messages."""
-        if not success_msg:
+        if success_msg is None:
             success_msg = _("Email successfully sent to {}").format(", ".join(self.to))
         try:
             self.send(fail_silently=False)

src/membership/permissions.py

@@ -2,13 +2,17 @@
 
 from __future__ import annotations
 
+from typing import TYPE_CHECKING
 from dataclasses import dataclass
 
 from django.contrib.auth.models import Permission as DjangoPermission
 from django.contrib.contenttypes.models import ContentType
 from django.utils.translation import gettext_lazy as _
 
-PERMISSIONS = []
+if TYPE_CHECKING:
+    from django_stubs_ext import StrOrPromise
+
+PERMISSIONS : list[Permission] = []
 
 
 def persist_permissions(*args, **kwargs) -> None:  # type: ignore[no-untyped-def] # noqa: ANN002, ANN003
@@ -21,7 +25,7 @@ def persist_permissions(*args, **kwargs) -> None:  # type: ignore[no-untyped-def
 class Permission:
     """Dataclass to define a permission."""
 
-    name: str
+    name: StrOrPromise
     codename: str
     app_label: str
     model: str

src/membership/views.py

@@ -25,6 +25,7 @@ from .selectors import get_memberships
 from .selectors import get_subscription_periods
 
 if TYPE_CHECKING:
+    from utils.types import AuthenticatedHttpRequest
     from django.http import HttpRequest
     from django.http import HttpResponse
 
@@ -36,7 +37,7 @@ member_view = namespaced_decorator_factory(namespace="member", base_path="member
     name="membership-overview",
     login_required=True,
 )
-def membership_overview(request: HttpRequest) -> HttpResponse:
+def membership_overview(request: AuthenticatedHttpRequest) -> HttpResponse:
     """View to show the membership overview."""
     memberships = get_memberships(member=request.user)
     current_membership = memberships.current()
@@ -69,7 +70,7 @@ admin_members_view = namespaced_decorator_factory(
     login_required=True,
     permissions=[ADMINISTRATE_MEMBERS.path],
 )
-def members_admin(request: HttpRequest) -> HttpResponse:
+def members_admin(request: AuthenticatedHttpRequest) -> HttpResponse:
     """View to list all members."""
     users = get_members()
 
@@ -105,7 +106,7 @@ def members_admin(request: HttpRequest) -> HttpResponse:
     login_required=True,
     permissions=[ADMINISTRATE_MEMBERS.path],
 )
-def members_admin_detail(request: HttpRequest, member_id: int) -> HttpResponse:
+def members_admin_detail(request: AuthenticatedHttpRequest, member_id: int) -> HttpResponse:
     """View to show the details of a member."""
     member = get_member(member_id=member_id)
     subscription_periods = get_subscription_periods(member=member)
@@ -123,12 +124,6 @@ def members_admin_detail(request: HttpRequest, member_id: int) -> HttpResponse:
     )
 
 
-class InvalidTokenError(HttpResponseForbidden):
-    """Exception raised when an invalid token is encountered."""
-
-    content = "Token not valid - maybe it expired?"
-
-
 @ratelimit(group="membership", key="ip", rate="10/d", method="ALL", block=True)
 @member_view(
     paths="invite/<str:referral_code>/<str:token>/",
@@ -152,7 +147,7 @@ def invite(request: HttpRequest, referral_code: str, token: str) -> HttpResponse
     token_valid = default_token_generator.check_token(membership.user, token)
 
     if not token_valid:
-        raise InvalidTokenError
+        return HttpResponseForbidden("Token not valid - maybe it expired?")
 
     if request.method == "POST":
         form = InviteForm(membership=membership, data=request.POST)

src/project/settings.py

@@ -108,7 +108,7 @@ AUTHENTICATION_BACKENDS = (
 WSGI_APPLICATION = "project.wsgi.application"
 
 
-AUTH_PASSWORD_VALIDATORS = []
+AUTH_PASSWORD_VALIDATORS : list[dict[str, str]] = []
 
 LANGUAGE_CODE = "da-dk"
 

src/services/registry.py

@@ -7,7 +7,7 @@ from django_registries.registry import Interface
 from django_registries.registry import Registry
 
 
-class ServiceRegistry(Registry):
+class ServiceRegistry(Registry["ServiceInterface"]):
     """Registry for services."""
 
     implementations_module = "services"
@@ -39,9 +39,9 @@ class ServiceInterface(Interface):
     # This is a way of saying that the service is "mandatory" to have.
     auto_create: bool = False
 
-    request_types: list[str, str] = DEFAULT_SERVICE_REQUEST_TYPES
+    request_types: list[ServiceRequests] = DEFAULT_SERVICE_REQUEST_TYPES
 
-    subscribe_fields: tuple[tuple[str, forms.Field]] = []
+    subscribe_fields: tuple[tuple[str, forms.Field],...] = ()
 
     def get_form_class(self) -> type:
         """Get the form class for the service."""

src/services/views.py

@@ -14,7 +14,7 @@ from services.registry import ServiceRegistry
 
 if TYPE_CHECKING:
     from django.contrib.auth.models import User
-    from django.http import HttpRequest
+    from utils.types import AuthenticatedHttpRequest
     from django.http import HttpResponse
 
 services_view = namespaced_decorator_factory(
@@ -28,16 +28,16 @@ services_view = namespaced_decorator_factory(
     name="list",
     login_required=True,
 )
-def services_overview(request: HttpRequest) -> HttpResponse:
+def services_overview(request: AuthenticatedHttpRequest) -> HttpResponse:
     """View all services."""
     active_services = get_services(user=request.user)
 
     active_service_classes = [service.__class__ for service in active_services]
 
-    services = [service for _, service in ServiceRegistry.get_items() if service not in active_service_classes]
+    non_active_services = [service for _, service in ServiceRegistry.get_items() if service not in active_service_classes]
 
     context = {
-        "non_active_services": services,
+        "non_active_services": non_active_services,
         "active_services": active_services,
     }
 
@@ -53,7 +53,7 @@ def services_overview(request: HttpRequest) -> HttpResponse:
     name="detail",
     login_required=True,
 )
-def service_detail(request: HttpRequest, service_slug: str) -> HttpResponse:
+def service_detail(request: AuthenticatedHttpRequest, service_slug: str) -> HttpResponse:
     """View a service."""
     service = ServiceRegistry.get(slug=service_slug)
 
@@ -73,7 +73,7 @@ def service_detail(request: HttpRequest, service_slug: str) -> HttpResponse:
     name="subscribe",
     login_required=True,
 )
-def service_subscribe(request: HttpRequest, service_slug: str) -> HttpResponse:
+def service_subscribe(request: AuthenticatedHttpRequest, service_slug: str) -> HttpResponse:
     """Subscribe to a service."""
     service = ServiceRegistry.get(slug=service_slug)
 

src/utils/types.py

@@ -7,4 +7,5 @@ from django.http import HttpRequest
 class AuthenticatedHttpRequest(HttpRequest):
     """HttpRequest with an authenticated user."""
 
+    # XXX(reynir): Should this be Member instead?!
     user: User

src/utils/view_utils.py

@@ -19,6 +19,7 @@ if TYPE_CHECKING:
     from django.db.models import QuerySet
     from django.http import HttpRequest
     from django.http import HttpResponse
+    from django_stubs_ext import StrOrPromise
 
 
 @dataclass
@@ -26,18 +27,18 @@ class Row:
     """A row in a table."""
 
     data: dict[str, str]
-    actions: list[dict[str, str]]
+    actions: list[dict[str, StrOrPromise]]
 
 
 @dataclass
 class RowAction:
     """An action that can be performed on a row in a table."""
 
-    label: str
+    label: StrOrPromise
     url_name: str
     url_kwargs: dict[str, str]
 
-    def render(self, obj: Model) -> dict[str, str]:
+    def render(self, obj: Model) -> dict[str, StrOrPromise]:
         """Render the action as a dictionary for the given object."""
         url = reverse(
             self.url_name,
@@ -53,7 +54,7 @@ class RenderConfig:
     entity_name: str
     entity_name_plural: str
     objects: QuerySet
-    columns: list[tuple[str, str]]
+    columns: list[tuple[str, StrOrPromise]]
     row_actions: list[RowAction] | None = None
     list_actions: list[tuple[str, str]] | None = None
     paginate_by: int | None = None
@@ -88,7 +89,8 @@ class RenderConfig:
         for obj in objects:
             with queries_disabled():
                 row = Row(
-                    data={column: getattr(obj, column[0]) for column in columns},
+                    # XXX(reynir): we never use the key
+                    data={column[0]: getattr(obj, column[0]) for column in columns},
                     actions=[action.render(obj) for action in row_actions],
                 )
                 rows.append(row)