Add section linking for the search result

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

@@ -5,6 +5,7 @@
 var rtddata = require('./rtd-data');
 var xss = require('xss/lib/index');
 var MAX_RESULT_PER_SECTION = 3;
+var MAX_SUBSTRING_LIMIT = 100;
 
 
 /*
@@ -97,7 +98,7 @@ function attach_elastic_search_query(data) {
                                 var section = inner_hits[j];
                                 var section_subtitle = section._source.title;
                                 var section_subtitle_link = link + "#" + section._source.id;
-                                var section_content = [section._source.content.substring(0, 100) + " ..."];
+                                var section_content = [section._source.content.substring(0, MAX_SUBSTRING_LIMIT) + " ..."];
 
                                 if (section.highlight) {
                                     if (section.highlight["sections.title"]) {
@@ -136,6 +137,7 @@ function attach_elastic_search_query(data) {
                                 var domain_subtitle = domain._source.role_name;
                                 var domain_subtitle_link = link + "#" + domain._source.anchor;
                                 var domain_content = "";
+                                var domain_name = domain._source.name;
 
                                 if (
                                     typeof domain._source.display_name === "string" &&
@@ -144,23 +146,15 @@ function attach_elastic_search_query(data) {
                                     domain_subtitle = "(" + domain._source.role_name + ") " + domain._source.display_name;
                                 }
 
-                                // preparing domain_content
-                                // domain_content = type_display --
-                                domain_content = domain._source.type_display + " -- ";
                                 if (domain.highlight) {
                                     if (domain.highlight["domains.name"]) {
                                         // domain_content = type_display -- name
-                                        domain_content += xss(domain.highlight["domains.name"][0]);
-                                    } else {
-                                        // domain_content = type_display -- name
-                                        domain_content += domain._source.name;
+                                        domain_name = xss(domain.highlight["domains.name"][0]);
                                     }
-                                } else {
-                                    // domain_content = type_display -- name
-                                    domain_content += domain._source.name;
                                 }
+
                                 // domain_content = type_display -- name -- in doc_display
-                                domain_content += " -- in " + domain._source.doc_display;
+                                domain_content = domain._source.type_display + " -- " + domain_name + " -- in " + domain._source.doc_display;
 
                                 contents.append(
                                     $u.template(

readthedocs/projects/tasks.py

@@ -1278,14 +1278,25 @@ def fileify(version_pk, commit, build):
         }
     )
     try:
-        _manage_imported_files(version, path, commit, build)
+        changed_files = _create_imported_files(version, path, commit, build)
     except Exception:
+        changed_files = set()
         log.exception('Failed during ImportedFile creation')
 
+    try:
+        _create_intersphinx_data(version, path, commit, build)
+    except Exception:
+        log.exception('Failed during SphinxDomain creation')
+
+    try:
+        _sync_imported_files(version, build, changed_files)
+    except Exception:
+        log.exception('Failed during ImportedFile syncing')
+
 
 def _create_intersphinx_data(version, path, commit, build):
     """
-    Update intersphinx data for this version.
+    Create intersphinx data for this version.
 
     :param version: Version instance
     :param path: Path to search
@@ -1401,14 +1412,16 @@ def clean_build(version_pk):
         return True
 
 
-def _manage_imported_files(version, path, commit, build):
+def _create_imported_files(version, path, commit, build):
     """
-    Update imported files for version.
+    Create imported files for version.
 
     :param version: Version instance
     :param path: Path to search
     :param commit: Commit that updated path
     :param build: Build id
+    :returns: paths of changed files
+    :rtype: set
     """
 
     changed_files = set()
@@ -1458,16 +1471,22 @@ def _manage_imported_files(version, path, commit, build):
                 build=build,
             )
 
-    # create SphinxDomain objects
-    try:
-        _create_intersphinx_data(version, path, commit, build)
-    except Exception:
-        log.exception('Failed during SphinxDomain objects creation')
+    return changed_files
+
+
+def _sync_imported_files(version, build, changed_files):
+    """
+    Sync/Update/Delete ImportedFiles objects of this version.
+
+    :param version: Version instance
+    :param build: Build id
+    :param changed_files: path of changed files
+    """
 
-    # Index new HTMLFiles to elasticsearch
+    # Index new HTMLFiles to ElasticSearch
     index_new_files(model=HTMLFile, version=version, build=build)
 
-    # Remove old HTMLFiles from elasticsearch
+    # Remove old HTMLFiles from ElasticSearch
     remove_indexed_files(
         model=HTMLFile,
         version=version,

readthedocs/rtd_tests/tests/test_imported_file.py

@@ -4,7 +4,7 @@
 from django.test import TestCase
 
 from readthedocs.projects.models import ImportedFile, Project
-from readthedocs.projects.tasks import _manage_imported_files
+from readthedocs.projects.tasks import _create_imported_files, _sync_imported_files
 
 
 base_dir = os.path.dirname(os.path.dirname(__file__))
@@ -16,21 +16,26 @@ class ImportedFileTests(TestCase):
     def setUp(self):
         self.project = Project.objects.get(slug='pip')
         self.version = self.project.versions.first()
+
+    def _manage_imported_files(self, version, path, commit, build):
+        """Helper function for the tests to create and sync ImportedFiles."""
+        _create_imported_files(version, path, commit, build)
+        _sync_imported_files(version, build, set())
 
     def test_properly_created(self):
         test_dir = os.path.join(base_dir, 'files')
         self.assertEqual(ImportedFile.objects.count(), 0)
-        _manage_imported_files(self.version, test_dir, 'commit01', 1)
+        self._manage_imported_files(self.version, test_dir, 'commit01', 1)
         self.assertEqual(ImportedFile.objects.count(), 3)
-        _manage_imported_files(self.version, test_dir, 'commit01', 2)
+        self._manage_imported_files(self.version, test_dir, 'commit01', 2)
         self.assertEqual(ImportedFile.objects.count(), 3)
 
     def test_update_commit(self):
         test_dir = os.path.join(base_dir, 'files')
         self.assertEqual(ImportedFile.objects.count(), 0)
-        _manage_imported_files(self.version, test_dir, 'commit01', 1)
+        self._manage_imported_files(self.version, test_dir, 'commit01', 1)
         self.assertEqual(ImportedFile.objects.first().commit, 'commit01')
-        _manage_imported_files(self.version, test_dir, 'commit02', 2)
+        self._manage_imported_files(self.version, test_dir, 'commit02', 2)
         self.assertEqual(ImportedFile.objects.first().commit, 'commit02')
 
     def test_update_content(self):
@@ -40,13 +45,13 @@ def test_update_content(self):
         with open(os.path.join(test_dir, 'test.html'), 'w+') as f:
             f.write('Woo')
 
-        _manage_imported_files(self.version, test_dir, 'commit01', 1)
+        self._manage_imported_files(self.version, test_dir, 'commit01', 1)
         self.assertEqual(ImportedFile.objects.get(name='test.html').md5, 'c7532f22a052d716f7b2310fb52ad981')
 
         with open(os.path.join(test_dir, 'test.html'), 'w+') as f:
             f.write('Something Else')
 
-        _manage_imported_files(self.version, test_dir, 'commit02', 2)
+        self._manage_imported_files(self.version, test_dir, 'commit02', 2)
         self.assertNotEqual(ImportedFile.objects.get(name='test.html').md5, 'c7532f22a052d716f7b2310fb52ad981')
 
         self.assertEqual(ImportedFile.objects.count(), 3)

readthedocs/rtd_tests/tests/test_search_json_parsing.py

@@ -17,7 +17,9 @@ def test_h2_parsing(self):
                 'files/api.fjson',
             ),
         )
+        self.assertEqual(data['path'], 'api')
         self.assertEqual(data['sections'][1]['id'], 'a-basic-api-client-using-slumber')
-        # Only capture h2's after the first section
-        for obj in data['sections'][1:]:
-            self.assertEqual(obj['content'][:5], '\n<h2>')
+        self.assertTrue(data['sections'][1]['content'].startswith(
+            'You can use Slumber'
+        ))
+        self.assertEqual(data['title'], 'Read the Docs Public API')

readthedocs/search/views.py

@@ -107,36 +107,34 @@ def elastic_search(request, project_slug=None):
     if results:
 
         # sorting inner_hits (if present)
-        try:
-            for result in results:
-
-                inner_hits = result.meta.inner_hits
-                sections = inner_hits.sections or []
-                domains = inner_hits.domains or []
-                all_results = itertools.chain(sections, domains)
-
-                sorted_results = (
-                    {
-                        'type': hit._nested.field,
-
-                        # here _source term is not used because
-                        # django gives error if the names of the
-                        # variables start with underscore
-                        'source': hit._source.to_dict(),
-
-                        'highlight': utils._remove_newlines_from_dict(
-                            hit.highlight.to_dict()
-                        ),
-                    }
-                    for hit in sorted(all_results, key=utils._get_hit_score, reverse=True)
-                )
-
-                result.meta.inner_hits = sorted_results
-
-        except Exception as e:
-            # if the control comes in this block,
-            # that implies that there was a PageSearch
-            pass
+        if user_input.type == 'file':
+
+            try:
+                for result in results:
+                    inner_hits = result.meta.inner_hits
+                    sections = inner_hits.sections or []
+                    domains = inner_hits.domains or []
+                    all_results = itertools.chain(sections, domains)
+
+                    sorted_results = (
+                        {
+                            'type': hit._nested.field,
+
+                            # here _source term is not used because
+                            # django gives error if the names of the
+                            # variables start with underscore
+                            'source': hit._source.to_dict(),
+
+                            'highlight': utils._remove_newlines_from_dict(
+                                hit.highlight.to_dict()
+                            ),
+                        }
+                        for hit in sorted(all_results, key=utils._get_hit_score, reverse=True)
+                    )
+
+                    result.meta.inner_hits = sorted_results
+            except Exception:
+                log.exception('Error while sorting the results (inner_hits).')
 
         log.debug('Search results: %s', pformat(results.to_dict()))
         log.debug('Search facets: %s', pformat(results.facets.to_dict()))

readthedocs/templates/search/elastic_search.html

@@ -25,6 +25,8 @@
 
 {% block content %}
 
+{% trans "100" as MAX_SUBSTRING_LIMIT %}
+
 <div class="navigable">
   <ul>
     <h5>{% trans 'Object Type' %}</h5>
@@ -226,7 +228,7 @@ <h3>
                             {% endfor %}
                           {% endwith %}
                         {% else %}
-                          {{ inner_hit.source.content|slice:"100" }} ...
+                          {{ inner_hit.source.content|slice:MAX_SUBSTRING_LIMIT }} ...
                         {% endif %}
                       {% endif %}
                     {% endfor %}