readthedocs.org
readthedocs.org copied to clipboard
Standardize error template paths
In trying to rework the templates for proxito, I found it quite confusing
keeping the dashboard and proxito templates separate. This separates proxito
specific errors into a separate path just for these templates, errors/proxito,
instead of using various paths and naming conventions for these files.
This makes it possible for the new dashboard to isolate proxito error templates and to extend from a common proxito error base.
- Refs https://github.com/readthedocs/ext-theme/issues/304
- Requires https://github.com/readthedocs/common/pull/220
I was expecting this PR to fail miserably, but seems it is either somehow okay or we don't have enough test coverage :shrug:
Hrm, seems the last remaining piece is whatever these failures are:
------------------------------ Captured log call -------------------------------
WARNING django.request:log.py:241 I'm a Teapot: /projects/subproject/
____________________ RedirectTests.test_subproject_root_url ____________________
self = <readthedocs.proxito.tests.test_redirects.RedirectTests testMethod=test_subproject_root_url>
def test_subproject_root_url(self):
r = self.client.get(
"/projects/subproject/",
secure=True,
headers={"host": "project.dev.readthedocs.io"},
)
> self.assertEqual(r.status_code, 302)
E AssertionError: 418 != 302
readthedocs/proxito/tests/test_redirects.py:89: AssertionError
I'm a little confused by them right now. I can see why these tests could fail with the 418 status response, but I can't yet explain why.
I don't have the answer here, but it seems those tests are hitting the dummy_dashboard_urls instead of the documentation Proxito urls.
Maybe the order of the definition changed somehow? /projects/subproject/ should be caught by https://github.com/readthedocs/readthedocs.org/blob/f941fc73b912ab885075f51d3ea6e6e4da7e03b9/readthedocs/proxito/urls.py#L144
Yeap, that's the issue. This diff makes that test to pass again:
diff --git a/readthedocs/proxito/urls.py b/readthedocs/proxito/urls.py
index ba465b3bc..2f3d33b8c 100644
--- a/readthedocs/proxito/urls.py
+++ b/readthedocs/proxito/urls.py
@@ -212,9 +212,10 @@ debug_urls = [
groups = [
health_check_urls,
proxied_urls,
- dummy_dashboard_urls,
core_urls,
docs_urls,
+ # Dummy dashboard URLs are required to be defined at the end
+ dummy_dashboard_urls,
]
if settings.SHOW_DEBUG_TOOLBAR:
Thanks for that! I figured it had to be something around the URL changes too, I ended my day before getting into debugging at all though.