diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py index 7baaa38a29f0..b4aa6aaccd53 100644 --- a/django/contrib/admin/checks.py +++ b/django/contrib/admin/checks.py @@ -1082,6 +1082,9 @@ def _check_list_select_related(self, obj): if not isinstance(obj.list_select_related, (bool, list, tuple)): return must_be( + # RemovedInDjango70Warning: when the deprecation ends, replace: + # "a tuple, list, or False", + # and also update docs/ref/checks.txt. "a boolean, tuple or list", option="list_select_related", obj=obj, diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index e05881b16a29..40ae27c22cc6 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -2,6 +2,7 @@ import enum import json import re +import warnings from functools import partial, update_wrapper from urllib.parse import parse_qsl from urllib.parse import quote as urlquote @@ -58,6 +59,7 @@ from django.template.response import SimpleTemplateResponse, TemplateResponse from django.urls import reverse from django.utils.decorators import method_decorator +from django.utils.deprecation import RemovedInDjango70Warning, django_file_prefixes from django.utils.html import format_html from django.utils.http import urlencode from django.utils.safestring import mark_safe @@ -676,6 +678,18 @@ class ModelAdmin(BaseModelAdmin): actions_selection_counter = True checks_class = ModelAdminChecks + def __init_subclass__(cls, **kwargs) -> None: + super().__init_subclass__(**kwargs) + if cls.__dict__.get("list_select_related") is True: + # RemovedInDjango70Warning: when the deprecation ends, raise a + # ValueError. + warnings.warn( + "Setting ModelAdmin.list_select_related to True is deprecated. " + "Use False or a list or tuple of fields to fetch instead.", + RemovedInDjango70Warning, + skip_file_prefixes=django_file_prefixes(), + ) + def __init__(self, model, admin_site): self.model = model self.opts = model._meta @@ -860,6 +874,17 @@ def get_changelist_instance(self, request): list_display = ["action_checkbox", *list_display] sortable_by = self.get_sortable_by(request) ChangeList = self.get_changelist(request) + list_select_related = self.get_list_select_related(request) + if list_select_related is True: + # RemovedInDjango70Warning: when the deprecation ends, remove the + # below 'if' clause and raise a ValueError here. + if self.list_select_related is not True: + warnings.warn( + "Returning True from ModelAdmin.get_list_select_related() is " + "deprecated. Return False or a list or tuple of fields to " + "fetch instead.", + RemovedInDjango70Warning, + ) return ChangeList( request, self.model, @@ -868,7 +893,7 @@ def get_changelist_instance(self, request): self.get_list_filter(request), self.date_hierarchy, self.get_search_fields(request), - self.get_list_select_related(request), + list_select_related, self.list_per_page, self.list_max_show_all, self.list_editable, @@ -2332,7 +2357,7 @@ def history_view(self, request, object_id, extra_context=None): object_id=unquote(object_id), content_type=get_content_type_for_model(model), ) - .select_related() + .select_related("user", "content_type") .order_by("action_time") ) diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index c510045db376..0ceaf5f7a5c4 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -529,25 +529,28 @@ def apply_select_related(self, qs): return qs.select_related() if self.list_select_related is False: - if self.has_related_field_in_list_display(): - return qs.select_related() + if fields := self.get_select_related_fields(): + return qs.select_related(*fields) if self.list_select_related: return qs.select_related(*self.list_select_related) return qs - def has_related_field_in_list_display(self): + def get_select_related_fields(self): + fields = [] for field_name in self.list_display: try: field = self.lookup_opts.get_field(field_name) except FieldDoesNotExist: pass else: - if isinstance(field.remote_field, ManyToOneRel): + if ( + isinstance(field.remote_field, ManyToOneRel) # _id field names don't require a join. - if field_name != field.attname: - return True - return False + and field_name != field.attname + ): + fields.append(field_name) + return fields def url_for_result(self, result): pk = getattr(result, self.pk_attname) diff --git a/django/db/models/query.py b/django/db/models/query.py index 8eed5bf2d411..6cdd7681b261 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -37,7 +37,7 @@ resolve_callables, ) from django.utils import timezone -from django.utils.deprecation import RemovedInDjango70Warning +from django.utils.deprecation import RemovedInDjango70Warning, django_file_prefixes from django.utils.functional import cached_property # The maximum number of results to fetch in a get() query. @@ -1774,6 +1774,14 @@ def select_related(self, *fields): elif fields: obj.query.add_select_related(fields) else: + # RemovedInDjango70Warning: when the deprecation ends, raise a + # TypeError instead. + warnings.warn( + "Calling select_related() with no arguments is deprecated. " + "Specify the fields to fetch instead.", + category=RemovedInDjango70Warning, + skip_file_prefixes=django_file_prefixes(), + ) obj.query.select_related = True return obj diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index a45b5cdad49b..d570972338c2 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -85,6 +85,15 @@ details on these changes. ``django.core.signing.base64_hmac()`` will change from ``"sha1"`` to ``"sha256"``. +* :meth:`.QuerySet.select_related` will raise :exc:`TypeError` if passed + ``True`` rather than :exc:`AttributeError`. + +* :class:`.ModelAdmin` will raise :exc:`ValueError` if subclassed with its + :attr:`.list_select_related` attribute set to ``True``. + +* :class:`.ModelAdmin` will raise :exc:`ValueError` if its + :meth:`.get_list_select_related` method returns ``True``. + .. _deprecation-removed-in-6.1: 6.1 diff --git a/docs/misc/design-philosophies.txt b/docs/misc/design-philosophies.txt index 0b989d760e06..f99990133707 100644 --- a/docs/misc/design-philosophies.txt +++ b/docs/misc/design-philosophies.txt @@ -126,9 +126,9 @@ optimize statements internally. This is why developers need to call ``save()`` explicitly, rather than the framework saving things behind the scenes silently. -This is also why the ``select_related()`` ``QuerySet`` method exists. It's an -optional performance booster for the common case of selecting "every related -object." +This is also why the ``FETCH_PEERS`` :doc:`fetch mode ` +exists. It's an optional performance booster for the common case of selecting +related objects for every peer in a ``QuerySet``. Terse, powerful syntax ---------------------- diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index 4b0e4a985de4..8ef697cfdeb6 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -961,6 +961,11 @@ subclass:: If you need to specify a dynamic value based on the request, you can implement a :meth:`~ModelAdmin.get_list_select_related` method. + .. deprecated:: 6.1 + + Using ``True`` for ``list_select_related`` is deprecated. Use a list or + tuple of field names instead. + .. note:: ``ModelAdmin`` ignores this attribute when @@ -1630,6 +1635,11 @@ default templates used by the :class:`ModelAdmin` views: should return a boolean or list as :attr:`ModelAdmin.list_select_related` does. + .. deprecated:: 6.1 + + Returning ``True`` is deprecated. Return a list or tuple of field names + instead. + .. method:: ModelAdmin.get_search_fields(request) The ``get_search_fields`` method is given the ``HttpRequest`` and is diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index 30eb99a7b2b9..081a0d6a99c8 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -1136,13 +1136,6 @@ is defined. Instead of specifying the field name, use the :attr:`related_name ` for the field on the related object. -There may be some situations where you wish to call ``select_related()`` with a -lot of related objects, or where you don't know all of the relations. In these -cases it is possible to call ``select_related()`` with no arguments. This will -follow all non-null foreign keys it can find - nullable foreign keys must be -specified. This is not recommended in most cases as it is likely to make the -underlying query more complex, and return more data, than is actually needed. - If you need to clear the list of related fields added by past calls of ``select_related`` on a ``QuerySet``, you can pass ``None`` as a parameter: @@ -1154,6 +1147,18 @@ Chaining ``select_related`` calls works in a similar way to other methods - that is that ``select_related('foo', 'bar')`` is equivalent to ``select_related('foo').select_related('bar')``. +There may be some situations where you wish to call ``select_related()`` with a +lot of related objects, or where you don't know all of the relations. In these +cases it is possible to call ``select_related()`` with no arguments. This will +follow all non-null foreign keys it can find - nullable foreign keys must be +specified. This is not recommended in most cases as it is likely to make the +underlying query more complex, and return more data, than is actually needed. + +.. deprecated:: 6.1 + + Calling ``select_related()`` with no arguments is deprecated, and support + for it will be removed in Django 7.0. Specify the fields to fetch instead. + ``prefetch_related()`` ~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/releases/6.1.txt b/docs/releases/6.1.txt index 412a99701985..146635841fb2 100644 --- a/docs/releases/6.1.txt +++ b/docs/releases/6.1.txt @@ -99,6 +99,11 @@ Minor features preserve :ref:`named groups ` (e.g. ``choices=[("Group", [("1", "Item")]), ...]``). +* When :attr:`.ModelAdmin.list_select_related` is ``False`` (the default), + the change list now selects only the foreign key fields specified in + :attr:`.ModelAdmin.list_display`, rather than all foreign key fields. This + should improve performance for models with many foreign key fields. + * The :attr:`~django.contrib.admin.ModelAdmin.delete_confirmation_max_display` option allows customizing how many objects are displayed on admin delete confirmation pages before the remainder is truncated. The default is @@ -480,6 +485,9 @@ backends. * The undocumented ``InclusionAdminNode.__init__()`` now takes the template tag ``name`` as the first positional argument. +* The undocumented ``ChangeList.has_related_field_in_list_display()`` method + has been replaced with ``ChangeList.get_select_related_fields()``. + :mod:`django.contrib.auth` -------------------------- @@ -592,6 +600,14 @@ Features deprecated in 6.1 Miscellaneous ------------- +* Calling :meth:`~django.db.models.query.QuerySet.select_related` with no + arguments to select all related fields, is deprecated. Specify the related + fields to fetch instead. + +* Setting :attr:`.ModelAdmin.list_select_related` to ``True`` and returning + ``True`` from :attr:`.ModelAdmin.get_list_select_related()` are deprecated. + Specify the related fields to fetch instead. + * Calling :meth:`.QuerySet.values_list` with ``flat=True`` and no field name is deprecated. Pass an explicit field name, like ``values_list("pk", flat=True)``. diff --git a/docs/topics/db/search.txt b/docs/topics/db/search.txt index 65d3be2c298e..7ec385472171 100644 --- a/docs/topics/db/search.txt +++ b/docs/topics/db/search.txt @@ -17,7 +17,7 @@ Standard textual queries ------------------------ Text-based fields have a selection of matching operations. For example, you may -wish to allow lookup up an author like so: +wish to allow the lookup of an author like so: .. code-block:: pycon diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index d0448a1b640b..aec0dd6af12a 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -292,13 +292,17 @@ def has_module_permission(self, request): class RowLevelChangePermissionModelAdmin(admin.ModelAdmin): + # These fields aren't intended to be modified by the change form. By + # making them read-only, they don't need to be included in post data. + readonly_fields = ("can_change", "can_view") + def has_change_permission(self, request, obj=None): - """Only allow changing objects with even id number""" - return request.user.is_staff and (obj is not None) and (obj.id % 2 == 0) + """Only allow changing objects with can_change=True.""" + return request.user.is_staff and obj is not None and obj.can_change def has_view_permission(self, request, obj=None): - """Only allow viewing objects if id is a multiple of 3.""" - return request.user.is_staff and obj is not None and obj.id % 3 == 0 + """Only allow viewing objects with can_view=True.""" + return request.user.is_staff and obj is not None and obj.can_view class CustomArticleAdmin(admin.ModelAdmin): diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py index 38e26cb95a52..50abbfdfe2e1 100644 --- a/tests/admin_views/models.py +++ b/tests/admin_views/models.py @@ -128,6 +128,8 @@ def __str__(self): class RowLevelChangePermissionModel(models.Model): name = models.CharField(max_length=100, blank=True) + can_change = models.BooleanField(default=False) + can_view = models.BooleanField(default=False) class CustomArticle(models.Model): diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 3dba13d1851c..a1251103aad8 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -995,29 +995,29 @@ def test_sort_indicators_admin_order(self): self.assertContentBefore(response, "The First Item", "The Middle Item") self.assertContentBefore(response, "The Middle Item", "The Last Item") - def test_has_related_field_in_list_display_fk(self): + def test_get_select_related_fields_in_list_display_fk(self): """Joins shouldn't be performed for _id fields in list display.""" state = State.objects.create(name="Karnataka") City.objects.create(state=state, name="Bangalore") response = self.client.get(reverse("admin:admin_views_city_changelist"), {}) response.context["cl"].list_display = ["id", "name", "state"] - self.assertIs(response.context["cl"].has_related_field_in_list_display(), True) + self.assertEqual(response.context["cl"].get_select_related_fields(), ["state"]) response.context["cl"].list_display = ["id", "name", "state_id"] - self.assertIs(response.context["cl"].has_related_field_in_list_display(), False) + self.assertEqual(response.context["cl"].get_select_related_fields(), []) - def test_has_related_field_in_list_display_o2o(self): + def test_get_select_related_fields_in_list_display_o2o(self): """Joins shouldn't be performed for _id fields in list display.""" media = Media.objects.create(name="Foo") Vodcast.objects.create(media=media) response = self.client.get(reverse("admin:admin_views_vodcast_changelist"), {}) response.context["cl"].list_display = ["media"] - self.assertIs(response.context["cl"].has_related_field_in_list_display(), True) + self.assertEqual(response.context["cl"].get_select_related_fields(), ["media"]) response.context["cl"].list_display = ["media_id"] - self.assertIs(response.context["cl"].has_related_field_in_list_display(), False) + self.assertEqual(response.context["cl"].get_select_related_fields(), []) def test_limited_filter(self): """ @@ -2930,10 +2930,18 @@ def test_change_view(self): # Test redirection when using row-level change permissions. Refs # #11513. - r1 = RowLevelChangePermissionModel.objects.create(id=1, name="odd id") - r2 = RowLevelChangePermissionModel.objects.create(id=2, name="even id") - r3 = RowLevelChangePermissionModel.objects.create(id=3, name="odd id mult 3") - r6 = RowLevelChangePermissionModel.objects.create(id=6, name="even id mult 3") + r1 = RowLevelChangePermissionModel.objects.create( + name="A", can_change=False, can_view=False + ) + r2 = RowLevelChangePermissionModel.objects.create( + name="B", can_change=True, can_view=False + ) + r3 = RowLevelChangePermissionModel.objects.create( + name="C", can_change=False, can_view=True + ) + r4 = RowLevelChangePermissionModel.objects.create( + name="D", can_change=True, can_view=True + ) change_url_1 = reverse( "admin:admin_views_rowlevelchangepermissionmodel_change", args=(r1.pk,) ) @@ -2943,8 +2951,8 @@ def test_change_view(self): change_url_3 = reverse( "admin:admin_views_rowlevelchangepermissionmodel_change", args=(r3.pk,) ) - change_url_6 = reverse( - "admin:admin_views_rowlevelchangepermissionmodel_change", args=(r6.pk,) + change_url_4 = reverse( + "admin:admin_views_rowlevelchangepermissionmodel_change", args=(r4.pk,) ) logins = [ self.superuser, @@ -2960,14 +2968,16 @@ def test_change_view(self): self.assertEqual(response.status_code, 403) response = self.client.post(change_url_1, {"name": "changed"}) self.assertEqual( - RowLevelChangePermissionModel.objects.get(id=1).name, "odd id" + RowLevelChangePermissionModel.objects.get(pk=r1.pk).name, + r1.name, ) self.assertEqual(response.status_code, 403) response = self.client.get(change_url_2) self.assertEqual(response.status_code, 200) response = self.client.post(change_url_2, {"name": "changed"}) self.assertEqual( - RowLevelChangePermissionModel.objects.get(id=2).name, "changed" + RowLevelChangePermissionModel.objects.get(pk=r2.pk).name, + "changed", ) self.assertRedirects(response, self.index_url) response = self.client.get(change_url_3) @@ -2975,14 +2985,15 @@ def test_change_view(self): response = self.client.post(change_url_3, {"name": "changed"}) self.assertEqual(response.status_code, 403) self.assertEqual( - RowLevelChangePermissionModel.objects.get(id=3).name, - "odd id mult 3", + RowLevelChangePermissionModel.objects.get(pk=r3.pk).name, + r3.name, ) - response = self.client.get(change_url_6) + response = self.client.get(change_url_4) self.assertEqual(response.status_code, 200) - response = self.client.post(change_url_6, {"name": "changed"}) + response = self.client.post(change_url_4, {"name": "changed"}) self.assertEqual( - RowLevelChangePermissionModel.objects.get(id=6).name, "changed" + RowLevelChangePermissionModel.objects.get(pk=r4.pk).name, + "changed", ) self.assertRedirects(response, self.index_url) @@ -2997,7 +3008,8 @@ def test_change_view(self): change_url_1, {"name": "changed"}, follow=True ) self.assertEqual( - RowLevelChangePermissionModel.objects.get(id=1).name, "odd id" + RowLevelChangePermissionModel.objects.get(pk=r1.pk).name, + r1.name, ) self.assertContains(response, "login-form") response = self.client.get(change_url_2, follow=True) @@ -3006,7 +3018,8 @@ def test_change_view(self): change_url_2, {"name": "changed again"}, follow=True ) self.assertEqual( - RowLevelChangePermissionModel.objects.get(id=2).name, "changed" + RowLevelChangePermissionModel.objects.get(pk=r2.pk).name, + "changed", ) self.assertContains(response, "login-form") self.client.post(reverse("admin:logout")) @@ -3306,8 +3319,8 @@ def test_history_view(self): # Test redirection when using row-level change permissions. Refs # #11513. - rl1 = RowLevelChangePermissionModel.objects.create(id=1, name="odd id") - rl2 = RowLevelChangePermissionModel.objects.create(id=2, name="even id") + rl1 = RowLevelChangePermissionModel.objects.create(name="A", can_change=False) + rl2 = RowLevelChangePermissionModel.objects.create(name="B", can_change=True) logins = [ self.superuser, self.viewuser, diff --git a/tests/aggregation_regress/tests.py b/tests/aggregation_regress/tests.py index f2ed13420f64..ba1d8e77b231 100644 --- a/tests/aggregation_regress/tests.py +++ b/tests/aggregation_regress/tests.py @@ -26,7 +26,8 @@ ) from django.db.models.functions import Cast, Concat from django.test import TestCase, skipUnlessDBFeature -from django.test.utils import Approximate +from django.test.utils import Approximate, ignore_warnings +from django.utils.deprecation import RemovedInDjango70Warning from .models import ( Alfa, @@ -883,12 +884,18 @@ def test_order_by_group_by_join(self): def test_annotate_select_related(self): # Regression for #10127 - Empty select_related() works with annotate - qs = ( - Book.objects.filter(rating__lt=4.5) - .select_related() - .annotate(Avg("authors__age")) - .order_by("name") - ) + # RemovedInDjango70Warning: when the deprecation ends, the below + # queryset and assertion can be removed. + with ignore_warnings( + category=RemovedInDjango70Warning, + message=r"Calling select_related\(\) with no arguments is deprecated\.", + ): + qs = ( + Book.objects.filter(rating__lt=4.5) + .select_related() + .annotate(Avg("authors__age")) + .order_by("name") + ) self.assertQuerySetEqual( qs, [ diff --git a/tests/defer/tests.py b/tests/defer/tests.py index ea7703c1e369..c1fdfa772896 100644 --- a/tests/defer/tests.py +++ b/tests/defer/tests.py @@ -1,6 +1,8 @@ from django.core.exceptions import FieldDoesNotExist, FieldError, FieldFetchBlocked from django.db.models import FETCH_PEERS, RAISE from django.test import SimpleTestCase, TestCase +from django.test.utils import ignore_warnings +from django.utils.deprecation import RemovedInDjango70Warning from .models import ( BigChild, @@ -129,23 +131,28 @@ def test_get(self): self.assert_delayed(qs.only("name").get(pk=self.p1.pk), 2) def test_defer_with_select_related(self): - obj = Primary.objects.select_related().defer( + obj = Primary.objects.select_related("related").defer( "related__first", "related__second" )[0] self.assert_delayed(obj.related, 2) self.assert_delayed(obj, 0) def test_only_with_select_related(self): - obj = Primary.objects.select_related().only("related__first")[0] + obj = Primary.objects.select_related("related").only("related__first")[0] self.assert_delayed(obj, 2) self.assert_delayed(obj.related, 1) self.assertEqual(obj.related_id, self.s1.pk) self.assertEqual(obj.name, "p1") + # RemovedInDjango70Warning: when the deprecation ends, remove this test. def test_defer_foreign_keys_are_deferred_and_not_traversed(self): # select_related() overrides defer(). with self.assertNumQueries(1): - obj = Primary.objects.defer("related").select_related()[0] + with ignore_warnings( + category=RemovedInDjango70Warning, + message=r"Calling select_related\(\) with no arguments is deprecated\.", + ): + obj = Primary.objects.defer("related").select_related()[0] self.assert_delayed(obj, 1) self.assertEqual(obj.related.id, self.s1.pk) @@ -325,7 +332,11 @@ def test_defer_proxy(self): """ related = Secondary.objects.create(first="x1", second="x2") ChildProxy.objects.create(name="p1", value="xx", related=related) - children = ChildProxy.objects.select_related().only("id", "name") + with ignore_warnings( + category=RemovedInDjango70Warning, + message=r"Calling select_related\(\) with no arguments is deprecated\.", + ): + children = ChildProxy.objects.select_related().only("id", "name") self.assertEqual(len(children), 1) child = children[0] self.assert_delayed(child, 2) diff --git a/tests/defer_regress/tests.py b/tests/defer_regress/tests.py index 2089c8603f46..a54e9a8ea73f 100644 --- a/tests/defer_regress/tests.py +++ b/tests/defer_regress/tests.py @@ -65,11 +65,13 @@ def test_basic(self): c2 = Child.objects.create(name="c2", value=37) Leaf.objects.create(name="l1", child=c1, second_child=c2) - obj = Leaf.objects.only("name", "child").select_related()[0] + obj = Leaf.objects.only("name", "child").select_related("child")[0] self.assertEqual(obj.child.name, "c1") self.assertQuerySetEqual( - Leaf.objects.select_related().only("child__name", "second_child__name"), + Leaf.objects.select_related("child", "second_child").only( + "child__name", "second_child__name" + ), [ "l1", ], @@ -86,13 +88,15 @@ def test_basic(self): # Regression for #10733 - only() can be used on a model with two # foreign keys. - results = Leaf.objects.only("name", "child", "second_child").select_related() + results = Leaf.objects.only("name", "child", "second_child").select_related( + "child", "second_child" + ) self.assertEqual(results[0].child.name, "c1") self.assertEqual(results[0].second_child.name, "c2") results = Leaf.objects.only( "name", "child", "second_child", "child__name", "second_child__name" - ).select_related() + ).select_related("child", "second_child") self.assertEqual(results[0].child.name, "c1") self.assertEqual(results[0].second_child.name, "c2") @@ -213,7 +217,7 @@ def test_proxy_model_defer_with_select_related(self): item = Item.objects.create(name="first", value=47) RelatedItem.objects.create(item=item) # Defer fields with only() - obj = ProxyRelated.objects.select_related().only("item__name")[0] + obj = ProxyRelated.objects.select_related("item").only("item__name")[0] with self.assertNumQueries(0): self.assertEqual(obj.item.name, "first") with self.assertNumQueries(1): diff --git a/tests/gis_tests/relatedapp/tests.py b/tests/gis_tests/relatedapp/tests.py index 99294f3c50f6..7ff7654b7549 100644 --- a/tests/gis_tests/relatedapp/tests.py +++ b/tests/gis_tests/relatedapp/tests.py @@ -3,8 +3,9 @@ from django.contrib.gis.geos import GEOSGeometry, MultiPoint, Point from django.db import NotSupportedError, connection from django.test import TestCase, skipUnlessDBFeature -from django.test.utils import override_settings +from django.test.utils import ignore_warnings, override_settings from django.utils import timezone +from django.utils.deprecation import RemovedInDjango70Warning from ..utils import skipUnlessGISLookup from .models import Article, Author, Book, City, DirectoryEntry, Event, Location, Parcel @@ -16,7 +17,13 @@ class RelatedGeoModelTest(TestCase): def test02_select_related(self): "Testing `select_related` on geographic models (see #7126)." qs1 = City.objects.order_by("id") - qs2 = City.objects.order_by("id").select_related() + # RemovedInDjango70Warning: when the deprecation ends, the below + # queryset can be removed. + with ignore_warnings( + category=RemovedInDjango70Warning, + message=r"Calling select_related\(\) with no arguments is deprecated\.", + ): + qs2 = City.objects.order_by("id").select_related() qs3 = City.objects.order_by("id").select_related("location") # Reference data for what's in the fixtures. @@ -116,7 +123,7 @@ def test05_select_related_fk_to_subclass(self): select_related on a query over a model with an FK to a model subclass. """ # Regression test for #9752. - list(DirectoryEntry.objects.select_related()) + list(DirectoryEntry.objects.select_related("location")) @skipUnlessGISLookup("within") def test06_f_expressions(self): diff --git a/tests/m2m_intermediary/tests.py b/tests/m2m_intermediary/tests.py index 93abbde65ecf..0dcb00fb3a49 100644 --- a/tests/m2m_intermediary/tests.py +++ b/tests/m2m_intermediary/tests.py @@ -18,7 +18,7 @@ def test_intermediary(self): w2 = Writer.objects.create(reporter=r2, article=a, position="Contributor") self.assertQuerySetEqual( - a.writer_set.select_related().order_by("-position"), + a.writer_set.select_related("reporter", "article").order_by("-position"), [ ("John Smith", "Main writer"), ("Jane Doe", "Contributor"), diff --git a/tests/many_to_one/tests.py b/tests/many_to_one/tests.py index d6149d521f2a..3e665979ee33 100644 --- a/tests/many_to_one/tests.py +++ b/tests/many_to_one/tests.py @@ -467,15 +467,15 @@ def test_select_related(self): headline="Second", pub_date=datetime.date(1980, 4, 23), reporter=r2 ) self.assertEqual( - list(Article.objects.select_related().dates("pub_date", "day")), + list(Article.objects.select_related("reporter").dates("pub_date", "day")), [datetime.date(1980, 4, 23), datetime.date(2005, 7, 27)], ) self.assertEqual( - list(Article.objects.select_related().dates("pub_date", "month")), + list(Article.objects.select_related("reporter").dates("pub_date", "month")), [datetime.date(1980, 4, 1), datetime.date(2005, 7, 1)], ) self.assertEqual( - list(Article.objects.select_related().dates("pub_date", "year")), + list(Article.objects.select_related("reporter").dates("pub_date", "year")), [datetime.date(1980, 1, 1), datetime.date(2005, 1, 1)], ) diff --git a/tests/model_fields/test_booleanfield.py b/tests/model_fields/test_booleanfield.py index d0ed6e86ccc0..7b656b417687 100644 --- a/tests/model_fields/test_booleanfield.py +++ b/tests/model_fields/test_booleanfield.py @@ -3,6 +3,8 @@ from django.db import IntegrityError, models, transaction from django.db.models.utils import get_blank_choice_label from django.test import SimpleTestCase, TestCase +from django.test.utils import ignore_warnings +from django.utils.deprecation import RemovedInDjango70Warning from .models import BooleanModel, FksToBooleans, NullBooleanModel @@ -91,8 +93,14 @@ def test_select_related(self): self.assertIs(ma.nbf.nbfield, True) # select_related() - mb = FksToBooleans.objects.select_related().get(pk=m1.id) - mc = FksToBooleans.objects.select_related().get(pk=m2.id) + # RemovedInDjango70Warning: when the deprecation ends, remove this part + # of the test. + with ignore_warnings( + category=RemovedInDjango70Warning, + message=r"Calling select_related\(\) with no arguments is deprecated\.", + ): + mb = FksToBooleans.objects.select_related().get(pk=m1.id) + mc = FksToBooleans.objects.select_related().get(pk=m2.id) self.assertIs(mb.bf.bfield, True) self.assertIs(mb.nbf.nbfield, True) self.assertIs(mc.bf.bfield, False) diff --git a/tests/model_inheritance_regress/tests.py b/tests/model_inheritance_regress/tests.py index adc2a22fc403..ab4edea0acc7 100644 --- a/tests/model_inheritance_regress/tests.py +++ b/tests/model_inheritance_regress/tests.py @@ -9,6 +9,8 @@ from django import forms from django.db.models import FETCH_PEERS from django.test import TestCase +from django.test.utils import ignore_warnings +from django.utils.deprecation import RemovedInDjango70Warning from .models import ( ArticleWithAuthor, @@ -252,7 +254,13 @@ def test_issue_11764(self): """ Regression test for #11764 """ - wholesalers = list(Wholesaler.objects.select_related()) + # RemovedInDjango70Warning: when the deprecation ends, this test can + # be removed. + with ignore_warnings( + category=RemovedInDjango70Warning, + message=r"Calling select_related\(\) with no arguments is deprecated\.", + ): + wholesalers = list(Wholesaler.objects.select_related()) self.assertEqual(wholesalers, []) def test_issue_7853(self): @@ -528,7 +536,7 @@ def test_inheritance_select_related(self): Supplier.objects.create(name="Jane", restaurant=r2) self.assertQuerySetEqual( - Supplier.objects.order_by("name").select_related(), + Supplier.objects.order_by("name").select_related("restaurant"), [ "Jane", "John", diff --git a/tests/modeladmin/test_checks.py b/tests/modeladmin/test_checks.py index 496e46e8b01a..bcac262d695e 100644 --- a/tests/modeladmin/test_checks.py +++ b/tests/modeladmin/test_checks.py @@ -1233,6 +1233,8 @@ class TestModelAdmin(ModelAdmin): self.assertIsInvalid( TestModelAdmin, ValidationTestModel, + # RemovedInDjango70Warning: when the deprecation ends, replace: + # ... must be a tuple, list, or False.", "The value of 'list_select_related' must be a boolean, tuple or list.", "admin.E117", ) diff --git a/tests/modeladmin/tests.py b/tests/modeladmin/tests.py index 81119a27bf81..6a3574653344 100644 --- a/tests/modeladmin/tests.py +++ b/tests/modeladmin/tests.py @@ -21,7 +21,8 @@ from django.db.models.utils import get_blank_choice_label from django.forms.widgets import Select from django.test import RequestFactory, SimpleTestCase, TestCase -from django.test.utils import isolate_apps +from django.test.utils import ignore_warnings, isolate_apps +from django.utils.deprecation import RemovedInDjango70Warning from .models import Band, Concert, Song @@ -1027,6 +1028,54 @@ def test_modeladmin_repr(self): "", ) + def test_list_select_related_true_deprecated(self): + msg = ( + r"Setting ModelAdmin.list_select_related to True is deprecated. " + r"Use False or a list or tuple of fields to fetch instead." + ) + # RemovedInDjango70Warning: + # with self.assertRaisesMessage(ValueError, msg) + with self.assertWarnsMessage(RemovedInDjango70Warning, msg): + + class TestModelAdmin(ModelAdmin): + list_select_related = True + + # RemovedInDjango70Warning: when the deprecation ends, remove. + def test_list_select_related_true_deprecated_subclass(self): + with ignore_warnings( + category=RemovedInDjango70Warning, + message=r"Setting ModelAdmin.list_select_related to True is deprecated\.", + ): + + class BaseTestModelAdmin(ModelAdmin): + list_select_related = True + + # Does not warn + class TestModelAdmin(BaseTestModelAdmin): + pass + + # RemovedInDjango70Warning: when the deprecation ends, rename. + def test_get_list_select_related_returns_true_deprecated(self): + msg = ( + "Returning True from ModelAdmin.get_list_select_related() is " + "deprecated. Return False or a list or tuple of fields to fetch " + "instead." + ) + + class TestModelAdmin(ModelAdmin): + def get_list_select_related(self, request): + return True + + request = RequestFactory().get("/") + request.user = User.objects.create_superuser( + username="bob", email="bob@test.com", password="test" + ) + + # RemovedInDjango70Warning: + # with self.assertRaisesMessage(ValueError, msg) + with self.assertWarnsMessage(RemovedInDjango70Warning, msg): + TestModelAdmin(Band, self.site).get_changelist_instance(request) + class ModelAdminPermissionTests(SimpleTestCase): class MockUser: diff --git a/tests/null_fk/tests.py b/tests/null_fk/tests.py index ac7f7a82895a..7abcc5addbe6 100644 --- a/tests/null_fk/tests.py +++ b/tests/null_fk/tests.py @@ -17,9 +17,9 @@ def test_null_fk(self): # specified set of fields will properly LEFT JOIN multiple levels of # NULLs (and the things that come after the NULLs, or else data that # should exist won't). Regression test for #7369. - c = Comment.objects.select_related().get(id=c1.id) + c = Comment.objects.select_related("post").get(id=c1.id) self.assertEqual(c.post, p) - self.assertIsNone(Comment.objects.select_related().get(id=c2.id).post) + self.assertIsNone(Comment.objects.select_related("post").get(id=c2.id).post) self.assertQuerySetEqual( Comment.objects.select_related("post__forum__system_info").all(), diff --git a/tests/proxy_models/tests.py b/tests/proxy_models/tests.py index 98bba949f5ae..e7d4ec714fe2 100644 --- a/tests/proxy_models/tests.py +++ b/tests/proxy_models/tests.py @@ -320,17 +320,17 @@ def test_select_related(self): country = Country.objects.create(name="Australia") State.objects.create(name="New South Wales", country=country) - resp = [s.name for s in State.objects.select_related()] + resp = [s.name for s in State.objects.select_related("country")] self.assertEqual(resp, ["New South Wales"]) - resp = [s.name for s in StateProxy.objects.select_related()] + resp = [s.name for s in StateProxy.objects.select_related("country")] self.assertEqual(resp, ["New South Wales"]) self.assertEqual( StateProxy.objects.get(name="New South Wales").name, "New South Wales" ) - resp = StateProxy.objects.select_related().get(name="New South Wales") + resp = StateProxy.objects.select_related("country").get(name="New South Wales") self.assertEqual(resp.name, "New South Wales") def test_filter_proxy_relation_reverse(self): @@ -369,15 +369,19 @@ def test_proxy_bug(self): self.assertEqual(repr(resp), "") # Select related + filter on proxy - resp = ProxyBug.objects.select_related().get(version__icontains="beta") + resp = ProxyBug.objects.select_related("reporter").get( + version__icontains="beta" + ) self.assertEqual(repr(resp), "") # Proxy of proxy, select_related + filter - resp = ProxyProxyBug.objects.select_related().get(version__icontains="beta") + resp = ProxyProxyBug.objects.select_related("reporter").get( + version__icontains="beta" + ) self.assertEqual(repr(resp), "") # Select related + filter on a related proxy field - resp = ProxyImprovement.objects.select_related().get( + resp = ProxyImprovement.objects.select_related("reporter").get( reporter__name__icontains="butor" ) self.assertEqual( @@ -385,7 +389,7 @@ def test_proxy_bug(self): ) # Select related + filter on a related proxy of proxy field - resp = ProxyImprovement.objects.select_related().get( + resp = ProxyImprovement.objects.select_related("associated_bug").get( associated_bug__summary__icontains="fix" ) self.assertEqual( diff --git a/tests/queries/tests.py b/tests/queries/tests.py index a9f10de7b368..169ca4924af4 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -13,7 +13,8 @@ from django.db.models.sql.constants import LOUTER from django.db.models.sql.where import AND, OR, NothingNode, WhereNode from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature -from django.test.utils import CaptureQueriesContext, register_lookup +from django.test.utils import CaptureQueriesContext, ignore_warnings, register_lookup +from django.utils.deprecation import RemovedInDjango70Warning from .models import ( FK1, @@ -617,7 +618,7 @@ def test_ticket2400(self): def test_ticket2496(self): self.assertSequenceEqual( Item.objects.extra(tables=["queries_author"]) - .select_related() + .select_related("creator") .order_by("name")[:1], [self.i4], ) @@ -668,7 +669,9 @@ def test_tickets_2076_7256(self): self.assertEqual(len(qs.query.alias_map), 1) def test_tickets_2874_3002(self): - qs = Item.objects.select_related().order_by("note__note", "name") + qs = Item.objects.select_related("creator", "note").order_by( + "note__note", "name" + ) self.assertQuerySetEqual(qs, [self.i2, self.i4, self.i1, self.i3]) # This is also a good select_related() test because there are multiple @@ -853,7 +856,7 @@ def test_ticket7813(self): # We should also be able to pickle things that use select_related(). # The only tricky thing here is to ensure that we do the related # selections properly after unpickling. - qs = Item.objects.select_related() + qs = Item.objects.select_related("creator", "note") query = qs.query.get_compiler(qs.db).as_sql()[0] query2 = pickle.loads(pickle.dumps(qs.query)) self.assertEqual(query2.get_compiler(qs.db).as_sql()[0], query) @@ -2009,7 +2012,13 @@ def test_tickets_3045_3288(self): # infinitely if you forgot to specify "depth". Now we set an arbitrary # default upper bound. self.assertSequenceEqual(X.objects.all(), []) - self.assertSequenceEqual(X.objects.select_related(), []) + # RemovedInDjango70Warning: when the deprecation ends, remove this test + # case. + with ignore_warnings( + category=RemovedInDjango70Warning, + message=r"Calling select_related\(\) with no arguments is deprecated\.", + ): + self.assertSequenceEqual(X.objects.select_related(), []) class SubclassFKTests(TestCase): diff --git a/tests/select_related/tests.py b/tests/select_related/tests.py index 59d1270aa069..97c146ed92c7 100644 --- a/tests/select_related/tests.py +++ b/tests/select_related/tests.py @@ -3,7 +3,8 @@ from django.core.exceptions import FieldError from django.db.models import FETCH_PEERS from django.test import SimpleTestCase, TestCase -from django.test.utils import garbage_collect +from django.test.utils import garbage_collect, ignore_warnings +from django.utils.deprecation import RemovedInDjango70Warning from .models import ( Bookmark, @@ -106,10 +107,15 @@ def test_list_without_select_related(self): ], ) + # RemovedInDjango70Warning. def test_list_with_select_related(self): """select_related() applies to entire lists, not just items.""" with self.assertNumQueries(1): - world = Species.objects.select_related() + with ignore_warnings( + category=RemovedInDjango70Warning, + message=r"Calling select_related\(\) with no arguments is deprecated\.", + ): + world = Species.objects.select_related() families = [o.genus.family.name for o in world] self.assertEqual( sorted(families), @@ -121,6 +127,15 @@ def test_list_with_select_related(self): ], ) + # RemovedInDjango70Warning. + def test_select_related_no_arguments_deprecated(self): + msg = ( + "Calling select_related() with no arguments is deprecated. " + "Specify the fields to fetch instead." + ) + with self.assertWarnsMessage(RemovedInDjango70Warning, msg): + Species.objects.select_related() + def test_list_with_depth(self): """ Passing a relationship field lookup specifier to select_related() will @@ -137,7 +152,7 @@ def test_list_with_depth(self): def test_select_related_with_extra(self): s = ( Species.objects.all() - .select_related() + .select_related("genus") .extra(select={"a": "select_related_species.id + 10"})[0] ) self.assertEqual(s.id + 10, s.a) diff --git a/tests/select_related_onetoone/tests.py b/tests/select_related_onetoone/tests.py index 83462ed07166..00025fcdca55 100644 --- a/tests/select_related_onetoone/tests.py +++ b/tests/select_related_onetoone/tests.py @@ -1,6 +1,8 @@ from django.core.exceptions import FieldError from django.db.models import FilteredRelation from django.test import SimpleTestCase, TestCase +from django.test.utils import ignore_warnings +from django.utils.deprecation import RemovedInDjango70Warning from .models import ( AdvancedUserStat, @@ -86,7 +88,15 @@ def test_back_and_forward(self): self.assertEqual(u.userstat.user.username, "test") def test_not_followed_by_default(self): - with self.assertNumQueries(2): + # RemovedInDjango70Warning: when the deprecation ends, remove this + # test. + with ( + self.assertNumQueries(2), + ignore_warnings( + category=RemovedInDjango70Warning, + message=r"Calling select_related\(\) with no arguments is deprecated\.", + ), + ): u = User.objects.select_related().get(username="test") self.assertEqual(u.userstat.posts, 150) diff --git a/tests/select_related_regress/tests.py b/tests/select_related_regress/tests.py index b271c5b48c00..8e24098e28af 100644 --- a/tests/select_related_regress/tests.py +++ b/tests/select_related_regress/tests.py @@ -62,7 +62,7 @@ def test_regression_7110(self): Connection.objects.filter( start__device__building=b, end__device__building=b ) - .select_related() + .select_related("start__device__building", "end__device__building") .order_by("id") ) self.assertEqual( @@ -93,7 +93,7 @@ def test_regression_8106(self): c = Class.objects.create(org=o) Enrollment.objects.create(std=s, cls=c) - e_related = Enrollment.objects.select_related()[0] + e_related = Enrollment.objects.select_related("std", "cls")[0] self.assertEqual(e_related.std.person.user.name, "std") self.assertEqual(e_related.cls.org.person.user.name, "org") @@ -112,7 +112,9 @@ def test_regression_8036(self): client = Client.objects.create(name="client", status=active) self.assertEqual(client.status, active) - self.assertEqual(Client.objects.select_related()[0].status, active) + self.assertEqual( + Client.objects.select_related("state", "status")[0].status, active + ) self.assertEqual(Client.objects.select_related("state")[0].status, active) self.assertEqual( Client.objects.select_related("state", "status")[0].status, active @@ -221,7 +223,7 @@ def test_regression_19870(self): Chick.objects.create(name="Chick", mother=hen) self.assertEqual(Chick.objects.all()[0].mother.name, "Hen") - self.assertEqual(Chick.objects.select_related()[0].mother.name, "Hen") + self.assertEqual(Chick.objects.select_related("mother")[0].mother.name, "Hen") def test_regression_10733(self): a = A.objects.create(name="a", lots_of_text="lots_of_text_a", a_field="a_field") @@ -237,7 +239,7 @@ def test_regression_10733(self): "c_b__lots_of_text", "c_a__name", "c_b__name", - ).select_related() + ).select_related("c_a", "c_b") self.assertSequenceEqual(results, [c]) with self.assertNumQueries(0): qs_c = results[0]