Merge pull request #7070 from readthedocs/generate-full-link

Search: generate full link from the server side

Author: stsewd

readthedocs/core/static-src/core/js/doc-embed/search.js

@@ -74,14 +74,7 @@ function attach_elastic_search_query(data) {
                             }
                         }
 
-                        // Creating the result from elements
-                        var suffix = DOCUMENTATION_OPTIONS.FILE_SUFFIX;
-                        // Since sphinx 2.2.1 FILE_SUFFIX is .html for all builders,
-                        // and there is a new BUILDER option.
-                        if ('BUILDER' in DOCUMENTATION_OPTIONS && DOCUMENTATION_OPTIONS.BUILDER === 'readthedocsdirhtml') {
-                          suffix = '';
-                        }
-                        var link = doc.link + suffix + "?highlight=" + $.urlencode(query);
+                        var link = doc.link + "?highlight=" + $.urlencode(query);
 
                         var item = $('<a>', {'href': link});
 

readthedocs/core/static/core/js/readthedocs-doc-embed.js

@@ -1,5 +1,6 @@
 import itertools
 import logging
+import re
 
 from django.shortcuts import get_object_or_404
 from django.utils import timezone
@@ -9,6 +10,7 @@
 
 from readthedocs.api.v2.permissions import IsAuthorizedToViewVersion
 from readthedocs.builds.models import Version
+from readthedocs.projects.constants import MKDOCS, SPHINX_HTMLDIR
 from readthedocs.projects.models import HTMLFile, Project
 from readthedocs.search import tasks, utils
 from readthedocs.search.faceted_search import PageSearch
@@ -27,15 +29,28 @@ class PageSearchSerializer(serializers.Serializer):
     version = serializers.CharField()
     title = serializers.CharField()
     path = serializers.CharField()
+    full_path = serializers.CharField()
     link = serializers.SerializerMethodField()
     highlight = serializers.SerializerMethodField()
     inner_hits = serializers.SerializerMethodField()
 
     def get_link(self, obj):
-        projects_url = self.context.get('projects_url')
-        if projects_url:
-            docs_url = projects_url[obj.project]
-            return docs_url + obj.path
+        project_data = self.context['projects_data'].get(obj.project)
+        if not project_data:
+            return None
+
+        docs_url, doctype = project_data
+        path = obj.full_path
+
+        # Generate an appropriate link for the doctypes that use htmldir,
+        # and always end it with / so it goes directly to proxito.
+        if doctype in {SPHINX_HTMLDIR, MKDOCS}:
+            new_path = re.sub('(^|/)index.html$', '/', path)
+            # docs_url already ends with /,
+            # so path doesn't need to start with /.
+            path = new_path.lstrip('/')
+
+        return docs_url + path
 
     def get_highlight(self, obj):
         highlight = getattr(obj.meta, 'highlight', None)
@@ -160,7 +175,7 @@ def validate_query_params(self):
 
     def get_serializer_context(self):
         context = super().get_serializer_context()
-        context['projects_url'] = self.get_all_projects_url()
+        context['projects_data'] = self.get_all_projects_data()
         return context
 
     def get_all_projects(self):
@@ -188,29 +203,44 @@ def get_all_projects(self):
                 all_projects.append(version.project)
         return all_projects
 
-    def get_all_projects_url(self):
+    def get_all_projects_data(self):
         """
-        Return a dict containing the project slug and its version URL.
-
-        The dictionary contains the project and its subprojects . Each project's
-        slug is used as a key and the documentation URL for that project and
-        version as the value.
+        Return a dict containing the project slug and its version URL and version's doctype.
 
-        Example:
+        The dictionary contains the project and its subprojects. Each project's
+        slug is used as a key and a tuple with the documentation URL and doctype
+        from the version. Example:
 
         {
-            "requests": "https://requests.readthedocs.io/en/latest/",
-            "requests-oauth": "https://requests-oauth.readthedocs.io/en/latest/",
+            "requests": (
+                "https://requests.readthedocs.io/en/latest/",
+                "sphinx",
+            ),
+            "requests-oauth": (
+                "https://requests-oauth.readthedocs.io/en/latest/",
+                "sphinx_htmldir",
+            ),
         }
 
         :rtype: dict
         """
         all_projects = self.get_all_projects()
         version_slug = self._get_version().slug
