From a1dbe3e5f676a5560fab01c261e44db81eb42411 Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Mon, 27 Jul 2026 13:34:27 +0200 Subject: [PATCH 1/2] Use existence queries for org group membership checks Org.get_user_org_group previously used 'user in queryset' checks which load all members of each role; with large orgs this fetched thousands of user rows on every request via the context processor and permission checks. Replace with .filter(id=...).exists() lookups and add a query count test. --- dash/orgs/models.py | 6 +++--- test_runner/tests.py | 23 ++++++++++++++++++++--- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/dash/orgs/models.py b/dash/orgs/models.py index 6e7ad29..083e416 100644 --- a/dash/orgs/models.py +++ b/dash/orgs/models.py @@ -144,11 +144,11 @@ def get_org_users(self): return org_users.distinct() def get_user_org_group(self, user): - if user in self.get_org_admins(): + if self.administrators.filter(id=user.id).exists(): user._org_group = Group.objects.get(name="Administrators") - elif user in self.get_org_editors(): + elif self.editors.filter(id=user.id).exists(): user._org_group = Group.objects.get(name="Editors") - elif user in self.get_org_viewers(): + elif self.viewers.filter(id=user.id).exists(): user._org_group = Group.objects.get(name="Viewers") else: user._org_group = None diff --git a/test_runner/tests.py b/test_runner/tests.py index 0dcfdf0..742f4b4 100644 --- a/test_runner/tests.py +++ b/test_runner/tests.py @@ -1,6 +1,5 @@ import zoneinfo -from dash.tags.models import Tag -from unittest.mock import Mock, patch, call +from unittest.mock import Mock, call, patch import valkey from smartmin.tests import SmartminTest @@ -25,8 +24,9 @@ from dash.orgs.tasks import org_task from dash.orgs.templatetags.dashorgs import display_time, national_phone from dash.stories.models import Story, StoryImage -from dash.utils import random_string +from dash.tags.models import Tag from dash.test import MockResponse +from dash.utils import random_string class UserTest(SmartminTest): @@ -412,6 +412,23 @@ def setUp(self): self.org = self.create_org("uganda", self.admin) + def test_get_user_org_group_num_queries(self): + viewer = self.create_user("Viewer") + editor = self.create_user("Editor") + non_member = self.create_user("NonMember") + self.org.viewers.add(viewer) + self.org.editors.add(editor) + + # membership is checked with existence queries rather than fetching entire member lists + with self.assertNumQueries(2): + self.assertEqual(self.org.get_user_org_group(self.admin).name, "Administrators") + with self.assertNumQueries(3): + self.assertEqual(self.org.get_user_org_group(editor).name, "Editors") + with self.assertNumQueries(4): + self.assertEqual(self.org.get_user_org_group(viewer).name, "Viewers") + with self.assertNumQueries(3): + self.assertIsNone(self.org.get_user_org_group(non_member)) + def test_org_model(self): user = self.create_user("User") From b2290e0d124e506d2e2c7ec7b8b1ab820672037b Mon Sep 17 00:00:00 2001 From: Norbert Kwizera Date: Mon, 27 Jul 2026 16:29:06 +0200 Subject: [PATCH 2/2] Cache org group lookups on the user and pin query shape in tests Cache the computed org group on the user object so repeated calls within a request (context processor plus permission checks) only query once. Replace the query count test with assertions on the SQL shape of the membership checks, since counting queries alone did not distinguish existence queries from fetching full member lists. Also cover the anonymous user case. --- dash/orgs/models.py | 5 ++++- test_runner/tests.py | 38 +++++++++++++++++++++++++++++--------- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/dash/orgs/models.py b/dash/orgs/models.py index 083e416..c7ec263 100644 --- a/dash/orgs/models.py +++ b/dash/orgs/models.py @@ -144,6 +144,9 @@ def get_org_users(self): return org_users.distinct() def get_user_org_group(self, user): + if hasattr(user, "_org_group"): + return user._org_group + if self.administrators.filter(id=user.id).exists(): user._org_group = Group.objects.get(name="Administrators") elif self.editors.filter(id=user.id).exists(): @@ -153,7 +156,7 @@ def get_user_org_group(self, user): else: user._org_group = None - return getattr(user, "_org_group", None) + return user._org_group def get_user(self): user = self.administrators.filter(is_active=True).first() diff --git a/test_runner/tests.py b/test_runner/tests.py index 742f4b4..c541c16 100644 --- a/test_runner/tests.py +++ b/test_runner/tests.py @@ -6,11 +6,13 @@ from temba_client.v2 import TembaClient from django.conf import settings -from django.contrib.auth.models import Group, User +from django.contrib.auth.models import AnonymousUser, Group, User from django.core import mail from django.core.exceptions import DisallowedHost +from django.db import connection from django.db.utils import IntegrityError from django.http import HttpRequest, HttpResponse +from django.test.utils import CaptureQueriesContext from django.urls import ResolverMatch, reverse from django.utils.encoding import force_str @@ -412,21 +414,39 @@ def setUp(self): self.org = self.create_org("uganda", self.admin) - def test_get_user_org_group_num_queries(self): + def test_get_user_org_group_queries(self): viewer = self.create_user("Viewer") editor = self.create_user("Editor") non_member = self.create_user("NonMember") self.org.viewers.add(viewer) self.org.editors.add(editor) - # membership is checked with existence queries rather than fetching entire member lists - with self.assertNumQueries(2): + self.assertIsNone(self.org.get_user_org_group(AnonymousUser())) + + def assert_membership_queries(user, group_name): + with CaptureQueriesContext(connection) as context: + group = self.org.get_user_org_group(user) + + self.assertEqual(group_name, group.name if group else None) + + # ignore the query that fetches the group itself + membership_queries = [q["sql"] for q in context.captured_queries if '"auth_group"' not in q["sql"]] + self.assertTrue(membership_queries) + + # membership is checked with existence queries rather than fetching entire member lists + for sql in membership_queries: + self.assertTrue(sql.startswith("SELECT 1 AS"), f"expected an existence query but got: {sql}") + self.assertNotIn('"auth_user"."password"', sql) + + assert_membership_queries(self.admin, "Administrators") + assert_membership_queries(editor, "Editors") + assert_membership_queries(viewer, "Viewers") + assert_membership_queries(non_member, None) + + # result is cached on the user object so repeated lookups don't hit the database + with self.assertNumQueries(0): self.assertEqual(self.org.get_user_org_group(self.admin).name, "Administrators") - with self.assertNumQueries(3): - self.assertEqual(self.org.get_user_org_group(editor).name, "Editors") - with self.assertNumQueries(4): - self.assertEqual(self.org.get_user_org_group(viewer).name, "Viewers") - with self.assertNumQueries(3): + with self.assertNumQueries(0): self.assertIsNone(self.org.get_user_org_group(non_member)) def test_org_model(self):