Skip to content

Commit df31fc7

Browse files
rk-for-zulipgnprice
authored andcommitted
wip recent convs: Add new implementation of getRecentConversations.
TODO(greg): finish rebase Add new implementation of `getRecentConversations`, based on new `recent_private_conversations` data. We can't get this data on pre-2.1 servers, so we continue using the legacy data there, to avoid a regression for users on such servers. (Without this, they would never see any old conversations in the PMs window; only those with messages that have come in during the current session would be visible.) In upcoming commits: * New tests for the new implementation (alongside the retained tests for the legacy implementation). * Adjusting the main selector to be more efficient, rather than potentially computing both the modern and legacy implementations on every call. Based in part on work by Isham Mahajan <[email protected]>.
1 parent b9f71a2 commit df31fc7

File tree

2 files changed

+97
-4
lines changed

2 files changed

+97
-4
lines changed

src/pm-conversations/__tests__/pmConversationsSelectors-test.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@ import Immutable from 'immutable';
33

44
import { getRecentConversations } from '../pmConversationsSelectors';
55
import { ALL_PRIVATE_NARROW_STR } from '../../utils/narrow';
6+
import { ZulipVersion } from '../../utils/zulipVersion';
67
import * as eg from '../../__tests__/lib/exampleData';
78

8-
describe('getRecentConversations', () => {
9+
describe('getRecentConversations: legacy', () => {
10+
const zulipVersion = new ZulipVersion('2.0.0');
11+
912
const userJohn = eg.makeUser();
1013
const userMark = eg.makeUser();
1114
const keyForUsers = users =>
@@ -17,6 +20,7 @@ describe('getRecentConversations', () => {
1720

1821
test('when no messages, return no conversations', () => {
1922
const state = eg.reduxState({
23+
accounts: [eg.makeAccount({ user: eg.selfUser, zulipVersion })],
2024
realm: eg.realmState({ email: eg.selfUser.email }),
2125
users: [eg.selfUser],
2226
narrows: Immutable.Map({
@@ -36,6 +40,7 @@ describe('getRecentConversations', () => {
3640

3741
test('returns unique list of recipients, includes conversations with self', () => {
3842
const state = eg.reduxState({
43+
accounts: [eg.makeAccount({ user: eg.selfUser, zulipVersion })],
3944
realm: eg.realmState({ email: eg.selfUser.email }),
4045
users: [eg.selfUser, userJohn, userMark],
4146
narrows: Immutable.Map({
@@ -83,6 +88,7 @@ describe('getRecentConversations', () => {
8388

8489
test('returns recipients sorted by last activity', () => {
8590
const state = eg.reduxState({
91+
accounts: [eg.makeAccount({ user: eg.selfUser, zulipVersion })],
8692
realm: eg.realmState({ email: eg.selfUser.email }),
8793
users: [eg.selfUser, userJohn, userMark],
8894
narrows: Immutable.Map({

src/pm-conversations/pmConversationsSelectors.js

Lines changed: 90 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,26 @@
11
/* @flow strict-local */
22
import { createSelector } from 'reselect';
33

4-
import type { Message, PmConversationData, Selector, User } from '../types';
4+
import type {
5+
Message,
6+
PmConversationData,
7+
RecentPrivateConversation,
8+
Selector,
9+
User,
10+
UserOrBot,
11+
} from '../types';
12+
import { getServerVersion } from '../account/accountsSelectors';
513
import { getPrivateMessages } from '../message/messageSelectors';
614
import { getAllUsersById, getOwnUser } from '../users/userSelectors';
15+
import { getRecentPrivateConversations } from '../directSelectors';
716
import { getUnreadByPms, getUnreadByHuddles } from '../unread/unreadSelectors';
8-
import { pmUnreadsKeyFromMessage, pmKeyRecipientUsersFromMessage } from '../utils/recipient';
17+
import {
18+
pmUnreadsKeyFromMessage,
19+
pmKeyRecipientUsersFromMessage,
20+
pmKeyRecipientsFromIds,
21+
pmUnreadsKeyFromPmKeyIds,
22+
} from '../utils/recipient';
23+
import { ZulipVersion } from '../utils/zulipVersion';
924

1025
/**
1126
* Given a list of PmConversationPartial or PmConversationData, trim it down to
@@ -61,7 +76,12 @@ const getAttachUnread = createSelector(
6176
})),
6277
);
6378

64-
export const getRecentConversations: Selector<PmConversationData[]> = createSelector(
79+
/**
80+
* Legacy implementation of {@link getRecentConversations}. Computes an
81+
* approximation to the set of recent conversations, based on the messages we
82+
* already know about.
83+
*/
84+
const getRecentConversationsLegacyImpl: Selector<PmConversationData[]> = createSelector(
6585
getOwnUser,
6686
getPrivateMessages,
6787
getAttachUnread,
@@ -82,6 +102,73 @@ export const getRecentConversations: Selector<PmConversationData[]> = createSele
82102
},
83103
);
84104

105+
/**
106+
* Modern implementation of {@link getRecentConversations}. Returns exactly the
107+
* most recent conversations. Requires server-side support.
108+
*/
109+
const getRecentConversationsImpl: Selector<PmConversationData[]> = createSelector(
110+
getOwnUser,
111+
getAllUsersById,
112+
getRecentPrivateConversations,
113+
getAttachUnread,
114+
(
115+
ownUser: User,
116+
allUsersById: Map<number, UserOrBot>,
117+
recentPCs: RecentPrivateConversation[],
118+
attachUnread,
119+
) => {
120+
const recipients = recentPCs.map(conversation => {
121+
const keyRecipients = pmKeyRecipientsFromIds(
122+
conversation.user_ids,
123+
allUsersById,
124+
ownUser.user_id,
125+
);
126+
if (!keyRecipients) {
127+
throw new Error('getRecentConversations: unknown user id');
128+
}
129+
130+
return {
131+
unreadsKey: pmUnreadsKeyFromPmKeyIds(keyRecipients.map(r => r.user_id), ownUser.user_id),
132+
keyRecipients,
133+
msgId: conversation.max_message_id,
134+
};
135+
});
136+
137+
return attachUnread(recipients);
138+
},
139+
);
140+
141+
/**
142+
* The server version in which 'recent_private_conversations' was first made
143+
* available.
144+
*/
145+
const DIVIDING_LINE = new ZulipVersion('2.1-dev-384-g4c3c669b41');
146+
147+
/**
148+
* Get a list of the most recent private conversations, including the most
149+
* recent message from each.
150+
*/
151+
// TODO: don't compute `legacy` when `version` indicates it's unneeded
152+
export const getRecentConversations: Selector<PmConversationData[]> = createSelector(
153+
getRecentConversationsImpl,
154+
getRecentConversationsLegacyImpl,
155+
getServerVersion,
156+
(modern, legacy, version) => {
157+
// If we're talking to a new enough version of the Zulip server, we don't
158+
// need the legacy impl; the modern one will always return a superset of
159+
// its content.
160+
if (version && version.isAtLeast(DIVIDING_LINE)) {
161+
return modern;
162+
}
163+
164+
// If we're _not_ talking to a newer version of the Zulip server, then
165+
// there's no point in using the modern version; it will only return
166+
// messages received in the current session, which should all be in the
167+
// legacy impl's data as well.
168+
return legacy;
169+
},
170+
);
171+
85172
export const getUnreadConversations: Selector<PmConversationData[]> = createSelector(
86173
getRecentConversations,
87174
conversations => conversations.filter(c => c.unread > 0),

0 commit comments

Comments
 (0)