Refactor fetching, tracking multiple narrows (#1100)

src/baseSelectors.js

@@ -1,12 +1,16 @@
 /* @flow */
 import type { GlobalState, Narrow, Message, Subscription, Stream, Presence, Outbox } from './types';
 
+export const getApp = (state: GlobalState): Object => state.app;
+
 export const getMute = (state: GlobalState): Object => state.mute;
 
 export const getTyping = (state: GlobalState): Object => state.typing;
 
 export const getUsers = (state: GlobalState): any[] => state.users;
 
+export const getFetching = (state: GlobalState): Object => state.fetching;
+
 export const getFlags = (state: GlobalState): Object => state.flags;
 
 export const getReadFlags = (state: GlobalState): Object => state.flags.read;

src/caughtup/__tests__/caughtUpReducers-test.js

@@ -14,8 +14,6 @@ describe('caughtUpReducers', () => {
       const action = deepFreeze({
         type: MESSAGE_FETCH_START,
         narrow: [],
-        fetchingOlder: true,
-        fetchingNewer: true,
       });
 
       const expectedState = {

src/chat/__tests__/chatReducers-test.js

@@ -42,8 +42,6 @@ describe('chatReducers', () => {
 
       const expectedState = {
         narrow: streamNarrow('some stream'),
-        fetchingOlder: false,
-        fetchingNewer: false,
       };
 
       const newState = chatReducers(initialState, action);
@@ -658,8 +656,6 @@ describe('chatReducers', () => {
       });
 
       const expectedState = {
-        fetchingOlder: false,
-        fetchingNewer: false,
         messages: {
           [homeNarrowStr]: [{ id: 3, timestamp: 2 }, { id: 4, timestamp: 1 }],
         },

src/chat/__tests__/fetchingReducers-test.js

@@ -0,0 +1,95 @@
+import deepFreeze from 'deep-freeze';
+
+import fetchingReducers from '../fetchingReducers';
+import { homeNarrowStr, streamNarrow } from '../../utils/narrow';
+import { SWITCH_NARROW, MESSAGE_FETCH_START, MESSAGE_FETCH_SUCCESS } from '../../actionConstants';
+
+describe('fetchingReducers', () => {
+  describe('SWITCH_NARROW', () => {
+    test('resets state', () => {
+      const initialState = deepFreeze({
+        '[]': { older: true, newer: true },
+      });
+
+      const action = deepFreeze({
+        type: SWITCH_NARROW,
+        narrow: streamNarrow('some stream'),
+      });
+
+      const expectedState = {};
+
+      const newState = fetchingReducers(initialState, action);
+
+      expect(newState).toEqual(expectedState);
+    });
+  });
+
+  describe('MESSAGE_FETCH_START', () => {
+    test('if messages are fetched before or after the corresponding flag is set', () => {
+      const initialState = deepFreeze({
+        [homeNarrowStr]: { older: false, newer: false },
+      });
+
+      const action = deepFreeze({
+        type: MESSAGE_FETCH_START,
+        narrow: [],
+        numBefore: 10,
+        numAfter: 10,
+      });
+
+      const expectedState = {
+        [homeNarrowStr]: { older: true, newer: true },
+      };
+
+      const newState = fetchingReducers(initialState, action);
+
+      expect(newState).toEqual(expectedState);
+    });
+
+    test('if key for narrow does not exist, it is created and corresponding flags are set', () => {
+      const initialState = deepFreeze({
+        [homeNarrowStr]: { older: false, newer: false },
+      });
+
+      const action = deepFreeze({
+        type: MESSAGE_FETCH_START,
+        narrow: streamNarrow('some stream'),
+        numBefore: 10,
+        numAfter: 0,
+      });
+
+      const expectedState = {
+        [homeNarrowStr]: { older: false, newer: false },
+        [JSON.stringify(streamNarrow('some stream'))]: { older: true, newer: false },
+      };
+
+      const newState = fetchingReducers(initialState, action);
+
+      expect(newState).toEqual(expectedState);
+    });
+  });
+
+  describe('MESSAGE_FETCH_SUCCESS', () => {
+    test('sets corresponding fetching flags to false, if messages are received before or after', () => {
+      const initialState = deepFreeze({
+        [homeNarrowStr]: { older: true, newer: true },
+      });
+
+      const action = deepFreeze({
+        type: MESSAGE_FETCH_SUCCESS,
+        narrow: [],
+        messages: [{ id: 1 }],
+        numBefore: 10,
+        numAfter: 0,
+      });
+
+      const expectedState = {
+        [homeNarrowStr]: { older: false, newer: true },
+      };
+
+      const newState = fetchingReducers(initialState, action);
+
+      expect(newState).toEqual(expectedState);
+    });
+  });
+});

src/chat/__tests__/fetchingSelectors-test.js

@@ -0,0 +1,36 @@
+import deepFreeze from 'deep-freeze';
+
+import { getFetchingForActiveNarrow } from '../fetchingSelectors';
+import { homeNarrow, homeNarrowStr } from '../../utils/narrow';
+
+describe('getFetchingForActiveNarrow', () => {
+  test('if no narrow information exists in state, return a null fetching object', () => {
+    const state = deepFreeze({
+      chat: {
+        narrow: homeNarrow,
+      },
+      fetching: {},
+    });
+    const expectedResult = { older: false, newer: false };
+
+    const actualResult = getFetchingForActiveNarrow(state);
+
+    expect(actualResult).toEqual(expectedResult);
+  });
+
+  test('if an entry matching current narrow exists, it is returned', () => {
+    const state = deepFreeze({
+      chat: {
+        narrow: homeNarrow,
+      },
+      fetching: {
+        [homeNarrowStr]: { older: true, newer: true },
+      },
+    });
+    const expectedResult = { older: true, newer: true };
+
+    const actualResult = getFetchingForActiveNarrow(state);
+
+    expect(actualResult).toEqual(expectedResult);
+  });
+});

src/chat/chatReducers.js

@@ -7,7 +7,6 @@ import {
   LOGIN_SUCCESS,
   ACCOUNT_SWITCH,
   SWITCH_NARROW,
-  MESSAGE_FETCH_START,
   MESSAGE_FETCH_SUCCESS,
   EVENT_NEW_MESSAGE,
   EVENT_REACTION_ADD,
@@ -19,8 +18,6 @@ import chatUpdater from './chatUpdater';
 import { getMessagesById } from '../selectors';
 
 const initialState: ChatState = {
-  fetchingOlder: true,
-  fetchingNewer: true,
   narrow: homeNarrow,
   messages: {},
 };
@@ -31,23 +28,11 @@ export default (state: ChatState = initialState, action: Action) => {
     case LOGIN_SUCCESS:
     case ACCOUNT_SWITCH:
       return initialState;
-    case MESSAGE_FETCH_START:
-      if (!isEqual(action.narrow, state.narrow)) {
-        return state;
-      }
-
-      return {
-        ...state,
-        fetchingOlder: action.fetchingOlder || state.fetchingOlder,
-        fetchingNewer: action.fetchingNewer || state.fetchingNewer,
-      };
 
     case SWITCH_NARROW: {
       return {
         ...state,
         narrow: action.narrow,
-        fetchingOlder: false,
-        fetchingNewer: false,
       };
     }
 
@@ -69,8 +54,6 @@ export default (state: ChatState = initialState, action: Action) => {
 
       return {
         ...state,
-        fetchingOlder: false,
-        fetchingNewer: false,
         messages: {
           ...state.messages,
           [key]: newMessages,

src/chat/chatSelectors.js

@@ -1,7 +1,6 @@
 /* @flow */
 import { createSelector } from 'reselect';
 
-import type { GlobalState } from '../types';
 import {
   getAllMessages,
   getSubscriptions,
@@ -27,9 +26,6 @@ import { NULL_MESSAGE, NULL_USER, NULL_SUBSCRIPTION } from '../nullObjects';
 const getMessagesFromChatState = state =>
   state.messages[JSON.stringify(state.narrow || homeNarrow)] || [];
 
-export const getIsFetching = (state: GlobalState): boolean =>
-  (state.app.needsInitialFetch && state.chat.fetchingOlder) || state.chat.fetchingOlder;
-
 export const getMessagesInActiveNarrow = createSelector(
   getAllMessages,
   getActiveNarrow,

src/chat/fetchingReducers.js

@@ -0,0 +1,52 @@
+/* @flow */
+import type { FetchingState, Action } from '../types';
+import {
+  LOGOUT,
+  LOGIN_SUCCESS,
+  ACCOUNT_SWITCH,
+  SWITCH_NARROW,
+  MESSAGE_FETCH_START,
+  MESSAGE_FETCH_SUCCESS,
+} from '../actionConstants';
+import { NULL_FETCHING } from '../nullObjects';
+
+const initialState: FetchingState = {};
+
+export default (state: FetchingState = initialState, action: Action) => {
+  switch (action.type) {
+    case LOGOUT:
+    case LOGIN_SUCCESS:
+    case SWITCH_NARROW:
+    case ACCOUNT_SWITCH:
+      return initialState;
+
+    case MESSAGE_FETCH_START: {
+      const key = JSON.stringify(action.narrow);
+      const currentValue = state[key] || NULL_FETCHING;
+
+      return {
+        ...state,
+        [key]: {
+          older: action.numBefore > 0 || currentValue.older,
+          newer: action.numAfter > 0 || currentValue.newer,
+        },
+      };
+    }
+
+    case MESSAGE_FETCH_SUCCESS: {
+      const key = JSON.stringify(action.narrow);
+      const currentValue = state[key] || NULL_FETCHING;
+
+      return {
+        ...state,
+        [key]: {
+          older: currentValue.older && action.numBefore === 0,
+          newer: currentValue.newer && action.numAfter === 0,
+        },
+      };
+    }
+
+    default:
+      return state;
+  }
+};

src/chat/fetchingSelectors.js

@@ -0,0 +1,17 @@
+/* @flow */
+import { createSelector } from 'reselect';
+
+import { getActiveNarrowString, getApp, getFetching } from '../baseSelectors';
+import { NULL_FETCHING } from '../nullObjects';
+
+export const getFetchingForActiveNarrow = createSelector(
+  getFetching,
+  getActiveNarrowString,
+  (fetching, activeNarrowString) => fetching[activeNarrowString] || NULL_FETCHING,
+);
+
+export const getIsFetching = createSelector(
+  getApp,
+  getFetchingForActiveNarrow,
+  (app, fetching) => (app.needsInitialFetch && fetching.older) || fetching.older,
+);

src/message/MessageListContainer.js

@@ -11,6 +11,8 @@ import {
   getRenderedMessages,
   getActiveNarrow,
   getFlags,
+  getCaughtUpForActiveNarrow,
+  getFetchingForActiveNarrow,
 } from '../selectors';
 import { filterUnreadMessageIds } from '../utils/unread';
 import { registerAppActivity } from '../utils/activity';
@@ -35,10 +37,8 @@ class MessageListContainer extends PureComponent {
   render() {
     const {
       actions,
-      caughtUpOlder,
-      caughtUpNewer,
-      fetchingOlder,
-      fetchingNewer,
+      caughtUp,
+      fetching,
       typingUsers,
       onReplySelect,
       renderedMessages,
@@ -49,10 +49,10 @@ class MessageListContainer extends PureComponent {
     return (
       <MessageList
         actions={actions}
-        caughtUpNewer={caughtUpNewer}
-        caughtUpOlder={caughtUpOlder}
-        fetchingOlder={fetchingOlder}
-        fetchingNewer={fetchingNewer}
+        caughtUpNewer={caughtUp.newer}
+        caughtUpOlder={caughtUp.older}
+        fetchingOlder={fetching.older}
+        fetchingNewer={fetching.newer}
         onReplySelect={onReplySelect}
         typingUsers={typingUsers}
         renderedMessages={renderedMessages}
@@ -67,8 +67,8 @@ class MessageListContainer extends PureComponent {
 
 export default connect(
   state => ({
-    fetchingOlder: state.chat.fetchingOlder,
-    fetchingNewer: state.chat.fetchingNewer,
+    caughtUp: getCaughtUpForActiveNarrow(state),
+    fetching: getFetchingForActiveNarrow(state),
     typingUsers: getCurrentTypingUsers(state),
     renderedMessages: getRenderedMessages(state),
     narrow: getActiveNarrow(state),