redux: Store parsed ZulipVersion in Redux state, instead of string.

Storing the parsed ZulipVersion instance in Redux makes it easier to
work with, so we don't have to think about constructing a new
instance as part of every server feature check.

The one tricky bit is that our Redux state is serialized by
redux-persist so that it can be stored in ZulipAsyncStorage, but the
ZulipVersion instance itself is not serializable. Thankfully, it can
be fully represented by its raw version string, which is
serializable.

So, on entry into ZulipAsyncStorage, we just have to convert the
ZulipVersion instance into the raw version string, with .raw()
("replace" it), and on exit, make a new ZulipVersion instance, by
calling the constructor with that raw version string ("revive" it).

We considered two main strategies for locating the bit of state to
be transformed (in this case, the `zulipVersion` field, which stores
a ZulipVersion value, in elements of the `state.accounts` array):

1) The "outside-in" strategy, of identifying the value by the
extrinsic property of where it is; i.e., that it's at the field
named 'zulipVersion' on elements of the `state.accounts` array, or

2) The "inside-out" strategy, of identifying the value by its
intrinsic property of being `instanceof ZulipVersion`.

We chose the latter. When we work on zulip#3950, converting our
object-as-map fields to use Map or Immutable.Map, we'll be making
similar, sweeping changes to many different parts of the state, so
it's most natural for the bulk of our logic to be independent of the
location in state, and focus instead on the type of non-serializable
object being stored. This approach conveniently clears the path to
use ImmutableJS for additional reasons discussed later.

An exploration of the "outside-in" approach is left as the un-merged
PR zulip#3952. The main advantage of that approach is that we wouldn't
need to write migrations, but it had the significant drawback of
requiring a lot of code (for locating the bit of state to be
transformed) that was 1) boilerplate, and 2) difficult to get fully
type-checked, which is a bad combination. These concerns were not
sufficiently alleviated by the complexity of that boilerplate being
bounded by the complexity of the reducer(s) in charge of the
corresponding part(s) of the state: it's better just to not have to
write the boilerplate.

There's nothing stopping us from mixing the two approaches, in
future, but it would be nice to stick to one as far as possible, for
simplicity.

For the "inside-out" implementation, we use `remotedev-serialize`
(added in a recent commit) [1], with custom replacer and reviver
functions, defined in the previous commit. As Greg explains at
zulip#3952 (comment):

"""
* at save time, values that are `instanceof ZulipVersion` get turned
into objects like `{__serializedType__: 'ZulipVersion', data:
'2.0.0-rc1'}`, before being JSON-serialized

* at load time, after JSON deserialization, values that are objects
with a `__serializedType__` property get transformed in a way
identified by that property's value: in particular if
`__serializedType__: 'ZulipVersion'`, then `data` is fed to `new
ZulipVersion`
"""

Since we've never dealt with the Zulip version in this "live" (i.e.,
non-serializable) form, we have to write a migration. This was
straightforward, but we MUST remember to write this kind of
migration in the future.

For making a value "live", where it wasn't before, the migration
needs to:

1) As input, take the previous shape of the data. Don't confuse this
with the *current* way of storing the "dead" shape. Just like any
other migration, the previous shape is the input.

2) As output, give the "live" form of the data. Once it's in Redux,
the replacer will take care of persisting it in the correct "dead"
form.

As mentioned above, this approach further clears the way for
ImmutableJS; zulip#3949 and zulip#3950 will be much easier after this. I've
neglected to mention that the primary purpose of
`remotedev-serialize` is to replace and revive ImmutableJS objects!
Its "default" replacers and revivers do this. The custom
replacer/reviver functions we use here are a nice feature provided
on the side.

(We added `immutable` as a dependency in a recent commit, since
`remotedev-serialize` depends on it.)

Fixes: zulip#3951

[1]: We actually use our own fork,
`zulip/remotedev-serialize@5f9f759a4`, which fixes a bug where
"__serializedType__" keys in data fail to round-trip properly.

src/__tests__/lib/exampleData.js

