Make returning replies from the search API optional

By default only top-level annotations and no replies are returned.
If the undocumented _include_replies: True argument is passed, then
replies are also returned.

h/api/search/core.py

@@ -9,7 +9,7 @@
 log = logging.getLogger(__name__)
 
 
-def search(request, params, private=True):
+def search(request, params, private=True, include_replies=False):
     """
     Search with the given params and return the matching annotations.
 
@@ -23,8 +23,16 @@ def search(request, params, private=True):
         results
     :type private: bool
 
-    :returns: a dict with keys "rows" (the list of matching annotations, as
-        dicts) and "total" (the number of matching annotations, an int)
+    :param include_replies: Whether or not to include a "replies" key in the
+        result containing a list of all replies to the annotations in the
+        "rows" key.
+    :type private: bool
+
+    :returns: A dict with keys:
+      "rows" (the list of matching annotations, as dicts)
+      "total" (the number of matching annotations, an int)
+      "replies": (the list of all replies to the annotations in "rows", if
+        include_replies=True was passed)
     :rtype: dict
     """
     def make_builder():
@@ -44,25 +52,31 @@ def make_builder():
                                            raw_result=True,
                                            authorization_enabled=False)
 
-    # Do a second query for all replies to the annotations from the first
-    # query.
-    builder = make_builder()
-    builder.append_matcher(query.RepliesMatcher(
-        [h['_id'] for h in results['hits']['hits']]))
-    reply_results = models.Annotation.search_raw(builder.build({'limit': 100}),
-                                                 raw_result=True,
-                                                 authorization_enabled=False)
-
-    if len(reply_results['hits']['hits']) < reply_results['hits']['total']:
-        log.warn("The number of reply annotations exceeded the page size of "
-                 "the Elasticsearch query. We currently don't handle this, "
-                 "our search API doesn't support pagination of the reply set.")
-
     total = results['hits']['total']
     docs = results['hits']['hits']
     rows = [models.Annotation(d['_source'], id=d['_id']) for d in docs]
-    reply_docs = reply_results['hits']['hits']
-    reply_rows = [models.Annotation(d['_source'], id=d['_id'])
-                  for d in reply_docs]
+    return_value = {"rows": rows, "total": total}
+
+    if include_replies:
+        # Do a second query for all replies to the annotations from the first
+        # query.
+        builder = make_builder()
+        builder.append_matcher(query.RepliesMatcher(
+            [h['_id'] for h in results['hits']['hits']]))
+        reply_results = models.Annotation.search_raw(
+            builder.build({'limit': 100}), raw_result=True,
+            authorization_enabled=False)
+
+        if len(reply_results['hits']['hits']) < reply_results['hits']['total']:
+            log.warn("The number of reply annotations exceeded the page size "
+                     "of the Elasticsearch query. We currently don't handle "
+                     "this, our search API doesn't support pagination of the "
+                     "reply set.")
+
+        reply_docs = reply_results['hits']['hits']
+        reply_rows = [models.Annotation(d['_source'], id=d['_id'])
+                      for d in reply_docs]
+
+        return_value["replies"] = reply_rows
 
-    return {"rows": rows, "total": total, "replies": reply_rows}
+    return return_value

h/api/search/test/core_test.py

@@ -13,8 +13,7 @@ def test_search_passes_private_to_AuthFilter(query):
 
     core.search(request, mock.Mock(), private=True)
 
-    assert query.AuthFilter.call_args_list == [
-        mock.call(request, private=True), mock.call(request, private=True)]
+    query.AuthFilter.assert_called_once_with(request, private=True)
 
 
 @search_fixtures
@@ -55,7 +54,7 @@ def test_search_queries_for_replies(query, models):
         },
     ]
 
-    core.search(mock.Mock(), mock.Mock())
+    core.search(mock.Mock(), mock.Mock(), include_replies=True)
 
     # It should construct a RepliesMatcher for replies to the annotations from
     # the first search.
@@ -108,7 +107,7 @@ def test_search_returns_replies(models):
         mock.sentinel.reply_annotation_object_3,
     ]
 
