From f09ea0e4723a0ec16245e2f13d9a9d71258765d1 Mon Sep 17 00:00:00 2001 From: Benedikt Willi Date: Mon, 15 Dec 2025 11:38:33 +0100 Subject: [PATCH] Fix compatibility issues with django-modeltranslation by modifying manager mixins - Added a new class `_GenericMixin` to serve as a runtime placeholder for `Generic[ModelT]`. This change prevents `TypeError` during `__class__` assignments, which was an issue when mixins inherited from `Generic[T]` at runtime. - All manager mixins have been updated to inherit from `_GenericMixin` instead of `Generic[ModelT]`. This ensures compatibility with `django-modeltranslation`. - Introduced regressions tests to confirm that the manager instances support `__class__` reassignment without issues. Tests were added for `SoftDeletableManager`, `InheritanceManager`, `QueryManager`, and `JoinManager`. Closes GH-#636. --- AUTHORS.rst | 1 + CHANGES.rst | 3 + model_utils/managers.py | 23 +++++-- .../test_manager_class_assignment.py | 66 +++++++++++++++++++ 4 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 tests/test_managers/test_manager_class_assignment.py diff --git a/AUTHORS.rst b/AUTHORS.rst index c9036959..04796d00 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -12,6 +12,7 @@ | Arseny Sysolyatin | Artis Avotins | Asif Saif Uddin +| Benedikt Willi | Bo Marchman | Bojan Mihelac | Bruno Alla diff --git a/CHANGES.rst b/CHANGES.rst index bf96f0cd..e91541b9 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,6 +6,9 @@ To be released - Add support for `Python 3.13` (GH-#628) - Add formal support for `Django 5.2` (GH-#641) - Drop support for older versions than `Django 4.2` +- Fix compatibility with django-modeltranslation: manager mixins no longer + inherit from `Generic[T]` at runtime, preventing `TypeError` on `__class__` + assignment (GH-#636) 5.0.0 (2024-09-01) ------------------ diff --git a/model_utils/managers.py b/model_utils/managers.py index 4cb1ffcd..1cd21cf3 100644 --- a/model_utils/managers.py +++ b/model_utils/managers.py @@ -17,6 +17,19 @@ from django.db.models.query import BaseIterable + # Generic base for mixin classes - enables type checking support + # while avoiding runtime issues with __class__ assignment + # (e.g., when used with django-modeltranslation). + _GenericMixin = Generic[ModelT] +else: + # At runtime, use a subscriptable but non-Generic class to avoid + # __class__ assignment issues that occur when Generic[T] is in + # the class hierarchy (e.g., django-modeltranslation compatibility). + class _GenericMixin: + """Runtime placeholder for Generic[ModelT] that supports subscripting.""" + def __class_getitem__(cls, item: Any) -> type[_GenericMixin]: + return cls + def _iter_inheritance_queryset(queryset: QuerySet[ModelT]) -> Iterator[ModelT]: iter: ModelIterable[ModelT] = ModelIterable(queryset) @@ -63,7 +76,7 @@ def __iter__(self): return _iter_inheritance_queryset(self.queryset) -class InheritanceQuerySetMixin(Generic[ModelT]): +class InheritanceQuerySetMixin(_GenericMixin): model: type[ModelT] subclasses: Sequence[str] @@ -227,7 +240,7 @@ def instance_of(self, *models: type[ModelT]) -> InheritanceQuerySet[ModelT]: ) -class InheritanceManagerMixin(Generic[ModelT]): +class InheritanceManagerMixin(_GenericMixin): _queryset_class = InheritanceQuerySet if TYPE_CHECKING: @@ -323,7 +336,7 @@ class InheritanceManager(InheritanceManagerMixin[ModelT], models.Manager[ModelT] pass -class QueryManagerMixin(Generic[ModelT]): +class QueryManagerMixin(_GenericMixin): @overload def __init__(self, *args: models.Q): @@ -357,7 +370,7 @@ class QueryManager(QueryManagerMixin[ModelT], models.Manager[ModelT]): # type: pass -class SoftDeletableQuerySetMixin(Generic[ModelT]): +class SoftDeletableQuerySetMixin(_GenericMixin): """ QuerySet for SoftDeletableModel. Instead of removing instance sets its ``is_removed`` field to True. @@ -377,7 +390,7 @@ class SoftDeletableQuerySet(SoftDeletableQuerySetMixin[ModelT], QuerySet[ModelT] pass -class SoftDeletableManagerMixin(Generic[ModelT]): +class SoftDeletableManagerMixin(_GenericMixin): """ Manager that limits the queryset by default to show only not removed instances of model. diff --git a/tests/test_managers/test_manager_class_assignment.py b/tests/test_managers/test_manager_class_assignment.py new file mode 100644 index 00000000..8e96324f --- /dev/null +++ b/tests/test_managers/test_manager_class_assignment.py @@ -0,0 +1,66 @@ +from __future__ import annotations + +from django.db import models +from django.test import SimpleTestCase + +from model_utils.managers import ( + InheritanceManager, + JoinManager, + QueryManager, + SoftDeletableManager, +) + + +class ManagerClassAssignmentTests(SimpleTestCase): + """ + Tests for manager __class__ assignment compatibility. + + This is a regression test for GitHub issue #636 where manager mixins + inheriting from Generic[T] at runtime were incompatible with + django-modeltranslation due to Python's __class__ assignment restrictions. + + The fix moves Generic[T] inheritance behind TYPE_CHECKING so it's only + used for static type checking, not at runtime. + + See: https://github.com/jazzband/django-model-utils/issues/636 + """ + + def test_softdeletable_manager_class_can_be_reassigned(self) -> None: + """SoftDeletableManager instances support __class__ reassignment.""" + manager = SoftDeletableManager() + + class PatchedManager(SoftDeletableManager): + pass + + manager.__class__ = PatchedManager + self.assertIsInstance(manager, PatchedManager) + + def test_inheritance_manager_class_can_be_reassigned(self) -> None: + """InheritanceManager instances support __class__ reassignment.""" + manager = InheritanceManager() + + class PatchedManager(InheritanceManager): + pass + + manager.__class__ = PatchedManager + self.assertIsInstance(manager, PatchedManager) + + def test_query_manager_class_can_be_reassigned(self) -> None: + """QueryManager instances support __class__ reassignment.""" + manager = QueryManager(is_active=True) + + class PatchedManager(models.Manager): + pass + + manager.__class__ = PatchedManager + self.assertIsInstance(manager, PatchedManager) + + def test_join_manager_class_can_be_reassigned(self) -> None: + """JoinManager instances support __class__ reassignment.""" + manager = JoinManager() + + class PatchedManager(models.Manager): + pass + + manager.__class__ = PatchedManager + self.assertIsInstance(manager, PatchedManager)