From 1193a57cc346c9f2d684bcd6fee5bb0fa2d0ed11 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Tue, 5 Jan 2021 17:51:37 -0800 Subject: [PATCH 1/7] drafts: Fix broken migration of narrow keys. As the comment in migration 19 said, we were doing something a bit risky there in order to avoid duplicating the then-current version of `keyFromNarrow`. We simply invoked `keyFromNarrow` directly -- which called for being careful about migrations for any future change we made to `keyFromNarrow`. Then a bit later we changed `keyFromNarrow` quite dramatically, and didn't pay attention to migrations at all. Oops. Specifically, in cf4207f47 we changed the `Narrow` type to have quite a different structure. That's the type that `keyFromNarrow` accepts -- so the result is that `keyFromNarrow` stopped working on the old values. Which are exactly what `JSON.parse` is going to supply us when this migration is run. The result is probably that if the user has any drafts when they take an upgrade that spans migration 19 and commit cf4207f47, the app will crash on startup. We'd probably have found this bug at the alpha stage when next making a release, so it wouldn't have made it to users other than ourselves. Better to fix ASAP, though. The plan for migration 19, as described here: https://github.com/zulip/zulip-mobile/pull/4339#discussion_r543603022 knowingly involved breaking the basic rule of migrations: don't invoke any of the app's model code, because it will change. I figured it was OK because there were changes coming up soon which I'd planned out how I'd add migrations for. Then later the same week I sent #4346, which followed that plan... and I hadn't thought through this particular interaction, and it introduced this bug. So this is a fresh lesson in the basic rule of migrations: don't invoke any of the app's model code, because it will change. See discussion where we found this bug: https://github.com/zulip/zulip-mobile/pull/4382#discussion_r552283858 --- src/boot/store.js | 36 ++++++++++-------------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index 09873c239db..aeed0796a35 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -18,8 +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') { // Chrome dev tools for Immutable. @@ -226,31 +224,17 @@ 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]]), - ), - }), - - // Change format of keys representing PM narrows, adding user IDs. - '20': 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, 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: {}, }), // TIP: When adding a migration, consider just using `dropCache`. From f8964ac5c0f893fb561fbb5efe80070dd87b9ba8 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 4 Jan 2021 16:21:10 -0800 Subject: [PATCH 2/7] narrow [nfc]: Use user IDs instead of emails in apiNarrowOfNarrow. Well, use one to get the other: we still need emails to send to the server, until we drop support for servers older than 2.1. Also add a TODO(server-2.1) comment to help us find that when we do. But with this change, we don't use the emails *from the Narrow value* -- which means we're now all clear to remove the emails from Narrow values entirely. --- src/message/fetchActions.js | 5 +++-- src/utils/narrow.js | 18 ++++++++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) 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/utils/narrow.js b/src/utils/narrow.js index cb810142e33..6cfafc58b04 100644 --- a/src/utils/narrow.js +++ b/src/utils/narrow.js @@ -399,14 +399,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: (emails_, 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' }], From 7a8739e27b59a2c683f59b557aea4ccb631846a7 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 11 Dec 2020 23:21:44 -0800 Subject: [PATCH 3/7] narrow: In isNarrowValid use only user IDs, not emails, for PMs. This check is itself the last remaining place where we consume the emails from a PM narrow and then try to find the users in our data structures; everything else relies on the user IDs. So there are no longer any potential crashes that this check could prevent. We're about to remove those emails entirely. As part of that, cut this check. --- src/chat/narrowsSelectors.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/chat/narrowsSelectors.js b/src/chat/narrowsSelectors.js index 525200f82f2..33801c9586c 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: (emails, ids) => ids.every(id => allUsersById.get(id) !== undefined), }, () => true, ), From 3bedb18df5fdecd145153ba9d1882e2beeeb4a34 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sat, 12 Dec 2020 00:24:34 -0800 Subject: [PATCH 4/7] narrow: Stop storing emails! This completes a major objective of the long string of refactoring that appeared in the series of PRs #4330, #4332, #4335, #4339, #4342, then #4346, #4356, #4361, #4364, and most recently #4368. After this change, the portion of #4333 that's about PMs, emails, and user IDs -- aka the portion of #3764 that's about narrows -- is complete. Still open from #4333 is to convert stream and topic narrows from stream names to stream IDs. I'm hopeful that that will be simpler: (a) unlike the situation for PMs, there's just one stream mentioned at a time, so there's no question of sorting, and (b) there isn't the "include self or not" complication that's bedeviled much of our code related to PMs. In other words, stream and topic narrows don't suffer from #4035. --- src/boot/store.js | 11 +++++++++++ src/utils/narrow.js | 29 ++++++++++------------------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/boot/store.js b/src/boot/store.js index aeed0796a35..b8c2fdba5d6 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -18,6 +18,7 @@ import ZulipAsyncStorage from './ZulipAsyncStorage'; import createMigration from '../redux-persist-migrate/index'; import { provideLoggingContext } from './loggingContext'; import { tryGetActiveAccount } from '../account/accountsSelectors'; +import { objectFromEntries } from '../jsBackport'; if (process.env.NODE_ENV === 'development') { // Chrome dev tools for Immutable. @@ -237,6 +238,16 @@ const migrations: { [string]: (GlobalState) => GlobalState } = { drafts: {}, }), + // Change format of keys representing PM narrows, dropping emails. + '22': state => ({ + ...dropCache(state), + drafts: objectFromEntries( + Object.keys(state.drafts) + .map(key => key.replace(/^pm:d:(.*?):.*/s, 'pm:$1')) + .map(key => [key, state.drafts[key]]), + ), + }), + // TIP: When adding a migration, consider just using `dropCache`. }; diff --git a/src/utils/narrow.js b/src/utils/narrow.js index 6cfafc58b04..2ebb6230962 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, emails?: string[]): Narrow => + Object.freeze({ type: 'pm', userIds }); /** * A PM narrow, either 1:1 or group. @@ -153,7 +153,7 @@ export const SEARCH_NARROW = (query: string): Narrow => Object.freeze({ type: 's type NarrowCases = {| home: () => T, - pm: (emails: $ReadOnlyArray, ids: $ReadOnlyArray) => T, + pm: (emails: mixed, ids: $ReadOnlyArray) => T, starred: () => T, mentioned: () => T, allPrivate: () => T, @@ -171,7 +171,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(undefined, narrow.userIds); case 'search': return cases.search(narrow.query); case 'all': return cases.home(); case 'starred': return cases.starred(); @@ -237,8 +237,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 +253,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: (emails, ids) => `pm:${ids.join(',')}`, home: () => 'all', starred: () => 'starred', @@ -295,17 +295,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:': { From 1c5557c9a4c2c37f5e67e7a4b87c347689b733f1 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sat, 12 Dec 2020 00:26:49 -0800 Subject: [PATCH 5/7] narrow [nfc]: Drop fake emails value from caseNarrow. Since the previous commit this already doesn't contain anything meaningful -- and its type was `mixed`, to help confirm that no consumers were actually trying to use it. Now delete it. --- src/chat/narrowsSelectors.js | 2 +- src/compose/getComposeInputPlaceholder.js | 2 +- src/outbox/outboxActions.js | 2 +- src/title-buttons/titleButtonFromNarrow.js | 2 +- src/title/Title.js | 2 +- src/unread/unreadSelectors.js | 2 +- src/utils/__tests__/narrow-test.js | 2 +- src/utils/narrow.js | 16 ++++++++-------- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/chat/narrowsSelectors.js b/src/chat/narrowsSelectors.js index 33801c9586c..46970a3e3f7 100644 --- a/src/chat/narrowsSelectors.js +++ b/src/chat/narrowsSelectors.js @@ -141,7 +141,7 @@ export const isNarrowValid: Selector = createSelector( { stream: streamName => streams.find(s => s.name === streamName) !== undefined, topic: streamName => streams.find(s => s.name === streamName) !== undefined, - pm: (emails, ids) => 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/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 2ebb6230962..7eea61c3dda 100644 --- a/src/utils/narrow.js +++ b/src/utils/narrow.js @@ -153,7 +153,7 @@ export const SEARCH_NARROW = (query: string): Narrow => Object.freeze({ type: 's type NarrowCases = {| home: () => T, - pm: (emails: mixed, ids: $ReadOnlyArray) => T, + pm: (ids: $ReadOnlyArray) => T, starred: () => T, mentioned: () => T, allPrivate: () => T, @@ -171,7 +171,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(undefined, 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(); @@ -254,7 +254,7 @@ export function keyFromNarrow(narrow: Narrow): string { // An earlier version had `pm:s:`. // Another had `pm:d:`. - pm: (emails, ids) => `pm:${ids.join(',')}`, + pm: ids => `pm:${ids.join(',')}`, home: () => 'all', starred: () => 'starred', @@ -327,10 +327,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. @@ -339,7 +339,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. @@ -400,7 +400,7 @@ export const apiNarrowOfNarrow = ( { operator: 'stream', operand: streamName }, { operator: 'topic', operand: topic }, ], - pm: (emails_, ids) => { + pm: ids => { const emails = []; for (const id of ids) { const email = allUsersById.get(id)?.email; @@ -446,7 +446,7 @@ export const isMessageInNarrow = ( message.type === 'stream' && streamName === streamNameOfStreamMessage(message) && topic === message.subject, - pm: (emails, ids) => { + pm: ids => { if (message.type !== 'private') { return false; } From 69b017b890f40892640c0a7ee7b4e1b123e90795 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 4 Jan 2021 16:50:40 -0800 Subject: [PATCH 6/7] narrow [nfc]: Drop email argument from internal constructor. --- src/utils/narrow.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/utils/narrow.js b/src/utils/narrow.js index 7eea61c3dda..c5394d4ede6 100644 --- a/src/utils/narrow.js +++ b/src/utils/narrow.js @@ -59,7 +59,7 @@ 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 => +const pmNarrowInternal = (userIds: $ReadOnlyArray): Narrow => Object.freeze({ type: 'pm', userIds }); /** @@ -73,7 +73,7 @@ const pmNarrowInternal = (userIds: $ReadOnlyArray, emails?: string[]): N * different form of input. */ export const pmNarrowFromRecipients = (recipients: PmKeyRecipients): Narrow => - pmNarrowInternal(recipients.map(r => r.id), recipients.map(r => r.email)); + pmNarrowInternal(recipients.map(r => r.id)); /** * 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') { From a6798aaaaf283974ce674bf083cb9e655c72a0e3 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 4 Jan 2021 16:45:42 -0800 Subject: [PATCH 7/7] recipient [nfc]: Cut emails from PmKeyRecipients values. This type exists only to be passed to Narrow constructors; and those no longer need emails, only user IDs. So we can simplify it to contain just the user IDs. --- src/utils/narrow.js | 2 +- src/utils/recipient.js | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/utils/narrow.js b/src/utils/narrow.js index c5394d4ede6..f8e28e9f149 100644 --- a/src/utils/narrow.js +++ b/src/utils/narrow.js @@ -73,7 +73,7 @@ const pmNarrowInternal = (userIds: $ReadOnlyArray): Narrow => * different form of input. */ export const pmNarrowFromRecipients = (recipients: PmKeyRecipients): Narrow => - pmNarrowInternal(recipients.map(r => r.id)); + pmNarrowInternal(recipients); /** * A PM narrow, either 1:1 or group. 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, + ); }; /**