-    result = core.search(mock.Mock(), mock.Mock())
+    result = core.search(mock.Mock(), mock.Mock(), include_replies=True)
 
     assert result['replies'] == [
         mock.sentinel.reply_annotation_object_1,
@@ -141,7 +140,7 @@ def test_search_logs_a_warning_if_there_are_too_many_replies(models, log):
         },
     ]
 
-    core.search(mock.Mock(), mock.Mock())
+    core.search(mock.Mock(), mock.Mock(), include_replies=True)
 
     assert log.warn.call_count == 1
 
@@ -171,7 +170,7 @@ def test_search_does_not_log_a_warning_if_there_are_not_too_many_replies(
         },
     ]
 
-    core.search(mock.Mock(), mock.Mock())
+    core.search(mock.Mock(), mock.Mock(), include_replies=True)
 
     assert not log.warn.called
 

h/api/test/views_test.py

@@ -45,7 +45,8 @@ def test_search_searches(search_lib):
 
     views.search(request)
 
-    search_lib.search.assert_called_once_with(request, request.params)
+    search_lib.search.assert_called_once_with(
+        request, request.params, include_replies=False)
 
 
 def test_search_renders_results(search_lib):
@@ -62,7 +63,6 @@ def test_search_renders_results(search_lib):
     assert result == {
         'total': 3,
         'rows': ['A', 'B', 'C'],
-        'replies': []
     }
 
 

h/api/views.py

@@ -92,14 +92,23 @@ def index(context, request):
 @api_config(context=Root, name='search')
 def search(request):
     """Search the database for annotations matching with the given query."""
-    results = search_lib.search(request, request.params)
+    params = request.params.copy()
 
-    return {
+    include_replies = params.pop('_include_replies', False)
+    results = search_lib.search(request, params,
+                                include_replies=include_replies)
+
+    return_value = {
         'total': results['total'],
-        'rows': [search_lib.render(a) for a in results['rows']],
-        'replies': [search_lib.render(a) for a in results['replies']]
+        'rows': [search_lib.render(a) for a in results['rows']]
     }
 
+    if include_replies:
+        return_value['replies'] = [
+            search_lib.render(a) for a in results['replies']]
+
+    return return_value
+
 
 @api_config(route_name='access_token')
 def access_token(request):

h/static/scripts/annotation-viewer-controller.coffee

@@ -22,9 +22,14 @@ module.exports = class AnnotationViewerController
     $scope.search.update = (query) ->
       $location.path('/stream').search('q', query)
 
-    store.SearchResource.get _id: id, ({rows, replies}) ->
+    search_params = {
+      _id: id,
+      _include_replies: true
+    }
+    store.SearchResource.get(search_params, ({rows, replies}) ->
       annotationMapper.loadAnnotations(rows, replies)
       $scope.threadRoot = {children: [threading.idTable[id]]}
+    )
 
     streamFilter
       .setMatchPolicyIncludeAny()

h/static/scripts/stream-controller.coffee

@@ -19,6 +19,7 @@ module.exports = class StreamController
       options = {offset, limit}
       searchParams = searchFilter.toObject($routeParams.q)
       query = angular.extend(options, searchParams)
+      query._include_replies = true
       store.SearchResource.get(query, load)
 
     load = ({rows, replies}) ->

h/static/scripts/test/annotation-viewer-controller-test.coffee

@@ -49,7 +49,7 @@ describe "AnnotationViewerController", ->
     {store} = createAnnotationViewerController({})
     assert store.SearchResource.get.calledOnce
     assert store.SearchResource.get.calledWith(
-      {_id: "test_annotation_id"})
+      {_id: "test_annotation_id", _include_replies: true})
 
   it "passes the annotations and replies from search into loadAnnotations", ->
     {annotationMapper} = createAnnotationViewerController({

h/static/scripts/widget-controller.coffee

@@ -32,6 +32,7 @@ module.exports = class WidgetController
         order: 'asc'
         group: groups.focused().id
       q = angular.extend(queryCore, query)
+      q._include_replies = true
 
       store.SearchResource.get q, (results) ->
         total = results.total