Fixed #19195 -- Allow explicit ordering by a relation `_id` field.

Thanks to chrisedgemon for the report and shaib, akaariai and
timgraham for the review.

2 files changed, 80 insertions, 48 deletions

diff --git a/tests/ordering/models.py b/tests/ordering/models.py
index 415b28bb41..2ab16f3c5f 100644
--- a/tests/ordering/models.py
+++ b/tests/ordering/models.py
@@ -17,25 +17,19 @@ from django.db import models
 from django.utils.encoding import python_2_unicode_compatible
 
 
-@python_2_unicode_compatible
-class Article(models.Model):
-    headline = models.CharField(max_length=100)
-    pub_date = models.DateTimeField()
-
+class Author(models.Model):
     class Meta:
-        ordering = ('-pub_date', 'headline')
-
-    def __str__(self):
-        return self.headline
+        ordering = ('-pk',)
 
 
 @python_2_unicode_compatible
-class ArticlePKOrdering(models.Model):
+class Article(models.Model):
+    author = models.ForeignKey(Author, null=True)
     headline = models.CharField(max_length=100)
     pub_date = models.DateTimeField()
 
     class Meta:
-        ordering = ('-pk',)
+        ordering = ('-pub_date', 'headline')
 
     def __str__(self):
         return self.headline
diff --git a/tests/ordering/tests.py b/tests/ordering/tests.py
index 95631af5a7..5d323e006b 100644
--- a/tests/ordering/tests.py
+++ b/tests/ordering/tests.py
@@ -5,26 +5,29 @@ from operator import attrgetter
 
 from django.test import TestCase
 
-from .models import Article, ArticlePKOrdering
+from .models import Article, Author
 
 
 class OrderingTests(TestCase):
