Optimize the groups query and add test

hypha/apply/users/forms.py

@@ -216,26 +216,25 @@ def clean_full_name(self):
 
 
 def get_become_user_choices():
-    """Returns list of active non-superusers with their roles as choice tuples."""
-    active_users = User.objects.filter(is_active=True, is_superuser=False)
-    choices = []
+    """Returns list of active non-superusers with their roles as choice tuples.
 
-    for user in active_users:
-        role_names = user.get_role_names()
-        if role_names:
-            roles_html = ", ".join(
-                [
-                    f'<span class="inline-flex items-center px-2.5 py-0.5 rounded-full text-xs font-medium bg-gray-100 text-gray-800">{str(g)}</span>'
-                    for g in role_names
-                ]
-            )
-            label = f"{user} ({roles_html})"
-        else:
-            label = str(user)
+    Returns:
+        list: Tuples of (user_id, formatted_label) for form choices
+    """
+    active_users = (
+        User.objects.filter(is_active=True, is_superuser=False)
+        .prefetch_related("groups")
+        .order_by("last_login")
+    )
 
-        choices.append((user.pk, mark_safe(label)))
+    def format_user_label(user):
+        role_names = user.get_role_names()
+        if not role_names:
+            return str(user)
+        roles_text = ", ".join(map(str, role_names))
+        return mark_safe(f"{user} ({roles_text})")
 
-    return choices
+    return [(user.pk, format_user_label(user)) for user in active_users]
 
 
 class BecomeUserForm(forms.Form):

hypha/apply/users/models.py

@@ -244,41 +244,39 @@ def get_role_names(self):
 
     @cached_property
     def roles(self):
-        return list(self.groups.values_list("name", flat=True))
+        return [g.name for g in self.groups.all()]
 
     @cached_property
     def is_apply_staff(self):
-        return self.groups.filter(name=STAFF_GROUP_NAME).exists() or self.is_superuser
+        return STAFF_GROUP_NAME in self.roles or self.is_superuser
 
     @cached_property
     def is_apply_staff_admin(self):
-        return (
-            self.groups.filter(name=TEAMADMIN_GROUP_NAME).exists() or self.is_superuser
-        )
+        return TEAMADMIN_GROUP_NAME in self.roles or self.is_superuser
 
     @cached_property
     def is_reviewer(self):
-        return self.groups.filter(name=REVIEWER_GROUP_NAME).exists()
+        return REVIEWER_GROUP_NAME in self.roles
 
     @cached_property
     def is_partner(self):
-        return self.groups.filter(name=PARTNER_GROUP_NAME).exists()
+        return PARTNER_GROUP_NAME in self.roles
 
     @cached_property
     def is_community_reviewer(self):
-        return self.groups.filter(name=COMMUNITY_REVIEWER_GROUP_NAME).exists()
+        return COMMUNITY_REVIEWER_GROUP_NAME in self.roles
 
     @cached_property
     def is_applicant(self):
-        return self.groups.filter(name=APPLICANT_GROUP_NAME).exists()
+        return APPLICANT_GROUP_NAME in self.roles
 
     @cached_property
     def is_approver(self):
-        return self.groups.filter(name=APPROVER_GROUP_NAME).exists()
+        return APPROVER_GROUP_NAME in self.roles
 
     @cached_property
     def is_finance(self):
-        return self.groups.filter(name=FINANCE_GROUP_NAME).exists()
+        return FINANCE_GROUP_NAME in self.roles
 
     @cached_property
     def is_org_faculty(self):
@@ -298,14 +296,11 @@ def can_access_dashboard(self):
 
     @cached_property
     def is_contracting(self):
-        return self.groups.filter(name=CONTRACTING_GROUP_NAME).exists()
+        return CONTRACTING_GROUP_NAME in self.roles
 
     @cached_property
     def is_contracting_approver(self):
-        return (
-            self.groups.filter(name=CONTRACTING_GROUP_NAME).exists()
-            and self.groups.filter(name=APPROVER_GROUP_NAME).exists()
-        )
+        return self.is_contracting and self.is_approver
 
     def get_absolute_url(self):
         """Used in the activities messages to generate URL for user instances.

hypha/apply/users/tests/test_forms.py

@@ -1,7 +1,8 @@
+import pytest
 from django.forms.models import model_to_dict
 from django.test import RequestFactory, TestCase
 
-from ..forms import EmailChangePasswordForm, ProfileForm
+from ..forms import BecomeUserForm, EmailChangePasswordForm, ProfileForm
 from .factories import StaffFactory, UserFactory
 
 
@@ -113,3 +114,20 @@ def test_can_update_slack(self):
         form = EmailChangePasswordForm(self.user)
         form.save("", "", slack_name)
         self.assertEqual(self.user.slack, slack_name)
+
+
+@pytest.mark.django_db
+def test_become_user_form_query_count(django_assert_num_queries):
+    # create a three user
+    UserFactory(is_superuser=False)
+    UserFactory(is_superuser=False)
+    UserFactory(is_superuser=False)
+
+    # Enable query counting and verify only 2 queries are made
+    with django_assert_num_queries(2):
+        # This should trigger 2 queries:
+        # 1. Get active non-superusers
+        # 2. Prefetch related groups
+        form = BecomeUserForm()
+        # Access choices to trigger query execution
+        list(form.fields["user_pk"].choices)