narrow/recipient: Always centrally filter and sort PM users, and verify that.

src/account-info/AccountDetailsScreen.js

@@ -7,7 +7,7 @@ import { createStyleSheet } from '../styles';
 import { connect } from '../react-redux';
 import { Screen, ZulipButton, Label } from '../common';
 import { IconPrivateChat } from '../common/Icons';
-import { pmNarrowFromEmail } from '../utils/narrow';
+import { pm1to1NarrowFromUser } from '../utils/narrow';
 import AccountDetails from './AccountDetails';
 import { doNarrow } from '../actions';
 import { getUserIsActive, getUserForId } from '../users/userSelectors';
@@ -43,7 +43,7 @@ type Props = $ReadOnly<{|
 class AccountDetailsScreen extends PureComponent<Props> {
   handleChatPress = () => {
     const { user, dispatch } = this.props;
-    dispatch(doNarrow(pmNarrowFromEmail(user.email)));
+    dispatch(doNarrow(pm1to1NarrowFromUser(user)));
   };
 
   render() {

src/chat/__tests__/narrowsReducer-test.js

@@ -6,9 +6,9 @@ import narrowsReducer from '../narrowsReducer';
 import {
   HOME_NARROW,
   HOME_NARROW_STR,
-  pmNarrowFromEmail,
+  pm1to1NarrowFromUser,
   ALL_PRIVATE_NARROW_STR,
-  pmNarrowFromEmails,
+  pmNarrowFromUsersUnsafe,
   streamNarrow,
   topicNarrow,
   STARRED_NARROW_STR,
@@ -23,10 +23,8 @@ import { LAST_MESSAGE_ANCHOR, FIRST_UNREAD_ANCHOR } from '../../anchor';
 import * as eg from '../../__tests__/lib/exampleData';
 
 describe('narrowsReducer', () => {
-  const privateNarrowStr = JSON.stringify(pmNarrowFromEmail(eg.otherUser.email));
-  const groupNarrowStr = JSON.stringify(
-    pmNarrowFromEmails([eg.otherUser.email, eg.thirdUser.email]),
-  );
+  const privateNarrowStr = JSON.stringify(pm1to1NarrowFromUser(eg.otherUser));
+  const groupNarrowStr = JSON.stringify(pmNarrowFromUsersUnsafe([eg.otherUser, eg.thirdUser]));
   const streamNarrowStr = JSON.stringify(streamNarrow(eg.stream.name));
   const egTopic = eg.streamMessage().subject;
   const topicNarrowStr = JSON.stringify(topicNarrow(eg.stream.name, egTopic));
@@ -147,7 +145,7 @@ describe('narrowsReducer', () => {
   });
 
   test('message sent to self is stored correctly', () => {
-    const narrowWithSelfStr = JSON.stringify(pmNarrowFromEmail(eg.selfUser.email));
+    const narrowWithSelfStr = JSON.stringify(pm1to1NarrowFromUser(eg.selfUser));
     const initialState = Immutable.Map({
       [HOME_NARROW_STR]: [],
       [narrowWithSelfStr]: [],
@@ -373,7 +371,7 @@ describe('narrowsReducer', () => {
       const action = deepFreeze({
         type: MESSAGE_FETCH_COMPLETE,
         anchor: 2,
-        narrow: pmNarrowFromEmail(eg.otherUser.email),
+        narrow: pm1to1NarrowFromUser(eg.otherUser),
         messages: [],
         numBefore: 100,
         numAfter: 100,
@@ -383,7 +381,7 @@ describe('narrowsReducer', () => {
 
       const expectedState = Immutable.Map({
         [HOME_NARROW_STR]: [1, 2, 3],
-        [JSON.stringify(pmNarrowFromEmail(eg.otherUser.email))]: [],
+        [JSON.stringify(pm1to1NarrowFromUser(eg.otherUser))]: [],
       });
 
       const newState = narrowsReducer(initialState, action);

src/chat/__tests__/narrowsSelectors-test.js

@@ -15,7 +15,7 @@ import {
   streamNarrow,
   topicNarrow,
   STARRED_NARROW,
-  pmNarrowFromEmails,
+  pmNarrowFromUsersUnsafe,
 } from '../../utils/narrow';
 import { NULL_SUBSCRIPTION } from '../../nullObjects';
 import * as eg from '../../__tests__/lib/exampleData';
@@ -293,7 +293,7 @@ describe('isNarrowValid', () => {
       streams: [],
       users: [john, mark],
     });
-    const narrow = pmNarrowFromEmails([john.email, mark.email]);
+    const narrow = pmNarrowFromUsersUnsafe([john, mark]);
 
     const result = isNarrowValid(state, narrow);
 
@@ -312,7 +312,7 @@ describe('isNarrowValid', () => {
       streams: [],
       users: [john],
     });
-    const narrow = pmNarrowFromEmails([john.email, mark.email]);
+    const narrow = pmNarrowFromUsersUnsafe([john, mark]);
 
     const result = isNarrowValid(state, narrow);
 

src/compose/__tests__/getComposeInputPlaceholder-test.js

@@ -1,71 +1,49 @@
+/* @flow strict-local */
 import deepFreeze from 'deep-freeze';
 
 import getComposeInputPlaceholder from '../getComposeInputPlaceholder';
 import {
   pmNarrowFromEmail,
   streamNarrow,
   topicNarrow,
-  pmNarrowFromEmails,
+  pmNarrowFromUsersUnsafe,
 } from '../../utils/narrow';
+import * as eg from '../../__tests__/lib/exampleData';
 
 describe('getComposeInputPlaceholder', () => {
-  test('returns "Message @ThisPerson" object for person narrow', () => {
-    const narrow = deepFreeze(pmNarrowFromEmail('abc@zulip.com'));
-
-    const ownEmail = 'hamlet@zulip.com';
-
-    const usersByEmail = new Map([
-      [
-        'abc@zulip.com',
-        {
-          id: 23,
-          email: 'abc@zulip.com',
-          full_name: 'ABC',
-        },
-      ],
-      [
-        'xyz@zulip.com',
-        {
-          id: 22,
-          email: 'xyz@zulip.com',
-          full_name: 'XYZ',
-        },
-      ],
-    ]);
+  const usersByEmail = new Map([eg.selfUser, eg.otherUser, eg.thirdUser].map(u => [u.email, u]));
+  const ownEmail = eg.selfUser.email;
 
+  test('returns "Message @ThisPerson" object for person narrow', () => {
+    const narrow = deepFreeze(pmNarrowFromEmail(eg.otherUser.email));
     const placeholder = getComposeInputPlaceholder(narrow, ownEmail, usersByEmail);
-    expect(placeholder).toEqual({ text: 'Message {recipient}', values: { recipient: '@ABC' } });
+    expect(placeholder).toEqual({
+      text: 'Message {recipient}',
+      values: { recipient: `@${eg.otherUser.full_name}` },
+    });
   });
 
   test('returns "Jot down something" object for self narrow', () => {
-    const narrow = deepFreeze(pmNarrowFromEmail('abc@zulip.com'));
-
-    const ownEmail = 'abc@zulip.com';
-
-    const placeholder = getComposeInputPlaceholder(narrow, ownEmail);
+    const narrow = deepFreeze(pmNarrowFromEmail(eg.selfUser.email));
+    const placeholder = getComposeInputPlaceholder(narrow, ownEmail, usersByEmail);
     expect(placeholder).toEqual({ text: 'Jot down something' });
   });
 
   test('returns "Message #streamName" for stream narrow', () => {
     const narrow = deepFreeze(streamNarrow('Denmark'));
-
-    const placeholder = getComposeInputPlaceholder(narrow);
+    const placeholder = getComposeInputPlaceholder(narrow, ownEmail, usersByEmail);
     expect(placeholder).toEqual({ text: 'Message {recipient}', values: { recipient: '#Denmark' } });
   });
 
   test('returns properly for topic narrow', () => {
     const narrow = deepFreeze(topicNarrow('Denmark', 'Copenhagen'));
-
-    const placeholder = getComposeInputPlaceholder(narrow);
-    expect(placeholder).toEqual({
-      text: 'Reply',
-    });
+    const placeholder = getComposeInputPlaceholder(narrow, ownEmail, usersByEmail);
+    expect(placeholder).toEqual({ text: 'Reply' });
   });
 
   test('returns "Message group" object for group narrow', () => {
-    const narrow = deepFreeze(pmNarrowFromEmails(['abc@zulip.com, xyz@zulip.com']));
-
-    const placeholder = getComposeInputPlaceholder(narrow);
+    const narrow = deepFreeze(pmNarrowFromUsersUnsafe([eg.otherUser, eg.thirdUser]));
+    const placeholder = getComposeInputPlaceholder(narrow, ownEmail, usersByEmail);
     expect(placeholder).toEqual({ text: 'Message group' });
   });
 });

src/notification/__tests__/notification-test.js

@@ -4,7 +4,7 @@ import deepFreeze from 'deep-freeze';
 import type { UserOrBot } from '../../api/modelTypes';
 import type { JSONableDict } from '../../utils/jsonable';
 import { getNarrowFromNotificationData } from '..';
-import { topicNarrow, pmNarrowFromEmail, pmNarrowFromEmails } from '../../utils/narrow';
+import { topicNarrow, pmNarrowFromEmail, pmNarrowFromUsersUnsafe } from '../../utils/narrow';
 
 import * as eg from '../../__tests__/lib/exampleData';
 import { fromAPNsImpl as extractIosNotificationData } from '../extract';
@@ -49,7 +49,7 @@ describe('getNarrowFromNotificationData', () => {
       pm_users: users.map(u => u.user_id).join(','),
     };
 
-    const expectedNarrow = pmNarrowFromEmails(users.slice(1).map(u => u.email));
+    const expectedNarrow = pmNarrowFromUsersUnsafe(users.slice(1));
 
     const narrow = getNarrowFromNotificationData(notification, allUsersById, ownUserId);
 
@@ -61,9 +61,9 @@ describe('getNarrowFromNotificationData', () => {
       recipient_type: 'private',
       pm_users: '1,2,4',
     };
-    const usersById = new Map();
+    const allUsersById = new Map();
 
-    const narrow = getNarrowFromNotificationData(notification, usersById, ownUserId);
+    const narrow = getNarrowFromNotificationData(notification, allUsersById, ownUserId);
 
     expect(narrow).toBe(null);
   });

src/notification/index.js

@@ -4,7 +4,7 @@ import NotificationsIOS from 'react-native-notifications';
 
 import type { Notification } from './types';
 import type { Auth, Dispatch, Identity, Narrow, UserOrBot } from '../types';
-import { topicNarrow, pmNarrowFromEmail, pmNarrowFromEmails } from '../utils/narrow';
+import { topicNarrow, pmNarrowFromEmail, pmNarrowFromUsers } from '../utils/narrow';
 import type { JSONable, JSONableDict } from '../utils/jsonable';
 import * as api from '../api';
 import * as logging from '../utils/logging';
@@ -107,7 +107,7 @@ export const getNarrowFromNotificationData = (
 
   const ids = data.pm_users.split(',').map(s => parseInt(s, 10));
   const users = pmKeyRecipientsFromIds(ids, allUsersById, ownUserId);
-  return users && pmNarrowFromEmails(users.map(u => u.email));
+  return users === null ? null : pmNarrowFromUsers(users);
 };
 
 const getInitialNotification = async (): Promise<Notification | null> => {

src/pm-conversations/GroupPmConversationItem.js

@@ -14,44 +14,36 @@ const componentStyles = createStyleSheet({
   },
 });
 
-type Props = $ReadOnly<{|
-  email: string,
-  usersByEmail: Map<string, UserOrBot>,
+type Props<U> = $ReadOnly<{|
+  users: U,
   unreadCount: number,
-  onPress: (emails: string) => void,
+  onPress: (users: U) => void,
 |}>;
 
 /**
  * A list item describing one group PM conversation.
  * */
-export default class GroupPmConversationItem extends PureComponent<Props> {
+export default class GroupPmConversationItem<U: $ReadOnlyArray<UserOrBot>> extends PureComponent<
+  Props<U>,
+> {
   handlePress = () => {
-    const { email, onPress } = this.props;
-    onPress(email);
+    const { users, onPress } = this.props;
+    onPress(users);
   };
 
   render() {
-    const { email, usersByEmail, unreadCount } = this.props;
-    const allUsers = email.split(',').map(e => usersByEmail.get(e));
-
-    const allUsersFound = allUsers.every(user => user);
-
-    if (!allUsersFound) {
-      return null;
-    }
-
-    // $FlowFixMe Flow doesn't see the `every` check above.
-    const allNames = allUsers.map(user => user.full_name);
+    const { users, unreadCount } = this.props;
+    const names = users.map(user => user.full_name);
 
     return (
       <Touchable onPress={this.handlePress}>
         <View style={styles.listItem}>
-          <GroupAvatar size={48} names={allNames} />
+          <GroupAvatar size={48} names={names} />
           <RawLabel
             style={componentStyles.text}
             numberOfLines={2}
             ellipsizeMode="tail"
-            text={allNames.join(', ')}
+            text={names.join(', ')}
           />
           <UnreadCount count={unreadCount} />
         </View>

src/pm-conversations/PmConversationList.js

@@ -4,7 +4,8 @@ import { FlatList } from 'react-native';
 
 import type { Dispatch, PmConversationData, UserOrBot } from '../types';
 import { createStyleSheet } from '../styles';
-import { pmNarrowFromEmail, pmNarrowFromEmails } from '../utils/narrow';
+import { type PmKeyUsers } from '../utils/recipient';
+import { pm1to1NarrowFromUser, pmNarrowFromUsers } from '../utils/narrow';
 import UserItem from '../users/UserItem';
 import GroupPmConversationItem from './GroupPmConversationItem';
 import { doNarrow } from '../actions';
@@ -19,51 +20,45 @@ const styles = createStyleSheet({
 type Props = $ReadOnly<{|
   dispatch: Dispatch,
   conversations: PmConversationData[],
-  usersByEmail: Map<string, UserOrBot>,
+  allUsersByEmail: Map<string, UserOrBot>,
 |}>;
 
 /**
  * A list describing all PM conversations.
  * */
 export default class PmConversationList extends PureComponent<Props> {
   handleUserNarrow = (user: UserOrBot) => {
-    this.props.dispatch(doNarrow(pmNarrowFromEmail(user.email)));
+    this.props.dispatch(doNarrow(pm1to1NarrowFromUser(user)));
   };
 
-  handleGroupNarrow = (email: string) => {
-    this.props.dispatch(doNarrow(pmNarrowFromEmails(email.split(','))));
+  handleGroupNarrow = (users: PmKeyUsers) => {
+    this.props.dispatch(doNarrow(pmNarrowFromUsers(users)));
   };
 
   render() {
-    const { conversations, usersByEmail } = this.props;
+    const { conversations } = this.props;
 
     return (
       <FlatList
         style={styles.list}
         initialNumToRender={20}
         data={conversations}
-        keyExtractor={item => item.recipients}
+        keyExtractor={item => item.key}
         renderItem={({ item }) => {
-          if (item.recipients.indexOf(',') === -1) {
-            const user = usersByEmail.get(item.recipients);
-
-            if (!user) {
-              return null;
-            }
-
+          const users = item.keyRecipients;
+          if (users.length === 1) {
             return (
-              <UserItem user={user} unreadCount={item.unread} onPress={this.handleUserNarrow} />
+              <UserItem user={users[0]} unreadCount={item.unread} onPress={this.handleUserNarrow} />
+            );
+          } else {
+            return (
+              <GroupPmConversationItem
+                users={users}
+                unreadCount={item.unread}
+                onPress={this.handleGroupNarrow}
+              />
             );
           }
-
-          return (
-            <GroupPmConversationItem
-              email={item.recipients}
-              unreadCount={item.unread}
-              usersByEmail={usersByEmail}
-              onPress={this.handleGroupNarrow}
-            />
-          );
         }}
       />
     );