-    def test_basic(self):
-        Article.objects.create(
+    def setUp(self):
+        self.a1 = Article.objects.create(
             headline="Article 1", pub_date=datetime(2005, 7, 26)
         )
-        Article.objects.create(
+        self.a2 = Article.objects.create(
             headline="Article 2", pub_date=datetime(2005, 7, 27)
         )
-        Article.objects.create(
+        self.a3 = Article.objects.create(
             headline="Article 3", pub_date=datetime(2005, 7, 27)
         )
-        a4 = Article.objects.create(
+        self.a4 = Article.objects.create(
             headline="Article 4", pub_date=datetime(2005, 7, 28)
         )
 
-        # By default, Article.objects.all() orders by pub_date descending, then
-        # headline ascending.
+    def test_default_ordering(self):
+        """
+        By default, Article.objects.all() orders by pub_date descending, then
+        headline ascending.
+        """
         self.assertQuerysetEqual(
             Article.objects.all(), [
                 "Article 4",
@@ -35,8 +38,14 @@ class OrderingTests(TestCase):
             attrgetter("headline")
         )
 
-        # Override ordering with order_by, which is in the same format as the
-        # ordering attribute in models.
+        # Getting a single item should work too:
+        self.assertEqual(Article.objects.all()[0], self.a4)
+
+    def test_default_ordering_override(self):
+        """
+        Override ordering with order_by, which is in the same format as the
+        ordering attribute in models.
+        """
         self.assertQuerysetEqual(
             Article.objects.order_by("headline"), [
                 "Article 1",
@@ -56,8 +65,11 @@ class OrderingTests(TestCase):
             attrgetter("headline")
         )
 
-        # Only the last order_by has any effect (since they each override any
-        # previous ordering).
+    def test_order_by_override(self):
+        """
+        Only the last order_by has any effect (since they each override any
+        previous ordering).
+        """
         self.assertQuerysetEqual(
             Article.objects.order_by("id"), [
                 "Article 1",
@@ -77,7 +89,10 @@ class OrderingTests(TestCase):
             attrgetter("headline")
         )
 
-        # Use the 'stop' part of slicing notation to limit the results.
+    def test_stop_slicing(self):
+        """
+        Use the 'stop' part of slicing notation to limit the results.
+        """
         self.assertQuerysetEqual(
             Article.objects.order_by("headline")[:2], [
                 "Article 1",
@@ -86,8 +101,11 @@ class OrderingTests(TestCase):
             attrgetter("headline")
         )
 
-        # Use the 'stop' and 'start' parts of slicing notation to offset the
-        # result list.
+    def test_stop_start_slicing(self):
+        """
+        Use the 'stop' and 'start' parts of slicing notation to offset the
+        result list.
+        """
         self.assertQuerysetEqual(
             Article.objects.order_by("headline")[1:3], [
                 "Article 2",
@@ -96,17 +114,20 @@ class OrderingTests(TestCase):
             attrgetter("headline")
         )
 
-        # Getting a single item should work too:
-        self.assertEqual(Article.objects.all()[0], a4)
-
-        # Use '?' to order randomly.
+    def test_random_ordering(self):
+        """
+        Use '?' to order randomly.
+        """
         self.assertEqual(
             len(list(Article.objects.order_by("?"))), 4
         )
 
-        # Ordering can be reversed using the reverse() method on a queryset.
-        # This allows you to extract things like "the last two items" (reverse
-        # and then take the first two).
+    def test_reversed_ordering(self):
+        """
+        Ordering can be reversed using the reverse() method on a queryset.
+        This allows you to extract things like "the last two items" (reverse
+        and then take the first two).
+        """
         self.assertQuerysetEqual(
             Article.objects.all().reverse()[:2], [
                 "Article 1",
@@ -115,7 +136,10 @@ class OrderingTests(TestCase):
             attrgetter("headline")
         )
 
-        # Ordering can be based on fields included from an 'extra' clause
+    def test_extra_ordering(self):
+        """
+        Ordering can be based on fields included from an 'extra' clause
+        """
         self.assertQuerysetEqual(
             Article.objects.extra(select={"foo": "pub_date"}, order_by=["foo", "headline"]), [
                 "Article 1",
@@ -126,8 +150,11 @@ class OrderingTests(TestCase):
             attrgetter("headline")
         )
 
-        # If the extra clause uses an SQL keyword for a name, it will be
-        # protected by quoting.
+    def test_extra_ordering_quoting(self):
+        """
+        If the extra clause uses an SQL keyword for a name, it will be
+        protected by quoting.
+        """
         self.assertQuerysetEqual(
             Article.objects.extra(select={"order": "pub_date"}, order_by=["order", "headline"]), [
                 "Article 1",
@@ -143,21 +170,32 @@ class OrderingTests(TestCase):
         Ensure that 'pk' works as an ordering option in Meta.
         Refs #8291.
         """
-        ArticlePKOrdering.objects.create(
-            pk=1, headline="Article 1", pub_date=datetime(2005, 7, 26)
-        )
-        ArticlePKOrdering.objects.create(
-            pk=2, headline="Article 2", pub_date=datetime(2005, 7, 27)
-        )
-        ArticlePKOrdering.objects.create(
-            pk=3, headline="Article 3", pub_date=datetime(2005, 7, 27)
-        )
-        ArticlePKOrdering.objects.create(
-            pk=4, headline="Article 4", pub_date=datetime(2005, 7, 28)
+        Author.objects.create(pk=1)
+        Author.objects.create(pk=2)
+        Author.objects.create(pk=3)
+        Author.objects.create(pk=4)
+
+        self.assertQuerysetEqual(
+            Author.objects.all(), [
+                4, 3, 2, 1
+            ],
+            attrgetter("pk")
         )
 
+    def test_order_by_fk_attname(self):
+        """
+        Ensure that ordering by a foreign key by its attribute name prevents
+        the query from inheriting it's related model ordering option.
+        Refs #19195.
+        """
+        for i in range(1, 5):
+            author = Author.objects.create(pk=i)
+            article = getattr(self, "a%d" % (5 - i))
+            article.author = author
+            article.save(update_fields={'author'})
+
         self.assertQuerysetEqual(
-            ArticlePKOrdering.objects.all(), [
+            Article.objects.order_by('author_id'), [
                 "Article 4",
                 "Article 3",
                 "Article 2",