Handle unknown users on profile and reactions-list screens

src/account-info/AccountDetailsScreen.js

@@ -3,6 +3,7 @@ import React, { useCallback } from 'react';
 import type { Node } from 'react';
 import { View } from 'react-native';
 
+import invariant from 'invariant';
 import type { RouteProp } from '../react-navigation';
 import type { AppNavigationProp } from '../nav/AppNavigator';
 import type { UserId } from '../types';
@@ -17,11 +18,16 @@ import AccountDetails from './AccountDetails';
 import ZulipText from '../common/ZulipText';
 import ActivityText from '../title/ActivityText';
 import { doNarrow } from '../actions';
-import { getUserIsActive, getUserForId } from '../users/userSelectors';
+import { getUserIsActive, tryGetUserForId } from '../users/userSelectors';
 import { nowInTimeZone } from '../utils/date';
 import CustomProfileFields from './CustomProfileFields';
 
 const styles = createStyleSheet({
+  errorText: {
+    marginHorizontal: 16,
+    marginVertical: 32,
+    textAlign: 'center',
+  },
   pmButton: {
     marginHorizontal: 16,
     marginBottom: 16,
@@ -46,13 +52,22 @@ type Props = $ReadOnly<{|
 
 export default function AccountDetailsScreen(props: Props): Node {
   const dispatch = useDispatch();
-  const user = useSelector(state => getUserForId(state, props.route.params.userId));
+  const user = useSelector(state => tryGetUserForId(state, props.route.params.userId));
   const isActive = useSelector(state => getUserIsActive(state, props.route.params.userId));
 
   const handleChatPress = useCallback(() => {
+    invariant(user, 'Callback handleChatPress is used only if user is known');
     dispatch(doNarrow(pm1to1NarrowFromUser(user)));
   }, [user, dispatch]);
 
+  if (!user) {
+    return (
+      <Screen title="Error">
+        <ZulipText style={styles.errorText} text="Could not show user profile." />
+      </Screen>
+    );
+  }
+
   let localTime: string | null = null;
   // See comments at CrossRealmBot and User at src/api/modelTypes.js.
   if (user.timezone !== '' && user.timezone !== undefined) {

src/common/Icons.js

@@ -107,7 +107,7 @@ export const IconServer: SpecificIconType = makeIcon(Feather, 'server');
 export const IconEdit: SpecificIconType = makeIcon(Feather, 'edit');
 export const IconPlusSquare: SpecificIconType = makeIcon(Feather, 'plus-square');
 export const IconVideo: SpecificIconType = makeIcon(Feather, 'video');
-export const IconUserMuted: SpecificIconType = makeIcon(FontAwesome, 'user');
+export const IconUserBlank: SpecificIconType = makeIcon(FontAwesome, 'user');
 export const IconAttach: SpecificIconType = makeIcon(Feather, 'paperclip');
 export const IconAttachment: SpecificIconType = makeIcon(IoniconsIcon, 'document-attach-outline');
 export const IconGroup: SpecificIconType = makeIcon(FontAwesome, 'group');

src/common/UserAvatar.js

@@ -8,11 +8,11 @@ import { getAuthHeaders } from '../api/transport';
 import { getAuth } from '../account/accountsSelectors';
 import Touchable from './Touchable';
 import { AvatarURL, FallbackAvatarURL } from '../utils/avatar';
-import { IconUserMuted } from './Icons';
+import { IconUserBlank } from './Icons';
 import { ThemeContext } from '../styles';
 
 type Props = $ReadOnly<{|
-  avatarUrl: AvatarURL,
+  avatarUrl: AvatarURL | null,
   size: number,
   isMuted?: boolean,
   children?: Node,
@@ -45,23 +45,28 @@ function UserAvatar(props: Props): Node {
 
   const auth = useSelector(state => getAuth(state));
 
+  let userImage;
+  if (isMuted || !avatarUrl) {
+    userImage = <IconUserBlank size={size} color={color} style={iconStyle} />;
+  } else {
+    userImage = (
+      <Image
+        source={{
+          uri: avatarUrl.get(PixelRatio.getPixelSizeForLayoutSize(size)).toString(),
+          ...(avatarUrl instanceof FallbackAvatarURL
+            ? { headers: getAuthHeaders(auth) }
+            : undefined),
+        }}
+        style={style}
+        resizeMode="cover"
+      />
+    );
+  }
+
   return (
     <Touchable onPress={onPress}>
       <View accessibilityIgnoresInvertColors>
-        {!isMuted ? (
-          <Image
-            source={{
-              uri: avatarUrl.get(PixelRatio.getPixelSizeForLayoutSize(size)).toString(),
-              ...(avatarUrl instanceof FallbackAvatarURL
-                ? { headers: getAuthHeaders(auth) }
-                : undefined),
-            }}
-            style={style}
-            resizeMode="cover"
-          />
-        ) : (
-          <IconUserMuted size={size} color={color} style={iconStyle} />
-        )}
+        {userImage}
         {children}
       </View>
     </Touchable>

src/common/UserAvatarWithPresence.js

@@ -40,7 +40,7 @@ export default function UserAvatarWithPresence(props: Props): Node {
 
   if (!user) {
     logging.warn("UserAvatarWithPresence: couldn't find user for ID", { userId });
-    return null;
+    return <UserAvatar avatarUrl={null} size={size} isMuted onPress={undefined} />;
   }
 
   return (

src/users/UserItem.js

@@ -1,4 +1,5 @@
 /* @flow strict-local */
+import invariant from 'invariant';
 import React, { useCallback, useContext } from 'react';
 import type { Node } from 'react';
 import { View } from 'react-native';
@@ -11,7 +12,7 @@ import UnreadCount from '../common/UnreadCount';
 import UserAvatarWithPresence from '../common/UserAvatarWithPresence';
 import { createStyleSheet, BRAND_COLOR } from '../styles';
 import { useSelector } from '../react-redux';
-import { getUserForId } from './userSelectors';
+import { tryGetUserForId } from './userSelectors';
 import { getMutedUsers } from '../selectors';
 import { getUserStatus } from '../user-statuses/userStatusesModel';
 import { emojiTypeFromReactionType } from '../emoji/data';
@@ -40,16 +41,33 @@ export default function UserItem(props: Props): Node {
   } = props;
   const _ = useContext(TranslationContext);
 
-  const user = useSelector(state => getUserForId(state, userId));
+  const user = useSelector(state => tryGetUserForId(state, userId));
 
-  const isMuted = useSelector(getMutedUsers).has(user.user_id);
-  const userStatusEmoji = useSelector(state => getUserStatus(state, user.user_id)).status_emoji;
+  const isMuted = useSelector(getMutedUsers).has(userId);
+  const userStatusEmoji = useSelector(
+    state => user && getUserStatus(state, user.user_id),
+  )?.status_emoji;
 
-  const handlePress = useCallback(() => {
-    if (onPress) {
-      onPress(user);
-    }
+  // eslint-disable-next-line no-underscore-dangle
+  const _handlePress = useCallback(() => {
+    invariant(user, 'Callback is used only if user is known');
+    invariant(onPress, 'Callback is used only if onPress provided');
+    onPress(user);
   }, [onPress, user]);
+  const handlePress = onPress && user ? _handlePress : undefined;
+
+  let displayName;
+  let displayEmail;
+  if (!user) {
+    displayName = _('(unknown user)');
+    displayEmail = null;
+  } else if (isMuted) {
+    displayName = _('Muted user');
+    displayEmail = null;
+  } else {
+    displayName = user.full_name;
+    displayEmail = showEmail ? user.email : null;
+  }
 
   const styles = React.useMemo(
     () =>
@@ -94,20 +112,20 @@ export default function UserItem(props: Props): Node {
         <UserAvatarWithPresence
           // At size medium, keep just big enough for a 48px touch target.
           size={size === 'large' ? 48 : 32}
-          userId={user.user_id}
-          onPress={onPress && handlePress}
+          userId={userId}
+          onPress={handlePress}
         />
         <View style={styles.textWrapper}>
           <ZulipText
             style={[styles.text, isSelected && styles.selectedText]}
-            text={isMuted ? _('Muted user') : user.full_name}
+            text={displayName}
             numberOfLines={1}
             ellipsizeMode="tail"
           />
-          {showEmail && !isMuted && (
+          {displayEmail != null && (
             <ZulipText
               style={[styles.text, styles.textEmail, isSelected && styles.selectedText]}
-              text={user.email}
+              text={displayEmail}
               numberOfLines={1}
               ellipsizeMode="tail"
             />

src/users/userSelectors.js

@@ -125,7 +125,7 @@ export const getOwnUser = (state: PerAccountState): User => {
  *
  * See `getUserForId` for a version which only ever returns a real user,
  * throwing if none.  That makes it a bit simpler to use in contexts where
- * we assume the relevant user must exist, which is true of most of the app.
+ * we assume the relevant user must exist.
  */
 export const tryGetUserForId = (state: PerAccountState, userId: UserId): UserOrBot | null =>
   getAllUsersById(state).get(userId) ?? null;
@@ -136,9 +136,13 @@ export const tryGetUserForId = (state: PerAccountState, userId: UserId): UserOrB
  * This works for any user in this Zulip org/realm, including deactivated
  * users and cross-realm bots.  See `getAllUsers` for details.
  *
- * Throws if no such user exists.
+ * Throws if no such user exists in our data.  This is therefore only
+ * appropriate when we can know the given user must be one we know about,
+ * which is uncommon.  (For example, even if we're looking at a message the
+ * user sent: we might be a guest with limited access to users, and the
+ * other user might no longer be in the stream so not be in our data.)
  *
- * See `tryGetUserForId` for a non-throwing version.
+ * Generally use `tryGetUserForId` instead.
  */
 export const getUserForId = (state: PerAccountState, userId: UserId): UserOrBot => {
   const user = tryGetUserForId(state, userId);