Merge pull request #1368 from hypothesis/search-callback-routing-cleanup

Search callback routing cleanup

h/static/scripts/controllers.coffee

@@ -10,6 +10,7 @@ class App
     frame:
       visible: false
     ongoingHighlightSwitch: false
+    search: {}
     sheet: {}
     sorts: [
       'Newest'
@@ -40,8 +41,7 @@ class App
         socialView: annotator.socialView
         ongoingHighlightSwitch: false
         search:
-          query: $location.search()
-          show: not angular.equals($location.search(), {})
+          query: $location.search()['q']
         session: session
 
     _reset()
@@ -105,12 +105,6 @@ class App
             $scope.reloadAnnotations()
             $scope.initUpdater()
 
-    $scope.$watch 'search.show', (visible) ->
-      if visible
-        $timeout ->
-          $element.find('.visual-search').find('input').last().focus()
-        , 10
-
     $scope.$watch 'socialView.name', (newValue, oldValue) ->
       return if newValue is oldValue
       console.log "Social View changed to '" + newValue + "'. Reloading annotations."
@@ -219,41 +213,23 @@ class App
 
     $rootScope.applySort "Location"
 
-    $scope.query = $location.search()
-
-    $scope.search = {}
-    $scope.search.update = angular.noop
-    $scope.search.clear = angular.noop
-
-    #$scope.show_search = Object.keys($scope.query).length > 0
-
     $rootScope.$on '$routeChangeSuccess', (event, next, current) ->
       unless next.$$route? then return
 
-      $scope.search.query = $location.search()
-      $scope.search.show = not angular.equals($location.search(), {})
-
-      if next.$$route.originalPath is '/viewer'
-        $rootScope.viewState.show = true
-      else
-        $rootScope.viewState.show = false
+      $scope.search.query = $location.search()['q']
+      $rootScope.viewState.show = next.$$route.originalPath is '/viewer'
 
       unless next.$$route.originalPath is '/stream'
         if current and next.$$route.originalPath is '/a/:id'
           $scope.reloadAnnotations()
 
-        $scope.search.update = (searchCollection) ->
-          return unless annotator.discardDrafts()
-
-          query = {query: searchCollection}
-          unless angular.equals $location.search(), query
-            if $location.path() == '/viewer' or $location.path() == '/page_search'
-              $location.path('/page_search').search(query)
-            else
-              $location.path('/stream').search(query)
+    $scope.search.clear = ->
+      $location.search('q', null)
 
-        $scope.search.clear = ->
-          $location.url('/viewer')
+    $scope.search.update = (query) ->
+      unless angular.equals $location.search()['q'], query
+        if annotator.discardDrafts()
+          $location.search('q', query or null)
 
     $scope.reloadAnnotations = ->
       Store = plugins.Store
@@ -730,6 +706,9 @@ class Viewer
     $location, $rootScope, $routeParams, $scope,
     annotator
   ) ->
+    if $routeParams.q
+      return $location.path('/page_search').replace()
+
     {providers, threading} = annotator
 
     $scope.activate = (annotation) ->
@@ -756,13 +735,14 @@ class Search
                   'annotator', 'viewFilter']
   constructor: ($filter, $location, $rootScope, $routeParams, $sce, $scope,
                 annotator, viewFilter) ->
+    unless $routeParams.q
+      return $location.path('/viewer').replace()
+
     {providers, threading} = annotator
 
     $scope.highlighter = '<span class="search-hl-active">$&</span>'
     $scope.filter_orderBy = $filter('orderBy')
     $scope.matches = []
-    $scope.search.query = $location.search()
-    $scope.search.show = true
     $scope.render_order = {}
     $scope.render_pos = {}
     $scope.ann_info =
@@ -834,7 +814,8 @@ class Search
       refresh()
 
     refresh = =>
-      [$scope.matches, $scope.filters] = viewFilter.filter $rootScope.annotations, $routeParams
+      query = $routeParams.q
+      [$scope.matches, $scope.filters] = viewFilter.filter $rootScope.annotations, query
       # Create the regexps for highlighting the matches inside the annotations' bodies
       $scope.text_tokens = $scope.filters.text.terms.slice()
       $scope.text_regexp = []

h/static/scripts/directives.coffee

