Restructure root-thread building

* Establish a `root-thread` utility that is pure and takes its state
  as a single argument; wrap with `memoize` and export
* Create a memoized rootSelector in the `selection` module that composes
  the state needed by the `root-thead` utility
* The `useRootThread` hook uses the selector and the `root-thread` util

src/sidebar/components/hooks/use-root-thread.js

-import { useService } from '../../util/service-context';
 import useStore from '../../store/use-store';
+import thread from '../../util/root-thread';
+
+/** @typedef {import('../../util/build-thread').Thread} Thread */
 
 /**
  * Gather together state relevant to building a root thread of annotations and
  * replies and return an updated root thread when changes occur.
+ *
+ * @return {Thread}
  */
 export default function useRootThread() {
-  const rootThreadService = useService('rootThread');
-  // Use a to-be-written selector to get relevant selection state, e.g.
-  // filters and forced-visible annotations, etc.
-  // const selectionState = useStore(store => store.getSelectionState());
-  // const route = useStore(store => store.routeName());
-  // const annotations = useStore(store => store.annotations());
-  // return useMemo(() => rootThreadService.buildRootThread(annotations, selectionState, route), [annotations, selectionState, route]);
-  return useStore(store => rootThreadService.thread(store.getState()));
+  return useStore(store => thread(store.threadState()));
 }

src/sidebar/components/test/stream-content-test.js

