Fixed #13312 -- Adds nulls argument on asc and desc

django/db/models/expressions.py

@@ -462,14 +462,11 @@ def __repr__(self):
     def resolve_expression(self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False):
         return query.resolve_ref(self.name, allow_joins, reuse, summarize)
 
-    def asc(self):
-        return OrderBy(self)
-
-    def desc(self):
-        return OrderBy(self, descending=True)
+    def asc(self, **kwargs):
+        return OrderBy(self, **kwargs)
 
-    def nullslast(self):
-        return OrderBy(self, nullslast=True)
+    def desc(self, **kwargs):
+        return OrderBy(self, descending=True, **kwargs)
 
 
 class Func(Expression):
@@ -865,8 +862,12 @@ def as_sql(self, compiler, connection, template=None, case_joiner=None, **extra_
 class OrderBy(BaseExpression):
     template = '%(expression)s %(ordering)s'
 
-    def __init__(self, expression, descending=False, nullslast=False):
-        self.nullslast = nullslast
+    def __init__(self, expression, descending=False, nulls_first=False,
+                 nulls_last=False):
+        if nulls_first and nulls_last:
+            raise ValueError("nulls_first and nulls_last are mutualy exclusive")
+        self.nulls_first = nulls_first
+        self.nulls_last = nulls_last
         self.descending = descending
         if not hasattr(expression, 'resolve_expression'):
             raise ValueError('expression must be an expression type')
@@ -883,6 +884,11 @@ def get_source_expressions(self):
         return [self.expression]
 
     def as_sql(self, compiler, connection, template=None, **extra_context):
+        if not template:
+            if self.nulls_last:
+                template = "%s NULLS LAST" % self.template
+            elif self.nulls_first:
+                template = "%s NULLS FIRST" % self.template
         connection.ops.check_expression_support(self)
         expression_sql, params = compiler.compile(self.expression)
         placeholders = {
@@ -895,27 +901,19 @@ def as_sql(self, compiler, connection, template=None, **extra_context):
 
     def as_sqlite(self, compiler, connection):
         template = None
-        if self.nullslast:
+        if self.nulls_last:
             template = '%(expression)s IS NULL, %(expression)s %(ordering)s'
-        sql, params = self.as_sql(compiler, connection, template=template)
-        return sql, params
-
-    def as_postgresql(self, compiler, connection):
-        template = None
-        if self.nullslast:
-            template = '%(expression)s %(ordering)s NULLS LAST'
-        sql, params = self.as_sql(compiler, connection, template=template)
-        return sql, params
-
-    def as_oracle(self, compiler, connection):
-        return self.as_postgresql(self, compiler, connection)
+        elif self.nulls_first:
+            template = '%(expression)s IS NOT NULL, %(expression)s %(ordering)s'
+        return self.as_sql(compiler, connection, template=template)
 
     def as_mysql(self, compiler, connection):
         template = None
-        if self.nullslast:
+        if self.nulls_last:
             template = 'IF(ISNULL(%(expression)s),1,0),%(expression)s %(ordering)s '
-        sql, params = self.as_sql(compiler, connection, template=template)
-        return sql, params
+        elif self.nulls_first:
+            template = 'IF(ISNULL(%(expression)s),0,1),%(expression)s %(ordering)s '
+        return self.as_sql(compiler, connection, template=template)
 
     def get_group_by_cols(self):
         cols = []
@@ -932,6 +930,3 @@ def asc(self):
 
     def desc(self):
         self.descending = True
-
-    def nullslast(self):
-        self.nullslast = True

tests/ordering/tests.py

@@ -98,9 +98,31 @@ def test_order_by_nullslast(self):
             pub_date=datetime(2005, 7, 28)
         )
 
+        author_2 = Author.objects.create()
+        Article.objects.create(
+            headline="Article 6",
+            author=author_2,
+            pub_date=datetime(2005, 7, 28)
+        )
+
+        # check that asc and desc are chainable with nulls_last
+
+        self.assertQuerysetEqual(
+            Article.objects.order_by(F("author").desc(nulls_last=True)), [
+                "Article 6",
+                "Article 5",
+                "Article 1",
+                "Article 2",
+                "Article 3",
+                "Article 4"
+            ],
+            attrgetter("headline")
+        )
+
         self.assertQuerysetEqual(
-            Article.objects.order_by(F("author").nullslast()), [
+            Article.objects.order_by(F("author").asc(nulls_last=True)), [
                 "Article 5",
+                "Article 6",
                 "Article 1",
                 "Article 2",
                 "Article 3",
@@ -109,6 +131,30 @@ def test_order_by_nullslast(self):
             attrgetter("headline")
         )
 
+        self.assertQuerysetEqual(
+            Article.objects.order_by(F("author").asc(nulls_first=True)), [
+                "Article 1",
+                "Article 2",
+                "Article 3",
+                "Article 4",
+                "Article 5",
+                "Article 6",
+            ],
+            attrgetter("headline")
+        )
+
+        self.assertQuerysetEqual(
+            Article.objects.order_by(F("author").desc(nulls_first=True)), [
+                "Article 1",
+                "Article 2",
+                "Article 3",
+                "Article 4",
+                "Article 6",
+                "Article 5",
+            ],
+            attrgetter("headline")
+        )
+
     def test_stop_slicing(self):
         """
         Use the 'stop' part of slicing notation to limit the results.