From ad4a8acdb55a80a6a6dec60724b22abed0025399 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Freitag?= Date: Wed, 21 Jun 2017 13:28:16 -0700 Subject: [PATCH] Fixed #11557 -- Added support for a list of fields in Meta.get_latest_by and QuerySet.earliest()/latest(). --- django/db/models/query.py | 39 ++++++++++---- docs/internals/deprecation.txt | 3 ++ docs/ref/models/options.txt | 14 +++-- docs/ref/models/querysets.txt | 27 +++++++--- docs/releases/2.0.txt | 9 ++++ tests/get_earliest_or_latest/tests.py | 75 ++++++++++++++++++++++----- tests/queries/tests.py | 2 +- 7 files changed, 137 insertions(+), 32 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index 7937d4ede3..acc8106fd1 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -25,6 +25,7 @@ from django.db.models.functions import Trunc from django.db.models.query_utils import InvalidQuery, Q from django.db.models.sql.constants import CURSOR, GET_ITERATOR_CHUNK_SIZE from django.utils import timezone +from django.utils.deprecation import RemovedInDjango30Warning from django.utils.functional import cached_property, partition from django.utils.version import get_version @@ -525,27 +526,47 @@ class QuerySet: )) return lookup, params - def _earliest_or_latest(self, field_name=None, direction="-"): + def _earliest_or_latest(self, *fields, field_name=None): """ Return the latest object, according to the model's 'get_latest_by' option or optional given field_name. """ - order_by = field_name or getattr(self.model._meta, 'get_latest_by') - assert bool(order_by), "earliest() and latest() require either a "\ - "field_name parameter or 'get_latest_by' in the model" + if fields and field_name is not None: + raise ValueError('Cannot use both positional arguments and the field_name keyword argument.') + + order_by = None + if field_name is not None: + warnings.warn( + 'The field_name keyword argument to earliest() and latest() ' + 'is deprecated in favor of passing positional arguments.', + RemovedInDjango30Warning, + ) + order_by = (field_name,) + elif fields: + order_by = fields + else: + order_by = getattr(self.model._meta, 'get_latest_by') + if order_by and not isinstance(order_by, (tuple, list)): + order_by = (order_by,) + if order_by is None: + raise ValueError( + "earliest() and latest() require either fields as positional " + "arguments or 'get_latest_by' in the model's Meta." + ) + assert self.query.can_filter(), \ "Cannot change a query once a slice has been taken." obj = self._chain() obj.query.set_limits(high=1) obj.query.clear_ordering(force_empty=True) - obj.query.add_ordering('%s%s' % (direction, order_by)) + obj.query.add_ordering(*order_by) return obj.get() - def earliest(self, field_name=None): - return self._earliest_or_latest(field_name=field_name, direction="") + def earliest(self, *fields, field_name=None): + return self._earliest_or_latest(*fields, field_name=field_name) - def latest(self, field_name=None): - return self._earliest_or_latest(field_name=field_name, direction="-") + def latest(self, *fields, field_name=None): + return self.reverse()._earliest_or_latest(*fields, field_name=field_name) def first(self): """Return the first object of a query or None if no match is found.""" diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index c41245ac65..c49b124f9c 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -26,6 +26,9 @@ details on these changes. * Support for the ``context`` argument of ``Field.from_db_value()`` and ``Expression.convert_value()`` will be removed. +* The ``field_name`` keyword argument of ``QuerySet.earliest()` and + ``latest()`` will be removed. + .. _deprecation-removed-in-2.1: 2.1 diff --git a/docs/ref/models/options.txt b/docs/ref/models/options.txt index 33ff581d39..188552dbb2 100644 --- a/docs/ref/models/options.txt +++ b/docs/ref/models/options.txt @@ -131,18 +131,26 @@ Django quotes column and table names behind the scenes. .. attribute:: Options.get_latest_by - The name of an orderable field in the model, typically a :class:`DateField`, - :class:`DateTimeField`, or :class:`IntegerField`. This specifies the default - field to use in your model :class:`Manager`’s + The name of a field or a list of field names in the model, typically + :class:`DateField`, :class:`DateTimeField`, or :class:`IntegerField`. This + specifies the default field(s) to use in your model :class:`Manager`’s :meth:`~django.db.models.query.QuerySet.latest` and :meth:`~django.db.models.query.QuerySet.earliest` methods. Example:: + # Latest by ascending order_date. get_latest_by = "order_date" + # Latest by priority descending, order_date ascending. + get_latest_by = ['-priority', 'order_date'] + See the :meth:`~django.db.models.query.QuerySet.latest` docs for more. + .. versionchanged:: 2.0 + + Support for a list of fields was added. + ``managed`` ----------- diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index 9c329d48ee..5973f2b5a3 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -2099,20 +2099,29 @@ psycopg mailing list ` specifies -:attr:`~django.db.models.Options.get_latest_by`, you can leave off the -``field_name`` argument to ``earliest()`` or ``latest()``. Django will use the -field specified in :attr:`~django.db.models.Options.get_latest_by` by default. +:attr:`~django.db.models.Options.get_latest_by`, you can omit any arguments to +``earliest()`` or ``latest()``. The fields specified in +:attr:`~django.db.models.Options.get_latest_by` will be used by default. Like :meth:`get()`, ``earliest()`` and ``latest()`` raise :exc:`~django.db.models.Model.DoesNotExist` if there is no object with the @@ -2121,6 +2130,10 @@ given parameters. Note that ``earliest()`` and ``latest()`` exist purely for convenience and readability. +.. versionchanged:: 2.0 + + Support for several arguments was added. + .. admonition:: ``earliest()`` and ``latest()`` may return instances with null dates. Since ordering is delegated to the database, results on fields that allow @@ -2135,7 +2148,7 @@ readability. ``earliest()`` ~~~~~~~~~~~~~~ -.. method:: earliest(field_name=None) +.. method:: earliest(*fields) Works otherwise like :meth:`~django.db.models.query.QuerySet.latest` except the direction is changed. diff --git a/docs/releases/2.0.txt b/docs/releases/2.0.txt index a8530d11c2..dd8183d798 100644 --- a/docs/releases/2.0.txt +++ b/docs/releases/2.0.txt @@ -250,6 +250,10 @@ Models from the database. For databases that don't support server-side cursors, it controls the number of results Django fetches from the database adapter. +* :meth:`.QuerySet.earliest`, :meth:`.QuerySet.latest`, and + :attr:`Meta.get_latest_by ` now + allow ordering by several fields. + * Added the :class:`~django.db.models.functions.datetime.ExtractQuarter` function to extract the quarter from :class:`~django.db.models.DateField` and :class:`~django.db.models.DateTimeField`, and exposed it through the @@ -642,6 +646,11 @@ Miscellaneous * ``HttpRequest.xreadlines()`` is deprecated in favor of iterating over the request. +* The ``field_name`` keyword argument to :meth:`.QuerySet.earliest` and + :meth:`.QuerySet.latest` is deprecated in favor of passing the field + names as arguments. Write ``.earliest('pub_date')`` instead of + ``.earliest(field_name='pub_date')``. + .. _removed-features-2.0: Features removed in 2.0 diff --git a/tests/get_earliest_or_latest/tests.py b/tests/get_earliest_or_latest/tests.py index 5b05dff487..62f6f85ce2 100644 --- a/tests/get_earliest_or_latest/tests.py +++ b/tests/get_earliest_or_latest/tests.py @@ -1,3 +1,4 @@ +import warnings from datetime import datetime from django.test import TestCase @@ -29,11 +30,11 @@ class EarliestOrLatestTests(TestCase): headline="Article 2", pub_date=datetime(2005, 7, 27), expire_date=datetime(2005, 7, 28) ) - Article.objects.create( + a3 = Article.objects.create( headline="Article 3", pub_date=datetime(2005, 7, 28), expire_date=datetime(2005, 8, 27) ) - Article.objects.create( + a4 = Article.objects.create( headline="Article 4", pub_date=datetime(2005, 7, 28), expire_date=datetime(2005, 7, 30) ) @@ -60,12 +61,32 @@ class EarliestOrLatestTests(TestCase): # in the Model.Meta Article.objects.model._meta.get_latest_by = None with self.assertRaisesMessage( - AssertionError, - "earliest() and latest() require either a field_name parameter or " - "'get_latest_by' in the model" + ValueError, + "earliest() and latest() require either fields as positional " + "arguments or 'get_latest_by' in the model's Meta." ): Article.objects.earliest() + # Earliest publication date, earliest expire date. + self.assertEqual( + Article.objects.filter(pub_date=datetime(2005, 7, 28)).earliest('pub_date', 'expire_date'), + a4, + ) + # Earliest publication date, latest expire date. + self.assertEqual( + Article.objects.filter(pub_date=datetime(2005, 7, 28)).earliest('pub_date', '-expire_date'), + a3, + ) + + # Meta.get_latest_by may be a tuple. + Article.objects.model._meta.get_latest_by = ('pub_date', 'expire_date') + self.assertEqual(Article.objects.filter(pub_date=datetime(2005, 7, 28)).earliest(), a4) + + def test_earliest_fields_and_field_name(self): + msg = 'Cannot use both positional arguments and the field_name keyword argument.' + with self.assertRaisesMessage(ValueError, msg): + Article.objects.earliest('pub_date', field_name='expire_date') + def test_latest(self): # Because no Articles exist yet, latest() raises ArticleDoesNotExist. with self.assertRaises(Article.DoesNotExist): @@ -75,7 +96,7 @@ class EarliestOrLatestTests(TestCase): headline="Article 1", pub_date=datetime(2005, 7, 26), expire_date=datetime(2005, 9, 1) ) - Article.objects.create( + a2 = Article.objects.create( headline="Article 2", pub_date=datetime(2005, 7, 27), expire_date=datetime(2005, 7, 28) ) @@ -110,25 +131,55 @@ class EarliestOrLatestTests(TestCase): # Error is raised if get_latest_by isn't in Model.Meta. Article.objects.model._meta.get_latest_by = None with self.assertRaisesMessage( - AssertionError, - "earliest() and latest() require either a field_name parameter or " - "'get_latest_by' in the model" + ValueError, + "earliest() and latest() require either fields as positional " + "arguments or 'get_latest_by' in the model's Meta." ): Article.objects.latest() + # Latest publication date, latest expire date. + self.assertEqual(Article.objects.filter(pub_date=datetime(2005, 7, 27)).latest('pub_date', 'expire_date'), a3) + # Latest publication date, earliest expire date. + self.assertEqual( + Article.objects.filter(pub_date=datetime(2005, 7, 27)).latest('pub_date', '-expire_date'), + a2, + ) + + # Meta.get_latest_by may be a tuple. + Article.objects.model._meta.get_latest_by = ('pub_date', 'expire_date') + self.assertEqual(Article.objects.filter(pub_date=datetime(2005, 7, 27)).latest(), a3) + + def test_latest_fields_and_field_name(self): + msg = 'Cannot use both positional arguments and the field_name keyword argument.' + with self.assertRaisesMessage(ValueError, msg): + Article.objects.latest('pub_date', field_name='expire_date') + def test_latest_manual(self): # You can still use latest() with a model that doesn't have # "get_latest_by" set -- just pass in the field name manually. Person.objects.create(name="Ralph", birthday=datetime(1950, 1, 1)) p2 = Person.objects.create(name="Stephanie", birthday=datetime(1960, 2, 3)) msg = ( - "earliest() and latest() require either a field_name parameter or " - "'get_latest_by' in the model" + "earliest() and latest() require either fields as positional arguments " + "or 'get_latest_by' in the model's Meta." ) - with self.assertRaisesMessage(AssertionError, msg): + with self.assertRaisesMessage(ValueError, msg): Person.objects.latest() self.assertEqual(Person.objects.latest("birthday"), p2) + def test_field_name_kwarg_deprecation(self): + Person.objects.create(name='Deprecator', birthday=datetime(1950, 1, 1)) + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always') + Person.objects.latest(field_name='birthday') + + self.assertEqual(len(warns), 1) + self.assertEqual( + str(warns[0].message), + 'The field_name keyword argument to earliest() and latest() ' + 'is deprecated in favor of passing positional arguments.', + ) + class TestFirstLast(TestCase): diff --git a/tests/queries/tests.py b/tests/queries/tests.py index 47f2ece35b..1c6dd8bcc5 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -2365,7 +2365,7 @@ class WeirdQuerysetSlicingTests(TestCase): self.assertQuerysetEqual(Article.objects.all()[0:0], []) self.assertQuerysetEqual(Article.objects.all()[0:0][:10], []) self.assertEqual(Article.objects.all()[:0].count(), 0) - with self.assertRaisesMessage(AssertionError, 'Cannot change a query once a slice has been taken.'): + with self.assertRaisesMessage(TypeError, 'Cannot reverse a query once a slice has been taken.'): Article.objects.all()[:0].latest('created') def test_empty_resultset_sql(self):