-        projects_url = {}
+        project_urls = {}
         for project in all_projects:
-            projects_url[project.slug] = project.get_docs_url(version_slug=version_slug)
-        return projects_url
+            project_urls[project.slug] = project.get_docs_url(version_slug=version_slug)
+
+        versions_doctype = (
+            Version.objects
+            .filter(project__slug__in=project_urls.keys(), slug=version_slug)
+            .values_list('project__slug', 'documentation_type')
+        )
+
+        projects_data = {
+            project_slug: (project_urls[project_slug], doctype)
+            for project_slug, doctype in versions_doctype
+        }
+        return projects_data
 
     def list(self, request, *args, **kwargs):
         """Overriding ``list`` method to record query in database."""

readthedocs/search/api.py

@@ -4,7 +4,7 @@
 
 import pytest
 from django.core.management import call_command
-from django_dynamic_fixture import G
+from django_dynamic_fixture import get
 
 from readthedocs.projects.constants import PUBLIC
 from readthedocs.projects.models import HTMLFile, Project
@@ -28,7 +28,7 @@ def all_projects(es_index, mock_processed_json, db, settings):
     settings.ELASTICSEARCH_DSL_AUTOSYNC = True
     projects_list = []
     for project_slug in ALL_PROJECTS:
-        project = G(
+        project = get(
             Project,
             slug=project_slug,
             name=project_slug,
@@ -41,7 +41,13 @@ def all_projects(es_index, mock_processed_json, db, settings):
             # file_basename in config are without extension so add html extension
             file_name = file_basename + '.html'
             version = project.versions.all()[0]
-            html_file = G(HTMLFile, project=project, version=version, name=file_name)
+            html_file = get(
+                HTMLFile,
+                project=project,
+                version=version,
+                name=file_name,
+                path=file_name,
+            )
 
             # creating sphinx domain test objects
             file_path = get_json_file_path(project.slug, file_basename)
@@ -54,7 +60,7 @@ def all_projects(es_index, mock_processed_json, db, settings):
                         domain_role_name = domain_data.pop('role_name')
                         domain, type_ = domain_role_name.split(':')
 
-                        G(
+                        get(
                             SphinxDomain,
                             project=project,
                             version=version,

readthedocs/search/tests/conftest.py

@@ -0,0 +1,13 @@
+{
+    "path": "guides/index",
+    "title": "Guides",
+    "sections": [
+        {
+            "id": "guides",
+            "title": "Guides",
+            "content": "Content from guides/index"
+        }
+    ],
+    "domains": [],
+    "domain_data": {}
+}

readthedocs/search/tests/data/docs/guides/index.json

@@ -0,0 +1,13 @@
+{
+    "path": "index",
+    "title": "Index",
+    "sections": [
+        {
+            "id": "title",
+            "title": "Title",
+            "content": "Some content from index"
+        }
+    ],
+    "domains": [],
+    "domain_data": {}
+}

readthedocs/search/tests/data/docs/index.json

@@ -1,7 +1,7 @@
 PROJECT_DATA_FILES = {
     'pipeline': ['installation', 'signals'],
     'kuma': ['documentation', 'docker'],
-    'docs': ['support', 'wiping'],
+    'docs': ['support', 'wiping', 'index', 'guides/index'],
 }
 
 ALL_PROJECTS = PROJECT_DATA_FILES.keys()

readthedocs/search/tests/dummy_data.py

@@ -6,7 +6,14 @@
 from django_dynamic_fixture import G
 
 from readthedocs.builds.models import Version
-from readthedocs.projects.constants import PUBLIC
+from readthedocs.projects.constants import (
+    MKDOCS,
+    MKDOCS_HTML,
+    PUBLIC,
+    SPHINX,
+    SPHINX_HTMLDIR,
+    SPHINX_SINGLEHTML,
+)
 from readthedocs.projects.models import HTMLFile, Project
 from readthedocs.search.api import PageSearchAPIView
 from readthedocs.search.documents import PageDocument
@@ -324,6 +331,114 @@ def test_doc_search_hidden_versions(self, api_client, all_projects):
         first_result = data[0]
         assert first_result['project'] == subproject.slug
 
+    @pytest.mark.parametrize('doctype', [SPHINX, SPHINX_SINGLEHTML, MKDOCS_HTML])
+    def test_search_correct_link_for_normal_page_html_projects(self, api_client, doctype):
+        project = Project.objects.get(slug='docs')
+        project.versions.update(documentation_type=doctype)
+        version = project.versions.all().first()
+
+        search_params = {
+            'project': project.slug,
+            'version': version.slug,
+            'q': 'Support',
+        }
+        resp = self.get_search(api_client, search_params)
+        assert resp.status_code == 200
+
+        result = resp.data['results'][0]
+        assert result['project'] == project.slug
+        assert result['link'].endswith('en/latest/support.html')
+
+    @pytest.mark.parametrize('doctype', [SPHINX, SPHINX_SINGLEHTML, MKDOCS_HTML])
+    def test_search_correct_link_for_index_page_html_projects(self, api_client, doctype):
+        project = Project.objects.get(slug='docs')
+        project.versions.update(documentation_type=doctype)
+        version = project.versions.all().first()
+
+        search_params = {
+            'project': project.slug,
+            'version': version.slug,
+            'q': 'Some content from index',
+        }
+        resp = self.get_search(api_client, search_params)
+        assert resp.status_code == 200
+
+        result = resp.data['results'][0]
+        assert result['project'] == project.slug
+        assert result['link'].endswith('en/latest/index.html')
+
+    @pytest.mark.parametrize('doctype', [SPHINX, SPHINX_SINGLEHTML, MKDOCS_HTML])
+    def test_search_correct_link_for_index_page_subdirectory_html_projects(self, api_client, doctype):
+        project = Project.objects.get(slug='docs')
+        project.versions.update(documentation_type=doctype)
+        version = project.versions.all().first()
+
+        search_params = {
+            'project': project.slug,
+            'version': version.slug,
+            'q': 'Some content from guides/index',
+        }
+        resp = self.get_search(api_client, search_params)
+        assert resp.status_code == 200
+
+        result = resp.data['results'][0]
+        assert result['project'] == project.slug
+        assert result['link'].endswith('en/latest/guides/index.html')
+
+    @pytest.mark.parametrize('doctype', [SPHINX_HTMLDIR, MKDOCS])
+    def test_search_correct_link_for_normal_page_htmldir_projects(self, api_client, doctype):
+        project = Project.objects.get(slug='docs')
+        project.versions.update(documentation_type=doctype)
+        version = project.versions.all().first()
+
+        search_params = {
+            'project': project.slug,
+            'version': version.slug,
+            'q': 'Support',
+        }
+        resp = self.get_search(api_client, search_params)
+        assert resp.status_code == 200
+
+        result = resp.data['results'][0]
+        assert result['project'] == project.slug
+        assert result['link'].endswith('en/latest/support.html')
+
+    @pytest.mark.parametrize('doctype', [SPHINX_HTMLDIR, MKDOCS])
+    def test_search_correct_link_for_index_page_htmldir_projects(self, api_client, doctype):
+        project = Project.objects.get(slug='docs')
+        project.versions.update(documentation_type=doctype)
+        version = project.versions.all().first()
+
+        search_params = {
+            'project': project.slug,
+            'version': version.slug,
+            'q': 'Some content from index',
+        }
+        resp = self.get_search(api_client, search_params)
+        assert resp.status_code == 200
+
+        result = resp.data['results'][0]
+        assert result['project'] == project.slug
+        assert result['link'].endswith('en/latest/')
+
+    @pytest.mark.parametrize('doctype', [SPHINX_HTMLDIR, MKDOCS])
+    def test_search_correct_link_for_index_page_subdirectory_htmldir_projects(self, api_client, doctype):
+        project = Project.objects.get(slug='docs')
+        project.versions.update(documentation_type=doctype)
+        version = project.versions.all().first()
+
+        search_params = {
+            'project': project.slug,
+            'version': version.slug,
+            'q': 'Some content from guides/index',
+        }
+        resp = self.get_search(api_client, search_params)
+        assert resp.status_code == 200
+
+        result = resp.data['results'][0]
+        assert result['project'] == project.slug
+        assert result['link'].endswith('en/latest/guides/')
+
 
 class TestDocumentSearch(BaseTestDocumentSearch):