Merge pull request #6888 from readthedocs/fix-subproject-no-slash

Fix URLs like `/projects/subproject` from 404ing when they don't end with a slash

Author: ericholscher

readthedocs/proxito/tests/test_redirects.py

@@ -12,6 +12,13 @@
 )
 class RedirectTests(BaseDocServing):
 
+    def test_root_url_no_slash(self):
+        r = self.client.get('', HTTP_HOST='project.dev.readthedocs.io')
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], 'https://project.dev.readthedocs.io/en/latest/',
+        )
+
     def test_root_url(self):
         r = self.client.get('/', HTTP_HOST='project.dev.readthedocs.io')
         self.assertEqual(r.status_code, 302)
@@ -26,6 +33,22 @@ def test_subproject_root_url(self):
             r['Location'], 'https://project.dev.readthedocs.io/projects/subproject/en/latest/',
         )
 
+    def test_subproject_root_url_no_slash(self):
+        r = self.client.get('/projects/subproject', HTTP_HOST='project.dev.readthedocs.io')
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], 'https://project.dev.readthedocs.io/projects/subproject/en/latest/',
+        )
+
+    def test_single_version_subproject_root_url_no_slash(self):
+        self.subproject.single_version = True
+        self.subproject.save()
+        r = self.client.get('/projects/subproject', HTTP_HOST='project.dev.readthedocs.io')
+        self.assertEqual(r.status_code, 302)
+        self.assertEqual(
+            r['Location'], 'https://project.dev.readthedocs.io/projects/subproject/',
+        )
+
     def test_root_redirect_with_query_params(self):
         r = self.client.get('/?foo=bar', HTTP_HOST='project.dev.readthedocs.io')
         self.assertEqual(r.status_code, 302)

readthedocs/proxito/tests/test_urls.py

@@ -17,6 +17,7 @@ def test_root(self):
         self.assertEqual(
             match.kwargs, {
                 'subproject_slug': None,
+                'subproject_slash': None,
                 'filename': '',
             },
         )
@@ -80,6 +81,7 @@ def test_subproject_single_version(self):
         self.assertEqual(
             match.kwargs, {
                 'subproject_slug': 'bar',
+                'subproject_slash': '/',
                 'filename': 'index.html',
             },
         )
@@ -91,6 +93,7 @@ def test_subproject_root(self):
         self.assertEqual(
             match.kwargs, {
                 'subproject_slug': 'bar',
+                'subproject_slash': '/',
                 'filename': '',
             },
         )
@@ -101,6 +104,7 @@ def test_single_version(self):
         self.assertEqual(match.args, ())
         self.assertEqual(
             match.kwargs, {
+                'subproject_slash': None,
                 'subproject_slug': None,
                 'filename': 'some/path/index.html',
             },

readthedocs/proxito/urls.py

@@ -147,7 +147,9 @@
     # (Sub)project single version
     url(
         (
-            r'^(?:projects/(?P<subproject_slug>{project_slug})/)?'
+            # subproject_slash variable at the end of this regex is for ``/projects/subproject``
+            # so that it will get captured here and redirect properly.
+            r'^(?:projects/(?P<subproject_slug>{project_slug})(?P<subproject_slash>/?))?'
             r'(?P<filename>{filename_slug})$'.format(**pattern_opts)
         ),
         ServeDocs.as_view(),

readthedocs/proxito/views/serve.py

@@ -55,11 +55,17 @@ def get(self,
             request,
             project_slug=None,
             subproject_slug=None,
+            subproject_slash=None,
             lang_slug=None,
             version_slug=None,
             filename='',
     ):  # noqa
-        """Take the incoming parsed URL's and figure out what file to serve."""
+        """
+        Take the incoming parsed URL's and figure out what file to serve.
+
+        ``subproject_slash`` is used to determine if the subproject URL has a slash,
+        so that we can decide if we need to serve docs or add a /.
+        """
 
         version_slug = self.get_version_from_host(request, version_slug)
         final_project, lang_slug, version_slug, filename = _get_project_data_from_request(  # noqa
@@ -87,6 +93,14 @@ def get(self,
         ]):
             return self.system_redirect(request, final_project, lang_slug, version_slug, filename)
 
+        # Handle `/projects/subproject` URL redirection
+        if all([
+                final_project.single_version,
+                filename == '',
+                not subproject_slash,
+        ]):
+            return self.system_redirect(request, final_project, lang_slug, version_slug, filename)
+
         if all([
                 (lang_slug is None or version_slug is None),
                 not final_project.single_version,