[fix #2013] Delete search index after file get removed

Author: safwanrahman

readthedocs/search/documents.py

@@ -96,11 +96,13 @@ def update(self, thing, refresh=None, action='index', **kwargs):
         # TODO: remove this overwrite when the issue has been fixed
         # See below link for more information
         # https://github.com/sabricot/django-elasticsearch-dsl/issues/111
-        if isinstance(thing, HTMLFile):
+        # Moreover, do not need to check if its a delete action
+        # Because while delete action, the object is already remove from database
+        if isinstance(thing, HTMLFile) and action != 'delete':
             # Its a model instance.
             queryset = self.get_queryset()
             obj = queryset.filter(pk=thing.pk)
             if not obj.exists():
                 return None
 
-        return super(PageDocument, self).update(thing=thing, refresh=None, action='index', **kwargs)
+        return super(PageDocument, self).update(thing=thing, refresh=refresh, action=action, **kwargs)

readthedocs/search/tests/test_views.py

@@ -1,17 +1,11 @@
-import random
-import string
-
 import pytest
-from django.core.management import call_command
 from django.core.urlresolvers import reverse
 from django_dynamic_fixture import G
-from django_elasticsearch_dsl import Index
 from pyquery import PyQuery as pq
-from pytest_mock import mock
 
 from readthedocs.builds.constants import LATEST
 from readthedocs.builds.models import Version
-from readthedocs.projects.models import Project
+from readthedocs.projects.models import Project, HTMLFile
 from readthedocs.search.tests.utils import get_search_query_from_project_file
 
 
@@ -75,7 +69,7 @@ def _get_search_result(self, url, client, search_params):
         assert resp.status_code == 200
 
         page = pq(resp.content)
-        result = page.find('.module-list-wrapper .module-item')
+        result = page.find('.module-list-wrapper .search-result-item')
         return result, page
 
     @pytest.mark.parametrize('data_type', ['content', 'headers', 'title'])
@@ -95,16 +89,26 @@ def test_file_search_case_insensitive(self, client, project, case):
 
         It tests with uppercase, lowercase and camelcase
         """
-        query = get_search_query_from_project_file(project_slug=project.slug)\
+        query_text = get_search_query_from_project_file(project_slug=project.slug)
 
-        cased_query = getattr(query, case)
+        cased_query = getattr(query_text, case)
+        query = cased_query()
 
         result, _ = self._get_search_result(url=self.url, client=client,
-                                            search_params={'q': cased_query(), 'type': 'file'})
+                                            search_params={'q': query, 'type': 'file'})
 
         assert len(result) == 1
         # Check the actual text is in the result, not the cased one
-        assert query in result.text()
+        assert query_text in result.text()
+
+    def test_page_search_not_return_removed_page(self, client, project):
+        """Check removed page are not in the search index"""
+        query = get_search_query_from_project_file(project_slug=project.slug)
+        # Delete all the HTML files of the project
+        p = HTMLFile.objects.filter(project=project).delete()
+        result, _ = self._get_search_result(url=self.url, client=client,
+                                            search_params={'q': query, 'type': 'file'})
+        assert len(result) == 0
 
     def test_file_search_show_projects(self, client, all_projects):
         """Test that search result page shows list of projects while searching for files"""

readthedocs/templates/search/elastic_search.html

@@ -123,15 +123,16 @@ <h3>{% blocktrans with query=query|default:"" %}Results for {{ query }}{% endblo
 
                 <ul>
                   {% for result in results %}
-                        <li class="module-item">
+                        <li class="module-item search-result-item">
                           <p class="module-item-title">
                             {% if result.name %}
 
                               {# Project #}
                               <a href="{{ result.url }}">{{ result.name }}</a>
+                          </p>
                                 {{ result.meta.description }}
                               {% for fragment in result.meta.highlight.description|slice:":3" %}
-                                <p>
+                                <p class="module-item-highlight">
                                 ...{{ fragment|safe }}...
                                 </p>
                                 <br>
@@ -150,7 +151,6 @@ <h3>{% blocktrans with query=query|default:"" %}Results for {{ query }}{% endblo
                               {# End File #}
 
                             {% endif %}
-                          </p>
                         </li>
                   {% empty %}
                     <li class="module-item"><span class="quiet">{% trans "No results found. Bummer." %}</span></li>