zulip / zulip-mobile / 4132

65%

push

travis-ci

fetch: Revert "Remove unused message fetch code".

Fixes #3284.

In commit 20.0.103~40 aka 1d05ecdb8
"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... (continued)

1401 of 2912 branches covered (48.11%)

Branch coverage included in aggregate %.

4967 of 6724 relevant lines covered (73.87%)

16.41 hits per line