From f1f041a8351608bae7bdd0a4616cd763e981d6e5 Mon Sep 17 00:00:00 2001
From: Chris Bobbe <cbobbe@zulip.com>
Date: Mon, 5 Oct 2020 17:27:40 -0700
Subject: [PATCH] navReducer: Use `getNavigationRoutes`.

Also add a comment, with Greg's description [1] of what's going on here.

We have this handy function for grabbing what we want from the
navigation state, so we might as well use it. When we use this
function, the navigation state is gotten from `NavigationService`,
not from Redux, which moves us closer to #3804.

To be more rigorous, though, we wouldn't want this to stay this way
in `navReduce` forever: reducers are supposed to be pure functions
of state and actions. But the incorrectness isn't obviously harmful
here, and we're about to transplant this code somewhere else, and
it's helpful to separate a small tweak like this from the commit
where we move the code.

[1] https://github.com/zulip/zulip-mobile/pull/4274#discussion_r500688470
---
 src/nav/__tests__/navReducer-test.js |  7 +++++++
 src/nav/navReducer.js                | 12 +++++++++++-
 src/nav/navSelectors.js              |  2 +-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/nav/__tests__/navReducer-test.js b/src/nav/__tests__/navReducer-test.js
index 2b468f38cdb..8b3215b0538 100644
--- a/src/nav/__tests__/navReducer-test.js
+++ b/src/nav/__tests__/navReducer-test.js
@@ -2,10 +2,17 @@ import deepFreeze from 'deep-freeze';
 
 import { INITIAL_FETCH_COMPLETE } from '../../actionConstants';
 import navReducer, { getStateForRoute } from '../navReducer';
+import NavigationService from '../NavigationService';
 
 describe('navReducer', () => {
   describe('INITIAL_FETCH_COMPLETE', () => {
     test('do not mutate navigation state if already at the same route', () => {
+      NavigationService.getState = jest.fn().mockReturnValue(
+        deepFreeze({
+          index: 0,
+          routes: [{ routeName: 'main' }],
+        }),
+      );
       const prevState = getStateForRoute('main');
 
       const action = deepFreeze({
diff --git a/src/nav/navReducer.js b/src/nav/navReducer.js
index 5375e10bcc4..4db70b7a7c3 100644
--- a/src/nav/navReducer.js
+++ b/src/nav/navReducer.js
@@ -2,6 +2,7 @@
 import type { NavigationAction } from 'react-navigation';
 
 import type { NavigationState, Action } from '../types';
+import { getNavigationRoutes } from './navSelectors';
 import AppNavigator from './AppNavigator';
 import { INITIAL_FETCH_COMPLETE } from '../actionConstants';
 
@@ -29,7 +30,16 @@ export const initialState = getStateForRoute('loading');
 export default (state: NavigationState = initialState, action: Action): NavigationState => {
   switch (action.type) {
     case INITIAL_FETCH_COMPLETE:
-      return state.routes[0].routeName === 'main' ? state : getStateForRoute('main');
+      // If we're anywhere in the normal UI of the app, then remain
+      // where we are. Only reset the nav state if we're elsewhere,
+      // and in that case, go to the main screen.
+      //
+      // TODO: "elsewhere" is probably just a way of saying "on the
+      // loading screen", but we're not sure. We could adjust the
+      // conditional accordingly, if we found out we're not depending on
+      // the more general condition; see
+      //   https://github.com/zulip/zulip-mobile/pull/4274#discussion_r505941875
+      return getNavigationRoutes()[0].routeName === 'main' ? state : getStateForRoute('main');
 
     default: {
       // The `react-navigation` libdef says this only takes a NavigationAction,
diff --git a/src/nav/navSelectors.js b/src/nav/navSelectors.js
index 44ef8084817..dce36f6bc65 100644
--- a/src/nav/navSelectors.js
+++ b/src/nav/navSelectors.js
@@ -5,7 +5,7 @@ import NavigationService from './NavigationService';
 
 export const getNavState = (): NavigationState => NavigationService.getState();
 
-const getNavigationRoutes = () => getNavState().routes;
+export const getNavigationRoutes = () => getNavState().routes;
 
 const getNavigationIndex = () => getNavState().index;