From ae71f9080e3eb83b3d2a98294b28dd3ba711196b Mon Sep 17 00:00:00 2001 From: Keryn Knight Date: Tue, 22 Oct 2013 22:01:28 +0100 Subject: [PATCH 1/4] Tests for #59 - a manually specified OneToOne should behave the same as an implicit one. --- model_utils/tests/models.py | 9 +++++ model_utils/tests/tests.py | 75 ++++++++++++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/model_utils/tests/models.py b/model_utils/tests/models.py index 88897f9..5d13f46 100644 --- a/model_utils/tests/models.py +++ b/model_utils/tests/models.py @@ -44,20 +44,29 @@ class InheritanceManagerTestChild1(InheritanceManagerTestParent): objects = InheritanceManager() + class InheritanceManagerTestGrandChild1(InheritanceManagerTestChild1): text_field = models.TextField() + class InheritanceManagerTestGrandChild1_2(InheritanceManagerTestChild1): text_field = models.TextField() + class InheritanceManagerTestChild2(InheritanceManagerTestParent): non_related_field_using_descriptor_2 = models.FileField(upload_to="test") normal_field_2 = models.TextField() +class InheritanceManagerTestChild3(InheritanceManagerTestParent): + parent_ptr = models.OneToOneField( + InheritanceManagerTestParent, related_name='manual_onetoone', + parent_link=True) + + class TimeStamp(TimeStampedModel): pass diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index 4dda351..5d66d75 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -27,7 +27,8 @@ from model_utils.tests.models import ( TimeFrameManagerAdded, Dude, SplitFieldAbstractParent, Car, Spot, ModelTracked, ModelTrackedFK, ModelTrackedNotDefault, ModelTrackedMultiple, InheritedModelTracked, Tracked, TrackedFK, TrackedNotDefault, TrackedNonFieldAttr, TrackedMultiple, - InheritedTracked, StatusFieldDefaultFilled, StatusFieldDefaultNotFilled) + InheritedTracked, StatusFieldDefaultFilled, StatusFieldDefaultNotFilled, + InheritanceManagerTestChild3) class GetExcerptTests(TestCase): @@ -683,6 +684,55 @@ class InheritanceManagerTests(TestCase): self.assertEqual(1, self.get_manager().all()._get_maximum_depth()) + @skipUnless(django.VERSION < (1, 6, 0), "test only applies to Django < 1.6") + def test_manually_specifying_parent_fk_only_children(self): + """ + given a Model which inherits from another Model, but also declares + the OneToOne link manually using `related_name` and `parent_link`, + ensure that the relation names and subclasses are obtained correctly. + """ + child3 = InheritanceManagerTestChild3.objects.create() + results = InheritanceManagerTestParent.objects.all().select_subclasses() + + expected_objs = [self.child1, self.child2, + InheritanceManagerTestChild1(pk=3), + InheritanceManagerTestChild1(pk=4), child3] + self.assertEqual(list(results), expected_objs) + + expected_related_names = [ + 'inheritancemanagertestchild1', + 'inheritancemanagertestchild2', + 'manual_onetoone', # this was set via parent_link & related_name + ] + self.assertEqual(set(results.subclasses), + set(expected_related_names)) + + @skipUnless(django.VERSION >= (1, 6, 0), "test only applies to Django 1.6+") + def test_manually_specifying_parent_fk_including_grandchildren(self): + """ + given a Model which inherits from another Model, but also declares + the OneToOne link manually using `related_name` and `parent_link`, + ensure that the relation names and subclasses are obtained correctly. + """ + child3 = InheritanceManagerTestChild3.objects.create() + results = InheritanceManagerTestParent.objects.all().select_subclasses() + + expected_objs = [self.child1, self.child2, self.grandchild1, + self.grandchild1_2, child3] + self.assertEqual(list(results), expected_objs) + + expected_related_names = [ + 'inheritancemanagertestchild1__inheritancemanagertestgrandchild1', + 'inheritancemanagertestchild1__inheritancemanagertestgrandchild1_2', + 'inheritancemanagertestchild1', + 'inheritancemanagertestchild2', + 'manual_onetoone', # this was set via parent_link & related_name + ] + self.assertEqual(set(results.subclasses), + set(expected_related_names)) + + + class InheritanceManagerUsingModelsTests(TestCase): def setUp(self): @@ -873,6 +923,29 @@ class InheritanceManagerUsingModelsTests(TestCase): ], list(objs)) + def test_manually_specifying_parent_fk_only_specific_child(self): + """ + given a Model which inherits from another Model, but also declares + the OneToOne link manually using `related_name` and `parent_link`, + ensure that the relation names and subclasses are obtained correctly. + """ + child3 = InheritanceManagerTestChild3.objects.create() + results = InheritanceManagerTestParent.objects.all().select_subclasses( + InheritanceManagerTestChild3) + + expected_objs = [InheritanceManagerTestParent(pk=1), + InheritanceManagerTestParent(pk=2), + InheritanceManagerTestParent(pk=3), + InheritanceManagerTestParent(pk=4), + InheritanceManagerTestParent(pk=5), + child3] + self.assertEqual(list(results), expected_objs) + + expected_related_names = ['manual_onetoone'] + self.assertEqual(set(results.subclasses), + set(expected_related_names)) + + class InheritanceManagerRelatedTests(InheritanceManagerTests): def setUp(self): From 262455a60dd7a45779817545791b213bc4ac701b Mon Sep 17 00:00:00 2001 From: Keryn Knight Date: Sat, 26 Oct 2013 15:47:48 +0100 Subject: [PATCH 2/4] Update model tests to include our new InheritanceManagerTestChild3 as part of the emulation of select_subclasses. Test still fails because of #59. --- model_utils/tests/tests.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index 5d66d75..17fe2f2 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -781,6 +781,7 @@ class InheritanceManagerUsingModelsTests(TestCase): objs = InheritanceManagerTestParent.objects.select_subclasses().order_by('pk') objsmodels = InheritanceManagerTestParent.objects.select_subclasses( InheritanceManagerTestChild1, InheritanceManagerTestChild2, + InheritanceManagerTestChild3, InheritanceManagerTestGrandChild1, InheritanceManagerTestGrandChild1_2).order_by('pk') self.assertEqual(set(objs.subclasses), set(objsmodels.subclasses)) @@ -801,11 +802,15 @@ class InheritanceManagerUsingModelsTests(TestCase): objs = InheritanceManagerTestParent.objects.select_subclasses().order_by('pk') if django.VERSION >= (1, 6, 0): - models = (InheritanceManagerTestChild1, InheritanceManagerTestChild2, + models = (InheritanceManagerTestChild1, + InheritanceManagerTestChild2, + InheritanceManagerTestChild3, InheritanceManagerTestGrandChild1, InheritanceManagerTestGrandChild1_2) else: - models = (InheritanceManagerTestChild1, InheritanceManagerTestChild2) + models = (InheritanceManagerTestChild1, + InheritanceManagerTestChild2, + InheritanceManagerTestChild3) objsmodels = InheritanceManagerTestParent.objects.select_subclasses( *models).order_by('pk') From a270eef1fd3420e91a5a2516625f612b9a204229 Mon Sep 17 00:00:00 2001 From: Keryn Knight Date: Sat, 26 Oct 2013 15:53:15 +0100 Subject: [PATCH 3/4] Fixed #59 - manually setting the parent relation via a OneToOne should present the same behaviour as the implicit ptr Django generates on subclasses. Thanks to Eran Rundstein for reporting the issue and proposing the fix. --- model_utils/managers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/model_utils/managers.py b/model_utils/managers.py index f0e34ee..9ac0317 100644 --- a/model_utils/managers.py +++ b/model_utils/managers.py @@ -110,8 +110,8 @@ class InheritanceQuerySet(QuerySet): if levels or levels is None: for subclass in self._get_subclasses_recurse( rel.field.model, levels=levels): - subclasses.append(rel.var_name + LOOKUP_SEP + subclass) - subclasses.append(rel.var_name) + subclasses.append(rel.get_accessor_name() + LOOKUP_SEP + subclass) + subclasses.append(rel.get_accessor_name()) return subclasses @@ -130,7 +130,7 @@ class InheritanceQuerySet(QuerySet): if levels: levels -= 1 while parent is not None: - ancestry.insert(0, parent.related.var_name) + ancestry.insert(0, parent.related.get_accessor_name()) if levels or levels is None: parent = parent.related.parent_model._meta.get_ancestor_link( self.model) From dd469a0e8f8c69d6cdf7510330239802194e24bb Mon Sep 17 00:00:00 2001 From: Keryn Knight Date: Sat, 26 Oct 2013 16:34:21 +0100 Subject: [PATCH 4/4] Added test to explicitly demonstrate manually defined related_name usage works in select_subclasses going forwards. Also fixed fragility of 2 tests introduced in ae71f9080e3eb83b3d2a98294b28dd3ba711196b --- model_utils/tests/tests.py | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/model_utils/tests/tests.py b/model_utils/tests/tests.py index 17fe2f2..5cb0413 100644 --- a/model_utils/tests/tests.py +++ b/model_utils/tests/tests.py @@ -695,8 +695,9 @@ class InheritanceManagerTests(TestCase): results = InheritanceManagerTestParent.objects.all().select_subclasses() expected_objs = [self.child1, self.child2, - InheritanceManagerTestChild1(pk=3), - InheritanceManagerTestChild1(pk=4), child3] + InheritanceManagerTestChild1(pk=self.grandchild1.pk), + InheritanceManagerTestChild1(pk=self.grandchild1_2.pk), + child3] self.assertEqual(list(results), expected_objs) expected_related_names = [ @@ -707,6 +708,7 @@ class InheritanceManagerTests(TestCase): self.assertEqual(set(results.subclasses), set(expected_related_names)) + @skipUnless(django.VERSION >= (1, 6, 0), "test only applies to Django 1.6+") def test_manually_specifying_parent_fk_including_grandchildren(self): """ @@ -732,6 +734,25 @@ class InheritanceManagerTests(TestCase): set(expected_related_names)) + def test_manually_specifying_parent_fk_single_subclass(self): + """ + Using a string related_name when the relation is manually defined + instead of implicit should still work in the same way. + """ + related_name = 'manual_onetoone' + child3 = InheritanceManagerTestChild3.objects.create() + results = InheritanceManagerTestParent.objects.all().select_subclasses(related_name) + + expected_objs = [InheritanceManagerTestParent(pk=self.child1.pk), + InheritanceManagerTestParent(pk=self.child2.pk), + InheritanceManagerTestParent(pk=self.grandchild1.pk), + InheritanceManagerTestParent(pk=self.grandchild1_2.pk), + child3] + self.assertEqual(list(results), expected_objs) + expected_related_names = [related_name] + self.assertEqual(set(results.subclasses), + set(expected_related_names)) + class InheritanceManagerUsingModelsTests(TestCase): @@ -938,11 +959,11 @@ class InheritanceManagerUsingModelsTests(TestCase): results = InheritanceManagerTestParent.objects.all().select_subclasses( InheritanceManagerTestChild3) - expected_objs = [InheritanceManagerTestParent(pk=1), - InheritanceManagerTestParent(pk=2), - InheritanceManagerTestParent(pk=3), - InheritanceManagerTestParent(pk=4), - InheritanceManagerTestParent(pk=5), + expected_objs = [InheritanceManagerTestParent(pk=self.parent1.pk), + InheritanceManagerTestParent(pk=self.child1.pk), + InheritanceManagerTestParent(pk=self.child2.pk), + InheritanceManagerTestParent(pk=self.grandchild1.pk), + InheritanceManagerTestParent(pk=self.grandchild1_2.pk), child3] self.assertEqual(list(results), expected_objs)