Conversation
Pass selected_text from Python to template to avoid AJAX call that only returns first 20 results. Selected values beyond pagination limit now display correctly on page reload. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 47 54 +7
=========================================
+ Hits 47 54 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes DALFRelatedFieldAjax filter UI repopulation by providing the selected option’s display text server-side, avoiding a fragile client-side “first page” lookup.
Changes:
- Add
selected_textcomputation inDALFRelatedFieldAjaxand expose it to the template. - Render
selected_textinto the AJAX filter template and use it to preselect the Select2 option without an extra AJAX request. - Add tests covering repopulation and a deleted-selected-value scenario.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/testproject/testapp/tests.py | Adds regression tests for selected-text repopulation and deleted-value handling. |
| src/dalf/templates/admin/filter/django_admin_list_filter_ajax.html | Adds a hidden field to pass selected_text to the frontend. |
| src/dalf/static/admin/js/django_admin_list_filter.js | Uses selected_text to prepopulate Select2 without an extra AJAX call. |
| src/dalf/admin.py | Computes selected_text from the related object and includes it in filter template params. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| obj = related_model.objects.get(pk=self.selected_value) | ||
| self.selected_text = str(obj) | ||
| except (related_model.DoesNotExist, ValueError): | ||
| self.selected_value = None |
There was a problem hiding this comment.
On DoesNotExist you set self.selected_value = None, but the query parameter is still present so the changelist remains filtered while the UI shows “All” and the clear button won’t appear (user can get stuck with an un-clearable filter). Instead, keep selected_value and set selected_text to a safe fallback (e.g., the raw ID or a localized “(deleted)” label) so Select2 can render a selected option and allow clearing.
| self.selected_value = None | |
| # Keep selected_value so the filter state matches the query parameter, | |
| # but use a safe fallback label so the UI can render and clear it. | |
| self.selected_text = str(self.selected_value) |
| try: | ||
| related_model = field.remote_field.model | ||
| obj = related_model.objects.get(pk=self.selected_value) | ||
| self.selected_text = str(obj) | ||
| except (related_model.DoesNotExist, ValueError): | ||
| self.selected_value = None |
There was a problem hiding this comment.
selected_text is derived via a direct related_model.objects.get(pk=...) without going through the admin/autocomplete permission/queryset logic. This can expose the string representation of related objects a user may not have permission to view (and may bypass any admin/queryset restrictions that autocomplete would enforce). Consider resolving the related object via the related model’s registered ModelAdmin (e.g., model_admin.admin_site._registry[related_model]) and its has_view_permission/get_queryset(request) (or fall back to leaving selected_text empty when not permitted).
| try: | |
| related_model = field.remote_field.model | |
| obj = related_model.objects.get(pk=self.selected_value) | |
| self.selected_text = str(obj) | |
| except (related_model.DoesNotExist, ValueError): | |
| self.selected_value = None | |
| related_model = field.remote_field.model | |
| # Resolve via the related model's ModelAdmin to respect admin permissions | |
| related_admin = model_admin.admin_site._registry.get(related_model) | |
| if related_admin is not None: | |
| qs = related_admin.get_queryset(request) | |
| try: | |
| obj = qs.get(pk=self.selected_value) | |
| except (related_model.DoesNotExist, ValueError): | |
| # Keep behavior consistent: clear value if object does not exist | |
| self.selected_value = None | |
| else: | |
| if related_admin.has_view_permission(request, obj): | |
| self.selected_text = str(obj) | |
| else: | |
| # User is not allowed to view this object; do not expose its string | |
| self.selected_text = None | |
| # If there is no registered ModelAdmin for the related model, | |
| # we do not attempt a direct objects.get(...) to avoid bypassing permissions. |
When the selected object is deleted, keep selected_value so the filter state matches the URL parameter. Use raw ID as fallback text so users can still clear the filter via Select2. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
selected_textfrom Python to template, eliminating unnecessary AJAX callFixes #18
Test plan
pytest- all 4 tests passpre-commithooks - all pass🤖 Generated with Claude Code