diff --git a/src/__tests__/lib/exampleData.js b/src/__tests__/lib/exampleData.js index 3abb814e224..00295a1882c 100644 --- a/src/__tests__/lib/exampleData.js +++ b/src/__tests__/lib/exampleData.js @@ -147,7 +147,7 @@ export const stream: Stream = makeStream({ description: 'An example stream.', }); -const displayRecipientFromUser = (user: User): PmRecipientUser => { +export const displayRecipientFromUser = (user: User): PmRecipientUser => { const { email, full_name, user_id: id } = user; return deepFreeze({ email, @@ -337,6 +337,7 @@ export const action = deepFreeze({ realm_users: [], user_id: 4, realm_user_groups: [], + recent_private_conversations: [], streams: [], never_subscribed: [], subscriptions: [], diff --git a/src/api/initialDataTypes.js b/src/api/initialDataTypes.js index 4ec53caa21b..e24b5a03639 100644 --- a/src/api/initialDataTypes.js +++ b/src/api/initialDataTypes.js @@ -4,6 +4,7 @@ import type { CrossRealmBot, RealmEmojiById, RealmFilter, + RecentPrivateConversation, Stream, Subscription, User, @@ -107,6 +108,10 @@ export type InitialDataRealmUserGroups = {| realm_user_groups: UserGroup[], |}; +export type InitialDataRecentPmConversations = {| + recent_private_conversations: RecentPrivateConversation[], +|}; + type NeverSubscribedStream = {| description: string, invite_only: boolean, @@ -273,6 +278,7 @@ export type InitialData = {| ...InitialDataRealmFilters, ...InitialDataRealmUser, ...InitialDataRealmUserGroups, + ...InitialDataRecentPmConversations, ...InitialDataStream, ...InitialDataSubscription, ...InitialDataUpdateDisplaySettings, diff --git a/src/api/modelTypes.js b/src/api/modelTypes.js index ac0eec32c3e..960000f2ea0 100644 --- a/src/api/modelTypes.js +++ b/src/api/modelTypes.js @@ -536,3 +536,26 @@ export type Message = $ReadOnly<{ subject: string, subject_links: $ReadOnlyArray, }>; + +// +// +// +// =================================================================== +// Summaries of messages and conversations. +// +// + +/** + * Describes a recent PM conversation. + * + * See https://github.com/zulip/zulip/commit/4c3c669b4#diff-2c2aa234cb4a0120fa5f2eeaf8a94fa2R283 + * for the structure of this type and the meaning of its properties. + * + * `user_ids` does not contain the `user_id` of the current user. Consequently, + * a user's conversation with themselves will return [], which is unlike the + * behaviour found in other parts of codebase. + */ +export type RecentPrivateConversation = {| + max_message_id: number, + user_ids: number[], +|}; diff --git a/src/boot/reducers.js b/src/boot/reducers.js index a112ef84259..f418072e511 100644 --- a/src/boot/reducers.js +++ b/src/boot/reducers.js @@ -21,6 +21,7 @@ import nav from '../nav/navReducer'; import outbox from '../outbox/outboxReducer'; import presence from '../presence/presenceReducer'; import realm from '../realm/realmReducer'; +import recentPrivateConversations from '../pm-conversations/recentPmConversationsReducer'; import session from '../session/sessionReducer'; import settings from '../settings/settingsReducer'; import streams from '../streams/streamsReducer'; @@ -50,6 +51,7 @@ const reducers = { outbox, presence, realm, + recentPrivateConversations, session, settings, streams, diff --git a/src/boot/store.js b/src/boot/store.js index db6dbda2d58..c3a9ac398a2 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -44,9 +44,9 @@ export const storeKeys: Array<$Keys> = [ * don't have to re-download it. */ // prettier-ignore -export const cacheKeys: Array<$Keys> = [ - 'flags', 'messages', 'mute', 'narrows', 'realm', 'streams', - 'subscriptions', 'unread', 'userGroups', 'users', +export const cacheKeys = [ + 'flags', 'messages', 'mute', 'narrows', 'realm', 'recentPrivateConversations', + 'streams', 'subscriptions', 'unread', 'userGroups', 'users', ]; /** diff --git a/src/config.js b/src/config.js index 1a893b21223..e260a14e358 100644 --- a/src/config.js +++ b/src/config.js @@ -32,6 +32,7 @@ const config: Config = { 'realm_filters', 'realm_user', 'realm_user_groups', + 'recent_private_conversations', 'stream', 'subscription', 'update_display_settings', diff --git a/src/directSelectors.js b/src/directSelectors.js index 7bc77e89e82..201d07695c7 100644 --- a/src/directSelectors.js +++ b/src/directSelectors.js @@ -23,6 +23,7 @@ import type { Subscription, Stream, Outbox, + RecentPrivateConversation, User, UserGroup, UserStatusState, @@ -83,6 +84,9 @@ export const getSubscriptions = (state: GlobalState): Subscription[] => state.su */ export const getStreams = (state: GlobalState): Stream[] => state.streams; +export const getRecentPrivateConversations = (state: GlobalState): RecentPrivateConversation[] => + state.recentPrivateConversations; + export const getPresence = (state: GlobalState): PresenceState => state.presence; export const getOutbox = (state: GlobalState): Outbox[] => state.outbox; diff --git a/src/pm-conversations/__tests__/pmConversationsSelectors-test.js b/src/pm-conversations/__tests__/pmConversationsSelectors-test.js index f665209a1ac..24d4bbd5da4 100644 --- a/src/pm-conversations/__tests__/pmConversationsSelectors-test.js +++ b/src/pm-conversations/__tests__/pmConversationsSelectors-test.js @@ -1,246 +1,155 @@ -import deepFreeze from 'deep-freeze'; +/* @flow strict-local */ import { getRecentConversations } from '../pmConversationsSelectors'; -import { ALL_PRIVATE_NARROW_STR } from '../../utils/narrow'; +import * as eg from '../../__tests__/lib/exampleData'; describe('getRecentConversations', () => { - test('when no messages, return no conversations', () => { - const state = deepFreeze({ - realm: { email: 'me@example.com' }, - users: [{ user_id: 0, email: 'me@example.com' }], - narrows: { - [ALL_PRIVATE_NARROW_STR]: [], - }, - unread: { - pms: [], - huddles: [], - }, + test('when no recent conversations, return no conversations', () => { + const state = eg.reduxState({ + realm: eg.realmState({ email: eg.selfUser.email }), + users: [eg.selfUser], }); - const actual = getRecentConversations(state); - - expect(actual).toEqual([]); + expect(getRecentConversations(state)).toEqual([]); }); test('returns unique list of recipients, includes conversations with self', () => { - const state = deepFreeze({ - realm: { email: 'me@example.com' }, - users: [ - { user_id: 0, email: 'me@example.com' }, - { user_id: 1, email: 'john@example.com' }, - { user_id: 2, email: 'mark@example.com' }, - ], - narrows: { - [ALL_PRIVATE_NARROW_STR]: [0, 1, 2, 3, 4], - }, - messages: { - 1: { - id: 1, - type: 'private', - display_recipient: [ - { id: 0, email: 'me@example.com' }, - { id: 1, email: 'john@example.com' }, - ], - }, - 2: { - id: 2, - type: 'private', - display_recipient: [ - { id: 0, email: 'me@example.com' }, - { id: 2, email: 'mark@example.com' }, - ], + const users = [eg.selfUser, eg.makeUser({ name: 'john' }), eg.makeUser({ name: 'mark' })]; + const recentPrivateConversations = [ + { max_message_id: 4, user_ids: [] }, + { max_message_id: 3, user_ids: [users[1].user_id] }, + { max_message_id: 2, user_ids: [users[2].user_id] }, + { max_message_id: 0, user_ids: [users[1].user_id, users[2].user_id] }, + ]; + const unread = { + streams: [], + huddles: [ + { + user_ids_string: [eg.selfUser.user_id, users[1].user_id, users[2].user_id] + .sort((a, b) => a - b) + .join(), + unread_message_ids: [5], }, - 3: { - id: 3, - type: 'private', - display_recipient: [ - { id: 0, email: 'me@example.com' }, - { id: 1, email: 'john@example.com' }, - ], + ], + pms: [ + { + sender_id: eg.selfUser.user_id, + unread_message_ids: [4], }, - 4: { - id: 4, - type: 'private', - display_recipient: [{ id: 0, email: 'me@example.com' }], + { + sender_id: users[1].user_id, + unread_message_ids: [1, 3], }, - 0: { - id: 0, - type: 'private', - display_recipient: [ - { id: 0, email: 'me@example.com' }, - { id: 1, email: 'john@example.com' }, - { id: 2, email: 'mark@example.com' }, - ], + { + sender_id: users[2].user_id, + unread_message_ids: [2], }, - }, - unread: { - pms: [ - { - sender_id: 0, - unread_message_ids: [4], - }, - { - sender_id: 1, - unread_message_ids: [1, 3], - }, - { - sender_id: 2, - unread_message_ids: [2], - }, - ], - huddles: [ - { - user_ids_string: '0,1,2', - unread_message_ids: [5], - }, - ], - }, + ], + mentions: [], + }; + + const state = eg.reduxState({ + realm: eg.realmState({ email: eg.selfUser.email }), + users, + recentPrivateConversations, + unread, }); - const expectedResult = [ + expect(getRecentConversations(state)).toEqual([ { - ids: '0', - recipients: 'me@example.com', + ids: eg.selfUser.user_id.toString(), + recipients: eg.selfUser.email, msgId: 4, unread: 1, }, { - ids: '1', - recipients: 'john@example.com', + ids: users[1].user_id.toString(), + recipients: users[1].email, msgId: 3, unread: 2, }, { - ids: '2', - recipients: 'mark@example.com', + ids: users[2].user_id.toString(), + recipients: users[2].email, msgId: 2, unread: 1, }, { - ids: '0,1,2', - recipients: 'john@example.com,mark@example.com', + ids: [eg.selfUser.user_id, users[1].user_id, users[2].user_id].sort((a, b) => a - b).join(), + recipients: [users[1].email, users[2].email].join(), msgId: 0, unread: 1, }, - ]; - - const actual = getRecentConversations(state); - - expect(actual).toEqual(expectedResult); + ]); }); test('returns recipients sorted by last activity', () => { - const state = deepFreeze({ - realm: { email: 'me@example.com' }, - users: [ - { user_id: 0, email: 'me@example.com' }, - { user_id: 1, email: 'john@example.com' }, - { user_id: 2, email: 'mark@example.com' }, - ], - narrows: { - [ALL_PRIVATE_NARROW_STR]: [1, 2, 3, 4, 5, 6], - }, - messages: { - 2: { - id: 2, - type: 'private', - display_recipient: [ - { id: 0, email: 'me@example.com' }, - { id: 1, email: 'john@example.com' }, - ], - }, - 1: { - id: 1, - type: 'private', - display_recipient: [ - { id: 0, email: 'me@example.com' }, - { id: 2, email: 'mark@example.com' }, - ], - }, - 4: { - id: 4, - type: 'private', - display_recipient: [ - { id: 0, email: 'me@example.com' }, - { id: 1, email: 'john@example.com' }, - ], + const users = [eg.selfUser, eg.makeUser({ name: 'john' }), eg.makeUser({ name: 'mark' })]; + const recentPrivateConversations = [ + { max_message_id: 6, user_ids: [] }, + { max_message_id: 5, user_ids: [users[1].user_id, users[2].user_id] }, + { max_message_id: 4, user_ids: [users[1].user_id] }, + { max_message_id: 3, user_ids: [users[2].user_id] }, + ]; + const unread = { + streams: [], + huddles: [ + { + user_ids_string: [eg.selfUser.user_id, users[1].user_id, users[2].user_id] + .sort((a, b) => a - b) + .join(), + unread_message_ids: [5], }, - 3: { - id: 3, - type: 'private', - display_recipient: [ - { id: 0, email: 'me@example.com' }, - { id: 2, email: 'mark@example.com' }, - ], + ], + pms: [ + { + sender_id: eg.selfUser.user_id, + unread_message_ids: [4], }, - 5: { - id: 5, - type: 'private', - display_recipient: [ - { id: 0, email: 'me@example.com' }, - { id: 1, email: 'john@example.com' }, - { id: 2, email: 'mark@example.com' }, - ], + { + sender_id: users[1].user_id, + unread_message_ids: [1, 3], }, - 6: { - id: 6, - type: 'private', - display_recipient: [{ id: 0, email: 'me@example.com' }], + { + sender_id: users[2].user_id, + unread_message_ids: [2], }, - }, - unread: { - pms: [ - { - sender_id: 0, - unread_message_ids: [4], - }, - { - sender_id: 1, - unread_message_ids: [1, 3], - }, - { - sender_id: 2, - unread_message_ids: [2], - }, - ], - huddles: [ - { - user_ids_string: '0,1,2', - unread_message_ids: [5], - }, - ], - }, + ], + mentions: [], + }; + + const state = eg.reduxState({ + realm: eg.realmState({ email: eg.selfUser.email }), + users, + recentPrivateConversations, + unread, }); - const expectedResult = [ + expect(getRecentConversations(state)).toEqual([ { - ids: '0', - recipients: 'me@example.com', + ids: eg.selfUser.user_id.toString(), + recipients: eg.selfUser.email, msgId: 6, unread: 1, }, { - ids: '0,1,2', - recipients: 'john@example.com,mark@example.com', + ids: [eg.selfUser.user_id, users[1].user_id, users[2].user_id].sort((a, b) => a - b).join(), + recipients: [users[1].email, users[2].email].join(), msgId: 5, unread: 1, }, { - ids: '1', - recipients: 'john@example.com', + ids: users[1].user_id.toString(), + recipients: users[1].email, msgId: 4, unread: 2, }, { - ids: '2', - recipients: 'mark@example.com', + ids: users[2].user_id.toString(), + recipients: users[2].email, msgId: 3, unread: 1, }, - ]; - - const actual = getRecentConversations(state); - - expect(actual).toEqual(expectedResult); + ]); }); }); diff --git a/src/pm-conversations/__tests__/recentPmConversationsReducer-test.js b/src/pm-conversations/__tests__/recentPmConversationsReducer-test.js new file mode 100644 index 00000000000..eb3aa4fcf15 --- /dev/null +++ b/src/pm-conversations/__tests__/recentPmConversationsReducer-test.js @@ -0,0 +1,35 @@ +/* @flow strict-local */ +import deepFreeze from 'deep-freeze'; + +import recentPmConversationsReducer from '../recentPmConversationsReducer'; +import { EVENT_NEW_MESSAGE } from '../../actionConstants'; +import * as eg from '../../__tests__/lib/exampleData'; + +describe('recentPmConversationsReducer', () => { + describe('EVENT_NEW_MESSAGE', () => { + test('reorder correctly upon recieving a new message', () => { + const users = [eg.makeUser({ name: 'john' }), eg.makeUser({ name: 'mark' })]; + const newMessage = eg.pmMessage({ + id: 2, + display_recipient: [eg.displayRecipientFromUser(users[1])], + }); + const initialState = deepFreeze([ + { max_message_id: 1, user_ids: [users[0].user_id] }, + { max_message_id: 0, user_ids: [users[1].user_id] }, + ]); + + const action = deepFreeze({ + type: EVENT_NEW_MESSAGE, + message: newMessage, + id: 0, + caughtUp: {}, + ownEmail: '', + }); + + expect(recentPmConversationsReducer(initialState, action)).toEqual([ + { max_message_id: 2, user_ids: [users[1].user_id] }, + { max_message_id: 1, user_ids: [users[0].user_id] }, + ]); + }); + }); +}); diff --git a/src/pm-conversations/pmConversationsSelectors.js b/src/pm-conversations/pmConversationsSelectors.js index f8efbd8ba21..bda01858e5b 100644 --- a/src/pm-conversations/pmConversationsSelectors.js +++ b/src/pm-conversations/pmConversationsSelectors.js @@ -1,58 +1,60 @@ /* @flow strict-local */ import { createSelector } from 'reselect'; -import type { Message, PmConversationData, Selector, User } from '../types'; -import { getPrivateMessages } from '../message/messageSelectors'; -import { getOwnUser } from '../users/userSelectors'; +import type { RecentPrivateConversation, PmConversationData, Selector, User } from '../types'; +import { getRecentPrivateConversations } from '../directSelectors'; +import { getOwnUser, getAllUsersById } from '../users/userSelectors'; import { getUnreadByPms, getUnreadByHuddles } from '../unread/unreadSelectors'; -import { normalizeRecipientsSansMe, pmUnreadsKeyFromMessage } from '../utils/recipient'; +import { normalizeRecipientsSansMe, sortIds } from '../utils/recipient'; export const getRecentConversations: Selector = createSelector( + getRecentPrivateConversations, + getAllUsersById, getOwnUser, - getPrivateMessages, getUnreadByPms, getUnreadByHuddles, ( + recentPrivateConversations: RecentPrivateConversation[], + allUsersById, ownUser: User, - messages: Message[], unreadPms: { [number]: number }, unreadHuddles: { [string]: number }, - ): PmConversationData[] => { - const recipients = messages.map(msg => ({ - ids: pmUnreadsKeyFromMessage(msg, ownUser.user_id), - emails: normalizeRecipientsSansMe(msg.display_recipient, ownUser.email), - msgId: msg.id, - })); + ): PmConversationData[] => + recentPrivateConversations.map(conversation => { + // TODO make recipient.js handle this nastiness + const conversationUserIdsIncludeMe = conversation.user_ids.slice(); + if (conversationUserIdsIncludeMe.length !== 1) { + conversationUserIdsIncludeMe.push(ownUser.user_id); + } - const latestByRecipient = new Map(); - recipients.forEach(recipient => { - const prev = latestByRecipient.get(recipient.emails); - if (!prev || recipient.msgId > prev.msgId) { - latestByRecipient.set(recipient.emails, { - ids: recipient.ids, - recipients: recipient.emails, - msgId: recipient.msgId, - }); + // TODO make recipient.js handle this nastiness too + const emails = []; + for (const userId of conversationUserIdsIncludeMe) { + const user = allUsersById.get(userId); + if (!user) { + throw new Error('getRecentConversations: unknown user id'); + } + emails.push(user.email); } - }); - const sortedByMostRecent = Array.from(latestByRecipient.values()).sort( - (a, b) => +b.msgId - +a.msgId, - ); + const userIdsString = sortIds(conversationUserIdsIncludeMe); - return sortedByMostRecent.map(recipient => ({ - ...recipient, - unread: - // This business of looking in one place and then the other is kind - // of messy. Fortunately it always works, because the key spaces - // are disjoint: all `unreadHuddles` keys contain a comma, and all - // `unreadPms` keys don't. - /* $FlowFixMe: The keys of unreadPms are logically numbers, but because it's an object they - end up converted to strings, so this access with string keys works. We should probably use - a Map for this and similar maps. */ - unreadPms[recipient.ids] || unreadHuddles[recipient.ids], - })); - }, + return { + ids: userIdsString, + recipients: normalizeRecipientsSansMe(emails.map(email => ({ email })), ownUser.email), + msgId: conversation.max_message_id, + unread: + // This business of looking in one place and then the other is kind + // of messy. Fortunately it always works, because the key spaces + // are disjoint: all `unreadHuddles` keys contain a comma, and all + // `unreadPms` keys don't. + /* $FlowFixMe: The keys of unreadPms are logically numbers, but + because it's an object they end up converted to strings, so + this access with string keys works. We should probably use a + Map for this and similar maps. */ + unreadPms[userIdsString] || unreadHuddles[userIdsString], + }; + }), ); export const getUnreadConversations: Selector = createSelector( diff --git a/src/pm-conversations/recentPmConversationsReducer.js b/src/pm-conversations/recentPmConversationsReducer.js new file mode 100644 index 00000000000..7b12f04a39e --- /dev/null +++ b/src/pm-conversations/recentPmConversationsReducer.js @@ -0,0 +1,43 @@ +/* @flow strict-local */ +import isEqual from 'lodash.isequal'; + +import type { Action, RecentPrivateConversationsState } from '../types'; +import { REALM_INIT, EVENT_NEW_MESSAGE } from '../actionConstants'; +import { NULL_ARRAY } from '../nullObjects'; + +const initialState: RecentPrivateConversationsState = NULL_ARRAY; + +const newMessage = (state, action) => { + if (action.message.type !== 'private') { + return state; + } + + const userIds = action.message.display_recipient.map(recipient => recipient.id); + const index = state.findIndex(item => isEqual(item.user_ids, userIds)); + const oldMaxMsgId = index < 0 ? null : state[index].max_message_id; + const item = { + user_ids: userIds, + max_message_id: Math.max(action.message.id, oldMaxMsgId ?? -Infinity), + }; + + const unsorted = + index < 0 + ? [item, ...state] /* force linebreak */ + : [item, ...state.slice(0, index), ...state.slice(index + 1)]; + + return unsorted.sort((a, b) => -(a.max_message_id - b.max_message_id)); +}; + +export default ( + state: RecentPrivateConversationsState = initialState, + action: Action, +): RecentPrivateConversationsState => { + switch (action.type) { + case REALM_INIT: + return action.data.recent_private_conversations || initialState; + case EVENT_NEW_MESSAGE: + return newMessage(state, action); + default: + return state; + } +}; diff --git a/src/reduxTypes.js b/src/reduxTypes.js index aba9d0e4f95..ac444ece2da 100644 --- a/src/reduxTypes.js +++ b/src/reduxTypes.js @@ -22,6 +22,7 @@ import type { RealmEmojiById, RealmFilter, Narrow, + RecentPrivateConversation, Stream, StreamUnreadItem, Subscription, @@ -251,6 +252,13 @@ export type RealmState = {| isAdmin: boolean, |}; +/** + * The PM conversations (group or 1:1) found in the last 1000 PMs. + * + * Sorted by `max_message_id` descending. + */ +export type RecentPrivateConversationsState = RecentPrivateConversation[]; + export type ThemeName = 'default' | 'night'; export type SettingsState = {| @@ -338,6 +346,7 @@ export type GlobalState = {| outbox: OutboxState, presence: PresenceState, realm: RealmState, + recentPrivateConversations: RecentPrivateConversationsState, session: SessionState, settings: SettingsState, streams: StreamsState, diff --git a/src/utils/recipient.js b/src/utils/recipient.js index b6f8ee456c2..8ac0fa93105 100644 --- a/src/utils/recipient.js +++ b/src/utils/recipient.js @@ -97,6 +97,12 @@ export const pmKeyRecipientsFromMessage = ( return filterRecipients(message.display_recipient, ownUser.user_id); }; +export const sortIds = (ids: number[]): string => + ids + .slice() + .sort((a, b) => a - b) + .join(','); + /** * The key this PM is filed under in the "unread messages" data structure. *