@@ -5,6 +5,7 @@ import { createStore } from 'redux';
 import type { CrossRealmBot, Message, PmRecipientUser, Stream, User } from '../../api/modelTypes';
 import type { Action, GlobalState, RealmState } from '../../reduxTypes';
 import type { Auth, Account, Outbox } from '../../types';
+import { ZulipVersion } from '../../utils/zulipVersion';
 import {
   ACCOUNT_SWITCH,
   LOGIN_SUCCESS,
@@ -112,15 +113,15 @@ export const makeCrossRealmBot = (args: { name?: string } = {}): CrossRealmBot =
 
 export const realm = 'https://zulip.example.org';
 
-export const zulipVersion = '2.1.0-234-g7c3acf4';
+export const zulipVersion = new ZulipVersion('2.1.0-234-g7c3acf4');
 
 export const makeAccount = (
   args: {
     user?: User,
     email?: string,
     realm?: string,
     apiKey?: string,
-    zulipVersion?: string | null,
+    zulipVersion?: ZulipVersion | null,
     ackedPushToken?: string | null,
   } = {},
 ): Account => {

src/account/__tests__/accountsReducer-test.js

@@ -10,6 +10,7 @@ import {
   ACCOUNT_REMOVE,
 } from '../../actionConstants';
 import accountsReducer from '../accountsReducer';
+import { ZulipVersion } from '../../utils/zulipVersion';
 
 import * as eg from '../../__tests__/lib/exampleData';
 
@@ -77,7 +78,7 @@ describe('accountsReducer', () => {
 
     test('records zulipVersion on active account', () => {
       const prevState = deepFreeze([account1, account2, account3]);
-      const newZulipVersion = '2.0.0';
+      const newZulipVersion = new ZulipVersion('2.0.0');
       const action = deepFreeze({ ...eg.action.realm_init, zulipVersion: newZulipVersion });
 
       const expectedState = [{ ...account1, zulipVersion: newZulipVersion }, account2, account3];

src/account/accountActions.js

@@ -7,13 +7,14 @@ import {
   LOGIN_SUCCESS,
   LOGOUT,
 } from '../actionConstants';
+import type { ZulipVersion } from '../utils/zulipVersion';
 
 export const switchAccount = (index: number): Action => ({
   type: ACCOUNT_SWITCH,
   index,
 });
 
-export const realmAdd = (realm: string, zulipVersion: string): Action => ({
+export const realmAdd = (realm: string, zulipVersion: ZulipVersion): Action => ({
   type: REALM_ADD,
   realm,
   zulipVersion,

src/account/accountsSelectors.js

@@ -162,5 +162,5 @@ export const getServerVersion = (state: GlobalState): ZulipVersion | null => {
   if (activeAccount.zulipVersion === null) {
     return null;
   }
-  return new ZulipVersion(activeAccount.zulipVersion);
+  return activeAccount.zulipVersion;
 };

src/actionTypes.js

@@ -86,6 +86,7 @@ import type {
   AlertWordsState,
   UserStatusEvent,
 } from './types';
+import type { ZulipVersion } from './utils/zulipVersion';
 
 export type { NavigationAction } from 'react-navigation';
 
@@ -154,7 +155,7 @@ type AccountSwitchAction = {|
 type RealmAddAction = {|
   type: typeof REALM_ADD,
   realm: string,
-  zulipVersion: string,
+  zulipVersion: ZulipVersion,
 |};
 
 type AccountRemoveAction = {|
@@ -176,7 +177,7 @@ type LogoutAction = {|
 type RealmInitAction = {|
   type: typeof REALM_INIT,
   data: InitialData,
-  zulipVersion: string,
+  zulipVersion: ZulipVersion,
 |};
 
 /** We learned the device token from the system.  See `SessionState`. */

src/boot/store.js

@@ -6,8 +6,8 @@ import * as Serialize from 'remotedev-serialize';
 import { persistStore, autoRehydrate } from '../third/redux-persist';
 import type { Config } from '../third/redux-persist';
 
-import type { Action, GlobalState } from '../types';
 import { ZulipVersion } from '../utils/zulipVersion';
+import type { Action, GlobalState } from '../types';
 import rootReducer from './reducers';
 import middleware from './middleware';
 import ZulipAsyncStorage from './ZulipAsyncStorage';
@@ -153,6 +153,16 @@ const migrations: { [string]: (GlobalState) => GlobalState } = {
       zulipVersion: a.zulipVersion !== undefined ? a.zulipVersion : null,
     })),
   }),
+
+  // Accounts.zulipVersion is now ZulipVersion | null
+  '13': state => ({
+    ...state,
+    accounts: state.accounts.map(a => ({
+      ...a,
+      zulipVersion:
+        typeof a.zulipVersion === 'string' ? new ZulipVersion(a.zulipVersion) : a.zulipVersion,
+    })),
+  }),
 };
 
 /**

src/message/fetchActions.js

@@ -34,6 +34,7 @@ import { addToOutbox, sendOutbox } from '../outbox/outboxActions';
 import { realmInit } from '../realm/realmActions';
 import { startEventPolling } from '../events/eventActions';
 import { logout } from '../account/accountActions';
+import { ZulipVersion } from '../utils/zulipVersion';
 
 const messageFetchStart = (narrow: Narrow, numBefore: number, numAfter: number): Action => ({
   type: MESSAGE_FETCH_START,
@@ -249,7 +250,7 @@ export const doInitialFetch = () => async (dispatch: Dispatch, getState: GetStat
     return;
   }
 
-  dispatch(realmInit(initData, serverSettings.zulip_version));
+  dispatch(realmInit(initData, new ZulipVersion(serverSettings.zulip_version)));
   dispatch(fetchTopMostNarrow());
   dispatch(initialFetchComplete());
   dispatch(startEventPolling(initData.queue_id, initData.last_event_id));

src/realm/realmActions.js

@@ -1,8 +1,9 @@
 /* @flow strict-local */
 import type { InitialData, Action } from '../types';
+import type { ZulipVersion } from '../utils/zulipVersion';
 import { REALM_INIT } from '../actionConstants';
 
-export const realmInit = (data: InitialData, zulipVersion: string): Action => ({
+export const realmInit = (data: InitialData, zulipVersion: ZulipVersion): Action => ({
   type: REALM_INIT,
   data,
   zulipVersion,

src/start/RealmScreen.js

@@ -3,6 +3,7 @@ import React, { PureComponent } from 'react';
 import { ScrollView, Keyboard } from 'react-native';
 import type { NavigationScreenProp } from 'react-navigation';
 
+import { ZulipVersion } from '../utils/zulipVersion';
 import type { ApiResponseServerSettings, Dispatch } from '../types';
 import { connect } from '../react-redux';
 import { ErrorMsg, Label, SmartUrlInput, Screen, ZulipButton } from '../common';
@@ -54,7 +55,7 @@ class RealmScreen extends PureComponent<Props, State> {
 
     try {
       const serverSettings: ApiResponseServerSettings = await api.getServerSettings(realm);
-      dispatch(realmAdd(realm, serverSettings.zulip_version));
+      dispatch(realmAdd(realm, new ZulipVersion(serverSettings.zulip_version)));
       dispatch(navigateToAuth(serverSettings));
       Keyboard.dismiss();
     } catch (err) {

src/types.js

@@ -4,6 +4,7 @@ import type { DangerouslyImpreciseStyleProp } from 'react-native/Libraries/Style
 
 import type { Auth, Topic, Message, Reaction, ReactionType, Narrow } from './api/apiTypes';
 import type { AppStyles } from './styles/theme';
+import type { ZulipVersion } from './utils/zulipVersion';
 
 export type * from './reduxTypes';
 export type * from './api/apiTypes';
@@ -92,7 +93,7 @@ export type Account = {|
    *    "crunchy shell" pattern (see docs/architecture/crunchy-shell.md);
    *  * context data in Sentry reports.
    */
-  zulipVersion: string | null,
+  zulipVersion: ZulipVersion | null,
 
   /**
    * The last device token value the server has definitely heard from us.