From 0e93adb79adbc623991a0e4b3375ddcb18907c00 Mon Sep 17 00:00:00 2001 From: Eric Holscher <25510+ericholscher@users.noreply.github.com> Date: Tue, 2 Dec 2025 21:05:04 +0100 Subject: [PATCH 1/2] Fix ruff lint warning for ServePageRedirect --- docs/design/subproject-page-redirect.md | 30 +++++++++++++++++++ .../proxito/tests/test_old_redirects.py | 11 +++++++ readthedocs/proxito/views/serve.py | 16 ++++++++-- 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 docs/design/subproject-page-redirect.md diff --git a/docs/design/subproject-page-redirect.md b/docs/design/subproject-page-redirect.md new file mode 100644 index 00000000000..a16ecd45b53 --- /dev/null +++ b/docs/design/subproject-page-redirect.md @@ -0,0 +1,30 @@ +# Spike: /page/ redirects for subprojects + +## Background and problem + +Users expect `/page/` URLs on subprojects to redirect to the default version, the same way they do for top-level projects. Today those requests return 404 because the redirect view only looks up subprojects by alias on the parent project. When the current request context already belongs to the subproject (for example, after middleware resolves the host to the child project) or when the stored alias differs from the slug, that lookup fails and the redirect short-circuits. + +## Findings + +* The proxito URL configuration already routes `/projects//page/` to `ServePageRedirect`, so the failure happens in the view rather than URL parsing. +* `ServePageRedirect` fetched subprojects exclusively via `project.subprojects.filter(alias=...)`, which raises `Http404` when the request context is already scoped to the subproject or when an alias mismatch occurs. +* The redirect logic ultimately just needs the correct `Project` instance to resolve the default version and build the target URL; it should tolerate being invoked from either the parent or child context. + +## Proposed approach + +* Expand subproject resolution in `ServePageRedirect` to handle three cases: + * If the current project already matches the requested slug, treat it as the target subproject. + * Otherwise, try to find a `ProjectRelationship` by alias and fall back to matching the child slug. + * Raise `Http404` only when no relationship matches, preserving existing behavior for invalid slugs. +* Keep the rest of the redirect flow unchanged so the spike stays low risk while we validate that subproject `/page/` redirects behave like top-level projects. + +## Validation plan + +* Add a focused proxito test that requests `/projects//page/` on the public domain and asserts a 302 to the subproject's default version. +* Run the proxito redirect tests locally to confirm the new lookup path fixes the 404 without affecting existing redirects. + Use the proxito test settings so the proxito URLConf and middleware are active: + + ``` + DJANGO_SETTINGS_MODULE=readthedocs.settings.proxito.test \ + uv run pytest readthedocs/proxito/tests/test_old_redirects.py::InternalRedirectTests::test_page_redirect_on_subproject -vv -s + ``` diff --git a/readthedocs/proxito/tests/test_old_redirects.py b/readthedocs/proxito/tests/test_old_redirects.py index 52809377162..706701d0acf 100644 --- a/readthedocs/proxito/tests/test_old_redirects.py +++ b/readthedocs/proxito/tests/test_old_redirects.py @@ -84,6 +84,17 @@ def test_page_redirect_with_query_params(self): "http://project.dev.readthedocs.io/en/latest/test.html?foo=bar", ) + def test_page_redirect_on_subproject(self): + r = self.client.get( + "/projects/subproject/page/test.html", + headers={"host": "project.dev.readthedocs.io"}, + ) + self.assertEqual(r.status_code, 302) + self.assertEqual( + r["Location"], + "http://project.dev.readthedocs.io/projects/subproject/en/latest/test.html", + ) + def test_url_with_nonexistent_slug(self): # Invalid URL for a not single version project r = self.client.get( diff --git a/readthedocs/proxito/views/serve.py b/readthedocs/proxito/views/serve.py index 085b714d082..972219ba325 100644 --- a/readthedocs/proxito/views/serve.py +++ b/readthedocs/proxito/views/serve.py @@ -8,7 +8,6 @@ from django.http import Http404 from django.http import HttpResponse from django.http import HttpResponseRedirect -from django.shortcuts import get_object_or_404 from django.shortcuts import render from django.views import View @@ -67,7 +66,20 @@ def get(self, request, subproject_slug=None, filename=""): # Use the project from the domain, or use the subproject slug. if subproject_slug: - project = get_object_or_404(project.subprojects, alias=subproject_slug).child + if project.slug == subproject_slug: + subproject = project + else: + relationship = project.subprojects.filter(alias=subproject_slug).first() + + if not relationship: + relationship = project.subprojects.filter(child__slug=subproject_slug).first() + + if not relationship: + raise Http404 + + subproject = relationship.child + + project = subproject # Get the default version from the current project, # or the version from the external domain. From 58191a481a8392fea36d48da149a21afa00dad55 Mon Sep 17 00:00:00 2001 From: Eric Holscher <25510+ericholscher@users.noreply.github.com> Date: Tue, 2 Dec 2025 21:12:42 +0100 Subject: [PATCH 2/2] Add subproject alias page redirect test --- readthedocs/proxito/tests/test_old_redirects.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/readthedocs/proxito/tests/test_old_redirects.py b/readthedocs/proxito/tests/test_old_redirects.py index 706701d0acf..2a45b3b3ccc 100644 --- a/readthedocs/proxito/tests/test_old_redirects.py +++ b/readthedocs/proxito/tests/test_old_redirects.py @@ -95,6 +95,17 @@ def test_page_redirect_on_subproject(self): "http://project.dev.readthedocs.io/projects/subproject/en/latest/test.html", ) + def test_page_redirect_on_subproject_alias(self): + r = self.client.get( + "/projects/this-is-an-alias/page/test.html", + headers={"host": "project.dev.readthedocs.io"}, + ) + self.assertEqual(r.status_code, 302) + self.assertEqual( + r["Location"], + "http://project.dev.readthedocs.io/projects/this-is-an-alias/en/latest/test.html", + ) + def test_url_with_nonexistent_slug(self): # Invalid URL for a not single version project r = self.client.get(