diff --git a/src/boot/store.js b/src/boot/store.js index f18e5e450de..c944a886ebc 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -239,6 +239,21 @@ const migrations: { [string]: (GlobalState) => GlobalState } = { ), }), + // 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]]), + ), + }), + // TIP: When adding a migration, consider just using `dropCache`. }; diff --git a/src/chat/__tests__/narrowsSelectors-test.js b/src/chat/__tests__/narrowsSelectors-test.js index ebc921df2a6..f75f4d775ef 100644 --- a/src/chat/__tests__/narrowsSelectors-test.js +++ b/src/chat/__tests__/narrowsSelectors-test.js @@ -11,7 +11,7 @@ import { import { HOME_NARROW, HOME_NARROW_STR, - pmNarrowFromEmail, + pm1to1NarrowFromUser, streamNarrow, topicNarrow, STARRED_NARROW, @@ -81,14 +81,14 @@ describe('getMessagesForNarrow', () => { test('do not combine messages and outbox in different narrow', () => { const state = eg.reduxState({ narrows: Immutable.Map({ - [keyFromNarrow(pmNarrowFromEmail('john@example.com'))]: [123], + [keyFromNarrow(pm1to1NarrowFromUser(eg.otherUser))]: [123], }), messages, outbox: [outboxMessage], realm: eg.realmState({ email: eg.selfUser.email }), }); - const result = getMessagesForNarrow(state, pmNarrowFromEmail('john@example.com')); + const result = getMessagesForNarrow(state, pm1to1NarrowFromUser(eg.otherUser)); expect(result).toEqual([message]); }); @@ -193,7 +193,7 @@ describe('getStreamInNarrow', () => { }); test('return NULL_SUBSCRIPTION is narrow is not topic or stream', () => { - expect(getStreamInNarrow(state, pmNarrowFromEmail('abc@zulip.com'))).toEqual(NULL_SUBSCRIPTION); + expect(getStreamInNarrow(state, pm1to1NarrowFromUser(eg.otherUser))).toEqual(NULL_SUBSCRIPTION); expect(getStreamInNarrow(state, topicNarrow(stream4.name, 'topic'))).toEqual(NULL_SUBSCRIPTION); }); }); @@ -256,7 +256,7 @@ describe('isNarrowValid', () => { streams: [], users: [user], }); - const narrow = pmNarrowFromEmail(user.email); + const narrow = pm1to1NarrowFromUser(user); const result = isNarrowValid(state, narrow); @@ -274,7 +274,7 @@ describe('isNarrowValid', () => { streams: [], users: [], }); - const narrow = pmNarrowFromEmail(user.email); + const narrow = pm1to1NarrowFromUser(user); const result = isNarrowValid(state, narrow); @@ -331,7 +331,7 @@ describe('isNarrowValid', () => { streams: [], users: [], }); - const narrow = pmNarrowFromEmail(bot.email); + const narrow = pm1to1NarrowFromUser(bot); const result = isNarrowValid(state, narrow); @@ -349,7 +349,7 @@ describe('isNarrowValid', () => { streams: [], users: [], }); - const narrow = pmNarrowFromEmail(notActiveUser.email); + const narrow = pm1to1NarrowFromUser(notActiveUser); const result = isNarrowValid(state, narrow); diff --git a/src/compose/__tests__/getComposeInputPlaceholder-test.js b/src/compose/__tests__/getComposeInputPlaceholder-test.js index d93deef2c8d..eb056545123 100644 --- a/src/compose/__tests__/getComposeInputPlaceholder-test.js +++ b/src/compose/__tests__/getComposeInputPlaceholder-test.js @@ -3,7 +3,7 @@ import deepFreeze from 'deep-freeze'; import getComposeInputPlaceholder from '../getComposeInputPlaceholder'; import { - pmNarrowFromEmail, + pm1to1NarrowFromUser, streamNarrow, topicNarrow, pmNarrowFromUsersUnsafe, @@ -15,7 +15,7 @@ describe('getComposeInputPlaceholder', () => { const ownEmail = eg.selfUser.email; test('returns "Message @ThisPerson" object for person narrow', () => { - const narrow = deepFreeze(pmNarrowFromEmail(eg.otherUser.email)); + const narrow = deepFreeze(pm1to1NarrowFromUser(eg.otherUser)); const placeholder = getComposeInputPlaceholder(narrow, ownEmail, usersByEmail); expect(placeholder).toEqual({ text: 'Message {recipient}', @@ -24,7 +24,7 @@ describe('getComposeInputPlaceholder', () => { }); test('returns "Jot down something" object for self narrow', () => { - const narrow = deepFreeze(pmNarrowFromEmail(eg.selfUser.email)); + const narrow = deepFreeze(pm1to1NarrowFromUser(eg.selfUser)); const placeholder = getComposeInputPlaceholder(narrow, ownEmail, usersByEmail); expect(placeholder).toEqual({ text: 'Jot down something' }); }); diff --git a/src/notification/__tests__/notification-test.js b/src/notification/__tests__/notification-test.js index c48bf1ed312..1bc46de10f1 100644 --- a/src/notification/__tests__/notification-test.js +++ b/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, pmNarrowFromUsersUnsafe } from '../../utils/narrow'; +import { topicNarrow, pm1to1NarrowFromUser, pmNarrowFromUsersUnsafe } from '../../utils/narrow'; import * as eg from '../../__tests__/lib/exampleData'; import { fromAPNsImpl as extractIosNotificationData } from '../extract'; @@ -17,7 +17,7 @@ describe('getNarrowFromNotificationData', () => { test('unknown notification data returns null', () => { // $FlowFixMe: actually validate APNs messages const notification: Notification = {}; - const narrow = getNarrowFromNotificationData(notification, DEFAULT_MAP, ownUserId); + const narrow = getNarrowFromNotificationData(notification, DEFAULT_MAP, new Map(), ownUserId); expect(narrow).toBe(null); }); @@ -27,22 +27,31 @@ describe('getNarrowFromNotificationData', () => { stream: 'some stream', topic: 'some topic', }; - const narrow = getNarrowFromNotificationData(notification, DEFAULT_MAP, ownUserId); + const narrow = getNarrowFromNotificationData(notification, DEFAULT_MAP, new Map(), ownUserId); expect(narrow).toEqual(topicNarrow('some stream', 'some topic')); }); test('on notification for a private message returns a PM narrow', () => { + const users = [eg.selfUser, eg.otherUser]; + const allUsersById: Map = new Map(users.map(u => [u.user_id, u])); + const allUsersByEmail: Map = new Map(users.map(u => [u.email, u])); const notification = { recipient_type: 'private', - sender_email: 'mark@example.com', + sender_email: eg.otherUser.email, }; - const narrow = getNarrowFromNotificationData(notification, DEFAULT_MAP, ownUserId); - expect(narrow).toEqual(pmNarrowFromEmail('mark@example.com')); + const narrow = getNarrowFromNotificationData( + notification, + allUsersById, + allUsersByEmail, + ownUserId, + ); + expect(narrow).toEqual(pm1to1NarrowFromUser(eg.otherUser)); }); test('on notification for a group message returns a group narrow', () => { const users = [eg.selfUser, eg.makeUser(), eg.makeUser(), eg.makeUser()]; const allUsersById: Map = new Map(users.map(u => [u.user_id, u])); + const allUsersByEmail: Map = new Map(users.map(u => [u.email, u])); const notification = { recipient_type: 'private', @@ -51,7 +60,12 @@ describe('getNarrowFromNotificationData', () => { const expectedNarrow = pmNarrowFromUsersUnsafe(users.slice(1)); - const narrow = getNarrowFromNotificationData(notification, allUsersById, ownUserId); + const narrow = getNarrowFromNotificationData( + notification, + allUsersById, + allUsersByEmail, + ownUserId, + ); expect(narrow).toEqual(expectedNarrow); }); @@ -63,7 +77,7 @@ describe('getNarrowFromNotificationData', () => { }; const allUsersById = new Map(); - const narrow = getNarrowFromNotificationData(notification, allUsersById, ownUserId); + const narrow = getNarrowFromNotificationData(notification, allUsersById, new Map(), ownUserId); expect(narrow).toBe(null); }); diff --git a/src/notification/index.js b/src/notification/index.js index 03e0b6a7ad0..52300f52b23 100644 --- a/src/notification/index.js +++ b/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, pmNarrowFromUsers } from '../utils/narrow'; +import { topicNarrow, pmNarrowFromUsers, pm1to1NarrowFromUser } from '../utils/narrow'; import type { JSONable, JSONableDict } from '../utils/jsonable'; import * as api from '../api'; import * as logging from '../utils/logging'; @@ -86,6 +86,7 @@ export const getAccountFromNotificationData = ( export const getNarrowFromNotificationData = ( data: Notification, allUsersById: Map, + allUsersByEmail: Map, ownUserId: number, ): Narrow | null => { if (!data.recipient_type) { @@ -102,7 +103,8 @@ export const getNarrowFromNotificationData = ( } if (data.pm_users === undefined) { - return pmNarrowFromEmail(data.sender_email); + const user = allUsersByEmail.get(data.sender_email); + return (user && pm1to1NarrowFromUser(user)) ?? null; } const ids = data.pm_users.split(',').map(s => parseInt(s, 10)); diff --git a/src/notification/notificationActions.js b/src/notification/notificationActions.js index cdab584e6c9..8698d348164 100644 --- a/src/notification/notificationActions.js +++ b/src/notification/notificationActions.js @@ -14,7 +14,7 @@ import { getAuth, getActiveAccount } from '../selectors'; import { getSession, getAccounts } from '../directSelectors'; import { GOT_PUSH_TOKEN, ACK_PUSH_TOKEN, UNACK_PUSH_TOKEN } from '../actionConstants'; import { identityOfAccount, authOfAccount } from '../account/accountMisc'; -import { getAllUsersById, getOwnUserId } from '../users/userSelectors'; +import { getAllUsersByEmail, getAllUsersById, getOwnUserId } from '../users/userSelectors'; import { doNarrow } from '../message/messagesActions'; import { accountSwitch } from '../account/accountActions'; import { getIdentities } from '../account/accountsSelectors'; @@ -52,7 +52,12 @@ export const narrowToNotification = (data: ?Notification) => ( return; } - const narrow = getNarrowFromNotificationData(data, getAllUsersById(state), getOwnUserId(state)); + const narrow = getNarrowFromNotificationData( + data, + getAllUsersById(state), + getAllUsersByEmail(state), + getOwnUserId(state), + ); if (narrow) { dispatch(doNarrow(narrow)); } diff --git a/src/title/__tests__/titleSelectors-test.js b/src/title/__tests__/titleSelectors-test.js index c944bdaafce..fba7f1c2e8e 100644 --- a/src/title/__tests__/titleSelectors-test.js +++ b/src/title/__tests__/titleSelectors-test.js @@ -1,7 +1,7 @@ /* @flow strict-local */ import { DEFAULT_TITLE_BACKGROUND_COLOR, getTitleBackgroundColor } from '../titleSelectors'; -import { pmNarrowFromUsersUnsafe, streamNarrow, pmNarrowFromEmail } from '../../utils/narrow'; +import { pmNarrowFromUsersUnsafe, streamNarrow, pm1to1NarrowFromUser } from '../../utils/narrow'; import * as eg from '../../__tests__/lib/exampleData'; describe('getTitleBackgroundColor', () => { @@ -24,7 +24,7 @@ describe('getTitleBackgroundColor', () => { }); test('return default for non topic/stream narrow', () => { - expect(getTitleBackgroundColor(state, pmNarrowFromEmail(eg.otherUser.email))).toEqual( + expect(getTitleBackgroundColor(state, pm1to1NarrowFromUser(eg.otherUser))).toEqual( DEFAULT_TITLE_BACKGROUND_COLOR, ); expect( diff --git a/src/utils/__tests__/narrow-test.js b/src/utils/__tests__/narrow-test.js index d11d60e4620..a151d9ed740 100644 --- a/src/utils/__tests__/narrow-test.js +++ b/src/utils/__tests__/narrow-test.js @@ -3,10 +3,9 @@ import { HOME_NARROW, isHomeNarrow, - pmNarrowFromEmail, + pm1to1NarrowFromUser, is1to1PmNarrow, pmNarrowFromUsersUnsafe, - specialNarrow, isSpecialNarrow, ALL_PRIVATE_NARROW, streamNarrow, @@ -17,14 +16,12 @@ import { isSearchNarrow, isPmNarrow, isMessageInNarrow, - isSameNarrow, isStreamOrTopicNarrow, getNarrowsForMessage, getNarrowForReply, parseNarrow, STARRED_NARROW, MENTIONED_NARROW, - pm1to1NarrowFromUser, keyFromNarrow, streamNameOfNarrow, topicOfNarrow, @@ -39,16 +36,16 @@ describe('HOME_NARROW', () => { }); }); -describe('pmNarrowFromEmail', () => { +describe('pm1to1NarrowFromUser', () => { test('produces a 1:1 narrow', () => { - const narrow = pmNarrowFromEmail(eg.otherUser.email); + const narrow = pm1to1NarrowFromUser(eg.otherUser); expect(is1to1PmNarrow(narrow)).toBeTrue(); expect(emailOfPm1to1Narrow(narrow)).toEqual(eg.otherUser.email); }); test('if operator is "pm-with" and only one email, then it is a private narrow', () => { expect(is1to1PmNarrow(HOME_NARROW)).toBe(false); - expect(is1to1PmNarrow(pmNarrowFromEmail(eg.otherUser.email))).toBe(true); + expect(is1to1PmNarrow(pm1to1NarrowFromUser(eg.otherUser))).toBe(true); }); }); @@ -56,7 +53,7 @@ describe('isPmNarrow', () => { test('a private or group narrow is any "pm-with" narrow', () => { expect(isPmNarrow(undefined)).toBe(false); expect(isPmNarrow(HOME_NARROW)).toBe(false); - expect(isPmNarrow(pmNarrowFromEmail(eg.otherUser.email))).toBe(true); + expect(isPmNarrow(pm1to1NarrowFromUser(eg.otherUser))).toBe(true); expect(isPmNarrow(pmNarrowFromUsersUnsafe([eg.otherUser, eg.thirdUser]))).toBe(true); }); }); @@ -67,7 +64,7 @@ describe('isStreamOrTopicNarrow', () => { expect(isStreamOrTopicNarrow(streamNarrow('some stream'))).toBe(true); expect(isStreamOrTopicNarrow(topicNarrow('some stream', 'some topic'))).toBe(true); expect(isStreamOrTopicNarrow(HOME_NARROW)).toBe(false); - expect(isStreamOrTopicNarrow(pmNarrowFromEmail(eg.otherUser.email))).toBe(false); + expect(isStreamOrTopicNarrow(pm1to1NarrowFromUser(eg.otherUser))).toBe(false); expect(isStreamOrTopicNarrow(pmNarrowFromUsersUnsafe([eg.otherUser, eg.thirdUser]))).toBe( false, ); @@ -143,7 +140,7 @@ describe('isMessageInNarrow', () => { ['PM', false, eg.pmMessage()], ]], - ['1:1 PM conversation, non-self', pmNarrowFromEmail(eg.otherUser.email), [ + ['1:1 PM conversation, non-self', pm1to1NarrowFromUser(eg.otherUser), [ ['matching PM, inbound', true, eg.pmMessage()], ['matching PM, outbound', true, eg.pmMessage({ sender: eg.selfUser })], ['self-1:1 message', false, eg.pmMessage({ sender: eg.selfUser, recipients: [eg.selfUser] })], @@ -151,7 +148,7 @@ describe('isMessageInNarrow', () => { ['group-PM including this user, outbound', false, eg.pmMessage({ sender: eg.selfUser, recipients: [eg.selfUser, eg.otherUser, eg.thirdUser] })], ['stream message', false, eg.streamMessage()], ]], - ['self-1:1 conversation', pmNarrowFromEmail(eg.selfUser.email), [ + ['self-1:1 conversation', pm1to1NarrowFromUser(eg.selfUser), [ ['self-1:1 message', true, eg.pmMessage({ sender: eg.selfUser, recipients: [eg.selfUser] })], ['other 1:1 message, inbound', false, eg.pmMessage()], ['other 1:1 message, outbound', false, eg.pmMessage({ sender: eg.selfUser })], @@ -281,7 +278,7 @@ describe('getNarrowsForMessage', () => { { label: 'PM message with one person', message: eg.pmMessage({ sender: eg.otherUser }), - expectedNarrows: [HOME_NARROW, ALL_PRIVATE_NARROW, pmNarrowFromEmail(eg.otherUser.email)], + expectedNarrows: [HOME_NARROW, ALL_PRIVATE_NARROW, pm1to1NarrowFromUser(eg.otherUser)], }, { label: 'Group PM message', @@ -309,12 +306,12 @@ describe('getNarrowForReply', () => { eg.pmMessage({ sender: eg.selfUser, recipients: [eg.selfUser] }), eg.selfUser, ), - ).toEqual(pmNarrowFromEmail(eg.selfUser.email)); + ).toEqual(pm1to1NarrowFromUser(eg.selfUser)); }); test('for 1:1 PM, returns a 1:1 PM narrow', () => { const message = eg.pmMessage(); - const expectedNarrow = pmNarrowFromEmail(eg.otherUser.email); + const expectedNarrow = pm1to1NarrowFromUser(eg.otherUser); const actualNarrow = getNarrowForReply(message, eg.selfUser); @@ -342,20 +339,6 @@ describe('getNarrowForReply', () => { }); }); -describe('isSameNarrow', () => { - test('Return true if two narrows are same', () => { - expect(isSameNarrow(streamNarrow('stream'), streamNarrow('stream'))).toBe(true); - expect(isSameNarrow(streamNarrow('stream'), streamNarrow('stream1'))).toBe(false); - expect(isSameNarrow(streamNarrow('stream'), topicNarrow('stream', 'topic'))).toBe(false); - expect(isSameNarrow(topicNarrow('stream', 'topic'), topicNarrow('stream', 'topic'))).toBe(true); - expect(isSameNarrow(topicNarrow('stream', 'topic'), topicNarrow('stream', 'topic1'))).toBe( - false, - ); - expect(isSameNarrow(HOME_NARROW, specialNarrow('private'))).toBe(false); - expect(isSameNarrow(HOME_NARROW, HOME_NARROW)).toBe(true); - }); -}); - describe('keyFromNarrow+parseNarrow', () => { const baseNarrows = [ ['whole stream', streamNarrow(eg.stream.name)], diff --git a/src/utils/internalLinks.js b/src/utils/internalLinks.js index 3abbb7aaffe..c7842eefa0d 100644 --- a/src/utils/internalLinks.js +++ b/src/utils/internalLinks.js @@ -142,7 +142,11 @@ export const getNarrowFromLink = ( case 'stream': return streamNarrow(parseStreamOperand(paths[1], streamsById)); case 'special': - return specialNarrow(paths[1]); + try { + return specialNarrow(paths[1]); + } catch (e) { + return null; + } default: return null; } diff --git a/src/utils/narrow.js b/src/utils/narrow.js index 244a95b6814..03c6b8469eb 100644 --- a/src/utils/narrow.js +++ b/src/utils/narrow.js @@ -1,6 +1,5 @@ /* @flow strict-local */ import invariant from 'invariant'; -import isEqual from 'lodash.isequal'; import type { ApiNarrow, Message, Outbox, User, UserOrBot } from '../types'; import { @@ -31,32 +30,17 @@ import { * * `ApiNarrow` for the form we put a narrow in when talking to the * server, and `apiNarrowOfNarrow` for converting to it. */ -export opaque type Narrow = ApiNarrow; +export opaque type Narrow = + | {| type: 'stream', streamName: string |} + | {| type: 'topic', streamName: string, topic: string |} + | {| type: 'pm', userIds: $ReadOnlyArray, emails: $ReadOnlyArray |} + | {| type: 'search', query: string |} + | {| type: 'all' | 'starred' | 'mentioned' | 'all-pm' |}; -export const isSameNarrow = (narrow1: Narrow, narrow2: Narrow): boolean => - Array.isArray(narrow1) && Array.isArray(narrow2) && isEqual(narrow1, narrow2); - -export const HOME_NARROW: Narrow = []; +export const HOME_NARROW: Narrow = Object.freeze({ type: 'all' }); export const HOME_NARROW_STR: string = keyFromNarrow(HOME_NARROW); -/** - * A PM narrow, either 1:1 or group. - * - * Private to this module because the input format is kind of odd. - * Use `pmNarrowFromEmail` or `pmNarrowFromEmails` instead. - * - * For the quirks of the underlying format in the Zulip API, see: - * https://zulipchat.com/api/construct-narrow - * https://github.com/zulip/zulip/issues/13167 - */ -const pmNarrowByString = (emails: string): Narrow => [ - { - operator: 'pm-with', - operand: emails, - }, -]; - /** * A PM narrow, either 1:1 or group. * @@ -76,16 +60,8 @@ const pmNarrowByString = (emails: string): 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 pmNarrowFromEmails = (emails: string[]): Narrow => pmNarrowByString(emails.join()); - -/** - * DEPRECATED. Use `pm1to1NarrowFromUser` instead. - * - * This function is being replaced by `pm1to1NarrowFromUser`, as part of - * migrating from emails to user IDs to identify users. Don't add new uses - * of this function; use that one instead. - */ -export const pmNarrowFromEmail = (email: string): Narrow => pmNarrowFromEmails([email]); +const pmNarrowInternal = (userIds: $ReadOnlyArray, emails: string[]): Narrow => + Object.freeze({ type: 'pm', userIds, emails }); /** * A PM narrow, either 1:1 or group. @@ -98,7 +74,7 @@ export const pmNarrowFromEmail = (email: string): Narrow => pmNarrowFromEmails([ * different form of input. */ export const pmNarrowFromRecipients = (recipients: PmKeyRecipients): Narrow => - pmNarrowFromEmails(recipients.map(r => r.email)); + pmNarrowInternal(recipients.map(r => r.id), recipients.map(r => r.email)); /** * A PM narrow, either 1:1 or group. @@ -110,7 +86,7 @@ export const pmNarrowFromRecipients = (recipients: PmKeyRecipients): Narrow => * single specific user. */ export const pmNarrowFromUsers = (recipients: PmKeyUsers): Narrow => - pmNarrowFromEmails(recipients.map(r => r.email)); + pmNarrowInternal(recipients.map(r => r.user_id), recipients.map(r => r.email)); /** * FOR TESTS ONLY. Like pmNarrowFromUsers, but without validation. @@ -128,8 +104,10 @@ export const pmNarrowFromUsers = (recipients: PmKeyUsers): Narrow => */ // It'd be fine for test data to go through the usual filtering logic; the // annoying thing is just that that requires an ownUserId value. -export const pmNarrowFromUsersUnsafe = (recipients: UserOrBot[]): Narrow => - pmNarrowFromEmails(recipients.sort((a, b) => a.user_id - b.user_id).map(r => r.email)); +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)); +}; /** * A 1:1 PM narrow, possibly with self. @@ -138,14 +116,21 @@ 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 => pmNarrowFromEmails([user.email]); +export const pm1to1NarrowFromUser = (user: UserOrBot): Narrow => + pmNarrowInternal([user.user_id], [user.email]); -export const specialNarrow = (operand: string): Narrow => [ - { - operator: 'is', - operand, - }, -]; +export const specialNarrow = (operand: string): Narrow => { + if (operand === 'starred') { + return Object.freeze({ type: 'starred' }); + } + if (operand === 'mentioned') { + return Object.freeze({ type: 'mentioned' }); + } + if (operand === 'private') { + return Object.freeze({ type: 'all-pm' }); + } + throw new Error(`specialNarrow: got unsupported operand: ${operand}`); +}; export const STARRED_NARROW = specialNarrow('starred'); @@ -159,34 +144,17 @@ export const ALL_PRIVATE_NARROW = specialNarrow('private'); export const ALL_PRIVATE_NARROW_STR = keyFromNarrow(ALL_PRIVATE_NARROW); -export const streamNarrow = (stream: string): Narrow => [ - { - operator: 'stream', - operand: stream, - }, -]; - -export const topicNarrow = (stream: string, topic: string): Narrow => [ - { - operator: 'stream', - operand: stream, - }, - { - operator: 'topic', - operand: topic, - }, -]; - -export const SEARCH_NARROW = (query: string): Narrow => [ - { - operator: 'search', - operand: query, - }, -]; +export const streamNarrow = (stream: string): Narrow => + Object.freeze({ type: 'stream', streamName: stream }); + +export const topicNarrow = (stream: string, topic: string): Narrow => + Object.freeze({ type: 'topic', streamName: stream, topic }); + +export const SEARCH_NARROW = (query: string): Narrow => Object.freeze({ type: 'search', query }); type NarrowCases = {| home: () => T, - pm: (emails: string[]) => T, + pm: (emails: $ReadOnlyArray, ids: $ReadOnlyArray) => T, starred: () => T, mentioned: () => T, allPrivate: () => T, @@ -201,26 +169,15 @@ export function caseNarrow(narrow: Narrow, cases: NarrowCases): T { throw new Error(`bad narrow: ${JSON.stringify(narrow)}`); }; - switch (narrow.length) { - case 0: return cases.home(); - case 1: - switch (narrow[0].operator) { - case 'pm-with': { - const emails = narrow[0].operand.split(','); - return cases.pm(emails); - } - case 'is': - switch (narrow[0].operand) { - case 'starred': return cases.starred(); - case 'mentioned': return cases.mentioned(); - case 'private': return cases.allPrivate(); - default: return err(); - } - case 'stream': return cases.stream(narrow[0].operand); - case 'search': return cases.search(narrow[0].operand); - default: return err(); - } - case 2: return cases.topic(narrow[0].operand, narrow[1].operand); + 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 'search': return cases.search(narrow.query); + case 'all': return cases.home(); + case 'starred': return cases.starred(); + case 'mentioned': return cases.mentioned(); + case 'all-pm': return cases.allPrivate(); default: return err(); } } @@ -284,8 +241,10 @@ export function caseNarrowDefault( // 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. export function keyFromNarrow(narrow: Narrow): string { - // The ":s" bit in several of these is to keep them disjoint, out of an - // abundance of caution, from future keys that use numeric IDs. + // 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. + // + // Similarly, ":d" ("dual") marks those with both numeric IDs and strings. return caseNarrow(narrow, { // NB if you're changing any of these: be sure to do a migration. // Take a close look at migration 19 and any later related migrations. @@ -295,7 +254,8 @@ export function keyFromNarrow(narrow: Narrow): string { // (See `check_stream_name` in zulip.git:zerver/lib/streams.py.) topic: (streamName, topic) => `topic:s:${streamName}\x00${topic}`, - pm: emails => `pm:s:${emails.join(',')}`, + // An earlier version had `pm:s:`. + pm: (emails, ids) => `pm:d:${ids.join(',')}:${emails.join(',')}`, home: () => 'all', starred: () => 'starred', @@ -336,11 +296,17 @@ export const parseNarrow = (narrowStr: string): Narrow => { } case 'pm:': { - if (!rest.startsWith('s:')) { + // 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 emails = rest.substr('s:'.length).split(','); - return pmNarrowFromEmails(emails); + const ids = match[1].split(',').map(s => Number.parseInt(s, 10)); + const emails = match[2].split(','); + return pmNarrowInternal(ids, emails); } case 'search:': { @@ -382,7 +348,7 @@ export const isGroupPmNarrow = (narrow?: Narrow): boolean => * Any use of this probably means something higher up should be refactored * to use caseNarrow. */ -export const emailsOfGroupPmNarrow = (narrow: Narrow): string[] => +export const emailsOfGroupPmNarrow = (narrow: Narrow): $ReadOnlyArray => caseNarrowPartial(narrow, { pm: emails => { if (emails.length === 1) { @@ -417,7 +383,7 @@ export const emailOfPm1to1Narrow = (narrow: Narrow): string => * This is the same list of users that can appear in a `PmKeyRecipients` or * `PmKeyUsers`, but contains only their emails. */ -export const emailsOfPmNarrow = (narrow: Narrow): string[] => +export const emailsOfPmNarrow = (narrow: Narrow): $ReadOnlyArray => caseNarrowPartial(narrow, { pm: emails => emails }); /** @@ -469,7 +435,20 @@ 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 => narrow; +export const apiNarrowOfNarrow = (narrow: Narrow): 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(',') }], + search: query => [{ operator: 'search', operand: query }], + home: () => [], + starred: () => [{ operator: 'is', operand: 'starred' }], + mentioned: () => [{ operator: 'is', operand: 'mentioned' }], + allPrivate: () => [{ operator: 'is', operand: 'private' }], + }); /** * True just if the given message is part of the given narrow.