Reset expanded threads when filter query changes.

As noted in https://github.com/hypothesis/h/pull/3284#issuecomment-219031557
threads which have been explicitly collapsed need to have their
expansion state reset when the filter query changes.

This commit resolves this by moving the search query and sort UI state
into the annotationUI Redux store and handles the state update
in a SET_FILTER_QUERY action.

This means that the rootThread module no longer maintains any primary
state of its own, just state derived from the current annotationUI
state.

h/static/scripts/annotation-ui.js

@@ -49,6 +49,10 @@ function initialState(settings) {
     // Set of IDs of annotations that have been explicitly shown
     // by the user even if they do not match the current search filter
     forceVisible: {},
+
+    filterQuery: null,
+
+    sortMode: 'Location',
   });
 }
 
@@ -61,6 +65,8 @@ var types = {
   ADD_ANNOTATIONS: 'ADD_ANNOTATIONS',
   REMOVE_ANNOTATIONS: 'REMOVE_ANNOTATIONS',
   CLEAR_ANNOTATIONS: 'CLEAR_ANNOTATIONS',
+  SET_FILTER_QUERY: 'SET_FILTER_QUERY',
+  SORT_BY: 'SORT_BY',
 };
 
 function excludeAnnotations(current, annotations) {
@@ -104,6 +110,14 @@ function reducer(state, action) {
       return Object.assign({}, state, {forceVisible: action.forceVisible});
     case types.SET_EXPANDED:
       return Object.assign({}, state, {expanded: action.expanded});
+    case types.SET_FILTER_QUERY:
+      return Object.assign({}, state, {
+        filterQuery: action.query,
+        forceVisible: {},
+        expanded: {},
+      });
+    case types.SORT_BY:
+      return Object.assign({}, state, {sortMode: action.mode});
     default:
       return state;
   }
@@ -204,17 +218,6 @@ module.exports = function (settings) {
       });
     },
 
-    /**
-     * Clear the set of annotations which have been explicitly shown by
-     * setForceVisible()
-     */
-    clearForceVisible: function () {
-      store.dispatch({
-        type: types.SET_FORCE_VISIBLE,
-        forceVisible: {},
-      });
-    },
-
     /**
      * Returns true if the annotation with the given `id` is selected.
      */
@@ -289,5 +292,21 @@ module.exports = function (settings) {
     clearAnnotations: function () {
       store.dispatch({type: types.CLEAR_ANNOTATIONS});
     },
+
+    /** Set the query used to filter displayed annotations. */
+    setFilterQuery: function (query) {
+      store.dispatch({
+        type: types.SET_FILTER_QUERY,
+        query: query,
+      });
+    },
+
+    /** Sets the sort mode for the annotation list. */
+    sortBy: function (mode) {
+      store.dispatch({
+        type: types.SORT_BY,
+        mode: mode,
+      });
+    },
   };
 };

h/static/scripts/build-thread.js

