Improve typing (#112)

* Improve typing

* explicit params to fsm_transition_view

Author: pfouque

django_fsm/__init__.py

@@ -21,11 +21,6 @@
 from django_fsm.signals import pre_transition
 
 if typing.TYPE_CHECKING:  # pragma: no cover
-    from collections.abc import Callable
-    from collections.abc import Collection
-    from collections.abc import Generator
-    from collections.abc import Iterable
-    from collections.abc import Sequence
     from typing import Self
 
     from django.contrib.auth.models import AbstractUser
@@ -39,10 +34,11 @@
     IntegerField: typing.TypeAlias = models.IntegerField[int, int]
     ForeignKey: typing.TypeAlias = models.ForeignKey[typing.Any, typing.Any]
 
-    _FSMModel = models.Model
+    _FSMModel: typing.TypeAlias = models.Model
     _StateValue: typing.TypeAlias = str | int | models.Choices
-    _Permission: typing.TypeAlias = str | Callable[[_FSMModel, typing.Any], bool]
-    _Condition: typing.TypeAlias = Callable[[models.Model], bool]
+    _Permission: typing.TypeAlias = str | typing.Callable[[_FSMModel, UserWithPermissions], bool]
+    _Condition: typing.TypeAlias = typing.Callable[[_FSMModel], bool]
+    _TransitionFunc: typing.TypeAlias = typing.Callable[..., _StateValue | typing.Any | None]
 
 else:
     _FSMModel = object
@@ -99,7 +95,7 @@ class ConcurrentTransition(Exception):  # noqa: N818
 class Transition:
     def __init__(
         self,
-        method: Callable[..., _StateValue | None],
+        method: _TransitionFunc,
         source: _StateValue,
         target: _StateValue,
         on_error: _StateValue | None,
@@ -147,7 +143,7 @@ def __eq__(self, other: object) -> bool:
 
 def get_available_FIELD_transitions(  # noqa: N802
     instance: _FSMModel, field: FSMFieldMixin
-) -> Generator[Transition]:
+) -> typing.Generator[Transition]:
     """
     List of transitions available in current model state
     with all conditions met
@@ -161,7 +157,9 @@ def get_available_FIELD_transitions(  # noqa: N802
             yield meta.get_transition(curr_state)
 
 
-def get_all_FIELD_transitions(instance: _FSMModel, field: FSMFieldMixin) -> Generator[Transition]:  # noqa: N802
+def get_all_FIELD_transitions(  # noqa: N802
+    instance: _FSMModel, field: FSMFieldMixin
+) -> typing.Generator[Transition]:
     """
     List of all transitions available in current model state
     """
@@ -170,7 +168,7 @@ def get_all_FIELD_transitions(instance: _FSMModel, field: FSMFieldMixin) -> Gene
 
 def get_available_user_FIELD_transitions(  # noqa: N802
     instance: _FSMModel, user: UserWithPermissions, field: FSMFieldMixin
-) -> Generator[Transition]:
+) -> typing.Generator[Transition]:
     """
     List of transitions available in current model state
     with all conditions met and user have rights on it
@@ -199,12 +197,12 @@ def get_transition(self, source: _StateValue) -> Transition | None:
 
     def add_transition(
         self,
-        method: Callable[..., _StateValue],
+        method: _TransitionFunc,
         source: _StateValue,
         target: _StateValue,
         on_error: _StateValue | None = None,
         conditions: list[_Condition] | None = None,
-        permission: str | Callable[[_FSMModel, UserWithPermissions], bool] | None = None,
+        permission: _Permission | None = None,
         custom: dict[str, typing.Any] | None = None,
     ) -> None:
         if source in self.transitions:
@@ -320,7 +318,7 @@ def __init__(self, *args: typing.Any, **kwargs: typing.Any) -> None:
         super().__init__(*args, **kwargs)
 
     @override
-    def deconstruct(self) -> tuple[str, str, Sequence[typing.Any], dict[str, typing.Any]]:
+    def deconstruct(self) -> tuple[str, str, typing.Sequence[typing.Any], dict[str, typing.Any]]:
         name, path, args, kwargs = super().deconstruct()
         if self.protected:
             kwargs["protected"] = self.protected
@@ -414,7 +412,7 @@ def change_state(
 
         return result
 
-    def get_all_transitions(self, instance_cls: type[_FSMModel]) -> Generator[Transition]:
+    def get_all_transitions(self, instance_cls: type[_FSMModel]) -> typing.Generator[Transition]:
         """
         Returns [(source, target, name, method)] for all field transitions
         """
@@ -458,7 +456,7 @@ def _collect_transitions(self, *args: typing.Any, **kwargs: typing.Any) -> None:
         if not issubclass(sender, self.base_cls):
             return
 
-        def is_field_transition_method(attr: Callable[[typing.Any], typing.Any]) -> bool:
+        def is_field_transition_method(attr: _TransitionFunc) -> bool:
             return (
                 (inspect.ismethod(attr) or inspect.isfunction(attr))
                 and hasattr(attr, "_django_fsm")
@@ -567,7 +565,7 @@ def __init__(self, *args: typing.Any, **kwargs: typing.Any) -> None:
         self._update_initial_state()
 
     @property
-    def state_fields(self) -> Iterable[FSMFieldMixin]:
+    def state_fields(self) -> typing.Iterable[FSMFieldMixin]:
         return filter(lambda field: isinstance(field, FSMFieldMixin), self._meta.fields)  # type: ignore[arg-type]
 
     @override
@@ -576,8 +574,8 @@ def _do_update(
         base_qs: QuerySet[Self],
         using: str | None,
         pk_val: typing.Any,
-        values: Collection[tuple[_Field, type[models.Model] | None, typing.Any]],
-        update_fields: Iterable[str] | None,
+        values: typing.Collection[tuple[_Field, type[models.Model] | None, typing.Any]],
+        update_fields: typing.Iterable[str] | None,
         forced_update: bool,
         returning_fields: bool | None = None,
     ) -> bool:
@@ -644,13 +642,13 @@ def save(self, *args: typing.Any, **kwargs: typing.Any) -> None:
 
 def transition(
     field: FSMFieldMixin | str,
-    source: _StateValue | Sequence[_StateValue] = "*",
+    source: _StateValue | typing.Sequence[_StateValue] = "*",
     target: _StateValue | State | None = None,
     on_error: _StateValue | None = None,
     conditions: list[_Condition] | None = None,
     permission: _Permission | None = None,
     custom: dict[str, typing.Any] | None = None,
-) -> Callable[[typing.Any], typing.Any]:
+) -> typing.Callable[[typing.Any], typing.Any]:
     """
     Method decorator to mark allowed transitions.
 
@@ -728,14 +726,14 @@ def has_transition_perm(bound_method: typing.Any, user: UserWithPermissions) ->
 
 
 class State:
-    allowed_states: Sequence[_StateValue]
+    allowed_states: typing.Sequence[_StateValue]
 
     def get_state(
         self,
         model: _FSMModel,
         transition: Transition,
         result: typing.Any,
-        args: Sequence[typing.Any] | None = None,
+        args: typing.Sequence[typing.Any] | None = None,
         kwargs: dict[str, typing.Any] | None = None,
     ) -> typing.Any:
         raise NotImplementedError
@@ -750,7 +748,7 @@ def get_state(
         model: _FSMModel,
         transition: Transition,
         result: typing.Any,
-        args: Sequence[typing.Any] | None = None,
+        args: typing.Sequence[typing.Any] | None = None,
         kwargs: dict[str, typing.Any] | None = None,
     ) -> typing.Any:
         if self.allowed_states is not None and result not in self.allowed_states:
@@ -763,8 +761,8 @@ def get_state(
 class GET_STATE(State):  # noqa: N801
     def __init__(
         self,
-        func: Callable[..., _StateValue],
-        states: Sequence[_StateValue] | None = None,
+        func: typing.Callable[..., _StateValue],
+        states: typing.Sequence[_StateValue] | None = None,
     ) -> None:
         self.func = func
         self.allowed_states = states or []
@@ -774,7 +772,7 @@ def get_state(
         model: _FSMModel,
         transition: Transition,
         result: _StateValue,
-        args: Sequence[typing.Any] | None = None,
+        args: typing.Sequence[typing.Any] | None = None,
         kwargs: dict[str, typing.Any] | None = None,
     ) -> typing.Any:
         if args is None:

django_fsm/admin.py

@@ -37,7 +37,7 @@
 else:
     _ModelAdmin = admin.ModelAdmin
 
-_FormType = type[Form | ModelForm[typing.Any]]
+_FormType: typing.TypeAlias = type[Form | ModelForm[fsm._FSMModel]]
 
 
 @dataclass
@@ -85,7 +85,7 @@ def __init__(self, model: type[fsm._FSMModel], admin_site: admin.AdminSite) -> N
 
     @override
     def get_readonly_fields(
-        self, request: http.HttpRequest, obj: typing.Any = None
+        self, request: http.HttpRequest, obj: fsm._FSMModel | None = None
     ) -> tuple[str, ...]:
         """Ensures 'protected' fields are 'readonly'"""
 
@@ -141,7 +141,7 @@ def change_view(
         )
 
     @override
-    def response_change(self, request: http.HttpRequest, obj: typing.Any) -> http.HttpResponse:
+    def response_change(self, request: http.HttpRequest, obj: fsm._FSMModel) -> http.HttpResponse:
         transition_name = request.POST.get(self.fsm_post_param)
         if not transition_name:
             return super().response_change(request=request, obj=obj)
@@ -172,7 +172,7 @@ def response_change(self, request: http.HttpRequest, obj: typing.Any) -> http.Ht
                     "preserved_filters": self.get_preserved_filters(request),
                     "opts": self.model._meta,
                 },
-                url=self.get_fsm_redirect_url(request=request, obj=obj),
+                url=request.path,
             )
         )
 
@@ -191,9 +191,6 @@ def get_fsm_label(self, transition: fsm.Transition) -> str:
     def get_help_text(self, transition: fsm.Transition) -> str | None:
         return transition.custom.get("help_text")
 
-    def get_fsm_redirect_url(self, request: http.HttpRequest, obj: typing.Any) -> str:
-        return request.path
-
     def get_fsm_transition_form(self, transition: fsm.Transition) -> _FormType | None:
         """Get transition form class with error handling."""
         form = self.fsm_forms.get(transition.name, transition.custom.get("form"))
@@ -210,7 +207,7 @@ def get_fsm_transition_form(self, transition: fsm.Transition) -> _FormType | Non
     # Transition helpers
 
     def _get_fsm_extra_context(
-        self, *, request: http.HttpRequest, obj: typing.Any
+        self, *, request: http.HttpRequest, obj: fsm._FSMModel | None
     ) -> typing.Generator[FSMObjectTransition]:
         for field_name in sorted(self.fsm_fields):
             transitions_func = getattr(obj, f"get_available_user_{field_name}_transitions", None)
@@ -232,10 +229,10 @@ def _get_fsm_extra_context(
                     )
 
     def _get_fsm_transition_func(
-        self, *, obj: typing.Any, transition_name: str
-    ) -> typing.Callable[..., typing.Any]:
+        self, *, obj: fsm._FSMModel, transition_name: str
+    ) -> fsm._TransitionFunc:
         try:
-            transition_func: typing.Callable[..., typing.Any] = getattr(obj, transition_name)
+            transition_func: fsm._TransitionFunc = getattr(obj, transition_name)
         except AttributeError:
             raise AttributeError(
                 f"{obj.__class__.__name__} has no transition method '{transition_name}'."
@@ -251,7 +248,7 @@ def _get_fsm_transition_func(
         return transition_func
 
     def _get_fsm_transition_by_name(
-        self, *, obj: typing.Any, transition_name: str
+        self, *, obj: fsm._FSMModel, transition_name: str
     ) -> fsm.Transition:
         transition_func = self._get_fsm_transition_func(obj=obj, transition_name=transition_name)
         transitions = transition_func._django_fsm.transitions  # type: ignore[attr-defined]
@@ -271,7 +268,7 @@ def _is_fsm_log_enabled() -> bool:
     def _execute_fsm_transition(
         self,
         *,
-        transition_func: typing.Callable[..., typing.Any],
+        transition_func: fsm._TransitionFunc,
         request: http.HttpRequest,
         kwargs: typing.Mapping[str, typing.Any] | None = None,
     ) -> None:
@@ -287,7 +284,7 @@ def _execute_fsm_transition(
     def _apply_fsm_transition(
         self,
         *,
-        obj: typing.Any,
+        obj: fsm._FSMModel,
         transition_name: str,
         request: http.HttpRequest,
         kwargs: typing.Mapping[str, typing.Any] | None = None,
@@ -344,16 +341,14 @@ def _apply_fsm_transition(
     # Form handling
 
     def fsm_transition_view(
-        self, request: http.HttpRequest, *args: typing.Any, **kwargs: typing.Any
+        self, request: http.HttpRequest, object_id: str, transition_name: str, **kwargs: typing.Any
     ) -> http.HttpResponse:
         """Handle FSM transition form view with enhanced validation."""
-        object_id = kwargs["object_id"]
+
         obj = self.get_object(request, object_id)
         if obj is None:
             return self._get_obj_does_not_exist_redirect(request, self.opts, object_id)  # type: ignore[no-any-return, attr-defined]
 
-        transition_name = kwargs["transition_name"]
-
         transition = self._get_fsm_transition_by_name(obj=obj, transition_name=transition_name)
 
         if not transition.has_perm(obj, user=request.user):

tests/testapp/models.py

@@ -1,7 +1,6 @@
 from __future__ import annotations
 
-import typing
-
+from django.contrib.auth.models import AbstractUser
 from django.db import models
 from django_fsm_log.decorators import fsm_log_by
 from django_fsm_log.decorators import fsm_log_description
@@ -13,9 +12,6 @@
 from django_fsm import FSMKeyField
 from django_fsm import transition
 
-if typing.TYPE_CHECKING:
-    from django.contrib.auth.models import AbstractUser
-
 
 class Application(models.Model):
     """
@@ -210,8 +206,10 @@ class Meta:
             ("can_remove_post", "Can remove post"),
         ]
 
-    def can_restore(self: models.Model, user: AbstractUser) -> bool:
-        return bool(user.is_superuser or user.is_staff)
+    def can_restore(self: models.Model, user: fsm.UserWithPermissions) -> bool:
+        if isinstance(user, AbstractUser):
+            return bool(user.is_superuser or user.is_staff)
+        return False
 
     @transition(
         field=state,

tests/testapp/tests/test_admin.py

@@ -194,15 +194,6 @@ def setUp(self):
     def test_protected_fields_are_readonly(self):
         assert self.model_admin.get_readonly_fields(request=self.request) == ("state",)
 
-    def test_get_fsm_redirect_url(self):
-        assert (
-            self.model_admin.get_fsm_redirect_url(
-                request=RequestFactory().get(path="/path"),
-                obj=None,
-            )
-            == "/path"
-        )
-
     # Execution
     def test_execute_fsm_transition_falls_back_to_plain_call(self) -> None:
         called: dict[str, str] = {}