From bbbf47e5261162ddf72e6b02e29e11433b56215f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Apr 2026 21:04:54 +0000 Subject: [PATCH 1/3] redirects: use FilteredModelChoiceFilter for URL filter Mirror the version filter pattern in RedirectListFilterSet so the URL filter is a searchable model choice dropdown instead of a free-text icontains field. Generated by AI. --- readthedocs/projects/filters.py | 21 ++++++++++++--------- readthedocs/redirects/tests/test_views.py | 10 ++++------ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/readthedocs/projects/filters.py b/readthedocs/projects/filters.py index c3495c0beca..9a1587b9b06 100644 --- a/readthedocs/projects/filters.py +++ b/readthedocs/projects/filters.py @@ -4,9 +4,7 @@ from django.db.models import Count from django.db.models import F from django.db.models import Max -from django.db.models import Q from django.utils.translation import gettext_lazy as _ -from django_filters import CharFilter from django_filters import ChoiceFilter from django_filters import OrderingFilter @@ -261,12 +259,17 @@ class RedirectListFilterSet(ModelFilterSet): empty_label=_("All types"), ) - # Matches against either end of the redirect in a single field so the UI - # stays a single input. - url = CharFilter( - label=_("URL contains"), - method="filter_url_contains", + url = FilteredModelChoiceFilter( + label=_("URL"), + empty_label=_("All URLs"), + to_field_name="pk", + queryset_method="get_redirect_queryset", + method="get_redirect", + label_attribute="from_url", ) - def filter_url_contains(self, queryset, field_name, value): - return queryset.filter(Q(from_url__icontains=value) | Q(to_url__icontains=value)) + def get_redirect_queryset(self): + return self.queryset + + def get_redirect(self, queryset, field_name, redirect): + return queryset.filter(pk=redirect.pk) diff --git a/readthedocs/redirects/tests/test_views.py b/readthedocs/redirects/tests/test_views.py index 1e33469a1dc..2b0370815f1 100644 --- a/readthedocs/redirects/tests/test_views.py +++ b/readthedocs/redirects/tests/test_views.py @@ -98,8 +98,8 @@ def test_list_redirect_filter_by_type(self): assert len(resp.context["redirects"]) == 1 assert resp.context["redirects"][0].redirect_type == PAGE_REDIRECT - def test_list_redirect_filter_by_url_contains(self): - get( + def test_list_redirect_filter_by_url(self): + other = get( Redirect, project=self.project, redirect_type=PAGE_REDIRECT, @@ -108,13 +108,11 @@ def test_list_redirect_filter_by_url_contains(self): ) resp = self.client.get( reverse("projects_redirects", args=[self.project.slug]), - {"url": "configuration"}, + {"url": other.pk}, ) assert resp.status_code == 200 pks = {r.pk for r in resp.context["redirects"]} - assert len(pks) == 1 - # Matches on the ``to_url`` substring only. - assert self.redirect.pk not in pks + assert pks == {other.pk} def test_get_redirect(self): resp = self.client.get( From 1989a9c463c30a630a5547b2cc295bb957fc3b17 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Apr 2026 00:59:49 +0000 Subject: [PATCH 2/3] redirects: comment why get_redirect_queryset is needed Generated by AI. --- readthedocs/projects/filters.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/readthedocs/projects/filters.py b/readthedocs/projects/filters.py index 9a1587b9b06..3bdd448c64a 100644 --- a/readthedocs/projects/filters.py +++ b/readthedocs/projects/filters.py @@ -269,6 +269,8 @@ class RedirectListFilterSet(ModelFilterSet): ) def get_redirect_queryset(self): + # Scope choices to the project's redirects passed in at instantiation; + # otherwise the dropdown would show every Redirect across all projects. return self.queryset def get_redirect(self, queryset, field_name, redirect): From 03b4ef3caca191615dbbb3757fddd84c010fc7e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 2 Jun 2026 16:03:41 +0000 Subject: [PATCH 3/3] redirects: filter by from_url instead of pk Addresses review feedback: the filter is labeled "URL" so the URL parameter should also be a URL, not a primary key. Generated by AI. --- readthedocs/projects/filters.py | 7 ++++--- readthedocs/redirects/tests/test_views.py | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/readthedocs/projects/filters.py b/readthedocs/projects/filters.py index 3bdd448c64a..f9a9897eca9 100644 --- a/readthedocs/projects/filters.py +++ b/readthedocs/projects/filters.py @@ -262,7 +262,7 @@ class RedirectListFilterSet(ModelFilterSet): url = FilteredModelChoiceFilter( label=_("URL"), empty_label=_("All URLs"), - to_field_name="pk", + to_field_name="from_url", queryset_method="get_redirect_queryset", method="get_redirect", label_attribute="from_url", @@ -271,7 +271,8 @@ class RedirectListFilterSet(ModelFilterSet): def get_redirect_queryset(self): # Scope choices to the project's redirects passed in at instantiation; # otherwise the dropdown would show every Redirect across all projects. - return self.queryset + # Exclude redirects without a ``from_url`` (eg. clean URL <-> HTML types). + return self.queryset.exclude(from_url="") def get_redirect(self, queryset, field_name, redirect): - return queryset.filter(pk=redirect.pk) + return queryset.filter(from_url=redirect.from_url) diff --git a/readthedocs/redirects/tests/test_views.py b/readthedocs/redirects/tests/test_views.py index 2b0370815f1..a16f8923be3 100644 --- a/readthedocs/redirects/tests/test_views.py +++ b/readthedocs/redirects/tests/test_views.py @@ -108,7 +108,7 @@ def test_list_redirect_filter_by_url(self): ) resp = self.client.get( reverse("projects_redirects", args=[self.project.slug]), - {"url": other.pk}, + {"url": other.from_url}, ) assert resp.status_code == 200 pks = {r.pk for r in resp.context["redirects"]}