From ad9b1e29fbea671126ee0862d7ed0e1035985875 Mon Sep 17 00:00:00 2001
From: Jon Froehlich <jonf@cs.uw.edu>
Date: Wed, 24 Jun 2026 06:43:25 -0700
Subject: [PATCH] Filter advisor/mentor autocomplete dropdowns by role (#1126)

The advisor field's plain <select> was already role-filtered via
PositionInline.formfield_for_foreignkey, but co_advisor and grad_mentor
are autocomplete_fields: their options come from the admin autocomplete
JSON endpoint (AutocompleteJsonView -> PersonAdmin.get_search_results),
which bypasses formfield_for_foreignkey. So the autocomplete search
offered every person -- e.g. undergrads as co-advisors, exactly the
bug in #1126. Save-time validation caught bad picks, but only after
the editor had already selected an invalid option.

Add PersonAdmin.get_search_results that reads the requesting Position
field (model_name/field_name passed by the autocomplete view) and
narrows results to the same role-appropriate querysets used for the
plain dropdowns: active professors for advisor/co_advisor, active
senior members for grad_mentor.

Add integration tests that hit the real autocomplete endpoint and pin
the role filtering for both fields (failing before this change).

The remaining ask on #1126 (auto-focus the search box on open) is now
handled for free by the Select2 bundled with the current Django.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 website/admin/person_admin.py                 | 33 ++++++-
 .../tests/test_advisor_mentor_autocomplete.py | 85 +++++++++++++++++++
 2 files changed, 116 insertions(+), 2 deletions(-)
 create mode 100644 website/tests/test_advisor_mentor_autocomplete.py

diff --git a/website/admin/person_admin.py b/website/admin/person_admin.py
index 0923181f..e7da8523 100644
--- a/website/admin/person_admin.py
+++ b/website/admin/person_admin.py
@@ -176,8 +176,37 @@ def save_model(self, request, obj, form, change):
     inlines = [PositionInline, ProjectRoleInline]
 
     # We must define search_fields in order to use the autocomplete_fields option
-    search_fields = ['first_name', 'last_name',] 
-    
+    search_fields = ['first_name', 'last_name',]
+
+    def get_search_results(self, request, queryset, search_term):
+        """Role-filter the admin autocomplete results for advisor/mentor fields (#1126).
+
+        ``PositionInline.formfield_for_foreignkey`` filters the plain ``advisor``
+        <select>, but ``co_advisor`` and ``grad_mentor`` are ``autocomplete_fields``:
+        their options come from this endpoint (``AutocompleteJsonView``), which
+        bypasses ``formfield_for_foreignkey``. Without this, the autocomplete search
+        would offer every person (e.g. undergrads as co-advisors). We narrow the
+        queryset to the same role-appropriate sets used for the plain dropdowns,
+        keyed off the requesting Position field passed by the autocomplete view.
+        """
+        queryset, may_have_duplicates = super().get_search_results(
+            request, queryset, search_term)
+
+        if request.GET.get('model_name') == 'position':
+            field_name = request.GET.get('field_name')
+            if field_name in ('advisor', 'co_advisor'):
+                allowed = get_active_professors_queryset()
+            elif field_name == 'grad_mentor':
+                allowed = get_active_mentors_queryset()
+            else:
+                allowed = None
+
+            if allowed is not None:
+                queryset = queryset.filter(
+                    pk__in=allowed.values_list('pk', flat=True))
+
+        return queryset, may_have_duplicates
+
     # The list display lets us control what is shown in the default persons table at Home > Website > People
     # info on displaying multiple entries comes from http://stackoverflow.com/questions/9164610/custom-columns-using-django-admin
     # The count columns (project_count / pub_count / talk_count) read annotations
diff --git a/website/tests/test_advisor_mentor_autocomplete.py b/website/tests/test_advisor_mentor_autocomplete.py
new file mode 100644
index 00000000..c06c6912
--- /dev/null
+++ b/website/tests/test_advisor_mentor_autocomplete.py
@@ -0,0 +1,85 @@
+"""
+Regression tests for issue #1126.
+
+When an admin edits a Person and sets an advisor / co-advisor / mentor on an
+inline Position, the dropdown must only offer role-appropriate people:
+
+  - advisor / co_advisor  -> currently active professors only
+  - grad_mentor ("Mentor") -> currently active senior lab members only
+
+The plain ``advisor`` <select> is filtered by
+``PositionInline.formfield_for_foreignkey``. But ``co_advisor`` and
+``grad_mentor`` are ``autocomplete_fields``: their options come from the
+admin autocomplete JSON endpoint (``AutocompleteJsonView`` -> the *target*
+model admin's ``get_search_results``), which bypasses
+``formfield_for_foreignkey`` entirely. Without a ``get_search_results``
+filter on ``PersonAdmin``, the autocomplete search returns *every* person
+(undergrads included), which is the bug reported in #1126.
+
+These tests hit the real autocomplete endpoint and assert the role filtering
+holds for all three fields.
+"""
+
+from datetime import date
+
+from django.contrib.auth import get_user_model
+from django.urls import reverse
+
+from website.models import Person, Position
+from website.models.position import Title
+from website.tests.base import DatabaseTestCase
+
+
+class AdvisorMentorAutocompleteTests(DatabaseTestCase):
+    @classmethod
+    def setUpTestData(cls):
+        # An active full professor: valid advisor/co-advisor, also a valid mentor.
+        cls.prof = Person.objects.create(first_name="Pat", last_name="Professor")
+        Position.objects.create(person=cls.prof, start_date=date(2015, 1, 1),
+                                end_date=None, title=Title.FULL_PROF)
+
+        # An active PhD student: a valid mentor, but NOT a valid advisor.
+        cls.phd = Person.objects.create(first_name="Parker", last_name="Phd")
+        Position.objects.create(person=cls.phd, start_date=date(2021, 1, 1),
+                                end_date=None, title=Title.PHD_STUDENT)
+
+        # An active undergrad: NOT a valid advisor and NOT a valid mentor.
+        cls.ugrad = Person.objects.create(first_name="Uma", last_name="Undergrad")
+        Position.objects.create(person=cls.ugrad, start_date=date(2023, 1, 1),
+                                end_date=None, title=Title.UGRAD)
+
+    def setUp(self):
+        super().setUp()
+        User = get_user_model()
+        User.objects.create_superuser("admin1126", "admin1126@example.com", "pw")
+        self.client.login(username="admin1126", password="pw")
+
+    def _autocomplete_ids(self, field_name, term=""):
+        """Return the set of Person ids the admin autocomplete offers for the
+        given Position FK field."""
+        resp = self.client.get(
+            reverse("admin:autocomplete"),
+            {
+                "app_label": "website",
+                "model_name": "position",
+                "field_name": field_name,
+                "term": term,
+            },
+        )
+        self.assertEqual(resp.status_code, 200)
+        return {int(item["id"]) for item in resp.json()["results"]}
+
+    def test_co_advisor_autocomplete_only_offers_active_professors(self):
+        ids = self._autocomplete_ids("co_advisor")
+        self.assertIn(self.prof.id, ids)
+        self.assertNotIn(self.phd.id, ids)
+        self.assertNotIn(self.ugrad.id, ids)
+
+    def test_grad_mentor_autocomplete_only_offers_senior_members(self):
+        # Mentors are senior lab members (postdoc/PhD/MS/research-scientist/etc.).
+        # Professors are advisors, not mentors, so FULL_PROF is intentionally
+        # excluded by get_active_mentors_queryset.
+        ids = self._autocomplete_ids("grad_mentor")
+        self.assertIn(self.phd.id, ids)
+        self.assertNotIn(self.prof.id, ids)
+        self.assertNotIn(self.ugrad.id, ids)