Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: zulip/zulip-mobile
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: main
Choose a base ref
...
head repository: zulip/zulip-mobile
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: stable-22
Choose a head ref
  • 4 commits
  • 6 files changed
  • 1 contributor

Commits on Jan 16, 2019

  1. Revert "nav: Remove getTopMostNarrow and tests" 

    This reverts commit 761f007.

For use in the next commit, which reverts the change that deleted the
last user of this code.
    @gnprice
    gnprice committed Jan 16, 2019
    Configuration menu
    Copy the full SHA
    0944db4 View commit details
    Browse the repository at this point in the history

  2. 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.
    @gnprice
    gnprice committed Jan 16, 2019
    Configuration menu
    Copy the full SHA
    99f5404 View commit details
    Browse the repository at this point in the history

  3. changelog: Document bugfix.

    @gnprice
    gnprice committed Jan 16, 2019
    Configuration menu
    Copy the full SHA
    4471906 View commit details
    Browse the repository at this point in the history

  4. version: Bump version to 22.1.108.

    @gnprice
    gnprice committed Jan 16, 2019
    Configuration menu
    Copy the full SHA
    725a542 View commit details
    Browse the repository at this point in the history
Loading