Merge pull request #1938 from hypothesis/direct-linked-selectors

Add selectors for direct-linked annotation, group IDs

src/sidebar/services/groups.js

@@ -210,8 +210,7 @@ export default function groups(
     // If there is a direct-linked annotation, fetch the annotation in case
     // the associated group has not already been fetched and we need to make
     // an additional request for it.
-    const directLinkedAnnId = store.getState().directLinked
-      .directLinkedAnnotationId;
+    const directLinkedAnnId = store.directLinkedAnnotationId();
     let directLinkedAnnApi = null;
     if (directLinkedAnnId) {
       directLinkedAnnApi = api.annotation
@@ -225,8 +224,7 @@ export default function groups(
     // If there is a direct-linked group, add an API request to get that
     // particular group since it may not be in the set of groups that are
     // fetched by other requests.
-    const directLinkedGroupId = store.getState().directLinked
-      .directLinkedGroupId;
+    const directLinkedGroupId = store.directLinkedGroupId();
     let directLinkedGroupApi = null;
     if (directLinkedGroupId) {
       directLinkedGroupApi = fetchGroup({

src/sidebar/services/test/groups-test.js

@@ -71,6 +71,8 @@ describe('groups', function() {
       },
       {
         addAnnotations: sinon.stub(),
+        directLinkedAnnotationId: sinon.stub().returns(null),
+        directLinkedGroupId: sinon.stub().returns(null),
         focusGroup: sinon.stub(),
         focusedGroupId: sinon.stub(),
         getDefault: sinon.stub(),
@@ -245,11 +247,8 @@ describe('groups', function() {
         id: 'oos',
         scopes: { enforced: true, uri_patterns: ['http://foo.com'] },
       };
-      fakeStore.setState({
-        directLinked: {
-          directLinkedGroupId: outOfScopeEnforcedGroup.id,
-        },
-      });
+      fakeStore.directLinkedGroupId.returns(outOfScopeEnforcedGroup.id);
+
       fakeApi.group.read.returns(Promise.resolve(outOfScopeEnforcedGroup));
       return svc.load().then(groups => {
         // The failure state is captured in the store.
@@ -264,11 +263,8 @@ describe('groups', function() {
     it('catches error from api.group.read request', () => {
       const svc = service();
       fakeStore.getDefault.returns(dummyGroups[0].id);
-      fakeStore.setState({
-        directLinked: {
-          directLinkedGroupId: 'does-not-exist',
-        },
-      });
+      fakeStore.directLinkedGroupId.returns('does-not-exist');
+
       fakeApi.group.read.returns(
         Promise.reject(
           new Error(
@@ -306,12 +302,7 @@ describe('groups', function() {
     it('does not duplicate groups if the direct-linked group is also a featured group', () => {
       const svc = service();
 
-      // Set the direct-linked group to dummyGroups[0].
-      fakeStore.setState({
-        directLinkedGroupId: {
-          directLinkedGroupId: dummyGroups[0].id,
-        },
-      });
+      fakeStore.directLinkedGroupId.returns(dummyGroups[0].id);
       fakeApi.group.read.returns(Promise.resolve(dummyGroups[0]));
 
       // Include the dummyGroups[0] in the featured groups.
@@ -327,11 +318,8 @@ describe('groups', function() {
     it('combines groups from all 3 endpoints if there is a selectedGroup', () => {
       const svc = service();
 
-      fakeStore.setState({
-        directLinked: {
-          directLinkedGroupId: 'selected-id',
-        },
-      });
+      fakeStore.directLinkedGroupId.returns('selected-id');
+
       const groups = [
         { id: 'groupa', name: 'GroupA' },
         { id: 'groupb', name: 'GroupB' },
@@ -350,11 +338,8 @@ describe('groups', function() {
     it('passes the direct-linked group id from the store to the api.group.read call', () => {
       const svc = service();
 
-      fakeStore.setState({
-        directLinked: {
-          directLinkedGroupId: 'selected-id',
-        },
-      });
+      fakeStore.directLinkedGroupId.returns('selected-id');
+
       const group = { id: 'selected-id', name: 'Selected Group' };
 
       fakeApi.profile.groups.read.returns(Promise.resolve([]));
@@ -384,11 +369,7 @@ describe('groups', function() {
       fakeApi.groups.list.returns(
         Promise.resolve([{ id: 'groupa', name: 'GroupA' }])
       );
-      fakeStore.setState({
-        directLinked: {
-          directLinkedGroupId: 'group-id',
-        },
-      });
+      fakeStore.directLinkedGroupId.returns('group-id');
 
       return svc.load().then(() => {
         assert.calledWith(
@@ -416,12 +397,9 @@ describe('groups', function() {
 
     it("sets the direct-linked annotation's group to take precedence over the group saved in local storage and the direct-linked group", () => {
       const svc = service();
-      fakeStore.setState({
-        directLinked: {
-          directLinkedAnnotationId: 'ann-id',
-          directLinkedGroupId: dummyGroups[1].id,
-        },
-      });
+      fakeStore.directLinkedAnnotationId.returns('ann-id');
+      fakeStore.directLinkedGroupId.returns(dummyGroups[1].id);
+
       fakeStore.getDefault.returns(dummyGroups[0].id);
       fakeApi.groups.list.returns(Promise.resolve(dummyGroups));
       fakeApi.annotation.get.returns(
@@ -437,11 +415,8 @@ describe('groups', function() {
 
     it("sets the focused group to the direct-linked annotation's group", () => {
       const svc = service();
-      fakeStore.setState({
-        directLinked: {
-          directLinkedAnnotationId: 'ann-id',
-        },
-      });
+      fakeStore.directLinkedAnnotationId.returns('ann-id');
+
       fakeApi.groups.list.returns(Promise.resolve(dummyGroups));
       fakeStore.getDefault.returns(dummyGroups[0].id);
       fakeApi.annotation.get.returns(
@@ -457,11 +432,8 @@ describe('groups', function() {
 
     it('sets the direct-linked group to take precedence over the group saved in local storage', () => {
       const svc = service();
-      fakeStore.setState({
-        directLinked: {
-          directLinkedGroupId: dummyGroups[1].id,
-        },
-      });
+
+      fakeStore.directLinkedGroupId.returns(dummyGroups[1].id);
       fakeStore.getDefault.returns(dummyGroups[0].id);
       fakeApi.groups.list.returns(Promise.resolve(dummyGroups));
       return svc.load().then(() => {
@@ -471,11 +443,8 @@ describe('groups', function() {
 
     it('sets the focused group to the direct-linked group', () => {
       const svc = service();
-      fakeStore.setState({
-        directLinked: {
-          directLinkedGroupId: dummyGroups[1].id,
-        },
-      });
+
+      fakeStore.directLinkedGroupId.returns(dummyGroups[1].id);
       fakeApi.groups.list.returns(Promise.resolve(dummyGroups));
       return svc.load().then(() => {
         assert.calledWith(fakeStore.focusGroup, dummyGroups[1].id);
@@ -484,12 +453,9 @@ describe('groups', function() {
 
     it('clears the directLinkedGroupFetchFailed state if loading a direct-linked group', () => {
       const svc = service();
-      fakeStore.setState({
-        directLinked: {
-          directLinkedGroupId: dummyGroups[1].id,
-        },
-      });
+      fakeStore.directLinkedGroupId.returns(dummyGroups[1].id);
       fakeApi.groups.list.returns(Promise.resolve(dummyGroups));
+
       return svc.load().then(() => {
         assert.called(fakeStore.clearDirectLinkedGroupFetchFailed);
         assert.notCalled(fakeStore.setDirectLinkedGroupFetchFailed);
@@ -563,8 +529,7 @@ describe('groups', function() {
     it('catches error when fetching the direct-linked annotation', () => {
       const svc = service();
 
-      fakeStore.setState({ directLinkedAnnotationId: 'ann-id' });
-
+      fakeStore.directLinkedAnnotationId.returns('ann-id');
       fakeApi.profile.groups.read.returns(Promise.resolve([]));
       fakeApi.groups.list.returns(
         Promise.resolve([{ name: 'BioPub', id: 'biopub' }])
@@ -587,7 +552,7 @@ describe('groups', function() {
     it("catches error when fetching the direct-linked annotation's group", () => {
       const svc = service();
 
-      fakeStore.setState({ directLinkedAnnotationId: 'ann-id' });
+      fakeStore.directLinkedAnnotationId.returns('ann-id');
 
       fakeApi.profile.groups.read.returns(Promise.resolve([]));
       fakeApi.groups.list.returns(
@@ -623,11 +588,7 @@ describe('groups', function() {
     it("includes the direct-linked annotation's group when it is not in the normal list of groups", () => {
       const svc = service();
 
-      fakeStore.setState({
-        directLinked: {
-          directLinkedAnnotationId: 'ann-id',
-        },
-      });
+      fakeStore.directLinkedAnnotationId.returns('ann-id');
 
       fakeApi.profile.groups.read.returns(Promise.resolve([]));
       fakeApi.groups.list.returns(
@@ -659,12 +620,8 @@ describe('groups', function() {
       // the frame embedding the client.
       const svc = service();
 
-      fakeStore.setState({
-        directLinked: {
-          directLinkedGroupId: 'out-of-scope',
-          directLinkedAnnotationId: 'ann-id',
-        },
-      });
+      fakeStore.directLinkedGroupId.returns('out-of-scope');
+      fakeStore.directLinkedAnnotationId.returns('ann-id');
 
       fakeApi.profile.groups.read.returns(Promise.resolve([]));
       fakeApi.groups.list.returns(
@@ -700,12 +657,7 @@ describe('groups', function() {
       // out and there are associated groups.
       const svc = service();
 
-      fakeStore.setState({
-        directLinked: {
-          directLinkedGroupId: '__world__',
-          directLinkedAnnotationId: null,
-        },
-      });
+      fakeStore.directLinkedGroupId.returns('__world__');
 
       fakeApi.profile.groups.read.returns(Promise.resolve([]));
       fakeApi.groups.list.returns(
@@ -743,9 +695,7 @@ describe('groups', function() {
                 group: '__world__',
               })
             );
-            fakeStore.setState({
-              directLinked: { directLinkedAnnotationId: 'direct-linked-ann' },
-            });
+            fakeStore.directLinkedAnnotationId.returns('direct-linked-ann');
           }
 
           // Create groups response from server.

src/sidebar/store/modules/direct-linked.js

@@ -125,6 +125,17 @@ function clearDirectLinkedIds() {
   };
 }
 
+/**
+ * Selectors
+ */
+function directLinkedAnnotationId(state) {
+  return state.directLinked.directLinkedAnnotationId;
+}
+
+function directLinkedGroupId(state) {
+  return state.directLinked.directLinkedGroupId;
+}
+
 export default {
   init,
   namespace: 'directLinked',
@@ -136,5 +147,8 @@ export default {
     clearDirectLinkedGroupFetchFailed,
     clearDirectLinkedIds,
   },
-  selectors: {},
+  selectors: {
+    directLinkedAnnotationId,
+    directLinkedGroupId,
+  },
 };

src/sidebar/store/modules/test/direct-linked-test.js

@@ -3,13 +3,14 @@ import directLinked from '../direct-linked';
 
 describe('sidebar/store/modules/direct-linked', () => {
   let store;
-  let fakeSettings = {};
+  let fakeSettings;
 
   const getDirectLinkedState = () => {
     return store.getState().directLinked;
   };
 
   beforeEach(() => {
+    fakeSettings = {};
     store = createStore([directLinked], [fakeSettings]);
   });
 
@@ -36,14 +37,14 @@ describe('sidebar/store/modules/direct-linked', () => {
 
     store = createStore([directLinked], [fakeSettings]);
 
-    assert.equal(getDirectLinkedState().directLinkedAnnotationId, 'ann-id');
+    assert.equal(store.directLinkedAnnotationId(), 'ann-id');
   });
 
   describe('setDirectLinkedAnnotationId', () => {
     it('sets directLinkedAnnotationId to the specified annotation id', () => {
       store.setDirectLinkedAnnotationId('ann-id');
 
-      assert.equal(getDirectLinkedState().directLinkedAnnotationId, 'ann-id');
+      assert.equal(store.directLinkedAnnotationId(), 'ann-id');
     });
   });
 
@@ -52,14 +53,14 @@ describe('sidebar/store/modules/direct-linked', () => {
 
     store = createStore([directLinked], [fakeSettings]);
 
-    assert.equal(getDirectLinkedState().directLinkedGroupId, 'group-id');
+    assert.equal(store.directLinkedGroupId(), 'group-id');
   });
 
   describe('setDirectLinkedGroupId', () => {
     it('sets directLinkedGroupId to the specified group id', () => {
       store.setDirectLinkedGroupId('group-id');
 
-      assert.equal(getDirectLinkedState().directLinkedGroupId, 'group-id');
+      assert.equal(store.directLinkedGroupId(), 'group-id');
     });
   });
 
@@ -70,8 +71,32 @@ describe('sidebar/store/modules/direct-linked', () => {
 
       store.clearDirectLinkedIds();
 
-      assert.equal(getDirectLinkedState().directLinkedAnnotationId, null);
-      assert.equal(getDirectLinkedState().directLinkedGroupId, null);
+      assert.isNull(store.directLinkedGroupId());
+      assert.isNull(store.directLinkedAnnotationId());
+    });
+  });
+
+  describe('selectors', () => {
+    describe('#directLinkedAnnotationId', () => {
+      it('should return the current direct-linked annotation ID', () => {
+        store.setDirectLinkedAnnotationId('ann-id');
+        assert.equal(store.directLinkedAnnotationId(), 'ann-id');
+      });
+
+      it('should return `null` if no direct-linked annotation ID', () => {
+        assert.isNull(store.directLinkedAnnotationId());
+      });
+    });
+
+    describe('#directLinkedGroupId', () => {
+      it('should return the current direct-linked group ID', () => {
+        store.setDirectLinkedGroupId('group-id');
+        assert.equal(store.directLinkedGroupId(), 'group-id');
+      });
+
+      it('should return `null` if no direct-linked group ID', () => {
+        assert.isNull(store.directLinkedGroupId());
+      });
     });
   });
 });