Skip to content

Commit a53e20a

Browse files
committed
autocomplete: Sort user-mention autocomplete results
The @-mention autocomplete shows relevant results in the following order of priority: * Recent conversation users * Recent DM users * Alphabetical order Fixes: zulip#228
1 parent 74aa25d commit a53e20a

9 files changed

+349
-12
lines changed

lib/model/algorithms.dart

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,47 @@
11

22
import 'package:collection/collection.dart';
33

4+
/// The index of [value] in [sortedList].
5+
///
6+
/// Uses binary search to find the index, so the list must be sorted,
7+
/// otherwise the result is unpredictable.
8+
///
9+
/// If [value] is found, returns its index (0-based).
10+
///
11+
/// ```dart
12+
/// final somePrimes = [11, 13, 17, 23, 27];
13+
/// final index = possibleIndexIn(somePrimes, 17); // 2
14+
/// ```
15+
/// If [value] is not found, returns the possible index (1-based)
16+
/// with a negative sign, where if the [value] is inserted,
17+
/// the order of the list is maintained.
18+
///
19+
/// ```dart
20+
/// final somePrimes = [11, 13, 17, 23, 27];
21+
/// int index = possibleIndexIn(somePrimes, 7); // -1
22+
/// index = possibleIndexIn(somePrimes, 19); // -4
23+
/// ```
24+
///
25+
/// Note: Using a negative 1-based index will differentiate between the case where
26+
/// the element is found at index 0 and the case where the element is not
27+
/// found, but should be placed at the very start of the list (index -1).
28+
int possibleIndexIn<E extends Comparable>(List<E> sortedList, E value) {
29+
int min = 0;
30+
int max = sortedList.length - 1;
31+
while (min <= max) {
32+
final mid = min + ((max - min) >> 1);
33+
final midElement = sortedList[mid];
34+
final comp = midElement.compareTo(value);
35+
if (comp == 0) return mid;
36+
if (comp < 0) {
37+
min = mid + 1;
38+
} else {
39+
max = mid - 1;
40+
}
41+
}
42+
return -(min + 1);
43+
}
44+
445
/// Returns the index in [sortedList] of an element matching the given [key],
546
/// if there is one.
647
///

lib/model/autocomplete.dart

