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

Author: gnprice

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({

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,