@@ -299,8 +299,7 @@ function buildThread(annotations, opts) {
 
   // Expand any threads which:
   // 1) Have been explicitly expanded OR
-  // 2) Have children matching the filter OR
-  // 3) Contain children which have been selected
+  // 2) Have children matching the filter
   thread = mapThread(thread, function (thread) {
     var id = thread.id;
 

h/static/scripts/root-thread.js

@@ -27,11 +27,12 @@ var sortFns = {
 /**
  * Root conversation thread for the sidebar and stream.
  *
- * Listens for annotations being loaded, created and unloaded and
- * builds a conversation thread.
+ * This performs two functions:
  *
- * The thread is sorted and filtered according to
- * current sort and filter settings.
+ * 1. It listens for annotations being loaded, created and unloaded and
+ *    dispatches annotationUI.{addAnnotations|removeAnnotations} actions.
+ * 2. Listens for changes in the UI state and rebuilds the root conversation
+ *    thread.
  *
  * The root thread is then displayed by viewer.html
  */
@@ -39,24 +40,23 @@ var sortFns = {
 module.exports = function ($rootScope, annotationUI, searchFilter, viewFilter) {
   var thread;
 
-  var sortFn = sortFns.Location;
-  var searchQuery;
-
   /**
    * Rebuild the root conversation thread. This should be called
    * whenever the set of annotations to render or the sort/search/filter
    * settings change.
    */
   function rebuildRootThread() {
+    var sortFn = sortFns[annotationUI.getState().sortMode];
+
     var filters;
-    if (searchQuery) {
-      // TODO - Only regenerate the filter function when the search
-      // query changes
-      filters = searchFilter.generateFacetedFilter(searchQuery);
+    var filterQuery = annotationUI.getState().filterQuery;
+
+    if (filterQuery) {
+      filters = searchFilter.generateFacetedFilter(filterQuery);
     }
 
     var filterFn;
-    if (searchQuery) {
+    if (filterQuery) {
       filterFn = function (annot) {
         return viewFilter.filter([annot], filters).length > 0;
       };
@@ -77,7 +77,10 @@ module.exports = function ($rootScope, annotationUI, searchFilter, viewFilter) {
   annotationUI.subscribe(rebuildRootThread);
 
   // Listen for annotations being created or loaded
-  // and show them in the UI
+  // and show them in the UI.
+  //
+  // Note: These events could all be converted into actions that are handled by
+  // the Redux store in annotationUI.
   var loadEvents = [events.BEFORE_ANNOTATION_CREATED,
                     events.ANNOTATION_CREATED,
                     events.ANNOTATIONS_LOADED];
@@ -123,27 +126,5 @@ module.exports = function ($rootScope, annotationUI, searchFilter, viewFilter) {
     thread: function () {
       return thread;
     },
-
-    /**
-     * Set the sort order for annotations.
-     * @param {'Location'|'Newest'|'Oldest'} mode
-     */
-    sortBy: function (mode) {
-      if (!sortFns[mode]) {
-        throw new Error('Unknown sort mode: ' + mode);
-      }
-      sortFn = sortFns[mode];
-      rebuildRootThread();
-    },
-
-    /**
-     * Set the query to use when filtering annotations.
-     * @param {string} query - The filter query
-     */
-    setSearchQuery: function (query) {
-      searchQuery = query;
-      annotationUI.clearForceVisible();
-      rebuildRootThread();
-    },
   };
 };

h/static/scripts/stream-controller.coffee

@@ -47,7 +47,7 @@ module.exports = class StreamController
     fetch(20)
 
     $scope.$watch('sort.name', (name) ->
-      rootThread.sortBy(name)
+      annotationUI.sortBy(name)
     )
 
     $scope.setCollapsed = (id, collapsed) ->

h/static/scripts/test/annotation-ui-test.js

@@ -78,14 +78,6 @@ describe('annotationUI', function () {
     });
   });
 
-  describe('#clearForceVisible()', function () {
-    it('clears the forceVisible set', function () {
-      annotationUI.setForceVisible('id1', true);
-      annotationUI.clearForceVisible();
-      assert.deepEqual(annotationUI.getState().forceVisible, {});
-    });
-  });
-
   describe('#setCollapsed()', function () {
     it('sets the expanded state of the annotation', function () {
       annotationUI.setCollapsed('parent_id', false);
@@ -211,4 +203,19 @@ describe('annotationUI', function () {
       assert.isNull(annotationUI.getState().selectedAnnotationMap);
     });
   });
+
+  describe('#setFilterQuery()', function () {
+    it('sets the filter query', function () {
+      annotationUI.setFilterQuery('a-query');
+      assert.equal(annotationUI.getState().filterQuery, 'a-query');
+    });
+
+    it('resets the force-visible and expanded sets', function () {
+      annotationUI.setForceVisible('123', true);
+      annotationUI.setCollapsed('456', false);
+      annotationUI.setFilterQuery('some-query');
+      assert.deepEqual(annotationUI.getState().forceVisible, {});
+      assert.deepEqual(annotationUI.getState().expanded, {});
+    });
+  });
 });

h/static/scripts/test/integration/threading-test.js

@@ -63,14 +63,14 @@ describe('annotation threading', function () {
 
   it('should filter annotations when a search is set', function () {
     annotationUI.addAnnotations(fixtures.annotations);
-    rootThread.setSearchQuery('second');
+    annotationUI.setFilterQuery('second');
     assert.equal(rootThread.thread().children.length, 1);
     assert.equal(rootThread.thread().children[0].id, '2');
   });
 
   unroll('should sort annotations by #mode', function (testCase) {
     annotationUI.addAnnotations(fixtures.annotations);
-    rootThread.sortBy(testCase.mode);
+    annotationUI.sortBy(testCase.mode);
     var actualOrder = rootThread.thread().children.map(function (thread) {
       return thread.annotation.id;
     });

h/static/scripts/test/root-thread-test.js

@@ -36,6 +36,8 @@ describe('rootThread', function () {
         selectedAnnotationMap: null,
         expanded: {},
         forceVisible: {},
+        filterQuery: null,
+        sortMode: 'Location',
       },
 
       getState: function () {
@@ -46,7 +48,6 @@ describe('rootThread', function () {
       removeSelectedAnnotation: sinon.stub(),
       addAnnotations: sinon.stub(),
       setCollapsed: sinon.stub(),
-      clearForceVisible: sinon.stub(),
     };
 
     fakeBuildThread = sinon.stub().returns(fixtures.emptyThread);
@@ -146,13 +147,7 @@ describe('rootThread', function () {
     });
   });
 
-  describe('#sortBy', function () {
-    it('rebuilds the thread when the sort order changes', function () {
-      assertRebuildsThread(function () {
-        rootThread.sortBy('Newest');
-      });
-    });
-
+  describe('when the sort order changes', function () {
     function sortBy(annotations, sortCompareFn) {
       return annotations.slice().sort(function (a,b) {
         return sortCompareFn(a,b) ? -1 : sortCompareFn(b,a) ? 1 : 0;
@@ -181,7 +176,10 @@ describe('rootThread', function () {
       }];
 
       fakeBuildThread.reset();
-      rootThread.sortBy(testCase.order);
+      fakeAnnotationUI.state = Object.assign({}, fakeAnnotationUI.state, {
+        sortMode: testCase.order,
+      });
+      rootThread.rebuild();
       var sortCompareFn = fakeBuildThread.args[0][1].sortCompareFn;
       var actualOrder = sortBy(annotations, sortCompareFn).map(function (annot) {
         return annotations.indexOf(annot);
@@ -194,19 +192,15 @@ describe('rootThread', function () {
     ]);
   });
 
-  describe('#setSearchQuery', function () {
-    it('rebuilds the thread when the search query changes', function () {
-      assertRebuildsThread(function () {
-        rootThread.setSearchQuery('new query');
-      });
-    });
-
-    it('generates a thread filter function from the search query', function () {
+  describe('when the filter query changes', function () {
+    it('generates a thread filter function from the query', function () {
       fakeBuildThread.reset();
       var filters = [{any: {terms: ['queryterm']}}];
       var annotation = annotationFixtures.defaultAnnotation();
       fakeSearchFilter.generateFacetedFilter.returns(filters);
-      rootThread.setSearchQuery('queryterm');
+      fakeAnnotationUI.state = Object.assign({}, fakeAnnotationUI.state,
+        {filterQuery: 'queryterm'});
+      rootThread.rebuild();
       var filterFn = fakeBuildThread.args[0][1].filterFn;
 
       fakeViewFilter.filter.returns([annotation]);
@@ -214,11 +208,6 @@ describe('rootThread', function () {
       assert.calledWith(fakeViewFilter.filter, sinon.match([annotation]),
         filters);
     });
-
-    it('clears the set of explicitly shown conversations', function () {
-      rootThread.setSearchQuery('new query');
-      assert.called(fakeAnnotationUI.clearForceVisible);
-    });
   });
 
   context('when annotation events occur', function () {

h/static/scripts/widget-controller.js

@@ -207,10 +207,10 @@ module.exports = function WidgetController(
   // Watch the inputs that determine which annotations are currently
   // visible and how they are sorted and rebuild the thread when they change
   $scope.$watch('sort.name', function (mode) {
-    rootThread.sortBy(mode);
+    annotationUI.sortBy(mode);
   });
   $scope.$watch('search.query', function (query) {
-    rootThread.setSearchQuery(query);
+    annotationUI.setFilterQuery(query);
   });
 
   $scope.rootThread = function () {