Merge pull request #1145 from hypothesis/move-groupid-from-selection-to-store

Move firstKey function from sidebar-content to store

src/sidebar/components/sidebar-content.js

@@ -7,24 +7,13 @@ const memoize = require('../util/memoize');
 const tabs = require('../tabs');
 const uiConstants = require('../ui-constants');
 
-function firstKey(object) {
-  for (const k in object) {
-    if (!object.hasOwnProperty(k)) {
-      continue;
-    }
-    return k;
-  }
-  return null;
-}
-
 /**
  * Returns the group ID of the first annotation in `results` whose
- * ID is a key in `selection`.
+ * ID is `annId`.
  */
-function groupIDFromSelection(selection, results) {
-  const id = firstKey(selection);
+function getGroupID(annId, results) {
   const annot = results.find(function(annot) {
-    return annot.id === id;
+    return annot.id === annId;
   });
   if (!annot) {
     return null;
@@ -123,10 +112,7 @@ function SidebarContentController(
       if (store.hasSelectedAnnotations()) {
         // Focus the group containing the selected annotation and filter
         // annotations to those from this group
-        let groupID = groupIDFromSelection(
-          store.getState().selectedAnnotationMap,
-          results
-        );
+        let groupID = getGroupID(store.getFirstSelectedAnnotationId(), results);
         if (!groupID) {
           // If the selected annotation is not available, fall back to
           // loading annotations for the currently focused group
@@ -316,7 +302,7 @@ function SidebarContentController(
   };
 
   this.selectedAnnotationUnavailable = function() {
-    const selectedID = firstKey(store.getState().selectedAnnotationMap);
+    const selectedID = store.getFirstSelectedAnnotationId();
     return (
       !this.isLoading() && !!selectedID && !store.annotationExists(selectedID)
     );
@@ -343,7 +329,7 @@ function SidebarContentController(
     // The user is logged out and has landed on a direct linked
     // annotation. If there is an annotation selection and that
     // selection is available to the user, show the CTA.
-    const selectedID = firstKey(store.getState().selectedAnnotationMap);
+    const selectedID = store.getFirstSelectedAnnotationId();
     return (
       !this.isLoading() && !!selectedID && store.annotationExists(selectedID)
     );

src/sidebar/store/modules/selection.js

@@ -10,6 +10,7 @@
 
 'use strict';
 
+const { createSelector } = require('reselect');
 const immutable = require('seamless-immutable');
 
 const toSet = require('../../util/array-util').toSet;
@@ -314,6 +315,16 @@ function clearSelectedAnnotations() {
   return { type: actions.CLEAR_SELECTION };
 }
 
+/**
+ * Returns the annotation ID of the first annotation in `selectedAnnotationMap`.
+ *
+ * @return {string|null}
+ */
+const getFirstSelectedAnnotationId = createSelector(
+  state => state.selectedAnnotationMap,
+  selected => (selected ? Object.keys(selected)[0] : null)
+);
+
 module.exports = {
   init: init,
   update: update,
@@ -335,5 +346,6 @@ module.exports = {
   selectors: {
     hasSelectedAnnotations,
     isAnnotationSelected,
+    getFirstSelectedAnnotationId,
   },
 };

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

+'use strict';
+
+const createStore = require('../../create-store');
+const selection = require('../selection');
+const uiConstants = require('../../../ui-constants');
+
+describe('selection', () => {
+  let store;
+  let fakeSettings = {};
+
+  beforeEach(() => {
+    store = createStore([selection], [fakeSettings]);
+  });
+
+  describe('getFirstSelectedAnnotationId', function() {
+    it('returns the first selected annotation id it finds', function() {
+      store.selectAnnotations([1, 2]);
+      assert.equal(store.getFirstSelectedAnnotationId(), 1);
+    });
+
+    it('returns null if no selected annotation ids are found', function() {
+      store.selectAnnotations([]);
+      assert.isNull(store.getFirstSelectedAnnotationId());
+    });
+  });
+
+  describe('setForceVisible()', function() {
+    it('sets the visibility of the annotation', function() {
+      store.setForceVisible('id1', true);
+      assert.deepEqual(store.getState().forceVisible, { id1: true });
+    });
+  });
+
+  describe('setCollapsed()', function() {
+    it('sets the expanded state of the annotation', function() {
+      store.setCollapsed('parent_id', false);
+      assert.deepEqual(store.getState().expanded, { parent_id: true });
+    });
+  });
+
+  describe('focusAnnotations()', function() {
+    it('adds the passed annotations to the focusedAnnotationMap', function() {
+      store.focusAnnotations([1, 2, 3]);
+      assert.deepEqual(store.getState().focusedAnnotationMap, {
+        1: true,
+        2: true,
+        3: true,
+      });
+    });
+
+    it('replaces any annotations originally in the map', function() {
+      store.focusAnnotations([1]);
+      store.focusAnnotations([2, 3]);
+      assert.deepEqual(store.getState().focusedAnnotationMap, {
+        2: true,
+        3: true,
+      });
+    });
+
+    it('nulls the map if no annotations are focused', function() {
+      store.focusAnnotations([1]);
+      store.focusAnnotations([]);
+      assert.isNull(store.getState().focusedAnnotationMap);
+    });
+  });
+
+  describe('hasSelectedAnnotations', function() {
+    it('returns true if there are any selected annotations', function() {
+      store.selectAnnotations([1]);
+      assert.isTrue(store.hasSelectedAnnotations());
+    });
+
+    it('returns false if there are no selected annotations', function() {
+      assert.isFalse(store.hasSelectedAnnotations());
+    });
+  });
+
+  describe('isAnnotationSelected', function() {
+    it('returns true if the id provided is selected', function() {
+      store.selectAnnotations([1]);
+      assert.isTrue(store.isAnnotationSelected(1));
+    });
+
+    it('returns false if the id provided is not selected', function() {
+      store.selectAnnotations([1]);
+      assert.isFalse(store.isAnnotationSelected(2));
+    });
+
+    it('returns false if there are no selected annotations', function() {
+      assert.isFalse(store.isAnnotationSelected(1));
+    });
+  });
+
+  describe('selectAnnotations()', function() {
+    it('adds the passed annotations to the selectedAnnotationMap', function() {
+      store.selectAnnotations([1, 2, 3]);
+      assert.deepEqual(store.getState().selectedAnnotationMap, {
+        1: true,
+        2: true,
+        3: true,
+      });
+    });
+
+    it('replaces any annotations originally in the map', function() {
+      store.selectAnnotations([1]);
+      store.selectAnnotations([2, 3]);
+      assert.deepEqual(store.getState().selectedAnnotationMap, {
+        2: true,
+        3: true,
+      });
+    });
+
+    it('nulls the map if no annotations are selected', function() {
+      store.selectAnnotations([1]);
+      store.selectAnnotations([]);
+      assert.isNull(store.getState().selectedAnnotationMap);
+    });
+  });
+
+  describe('toggleSelectedAnnotations()', function() {
+    it('adds annotations missing from the selectedAnnotationMap', function() {
+      store.selectAnnotations([1, 2]);
+      store.toggleSelectedAnnotations([3, 4]);
+      assert.deepEqual(store.getState().selectedAnnotationMap, {
+        1: true,
+        2: true,
+        3: true,
+        4: true,
+      });
+    });
+
+    it('removes annotations already in the selectedAnnotationMap', function() {
+      store.selectAnnotations([1, 3]);
+      store.toggleSelectedAnnotations([1, 2]);
+      assert.deepEqual(store.getState().selectedAnnotationMap, {
+        2: true,
+        3: true,
+      });
+    });
+
+    it('nulls the map if no annotations are selected', function() {
+      store.selectAnnotations([1]);
+      store.toggleSelectedAnnotations([1]);
+      assert.isNull(store.getState().selectedAnnotationMap);
+    });
+  });
+
+  describe('removeSelectedAnnotation()', function() {
+    it('removes an annotation from the selectedAnnotationMap', function() {
+      store.selectAnnotations([1, 2, 3]);
+      store.removeSelectedAnnotation(2);
+      assert.deepEqual(store.getState().selectedAnnotationMap, {
+        1: true,
+        3: true,
+      });
+    });
+
+    it('nulls the map if no annotations are selected', function() {
+      store.selectAnnotations([1]);
+      store.removeSelectedAnnotation(1);
+      assert.isNull(store.getState().selectedAnnotationMap);
+    });
+  });
+
+  describe('clearSelectedAnnotations()', function() {
+    it('removes all annotations from the selection', function() {
+      store.selectAnnotations([1]);
+      store.clearSelectedAnnotations();
+      assert.isNull(store.getState().selectedAnnotationMap);
+    });
+
+    it('clears the current search query', function() {
+      store.setFilterQuery('foo');
+      store.clearSelectedAnnotations();
+      assert.isNull(store.getState().filterQuery);
+    });
+  });
+
+  describe('setFilterQuery()', function() {
+    it('sets the filter query', function() {
+      store.setFilterQuery('a-query');
+      assert.equal(store.getState().filterQuery, 'a-query');
+    });
+
+    it('resets the force-visible and expanded sets', function() {
+      store.setForceVisible('123', true);
+      store.setCollapsed('456', false);
+      store.setFilterQuery('some-query');
+      assert.deepEqual(store.getState().forceVisible, {});
+      assert.deepEqual(store.getState().expanded, {});
+    });
+  });
+
+  describe('highlightAnnotations()', function() {
+    it('sets the highlighted annotations', function() {
+      store.highlightAnnotations(['id1', 'id2']);
+      assert.deepEqual(store.getState().highlighted, ['id1', 'id2']);
+    });
+  });
+
+  describe('selectTab()', function() {
+    it('sets the selected tab', function() {
+      store.selectTab(uiConstants.TAB_ANNOTATIONS);
+      assert.equal(store.getState().selectedTab, uiConstants.TAB_ANNOTATIONS);
+    });
+
+    it('ignores junk tag names', function() {
+      store.selectTab('flibbertigibbert');
+      assert.equal(store.getState().selectedTab, uiConstants.TAB_ANNOTATIONS);
+    });
+
+    it('allows sorting annotations by time and document location', function() {
+      store.selectTab(uiConstants.TAB_ANNOTATIONS);
+      assert.deepEqual(store.getState().sortKeysAvailable, [
+        'Newest',
+        'Oldest',
+        'Location',
+      ]);
+    });
+
+    it('allows sorting page notes by time', function() {
+      store.selectTab(uiConstants.TAB_NOTES);
+      assert.deepEqual(store.getState().sortKeysAvailable, [
+        'Newest',
+        'Oldest',
+      ]);
+    });
+
+    it('allows sorting orphans by time and document location', function() {
+      store.selectTab(uiConstants.TAB_ORPHANS);
+      assert.deepEqual(store.getState().sortKeysAvailable, [
+        'Newest',
+        'Oldest',
+        'Location',
+      ]);
+    });
+
+    it('sorts annotations by document location by default', function() {
+      store.selectTab(uiConstants.TAB_ANNOTATIONS);
+      assert.deepEqual(store.getState().sortKey, 'Location');
+    });
+
+    it('sorts page notes from oldest to newest by default', function() {
+      store.selectTab(uiConstants.TAB_NOTES);
+      assert.deepEqual(store.getState().sortKey, 'Oldest');
+    });
+
+    it('sorts orphans by document location by default', function() {
+      store.selectTab(uiConstants.TAB_ORPHANS);
+      assert.deepEqual(store.getState().sortKey, 'Location');
+    });
+
+    it('does not reset the sort key unless necessary', function() {
+      // Select the tab, setting sort key to 'Oldest', and then manually
+      // override the sort key.
+      store.selectTab(uiConstants.TAB_NOTES);
+      store.setSortKey('Newest');
+
+      store.selectTab(uiConstants.TAB_NOTES);
+
+      assert.equal(store.getState().sortKey, 'Newest');
+    });
+  });
+});

src/sidebar/store/test/index-test.js

@@ -289,244 +289,6 @@ describe('store', function() {
     );
   });
 
-  describe('#setForceVisible()', function() {
-    it('sets the visibility of the annotation', function() {
-      store.setForceVisible('id1', true);
-      assert.deepEqual(store.getState().forceVisible, { id1: true });
-    });
-  });
-
-  describe('#setCollapsed()', function() {
-    it('sets the expanded state of the annotation', function() {
-      store.setCollapsed('parent_id', false);
-      assert.deepEqual(store.getState().expanded, { parent_id: true });
-    });
-  });
-
-  describe('#focusAnnotations()', function() {
-    it('adds the passed annotations to the focusedAnnotationMap', function() {
-      store.focusAnnotations([1, 2, 3]);
-      assert.deepEqual(store.getState().focusedAnnotationMap, {
-        1: true,
-        2: true,
-        3: true,
-      });
-    });
-
-    it('replaces any annotations originally in the map', function() {
-      store.focusAnnotations([1]);
-      store.focusAnnotations([2, 3]);
-      assert.deepEqual(store.getState().focusedAnnotationMap, {
-        2: true,
-        3: true,
-      });
-    });
-
-    it('nulls the map if no annotations are focused', function() {
-      store.focusAnnotations([1]);
-      store.focusAnnotations([]);
-      assert.isNull(store.getState().focusedAnnotationMap);
-    });
-  });
-
-  describe('#hasSelectedAnnotations', function() {
-    it('returns true if there are any selected annotations', function() {
-      store.selectAnnotations([1]);
-      assert.isTrue(store.hasSelectedAnnotations());
-    });
-
-    it('returns false if there are no selected annotations', function() {
-      assert.isFalse(store.hasSelectedAnnotations());
-    });
-  });
-
-  describe('#isAnnotationSelected', function() {
-    it('returns true if the id provided is selected', function() {
-      store.selectAnnotations([1]);
-      assert.isTrue(store.isAnnotationSelected(1));
-    });
-
-    it('returns false if the id provided is not selected', function() {
-      store.selectAnnotations([1]);
-      assert.isFalse(store.isAnnotationSelected(2));
-    });
-
-    it('returns false if there are no selected annotations', function() {
-      assert.isFalse(store.isAnnotationSelected(1));
-    });
-  });
-
-  describe('#selectAnnotations()', function() {
-    it('adds the passed annotations to the selectedAnnotationMap', function() {
-      store.selectAnnotations([1, 2, 3]);
-      assert.deepEqual(store.getState().selectedAnnotationMap, {
-        1: true,
-        2: true,
-        3: true,
-      });
-    });
-
-    it('replaces any annotations originally in the map', function() {
-      store.selectAnnotations([1]);
-      store.selectAnnotations([2, 3]);
-      assert.deepEqual(store.getState().selectedAnnotationMap, {
-        2: true,
-        3: true,
-      });
-    });
-
-    it('nulls the map if no annotations are selected', function() {
-      store.selectAnnotations([1]);
-      store.selectAnnotations([]);
-      assert.isNull(store.getState().selectedAnnotationMap);
-    });
-  });
-
-  describe('#toggleSelectedAnnotations()', function() {
-    it('adds annotations missing from the selectedAnnotationMap', function() {
-      store.selectAnnotations([1, 2]);
-      store.toggleSelectedAnnotations([3, 4]);
-      assert.deepEqual(store.getState().selectedAnnotationMap, {
-        1: true,
-        2: true,
-        3: true,
-        4: true,
-      });
-    });
-
-    it('removes annotations already in the selectedAnnotationMap', function() {
-      store.selectAnnotations([1, 3]);
-      store.toggleSelectedAnnotations([1, 2]);
-      assert.deepEqual(store.getState().selectedAnnotationMap, {
-        2: true,
-        3: true,
-      });
-    });
-
-    it('nulls the map if no annotations are selected', function() {
-      store.selectAnnotations([1]);
-      store.toggleSelectedAnnotations([1]);
-      assert.isNull(store.getState().selectedAnnotationMap);
-    });
-  });
-
-  describe('#removeSelectedAnnotation()', function() {
-    it('removes an annotation from the selectedAnnotationMap', function() {
-      store.selectAnnotations([1, 2, 3]);
-      store.removeSelectedAnnotation(2);
-      assert.deepEqual(store.getState().selectedAnnotationMap, {
-        1: true,
-        3: true,
-      });
-    });
-
-    it('nulls the map if no annotations are selected', function() {
-      store.selectAnnotations([1]);
-      store.removeSelectedAnnotation(1);
-      assert.isNull(store.getState().selectedAnnotationMap);
-    });
-  });
-
-  describe('#clearSelectedAnnotations()', function() {
-    it('removes all annotations from the selection', function() {
-      store.selectAnnotations([1]);
-      store.clearSelectedAnnotations();
-      assert.isNull(store.getState().selectedAnnotationMap);
-    });
-
-    it('clears the current search query', function() {
-      store.setFilterQuery('foo');
-      store.clearSelectedAnnotations();
-      assert.isNull(store.getState().filterQuery);
-    });
-  });
-
-  describe('#setFilterQuery()', function() {
-    it('sets the filter query', function() {
-      store.setFilterQuery('a-query');
-      assert.equal(store.getState().filterQuery, 'a-query');
-    });
-
-    it('resets the force-visible and expanded sets', function() {
-      store.setForceVisible('123', true);
-      store.setCollapsed('456', false);
-      store.setFilterQuery('some-query');
-      assert.deepEqual(store.getState().forceVisible, {});
-      assert.deepEqual(store.getState().expanded, {});
-    });
-  });
-
-  describe('#highlightAnnotations()', function() {
-    it('sets the highlighted annotations', function() {
-      store.highlightAnnotations(['id1', 'id2']);
-      assert.deepEqual(store.getState().highlighted, ['id1', 'id2']);
-    });
-  });
-
-  describe('#selectTab()', function() {
-    it('sets the selected tab', function() {
-      store.selectTab(uiConstants.TAB_ANNOTATIONS);
-      assert.equal(store.getState().selectedTab, uiConstants.TAB_ANNOTATIONS);
-    });
-
-    it('ignores junk tag names', function() {
-      store.selectTab('flibbertigibbert');
-      assert.equal(store.getState().selectedTab, uiConstants.TAB_ANNOTATIONS);
-    });
-
-    it('allows sorting annotations by time and document location', function() {
-      store.selectTab(uiConstants.TAB_ANNOTATIONS);
-      assert.deepEqual(store.getState().sortKeysAvailable, [
-        'Newest',
-        'Oldest',
-        'Location',
-      ]);
-    });
-
-    it('allows sorting page notes by time', function() {
-      store.selectTab(uiConstants.TAB_NOTES);
-      assert.deepEqual(store.getState().sortKeysAvailable, [
-        'Newest',
-        'Oldest',
-      ]);
-    });
-
-    it('allows sorting orphans by time and document location', function() {
-      store.selectTab(uiConstants.TAB_ORPHANS);
-      assert.deepEqual(store.getState().sortKeysAvailable, [
-        'Newest',
-        'Oldest',
-        'Location',
-      ]);
-    });
-
-    it('sorts annotations by document location by default', function() {
-      store.selectTab(uiConstants.TAB_ANNOTATIONS);
-      assert.deepEqual(store.getState().sortKey, 'Location');
-    });
-
-    it('sorts page notes from oldest to newest by default', function() {
-      store.selectTab(uiConstants.TAB_NOTES);
-      assert.deepEqual(store.getState().sortKey, 'Oldest');
-    });
-
-    it('sorts orphans by document location by default', function() {
-      store.selectTab(uiConstants.TAB_ORPHANS);
-      assert.deepEqual(store.getState().sortKey, 'Location');
-    });
-
-    it('does not reset the sort key unless necessary', function() {
-      // Select the tab, setting sort key to 'Oldest', and then manually
-      // override the sort key.
-      store.selectTab(uiConstants.TAB_NOTES);
-      store.setSortKey('Newest');
-
-      store.selectTab(uiConstants.TAB_NOTES);
-
-      assert.equal(store.getState().sortKey, 'Newest');
-    });
-  });
-
   describe('#updatingAnchorStatus', function() {
     it("updates the annotation's orphan flag", function() {
       const annot = defaultAnnotation();