Conversation
📝 WalkthroughWalkthroughThis PR introduces new autocomplete filters for Posts and Questions to filter by default or secondary project membership, and refactors the Projects admin summary methods to use queryset-based counting with a URL-encoding helper instead of explicit ID enumeration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@projects/admin.py`:
- Around line 877-887: The links use the `id__in` lookup which Django 5.2+ will
block unless whitelisted; update the admin to allow this lookup by either adding
a custom list filter class (subclassing SimpleListFilter) and include it in
QuestionAdmin.list_filter to expose/handle the "id__in" selection, or override
QuestionAdmin.lookup_allowed(self, lookup, value) to return True for lookups
starting with "id__in"; ensure the new filter class or lookup_allowed explicitly
permits the "id__in" lookup so the make_link-generated URLs do not raise
DisallowedModelAdminLookup.
In `@questions/admin.py`:
- Around line 484-502: The filter is using the wrong related model and field:
update DefaultOrSecondaryProjectFilter by changing rel_model from Post to
Project and adjust field_name to refer to the project's relation (e.g.,
"post__default_project") so the autocomplete shows Project instances and the
value is a Project PK; keep the queryset method that looks up Project
(Project.objects.get(...)) and continue returning
queryset.filter(post__in=Post.objects.filter_projects(project)) so the selected
project correctly filters Questions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6ef3dc21-3ea4-422f-badc-bcaed628958e
📒 Files selected for processing (3)
posts/admin.pyprojects/admin.pyquestions/admin.py
| ( | ||
| make_link( | ||
| f"{in_leaderboard_count} Questions in Primary Leaderboard", | ||
| id__in=",".join(map(str, in_leaderboard_ids)) or "0", | ||
| ), | ||
| ), | ||
| ( | ||
| make_link( | ||
| not_in_leaderboard_ids, | ||
| f"{not_in_leaderboard_count} Questions NOT in Primary Leaderboard", | ||
| id__in=",".join(map(str, not_in_leaderboard_ids)) or "0", | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's any custom id__in filter defined for QuestionAdmin
rg -n "id__in" questions/admin.py
# Check if there's a base admin class that might handle this
rg -nP "class.*Filter.*:" questions/admin.pyRepository: Metaculus/metaculus
Length of output: 126
🏁 Script executed:
# Check the actual code in projects/admin.py at the specified lines
sed -n '870,895p' projects/admin.py | cat -nRepository: Metaculus/metaculus
Length of output: 1141
🏁 Script executed:
# Look for QuestionAdmin class and its list_filter definition
rg -n "class QuestionAdmin" questions/admin.py -A 30Repository: Metaculus/metaculus
Length of output: 2364
🏁 Script executed:
# Check the make_link function definition
rg -n "def make_link" projects/admin.py -B 2 -A 15Repository: Metaculus/metaculus
Length of output: 1440
🏁 Script executed:
# Check if id__in filter exists or any custom filter handling in questions/admin.py
rg -n "list_filter" questions/admin.py -A 10Repository: Metaculus/metaculus
Length of output: 1205
🌐 Web query:
Django admin list_filter URL parameters unregistered filter behavior
💡 Result:
In Django admin, when accessing the changelist view with a URL parameter that does not correspond to any registered list_filter (an "unregistered filter"), the behavior depends on whether the parameter represents a valid model lookup and passes ModelAdmin.lookup_allowed. If the parameter is a valid lookup (e.g., ?myfield=value where myfield exists on the model or through relations) but not whitelisted by list_filter or lookup_allowed, Django raises DisallowedModelAdminLookup (a SuspiciousOperation subclass). This typically results in a 400 Bad Request response in production (DEBUG=False) or a debug page in development. If the parameter does not represent a valid lookup (e.g., ?invalidparam=value), it is silently ignored. The ChangeList processes only recognized parameters from registered filters (via get_filters), and unrecognized keys remain in self.params but do not affect the queryset or trigger errors. The page renders normally with the full (unfiltered by that param) queryset. The ?e=1 parameter appears in URLs after a previous DisallowedModelAdminLookup redirect (handled in older Django versions or custom code), indicating a prior invalid lookup attempt. In modern Django (4.2+), invalid lookups raise exceptions rather than redirecting. To allow custom unregistered parameters without affecting the queryset, add a pass-through SimpleListFilter with matching parameter_name whose lookups accepts the value and queryset returns the unchanged queryset, or override changelist_view to pop the parameter from request.GET after making it mutable.
Citations:
- 1: https://stackoverflow.com/questions/8494200/django-admin-custom-change-list-arguments-override-e-1
- 2: https://github.com/django/django/blob/master/django/contrib/admin/views/main.py
- 3: https://docs.djangoproject.com/en/4.2/_modules/django/contrib/admin/options
- 4: https://docs.djangoproject.com/en/4.1/ref/contrib/admin/
- 5: https://code.djangoproject.com/ticket/17936
🏁 Script executed:
# Check for lookup_allowed override in QuestionAdmin
rg -n "lookup_allowed" questions/admin.py -B 2 -A 5Repository: Metaculus/metaculus
Length of output: 45
🏁 Script executed:
# Check if there's any parent class that might handle this
rg -n "class CustomTranslationAdmin" questions/admin.py -B 2 -A 20Repository: Metaculus/metaculus
Length of output: 45
🏁 Script executed:
# Look for any configuration in base admin classes
fd -name "admin*.py" | xargs rg -l "lookup_allowed" | head -5Repository: Metaculus/metaculus
Length of output: 294
🏁 Script executed:
# Fix the fd command to search for admin files and check lookup_allowed
rg "lookup_allowed" --type python | head -20Repository: Metaculus/metaculus
Length of output: 94
🏁 Script executed:
# Check imports in questions/admin.py to see where CustomTranslationAdmin comes from
head -50 questions/admin.py | grep -E "^from|^import"Repository: Metaculus/metaculus
Length of output: 1249
🏁 Script executed:
# Check if QuestionAdmin or base classes override lookup_allowed
rg -A 20 "class QuestionAdmin" questions/admin.py | grep -E "lookup_allowed|class " | head -10Repository: Metaculus/metaculus
Length of output: 185
🏁 Script executed:
# Find and check CustomTranslationAdmin
rg -n "class CustomTranslationAdmin" -A 30Repository: Metaculus/metaculus
Length of output: 1908
🏁 Script executed:
# Check if there's any custom changelist_view or lookup_allowed in the admin configuration
rg -n "changelist_view|lookup_allowed" projects/admin.pyRepository: Metaculus/metaculus
Length of output: 45
🏁 Script executed:
# Check if there's any custom error handling or middleware
rg -n "DisallowedModelAdminLookup" --type pyRepository: Metaculus/metaculus
Length of output: 45
🏁 Script executed:
# Verify the Django version to confirm the behavior
grep -E "Django|django" pyproject.toml requirements.txt setup.py 2>/dev/null | head -5Repository: Metaculus/metaculus
Length of output: 314
The id__in URL parameter will cause a DisallowedModelAdminLookup error.
In Django 5.2+, using a valid lookup like id__in that isn't whitelisted in QuestionAdmin.list_filter raises a security exception (DisallowedModelAdminLookup), resulting in a 400 Bad Request response. The links for "Questions in Primary Leaderboard" and "Questions NOT in Primary Leaderboard" will error instead of filtering.
Add a custom filter to QuestionAdmin.list_filter to whitelist the id__in lookup (e.g., using SimpleListFilter or a custom filter class), or override lookup_allowed() to explicitly allow this lookup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@projects/admin.py` around lines 877 - 887, The links use the `id__in` lookup
which Django 5.2+ will block unless whitelisted; update the admin to allow this
lookup by either adding a custom list filter class (subclassing
SimpleListFilter) and include it in QuestionAdmin.list_filter to expose/handle
the "id__in" selection, or override QuestionAdmin.lookup_allowed(self, lookup,
value) to return True for lookups starting with "id__in"; ensure the new filter
class or lookup_allowed explicitly permits the "id__in" lookup so the
make_link-generated URLs do not raise DisallowedModelAdminLookup.
| class DefaultOrSecondaryProjectFilter(AutocompleteFilter): | ||
| """Autocomplete filter — `?default_or_secondary_project=<id>` matches | ||
| questions whose post has the project as default_project OR in the projects M2M.""" | ||
|
|
||
| title = "Default or Secondary Project" | ||
| field_name = "default_project" | ||
| rel_model = Post | ||
| parameter_name = "default_or_secondary_project" | ||
| use_pk_exact = False | ||
|
|
||
| def queryset(self, request, queryset): | ||
| value = self.value() | ||
| if not value: | ||
| return queryset | ||
| try: | ||
| project = Project.objects.get(pk=int(value)) | ||
| except (Project.DoesNotExist, ValueError): | ||
| return queryset.none() | ||
| return queryset.filter(post__in=Post.objects.filter_projects(project)) |
There was a problem hiding this comment.
Bug: rel_model = Post should be rel_model = Project.
The autocomplete widget will display Posts instead of Projects because rel_model = Post. When a user selects from the dropdown, the selected value will be a Post ID, but the queryset method tries to look it up as a Project ID (Project.objects.get(pk=int(value))), causing incorrect behavior or no results.
Additionally, field_name = "default_project" doesn't exist directly on Question. Consider using post__default_project for clarity, though this may be optional if rel_model is correctly set to Project.
🐛 Proposed fix
class DefaultOrSecondaryProjectFilter(AutocompleteFilter):
"""Autocomplete filter — `?default_or_secondary_project=<id>` matches
questions whose post has the project as default_project OR in the projects M2M."""
title = "Default or Secondary Project"
- field_name = "default_project"
- rel_model = Post
+ field_name = "post__default_project"
+ rel_model = Project
parameter_name = "default_or_secondary_project"
use_pk_exact = False📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class DefaultOrSecondaryProjectFilter(AutocompleteFilter): | |
| """Autocomplete filter — `?default_or_secondary_project=<id>` matches | |
| questions whose post has the project as default_project OR in the projects M2M.""" | |
| title = "Default or Secondary Project" | |
| field_name = "default_project" | |
| rel_model = Post | |
| parameter_name = "default_or_secondary_project" | |
| use_pk_exact = False | |
| def queryset(self, request, queryset): | |
| value = self.value() | |
| if not value: | |
| return queryset | |
| try: | |
| project = Project.objects.get(pk=int(value)) | |
| except (Project.DoesNotExist, ValueError): | |
| return queryset.none() | |
| return queryset.filter(post__in=Post.objects.filter_projects(project)) | |
| class DefaultOrSecondaryProjectFilter(AutocompleteFilter): | |
| """Autocomplete filter — `?default_or_secondary_project=<id>` matches | |
| questions whose post has the project as default_project OR in the projects M2M.""" | |
| title = "Default or Secondary Project" | |
| field_name = "post__default_project" | |
| rel_model = Project | |
| parameter_name = "default_or_secondary_project" | |
| use_pk_exact = False | |
| def queryset(self, request, queryset): | |
| value = self.value() | |
| if not value: | |
| return queryset | |
| try: | |
| project = Project.objects.get(pk=int(value)) | |
| except (Project.DoesNotExist, ValueError): | |
| return queryset.none() | |
| return queryset.filter(post__in=Post.objects.filter_projects(project)) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@questions/admin.py` around lines 484 - 502, The filter is using the wrong
related model and field: update DefaultOrSecondaryProjectFilter by changing
rel_model from Post to Project and adjust field_name to refer to the project's
relation (e.g., "post__default_project") so the autocomplete shows Project
instances and the value is a Project PK; keep the queryset method that looks up
Project (Project.objects.get(...)) and continue returning
queryset.filter(post__in=Post.objects.filter_projects(project)) so the selected
project correctly filters Questions.
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
Summary by CodeRabbit
New Features
Refactor