From 4e30b4d6d994cca1625aa9c90ea3b8266ed6d9c4 Mon Sep 17 00:00:00 2001
From: Safwan Rahman <safwan.rahman15@gmail.com>
Date: Sun, 1 Jul 2018 01:59:35 +0600
Subject: [PATCH] adding more tests and fixup

---
 readthedocs/restapi/urls.py          |  2 --
 readthedocs/search/api.py            |  7 +++--
 readthedocs/search/faceted_search.py |  6 ++++-
 readthedocs/search/tests/test_api.py | 38 ++++++++++++++++++++++++++++
 readthedocs/urls.py                  |  3 +++
 5 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/readthedocs/restapi/urls.py b/readthedocs/restapi/urls.py
index 39f9dcd7a58..0a973a39d64 100644
--- a/readthedocs/restapi/urls.py
+++ b/readthedocs/restapi/urls.py
@@ -11,7 +11,6 @@
 from readthedocs.restapi.views import (
     core_views, footer_views, search_views, task_views, integrations
 )
-from readthedocs.search.api import PageSearchAPIView
 
 from .views.model_views import (BuildViewSet, BuildCommandViewSet,
                                 ProjectViewSet, NotificationViewSet,
@@ -56,7 +55,6 @@
     url(r'search/section/$',
         search_views.section_search,
         name='api_section_search'),
-    url(r'^docsearch/$', PageSearchAPIView.as_view(), name='doc_search'),
 ]
 
 task_urls = [
diff --git a/readthedocs/search/api.py b/readthedocs/search/api.py
index 8e72547d195..82f735b7f76 100644
--- a/readthedocs/search/api.py
+++ b/readthedocs/search/api.py
@@ -28,13 +28,12 @@ def get_queryset(self):
         return queryset
 
     def validate_query_params(self):
-        required_query_params = set(['query', 'project', 'version'])
+        required_query_params = {'query', 'project', 'version'}  # python `set` literal is `{}`
         request_params = set(self.request.query_params.keys())
         missing_params = required_query_params - request_params
         if missing_params:
-            errors = []
+            errors = {}
             for param in missing_params:
-                e = {param: "This query param is required"}
-                errors.append(e)
+                errors[param] = ["This query param is required"]
 
             raise ValidationError(errors)
diff --git a/readthedocs/search/faceted_search.py b/readthedocs/search/faceted_search.py
index b483c8c5ea9..8e8eee0fe1a 100644
--- a/readthedocs/search/faceted_search.py
+++ b/readthedocs/search/faceted_search.py
@@ -33,7 +33,11 @@ class FileSearch(RTDFacetedSearch):
     }
 
     def query(self, search, query):
-        """Add query part to ``search``"""
+        """Add query part to ``search``
+
+        Overriding because we pass ES Query object instead of
+        string search.
+        """
         if query:
             search = search.query(query)
 
diff --git a/readthedocs/search/tests/test_api.py b/readthedocs/search/tests/test_api.py
index f02ab7b9e78..ca951b509b5 100644
--- a/readthedocs/search/tests/test_api.py
+++ b/readthedocs/search/tests/test_api.py
@@ -69,6 +69,44 @@ def test_doc_search_filter_by_version(self, api_client, project):
         assert len(data) == 1
         assert data[0]['project'] == project.slug
 
+    def test_doc_search_pagination(self, api_client, project):
+        """Test Doc search result can be paginated"""
+        latest_version = project.versions.all()[0]
+        html_file = HTMLFile.objects.filter(version=latest_version)[0]
+        title = html_file.processed_json['title']
+        query = title.split()[0]
+
+        # Create 15 more same html file
+        for _ in range(15):
+            # Make primary key to None, so django will create new object
+            html_file.pk = None
+            html_file.save()
+
+        search_params = {'query': query, 'project': project.slug, 'version': latest_version.slug}
+        resp = api_client.get(self.url, search_params)
+        assert resp.status_code == 200
+
+        # Check the count is 16 (1 existing and 15 new created)
+        assert resp.data['count'] == 16
+        # Check there are next url
+        assert resp.data['next'] is not None
+        # There should be only 10 data as the pagination is 10 by default
+        assert len(resp.data['results']) == 10
+
+        # Add `page_size` parameter and check the data is paginated accordingly
+        search_params['page_size'] = 5
+        resp = api_client.get(self.url, search_params)
+        assert resp.status_code == 200
+
+        assert len(resp.data['results']) == 5
+
+    def test_doc_search_without_parameters(self, api_client, project):
+        """Hitting Document Search endpoint without query parameters should return error"""
+        resp = api_client.get(self.url)
+        assert resp.status_code == 400
+        # Check error message is there
+        assert sorted(['query', 'project', 'version']) == sorted(resp.data.keys())
+
     def test_doc_search_subprojects(self, api_client, all_projects):
         """Test Document search return results from subprojects also"""
         project = all_projects[0]
diff --git a/readthedocs/urls.py b/readthedocs/urls.py
index d6dbc593fdf..9a50b9b7619 100644
--- a/readthedocs/urls.py
+++ b/readthedocs/urls.py
@@ -22,6 +22,7 @@
     do_not_track,
 )
 from readthedocs.search import views as search_views
+from readthedocs.search.api import PageSearchAPIView
 
 v1_api = Api(api_name='v1')
 v1_api.register(UserResource())
@@ -65,6 +66,8 @@
 api_urls = [
     url(r'^api/', include(v1_api.urls)),
     url(r'^api/v2/', include('readthedocs.restapi.urls')),
+    # Keep the `doc_search` at root level, so the test does not fail for other API
+    url(r'^api/v2/docsearch/$', PageSearchAPIView.as_view(), name='doc_search'),
     url(r'^api-auth/', include('rest_framework.urls', namespace='rest_framework')),
 ]