@@ -18,7 +18,7 @@ describe('StreamContent', () => {
       search: sinon.stub().resolves({ rows: [], replies: [], total: 0 }),
     };
 
-    fakeUseRootThread = sinon.stub();
+    fakeUseRootThread = sinon.stub().returns({});
 
     fakeSearchFilter = {
       toObject: sinon.stub().returns({}),

src/sidebar/store/modules/selection.js

@@ -3,6 +3,10 @@
  * annotations and threads in the UI.
  */
 
+/**
+ * @typedef {import('../../../types/api').Annotation} Annotation
+ */
+
 /**
  * @typedef User
  * @prop {string} [userid]
@@ -31,6 +35,20 @@
  * @prop {User} user
  */
 
+/**
+ * @typedef ThreadState
+ * @prop {Annotation[]} annotations
+ * @prop {Object} selection
+ *   @prop {Object<string,boolean>} selection.expanded
+ *   @prop {string|null} selection.filterQuery
+ *   @prop {Object<string,string>} selection.filters
+ *   @prop {string[]} selection.forcedVisible
+ *   @prop {string[]} selection.selected
+ *   @prop {string} selection.sortKey
+ *   @prop {'annotation'|'note'|'orphan'} selection.selectedTab
+ * @prop {string} route
+ */
+
 import { createSelector } from 'reselect';
 
 import uiConstants from '../../ui-constants';
@@ -173,7 +191,9 @@ const update = {
   },
 
   SET_EXPANDED: function (state, action) {
-    return { expanded: { ...state.expanded, [action.id]: action.expanded } };
+    const newExpanded = { ...state.expanded };
+    newExpanded[action.id] = action.expanded;
+    return { expanded: newExpanded };
   },
 
   SET_FILTER_QUERY: function (state, action) {
@@ -506,6 +526,35 @@ function sortKeys(state) {
   return sortKeysForTab;
 }
 
+/* Selectors that take root state */
+
+/**
+ * Retrieve state needed to calculate the root thread
+ *
+ * @return {ThreadState}
+ */
+const threadState = createSelector(
+  rootState => rootState.annotations.annotations,
+  rootState => rootState.route.name,
+  rootState => rootState.selection,
+  (annotations, routeName, selection) => {
+    const filters = {};
+    if (focusModeActive(selection)) {
+      filters.user = focusModeUserFilter(selection);
+    }
+    const selectionState = {
+      expanded: expandedMap(selection),
+      filterQuery: filterQuery(selection),
+      filters,
+      forcedVisible: forcedVisibleAnnotations(selection),
+      selected: selectedAnnotations(selection),
+      sortKey: selection.sortKey, // TODO: This should have a selector
+      selectedTab: selection.selectedTab, // TODO: This should have a selector
+    };
+    return { annotations, route: routeName, selection: selectionState };
+  }
+);
+
 export default {
   init: init,
   namespace: 'selection',
@@ -539,4 +588,8 @@ export default {
     selectedAnnotations,
     sortKeys,
   },
+
+  rootSelectors: {
+    threadState,
+  },
 };

src/sidebar/store/modules/test/selection-test.js

@@ -2,6 +2,7 @@ import uiConstants from '../../../ui-constants';
 import createStore from '../../create-store';
 import annotations from '../annotations';
 import selection from '../selection';
+import route from '../route';
 import * as fixtures from '../../../test/annotation-fixtures';
 
 describe('sidebar/store/modules/selection', () => {
@@ -13,7 +14,7 @@ describe('sidebar/store/modules/selection', () => {
   };
 
   beforeEach(() => {
-    store = createStore([annotations, selection], fakeSettings);
+    store = createStore([annotations, selection, route], fakeSettings);
   });
 
   describe('getFirstSelectedAnnotationId', function () {
@@ -106,6 +107,52 @@ describe('sidebar/store/modules/selection', () => {
     });
   });
 
+  describe('threadState', () => {
+    it('returns the current annotations in rootState', () => {
+      const myAnnotation = fixtures.defaultAnnotation();
+      store.addAnnotations([myAnnotation]);
+
+      // `addAnnotations` injects some additional properties to annotations,
+      // so we can't compare objects
+      assert.equal(store.threadState().annotations[0].id, myAnnotation.id);
+      assert.lengthOf(store.threadState().annotations, 1);
+    });
+
+    it('returns the current route name from rootState', () => {
+      store.changeRoute('kamchatka');
+
+      assert.equal(store.threadState().route, 'kamchatka');
+    });
+
+    it('returns relevant state from selection', () => {
+      // The order of these matters, as these actions change multiple properties
+      // on the selection state
+      store.selectTab('orphan');
+      store.setSortKey('pyrrhic');
+      store.changeFocusModeUser({
+        username: 'testuser',
+        displayName: 'Test User',
+      });
+      store.setFilterQuery('frappe');
+      // Order doesn't matter past here
+      store.selectAnnotations(['1', '2']);
+      store.setExpanded('3', true);
+      store.setExpanded('4', false);
+      store.setForcedVisible('5', true);
+      store.setForcedVisible('6', false);
+
+      const selection = store.threadState().selection;
+
+      assert.equal(selection.filterQuery, 'frappe');
+      assert.equal(selection.selectedTab, 'orphan');
+      assert.equal(selection.sortKey, 'pyrrhic');
+      assert.deepEqual(selection.selected, ['1', '2']);
+      assert.deepEqual(selection.expanded, { '3': true, '4': false });
+      assert.deepEqual(selection.forcedVisible, ['5']);
+      assert.deepEqual(selection.filters, { user: 'testuser' });
+    });
+  });
+
   describe('selectAnnotations()', function () {
     it('adds the passed annotations to the selectedAnnotations', function () {
       store.selectAnnotations([1, 2, 3]);

src/sidebar/test/integration/threading-test.js

 import { Injector } from '../../../shared/injector';
 
-import rootThreadFactory from '../../services/root-thread';
 import storeFactory from '../../store';
 import immutable from '../../util/immutable';
+import thread from '../../util/root-thread';
 
 const fixtures = immutable({
   annotations: [
@@ -34,7 +34,6 @@ const fixtures = immutable({
 
 describe('annotation threading', function () {
   let store;
-  let rootThread;
 
   beforeEach(function () {
     const fakeFeatures = {
@@ -43,31 +42,29 @@ describe('annotation threading', function () {
 
     const container = new Injector()
       .register('store', storeFactory)
-      .register('rootThread', rootThreadFactory)
       .register('annotationsService', () => {})
       .register('features', { value: fakeFeatures })
       .register('settings', { value: {} });
 
     store = container.get('store');
-    rootThread = container.get('rootThread');
   });
 
   it('should display newly loaded annotations', function () {
     store.addAnnotations(fixtures.annotations);
-    assert.equal(rootThread.thread(store.getState()).children.length, 2);
+    assert.equal(thread(store.threadState()).children.length, 2);
   });
 
   it('should not display unloaded annotations', function () {
     store.addAnnotations(fixtures.annotations);
     store.removeAnnotations(fixtures.annotations);
-    assert.equal(rootThread.thread(store.getState()).children.length, 0);
+    assert.equal(thread(store.threadState()).children.length, 0);
   });
 
   it('should filter annotations when a search is set', function () {
     store.addAnnotations(fixtures.annotations);
     store.setFilterQuery('second');
-    assert.equal(rootThread.thread(store.getState()).children.length, 1);
-    assert.equal(rootThread.thread(store.getState()).children[0].id, '2');
+    assert.equal(thread(store.threadState()).children.length, 1);
+    assert.equal(thread(store.threadState()).children[0].id, '2');
   });
 
   [
@@ -83,11 +80,11 @@ describe('annotation threading', function () {
     it(`should sort annotations by ${testCase.mode}`, () => {
       store.addAnnotations(fixtures.annotations);
       store.setSortKey(testCase.sortKey);
-      const actualOrder = rootThread
-        .thread(store.getState())
-        .children.map(function (thread) {
-          return thread.annotation.id;
-        });
+      const actualOrder = thread(store.threadState()).children.map(function (
+        thread
+      ) {
+        return thread.annotation.id;
+      });
       assert.deepEqual(actualOrder, testCase.expectedOrder);
     });
   });

src/sidebar/util/root-thread.js

+import buildThread from './build-thread';
+import memoize from './memoize';
+import * as metadata from './annotation-metadata';
+import { generateFacetedFilter } from './search-filter';
+import filterAnnotations from './view-filter';
+import { shouldShowInTab } from './tabs';
+
+/** @typedef {import('./build-thread').Thread} Thread */
+/** @typedef {import('./build-thread').Options} BuildThreadOptions */
+/** @typedef {import('../store/modules/selection').ThreadState} ThreadState */
+
+// Sort functions keyed on sort option
+const sortFns = {
+  Newest: function (a, b) {
+    return a.updated > b.updated;
+  },
+  Oldest: function (a, b) {
+    return a.updated < b.updated;
+  },
+  Location: function (a, b) {
+    return metadata.location(a) < metadata.location(b);
+  },
+};
+
+/**
+ * Cobble together the right set of options and filters based on current
+ * `threadState` to build the root thread.
+ *
+ * @param {ThreadState} threadState
+ * @return {Thread}
+ */
+function buildRootThread(threadState) {
+  const selection = threadState.selection;
+
+  /** @type {BuildThreadOptions} */
+  const options = {
+    expanded: selection.expanded,
+    forcedVisible: selection.forcedVisible,
+    selected: selection.selected,
+    sortCompareFn: sortFns[selection.sortKey],
+  };
+
+  // Is there a filter query present, or an applied user (focus) filter?
+  // If so, we'll need to filter the annotations
+  const annotationsFiltered =
+    !!selection.filterQuery || Object.keys(selection.filters).length > 0;
+
+  if (annotationsFiltered) {
+    const filters = generateFacetedFilter(
+      selection.filterQuery,
+      selection.filters
+    );
+    options.filterFn = ann => filterAnnotations([ann], filters).length > 0;
+  }
+
+  // If annotations aren't filtered, should we filter out tab-irrelevant
+  // annotations (e.g. we should only show notes in the `Notes` tab)
+  // in the sidebar?
+  const threadFiltered =
+    !annotationsFiltered && threadState.route === 'sidebar';
+
+  if (threadFiltered) {
+    options.threadFilterFn = thread => {
+      if (!thread.annotation) {
+        return false;
+      }
+      return shouldShowInTab(thread.annotation, selection.selectedTab);
+    };
+  }
+  return buildThread(threadState.annotations, options);
+}
+
+const thread = memoize(buildRootThread);
+
+export default thread;

src/sidebar/util/search-filter.js

@@ -134,7 +134,7 @@ export function toObject(searchText) {
  * Terms that are not associated with a particular facet are stored in the "any"
  * facet.
  *
- * @param {string} searchText - Filter query to parse
+ * @param {string|null} searchText - Filter query to parse
  * @param {FocusFilter} focusFilters - Additional filter terms to mix in
  * @return {Object.<string,Facet>}
  */

src/sidebar/util/test/root-thread-test.js

+import * as annotationFixtures from '../../test/annotation-fixtures';
+import uiConstants from '../../ui-constants';
+import thread from '../root-thread';
+import { $imports } from '../root-thread';
+import immutable from '../immutable';
+
+const fixtures = immutable({
+  emptyThread: {
+    annotation: undefined,
+    children: [],
+  },
+  nonEmptyDraft: {
+    text: 'Some text',
+    tags: [],
+    isPrivate: false,
+  },
+});
+
+describe('sidebar/utils/rootThread', () => {
+  let fakeBuildThread;
+  let fakeFilterAnnotations;
+  let fakeSearchFilter;
+  let fakeThreadState;
+
+  beforeEach(() => {
+    fakeThreadState = {
+      annotations: [],
+      route: 'sidebar',
+      selection: {
+        expanded: {},
+        forcedVisible: [],
+        filters: {},
+        filterQuery: null,
+        selected: [],
+        sortKey: 'Location',
+        selectedTab: uiConstants.TAB_ANNOTATION,
+      },
+    };
+
+    fakeBuildThread = sinon.stub().returns(fixtures.emptyThread);
+    fakeFilterAnnotations = sinon.stub();
+    fakeSearchFilter = {
+      generateFacetedFilter: sinon.stub(),
+    };
+
+    $imports.$mock({
+      './build-thread': fakeBuildThread,
+      './search-filter': fakeSearchFilter,
+      './view-filter': fakeFilterAnnotations,
+    });
+  });
+
+  afterEach(() => {
+    $imports.$restore();
+  });
+
+  describe('#thread', () => {
+    it('returns the result of buildThread', () => {
+      assert.equal(thread(fakeThreadState), fixtures.emptyThread);
+    });
+
+    it('memoizes on `threadState`', () => {
+      fakeBuildThread.onCall(0).returns({ brisket: 'fingers' });
+      fakeBuildThread.onCall(1).returns({ brisket: 'bananas' });
+
+      const thread1 = thread(fakeThreadState);
+      const thread2 = thread(fakeThreadState);
+
+      assert.calledOnce(fakeBuildThread);
+      assert.strictEqual(thread1, thread2);
+
+      fakeThreadState = { ...fakeThreadState };
+
+      const thread3 = thread(fakeThreadState);
+
+      assert.calledTwice(fakeBuildThread);
+      assert.notStrictEqual(thread2, thread3);
+    });
+
+    it('passes annotations to buildThread', () => {
+      const annotation = annotationFixtures.defaultAnnotation();
+      fakeThreadState.annotations = [annotation];
+
+      thread(fakeThreadState);
+      assert.calledWith(fakeBuildThread, sinon.match([annotation]));
+    });
+
+    it('passes on annotation states to buildThread as options', () => {
+      thread(fakeThreadState);
+
+      assert.calledWith(
+        fakeBuildThread,
+        [],
+        sinon.match({
+          expanded: fakeThreadState.selection.expanded,
+          forcedVisible: fakeThreadState.selection.forcedVisible,
+          selected: fakeThreadState.selection.selected,
+        })
+      );
+    });
+
+    describe('when sort order changes', () => {
+      function sortBy(annotations, sortCompareFn) {
+        return annotations.slice().sort((a, b) => {
+          if (sortCompareFn(a, b)) {
+            return -1;
+          }
+          return sortCompareFn(b, a) ? 1 : 0;
+        });
+      }
+
+      // Format TextPositionSelector for the given position `pos`
+      function targetWithPos(pos) {
+        return [
+          {
+            selector: [{ type: 'TextPositionSelector', start: pos }],
+          },
+        ];
+      }
+
+      const annotations = [
+        {
+          target: targetWithPos(1),
+          updated: 20,
+        },
+        {
+          target: targetWithPos(100),
+          updated: 100,
+        },
+        {
+          target: targetWithPos(50),
+          updated: 50,
+        },
+        {
+          target: targetWithPos(20),
+          updated: 10,
+        },
+      ];
+
+      [
+        {
+          order: 'Location',
+          expectedOrder: [0, 3, 2, 1],
+        },
+        {
+          order: 'Oldest',
+          expectedOrder: [3, 0, 2, 1],
+        },
+        {
+          order: 'Newest',
+          expectedOrder: [1, 2, 0, 3],
+        },
+      ].forEach(testCase => {
+        it(`sorts correctly when sorting by ${testCase.order}`, () => {
+          fakeThreadState.selection.sortKey = testCase.order;
+
+          thread(fakeThreadState);
+
+          // The sort compare fn passed to `buildThread`
+          const sortCompareFn = fakeBuildThread.args[0][1].sortCompareFn;
+
+          // Sort the test annotations by the sort compare fn that would be
+          // used by `build-thread` and make sure it's as expected
+          const actualOrder = sortBy(annotations, sortCompareFn).map(annot =>
+            annotations.indexOf(annot)
+          );
+          assert.deepEqual(actualOrder, testCase.expectedOrder);
+        });
+      });
+    });
+
+    describe('annotation and thread filtering', () => {
+      context('sidebar route', () => {
+        [
+          uiConstants.TAB_NOTES,
+          uiConstants.TAB_ANNOTATIONS,
+          uiConstants.TAB_ORPHANS,
+        ].forEach(selectedTab => {
+          it(`should filter the thread for the tab '${selectedTab}'`, () => {
+            const annotations = {
+              [uiConstants.TAB_ANNOTATIONS]: {
+                ...annotationFixtures.defaultAnnotation(),
+                $orphan: false,
+              },
+              [uiConstants.TAB_NOTES]: annotationFixtures.oldPageNote(),
+              [uiConstants.TAB_ORPHANS]: {
+                ...annotationFixtures.defaultAnnotation(),
+                $orphan: true,
+              },
+            };
+            const fakeThreads = [
+              {},
+              { annotation: annotations[uiConstants.TAB_ANNOTATIONS] },
+              { annotation: annotations[uiConstants.TAB_NOTES] },
+              { annotation: annotations[uiConstants.TAB_ORPHANS] },
+            ];
+            fakeThreadState.selection.selectedTab = selectedTab;
+
+            thread(fakeThreadState);
+
+            const threadFilterFn = fakeBuildThread.args[0][1].threadFilterFn;
+            const filteredThreads = fakeThreads.filter(thread =>
+              threadFilterFn(thread)
+            );
+
+            assert.lengthOf(filteredThreads, 1);
+            assert.equal(
+              filteredThreads[0].annotation,
+              annotations[selectedTab]
+            );
+          });
+        });
+
+        it('should not filter the thread if annotations are filtered', () => {
+          fakeThreadState.selection.filterQuery = 'foo';
+
+          thread(fakeThreadState);
+
+          assert.isUndefined(fakeBuildThread.args[0][1].threadFilterFn);
+        });
+
+        it('should not filter the thread if there are applied focus filters', () => {
+          fakeThreadState.selection.filters = { user: 'someusername' };
+
+          thread(fakeThreadState);
+
+          assert.isUndefined(fakeBuildThread.args[0][1].threadFilterFn);
+        });
+      });
+
+      context('other routes', () => {
+        it('should not filter the thread', () => {
+          fakeThreadState.route = 'nonsense';
+
+          thread(fakeThreadState);
+
+          assert.isUndefined(fakeBuildThread.args[0][1].threadFilterFn);
+        });
+      });
+
+      it('should filter annotations if a filter query is set', () => {
+        fakeThreadState.selection.filterQuery = 'anything';
+        const annotation = annotationFixtures.defaultAnnotation();
+        fakeFilterAnnotations.returns([annotation]);
+
+        thread(fakeThreadState);
+
+        const filterFn = fakeBuildThread.args[0][1].filterFn;
+
+        assert.isFunction(filterFn);
+        assert.calledOnce(fakeSearchFilter.generateFacetedFilter);
+        assert.calledWith(
+          fakeSearchFilter.generateFacetedFilter,
+          fakeThreadState.selection.filterQuery,
+          fakeThreadState.selection.filters
+        );
+        assert.isTrue(filterFn(annotation));
+      });
+
+      it('should filter annotations if there is an applied focus filter', () => {
+        fakeThreadState.selection.filters = { user: 'somebody' };
+
+        thread(fakeThreadState);
+
+        assert.isFunction(fakeBuildThread.args[0][1].filterFn);
+        assert.calledWith(
+          fakeSearchFilter.generateFacetedFilter,
+          sinon.match.any,
+          sinon.match({ user: 'somebody' })
+        );
+      });
+    });
+  });
+});