Skip to content

Commit 99f5404

Browse files
committed
fetch: Revert "Remove unused message fetch code".
Fixes #3284. In commit 20.0.103~40 aka 1d05ecd "fetch: Remove unused message fetch code in fetchEssentialInitialData", we removed some old code that had been intended to fetch the messages found in the current narrow. We reasoned at the time that (a) this code no longer ever did anything, because `getTopMostNarrow` would always return `undefined`, because we don't persist the nav state (as we used to long ago); (b) in the one case where we don't start on the main nav screen when a logged-in user starts the app -- namely a notification -- we did that via a `doNarrow`, which takes care of the fetch. So this code's job was already being done a different way. In fact, it turns out this code was filling an essential role in making the experience of "start Zulip by following a notification" work at all. Even though its intended design no longer made sense! On (a): this code runs in the initial fetch, which is triggered *after* rehydrate; and *during* rehydrate, `navReducers` takes us to the narrow for the initial notification, if there is one. So there is a topmost narrow here in that case. On (b): the `doNarrow` in question happens very early: dispatched from `handlePendingNotifications`, called from `handleInitialNotification`, which is called at the top of the `componentDidMount` of `AppEventHandlers`. At this stage, it's likely (perhaps certain) we *haven't* rehydrated yet... and `doNarrow` has a conditional to bail out and do nothing if we aren't rehydrated. As a result, removing this meant that tapping a notification, if Zulip wasn't already running, showed a "No messages" screen. Ouch. As a minimal fix for now, put this logic back. The new version is a bit shorter than the old, using the `fetchMessagesInNarrow` helper to conform to surrounding code in the current codebase. A more literal rendition of the old version didn't work; I didn't fully debug, in the interest of getting this out. This code still isn't the right design for handling this; I wouldn't merge something like this if it didn't fix an urgent critical bug. We'll want to sort this logic out better sometime soon.
1 parent 0944db4 commit 99f5404

File tree

1 file changed

+11
-0
lines changed

1 file changed

+11
-0
lines changed

src/message/fetchActions.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
getLastMessageId,
2020
getCaughtUpForActiveNarrow,
2121
getFetchingForActiveNarrow,
22+
getTopMostNarrow,
2223
} from '../selectors';
2324
import config from '../config';
2425
import {
@@ -171,6 +172,15 @@ export const fetchStreams = () => async (dispatch: Dispatch, getState: GetState)
171172
dispatch(initStreams(streams));
172173
};
173174

175+
const fetchTopMostNarrow = () => async (dispatch: Dispatch, getState: GetState) => {
176+
// only fetch messages if chat screen is at the top of stack
177+
// get narrow of top most chat screen in the stack
178+
const narrow = getTopMostNarrow(getState());
179+
if (narrow) {
180+
dispatch(fetchMessagesInNarrow(narrow));
181+
}
182+
};
183+
174184
export const fetchInitialData = () => async (dispatch: Dispatch, getState: GetState) => {
175185
dispatch(initialFetchStart());
176186
const auth = getAuth(getState());
@@ -185,6 +195,7 @@ export const fetchInitialData = () => async (dispatch: Dispatch, getState: GetSt
185195
);
186196

187197
dispatch(realmInit(initData));
198+
dispatch(fetchTopMostNarrow());
188199
dispatch(initialFetchComplete());
189200
dispatch(startEventPolling(initData.queue_id, initData.last_event_id));
190201

0 commit comments

Comments
 (0)