Merge pull request #6789 from readthedocs/fix-index-file-htmldir

Check for /index on pages' slug

Author: stsewd

readthedocs/api/v2/views/footer_views.py

@@ -1,5 +1,7 @@
 """Endpoint to generate footer HTML."""
 
+import re
+
 from django.conf import settings
 from django.shortcuts import get_object_or_404
 from django.template import loader as template_loader
@@ -142,13 +144,12 @@ def _get_context(self):
         version = self._get_version()
 
         page_slug = self.request.GET.get('page', '')
+        path = ''
         if page_slug and page_slug != 'index':
-            if main_project.documentation_type == 'sphinx_htmldir':
-                path = page_slug + '/'
+            if version.documentation_type == 'sphinx_htmldir':
+                path = re.sub('/index$', '', page_slug) + '/'
             else:
                 path = page_slug + '.html'
-        else:
-            path = ''
 
         context = {
             'project': project,

readthedocs/config/config.py

@@ -685,8 +685,6 @@ def validate(self):
         self.validate_doc_types()
         self._config['mkdocs'] = self.validate_mkdocs()
         self._config['sphinx'] = self.validate_sphinx()
-        # TODO: remove later
-        self.validate_final_doc_type()
         self._config['submodules'] = self.validate_submodules()
         self.validate_keys()
 
@@ -965,29 +963,6 @@ def validate_sphinx(self):
 
         return sphinx
 
-    def validate_final_doc_type(self):
-        """
-        Validates that the doctype is the same as the admin panel.
-
-        This a temporal validation, as the configuration file should support per
-        version doctype, but we need to adapt the rtd code for that.
-        """
-        dashboard_doctype = self.defaults.get('doctype', 'sphinx')
-        if self.doctype != dashboard_doctype:
-            error_msg = (
-                'Your project is configured as "{}" in your admin dashboard,'
-            ).format(self.builders_display[dashboard_doctype])
-
-            if dashboard_doctype == 'mkdocs' or not self.sphinx:
-                error_msg += ' but there is no "{}" key specified.'.format(
-                    'mkdocs' if dashboard_doctype == 'mkdocs' else 'sphinx',
-                )
-            else:
-                error_msg += ' but your "sphinx.builder" key does not match.'
-
-            key = 'mkdocs' if self.doctype == 'mkdocs' else 'sphinx.builder'
-            self.error(key, error_msg, code=INVALID_KEYS_COMBINATION)
-
     def validate_submodules(self):
         """
         Validates the submodules key.

readthedocs/config/tests/test_config.py

@@ -1511,9 +1511,6 @@ def test_sphinx_builder_default(self):
         build.validate()
         build.sphinx.builder == 'sphinx'
 
-    @pytest.mark.skip(
-        'This test is not compatible with the new validation around doctype.',
-    )
     def test_sphinx_builder_ignores_default(self):
         build = self.get_build_config(
             {},
@@ -1686,28 +1683,6 @@ def test_mkdocs_fail_on_warning_check_default(self):
         build.validate()
         assert build.mkdocs.fail_on_warning is False
 
-    def test_validates_different_filetype_mkdocs(self):
-        build = self.get_build_config(
-            {'mkdocs': {}},
-            {'defaults': {'doctype': 'sphinx'}},
-        )
-        with raises(InvalidConfig) as excinfo:
-            build.validate()
-        assert excinfo.value.key == 'mkdocs'
-        assert 'configured as "Sphinx Html"' in str(excinfo.value)
-        assert 'there is no "sphinx" key' in str(excinfo.value)
-
-    def test_validates_different_filetype_sphinx(self):
-        build = self.get_build_config(
-            {'sphinx': {}},
-            {'defaults': {'doctype': 'sphinx_htmldir'}},
-        )
-        with raises(InvalidConfig) as excinfo:
-            build.validate()
-        assert excinfo.value.key == 'sphinx.builder'
-        assert 'configured as "Sphinx HtmlDir"' in str(excinfo.value)
-        assert 'your "sphinx.builder" key does not match' in str(excinfo.value)
-
     def test_submodule_defaults(self):
         build = self.get_build_config({})
         build.validate()

readthedocs/rtd_tests/tests/test_config_integration.py

@@ -781,9 +781,6 @@ def test_sphinx_builder(
 
         get_builder_class.assert_called_with(result)
 
-    @pytest.mark.skip(
-        'This test is not compatible with the new validation around doctype.',
-    )
     @patch('readthedocs.projects.tasks.get_builder_class')
     def test_sphinx_builder_default(
             self, get_builder_class, checkout_path, tmpdir,

readthedocs/rtd_tests/tests/test_footer.py

@@ -143,6 +143,60 @@ def test_not_show_edit_on_github(self):
         self.assertIn('View', response.data['html'])
         self.assertNotIn('Edit', response.data['html'])
 
+    def test_index_pages_sphinx_htmldir(self):
+        version = self.pip.versions.get(slug=LATEST)
+        version.documentation_type = 'sphinx_htmldir'
+        version.save()
+
+        # A page with slug 'index' should render like /en/latest/
+        self.url = (
+            reverse('footer_html') +
+            f'?project={self.pip.slug}&version={self.latest.slug}&page=index&docroot=/'
+        )
+        response = self.render()
+        self.assertIn('/en/latest/', response.data['html'])
+        self.assertNotIn('/en/latest/index.html', response.data['html'])
+
+        # A page with slug 'foo/index' should render like /en/latest/foo/
+        self.url = (
+            reverse('footer_html') +
+            f'?project={self.pip.slug}&version={self.latest.slug}&page=foo/index&docroot=/'
+        )
+        response = self.render()
+        self.assertIn('/en/latest/foo/', response.data['html'])
+        self.assertNotIn('/en/latest/foo.html', response.data['html'])
+        self.assertNotIn('/en/latest/foo/index.html', response.data['html'])
+
+        # A page with slug 'foo/bar' should render like /en/latest/foo/bar/
+        self.url = (
+            reverse('footer_html') +
+            f'?project={self.pip.slug}&version={self.latest.slug}&page=foo/bar&docroot=/'
+        )
+        response = self.render()
+        self.assertIn('/en/latest/foo/bar/', response.data['html'])
+        self.assertNotIn('/en/latest/foo/bar.html', response.data['html'])
+        self.assertNotIn('/en/latest/foo/bar/index.html', response.data['html'])
+
+        # A page with slug 'foo/bar/index' should render like /en/latest/foo/bar/
+        self.url = (
+            reverse('footer_html') +
+            f'?project={self.pip.slug}&version={self.latest.slug}&page=foo/bar/index&docroot=/'
+        )
+        response = self.render()
+        self.assertIn('/en/latest/foo/bar/', response.data['html'])
+        self.assertNotIn('/en/latest/foo/bar.html', response.data['html'])
+        self.assertNotIn('/en/latest/foo/bar/index.html', response.data['html'])
+
+        # A page with slug 'foo/index/bar' should render like /en/latest/foo/index/bar/
+        self.url = (
+            reverse('footer_html') +
+            f'?project={self.pip.slug}&version={self.latest.slug}&page=foo/index/bar&docroot=/'
+        )
+        response = self.render()
+        self.assertIn('/en/latest/foo/index/bar/', response.data['html'])
+        self.assertNotIn('/en/latest/foo/index/bar.html', response.data['html'])
+        self.assertNotIn('/en/latest/foo/index/bar/index.html', response.data['html'])
+
 
 class TestFooterHTML(BaseTestFooterHTML, TestCase):