From 966a29c2b83fed2fc7c2b5b09de80cbebc94ac74 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Tue, 17 Mar 2015 07:42:53 -0700 Subject: [PATCH] Fixed #24479 -- Added system check to prevent both ordering and order_wrt. --- django/db/models/base.py | 15 +++++--- django/db/models/options.py | 4 +++ docs/ref/checks.txt | 2 ++ docs/releases/1.9.txt | 3 ++ tests/invalid_models_tests/test_models.py | 44 +++++++++++++++++++++++ 5 files changed, 64 insertions(+), 4 deletions(-) diff --git a/django/db/models/base.py b/django/db/models/base.py index 07a4f5f7e1..0a9a837692 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -1479,7 +1479,17 @@ class Model(six.with_metaclass(ModelBase)): def _check_ordering(cls): """ Check "ordering" option -- is it a list of strings and do all fields exist? """ - if not cls._meta.ordering: + if cls._meta._ordering_clash: + return [ + checks.Error( + "'ordering' and 'order_with_respect_to' cannot be used together.", + hint=None, + obj=cls, + id='models.E021', + ), + ] + + if cls._meta.order_with_respect_to or not cls._meta.ordering: return [] if not isinstance(cls._meta.ordering, (list, tuple)): @@ -1502,9 +1512,6 @@ class Model(six.with_metaclass(ModelBase)): # Convert "-field" to "field". fields = ((f[1:] if f.startswith('-') else f) for f in fields) - fields = (f for f in fields if - f != '_order' or not cls._meta.order_with_respect_to) - # Skip ordering in the format field1__field2 (FIXME: checking # this format would be nice, but it's a little fiddly). fields = (f for f in fields if '__' not in f) diff --git a/django/db/models/options.py b/django/db/models/options.py index 0762aa8df3..4a2ab1036f 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -100,6 +100,7 @@ class Options(object): self.verbose_name_plural = None self.db_table = '' self.ordering = [] + self._ordering_clash = False self.unique_together = [] self.index_together = [] self.select_on_save = False @@ -234,6 +235,9 @@ class Options(object): if self.verbose_name_plural is None: self.verbose_name_plural = string_concat(self.verbose_name, 's') + # order_with_respect_and ordering are mutually exclusive. + self._ordering_clash = bool(self.ordering and self.order_with_respect_to) + # Any leftover attributes must be invalid. if meta_attrs != {}: raise TypeError("'class Meta' got invalid attribute(s): %s" % ','.join(meta_attrs.keys())) diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 9ba3c492f7..eb6fa688a2 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -63,6 +63,8 @@ Models ````. Maximum length is ```` for database ````. * **models.E020**: The ``.check()`` class method is currently overridden. +* **models.E021**: ``ordering`` and ``order_with_respect_to`` cannot be used + together. Fields ~~~~~~ diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 6ec8a3f455..3fa068c97f 100644 --- a/docs/releases/1.9.txt +++ b/docs/releases/1.9.txt @@ -164,6 +164,9 @@ Models ` to allow deleting only a child's data in a model that uses multi-table inheritance. +* Added a system check to prevent defining both ``Meta.ordering`` and + ``order_with_respect_to`` on the same model. + CSRF ^^^^ diff --git a/tests/invalid_models_tests/test_models.py b/tests/invalid_models_tests/test_models.py index 87a099f3a9..3bc6856480 100644 --- a/tests/invalid_models_tests/test_models.py +++ b/tests/invalid_models_tests/test_models.py @@ -566,6 +566,50 @@ class OtherModelTests(IsolatedModelsTestCase): ] self.assertEqual(errors, expected) + def test_just_ordering_no_errors(self): + class Model(models.Model): + order = models.PositiveIntegerField() + + class Meta: + ordering = ['order'] + + self.assertEquals(Model.check(), []) + + def test_just_order_with_respect_to_no_errors(self): + class Question(models.Model): + pass + + class Answer(models.Model): + question = models.ForeignKey(Question) + + class Meta: + order_with_respect_to = 'question' + + self.assertEquals(Answer.check(), []) + + def test_ordering_with_order_with_respect_to(self): + class Question(models.Model): + pass + + class Answer(models.Model): + question = models.ForeignKey(Question) + order = models.IntegerField() + + class Meta: + order_with_respect_to = 'question' + ordering = ['order'] + + errors = Answer.check() + expected = [ + Error( + "'ordering' and 'order_with_respect_to' cannot be used together.", + hint=None, + obj=Answer, + id='models.E021', + ), + ] + self.assertEqual(errors, expected) + def test_non_valid(self): class RelationModel(models.Model): pass