Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion website/admin/project_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class GrantInline(admin.TabularInline):
@admin.register(Project, site=ml_admin_site)
class ProjectAdmin(ImageCroppingMixin, admin.ModelAdmin):
# Search by name plus the research-area facets editors think in (umbrella, keyword).
search_fields = ['name', 'short_name', 'project_umbrellas__name', 'keywords__name']
search_fields = ['name', 'short_name', 'project_umbrellas__name', 'keywords__keyword']
ordering = ('name',) # deterministic alphabetical sort (matched the autocomplete already)
inlines = [GrantInline, BannerInline, PhotoInline, ProjectRoleInline]

Expand Down
57 changes: 56 additions & 1 deletion website/tests/test_admin_changelist.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@
regress.
"""

from django.contrib.auth import get_user_model
from django.core.files.base import ContentFile
from django.test import RequestFactory

from website.models import News, Photo
from website.models import News, Photo, Keyword
from website.admin.admin_site import ml_admin_site
from website.admin.news_admin import NewsAdmin
from website.admin.keyword_admin import KeywordAdmin
Expand Down Expand Up @@ -97,6 +99,22 @@ def test_project_search_and_ordering(self):
self.assertIn('project_umbrellas__name', ProjectAdmin.search_fields)
self.assertEqual(ProjectAdmin.ordering, ('name',))

def test_project_search_executes(self):
"""Regression for the project changelist search 500: search_fields
referenced 'keywords__name', but Keyword's field is 'keyword', so any
admin search raised FieldError ('Unsupported lookup name__icontains for
ForeignKey'). Exercise the real search query so a bad field path can't
silently regress. (admin search on /admin/website/project/?q=...)"""
project = self.make_project(name="Sound Awareness", short_name="soundaware")
project.keywords.add(Keyword.objects.create(keyword="accessibility"))

admin = ProjectAdmin(type(project), ml_admin_site)
request = RequestFactory().get('/admin/website/project/', {'q': 'access'})
request.user = None
qs, _ = admin.get_search_results(request, admin.get_queryset(request), 'access')
# Force SQL evaluation — this is what raised the 500 before the fix.
self.assertIn(project, list(qs))

def test_project_umbrella_search_and_ordering(self):
self.assertEqual(ProjectUmbrellaAdmin.search_fields, ['name', 'short_name'])
self.assertEqual(ProjectUmbrellaAdmin.ordering, ('name',))
Expand All @@ -115,3 +133,40 @@ def test_position_search(self):

def test_sponsor_ordering(self):
self.assertEqual(SponsorAdmin.ordering, ('name',))


class AdminSearchExecutesTests(DatabaseTestCase):
"""Every registered admin's search must actually *run*, not just be declared.

The project changelist 500'd in prod because search_fields pointed at a
field path that doesn't exist ('keywords__name' — Keyword's field is
'keyword'). The older config tests only asserted that search_fields
*contained* a given string, so they never built the SQL and couldn't catch a
bad path. This sweep loops over every ModelAdmin registered on the custom
admin site, runs its search with a benign term, and forces query evaluation
— turning any invalid field path (FieldError) into a failing test for the
specific admin, automatically covering admins added in the future.
"""

@classmethod
def setUpTestData(cls):
cls.superuser = get_user_model().objects.create_superuser(
username="search_sweep_admin", email="sweep@example.com", password="pw")

def test_all_admin_searches_execute(self):
rf = RequestFactory()
checked = []
for model, model_admin in ml_admin_site._registry.items():
if not model_admin.search_fields:
continue
url = f"/admin/{model._meta.app_label}/{model._meta.model_name}/"
request = rf.get(url, {"q": "test"})
request.user = self.superuser
with self.subTest(admin=type(model_admin).__name__):
qs, _ = model_admin.get_search_results(
request, model_admin.get_queryset(request), "test")
# Force SQL evaluation — a bad field path raises here, not above.
list(qs[:1])
checked.append(type(model_admin).__name__)
# Guard against the loop silently finding nothing to check.
self.assertGreater(len(checked), 5)
Loading