Lines changed: 108 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ class MentionAutocompleteView extends ChangeNotifier {
183183
@override
184184
void dispose() {
185185
store.autocompleteViewManager.unregisterMentionAutocomplete(this);
186+
_sortedUsers = null;
186187
// We cancel in-progress computations by checking [hasListeners] between tasks.
187188
// After [super.dispose] is called, [hasListeners] returns false.
188189
// TODO test that logic (may involve detecting an unhandled Future rejection; how?)
@@ -206,6 +207,7 @@ class MentionAutocompleteView extends ChangeNotifier {
206207
/// Called in particular when we get a [RealmUserEvent].
207208
void refreshStaleUserResults() {
208209
if (_query != null) {
210+
_sortedUsers = null;
209211
_startSearch(_query!);
210212
}
211213
}
@@ -244,11 +246,95 @@ class MentionAutocompleteView extends ChangeNotifier {
244246
notifyListeners();
245247
}
246248

249+
List<User>? _sortedUsers;
250+
251+
int compareByRelevance({
252+
required User userA,
253+
required User userB,
254+
required int? streamId,
255+
required String? topic,
256+
}) {
257+
// TODO(#234): give preference to "all", "everyone" or "stream".
258+
259+
// TODO(#618): give preference to subscribed users first.
260+
261+
if (streamId != null) {
262+
final conversationPrecedence = store.recentSenders.compareByRecency(
263+
userA: userA,
264+
userB: userB,
265+
streamId: streamId,
266+
topic: topic);
267+
if (conversationPrecedence != 0) {
268+
return conversationPrecedence;
269+
}
270+
}
271+
272+
final dmPrecedence = store.recentDmConversationsView.compareByDms(userA, userB);
273+
if (dmPrecedence != 0) {
274+
return dmPrecedence;
275+
}
276+
277+
if (!userA.isBot && userB.isBot) {
278+
return -1;
279+
} else if (userA.isBot && !userB.isBot) {
280+
return 1;
281+
}
282+
283+
final userAName = store.autocompleteViewManager.autocompleteDataCache
284+
.nameForUser(userA);
285+
final userBName = store.autocompleteViewManager.autocompleteDataCache
286+
.nameForUser(userB);
287+
return userAName.compareTo(userBName);
288+
}
289+
290+
List<User> sortByRelevance({
291+
required List<User> users,
292+
required Narrow narrow,
293+
}) {
294+
switch (narrow) {
295+
case StreamNarrow():
296+
users.sort((userA, userB) => compareByRelevance(
297+
userA: userA,
298+
userB: userB,
299+
streamId: narrow.streamId,
300+
topic: null));
301+
case TopicNarrow():
302+
users.sort((userA, userB) => compareByRelevance(
303+
userA: userA,
304+
userB: userB,
305+
streamId: narrow.streamId,
306+
topic: narrow.topic));
307+
case DmNarrow():
308+
users.sort((userA, userB) => compareByRelevance(
309+
userA: userA,
310+
userB: userB,
311+
streamId: null,
312+
topic: null));
313+
case AllMessagesNarrow():
314+
// do nothing in this case for now
315+
}
316+
return users;
317+
}
318+
319+
void _sortUsers() {
320+
final users = store.users.values.toList();
321+
_sortedUsers = sortByRelevance(users: users, narrow: narrow);
322+
}
323+
324+
bool _areUsersModified = false;
325+
void _usersModified() {
326+
_areUsersModified = true;
327+
}
328+
247329
Future<List<MentionAutocompleteResult>?> _computeResults(MentionAutocompleteQuery query) async {
248330
final List<MentionAutocompleteResult> results = [];
249-
final Iterable<User> users = store.users.values;
250331

251-
final iterator = users.iterator;
332+
if (_sortedUsers == null) {
333+
_sortUsers();
334+
}
335+
336+
final iterator = _sortedUsers!.iterator;
337+
store.users.addListener(_usersModified);
252338
bool isDone = false;
253339
while (!isDone) {
254340
// CPU perf: End this task; enqueue a new one for resuming this work
@@ -259,6 +345,11 @@ class MentionAutocompleteView extends ChangeNotifier {
259345
}
260346

261347
for (int i = 0; i < 1000; i++) {
348+
if (_areUsersModified) {
349+
_areUsersModified = false;
350+
_sortedUsers = null;
351+
throw ConcurrentModificationError();
352+
}
262353
if (!iterator.moveNext()) { // Can throw ConcurrentModificationError
263354
isDone = true;
264355
break;
@@ -270,7 +361,8 @@ class MentionAutocompleteView extends ChangeNotifier {
270361
}
271362
}
272363
}
273-
return results; // TODO(#228) sort for most relevant first
364+
store.users.removeListener(_usersModified);
365+
return results;
274366
}
275367
}
276368

@@ -330,13 +422,25 @@ class MentionAutocompleteQuery {
330422
}
331423

332424
class AutocompleteDataCache {
425+
final Map<int, String> _namesByUser = {};
426+
427+
/// The lowercase `fullName` of [user].
428+
String nameForUser(User user) {
429+
return _namesByUser[user.userId] ??= user.fullName.toLowerCase();
430+
}
431+
333432
final Map<int, List<String>> _nameWordsByUser = {};
334433

335434
List<String> nameWordsForUser(User user) {
336-
return _nameWordsByUser[user.userId] ??= user.fullName.toLowerCase().split(' ');
435+
final nameWords = _nameWordsByUser[user.userId];
436+
if (nameWords != null) return nameWords;
437+
438+
final name = _namesByUser[user.userId] ??= user.fullName.toLowerCase();
439+
return _nameWordsByUser[user.userId] = name.split(' ');
337440
}
338441

339442
void invalidateUser(int userId) {
443+
_namesByUser.remove(userId);
340444
_nameWordsByUser.remove(userId);
341445
}
342446
}

lib/model/message_list.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
381381
for (final message in result.messages) {
382382
if (_messageVisible(message)) {
383383
_addMessage(message);
384+
store.recentSenders.processAsStreamMessage(message);
384385
}
385386
}
386387
_fetched = true;

lib/model/recent_dm_conversations.dart

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ class RecentDmConversationsView extends ChangeNotifier {
3030
required this.map,
3131
required this.sorted,
3232
required this.selfUserId,
33-
});
33+
}) {
34+
_computeLatestMessagesWithUsers();
35+
}
3436

3537
/// The latest message ID in each conversation.
3638
final Map<DmNarrow, int> map;
@@ -40,6 +42,32 @@ class RecentDmConversationsView extends ChangeNotifier {
4042

4143
final int selfUserId;
4244

45+
/// Map from user ID to the latest message ID in any conversation with the user.
46+
///
47+
/// Both 1:1 and group DM conversations are considered.
48+
/// The self-user ID is excluded even if there is a self-DM conversation.
49+
///
50+
/// (The identified message was not necessarily sent by the identified user;
51+
/// it might have been sent by anyone in its conversation.)
52+
final Map<int, int> _latestMessageByRecipient = {};
53+
54+
void _computeLatestMessagesWithUsers() {
55+
for (final dmNarrow in sorted) {
56+
for (final userId in dmNarrow.otherRecipientIds) {
57+
if (!_latestMessageByRecipient.containsKey(userId)) {
58+
_latestMessageByRecipient[userId] = map[dmNarrow]!;
59+
}
60+
}
61+
}
62+
}
63+
64+
int compareByDms(User userA, User userB) {
65+
final aLatestMessageId = _latestMessageByRecipient[userA.userId] ?? -1;
66+
final bLatestMessageId = _latestMessageByRecipient[userB.userId] ?? -1;
67+
68+
return bLatestMessageId - aLatestMessageId;
69+
}
70+
4371
/// Insert the key at the proper place in [sorted].
4472
///
4573
/// Optimized, taking O(1) time, for the case where that place is the start,

0 commit comments

Comments
 (0)