Skip to content

[Fix #2328 #2013] Refresh search index and test for case insensitive search #4277

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions readthedocs/search/documents.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,14 @@ 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':
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not consider delete case. While a object gets deleted, it calls update with delete action.

# 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,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My fault while fixing lint! 😢

action=action, **kwargs)
50 changes: 39 additions & 11 deletions readthedocs/search/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -67,26 +61,60 @@ def test_search_project_filter_language(self, client, project):

@pytest.mark.django_db
@pytest.mark.search
class TestElasticSearch(object):
class TestPageSearch(object):
url = reverse('search')

def _get_search_result(self, url, client, search_params):
resp = client.get(url, search_params)
assert resp.status_code == 200

page = pq(resp.content)
result = page.find('.module-list-wrapper .module-item-title')
result = page.find('.module-list-wrapper .search-result-item')
return result, page

@pytest.mark.parametrize('data_type', ['content', 'headers', 'title'])
@pytest.mark.parametrize('page_num', [0, 1])
def test_search_by_file_content(self, client, project, data_type, page_num):
def test_file_search(self, client, project, data_type, page_num):
query = get_search_query_from_project_file(project_slug=project.slug, page_num=page_num,
data_type=data_type)

result, _ = self._get_search_result(url=self.url, client=client,
search_params={'q': query, 'type': 'file'})
assert len(result) == 1, ("failed"+ query)
assert len(result) == 1
assert query in result.text()

@pytest.mark.parametrize('case', ['upper', 'lower', 'title'])
def test_file_search_case_insensitive(self, client, project, case):
"""Check File search is case insensitive

It tests with uppercase, lowercase and camelcase
"""
query_text = get_search_query_from_project_file(project_slug=project.slug)

cased_query = getattr(query_text, case)
query = cased_query()

result, _ = self._get_search_result(url=self.url, client=client,
search_params={'q': query, 'type': 'file'})

assert len(result) == 1
# Check the actual text is in the result, not the cased one
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)
# Make a query to check it returns result
result, _ = self._get_search_result(url=self.url, client=client,
search_params={'q': query, 'type': 'file'})
assert len(result) == 1

# Delete all the HTML files of the project
HTMLFile.objects.filter(project=project).delete()
# Run the query again and this time there should not be any result
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"""
Expand Down
4 changes: 2 additions & 2 deletions readthedocs/templates/search/elastic_search.html
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,13 @@ <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>
Expand All @@ -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>
Expand Down