-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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': | ||
# 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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My fault while fixing lint! 😢 |
||
action=action, **kwargs) |
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 | ||
|
||
|
||
|
@@ -67,26 +61,54 @@ 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) | ||
# Delete all the HTML files of the project | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should confirm that the query returns a result here, so that we know that then it doesn't return later it's a direct result of the deletion of the file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ericholscher Good point. Fixed it in latest commit |
||
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""" | ||
|
There was a problem hiding this comment.
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
withdelete
action.