From a6c9409e03b4e75a356ff9912ffd7071ff543495 Mon Sep 17 00:00:00 2001 From: Tidiane Dia Date: Wed, 24 May 2023 17:11:56 +0100 Subject: [PATCH] Avoid N+1 queries in users index view --- CHANGELOG.txt | 1 + docs/releases/5.1.md | 1 + wagtail/users/tests/test_admin_views.py | 13 +++++++++++++ .../users/views/bulk_actions/user_bulk_action.py | 2 +- wagtail/users/views/users.py | 11 ++++++++--- 5 files changed, 24 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index abecdf7a6abc..4ac1026d462b 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -60,6 +60,7 @@ Changelog * Fix: Improve visibility of scheduled publishing errors in status side panel (Sage Abdullah) * Fix: Prevent 'choose' permission from being ignored when looking up 'choose', 'edit' and 'delete' permissions in combination (Sage Abdullah) * Fix: Take user's permissions into account for image / document counts on the admin dashboard (Sage Abdullah) + * Fix: Avoid N+1 queries in users index view (Tidiane Dia) * Docs: Document how to add non-ModelAdmin views to a `ModelAdminGroup` (Onno Timmerman) * Docs: Document how to add StructBlock data to a StreamField (Ramon Wenger) * Docs: Update ReadTheDocs settings to v2 to resolve urllib3 issue in linkcheck extension (Thibaud Colas) diff --git a/docs/releases/5.1.md b/docs/releases/5.1.md index e1a441e713ed..03a37b3ce320 100644 --- a/docs/releases/5.1.md +++ b/docs/releases/5.1.md @@ -117,6 +117,7 @@ This feature was developed by Aman Pandey as part of the Google Summer of Code p * Improve visibility of scheduled publishing errors in status side panel (Sage Abdullah) * Prevent 'choose' permission from being ignored when looking up 'choose', 'edit' and 'delete' permissions in combination (Sage Abdullah) * Take user's permissions into account for image / document counts on the admin dashboard (Sage Abdullah) + * Avoid N+1 queries in users index view (Tidiane Dia) ### Documentation diff --git a/wagtail/users/tests/test_admin_views.py b/wagtail/users/tests/test_admin_views.py index f5ed8e88e022..be2748b2e383 100644 --- a/wagtail/users/tests/test_admin_views.py +++ b/wagtail/users/tests/test_admin_views.py @@ -246,6 +246,19 @@ def test_valid_ordering(self): response = self.get({"ordering": "username"}) self.assertEqual(response.context_data["ordering"], "username") + def test_num_queries(self): + # Warm up + self.get() + + num_queries = 9 + with self.assertNumQueries(num_queries): + self.get() + + # Ensure we don't have any N+1 queries + self.create_user("test", "test@example.com", "gu@rd14n") + with self.assertNumQueries(num_queries): + self.get() + class TestUserIndexResultsView(WagtailTestUtils, TestCase): def setUp(self): diff --git a/wagtail/users/views/bulk_actions/user_bulk_action.py b/wagtail/users/views/bulk_actions/user_bulk_action.py index c862da032b10..76d05db58df9 100644 --- a/wagtail/users/views/bulk_actions/user_bulk_action.py +++ b/wagtail/users/views/bulk_actions/user_bulk_action.py @@ -11,7 +11,7 @@ def get_all_objects_in_listing_query(self, parent_id): listing_objects = self.model.objects.all().values_list("pk", flat=True) if "q" in self.request.GET: q = self.request.GET.get("q") - model_fields = [f.name for f in self.model._meta.get_fields()] + model_fields = {f.name for f in self.model._meta.get_fields()} conditions = get_users_filter_query(q, model_fields) listing_objects = listing_objects.filter(conditions) diff --git a/wagtail/users/views/users.py b/wagtail/users/views/users.py index b71b4075b97f..936412872fa9 100644 --- a/wagtail/users/views/users.py +++ b/wagtail/users/views/users.py @@ -83,11 +83,12 @@ class Index(IndexView): is_searchable = True page_title = gettext_lazy("Users") + model_fields = [f.name for f in User._meta.get_fields()] + def setup(self, request, *args, **kwargs): super().setup(request, *args, **kwargs) self.group = get_object_or_404(Group, id=args[0]) if args else None self.group_filter = Q(groups=self.group) if self.group else Q() - self.model_fields = [f.name for f in User._meta.get_fields()] def get_index_results_url(self): if self.group: @@ -99,8 +100,9 @@ def get_valid_orderings(self): return ["name", "username"] def get_queryset(self): + model_fields = set(self.model_fields) if self.is_searching: - conditions = get_users_filter_query(self.search_query, self.model_fields) + conditions = get_users_filter_query(self.search_query, model_fields) users = User.objects.filter(self.group_filter & conditions) else: users = User.objects.filter(self.group_filter) @@ -108,7 +110,10 @@ def get_queryset(self): if self.locale: users = users.filter(locale=self.locale) - if "last_name" in self.model_fields and "first_name" in self.model_fields: + if "wagtail_userprofile" in model_fields: + users = users.select_related("wagtail_userprofile") + + if "last_name" in model_fields and "first_name" in model_fields: users = users.order_by("last_name", "first_name") if self.get_ordering() == "username":