diff --git a/src/boot/store.js b/src/boot/store.js index 09873c239db..b8c2fdba5d6 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -18,7 +18,6 @@ import ZulipAsyncStorage from './ZulipAsyncStorage'; import createMigration from '../redux-persist-migrate/index'; import { provideLoggingContext } from './loggingContext'; import { tryGetActiveAccount } from '../account/accountsSelectors'; -import { keyFromNarrow } from '../utils/narrow'; import { objectFromEntries } from '../jsBackport'; if (process.env.NODE_ENV === 'development') { @@ -226,29 +225,25 @@ const migrations: { [string]: (GlobalState) => GlobalState } = { // `AvatarURL`. '18': dropCache, - // Change format of keys representing narrows, from JSON to our format. - '19': state => ({ + // Change format of keys representing narrows: from JSON to our format, + // then for PM narrows adding user IDs. + '21': state => ({ ...dropCache(state), - drafts: objectFromEntries( - // Note this will migrate straight to our current format, even after - // that changes from when this migration was written! That saves us - // from duplicating `keyFromNarrow` here... but calls for care in - // migrations for future changes to `keyFromNarrow`. - Object.keys(state.drafts).map(key => [keyFromNarrow(JSON.parse(key)), state.drafts[key]]), - ), + // The old format was a rather hairy format that we don't want to + // permanently keep around the code to parse. For PMs, there's an + // extra wrinkle in that any conversion would require using additional + // information to look up the IDs. Drafts are inherently short-term, + // and are already discarded whenever switching between accounts; + // so we just drop them here. + drafts: {}, }), - // Change format of keys representing PM narrows, adding user IDs. - '20': state => ({ + // Change format of keys representing PM narrows, dropping emails. + '22': state => ({ ...dropCache(state), drafts: objectFromEntries( Object.keys(state.drafts) - // Just drop any old-style, email-only PM keys. Converting them - // would require using additional information to look up the IDs, - // which would make this more complex than any of our other - // migrations. Drafts are inherently short-term, and are already - // discarded whenever switching between accounts. - .filter(key => !key.startsWith('pm:s:')) + .map(key => key.replace(/^pm:d:(.*?):.*/s, 'pm:$1')) .map(key => [key, state.drafts[key]]), ), }), diff --git a/src/chat/narrowsSelectors.js b/src/chat/narrowsSelectors.js index 525200f82f2..46970a3e3f7 100644 --- a/src/chat/narrowsSelectors.js +++ b/src/chat/narrowsSelectors.js @@ -20,7 +20,7 @@ import { getOutbox, } from '../directSelectors'; import { getCaughtUpForNarrow } from '../caughtup/caughtUpSelectors'; -import { getAllUsersByEmail, getAllUsersById, getOwnUser } from '../users/userSelectors'; +import { getAllUsersById, getOwnUser } from '../users/userSelectors'; import { isStreamOrTopicNarrow, isMessageInNarrow, @@ -134,17 +134,14 @@ export const getStreamInNarrow: Selector = createSelector( (state, narrow) => narrow, state => getStreams(state), - state => getAllUsersByEmail(state), state => getAllUsersById(state), - (narrow, streams, allUsersByEmail, allUsersById) => + (narrow, streams, allUsersById) => caseNarrowDefault( narrow, { stream: streamName => streams.find(s => s.name === streamName) !== undefined, topic: streamName => streams.find(s => s.name === streamName) !== undefined, - pm: (emails, ids) => - emails.every(email => allUsersByEmail.get(email) !== undefined) - && ids.every(id => allUsersById.get(id) !== undefined), + pm: ids => ids.every(id => allUsersById.get(id) !== undefined), }, () => true, ), diff --git a/src/compose/getComposeInputPlaceholder.js b/src/compose/getComposeInputPlaceholder.js index e7e15bcbde5..0f833bced96 100644 --- a/src/compose/getComposeInputPlaceholder.js +++ b/src/compose/getComposeInputPlaceholder.js @@ -10,7 +10,7 @@ export default ( caseNarrowDefault( narrow, { - pm: (emails, ids) => { + pm: ids => { if (ids.length > 1) { return { text: 'Message group' }; } diff --git a/src/message/fetchActions.js b/src/message/fetchActions.js index e90c4196513..d234f47c2ba 100644 --- a/src/message/fetchActions.js +++ b/src/message/fetchActions.js @@ -32,6 +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'; const messageFetchStart = (narrow: Narrow, numBefore: number, numAfter: number): Action => ({ type: MESSAGE_FETCH_START, @@ -88,7 +89,7 @@ export const fetchMessages = (fetchArgs: {| try { const { messages, found_newest, found_oldest } = await api.getMessages(getAuth(getState()), { ...fetchArgs, - narrow: apiNarrowOfNarrow(fetchArgs.narrow), + narrow: apiNarrowOfNarrow(fetchArgs.narrow, getAllUsersById(getState())), useFirstUnread: fetchArgs.anchor === FIRST_UNREAD_ANCHOR, // TODO: don't use this; see #4203 }); dispatch( @@ -236,7 +237,7 @@ export const fetchMessagesInNarrow = ( const fetchPrivateMessages = () => async (dispatch: Dispatch, getState: GetState) => { const auth = getAuth(getState()); const { messages, found_newest, found_oldest } = await api.getMessages(auth, { - narrow: apiNarrowOfNarrow(ALL_PRIVATE_NARROW), + narrow: apiNarrowOfNarrow(ALL_PRIVATE_NARROW, getAllUsersById(getState())), anchor: LAST_MESSAGE_ANCHOR, numBefore: 100, numAfter: 0, diff --git a/src/outbox/outboxActions.js b/src/outbox/outboxActions.js index 2c68722eaa2..dde0cf423e6 100644 --- a/src/outbox/outboxActions.js +++ b/src/outbox/outboxActions.js @@ -124,7 +124,7 @@ const extractTypeToAndSubjectFromNarrow = ( ownUser: UserOrBot, ): DataFromNarrow => caseNarrowPartial(narrow, { - pm: (emails, ids) => ({ + pm: ids => ({ type: 'private', display_recipient: recipientsFromIds(ids, allUsersById, ownUser), subject: '', diff --git a/src/title-buttons/titleButtonFromNarrow.js b/src/title-buttons/titleButtonFromNarrow.js index 4726534838e..c6429f354a8 100644 --- a/src/title-buttons/titleButtonFromNarrow.js +++ b/src/title-buttons/titleButtonFromNarrow.js @@ -18,7 +18,7 @@ export const InfoButton: ComponentType = props => { stream: streamName => , topic: (streamName, topic) => , - pm: (emails, ids) => + pm: ids => ids.length === 1 ? ( ) : ( diff --git a/src/title/Title.js b/src/title/Title.js index f1c9ca07218..5a211090817 100644 --- a/src/title/Title.js +++ b/src/title/Title.js @@ -30,7 +30,7 @@ export default class Title extends PureComponent { allPrivate: () => , stream: () => , topic: () => , - pm: (emails, ids) => + pm: ids => ids.length === 1 ? ( ) : ( diff --git a/src/unread/unreadSelectors.js b/src/unread/unreadSelectors.js index 03382992af2..2939672a71d 100644 --- a/src/unread/unreadSelectors.js +++ b/src/unread/unreadSelectors.js @@ -256,7 +256,7 @@ export const getUnreadCountForNarrow: Selector = createSelector( ); }, - pm: (emails, ids) => { + pm: ids => { if (ids.length > 1) { const unreadsKey = pmUnreadsKeyFromPmKeyIds(ids, ownUserId); const unread = unreadHuddles.find(x => x.user_ids_string === unreadsKey); diff --git a/src/utils/__tests__/narrow-test.js b/src/utils/__tests__/narrow-test.js index ab722f92d14..e05f2b78438 100644 --- a/src/utils/__tests__/narrow-test.js +++ b/src/utils/__tests__/narrow-test.js @@ -40,7 +40,7 @@ describe('pm1to1NarrowFromUser', () => { test('produces a 1:1 narrow', () => { const narrow = pm1to1NarrowFromUser(eg.otherUser); expect(is1to1PmNarrow(narrow)).toBeTrue(); - expect(caseNarrowPartial(narrow, { pm: (emails, ids) => ids })).toEqual([eg.otherUser.user_id]); + expect(caseNarrowPartial(narrow, { pm: ids => ids })).toEqual([eg.otherUser.user_id]); }); test('if operator is "pm-with" and only one email, then it is a private narrow', () => { diff --git a/src/utils/narrow.js b/src/utils/narrow.js index cb810142e33..f8e28e9f149 100644 --- a/src/utils/narrow.js +++ b/src/utils/narrow.js @@ -32,7 +32,7 @@ import { export opaque type Narrow = | {| type: 'stream', streamName: string |} | {| type: 'topic', streamName: string, topic: string |} - | {| type: 'pm', userIds: $ReadOnlyArray, emails: $ReadOnlyArray |} + | {| type: 'pm', userIds: $ReadOnlyArray |} | {| type: 'search', query: string |} | {| type: 'all' | 'starred' | 'mentioned' | 'all-pm' |}; @@ -59,8 +59,8 @@ export const HOME_NARROW_STR: string = keyFromNarrow(HOME_NARROW); * accidentally disagreeing on whether to include the self-user, or on how * to sort the list (by user ID vs. email), or neglecting to sort it at all. */ -const pmNarrowInternal = (userIds: $ReadOnlyArray, emails: string[]): Narrow => - Object.freeze({ type: 'pm', userIds, emails }); +const pmNarrowInternal = (userIds: $ReadOnlyArray): Narrow => + Object.freeze({ type: 'pm', userIds }); /** * A PM narrow, either 1:1 or group. @@ -73,7 +73,7 @@ const pmNarrowInternal = (userIds: $ReadOnlyArray, emails: string[]): Na * different form of input. */ export const pmNarrowFromRecipients = (recipients: PmKeyRecipients): Narrow => - pmNarrowInternal(recipients.map(r => r.id), recipients.map(r => r.email)); + pmNarrowInternal(recipients); /** * A PM narrow, either 1:1 or group. @@ -85,7 +85,7 @@ export const pmNarrowFromRecipients = (recipients: PmKeyRecipients): Narrow => * single specific user. */ export const pmNarrowFromUsers = (recipients: PmKeyUsers): Narrow => - pmNarrowInternal(recipients.map(r => r.user_id), recipients.map(r => r.email)); + pmNarrowInternal(recipients.map(r => r.user_id)); /** * FOR TESTS ONLY. Like pmNarrowFromUsers, but without validation. @@ -105,7 +105,7 @@ export const pmNarrowFromUsers = (recipients: PmKeyUsers): Narrow => // annoying thing is just that that requires an ownUserId value. export const pmNarrowFromUsersUnsafe = (recipients: UserOrBot[]): Narrow => { recipients.sort((a, b) => a.user_id - b.user_id); - return pmNarrowInternal(recipients.map(r => r.user_id), recipients.map(r => r.email)); + return pmNarrowInternal(recipients.map(r => r.user_id)); }; /** @@ -115,8 +115,7 @@ export const pmNarrowFromUsersUnsafe = (recipients: UserOrBot[]): Narrow => { * statically has just one other user it's a bit more convenient because it * doesn't require going through our `recipient` helpers. */ -export const pm1to1NarrowFromUser = (user: UserOrBot): Narrow => - pmNarrowInternal([user.user_id], [user.email]); +export const pm1to1NarrowFromUser = (user: UserOrBot): Narrow => pmNarrowInternal([user.user_id]); export const specialNarrow = (operand: string): Narrow => { if (operand === 'starred') { @@ -153,7 +152,7 @@ export const SEARCH_NARROW = (query: string): Narrow => Object.freeze({ type: 's type NarrowCases = {| home: () => T, - pm: (emails: $ReadOnlyArray, ids: $ReadOnlyArray) => T, + pm: (ids: $ReadOnlyArray) => T, starred: () => T, mentioned: () => T, allPrivate: () => T, @@ -171,7 +170,7 @@ export function caseNarrow(narrow: Narrow, cases: NarrowCases): T { switch (narrow.type) { case 'stream': return cases.stream(narrow.streamName); case 'topic': return cases.topic(narrow.streamName, narrow.topic); - case 'pm': return cases.pm(narrow.emails, narrow.userIds); + case 'pm': return cases.pm(narrow.userIds); case 'search': return cases.search(narrow.query); case 'all': return cases.home(); case 'starred': return cases.starred(); @@ -237,8 +236,7 @@ export function caseNarrowDefault( * something like `base64Utf8Encode` to encode into the permitted subset. */ // The arbitrary Unicode codepoints come from (a) stream names and topics, -// and (b) our use of `\x00` as a delimiter. Also perhaps email addresses, -// if it's possible for those to have exciting characters in them. +// and (b) our use of `\x00` as a delimiter. export function keyFromNarrow(narrow: Narrow): string { // The ":s" (for "string") in several of these is to keep them disjoint, // out of an abundance of caution, from future keys that use numeric IDs. @@ -254,7 +252,8 @@ export function keyFromNarrow(narrow: Narrow): string { topic: (streamName, topic) => `topic:s:${streamName}\x00${topic}`, // An earlier version had `pm:s:`. - pm: (emails, ids) => `pm:d:${ids.join(',')}:${emails.join(',')}`, + // Another had `pm:d:`. + pm: ids => `pm:${ids.join(',')}`, home: () => 'all', starred: () => 'starred', @@ -295,17 +294,8 @@ export const parseNarrow = (narrowStr: string): Narrow => { } case 'pm:': { - // The `/s` regexp flag means the `.` patterns match absolutely - // anything. By default they reject certain "newline" characters, - // which might be impossible in email addresses but it's simplest - // not to have to care. - const match = /^d:(.*?):(.*)/s.exec(rest); - if (!match) { - throw makeError(); - } - const ids = match[1].split(',').map(s => Number.parseInt(s, 10)); - const emails = match[2].split(','); - return pmNarrowInternal(ids, emails); + const ids = rest.split(',').map(s => Number.parseInt(s, 10)); + return pmNarrowInternal(ids); } case 'search:': { @@ -336,10 +326,10 @@ export const isHomeNarrow = (narrow?: Narrow): boolean => !!narrow && caseNarrowDefault(narrow, { home: () => true }, () => false); export const is1to1PmNarrow = (narrow?: Narrow): boolean => - !!narrow && caseNarrowDefault(narrow, { pm: (emails, ids) => ids.length === 1 }, () => false); + !!narrow && caseNarrowDefault(narrow, { pm: ids => ids.length === 1 }, () => false); export const isGroupPmNarrow = (narrow?: Narrow): boolean => - !!narrow && caseNarrowDefault(narrow, { pm: (emails, ids) => ids.length > 1 }, () => false); + !!narrow && caseNarrowDefault(narrow, { pm: ids => ids.length > 1 }, () => false); /** * The "PM key recipients" IDs for a PM narrow; else error. @@ -348,7 +338,7 @@ export const isGroupPmNarrow = (narrow?: Narrow): boolean => * `PmKeyUsers`, but contains only their user IDs. */ export const userIdsOfPmNarrow = (narrow: Narrow): $ReadOnlyArray => - caseNarrowPartial(narrow, { pm: (emails, ids) => ids }); + caseNarrowPartial(narrow, { pm: ids => ids }); /** * The stream name for a stream or topic narrow; else error. @@ -399,14 +389,28 @@ export const isSearchNarrow = (narrow?: Narrow): boolean => /** * Convert the narrow into the form used in the Zulip API at get-messages. */ -export const apiNarrowOfNarrow = (narrow: Narrow): ApiNarrow => +export const apiNarrowOfNarrow = ( + narrow: Narrow, + allUsersById: Map, +): ApiNarrow => caseNarrow(narrow, { stream: streamName => [{ operator: 'stream', operand: streamName }], topic: (streamName, topic) => [ { operator: 'stream', operand: streamName }, { operator: 'topic', operand: topic }, ], - pm: emails => [{ operator: 'pm-with', operand: emails.join(',') }], + pm: ids => { + const emails = []; + for (const id of ids) { + const email = allUsersById.get(id)?.email; + if (email === undefined) { + throw new Error('apiNarrowOfNarrow: missing user'); + } + emails.push(email); + } + // TODO(server-2.1): just send IDs instead + return [{ operator: 'pm-with', operand: emails.join(',') }]; + }, search: query => [{ operator: 'search', operand: query }], home: () => [], starred: () => [{ operator: 'is', operand: 'starred' }], @@ -441,7 +445,7 @@ export const isMessageInNarrow = ( message.type === 'stream' && streamName === streamNameOfStreamMessage(message) && topic === message.subject, - pm: (emails, ids) => { + pm: ids => { if (message.type !== 'private') { return false; } diff --git a/src/utils/recipient.js b/src/utils/recipient.js index 2df3b220af6..5ea07850b75 100644 --- a/src/utils/recipient.js +++ b/src/utils/recipient.js @@ -59,7 +59,7 @@ export const recipientsOfPrivateMessage = ( * * See also `pmNarrowFromRecipients`, which requires a value of this type. */ -export opaque type PmKeyRecipients: $ReadOnlyArray = $ReadOnlyArray; +export opaque type PmKeyRecipients: $ReadOnlyArray = $ReadOnlyArray; /** * A list of users identifying a PM conversation, as per pmKeyRecipientsFromMessage. @@ -180,7 +180,10 @@ export const pmKeyRecipientsFromMessage = ( if (message.type !== 'private') { throw new Error('pmKeyRecipientsFromMessage: expected PM, got stream message'); } - return filterRecipients(recipientsOfPrivateMessage(message), ownUser.user_id); + return filterRecipientsAsUserIds( + recipientsOfPrivateMessage(message).map(r => r.id), + ownUser.user_id, + ); }; /**