From cbcf00c6599878b4e74c5bb1a7f9642a807c3178 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulipchat.com>
Date: Fri, 8 Jan 2021 15:09:52 -0800
Subject: [PATCH 01/11] recipient [nfc]: Document pmKeyRecipientsFromIds's
 error-handling.

---
 src/utils/recipient.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/utils/recipient.js b/src/utils/recipient.js
index d971e86ee32..0001f45c1d4 100644
--- a/src/utils/recipient.js
+++ b/src/utils/recipient.js
@@ -195,6 +195,8 @@ export const pmKeyRecipientsFromMessage = (
  *
  * The input may either include or exclude self, without affecting the
  * result.
+ *
+ * Returns null when a user couldn't be found in the given `allUsersById`.
  */
 export const pmKeyRecipientsFromIds = (
   userIds: number[],

From 232444ae9bd37e6f07cb7e612d37755a01c2f314 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulipchat.com>
Date: Fri, 8 Jan 2021 16:01:57 -0800
Subject: [PATCH 02/11] recipient: Log a warning if pmKeyRecipientsFromIds has
 to bail.

---
 src/utils/recipient.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/utils/recipient.js b/src/utils/recipient.js
index 0001f45c1d4..bacd7dfe808 100644
--- a/src/utils/recipient.js
+++ b/src/utils/recipient.js
@@ -1,8 +1,9 @@
 /* @flow strict-local */
 import invariant from 'invariant';
 import isEqual from 'lodash.isequal';
-import { mapOrNull } from '../collections';
 
+import { mapOrNull } from '../collections';
+import * as logging from './logging';
 import type { PmRecipientUser, Message, Outbox, User, UserOrBot } from '../types';
 
 /** The stream name a stream message was sent to.  Throws if a PM. */
@@ -210,6 +211,7 @@ export const pmKeyRecipientsFromIds = (
 
   const users = mapOrNull(resultIds, id => allUsersById.get(id));
   if (!users) {
+    logging.warn('pmKeyRecipientsFromIds: missing data on user');
     return null;
   }
   return users.sort((a, b) => a.user_id - b.user_id);

From 4fb15871af7fb2217341189c52437e1d95abafd7 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulipchat.com>
Date: Wed, 6 Jan 2021 16:16:34 -0800
Subject: [PATCH 03/11] replaceRevive: Add support for Immutable.List.

We'll start using these shortly, as part of the data structure for
`recent_private_conversations`.
---
 src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap | 2 ++
 src/boot/__tests__/replaceRevive-test.js                    | 1 +
 src/boot/replaceRevive.js                                   | 4 ++++
 3 files changed, 7 insertions(+)

diff --git a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap
index 1da54cef8db..dd48f3863dc 100644
--- a/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap
+++ b/src/boot/__tests__/__snapshots__/replaceRevive-test.js.snap
@@ -4,6 +4,8 @@ exports[`Stringify fallbackAvatarURL 1`] = `"{\\"data\\":\\"https://zulip.exampl
 
 exports[`Stringify gravatarURL 1`] = `"{\\"data\\":\\"https://secure.gravatar.com/avatar/3b01d0f626dc6944ed45dbe6c86d3e30?d=identicon\\",\\"__serializedType__\\":\\"GravatarURL\\"}"`;
 
+exports[`Stringify list 1`] = `"{\\"data\\":[1,2,\\"a\\",null],\\"__serializedType__\\":\\"ImmutableList\\"}"`;
+
 exports[`Stringify map 1`] = `"{\\"data\\":{\\"a\\":1,\\"b\\":2,\\"c\\":3,\\"d\\":4},\\"__serializedType__\\":\\"ImmutableMap\\"}"`;
 
 exports[`Stringify mapWithTypeKey 1`] = `"{\\"data\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"a\\":1},\\"__serializedType__value\\":{\\"__serializedType__\\":\\"Object\\",\\"data\\":{\\"b\\":[2]},\\"__serializedType__value\\":{\\"c\\":[3]}}},\\"__serializedType__\\":\\"ImmutableMap\\"}"`;
diff --git a/src/boot/__tests__/replaceRevive-test.js b/src/boot/__tests__/replaceRevive-test.js
index ad3d95630a8..fdce976eb80 100644
--- a/src/boot/__tests__/replaceRevive-test.js
+++ b/src/boot/__tests__/replaceRevive-test.js
@@ -10,6 +10,7 @@ import { stringify, parse, SERIALIZED_TYPE_FIELD_NAME } from '../replaceRevive';
 import * as eg from '../../__tests__/lib/exampleData';
 
 const data = {
+  list: Immutable.List([1, 2, 'a', null]),
   map: Immutable.Map({ a: 1, b: 2, c: 3, d: 4 }),
   mapWithTypeKey: Immutable.Map({
     a: 1,
diff --git a/src/boot/replaceRevive.js b/src/boot/replaceRevive.js
index ab39432d264..c7269d20c52 100644
--- a/src/boot/replaceRevive.js
+++ b/src/boot/replaceRevive.js
@@ -73,6 +73,8 @@ function replacer(key, value) {
         data: FallbackAvatarURL.serialize(value),
         [SERIALIZED_TYPE_FIELD_NAME]: 'FallbackAvatarURL',
       };
+    case (Immutable.List.prototype: $FlowFixMe):
+      return { data: value, [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableList' };
     case (Immutable.Map.prototype: $FlowFixMe):
       return { data: value, [SERIALIZED_TYPE_FIELD_NAME]: 'ImmutableMap' };
     default: {
@@ -132,6 +134,8 @@ function reviver(key, value) {
         return UploadedAvatarURL.deserialize(data);
       case 'FallbackAvatarURL':
         return FallbackAvatarURL.deserialize(data);
+      case 'ImmutableList':
+        return Immutable.List(data);
       case 'ImmutableMap':
         return Immutable.Map(data);
       case 'Object':

From 3cf2f854328349eb2124149fb8ca7ab1a44d692b Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulipchat.com>
Date: Thu, 7 Jan 2021 17:59:19 -0800
Subject: [PATCH 04/11] actions: Add ownUserId to FETCH_MESSAGES_COMPLETE.

We'll use this in a new reducer shortly.

There's a case where this can potentially change behavior: the thunk
actions that dispatch these actions do so after an `await`, which
means that by the time they get to calling `getOwnUserId` (after their
HTTP requests have returned) the user may have logged out, and it
might throw with "No server data found".

This is a case where we already have a worse bug, in that if the user
has switched to a *different* account and completed the initial fetch
for it by the time this request returns, then we'll be adding these
messages to our state to display alongside the messages that belong in
that account.  (Or try to, anyway: if the user IDs and stream IDs and
so on don't exist in the other account's realm, then we'll likely just
crash.)  We're aware of that bug and intend to fix it, but haven't
done the work for it yet.

See discussion:
  https://github.com/zulip/zulip-mobile/pull/4392#discussion_r554035895
---
 src/__tests__/lib/exampleData.js           |  1 +
 src/actionTypes.js                         |  1 +
 src/chat/__tests__/narrowsReducer-test.js  |  5 +++++
 src/message/__tests__/fetchActions-test.js |  4 ++++
 src/message/fetchActions.js                | 17 +++++++++++++++--
 5 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/__tests__/lib/exampleData.js b/src/__tests__/lib/exampleData.js
index 08748667bc2..1e3d52c76da 100644
--- a/src/__tests__/lib/exampleData.js
+++ b/src/__tests__/lib/exampleData.js
@@ -592,6 +592,7 @@ export const action = deepFreeze({
     numAfter: 50,
     foundNewest: undefined,
     foundOldest: undefined,
+    ownUserId: selfUser.user_id,
   },
 });
 
diff --git a/src/actionTypes.js b/src/actionTypes.js
index 29de5566a1c..34025284dbc 100644
--- a/src/actionTypes.js
+++ b/src/actionTypes.js
@@ -220,6 +220,7 @@ type MessageFetchCompleteAction = {|
   numAfter: number,
   foundNewest: boolean | void,
   foundOldest: boolean | void,
+  ownUserId: number,
 |};
 
 type InitialFetchStartAction = {|
diff --git a/src/chat/__tests__/narrowsReducer-test.js b/src/chat/__tests__/narrowsReducer-test.js
index bd7d05b0fc3..6014d2938db 100644
--- a/src/chat/__tests__/narrowsReducer-test.js
+++ b/src/chat/__tests__/narrowsReducer-test.js
@@ -379,6 +379,7 @@ describe('narrowsReducer', () => {
         numAfter: 100,
         foundNewest: false,
         foundOldest: false,
+        ownUserId: eg.selfUser.user_id,
       });
 
       const expectedState = Immutable.Map({
@@ -407,6 +408,7 @@ describe('narrowsReducer', () => {
         numAfter: 100,
         foundNewest: false,
         foundOldest: false,
+        ownUserId: eg.selfUser.user_id,
       });
 
       const expectedState = Immutable.Map({
@@ -434,6 +436,7 @@ describe('narrowsReducer', () => {
         numAfter: 100,
         foundNewest: false,
         foundOldest: false,
+        ownUserId: eg.selfUser.user_id,
       });
 
       const expectedState = Immutable.Map({
@@ -461,6 +464,7 @@ describe('narrowsReducer', () => {
         numAfter: 100,
         foundNewest: false,
         foundOldest: false,
+        ownUserId: eg.selfUser.user_id,
       });
 
       const expectedState = Immutable.Map({
@@ -487,6 +491,7 @@ describe('narrowsReducer', () => {
         numAfter: 0,
         foundNewest: true,
         foundOldest: false,
+        ownUserId: eg.selfUser.user_id,
       });
 
       const expectedState = Immutable.Map({
diff --git a/src/message/__tests__/fetchActions-test.js b/src/message/__tests__/fetchActions-test.js
index f53bcf1f7ef..06e34d46b8e 100644
--- a/src/message/__tests__/fetchActions-test.js
+++ b/src/message/__tests__/fetchActions-test.js
@@ -133,6 +133,7 @@ describe('fetchActions', () => {
 
     const baseState = eg.reduxState({
       accounts: [eg.makeAccount()],
+      realm: { ...eg.baseReduxState.realm, user_id: eg.selfUser.user_id },
       narrows: Immutable.Map({
         [streamNarrowStr]: [message1.id],
       }),
@@ -299,6 +300,7 @@ describe('fetchActions', () => {
       const store = mockStore<GlobalState, Action>(
         eg.reduxState({
           accounts: [eg.selfAccount],
+          realm: { ...eg.baseReduxState.realm, user_id: eg.selfUser.user_id },
           narrows: Immutable.Map({
             [streamNarrowStr]: [1],
           }),
@@ -324,6 +326,7 @@ describe('fetchActions', () => {
       const store = mockStore<GlobalState, Action>(
         eg.reduxState({
           accounts: [eg.selfAccount],
+          realm: { ...eg.baseReduxState.realm, user_id: eg.selfUser.user_id },
           narrows: Immutable.Map({
             [streamNarrowStr]: [1],
           }),
@@ -348,6 +351,7 @@ describe('fetchActions', () => {
       const store = mockStore<GlobalState, Action>(
         eg.reduxState({
           accounts: [eg.selfAccount],
+          realm: { ...eg.baseReduxState.realm, user_id: eg.selfUser.user_id },
           narrows: Immutable.Map({
             [streamNarrowStr]: [1],
           }),
diff --git a/src/message/fetchActions.js b/src/message/fetchActions.js
index d234f47c2ba..e58c1796137 100644
--- a/src/message/fetchActions.js
+++ b/src/message/fetchActions.js
@@ -32,7 +32,7 @@ import { realmInit } from '../realm/realmActions';
 import { startEventPolling } from '../events/eventActions';
 import { logout } from '../account/accountActions';
 import { ZulipVersion } from '../utils/zulipVersion';
-import { getAllUsersById } from '../users/userSelectors';
+import { getAllUsersById, getOwnUserId } from '../users/userSelectors';
 
 const messageFetchStart = (narrow: Narrow, numBefore: number, numAfter: number): Action => ({
   type: MESSAGE_FETCH_START,
@@ -58,8 +58,18 @@ const messageFetchComplete = (args: {|
   numAfter: number,
   foundNewest?: boolean,
   foundOldest?: boolean,
+  ownUserId: number,
 |}): Action => {
-  const { messages, narrow, anchor, numBefore, numAfter, foundNewest, foundOldest } = args;
+  const {
+    messages,
+    narrow,
+    anchor,
+    numBefore,
+    numAfter,
+    foundNewest,
+    foundOldest,
+    ownUserId,
+  } = args;
   return {
     type: MESSAGE_FETCH_COMPLETE,
     messages,
@@ -69,6 +79,7 @@ const messageFetchComplete = (args: {|
     numAfter,
     foundNewest,
     foundOldest,
+    ownUserId,
   };
 };
 
@@ -98,6 +109,7 @@ export const fetchMessages = (fetchArgs: {|
         messages,
         foundNewest: found_newest,
         foundOldest: found_oldest,
+        ownUserId: getOwnUserId(getState()),
       }),
     );
     return messages;
@@ -251,6 +263,7 @@ const fetchPrivateMessages = () => async (dispatch: Dispatch, getState: GetState
       numAfter: 0,
       foundNewest: found_newest,
       foundOldest: found_oldest,
+      ownUserId: getOwnUserId(getState()),
     }),
   );
 };

From 0ca375eb4f4507c364ef34ce4531a65897788304 Mon Sep 17 00:00:00 2001
From: Isham Mahajan <mahajan.isham@gmail.com>
Date: Sun, 30 Jun 2019 13:29:18 +0530
Subject: [PATCH 05/11] recent-pms: Add `recent_private_conversations` key to
 `/register` request.

The server added in zulip/zulip#11944 support for providing a summary
of the most recent PM conversations that a user was involved in, as
part of the `/register` response.

This commit adds only the minimum machinery necessary to request this
data from the server at `/register` time.

[ray: Split original initial commit into two parts, this being the
  first. Minor changes to Flow types.]
[greg: Squashed in a few bits of second commit; updated a comment;
  shortened commit message.]
---
 src/__tests__/lib/exampleData.js |  1 +
 src/api/initialDataTypes.js      | 10 ++++++++++
 src/api/modelTypes.js            | 26 ++++++++++++++++++++++++++
 src/config.js                    |  1 +
 4 files changed, 38 insertions(+)

diff --git a/src/__tests__/lib/exampleData.js b/src/__tests__/lib/exampleData.js
index 1e3d52c76da..8248429611a 100644
--- a/src/__tests__/lib/exampleData.js
+++ b/src/__tests__/lib/exampleData.js
@@ -539,6 +539,7 @@ export const action = deepFreeze({
       realm_users: [],
       user_id: 4,
       realm_user_groups: [],
+      recent_private_conversations: [],
       streams: [],
       never_subscribed: [],
       subscriptions: [],
diff --git a/src/api/initialDataTypes.js b/src/api/initialDataTypes.js
index 81718952b87..05cefb7ded4 100644
--- a/src/api/initialDataTypes.js
+++ b/src/api/initialDataTypes.js
@@ -4,6 +4,7 @@ import type {
   CrossRealmBot,
   RealmEmojiById,
   RealmFilter,
+  RecentPrivateConversation,
   Stream,
   Subscription,
   User,
@@ -129,6 +130,14 @@ export type InitialDataRealmUserGroups = {|
   realm_user_groups?: UserGroup[],
 |};
 
+export type InitialDataRecentPmConversations = {|
+  // * Added in server commit 2.1-dev-384-g4c3c669b41.
+  // * `user_id` fields are sorted as of commit 2.2-dev-53-g405a529340, which
+  //    was backported to 2.1.1-50-gd452ad31e0 -- meaning that they are _not_
+  //    sorted in either v2.1.0 or v2.1.1.
+  recent_private_conversations?: RecentPrivateConversation[],
+|};
+
 type NeverSubscribedStream = {|
   description: string,
   invite_only: boolean,
@@ -307,6 +316,7 @@ export type InitialData = {|
   ...InitialDataRealmFilters,
   ...InitialDataRealmUser,
   ...InitialDataRealmUserGroups,
+  ...InitialDataRecentPmConversations,
   ...InitialDataStream,
   ...InitialDataSubscription,
   ...InitialDataUpdateDisplaySettings,
diff --git a/src/api/modelTypes.js b/src/api/modelTypes.js
index 2cf4777e2f9..956d703155e 100644
--- a/src/api/modelTypes.js
+++ b/src/api/modelTypes.js
@@ -579,3 +579,29 @@ export type Message = $ReadOnly<{|
   subject: string,
   subject_links: $ReadOnlyArray<string>,
 |}>;
+
+//
+//
+//
+// ===================================================================
+// Summaries of messages and conversations.
+//
+//
+
+/**
+ * Describes a single recent PM conversation.
+ *
+ * See API documentation under `recent_private_conversations` at:
+ *   https://chat.zulip.org/api/register-queue
+ *
+ * Note that `user_ids` does not contain the `user_id` of the current user.
+ * Consequently, a user's conversation with themselves will be listed here
+ * as [], which is unlike the behaviour found in some other parts of the
+ * codebase.
+ */
+export type RecentPrivateConversation = {|
+  max_message_id: number,
+  // When received from the server, these are guaranteed to be sorted only after
+  // 2.2-dev-53-g405a529340. To be safe, we always sort them on receipt.
+  user_ids: number[],
+|};
diff --git a/src/config.js b/src/config.js
index ccc0610c6e9..9c69b4ff598 100644
--- a/src/config.js
+++ b/src/config.js
@@ -33,6 +33,7 @@ const config: Config = {
     'realm_filters',
     'realm_user',
     'realm_user_groups',
+    'recent_private_conversations',
     'stream',
     'subscription',
     'update_display_settings',

From 7c697fb74d312975a9df649d088f4ae712a7a381 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulipchat.com>
Date: Wed, 6 Jan 2021 12:50:17 -0800
Subject: [PATCH 06/11] recent-pms: Add a model file, including state type and
 reducer.

This takes the data from `recent_private_conversations` (which we
just started requesting in the previous commit) in the initial fetch,
and then maintains it as new messages arrive.

We make good use of our new support for Immutable.js data structures.
Instead of maintaining the data in the exact same format as it came
over the wire as JSON, we arrange it on receipt into a data structure
that's optimized for efficiently maintaining and using.

The organization of the code is slightly different from our usual
existing practice:

 * The state type is defined right in the same file as the reducer
   which updates it.

 * Also in that file are some related stateless helper functions:
   in particular `usersOfKey`, which code elsewhere in the app can
   use to help interpret the data provided by this model.

 * The file is named in a general way like "thingModel", rather than
   our usual "thingReducer", and the reducer is a named rather than
   default export.

I think this is helpful here for keeping together code that
logically goes together in that it has shared internal knowledge
and invariants, like the format of PmConversationKey strings.

I think something similar would be helpful for a lot of our model
code (including most of what's in reducers and selectors), but I'm
in no rush to do a sweeping refactor; we'll want to experiment first
and find the arrangement that works best.

This version doesn't put any selector code in this file.  The
existing selector in the neighboring `pmConversationsSelectors.js`
doesn't really belong here, because it mixes together a number of
different areas of the app model, notably the unread counts.  After
some future refactoring to separate that out, there may be a more
focused selector that naturally lives here... or it may be that no
selector is needed, because it'd just be accessing a property on the
state type, and we can simplify by just saying that directly.
---
 src/boot/reducers.js                          |   2 +
 src/boot/store.js                             |   2 +-
 .../__tests__/pmConversationsModel-test.js    | 144 +++++++++++++
 src/pm-conversations/pmConversationsModel.js  | 190 ++++++++++++++++++
 src/reduxTypes.js                             |   2 +
 5 files changed, 339 insertions(+), 1 deletion(-)
 create mode 100644 src/pm-conversations/__tests__/pmConversationsModel-test.js
 create mode 100644 src/pm-conversations/pmConversationsModel.js

diff --git a/src/boot/reducers.js b/src/boot/reducers.js
index cf9e95dc606..61f5b3d86e3 100644
--- a/src/boot/reducers.js
+++ b/src/boot/reducers.js
@@ -18,6 +18,7 @@ import narrows from '../chat/narrowsReducer';
 import messages from '../message/messagesReducer';
 import mute from '../mute/muteReducer';
 import outbox from '../outbox/outboxReducer';
+import { reducer as pmConversations } from '../pm-conversations/pmConversationsModel';
 import presence from '../presence/presenceReducer';
 import realm from '../realm/realmReducer';
 import session from '../session/sessionReducer';
@@ -46,6 +47,7 @@ const reducers = {
   narrows,
   mute,
   outbox,
+  pmConversations,
   presence,
   realm,
   session,
diff --git a/src/boot/store.js b/src/boot/store.js
index b8c2fdba5d6..3d7dda1a6af 100644
--- a/src/boot/store.js
+++ b/src/boot/store.js
@@ -67,7 +67,7 @@ export const storeKeys: Array<$Keys<GlobalState>> = [
  */
 // prettier-ignore
 export const cacheKeys: Array<$Keys<GlobalState>> = [
-  'flags', 'messages', 'mute', 'narrows', 'realm', 'streams',
+  'flags', 'messages', 'mute', 'narrows', 'pmConversations', 'realm', 'streams',
   'subscriptions', 'unread', 'userGroups', 'users',
 ];
 
diff --git a/src/pm-conversations/__tests__/pmConversationsModel-test.js b/src/pm-conversations/__tests__/pmConversationsModel-test.js
new file mode 100644
index 00000000000..c08859e206f
--- /dev/null
+++ b/src/pm-conversations/__tests__/pmConversationsModel-test.js
@@ -0,0 +1,144 @@
+/* @flow strict-local */
+import Immutable from 'immutable';
+
+import { usersOfKey, keyOfExactUsers, reducer } from '../pmConversationsModel';
+import * as eg from '../../__tests__/lib/exampleData';
+
+describe('usersOfKey', () => {
+  for (const [desc, ids] of [
+    ['self-1:1', []],
+    ['other 1:1 PM', [123]],
+    ['group PM', [123, 345, 567]],
+  ]) {
+    test(desc, () => {
+      expect(usersOfKey(keyOfExactUsers(ids))).toEqual(ids);
+    });
+  }
+});
+
+describe('reducer', () => {
+  const initialState = reducer(undefined, ({ type: eg.randString() }: $FlowFixMe));
+  const someKey = keyOfExactUsers([eg.makeUser().user_id]);
+  const someState = { map: Immutable.Map([[someKey, 123]]), sorted: Immutable.List([someKey]) };
+
+  describe('REALM_INIT', () => {
+    test('no data (old server)', () => {
+      /* eslint-disable-next-line no-unused-vars */
+      const { recent_private_conversations, ...rest } = eg.action.realm_init.data;
+      expect(reducer(someState, { ...eg.action.realm_init, data: rest })).toEqual(initialState);
+    });
+
+    test('works in normal case', () => {
+      // Includes self-1:1, other 1:1, and group PM thread.
+      // Out of order.
+      const recent_private_conversations = [
+        { user_ids: [], max_message_id: 234 },
+        { user_ids: [1], max_message_id: 123 },
+        { user_ids: [2, 1], max_message_id: 345 }, // user_ids out of order
+      ];
+      const expected = {
+        map: Immutable.Map([['', 234], ['1', 123], ['1,2', 345]]),
+        sorted: Immutable.List(['1,2', '', '1']),
+      };
+      expect(
+        reducer(someState, {
+          ...eg.action.realm_init,
+          data: { ...eg.action.realm_init.data, recent_private_conversations },
+        }),
+      ).toEqual(expected);
+    });
+  });
+
+  describe('MESSAGE_FETCH_COMPLETE', () => {
+    const [user1, user2] = [eg.makeUser({ user_id: 1 }), eg.makeUser({ user_id: 2 })];
+    const msg = (id, otherUsers) => eg.pmMessageFromTo(eg.selfUser, otherUsers, { id });
+    const action = messages => ({ ...eg.action.message_fetch_complete, messages });
+
+    test('works', () => {
+      let state = initialState;
+      state = reducer(
+        state,
+        action([msg(45, [user1]), msg(123, [user1, user2]), eg.streamMessage(), msg(234, [user1])]),
+      );
+      expect(state).toEqual({
+        map: Immutable.Map([['1', 234], ['1,2', 123]]),
+        sorted: Immutable.List(['1', '1,2']),
+      });
+
+      const newState = reducer(state, action([eg.streamMessage()]));
+      expect(newState).toBe(state);
+
+      state = reducer(
+        state,
+        action([
+          msg(345, [user2]),
+          msg(159, [user2]),
+          msg(456, [user1, user2]),
+          msg(102, [user1, user2]),
+        ]),
+      );
+      expect(state).toEqual({
+        map: Immutable.Map([['1', 234], ['1,2', 456], ['2', 345]]),
+        sorted: Immutable.List(['1,2', '2', '1']),
+      });
+    });
+  });
+
+  describe('EVENT_NEW_MESSAGE', () => {
+    const actionGeneral = message => ({ ...eg.eventNewMessageActionBase, message });
+    const [user1, user2] = [eg.makeUser({ user_id: 1 }), eg.makeUser({ user_id: 2 })];
+    const action = (id, otherUsers) =>
+      actionGeneral(eg.pmMessageFromTo(eg.selfUser, otherUsers, { id }));
+
+    // We'll start from here to test various updates.
+    const baseState = (() => {
+      let state = initialState;
+      state = reducer(state, action(234, [user1]));
+      state = reducer(state, action(123, [user1, user2]));
+      return state;
+    })();
+
+    test('(check base state)', () => {
+      // This is here mostly for checked documentation of what's in
+      // baseState, to help in reading the other test cases.
+      expect(baseState).toEqual({
+        map: Immutable.Map([['1', 234], ['1,2', 123]]),
+        sorted: Immutable.List(['1', '1,2']),
+      });
+    });
+
+    test('stream message -> do nothing', () => {
+      const state = reducer(baseState, actionGeneral(eg.streamMessage()));
+      expect(state).toBe(baseState);
+    });
+
+    test('new conversation, newest message', () => {
+      const state = reducer(baseState, action(345, [user2]));
+      expect(state).toEqual({
+        map: Immutable.Map([['1', 234], ['1,2', 123], ['2', 345]]),
+        sorted: Immutable.List(['2', '1', '1,2']),
+      });
+    });
+
+    test('new conversation, not newest message', () => {
+      const state = reducer(baseState, action(159, [user2]));
+      expect(state).toEqual({
+        map: Immutable.Map([['1', 234], ['1,2', 123], ['2', 159]]),
+        sorted: Immutable.List(['1', '2', '1,2']),
+      });
+    });
+
+    test('existing conversation, newest message', () => {
+      const state = reducer(baseState, action(345, [user1, user2]));
+      expect(state).toEqual({
+        map: Immutable.Map([['1', 234], ['1,2', 345]]),
+        sorted: Immutable.List(['1,2', '1']),
+      });
+    });
+
+    test('existing conversation, not newest in conversation', () => {
+      const state = reducer(baseState, action(102, [user1, user2]));
+      expect(state).toBe(baseState);
+    });
+  });
+});
diff --git a/src/pm-conversations/pmConversationsModel.js b/src/pm-conversations/pmConversationsModel.js
new file mode 100644
index 00000000000..ddc981556f4
--- /dev/null
+++ b/src/pm-conversations/pmConversationsModel.js
@@ -0,0 +1,190 @@
+/* @flow strict-local */
+import Immutable from 'immutable';
+import invariant from 'invariant';
+import {
+  ACCOUNT_SWITCH,
+  EVENT_NEW_MESSAGE,
+  LOGIN_SUCCESS,
+  LOGOUT,
+  MESSAGE_FETCH_COMPLETE,
+  REALM_INIT,
+} from '../actionConstants';
+
+import type { Action, Message, Outbox } from '../types';
+import { recipientsOfPrivateMessage } from '../utils/recipient';
+
+//
+//
+// Keys.
+//
+
+/** The key identifying a PM conversation in this data structure. */
+// User IDs, excluding self, sorted numerically, joined with commas.
+export opaque type PmConversationKey = string;
+
+/** PRIVATE.  Exported only for tests. */
+// Input must have the exact right (multi-)set of users.  Needn't be sorted.
+export function keyOfExactUsers(ids: number[]): PmConversationKey {
+  return ids.sort((a, b) => a - b).join(',');
+}
+
+// Input may contain self or not, and needn't be sorted.
+function keyOfUsers(ids: number[], ownUserId: number): PmConversationKey {
+  return keyOfExactUsers(ids.filter(id => id !== ownUserId));
+}
+
+// Input must indeed be a PM, else throws.
+function keyOfPrivateMessage(msg: Message | Outbox, ownUserId: number): PmConversationKey {
+  return keyOfUsers(recipientsOfPrivateMessage(msg).map(r => r.id), ownUserId);
+}
+
+/** The users in the conversation, other than self. */
+export function usersOfKey(key: PmConversationKey): number[] {
+  return key ? key.split(',').map(s => Number.parseInt(s, 10)) : [];
+}
+
+//
+//
+// State and reducer.
+//
+
+/**
+ * The list of recent PM conversations, plus data to efficiently maintain it.
+ *
+ * This gets initialized from the `recent_private_conversations` data
+ * structure in the `/register` response (aka our initial fetch), and then
+ * kept up to date as we learn about new or newly-fetched messages.
+ */
+// (Compare the webapp's implementation, in static/js/pm_conversations.js.)
+export type PmConversationsState = {|
+  // The latest message ID in each conversation.
+  map: Immutable.Map<PmConversationKey, number>,
+
+  // The keys of the map, sorted by latest message descending.
+  sorted: Immutable.List<PmConversationKey>,
+|};
+
+const initialState: PmConversationsState = { map: Immutable.Map(), sorted: Immutable.List() };
+
+// Insert the key at the proper place in the sorted list.
+//
+// Optimized, taking O(1) time, for the case where that place is the start,
+// because that's the common case for a new message.  May take O(n) time in
+// general.
+function insertSorted(sorted, map, key, msgId) {
+  const i = sorted.findIndex(k => {
+    const id = map.get(k);
+    invariant(id !== undefined, 'pm-conversations: key in sorted should be in map');
+    return id < msgId;
+  });
+
+  // Immutable.List is a deque, with O(1) shift/unshift as well as push/pop.
+  if (i === 0) {
+    return sorted.unshift(key);
+  }
+  if (i < 0) {
+    // (This case isn't common and isn't here to optimize, though it happens
+    // to be fast; it's just that `sorted.insert(-1, key)` wouldn't work.)
+    return sorted.push(key);
+  }
+  return sorted.insert(i, key);
+}
+
+// Insert the message into the state.
+//
+// Can take linear time in general.  That sounds inefficient...
+// but it's what the webapp does, so must not be catastrophic. 🤷
+// (In fact the webapp calls `Array#sort`, which takes at *least*
+// linear time, and may be 𝛳(N log N).)
+//
+// Optimized for the EVENT_NEW_MESSAGE case; for REALM_INIT and
+// FETCH_MESSAGES_COMPLETE, if we find we want to optimize them, the first
+// thing we'll want to do is probably to batch the work and skip this
+// function.
+//
+// For EVENT_NEW_MESSAGE, the point of the event is that we're learning
+// about the message in real time immediately after it was sent -- so the
+// overwhelmingly common case is that the message is newer than any existing
+// message we know about. (*)  That's therefore the case we optimize for,
+// particularly in the helper `insertSorted`.
+//
+// (*) The alternative is possible, but requires some kind of race to occur;
+// e.g., we get a FETCH_MESSAGES_COMPLETE that includes the just-sent
+// message 1002, and only after that get the event about message 1001, sent
+// moments earlier.  The event queue always delivers events in order, so
+// even the race is possible only because we fetch messages outside of the
+// event queue.
+function insert(
+  state: PmConversationsState,
+  key: PmConversationKey,
+  msgId: number,
+): PmConversationsState {
+  /* eslint-disable padded-blocks */
+  let { map, sorted } = state;
+  const prev = map.get(key);
+  // prettier-ignore
+  if (prev === undefined) {
+    // The conversation is new.  Add to both `map` and `sorted`.
+    map = map.set(key, msgId);
+    return { map, sorted: insertSorted(sorted, map, key, msgId) };
+
+  } else if (prev >= msgId) {
+    // The conversation already has a newer message.  Do nothing.
+    return state;
+
+  } else {
+    // The conversation needs to be (a) updated in `map`...
+    map = map.set(key, msgId);
+
+    // ... and (b) moved around in `sorted` to keep the list sorted.
+    const i = sorted.indexOf(key);
+    invariant(i >= 0, 'pm-conversations: key in map should be in sorted');
+    sorted = sorted.delete(i); // linear time, ouch
+    return { map, sorted: insertSorted(sorted, map, key, msgId) };
+  }
+}
+
+// Insert the message into the state.
+//
+// See `insert` for discussion of the time complexity.
+function insertMessage(state, message, ownUserId) {
+  if (message.type !== 'private') {
+    return state;
+  }
+  return insert(state, keyOfPrivateMessage(message, ownUserId), message.id);
+}
+
+export function reducer(state: PmConversationsState = initialState, action: Action) {
+  switch (action.type) {
+    case LOGOUT:
+    case LOGIN_SUCCESS:
+    case ACCOUNT_SWITCH:
+      return initialState;
+
+    case REALM_INIT: {
+      // TODO optimize; this is quadratic (but so is the webapp's version?!)
+      let st = initialState;
+      for (const r of action.data.recent_private_conversations ?? []) {
+        st = insert(st, keyOfExactUsers(r.user_ids), r.max_message_id);
+      }
+      return st;
+    }
+
+    case MESSAGE_FETCH_COMPLETE: {
+      // TODO optimize; this is quadratic (but so is the webapp's version?!)
+      let st = state;
+      for (const m of action.messages) {
+        st = insertMessage(st, m, action.ownUserId);
+      }
+      return st;
+    }
+
+    case EVENT_NEW_MESSAGE: {
+      const { message, ownUser } = action;
+      return insertMessage(state, message, ownUser.user_id);
+    }
+
+    default:
+      return state;
+  }
+}
diff --git a/src/reduxTypes.js b/src/reduxTypes.js
index af7c68ab794..8519ef11372 100644
--- a/src/reduxTypes.js
+++ b/src/reduxTypes.js
@@ -31,6 +31,7 @@ import type {
 } from './api/apiTypes';
 import type { Narrow } from './utils/narrow';
 import type { SessionState } from './session/sessionReducer';
+import type { PmConversationsState } from './pm-conversations/pmConversationsModel';
 
 export type * from './actionTypes';
 
@@ -358,6 +359,7 @@ export type GlobalState = {|
   mute: MuteState,
   narrows: NarrowsState,
   outbox: OutboxState,
+  pmConversations: PmConversationsState,
   presence: PresenceState,
   realm: RealmState,
   session: SessionState,

From cea4a44adb7bc4c0930fa3305a87236a75e4d049 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulipchat.com>
Date: Wed, 6 Jan 2021 12:50:44 -0800
Subject: [PATCH 07/11] recent-pms [nfc]: Add a version switch in
 getRecentConversations.

In this version, the "modern" implementation is a stub that just
invokes the "legacy" implementation.  Next, we'll actually write a
modern implementation.
---
 .../pmConversationsSelectors-test.js          |  7 +++++-
 src/pm-conversations/pmConversationsModel.js  |  8 +++++++
 .../pmConversationsSelectors.js               | 22 +++++++++++++++++--
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/src/pm-conversations/__tests__/pmConversationsSelectors-test.js b/src/pm-conversations/__tests__/pmConversationsSelectors-test.js
index 829d90eef3b..a1d988b6a13 100644
--- a/src/pm-conversations/__tests__/pmConversationsSelectors-test.js
+++ b/src/pm-conversations/__tests__/pmConversationsSelectors-test.js
@@ -4,8 +4,10 @@ import Immutable from 'immutable';
 import { getRecentConversations } from '../pmConversationsSelectors';
 import { ALL_PRIVATE_NARROW_STR } from '../../utils/narrow';
 import * as eg from '../../__tests__/lib/exampleData';
+import { ZulipVersion } from '../../utils/zulipVersion';
 
-describe('getRecentConversations', () => {
+describe('getRecentConversationsLegacy', () => {
+  const accounts = [{ ...eg.selfAccount, zulipVersion: new ZulipVersion('2.0') }];
   const userJohn = eg.makeUser();
   const userMark = eg.makeUser();
   const keyForUsers = users =>
@@ -17,6 +19,7 @@ describe('getRecentConversations', () => {
 
   test('when no messages, return no conversations', () => {
     const state = eg.reduxState({
+      accounts,
       realm: eg.realmState({ email: eg.selfUser.email }),
       users: [eg.selfUser],
       narrows: Immutable.Map({
@@ -36,6 +39,7 @@ describe('getRecentConversations', () => {
 
   test('returns unique list of recipients, includes conversations with self', () => {
     const state = eg.reduxState({
+      accounts,
       realm: eg.realmState({ email: eg.selfUser.email }),
       users: [eg.selfUser, userJohn, userMark],
       narrows: Immutable.Map({
@@ -83,6 +87,7 @@ describe('getRecentConversations', () => {
 
   test('returns recipients sorted by last activity', () => {
     const state = eg.reduxState({
+      accounts,
       realm: eg.realmState({ email: eg.selfUser.email }),
       users: [eg.selfUser, userJohn, userMark],
       narrows: Immutable.Map({
diff --git a/src/pm-conversations/pmConversationsModel.js b/src/pm-conversations/pmConversationsModel.js
index ddc981556f4..8c9b4de8d48 100644
--- a/src/pm-conversations/pmConversationsModel.js
+++ b/src/pm-conversations/pmConversationsModel.js
@@ -12,6 +12,14 @@ import {
 
 import type { Action, Message, Outbox } from '../types';
 import { recipientsOfPrivateMessage } from '../utils/recipient';
+import { ZulipVersion } from '../utils/zulipVersion';
+
+/** The minimum server version to expect this data to be available. */
+// Actually 2.1-dev-384-g4c3c669b41 according to our notes in src/api/;
+// but this is cleaner, and 2.1 is out long enough that few people, if any,
+// will be running a 2.1-dev version anymore (and nobody should be.)
+// TODO(server-2.1): Delete this and all code conditioned on older than it.
+export const MIN_RECENTPMS_SERVER_VERSION = new ZulipVersion('2.1');
 
 //
 //
diff --git a/src/pm-conversations/pmConversationsSelectors.js b/src/pm-conversations/pmConversationsSelectors.js
index a332d6533af..611ef29d0c2 100644
--- a/src/pm-conversations/pmConversationsSelectors.js
+++ b/src/pm-conversations/pmConversationsSelectors.js
@@ -1,13 +1,16 @@
 /* @flow strict-local */
 import { createSelector } from 'reselect';
 
-import type { Message, PmConversationData, Selector, User } from '../types';
+import type { GlobalState, Message, PmConversationData, Selector, User } from '../types';
 import { getPrivateMessages } from '../message/messageSelectors';
 import { getAllUsersById, getOwnUser } from '../users/userSelectors';
 import { getUnreadByPms, getUnreadByHuddles } from '../unread/unreadSelectors';
 import { pmUnreadsKeyFromMessage, pmKeyRecipientUsersFromMessage } from '../utils/recipient';
+import { getServerVersion } from '../account/accountsSelectors';
+import * as model from './pmConversationsModel';
 
-export const getRecentConversations: Selector<PmConversationData[]> = createSelector(
+// TODO(server-2.1): Delete this, and simplify logic around it.
+export const getRecentConversationsLegacy: Selector<PmConversationData[]> = createSelector(
   getOwnUser,
   getPrivateMessages,
   getUnreadByPms,
@@ -58,6 +61,21 @@ export const getRecentConversations: Selector<PmConversationData[]> = createSele
   },
 );
 
+export const getRecentConversationsModern: Selector<PmConversationData[]> = state =>
+  // TODO implement for real
+  getRecentConversationsLegacy(state);
+
+const getServerIsOld: Selector<boolean> = createSelector(
+  getServerVersion,
+  version => !(version && version.isAtLeast(model.MIN_RECENTPMS_SERVER_VERSION)),
+);
+
+/**
+ * The most recent PM conversations, with unread count and latest message ID.
+ */
+export const getRecentConversations = (state: GlobalState): PmConversationData[] =>
+  getServerIsOld(state) ? getRecentConversationsLegacy(state) : getRecentConversationsModern(state);
+
 export const getUnreadConversations: Selector<PmConversationData[]> = createSelector(
   getRecentConversations,
   conversations => conversations.filter(c => c.unread > 0),

From 81690257927b2455b5ad51011fdc11c338268265 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulipchat.com>
Date: Wed, 6 Jan 2021 13:26:10 -0800
Subject: [PATCH 08/11] recent-pms [nfc]: Pull out the code to look up unread
 counts.

We'll want to do the same thing in the upcoming "modern"
implementation of this selector (because this is using another data
structure unrelated to the one we're modernizing); so factor it out
as a function we can call there.
---
 .../pmConversationsSelectors.js               | 22 +++++++++++--------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/pm-conversations/pmConversationsSelectors.js b/src/pm-conversations/pmConversationsSelectors.js
index 611ef29d0c2..9aafe3a5026 100644
--- a/src/pm-conversations/pmConversationsSelectors.js
+++ b/src/pm-conversations/pmConversationsSelectors.js
@@ -9,6 +9,18 @@ import { pmUnreadsKeyFromMessage, pmKeyRecipientUsersFromMessage } from '../util
 import { getServerVersion } from '../account/accountsSelectors';
 import * as model from './pmConversationsModel';
 
+function unreadCount(unreadsKey, unreadPms, unreadHuddles): number {
+  // This business of looking in one place and then the other is kind
+  // of messy.  Fortunately it always works, because the key spaces
+  // are disjoint: all `unreadHuddles` keys contain a comma, and all
+  // `unreadPms` keys don't.
+  /* $FlowFixMe: The keys of unreadPms are logically numbers... but because
+       it's an object, they end up converted to strings, so this access with
+       string keys works.  We should probably use a Map for this and similar
+       maps. */
+  return unreadPms[unreadsKey] || unreadHuddles[unreadsKey];
+}
+
 // TODO(server-2.1): Delete this, and simplify logic around it.
 export const getRecentConversationsLegacy: Selector<PmConversationData[]> = createSelector(
   getOwnUser,
@@ -48,15 +60,7 @@ export const getRecentConversationsLegacy: Selector<PmConversationData[]> = crea
       key: conversation.unreadsKey,
       keyRecipients: conversation.keyRecipients,
       msgId: conversation.msgId,
-      unread:
-        // This business of looking in one place and then the other is kind
-        // of messy.  Fortunately it always works, because the key spaces
-        // are disjoint: all `unreadHuddles` keys contain a comma, and all
-        // `unreadPms` keys don't.
-        /* $FlowFixMe: The keys of unreadPms are logically numbers, but because it's an object they
-         end up converted to strings, so this access with string keys works.  We should probably use
-         a Map for this and similar maps. */
-        unreadPms[conversation.unreadsKey] || unreadHuddles[conversation.unreadsKey],
+      unread: unreadCount(conversation.unreadsKey, unreadPms, unreadHuddles),
     }));
   },
 );

From b00dc788c5f56034b5663763e44a7cbe3f117a76 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulipchat.com>
Date: Wed, 6 Jan 2021 13:27:07 -0800
Subject: [PATCH 09/11] recent-pms: Reimplement getRecentConversations using
 the nice new data.

This is only a little shorter in line count; but I think it has
a good bit less complexity to take in.

It's also doing quite a lot less work: instead of iterating through
all the PMs (messages!) we know about, it iterates through just one
item per PM *conversation*, in data that we've efficiently
maintained as the messages came in.  That's the big payoff of this
server API feature.

Or, relatedly: because this is so much more efficient, we're able
to provide much more thorough data.  The old way was fed primarily
by `fetchPrivateMessages`, where we fetch the latest 100 PMs --
which for someone who uses PMs a lot could leave out conversations
as recent as yesterday, and include just a handful of conversations.
With current servers, the new way gets initialized from the user's
last 1000 PMs.

As a result, if you look at the PM-conversations screen with the
app before this change and then after it, you'll probably see a
lot more conversations.  For me on czo, I see about 4x as many,
extending far back into the past.

Fixes: #3133
---
 .../pmConversationsSelectors-test.js          | 62 +++++++++++++++++--
 .../pmConversationsSelectors.js               | 58 +++++++++++++++--
 2 files changed, 109 insertions(+), 11 deletions(-)

diff --git a/src/pm-conversations/__tests__/pmConversationsSelectors-test.js b/src/pm-conversations/__tests__/pmConversationsSelectors-test.js
index a1d988b6a13..ff954fb5c54 100644
--- a/src/pm-conversations/__tests__/pmConversationsSelectors-test.js
+++ b/src/pm-conversations/__tests__/pmConversationsSelectors-test.js
@@ -2,20 +2,70 @@
 import Immutable from 'immutable';
 
 import { getRecentConversations } from '../pmConversationsSelectors';
+import { keyOfExactUsers } from '../pmConversationsModel';
 import { ALL_PRIVATE_NARROW_STR } from '../../utils/narrow';
 import * as eg from '../../__tests__/lib/exampleData';
 import { ZulipVersion } from '../../utils/zulipVersion';
 
+const keyForUsers = users =>
+  users
+    .map(u => u.user_id)
+    .sort((a, b) => a - b)
+    .map(String)
+    .join(',');
+
+describe('getRecentConversationsModern', () => {
+  const accounts = [{ ...eg.selfAccount, zulipVersion: new ZulipVersion('2.1') }];
+  const recentsKeyForUsers = users => keyOfExactUsers(users.map(u => u.user_id));
+
+  test('does its job', () => {
+    const state = eg.reduxState({
+      accounts,
+      realm: eg.realmState({ user_id: eg.selfUser.user_id }),
+      users: [eg.selfUser, eg.otherUser, eg.thirdUser],
+      unread: {
+        ...eg.baseReduxState.unread,
+        pms: [
+          { sender_id: eg.selfUser.user_id, unread_message_ids: [4] },
+          { sender_id: eg.otherUser.user_id, unread_message_ids: [1, 3] },
+        ],
+        huddles: [
+          {
+            user_ids_string: keyForUsers([eg.selfUser, eg.otherUser, eg.thirdUser]),
+            unread_message_ids: [2],
+          },
+        ],
+      },
+      pmConversations: {
+        // prettier-ignore
+        map: Immutable.Map([
+          [[], 4],
+          [[eg.otherUser], 3],
+          [[eg.otherUser, eg.thirdUser], 2],
+        ].map(([k, v]) => [recentsKeyForUsers(k), v])),
+        sorted: Immutable.List(
+          [[], [eg.otherUser], [eg.otherUser, eg.thirdUser]].map(recentsKeyForUsers),
+        ),
+      },
+    });
+
+    expect(getRecentConversations(state)).toEqual([
+      { key: eg.selfUser.user_id.toString(), keyRecipients: [eg.selfUser], msgId: 4, unread: 1 },
+      { key: eg.otherUser.user_id.toString(), keyRecipients: [eg.otherUser], msgId: 3, unread: 2 },
+      {
+        key: keyForUsers([eg.selfUser, eg.otherUser, eg.thirdUser]),
+        keyRecipients: [eg.otherUser, eg.thirdUser].sort((a, b) => a.user_id - b.user_id),
+        msgId: 2,
+        unread: 1,
+      },
+    ]);
+  });
+});
+
 describe('getRecentConversationsLegacy', () => {
   const accounts = [{ ...eg.selfAccount, zulipVersion: new ZulipVersion('2.0') }];
   const userJohn = eg.makeUser();
   const userMark = eg.makeUser();
-  const keyForUsers = users =>
-    users
-      .map(u => u.user_id)
-      .sort((a, b) => a - b)
-      .map(String)
-      .join(',');
 
   test('when no messages, return no conversations', () => {
     const state = eg.reduxState({
diff --git a/src/pm-conversations/pmConversationsSelectors.js b/src/pm-conversations/pmConversationsSelectors.js
index 9aafe3a5026..74e20434453 100644
--- a/src/pm-conversations/pmConversationsSelectors.js
+++ b/src/pm-conversations/pmConversationsSelectors.js
@@ -1,13 +1,20 @@
 /* @flow strict-local */
+import invariant from 'invariant';
 import { createSelector } from 'reselect';
 
 import type { GlobalState, Message, PmConversationData, Selector, User } from '../types';
 import { getPrivateMessages } from '../message/messageSelectors';
-import { getAllUsersById, getOwnUser } from '../users/userSelectors';
+import { getAllUsersById, getOwnUser, getOwnUserId } from '../users/userSelectors';
 import { getUnreadByPms, getUnreadByHuddles } from '../unread/unreadSelectors';
-import { pmUnreadsKeyFromMessage, pmKeyRecipientUsersFromMessage } from '../utils/recipient';
+import {
+  pmUnreadsKeyFromMessage,
+  pmKeyRecipientUsersFromMessage,
+  pmKeyRecipientsFromIds,
+  pmUnreadsKeyFromPmKeyIds,
+} from '../utils/recipient';
 import { getServerVersion } from '../account/accountsSelectors';
 import * as model from './pmConversationsModel';
+import { type PmConversationsState } from './pmConversationsModel';
 
 function unreadCount(unreadsKey, unreadPms, unreadHuddles): number {
   // This business of looking in one place and then the other is kind
@@ -65,9 +72,50 @@ export const getRecentConversationsLegacy: Selector<PmConversationData[]> = crea
   },
 );
 
-export const getRecentConversationsModern: Selector<PmConversationData[]> = state =>
-  // TODO implement for real
-  getRecentConversationsLegacy(state);
+export const getRecentConversationsModern: Selector<PmConversationData[]> = createSelector(
+  state => state.pmConversations,
+  getUnreadByPms,
+  getUnreadByHuddles,
+  getAllUsersById,
+  getOwnUserId,
+  // This is defined separately, just below.  When this is defined inline,
+  // if there's a type error in it, the message Flow gives is often pretty
+  // terrible: just highlighting the whole thing and pointing at something
+  // in the `reselect` libdef.  Defining it separately seems to help.
+  getRecentConversationsModernImpl, // eslint-disable-line no-use-before-define
+);
+
+function getRecentConversationsModernImpl(
+  { sorted, map }: PmConversationsState,
+  unreadPms,
+  unreadHuddles,
+  allUsersById,
+  ownUserId,
+): PmConversationData[] {
+  return sorted
+    .toSeq()
+    .map(recentsKey => {
+      const keyRecipients = pmKeyRecipientsFromIds(
+        model.usersOfKey(recentsKey),
+        allUsersById,
+        ownUserId,
+      );
+      if (keyRecipients === null) {
+        return null;
+      }
+
+      const unreadsKey = pmUnreadsKeyFromPmKeyIds(keyRecipients.map(r => r.user_id), ownUserId);
+
+      const msgId = map.get(recentsKey);
+      invariant(msgId !== undefined, 'pm-conversations: key in sorted should be in map');
+
+      const unread = unreadCount(unreadsKey, unreadPms, unreadHuddles);
+
+      return { key: unreadsKey, keyRecipients, msgId, unread };
+    })
+    .filter(Boolean)
+    .toArray();
+}
 
 const getServerIsOld: Selector<boolean> = createSelector(
   getServerVersion,

From d37eceacc4cfe921bbd14b2bdc71c69f2890fbe3 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulipchat.com>
Date: Thu, 7 Jan 2021 19:14:34 -0800
Subject: [PATCH 10/11] recent-pms: Optimize the case of a new message in the
 latest thread.

This seems likely to be a common case -- probably the majority of
the EVENT_NEW_MESSAGE events.  We already had it taking constant
time (aka O(1) time) thanks to the `i === 0` optimization in
`insertSorted`... but we can do better by not mutating `sorted`
at all.

Also add a test that this optimization is present.  In general we
can't easily test the performance, or the asymptotic complexity, of
our data structures and algorithms... but when the intended behavior
is "the new thing is identical to the old thing", that we can.
---
 .../__tests__/pmConversationsModel-test.js       |  9 +++++++++
 src/pm-conversations/pmConversationsModel.js     | 16 +++++++++++++---
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/src/pm-conversations/__tests__/pmConversationsModel-test.js b/src/pm-conversations/__tests__/pmConversationsModel-test.js
index c08859e206f..c7e2708e5f1 100644
--- a/src/pm-conversations/__tests__/pmConversationsModel-test.js
+++ b/src/pm-conversations/__tests__/pmConversationsModel-test.js
@@ -136,6 +136,15 @@ describe('reducer', () => {
       });
     });
 
+    test('existing newest conversation, newest message', () => {
+      const state = reducer(baseState, action(345, [user1]));
+      expect(state).toEqual({
+        map: Immutable.Map([['1', 345], ['1,2', 123]]),
+        sorted: Immutable.List(['1', '1,2']),
+      });
+      expect(state.sorted).toBe(baseState.sorted);
+    });
+
     test('existing conversation, not newest in conversation', () => {
       const state = reducer(baseState, action(102, [user1, user2]));
       expect(state).toBe(baseState);
diff --git a/src/pm-conversations/pmConversationsModel.js b/src/pm-conversations/pmConversationsModel.js
index 8c9b4de8d48..c504692a135 100644
--- a/src/pm-conversations/pmConversationsModel.js
+++ b/src/pm-conversations/pmConversationsModel.js
@@ -144,11 +144,21 @@ function insert(
     // The conversation needs to be (a) updated in `map`...
     map = map.set(key, msgId);
 
-    // ... and (b) moved around in `sorted` to keep the list sorted.
+    // ... and (b) possibly moved around in `sorted` to keep the list sorted.
     const i = sorted.indexOf(key);
     invariant(i >= 0, 'pm-conversations: key in map should be in sorted');
-    sorted = sorted.delete(i); // linear time, ouch
-    return { map, sorted: insertSorted(sorted, map, key, msgId) };
+    if (i === 0) {
+      // The conversation was already the latest, so no reordering needed.
+      // (This is likely a common case in practice -- happens every time
+      // the user gets several PMs in a row in the same thread -- so good to
+      // optimize.)
+    } else {
+      // It wasn't the latest.  Just handle the general case.
+      sorted = sorted.delete(i); // linear time, ouch
+      sorted = insertSorted(sorted, map, key, msgId);
+    }
+
+    return { map, sorted };
   }
 }
 

From 02d869ba4d1005ace9223eb9ac49d6be344b0f63 Mon Sep 17 00:00:00 2001
From: Greg Price <greg@zulipchat.com>
Date: Thu, 7 Jan 2021 20:12:45 -0800
Subject: [PATCH 11/11] recent-pms: Skip fetchPrivateMessages on recent
 servers.

With the spiffy new recent-pms data structure, this hack is
no longer needed!
---
 src/message/fetchActions.js | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/message/fetchActions.js b/src/message/fetchActions.js
index e58c1796137..894d7ef0d92 100644
--- a/src/message/fetchActions.js
+++ b/src/message/fetchActions.js
@@ -33,6 +33,7 @@ import { startEventPolling } from '../events/eventActions';
 import { logout } from '../account/accountActions';
 import { ZulipVersion } from '../utils/zulipVersion';
 import { getAllUsersById, getOwnUserId } from '../users/userSelectors';
+import { MIN_RECENTPMS_SERVER_VERSION } from '../pm-conversations/pmConversationsModel';
 
 const messageFetchStart = (narrow: Narrow, numBefore: number, numAfter: number): Action => ({
   type: MESSAGE_FETCH_START,
@@ -239,13 +240,14 @@ export const fetchMessagesInNarrow = (
 /**
  * Fetch the few most recent PMs.
  *
- * We do this eagerly in `doInitialFetch`, where it mainly serves to let us
- * show something useful in the PM conversations screen.  Recent server
- * versions have a custom-made API to help us do this better, which we hope
- * to use soon: see #3133.
+ * For old servers, we do this eagerly in `doInitialFetch`, in order to
+ * let us show something useful in the PM conversations screen.
+ * Zulip Server 2.1 added a custom-made API to help us do this better;
+ * see #3133.
  *
  * See `fetchMessagesInNarrow` for further background.
  */
+// TODO(server-2.1): Delete this.
 const fetchPrivateMessages = () => async (dispatch: Dispatch, getState: GetState) => {
   const auth = getAuth(getState());
   const { messages, found_newest, found_oldest } = await api.getMessages(auth, {
@@ -348,11 +350,14 @@ export const doInitialFetch = () => async (dispatch: Dispatch, getState: GetStat
     return;
   }
 
-  dispatch(realmInit(initData, new ZulipVersion(serverSettings.zulip_version)));
+  const serverVersion = new ZulipVersion(serverSettings.zulip_version);
+  dispatch(realmInit(initData, serverVersion));
   dispatch(initialFetchComplete());
   dispatch(startEventPolling(initData.queue_id, initData.last_event_id));
 
-  dispatch(fetchPrivateMessages());
+  if (!serverVersion.isAtLeast(MIN_RECENTPMS_SERVER_VERSION)) {
+    dispatch(fetchPrivateMessages());
+  }
 
   dispatch(sendOutbox());
   dispatch(initNotifications());