Skip to content

Don't error on non existing version #6325

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.

Sign up for GitHub

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 1 commit into from
Nov 6, 2019
Merged

Don't error on non existing version #6325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 6 additions & 3 deletions readthedocs/search/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
from rest_framework.exceptions import ValidationError
from rest_framework.pagination import PageNumberPagination

from readthedocs.projects.models import HTMLFile
from readthedocs.search import tasks, utils
from readthedocs.search.faceted_search import PageSearch
from readthedocs.search import utils, tasks


log = logging.getLogger(__name__)
Expand Down Expand Up @@ -79,9 +80,11 @@ def get_queryset(self):
kwargs['filters']['project'] = [p.slug for p in self.get_all_projects()]
kwargs['filters']['version'] = self.request.query_params.get('version')
if not kwargs['filters']['project']:
raise ValidationError("Unable to find a project to search")
log.info("Unable to find a project to search")
return HTMLFile.objects.none()
if not kwargs['filters']['version']:
raise ValidationError("Unable to find a version to search")
log.info("Unable to find a version to search")
return HTMLFile.objects.none()
Copy link
Member

Choose a reason for hiding this comment

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

Some lines below, we are returning a PageSearch object. Shouldn't we return something similar here instead of a different queryset object?

Copy link
Member Author

Choose a reason for hiding this comment

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

PageSearch isn't an django model, is an elastic search document based on HTMLFile. Actually it's weird becuase the PagaSearch has an interface similar to what django rest framework expects (there is a comment for that in this class or somewhere else). Also, funny that returning an empty PageSearch doesn't work with django rest framework.

From what I saw, we can return real HTMLFile objects, but that requires a query to the db, so I guess that's why we are not doing that and doing this hacky thing.

Copy link
Member

Choose a reason for hiding this comment

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

There's also an open issue related to this: #5235

user = self.request.user
queryset = PageSearch(
query=query, user=user, **kwargs
Expand Down
37 changes: 32 additions & 5 deletions readthedocs/search/tests/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import re
import pytest

import pytest
from django.core.urlresolvers import reverse
from django_dynamic_fixture import G

from readthedocs.builds.models import Version
from readthedocs.projects.models import HTMLFile
from readthedocs.projects.models import HTMLFile, Project
from readthedocs.search.documents import PageDocument
from readthedocs.search.tests.utils import (
get_search_query_from_project_file,
SECTION_FIELDS,
DOMAIN_FIELDS,
SECTION_FIELDS,
get_search_query_from_project_file,
)
from readthedocs.search.documents import PageDocument


@pytest.mark.django_db
Expand Down Expand Up @@ -224,3 +224,30 @@ def test_doc_search_subprojects(self, api_client, all_projects):
# Check the link is the subproject document link
document_link = subproject.get_docs_url(version_slug=version.slug)
assert document_link in first_result['link']

def test_doc_search_unexisting_project(self, api_client):
project = 'notfound'
assert not Project.objects.filter(slug=project).exists()

search_params = {
'q': 'documentation',
'project': project,
'version': 'latest',
}
resp = api_client.get(self.url, search_params)
assert resp.status_code == 404

def test_doc_search_unexisting_version(self, api_client, project):
version = 'notfound'
assert not project.versions.filter(slug=version).exists()

search_params = {
'q': 'documentation',
'project': project.slug,
'version': version,
}
resp = api_client.get(self.url, search_params)
assert resp.status_code == 200

data = resp.data['results']
assert len(data) == 0