diff --git a/src/chat/narrowsSelectors.js b/src/chat/narrowsSelectors.js index 7e5600dc0f4..f3c338c8662 100644 --- a/src/chat/narrowsSelectors.js +++ b/src/chat/narrowsSelectors.js @@ -22,7 +22,7 @@ import { getOutbox, } from '../directSelectors'; import { getCaughtUpForNarrow } from '../caughtup/caughtUpSelectors'; -import { getAllUsersByEmail, getOwnUser } from '../users/userSelectors'; +import { getAllUsersByEmail, getAllUsersById, getOwnUser } from '../users/userSelectors'; import { isStreamOrTopicNarrow, emailsOfGroupPmNarrow, @@ -149,13 +149,16 @@ export const isNarrowValid: Selector = createSelector( (state, narrow) => narrow, state => getStreams(state), state => getAllUsersByEmail(state), - (narrow, streams, allUsersByEmail) => + state => getAllUsersById(state), + (narrow, streams, allUsersByEmail, allUsersById) => caseNarrowDefault( narrow, { stream: streamName => streams.find(s => s.name === streamName) !== undefined, topic: streamName => streams.find(s => s.name === streamName) !== undefined, - pm: emails => emails.every(email => allUsersByEmail.get(email) !== undefined), + pm: (emails, ids) => + emails.every(email => allUsersByEmail.get(email) !== undefined) + && ids.every(id => allUsersById.get(id) !== undefined), }, () => true, ), diff --git a/src/common/GroupAvatar.js b/src/common/GroupAvatar.js index 29193e3d7cf..4f5f2909d62 100644 --- a/src/common/GroupAvatar.js +++ b/src/common/GroupAvatar.js @@ -20,7 +20,6 @@ const styles = createStyleSheet({ type Props = $ReadOnly<{| names: string[], size: number, - shape: 'rounded' | 'square', children?: React$Node, onPress?: () => void, |}>; @@ -43,22 +42,17 @@ export const initialsForGroupIcon = (names: string[]): string => { * * @prop names - The name of one or more users, used to extract their initials. * @prop size - Sets width and height in logical pixels. - * @prop [shape] * @prop children - If provided, will render inside the component body. * @prop onPress - Event fired on pressing the component. */ export default class GroupAvatar extends PureComponent { - static defaultProps = { - shape: 'rounded', - }; - render() { - const { children, names, size, shape, onPress } = this.props; + const { children, names, size, onPress } = this.props; const frameSize = { height: size, width: size, - borderRadius: shape === 'rounded' ? size / 8 : 0, + borderRadius: size / 8, backgroundColor: colorHashFromString(names.join(', ')), }; const textSize = { diff --git a/src/common/UserAvatar.js b/src/common/UserAvatar.js index 00e4fa73b9d..1c81ab9074f 100644 --- a/src/common/UserAvatar.js +++ b/src/common/UserAvatar.js @@ -8,7 +8,6 @@ import { AvatarURL } from '../utils/avatar'; type Props = $ReadOnly<{| avatarUrl: AvatarURL, size: number, - shape: 'rounded' | 'square', children?: React$Node, onPress?: () => void, |}>; @@ -18,18 +17,13 @@ type Props = $ReadOnly<{| * * @prop avatarUrl * @prop size - Sets width and height in logical pixels. - * @prop [shape] - 'rounded' (default) means a square with rounded corners. * @prop [children] - If provided, will render inside the component body. * @prop [onPress] - Event fired on pressing the component. */ export default class UserAvatar extends PureComponent { - static defaultProps = { - shape: 'rounded', - }; - render() { - const { avatarUrl, children, size, shape, onPress } = this.props; - const borderRadius = shape === 'rounded' ? size / 8 : 0; + const { avatarUrl, children, size, onPress } = this.props; + const borderRadius = size / 8; const style = { height: size, width: size, diff --git a/src/common/UserAvatarWithPresence.js b/src/common/UserAvatarWithPresence.js index 62736946cba..d35b534f085 100644 --- a/src/common/UserAvatarWithPresence.js +++ b/src/common/UserAvatarWithPresence.js @@ -1,11 +1,13 @@ /* @flow strict-local */ - -import React, { PureComponent } from 'react'; +import React, { type ComponentType, PureComponent } from 'react'; import { createStyleSheet } from '../styles'; +import type { Dispatch } from '../types'; import UserAvatar from './UserAvatar'; import PresenceStatusIndicator from './PresenceStatusIndicator'; import { AvatarURL } from '../utils/avatar'; +import { getUserForId } from '../users/userSelectors'; +import { connect } from '../react-redux'; const styles = createStyleSheet({ status: { @@ -17,33 +19,65 @@ const styles = createStyleSheet({ type Props = $ReadOnly<{| avatarUrl: AvatarURL, - email: string, size: number, - shape: 'rounded' | 'square', onPress?: () => void, + email: string, |}>; /** - * Renders a user avatar with a PresenceStatusIndicator in the corner + * A user avatar with a PresenceStatusIndicator in the corner. + * + * Prefer `UserAvatarWithPresenceById` over this component: it does the same + * thing but avoids an email in the component's interface. Once all callers + * have migrated to that version, it'll replace this one. * * @prop [avatarUrl] * @prop [email] - Sender's / user's email address, for the presence dot. * @prop [size] - Sets width and height in logical pixels. - * @prop [shape] - 'rounded' (default) means a square with rounded corners. * @prop [onPress] - Event fired on pressing the component. */ -export default class UserAvatarWithPresence extends PureComponent { - static defaultProps = { - shape: 'rounded', - }; - +// The underlying class gets an inexact Props type to express that it's fine +// for it to be passed extra props by the implementation of …ById. +// We don't export it with that type, because we want exact Props types to +// get the most effective type-checking. +class UserAvatarWithPresence extends PureComponent<$ReadOnly<{ ...Props, ... }>> { render() { - const { avatarUrl, email, size, onPress, shape } = this.props; + const { avatarUrl, email, size, onPress } = this.props; return ( - + ); } } + +// Export the class with a tighter constraint on acceptable props, namely +// that the type is an exact object type as usual. +export default (UserAvatarWithPresence: ComponentType); + +/** + * A user avatar with a PresenceStatusIndicator in the corner. + * + * Use this in preference to the default export `UserAvatarWithPresence`. + * We're migrating from that one to this in order to avoid using emails. + * + * @prop [userId] + * @prop [size] + * @prop [onPress] + */ +export const UserAvatarWithPresenceById = connect<{| avatarUrl: AvatarURL, email: string |}, _, _>( + (state, props) => { + const user = getUserForId(state, props.userId); + return { avatarUrl: user.avatar_url, email: user.email }; + }, +)( + // The type inference embedded in our `connect` type relies on seeing the + // exact Props type for the underlying component, complete with all the + // expected props. Normally that's just how our Props types are in the + // first place, but here we have to provide it explicitly. + /* eslint-disable flowtype/generic-spacing */ + (UserAvatarWithPresence: ComponentType< + $ReadOnly<{| ...Props, dispatch: Dispatch, userId: number |}>, + >), +); diff --git a/src/compose/ComposeBox.js b/src/compose/ComposeBox.js index 20f261dbaf6..8ed39ae54a6 100644 --- a/src/compose/ComposeBox.js +++ b/src/compose/ComposeBox.js @@ -44,7 +44,6 @@ import { getIsAdmin, getSession, getLastMessageTopic, - getActiveUsersByEmail, getCaughtUpForNarrow, getStreamInNarrow, getVideoChatProvider, @@ -56,12 +55,12 @@ import { import { getDraftForNarrow } from '../drafts/draftsSelectors'; import TopicAutocomplete from '../autocomplete/TopicAutocomplete'; import AutocompleteView from '../autocomplete/AutocompleteView'; -import { getOwnEmail } from '../users/userSelectors'; +import { getActiveUsersById, getOwnUserId } from '../users/userSelectors'; type SelectorProps = {| auth: Auth, - ownEmail: string, - usersByEmail: Map, + ownUserId: number, + usersById: Map, safeAreaInsets: Dimensions, isAdmin: boolean, isAnnouncementOnly: boolean, @@ -420,9 +419,9 @@ class ComposeBox extends PureComponent { render() { const { isTopicFocused, isMenuExpanded, height, message, topic, selection } = this.state; const { - ownEmail, + ownUserId, narrow, - usersByEmail, + usersById, editMessage, safeAreaInsets, isAdmin, @@ -441,7 +440,7 @@ class ComposeBox extends PureComponent { return ; } - const placeholder = getComposeInputPlaceholder(narrow, ownEmail, usersByEmail); + const placeholder = getComposeInputPlaceholder(narrow, ownUserId, usersById); const style = { paddingBottom: safeAreaInsets.bottom, backgroundColor: 'hsla(0, 0%, 50%, 0.1)', @@ -519,8 +518,8 @@ class ComposeBox extends PureComponent { export default connect((state, props) => ({ auth: getAuth(state), - ownEmail: getOwnEmail(state), - usersByEmail: getActiveUsersByEmail(state), + ownUserId: getOwnUserId(state), + usersById: getActiveUsersById(state), safeAreaInsets: getSession(state).safeAreaInsets, isAdmin: getIsAdmin(state), isAnnouncementOnly: getIsActiveStreamAnnouncementOnly(state, props.narrow), diff --git a/src/compose/__tests__/getComposeInputPlaceholder-test.js b/src/compose/__tests__/getComposeInputPlaceholder-test.js index eb056545123..a661fa377f6 100644 --- a/src/compose/__tests__/getComposeInputPlaceholder-test.js +++ b/src/compose/__tests__/getComposeInputPlaceholder-test.js @@ -11,12 +11,12 @@ import { import * as eg from '../../__tests__/lib/exampleData'; describe('getComposeInputPlaceholder', () => { - const usersByEmail = new Map([eg.selfUser, eg.otherUser, eg.thirdUser].map(u => [u.email, u])); - const ownEmail = eg.selfUser.email; + const usersById = new Map([eg.selfUser, eg.otherUser, eg.thirdUser].map(u => [u.user_id, u])); + const ownUserId = eg.selfUser.user_id; test('returns "Message @ThisPerson" object for person narrow', () => { const narrow = deepFreeze(pm1to1NarrowFromUser(eg.otherUser)); - const placeholder = getComposeInputPlaceholder(narrow, ownEmail, usersByEmail); + const placeholder = getComposeInputPlaceholder(narrow, ownUserId, usersById); expect(placeholder).toEqual({ text: 'Message {recipient}', values: { recipient: `@${eg.otherUser.full_name}` }, @@ -25,25 +25,25 @@ describe('getComposeInputPlaceholder', () => { test('returns "Jot down something" object for self narrow', () => { const narrow = deepFreeze(pm1to1NarrowFromUser(eg.selfUser)); - const placeholder = getComposeInputPlaceholder(narrow, ownEmail, usersByEmail); + const placeholder = getComposeInputPlaceholder(narrow, ownUserId, usersById); expect(placeholder).toEqual({ text: 'Jot down something' }); }); test('returns "Message #streamName" for stream narrow', () => { const narrow = deepFreeze(streamNarrow('Denmark')); - const placeholder = getComposeInputPlaceholder(narrow, ownEmail, usersByEmail); + const placeholder = getComposeInputPlaceholder(narrow, ownUserId, usersById); 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, ownEmail, usersByEmail); + const placeholder = getComposeInputPlaceholder(narrow, ownUserId, usersById); expect(placeholder).toEqual({ text: 'Reply' }); }); test('returns "Message group" object for group narrow', () => { const narrow = deepFreeze(pmNarrowFromUsersUnsafe([eg.otherUser, eg.thirdUser])); - const placeholder = getComposeInputPlaceholder(narrow, ownEmail, usersByEmail); + const placeholder = getComposeInputPlaceholder(narrow, ownUserId, usersById); expect(placeholder).toEqual({ text: 'Message group' }); }); }); diff --git a/src/compose/getComposeInputPlaceholder.js b/src/compose/getComposeInputPlaceholder.js index ff7a5563633..e7e15bcbde5 100644 --- a/src/compose/getComposeInputPlaceholder.js +++ b/src/compose/getComposeInputPlaceholder.js @@ -4,23 +4,23 @@ import { caseNarrowDefault } from '../utils/narrow'; export default ( narrow: Narrow, - ownEmail: string, - usersByEmail: Map, + ownUserId: number, + usersById: Map, ): LocalizableText => caseNarrowDefault( narrow, { - pm: emails => { - if (emails.length > 1) { + pm: (emails, ids) => { + if (ids.length > 1) { return { text: 'Message group' }; } - const email = emails[0]; + const userId = ids[0]; - if (email === ownEmail) { + if (userId === ownUserId) { return { text: 'Jot down something' }; } - const user = usersByEmail.get(email); + const user = usersById.get(userId); if (!user) { return { text: 'Type a message' }; } diff --git a/src/outbox/outboxActions.js b/src/outbox/outboxActions.js index 1a1d06f5617..2c68722eaa2 100644 --- a/src/outbox/outboxActions.js +++ b/src/outbox/outboxActions.js @@ -12,7 +12,7 @@ import { } from '../actionConstants'; import { getAuth } from '../selectors'; import * as api from '../api'; -import { getAllUsersByEmail, getOwnUser } from '../users/userSelectors'; +import { getAllUsersById, getOwnUser } from '../users/userSelectors'; import { getUsersAndWildcards } from '../users/userHelpers'; import { caseNarrowPartial } from '../utils/narrow'; import { BackoffMachine } from '../utils/async'; @@ -98,13 +98,13 @@ export const sendOutbox = () => async (dispatch: Dispatch, getState: GetState) = }; // A valid display_recipient with all the thread's users, sorted by ID. -const mapEmailsToUsers = (emails, allUsersByEmail, ownUser) => { - const result = emails.map(email => { - const user = allUsersByEmail.get(email); +const recipientsFromIds = (ids, allUsersById, ownUser) => { + const result = ids.map(id => { + const user = allUsersById.get(id); if (!user) { throw new Error('outbox: missing user when preparing to send PM'); } - return { email, id: user.user_id, full_name: user.full_name }; + return { id, email: user.email, full_name: user.full_name }; }); if (!result.some(r => r.id === ownUser.user_id)) { result.push({ email: ownUser.email, id: ownUser.user_id, full_name: ownUser.full_name }); @@ -120,13 +120,13 @@ type DataFromNarrow = SubsetProperties< const extractTypeToAndSubjectFromNarrow = ( narrow: Narrow, - allUsersByEmail: Map, + allUsersById: Map, ownUser: UserOrBot, ): DataFromNarrow => caseNarrowPartial(narrow, { - pm: emails => ({ + pm: (emails, ids) => ({ type: 'private', - display_recipient: mapEmailsToUsers(emails, allUsersByEmail, ownUser), + display_recipient: recipientsFromIds(ids, allUsersById, ownUser), subject: '', }), @@ -167,7 +167,7 @@ export const addToOutbox = (narrow: Narrow, content: string) => async ( dispatch( messageSendStart({ isSent: false, - ...extractTypeToAndSubjectFromNarrow(narrow, getAllUsersByEmail(state), ownUser), + ...extractTypeToAndSubjectFromNarrow(narrow, getAllUsersById(state), ownUser), markdownContent: content, content: getContentPreview(content, state), timestamp: localTime, diff --git a/src/title/Title.js b/src/title/Title.js index c25d81084b9..f1c9ca07218 100644 --- a/src/title/Title.js +++ b/src/title/Title.js @@ -30,11 +30,11 @@ export default class Title extends PureComponent { allPrivate: () => , stream: () => , topic: () => , - pm: emails => - emails.length === 1 ? ( - + pm: (emails, ids) => + ids.length === 1 ? ( + ) : ( - + ), search: () => null, }); diff --git a/src/title/TitleGroup.js b/src/title/TitleGroup.js index f88b0ac0cfc..01486f89c3d 100644 --- a/src/title/TitleGroup.js +++ b/src/title/TitleGroup.js @@ -4,27 +4,17 @@ import React, { PureComponent } from 'react'; import { View } from 'react-native'; import NavigationService from '../nav/NavigationService'; -import type { Dispatch, UserOrBot, Narrow } from '../types'; import styles, { createStyleSheet } from '../styles'; -import { connect } from '../react-redux'; -import { UserAvatarWithPresence } from '../common'; -import { getRecipientsInGroupPmNarrow } from '../selectors'; +import { UserAvatarWithPresenceById } from '../common/UserAvatarWithPresence'; import { navigateToAccountDetails } from '../nav/navActions'; -type SelectorProps = $ReadOnly<{| - recipients: UserOrBot[], -|}>; - type Props = $ReadOnly<{| - narrow: Narrow, - - dispatch: Dispatch, - ...SelectorProps, + userIds: $ReadOnlyArray, |}>; -class TitleGroup extends PureComponent { - handlePress = (user: UserOrBot) => { - NavigationService.dispatch(navigateToAccountDetails(user.user_id)); +export default class TitleGroup extends PureComponent { + handlePress = (userId: number) => { + NavigationService.dispatch(navigateToAccountDetails(userId)); }; styles = createStyleSheet({ @@ -34,17 +24,15 @@ class TitleGroup extends PureComponent { }); render() { - const { recipients } = this.props; - + const { userIds } = this.props; return ( - {recipients.map((user, index) => ( - - this.handlePress(user)} + {userIds.map(userId => ( + + this.handlePress(userId)} size={32} - avatarUrl={user.avatar_url} - email={user.email} + userId={userId} /> ))} @@ -52,7 +40,3 @@ class TitleGroup extends PureComponent { ); } } - -export default connect((state, props) => ({ - recipients: getRecipientsInGroupPmNarrow(state, props.narrow), -}))(TitleGroup); diff --git a/src/title/TitlePrivate.js b/src/title/TitlePrivate.js index dc9eb4bc738..69502a80cbe 100644 --- a/src/title/TitlePrivate.js +++ b/src/title/TitlePrivate.js @@ -7,9 +7,10 @@ import NavigationService from '../nav/NavigationService'; import type { Dispatch, UserOrBot } from '../types'; import styles, { createStyleSheet } from '../styles'; import { connect } from '../react-redux'; -import { Touchable, UserAvatarWithPresence, ViewPlaceholder } from '../common'; +import { Touchable, ViewPlaceholder } from '../common'; +import { UserAvatarWithPresenceById } from '../common/UserAvatarWithPresence'; import ActivityText from './ActivityText'; -import { getAllUsersByEmail } from '../users/userSelectors'; +import { getAllUsersById } from '../users/userSelectors'; import { navigateToAccountDetails } from '../nav/navActions'; import * as logging from '../utils/logging'; @@ -18,7 +19,7 @@ type SelectorProps = $ReadOnly<{| |}>; type Props = $ReadOnly<{ - email: string, + userId: number, color: string, dispatch: Dispatch, @@ -55,7 +56,7 @@ class TitlePrivate extends PureComponent { return ( - + @@ -70,6 +71,5 @@ class TitlePrivate extends PureComponent { } export default connect((state, props) => ({ - // TODO: use user_id, not email (https://github.com/zulip/zulip-mobile/issues/3764) - user: getAllUsersByEmail(state).get(props.email), + user: getAllUsersById(state).get(props.userId), }))(TitlePrivate); diff --git a/src/users/UserItem.js b/src/users/UserItem.js index 8f8ef6b6ea3..dcedc241e8f 100644 --- a/src/users/UserItem.js +++ b/src/users/UserItem.js @@ -3,7 +3,8 @@ import React, { PureComponent } from 'react'; import { View } from 'react-native'; import type { UserOrBot } from '../types'; -import { UserAvatarWithPresence, RawLabel, Touchable, UnreadCount } from '../common'; +import { RawLabel, Touchable, UnreadCount } from '../common'; +import { UserAvatarWithPresenceById } from '../common/UserAvatarWithPresence'; import styles, { createStyleSheet, BRAND_COLOR } from '../styles'; const componentStyles = createStyleSheet({ @@ -54,12 +55,7 @@ export default class UserItem extends PureComponent { return ( - + { - test('return users, bots, map by email and do not include inactive users', () => { - const state = eg.reduxState({ - users: [eg.selfUser], - realm: { - ...eg.baseReduxState.realm, - crossRealmBots: [eg.crossRealmBot], - nonActiveUsers: [eg.otherUser], - }, - }); - expect(getActiveUsersByEmail(state)).toEqual( - new Map([[eg.selfUser.email, eg.selfUser], [eg.crossRealmBot.email, eg.crossRealmBot]]), - ); - }); -}); - describe('getAllUsersByEmail', () => { test('return users mapped by their email', () => { const users = [eg.makeUser(), eg.makeUser(), eg.makeUser()]; diff --git a/src/users/userSelectors.js b/src/users/userSelectors.js index fbe1d622bc6..35a001ad06b 100644 --- a/src/users/userSelectors.js +++ b/src/users/userSelectors.js @@ -21,7 +21,7 @@ import { getUsers, getCrossRealmBots, getNonActiveUsers } from '../directSelecto * user to send a PM to -- deactivated users should be left out. * * See: - * * `getActiveUsersByEmail` for leaving out deactivated users + * * `getActiveUsersById` for leaving out deactivated users * * `User` for details on properties, and links to docs. */ const getAllUsers: Selector = createSelector( @@ -160,18 +160,6 @@ export const getUsersSansMe: Selector = createSelector( (users, ownEmail) => users.filter(user => user.email !== ownEmail), ); -/** - * Excludes deactivated users. See `getAllUsers` for discussion. - * - * Prefer `getActiveUsersById`; see #3764. - */ -export const getActiveUsersByEmail: Selector> = createSelector( - getUsers, - getCrossRealmBots, - (users = [], crossRealmBots = []) => - new Map([...users, ...crossRealmBots].map(user => [user.email, user])), -); - /** Excludes deactivated users. See `getAllUsers` for discussion. */ export const getActiveUsersById: Selector> = createSelector( getUsers, diff --git a/src/utils/narrow.js b/src/utils/narrow.js index c57793bee02..e603ce79486 100644 --- a/src/utils/narrow.js +++ b/src/utils/narrow.js @@ -337,10 +337,10 @@ export const isHomeNarrow = (narrow?: Narrow): boolean => !!narrow && caseNarrowDefault(narrow, { home: () => true }, () => false); export const is1to1PmNarrow = (narrow?: Narrow): boolean => - !!narrow && caseNarrowDefault(narrow, { pm: emails => emails.length === 1 }, () => false); + !!narrow && caseNarrowDefault(narrow, { pm: (emails, ids) => ids.length === 1 }, () => false); export const isGroupPmNarrow = (narrow?: Narrow): boolean => - !!narrow && caseNarrowDefault(narrow, { pm: emails => emails.length > 1 }, () => false); + !!narrow && caseNarrowDefault(narrow, { pm: (emails, ids) => ids.length > 1 }, () => false); /** * The recipients' emails if a group PM narrow; else error.