From 33f0ca9f6235b46a70caa69ab6bc74d6dd9fc8ab Mon Sep 17 00:00:00 2001 From: Dirk Groten Date: Sun, 20 Oct 2019 18:24:57 +0200 Subject: [PATCH 1/5] Fix creation_counter on the patched managers in order to ensure the original local managers are prioritised over the inherited managers. --- modeltranslation/translator.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/modeltranslation/translator.py b/modeltranslation/translator.py index 744ceed..3194b60 100644 --- a/modeltranslation/translator.py +++ b/modeltranslation/translator.py @@ -203,11 +203,17 @@ def add_manager(model): if model._meta.abstract: return # Make all managers local for this model to fix patching parent model managers + added = set(model._meta.managers) - set(model._meta.local_managers) model._meta.local_managers = model._meta.managers for current_manager in model._meta.local_managers: prev_class = current_manager.__class__ patch_manager_class(current_manager) + if current_manager in added: + # Since default_manager is fetched by order of creation, any manager + # moved from parent class to child class needs to receive a new creation_counter + # in order to be ordered after the original local managers + current_manager._set_creation_counter() if model._default_manager.__class__ is prev_class: # Normally model._default_manager is a reference to one of model's managers # (and would be patched by the way). From b6d4cf81d34be07a200017a88770e291d5ec7a11 Mon Sep 17 00:00:00 2001 From: Dirk Groten Date: Sun, 20 Oct 2019 19:18:27 +0200 Subject: [PATCH 2/5] Test to check the default manager after registering model for translation --- modeltranslation/tests/models.py | 21 +++++++++++++++++++++ modeltranslation/tests/tests.py | 4 ++++ modeltranslation/tests/translation.py | 3 +++ 3 files changed, 28 insertions(+) diff --git a/modeltranslation/tests/models.py b/modeltranslation/tests/models.py index 4b40982..bc25d9b 100644 --- a/modeltranslation/tests/models.py +++ b/modeltranslation/tests/models.py @@ -313,6 +313,27 @@ class CustomManager2TestModel(models.Model): objects = CustomManager2() +class CustomManagerAbstract(models.Manager): + def to_translate(self): + return self.get_queryset().filter(needs_translation=True) + + +class CustomManagerBaseModel(models.Model): + needs_translation = models.BooleanField(default=False) + + objects = models.Manager() + translations = CustomManagerAbstract() + + class Meta: + abstract = True + + +class CustomAbstractManagerTestModel(CustomManagerBaseModel): + title = models.CharField(ugettext_lazy('title'), max_length=255) + + objects = CustomManager2() + + # ######### Required fields testing class RequiredModel(models.Model): diff --git a/modeltranslation/tests/tests.py b/modeltranslation/tests/tests.py index 0c79dd0..5002422 100644 --- a/modeltranslation/tests/tests.py +++ b/modeltranslation/tests/tests.py @@ -2730,6 +2730,10 @@ class TestManager(ModeltranslationTestBase): manager = models.CustomManagerTestModel.another_mgr_name self.assertTrue(isinstance(manager, MultilingualManager)) + def test_default_manager_for_inherited_models(self): + manager = models.CustomAbstractManagerTestModel()._meta.default_manager + self.assertEqual('objects', manager.name) + def test_custom_manager2(self): """Test if user-defined queryset is still working""" from modeltranslation.manager import MultilingualManager, MultilingualQuerySet diff --git a/modeltranslation/tests/translation.py b/modeltranslation/tests/translation.py index 402217b..3f98fad 100644 --- a/modeltranslation/tests/translation.py +++ b/modeltranslation/tests/translation.py @@ -137,11 +137,14 @@ class ManagerTestModelTranslationOptions(TranslationOptions): @register([ models.CustomManagerTestModel, models.CustomManager2TestModel, + models.CustomAbstractManagerTestModel ]) class CustomManagerTestModelTranslationOptions(TranslationOptions): fields = ('title',) + + # ######### TranslationOptions field inheritance testing class FieldInheritanceATranslationOptions(TranslationOptions): From 50a7868fa7f03a3d75b549baa06e377318210b47 Mon Sep 17 00:00:00 2001 From: Dirk Groten Date: Sun, 20 Oct 2019 19:24:06 +0200 Subject: [PATCH 3/5] Documented tests better --- modeltranslation/tests/tests.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/modeltranslation/tests/tests.py b/modeltranslation/tests/tests.py index 5002422..979a8c2 100644 --- a/modeltranslation/tests/tests.py +++ b/modeltranslation/tests/tests.py @@ -25,6 +25,7 @@ from django.apps import apps as django_apps from modeltranslation import admin, settings as mt_settings, translator from modeltranslation.forms import TranslationModelForm +from modeltranslation.manager import MultilingualManager from modeltranslation.models import autodiscover from modeltranslation.tests.test_settings import TEST_SETTINGS from modeltranslation.utils import (build_css_class, build_localized_fieldname, @@ -39,7 +40,7 @@ models = translation = None request = None # How many models are registered for tests. -TEST_MODELS = 31 + (1 if MIGRATIONS else 0) +TEST_MODELS = 32 + (1 if MIGRATIONS else 0) class reload_override_settings(override_settings): @@ -2731,8 +2732,11 @@ class TestManager(ModeltranslationTestBase): self.assertTrue(isinstance(manager, MultilingualManager)) def test_default_manager_for_inherited_models(self): - manager = models.CustomAbstractManagerTestModel()._meta.default_manager + """Test if default manager is still set from local managers""" + manager = models.CustomAbstractManagerTestModel._meta.default_manager self.assertEqual('objects', manager.name) + self.assertTrue(isinstance(manager, MultilingualManager)) + self.assertTrue(isinstance(models.CustomAbstractManagerTestModel.translations, MultilingualManager)) def test_custom_manager2(self): """Test if user-defined queryset is still working""" From f42d3404e2b3036582b61947e047582e0549948b Mon Sep 17 00:00:00 2001 From: Dirk Groten Date: Sun, 20 Oct 2019 20:06:44 +0200 Subject: [PATCH 4/5] Added test to make sure inherited models with no custom manager still have the correct default manager --- modeltranslation/tests/models.py | 11 +++++++---- modeltranslation/tests/tests.py | 13 +++++++++---- modeltranslation/tests/translation.py | 3 ++- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/modeltranslation/tests/models.py b/modeltranslation/tests/models.py index bc25d9b..36f104b 100644 --- a/modeltranslation/tests/models.py +++ b/modeltranslation/tests/models.py @@ -314,26 +314,29 @@ class CustomManager2TestModel(models.Model): class CustomManagerAbstract(models.Manager): - def to_translate(self): - return self.get_queryset().filter(needs_translation=True) + pass class CustomManagerBaseModel(models.Model): needs_translation = models.BooleanField(default=False) - objects = models.Manager() + objects = models.Manager() # ensures objects is the default manager translations = CustomManagerAbstract() class Meta: abstract = True -class CustomAbstractManagerTestModel(CustomManagerBaseModel): +class CustomManagerChildTestModel(CustomManagerBaseModel): title = models.CharField(ugettext_lazy('title'), max_length=255) objects = CustomManager2() +class PlainChildTestModel(CustomManagerBaseModel): + title = models.CharField(ugettext_lazy('title'), max_length=255) + + # ######### Required fields testing class RequiredModel(models.Model): diff --git a/modeltranslation/tests/tests.py b/modeltranslation/tests/tests.py index 979a8c2..ee7f6f9 100644 --- a/modeltranslation/tests/tests.py +++ b/modeltranslation/tests/tests.py @@ -40,7 +40,7 @@ models = translation = None request = None # How many models are registered for tests. -TEST_MODELS = 32 + (1 if MIGRATIONS else 0) +TEST_MODELS = 33 + (1 if MIGRATIONS else 0) class reload_override_settings(override_settings): @@ -2731,12 +2731,17 @@ class TestManager(ModeltranslationTestBase): manager = models.CustomManagerTestModel.another_mgr_name self.assertTrue(isinstance(manager, MultilingualManager)) - def test_default_manager_for_inherited_models(self): + def test_default_manager_for_inherited_models_with_custom_manager(self): """Test if default manager is still set from local managers""" - manager = models.CustomAbstractManagerTestModel._meta.default_manager + manager = models.CustomManagerChildTestModel._meta.default_manager self.assertEqual('objects', manager.name) self.assertTrue(isinstance(manager, MultilingualManager)) - self.assertTrue(isinstance(models.CustomAbstractManagerTestModel.translations, MultilingualManager)) + self.assertTrue(isinstance(models.CustomManagerChildTestModel.translations, MultilingualManager)) + + def test_default_manager_for_inherited_models(self): + manager = models.PlainChildTestModel._meta.default_manager + self.assertEqual('objects', manager.name) + self.assertTrue(isinstance(models.PlainChildTestModel.translations, MultilingualManager)) def test_custom_manager2(self): """Test if user-defined queryset is still working""" diff --git a/modeltranslation/tests/translation.py b/modeltranslation/tests/translation.py index 3f98fad..a101c37 100644 --- a/modeltranslation/tests/translation.py +++ b/modeltranslation/tests/translation.py @@ -137,7 +137,8 @@ class ManagerTestModelTranslationOptions(TranslationOptions): @register([ models.CustomManagerTestModel, models.CustomManager2TestModel, - models.CustomAbstractManagerTestModel + models.CustomManagerChildTestModel, + models.PlainChildTestModel ]) class CustomManagerTestModelTranslationOptions(TranslationOptions): fields = ('title',) From 2a92d6c7b48083fc2eb378cccce5cbac2feca438 Mon Sep 17 00:00:00 2001 From: Dirk Groten Date: Sun, 20 Oct 2019 20:14:52 +0200 Subject: [PATCH 5/5] Fix flake8 formatting errors --- modeltranslation/tests/tests.py | 4 +++- modeltranslation/tests/translation.py | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modeltranslation/tests/tests.py b/modeltranslation/tests/tests.py index ee7f6f9..cd151d7 100644 --- a/modeltranslation/tests/tests.py +++ b/modeltranslation/tests/tests.py @@ -2736,7 +2736,9 @@ class TestManager(ModeltranslationTestBase): manager = models.CustomManagerChildTestModel._meta.default_manager self.assertEqual('objects', manager.name) self.assertTrue(isinstance(manager, MultilingualManager)) - self.assertTrue(isinstance(models.CustomManagerChildTestModel.translations, MultilingualManager)) + self.assertTrue(isinstance( + models.CustomManagerChildTestModel.translations, + MultilingualManager)) def test_default_manager_for_inherited_models(self): manager = models.PlainChildTestModel._meta.default_manager diff --git a/modeltranslation/tests/translation.py b/modeltranslation/tests/translation.py index a101c37..cd875d7 100644 --- a/modeltranslation/tests/translation.py +++ b/modeltranslation/tests/translation.py @@ -144,8 +144,6 @@ class CustomManagerTestModelTranslationOptions(TranslationOptions): fields = ('title',) - - # ######### TranslationOptions field inheritance testing class FieldInheritanceATranslationOptions(TranslationOptions):