mirror of
https://github.com/Hopiu/django-model-utils.git
synced 2026-03-16 20:00:23 +00:00
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.
This commit is contained in:
parent
af7c211f26
commit
f09ea0e472
4 changed files with 88 additions and 5 deletions
|
|
@ -12,6 +12,7 @@
|
||||||
| Arseny Sysolyatin <phelansav@gmail.com>
|
| Arseny Sysolyatin <phelansav@gmail.com>
|
||||||
| Artis Avotins <artis.avotins@gmail.com>
|
| Artis Avotins <artis.avotins@gmail.com>
|
||||||
| Asif Saif Uddin <auvipy@gmail.com>
|
| Asif Saif Uddin <auvipy@gmail.com>
|
||||||
|
| Benedikt Willi <ben.willi@gmail.com>
|
||||||
| Bo Marchman <bo.marchman@gmail.com>
|
| Bo Marchman <bo.marchman@gmail.com>
|
||||||
| Bojan Mihelac <bmihelac@mihelac.org>
|
| Bojan Mihelac <bmihelac@mihelac.org>
|
||||||
| Bruno Alla <bruno.alla@founders4schools.org.uk>
|
| Bruno Alla <bruno.alla@founders4schools.org.uk>
|
||||||
|
|
|
||||||
|
|
@ -6,6 +6,9 @@ To be released
|
||||||
- Add support for `Python 3.13` (GH-#628)
|
- Add support for `Python 3.13` (GH-#628)
|
||||||
- Add formal support for `Django 5.2` (GH-#641)
|
- Add formal support for `Django 5.2` (GH-#641)
|
||||||
- Drop support for older versions than `Django 4.2`
|
- 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)
|
5.0.0 (2024-09-01)
|
||||||
------------------
|
------------------
|
||||||
|
|
|
||||||
|
|
@ -17,6 +17,19 @@ if TYPE_CHECKING:
|
||||||
|
|
||||||
from django.db.models.query import BaseIterable
|
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]:
|
def _iter_inheritance_queryset(queryset: QuerySet[ModelT]) -> Iterator[ModelT]:
|
||||||
iter: ModelIterable[ModelT] = ModelIterable(queryset)
|
iter: ModelIterable[ModelT] = ModelIterable(queryset)
|
||||||
|
|
@ -63,7 +76,7 @@ else:
|
||||||
return _iter_inheritance_queryset(self.queryset)
|
return _iter_inheritance_queryset(self.queryset)
|
||||||
|
|
||||||
|
|
||||||
class InheritanceQuerySetMixin(Generic[ModelT]):
|
class InheritanceQuerySetMixin(_GenericMixin):
|
||||||
|
|
||||||
model: type[ModelT]
|
model: type[ModelT]
|
||||||
subclasses: Sequence[str]
|
subclasses: Sequence[str]
|
||||||
|
|
@ -227,7 +240,7 @@ class InheritanceQuerySet(InheritanceQuerySetMixin[ModelT], QuerySet[ModelT]):
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
class InheritanceManagerMixin(Generic[ModelT]):
|
class InheritanceManagerMixin(_GenericMixin):
|
||||||
_queryset_class = InheritanceQuerySet
|
_queryset_class = InheritanceQuerySet
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
|
|
@ -323,7 +336,7 @@ class InheritanceManager(InheritanceManagerMixin[ModelT], models.Manager[ModelT]
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
class QueryManagerMixin(Generic[ModelT]):
|
class QueryManagerMixin(_GenericMixin):
|
||||||
|
|
||||||
@overload
|
@overload
|
||||||
def __init__(self, *args: models.Q):
|
def __init__(self, *args: models.Q):
|
||||||
|
|
@ -357,7 +370,7 @@ class QueryManager(QueryManagerMixin[ModelT], models.Manager[ModelT]): # type:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
class SoftDeletableQuerySetMixin(Generic[ModelT]):
|
class SoftDeletableQuerySetMixin(_GenericMixin):
|
||||||
"""
|
"""
|
||||||
QuerySet for SoftDeletableModel. Instead of removing instance sets
|
QuerySet for SoftDeletableModel. Instead of removing instance sets
|
||||||
its ``is_removed`` field to True.
|
its ``is_removed`` field to True.
|
||||||
|
|
@ -377,7 +390,7 @@ class SoftDeletableQuerySet(SoftDeletableQuerySetMixin[ModelT], QuerySet[ModelT]
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
class SoftDeletableManagerMixin(Generic[ModelT]):
|
class SoftDeletableManagerMixin(_GenericMixin):
|
||||||
"""
|
"""
|
||||||
Manager that limits the queryset by default to show only not removed
|
Manager that limits the queryset by default to show only not removed
|
||||||
instances of model.
|
instances of model.
|
||||||
|
|
|
||||||
66
tests/test_managers/test_manager_class_assignment.py
Normal file
66
tests/test_managers/test_manager_class_assignment.py
Normal file
|
|
@ -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)
|
||||||
Loading…
Reference in a new issue