@@ -412,8 +412,8 @@ simpleSearch = ['$parse', ($parse) ->
       _clear(scope) if attr.onclear
 
     scope.$watch attr.query, (query) ->
-      if query.query?
-        scope.searchtext = query.query
+      if query?
+        scope.searchtext = query
         _search(scope, {"this": scope.searchtext})
 
   restrict: 'C'

h/static/scripts/searchfilters.coffee

@@ -162,13 +162,13 @@ class QueryParser
       exact_match: false
       case_sensitive: false
       and_or: 'and'
-    tags:
+    tag:
       path: '/tags'
       exact_match: false
       case_sensitive: false
       and_or: 'or'
     quote:
-      path: "/quote"
+      path: '/quote'
       exact_match: false
       case_sensitive: false
       and_or: 'and'
@@ -212,7 +212,7 @@ class QueryParser
          query_type: 'multi_match'
          match_type: 'cross_fields'
          and_or: 'and'
-         fields:   ['quote', 'tags', 'text', 'uri', 'user']
+         fields:   ['quote', 'tag', 'text', 'uri', 'user']
 
 
   parseModels: (models) ->

h/static/scripts/services.coffee

@@ -736,12 +736,13 @@ class ViewFilter
     matches = true
     # Make copy for filtering
     copy = value.slice()
-    copy.filter (e) ->
-      not match filter.terms, e
 
-    if (filter.operator is 'and' and copy.length < value.length) or
+    copy = copy.filter (e) ->
+      match filter.terms, e
+
+    if (filter.operator is 'and' and copy.length < filter.terms.length) or
     (filter.operator is 'or' and not copy.length)
-        matches = false
+      matches = false
     matches
 
   _anyMatches: (filter, value, match) ->
@@ -788,7 +789,7 @@ class ViewFilter
   #   the faceted filters
   # ]
   filter: (annotations, query) =>
-    filters = @searchfilter.generateFacetedFilter query.query
+    filters = @searchfilter.generateFacetedFilter query
     results = []
 
     # Check for given limit

h/static/scripts/streamsearch.coffee

@@ -11,11 +11,11 @@ imports = [
 
 class StreamSearch
   this.inject = [
-    '$location', '$scope', '$rootScope',
+    '$scope', '$rootScope',
     'queryparser', 'session', 'searchfilter', 'streamfilter'
   ]
   constructor: (
-     $location,   $scope,   $rootScope,
+     $scope,   $rootScope,
      queryparser,   session,   searchfilter,   streamfilter
   ) ->
     # Initialize the base filter
@@ -25,8 +25,7 @@ class StreamSearch
       .setPastDataHits(50)
 
     # Apply query clauses
-    $scope.query = $location.search()['query']
-    terms = searchfilter.generateFacetedFilter $scope.query
+    terms = searchfilter.generateFacetedFilter $scope.search.query
     queryparser.populateFilter streamfilter, terms
 
     $scope.updater?.then (sock) ->
@@ -37,16 +36,6 @@ class StreamSearch
     $rootScope.applyView "Document"  # Non-sensical, but best for the moment
     $rootScope.applySort "Newest"
 
-    $scope.search.query = $location.search()
-    $scope.search.show = not angular.equals($location.search(), {})
-
-    $scope.search.update = (query) ->
-      unless angular.equals $location.search(query), query
-        $location.search {query:query}
-
-    $scope.search.clear = ->
-      $location.search({})
-
     $scope.openDetails = (annotation) ->
     $scope.loadMore = (number) =>
       # TODO: debounce

tests/js/directives-test.coffee

@@ -136,7 +136,6 @@ describe 'h.directives', ->
   describe '.simpleSearch', ->
     $element = null
     beforeEach ->
-      $scope.query = {}
       $scope.update = sinon.spy()
       $scope.clear = sinon.spy()
 
@@ -152,12 +151,12 @@ describe 'h.directives', ->
       $scope.$digest()
 
     it 'updates the search-bar', ->
-      $scope.query = {query: "Test query"}
+      $scope.query = "Test query"
       $scope.$digest()
-      assert.equal($scope.searchtext, $scope.query.query)
+      assert.equal($scope.searchtext, $scope.query)
 
     it 'calls the given search function', ->
-      $scope.query = {query: "Test query"}
+      $scope.query = "Test query"
       $scope.$digest()
       $element.trigger('submit')
       sinon.assert.calledWith($scope.update, "Test query")
@@ -167,7 +166,7 @@ describe 'h.directives', ->
       assert($scope.clear.called)
 
     it 'clears the search-bar', ->
-      $scope.query = {query: "Test query"}
+      $scope.query = "Test query"
       $scope.$digest()
       $element.find('.simple-search-clear').click()
       assert.equal($scope.searchtext, '')
@@ -177,7 +176,7 @@ describe 'h.directives', ->
       assert.include($form.prop('className'), 'simple-search-inactive')
 
     it 'removes the class from the form when there is an input value', ->
-      $scope.query = {query: "Test query"}
+      $scope.query = "Test query"
       $scope.$digest()
 
       $form = $element.find('.simple-search-form')