Merge pull request #1333 from hypothesis/namespace-session-groups-modules

Namespace session and groups modules

src/sidebar/services/session.js

@@ -106,7 +106,7 @@ function session(
    * @return {Profile} The updated profile data
    */
   function update(model) {
-    const prevSession = store.getState().session;
+    const prevSession = store.getRootState().session;
     const userChanged = model.userid !== prevSession.userid;
 
     // Update the session model used by the application
@@ -184,7 +184,7 @@ function session(
     // this service. In future, other services which access the session state
     // will do so directly from store or via selector functions
     get state() {
-      return store.getState().session;
+      return store.getRootState().session;
     },
 
     update,

src/sidebar/services/streamer.js

@@ -98,7 +98,7 @@ function Streamer(
       } else if (message.type === 'session-change') {
         handleSessionChangeNotification(message);
       } else if (message.type === 'whoyouare') {
-        const userid = store.getState().session.userid;
+        const userid = store.getRootState().session.userid;
         if (message.userid !== userid) {
           console.warn(
             'WebSocket user ID "%s" does not match logged-in ID "%s"',

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

@@ -57,9 +57,11 @@ describe('groups', function() {
 
     fakeStore = fakeReduxStore(
       {
-        mainFrame: { uri: 'http://example.org' },
+        frames: [{ uri: 'http://example.org' }],
+        groups: {
           focusedGroup: null,
           groups: [],
+        },
         directLinked: {
           directLinkedGroupId: null,
           directLinkedAnnotationId: null,
@@ -70,19 +72,16 @@ describe('groups', function() {
         getGroup: sinon.stub(),
         loadGroups: sinon.stub(),
         allGroups() {
-          return this.getState().groups;
-        },
-        getInScopeGroups() {
-          return this.getState().groups;
+          return this.getRootState().groups.groups;
         },
         focusedGroup() {
-          return this.getState().focusedGroup;
+          return this.getRootState().groups.focusedGroup;
         },
         mainFrame() {
-          return this.getState().mainFrame;
+          return this.getRootState().frames[0];
         },
         focusedGroupId() {
-          const group = this.getState().focusedGroup;
+          const group = this.getRootState().groups.focusedGroup;
           return group ? group.id : null;
         },
         setDirectLinkedGroupFetchFailed: sinon.stub(),
@@ -430,9 +429,9 @@ describe('groups', function() {
       it('waits for the document URL to be determined', () => {
         const svc = service();
 
-        fakeStore.setState({ mainFrame: null });
+        fakeStore.setState({ frames: [null] });
         const loaded = svc.load();
-        fakeStore.setState({ mainFrame: { uri: 'https://asite.com' } });
+        fakeStore.setState({ frames: [{ uri: 'https://asite.com' }] });
 
         return loaded.then(() => {
           assert.calledWith(fakeApi.groups.list, {
@@ -449,7 +448,7 @@ describe('groups', function() {
       });
 
       it('does not wait for the document URL', () => {
-        fakeStore.setState({ mainFrame: null });
+        fakeStore.setState({ frames: [null] });
         const svc = service();
         return svc.load().then(() => {
           assert.calledWith(fakeApi.groups.list, {
@@ -751,7 +750,9 @@ describe('groups', function() {
   describe('#focused', function() {
     it('returns the focused group', function() {
       const svc = service();
-      fakeStore.setState({ groups: dummyGroups, focusedGroup: dummyGroups[2] });
+      fakeStore.setState({
+        groups: { groups: dummyGroups, focusedGroup: dummyGroups[2] },
+      });
       assert.equal(svc.focused(), dummyGroups[2]);
     });
   });
@@ -768,7 +769,9 @@ describe('groups', function() {
     it('stores the focused group id in localStorage', function() {
       service();
 
-      fakeStore.setState({ groups: dummyGroups, focusedGroup: dummyGroups[1] });
+      fakeStore.setState({
+        groups: { groups: dummyGroups, focusedGroup: dummyGroups[1] },
+      });
 
       assert.calledWithMatch(
         fakeLocalStorage.setItem,
@@ -780,7 +783,9 @@ describe('groups', function() {
     it('emits the GROUP_FOCUSED event if the focused group changed', function() {
       service();
 
-      fakeStore.setState({ groups: dummyGroups, focusedGroup: dummyGroups[1] });
+      fakeStore.setState({
+        groups: { groups: dummyGroups, focusedGroup: dummyGroups[1] },
+      });
 
       assert.calledWith(
         fakeRootScope.$broadcast,
@@ -792,9 +797,14 @@ describe('groups', function() {
     it('does not emit GROUP_FOCUSED if the focused group did not change', () => {
       service();
 
-      fakeStore.setState({ groups: dummyGroups, focusedGroup: dummyGroups[1] });
+      fakeStore.setState({
+        groups: { groups: dummyGroups, focusedGroup: dummyGroups[1] },
+      });
+
       fakeRootScope.$broadcast.reset();
-      fakeStore.setState({ groups: dummyGroups, focusedGroup: dummyGroups[1] });
+      fakeStore.setState({
+        groups: { groups: dummyGroups, focusedGroup: dummyGroups[1] },
+      });
 
       assert.notCalled(fakeRootScope.$broadcast);
     });
@@ -825,7 +835,9 @@ describe('groups', function() {
       it('should refetch groups if main frame URL has changed', () => {
         const svc = service();
 
-        fakeStore.setState({ mainFrame: { uri: 'https://domain.com/page-a' } });
+        fakeStore.setState({
+          frames: [{ uri: 'https://domain.com/page-a' }],
+        });
         return svc
           .load()
           .then(() => {
@@ -833,7 +845,7 @@ describe('groups', function() {
             // a single page application.
             fakeApi.groups.list.resetHistory();
             fakeStore.setState({
-              mainFrame: { uri: 'https://domain.com/page-b' },
+              frames: [{ uri: 'https://domain.com/page-b' }],
             });
 
             return fakeRootScope.eventCallbacks[events.FRAME_CONNECTED]();
@@ -846,7 +858,9 @@ describe('groups', function() {
       it('should not refetch groups if main frame URL has not changed', () => {
         const svc = service();
 
-        fakeStore.setState({ mainFrame: { uri: 'https://domain.com/page-a' } });
+        fakeStore.setState({
+          frames: [{ uri: 'https://domain.com/page-a' }],
+        });
         return svc
           .load()
           .then(() => {

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

@@ -35,7 +35,7 @@ describe('sidebar.session', function() {
       events: require('../analytics').events,
     };
     const fakeStore = {
-      getState: function() {
+      getRootState: function() {
         return { session: state };
       },
       updateSession: function(session) {

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

 'use strict';
 
 const EventEmitter = require('tiny-emitter');
-
-const unroll = require('../../../shared/test/util').unroll;
-
 const Streamer = require('../streamer');
 
 const fixtures = {
@@ -122,7 +119,7 @@ describe('Streamer', function() {
     fakeStore = {
       annotationExists: sinon.stub().returns(false),
       clearPendingUpdates: sinon.stub(),
-      getState: sinon.stub().returns({
+      getRootState: sinon.stub().returns({
         session: {
           userid: 'jim@hypothes.is',
         },
@@ -400,10 +397,18 @@ describe('Streamer', function() {
       console.warn.restore();
     });
 
-    unroll(
-      'does nothing if the userid matches the logged-in userid',
-      function(testCase) {
-        fakeStore.getState.returns({
+    [
+      {
+        userid: 'acct:mr_bond@hypothes.is',
+        websocketUserid: 'acct:mr_bond@hypothes.is',
+      },
+      {
+        userid: null,
+        websocketUserid: null,
+      },
+    ].forEach(testCase => {
+      it('does nothing if the userid matches the logged-in userid', () => {
+        fakeStore.getRootState.returns({
           session: {
             userid: testCase.userid,
           },
@@ -416,23 +421,21 @@ describe('Streamer', function() {
           });
           assert.notCalled(console.warn);
         });
-      },
+      });
+    });
+
     [
       {
         userid: 'acct:mr_bond@hypothes.is',
-          websocketUserid: 'acct:mr_bond@hypothes.is',
+        websocketUserid: 'acct:the_spanish_inquisition@hypothes.is',
       },
       {
         userid: null,
-          websocketUserid: null,
+        websocketUserid: 'acct:the_spanish_inquisition@hypothes.is',
       },
-      ]
-    );
-
-    unroll(
-      'logs a warning if the userid does not match the logged-in userid',
-      function(testCase) {
-        fakeStore.getState.returns({
+    ].forEach(testCase => {
+      it('logs a warning if the userid does not match the logged-in userid', () => {
+        fakeStore.getRootState.returns({
           session: {
             userid: testCase.userid,
           },
@@ -445,18 +448,8 @@ describe('Streamer', function() {
           });
           assert.called(console.warn);
         });
-      },
-      [
-        {
-          userid: 'acct:mr_bond@hypothes.is',
-          websocketUserid: 'acct:the_spanish_inquisition@hypothes.is',
-        },
-        {
-          userid: null,
-          websocketUserid: 'acct:the_spanish_inquisition@hypothes.is',
-        },
-      ]
-    );
+      });
+    });
   });
 
   describe('reconnections', function() {

src/sidebar/store/modules/groups.js

@@ -102,10 +102,10 @@ function loadGroups(groups) {
  * @return {Group|null}
  */
 function focusedGroup(state) {
-  if (!state.focusedGroupId) {
+  if (!state.groups.focusedGroupId) {
     return null;
   }
-  return getGroup(state, state.focusedGroupId);
+  return getGroup(state, state.groups.focusedGroupId);
 }
 
 /**
@@ -114,7 +114,7 @@ function focusedGroup(state) {
  * @return {string|null}
  */
 function focusedGroupId(state) {
-  return state.focusedGroupId;
+  return state.groups.focusedGroupId;
 }
 
 /**
@@ -123,16 +123,17 @@ function focusedGroupId(state) {
  * @return {Group[]}
  */
 function allGroups(state) {
-  return state.groups;
+  return state.groups.groups;
 }
 
 /**
  * Return the group with the given ID.
  *
+ * @param {string} id
  * @return {Group|undefined}
  */
 function getGroup(state, id) {
-  return state.groups.find(g => g.id === id);
+  return state.groups.groups.find(g => g.id === id);
 }
 
 /**
@@ -141,7 +142,7 @@ function getGroup(state, id) {
  * @return {Group[]}
  */
 const getMyGroups = createSelector(
-  state => state.groups,
+  state => state.groups.groups,
   isLoggedIn,
   (groups, loggedIn) => {
     // If logged out, the Public group still has isMember set to true so only
@@ -159,7 +160,7 @@ const getMyGroups = createSelector(
  * @return {Group[]}
  */
 const getFeaturedGroups = createSelector(
-  state => state.groups,
+  state => state.groups.groups,
   groups => groups.filter(group => !group.isMember && group.isScopedToUri)
 );
 
@@ -187,12 +188,13 @@ const getCurrentlyViewingGroups = createSelector(
  * @return {Group[]}
  */
 const getInScopeGroups = createSelector(
-  state => state.groups,
+  state => state.groups.groups,
   groups => groups.filter(g => g.isScopedToUri)
 );
 
 module.exports = {
   init,
+  namespace: 'groups',
   update,
   actions: {
     focusGroup,

src/sidebar/store/modules/session.js

@@ -3,11 +3,10 @@
 const util = require('../util');
 
 function init() {
-  return {
   /**
    * Profile/session information for the active user.
    */
-    session: {
+  return {
     /** A map of features that are enabled for the current user. */
     features: {},
     /** A map of preference names and values. */
@@ -16,14 +15,13 @@ function init() {
      * The authenticated user ID or null if the user is not logged in.
      */
     userid: null,
-    },
   };
 }
 
 const update = {
   UPDATE_SESSION: function(state, action) {
     return {
-      session: action.session,
+      ...action.session,
     };
   },
 };
@@ -71,6 +69,7 @@ function profile(state) {
 
 module.exports = {
   init,
+  namespace: 'session',
   update,
 
   actions: {

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

@@ -4,6 +4,7 @@ const immutable = require('seamless-immutable');
 
 const createStore = require('../../create-store');
 const groups = require('../groups');
+const session = require('../session');
 
 describe('sidebar.store.modules.groups', () => {
   const publicGroup = immutable({
@@ -73,7 +74,7 @@ describe('sidebar.store.modules.groups', () => {
   let store;
 
   beforeEach(() => {
-    store = createStore([groups]);
+    store = createStore([groups, session]);
   });
 
   describe('focusGroup', () => {
@@ -90,7 +91,7 @@ describe('sidebar.store.modules.groups', () => {
 
       store.focusGroup(publicGroup.id);
 
-      assert.equal(store.getState().focusedGroupId, publicGroup.id);
+      assert.equal(store.getRootState().groups.focusedGroupId, publicGroup.id);
       assert.notCalled(console.error);
     });
 
@@ -99,7 +100,7 @@ describe('sidebar.store.modules.groups', () => {
 
       store.focusGroup(privateGroup.id);
 
-      assert.equal(store.getState().focusedGroupId, publicGroup.id);
+      assert.equal(store.getRootState().groups.focusedGroupId, publicGroup.id);
       assert.called(console.error);
     });
   });
@@ -107,7 +108,7 @@ describe('sidebar.store.modules.groups', () => {
   describe('loadGroups', () => {
     it('updates the set of groups', () => {
       store.loadGroups([publicGroup]);
-      assert.deepEqual(store.getState().groups, [publicGroup]);
+      assert.deepEqual(store.getRootState().groups.groups, [publicGroup]);
     });
 
     it('resets the focused group if not in new set of groups', () => {
@@ -115,7 +116,7 @@ describe('sidebar.store.modules.groups', () => {
       store.focusGroup(publicGroup.id);
       store.loadGroups([]);
 
-      assert.equal(store.getState().focusedGroupId, null);
+      assert.equal(store.getRootState().groups.focusedGroupId, null);
     });
 
     it('leaves focused group unchanged if in new set of groups', () => {
@@ -123,7 +124,7 @@ describe('sidebar.store.modules.groups', () => {
       store.focusGroup(publicGroup.id);
       store.loadGroups([publicGroup, privateGroup]);
 
-      assert.equal(store.getState().focusedGroupId, publicGroup.id);
+      assert.equal(store.getRootState().groups.focusedGroupId, publicGroup.id);
     });
   });
 
@@ -133,7 +134,7 @@ describe('sidebar.store.modules.groups', () => {
 
       store.clearGroups();
 
-      assert.equal(store.getState().groups.length, 0);
+      assert.equal(store.getRootState().groups.groups.length, 0);
     });
 
     it('clears the focused group id', () => {
@@ -142,7 +143,7 @@ describe('sidebar.store.modules.groups', () => {
 
       store.clearGroups();
 
-      assert.equal(store.getState().focusedGroupId, null);
+      assert.equal(store.getRootState().groups.focusedGroupId, null);
     });
   });
 
@@ -231,11 +232,9 @@ describe('sidebar.store.modules.groups', () => {
     ].forEach(
       ({ description, isLoggedIn, allGroups, expectedFeaturedGroups }) => {
         it(description, () => {
-          store.getState().session = { userid: isLoggedIn ? '1234' : null };
+          store.updateSession({ userid: isLoggedIn ? '1234' : null });
           store.loadGroups(allGroups);
-
           const featuredGroups = getListAssertNoDupes(store, 'featuredGroups');
-
           assert.deepEqual(featuredGroups, expectedFeaturedGroups);
         });
       }
@@ -273,7 +272,7 @@ describe('sidebar.store.modules.groups', () => {
       },
     ].forEach(({ description, isLoggedIn, allGroups, expectedMyGroups }) => {
       it(description, () => {
-        store.getState().session = { userid: isLoggedIn ? '1234' : null };
+        store.updateSession({ userid: isLoggedIn ? '1234' : null });
         store.loadGroups(allGroups);
 
         const myGroups = getListAssertNoDupes(store, 'myGroups');
@@ -305,7 +304,7 @@ describe('sidebar.store.modules.groups', () => {
       },
     ].forEach(({ description, isLoggedIn, allGroups }) => {
       it(description, () => {
-        store.getState().session = { userid: isLoggedIn ? '1234' : null };
+        store.updateSession({ userid: isLoggedIn ? '1234' : null });
         store.loadGroups(allGroups);
 
         const currentlyViewing = getListAssertNoDupes(

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

 'use strict';
 
+const createStore = require('../../create-store');
 const session = require('../session');
 
-const util = require('../../util');
-
-const { init, actions, selectors } = session;
-const update = util.createReducer(session.update);
+const { init } = session;
 
 describe('sidebar.reducers.session', function() {
+  let store;
+
+  beforeEach(() => {
+    store = createStore([session]);
+  });
+
   describe('#updateSession', function() {
     it('updates the session state', function() {
       const newSession = Object.assign(init(), { userid: 'john' });
-      const state = update(init(), actions.updateSession(newSession));
-      assert.deepEqual(state.session, newSession);
+      store.updateSession({ userid: 'john' });
+      assert.deepEqual(store.getRootState().session, newSession);
     });
   });
 
@@ -22,18 +26,20 @@ describe('sidebar.reducers.session', function() {
       { userid: null, expectedIsLoggedIn: false },
     ].forEach(({ userid, expectedIsLoggedIn }) => {
       it('returns whether the user is logged in', () => {
-        const newSession = Object.assign(init(), { userid: userid });
-        const state = update(init(), actions.updateSession(newSession));
-        assert.equal(selectors.isLoggedIn(state), expectedIsLoggedIn);
+        store.updateSession({ userid: userid });
+        assert.equal(store.isLoggedIn(), expectedIsLoggedIn);
       });
     });
   });
 
   describe('#profile', () => {
     it("returns the user's profile", () => {
-      const newSession = Object.assign(init(), { userid: 'john' });
-      const state = update(init(), actions.updateSession(newSession));
-      assert.equal(selectors.profile(state), newSession);
+      store.updateSession({ userid: 'john' });
+      assert.deepEqual(store.profile(), {
+        userid: 'john',
+        features: {},
+        preferences: {},
+      });
     });
   });
 });