Skip to content

Commit 997dd5a

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 a7d97d5 commit 997dd5a

File tree

4 files changed

+150
-22
lines changed

4 files changed

+150
-22
lines changed

lib/model/autocomplete.dart

+110-3
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,116 @@ class MentionAutocompleteView extends ChangeNotifier {
244246
notifyListeners();
245247
}
246248

249+
List<User>? _sortedUsers;
250+
251+
int compareByDms(User userA, User userB) {
252+
final aRecencyIndex = store.recentDmConversationsView.getRecencyIndex(userA.userId);
253+
final bRecencyIndex = store.recentDmConversationsView.getRecencyIndex(userB.userId);
254+
255+
if (aRecencyIndex > bRecencyIndex) {
256+
return -1;
257+
} else if (bRecencyIndex > aRecencyIndex) {
258+
return 1;
259+
}
260+
261+
final aIsPartner = store.recentDmConversationsView.isPartner(userA.userId);
262+
final bIsPartner = store.recentDmConversationsView.isPartner(userB.userId);
263+
264+
// This code will never run except in the rare case that one has no
265+
// recent DM message history with a user, but does have some older
266+
// message history.
267+
if (aIsPartner && !bIsPartner) {
268+
return -1;
269+
} else if (bIsPartner && !aIsPartner) {
270+
return 1;
271+
}
272+
273+
if (!userA.isBot && userB.isBot) {
274+
return -1;
275+
} else if (userA.isBot && !userB.isBot) {
276+
return 1;
277+
}
278+
279+
if (userA.fullName.toLowerCase().compareTo(
280+
userB.fullName.toLowerCase()) < 0) {
281+
return -1;
282+
}
283+
return 1;
284+
}
285+
286+
int compareByRelevance({
287+
required User userA,
288+
required User userB,
289+
int Function(User, User)? compareByCurrentConversation,
290+
}) {
291+
// TODO(#234): give preference to "all", "everyone" or "stream".
292+
293+
// TODO(#618): give preference to subscribed users first.
294+
295+
if (compareByCurrentConversation != null) {
296+
final preference = compareByCurrentConversation(userA, userB);
297+
if (preference != 0) {
298+
return preference;
299+
}
300+
}
301+
302+
return compareByDms(userA, userB);
303+
}
304+
305+
List<User> sortByRelevance({
306+
required List<User> users,
307+
required int? currentStreamId,
308+
required String? currentTopic,
309+
}) {
310+
// Sorting for DMs narrow
311+
if (currentStreamId == null) {
312+
users.sort((userA, userB) => compareByRelevance(
313+
userA: userA,
314+
userB: userB));
315+
} else { // Sorting for stream/topic narrow
316+
users.sort((userA, userB) => compareByRelevance(
317+
userA: userA,
318+
userB: userB,
319+
compareByCurrentConversation: (userA, userB) => store.recentSenders.compareByRecency(
320+
userA: userA,
321+
userB: userB,
322+
streamId: currentStreamId,
323+
topic: currentTopic)));
324+
}
325+
return users;
326+
}
327+
328+
void _sortUsers() {
329+
final users = store.users.values.toList();
330+
switch(narrow) {
331+
case StreamNarrow():
332+
_sortedUsers = sortByRelevance(
333+
users: users,
334+
currentStreamId: (narrow as StreamNarrow).streamId,
335+
currentTopic: null);
336+
case TopicNarrow():
337+
_sortedUsers = sortByRelevance(
338+
users: users,
339+
currentStreamId: (narrow as TopicNarrow).streamId,
340+
currentTopic: (narrow as TopicNarrow).topic);
341+
case DmNarrow():
342+
_sortedUsers = sortByRelevance(
343+
users: users,
344+
currentStreamId: null,
345+
currentTopic: null);
346+
case AllMessagesNarrow():
347+
_sortedUsers = []; // TODO: sort (maybe in future)
348+
}
349+
}
350+
247351
Future<List<MentionAutocompleteResult>?> _computeResults(MentionAutocompleteQuery query) async {
248352
final List<MentionAutocompleteResult> results = [];
249-
final Iterable<User> users = store.users.values;
250353

251-
final iterator = users.iterator;
354+
if (_sortedUsers == null) {
355+
_sortUsers();
356+
}
357+
358+
final iterator = _sortedUsers!.iterator;
252359
bool isDone = false;
253360
while (!isDone) {
254361
// CPU perf: End this task; enqueue a new one for resuming this work
@@ -270,7 +377,7 @@ class MentionAutocompleteView extends ChangeNotifier {
270377
}
271378
}
272379
}
273-
return results; // TODO(#228) sort for most relevant first
380+
return results;
274381
}
275382
}
276383

lib/model/message_list.dart

+1
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.processStreamMessage(message);
384385
}
385386
}
386387
_fetched = true;

lib/model/recent_dm_conversations.dart

+34-19
Original file line numberDiff line numberDiff line change
@@ -11,35 +11,50 @@ import 'narrow.dart';
1111
/// This maintains the list of recent DM conversations,
1212
/// plus additional data in order to efficiently maintain the list.
1313
class RecentDmConversationsView extends ChangeNotifier {
14-
factory RecentDmConversationsView({
14+
RecentDmConversationsView({
1515
required List<RecentDmConversation> initial,
16-
required selfUserId,
16+
required this.selfUserId,
1717
}) {
18-
final entries = initial.map((conversation) => MapEntry(
19-
DmNarrow.ofRecentDmConversation(conversation, selfUserId: selfUserId),
20-
conversation.maxMessageId,
21-
)).toList()..sort((a, b) => -a.value.compareTo(b.value));
22-
return RecentDmConversationsView._(
23-
map: Map.fromEntries(entries),
24-
sorted: QueueList.from(entries.map((e) => e.key)),
25-
selfUserId: selfUserId,
26-
);
27-
}
18+
final entries = <MapEntry<DmNarrow, int>>[];
19+
for (final conversation in initial) {
20+
entries.add(MapEntry(
21+
DmNarrow.ofRecentDmConversation(conversation, selfUserId: selfUserId),
22+
conversation.maxMessageId));
2823

29-
RecentDmConversationsView._({
30-
required this.map,
31-
required this.sorted,
32-
required this.selfUserId,
33-
});
24+
for (final userId in conversation.userIds) {
25+
if (userId == selfUserId) {
26+
continue;
27+
}
28+
_addRecencyIndex(userId: userId, maxId: conversation.maxMessageId);
29+
}
30+
}
31+
entries.sort((a, b) => -a.value.compareTo(b.value));
32+
map = Map.fromEntries(entries);
33+
sorted = QueueList.from(entries.map((e) => e.key));
34+
}
3435

3536
/// The latest message ID in each conversation.
36-
final Map<DmNarrow, int> map;
37+
late final Map<DmNarrow, int> map;
3738

3839
/// The [DmNarrow] keys of the map, sorted by latest message descending.
39-
final QueueList<DmNarrow> sorted;
40+
late final QueueList<DmNarrow> sorted;
4041

4142
final int selfUserId;
4243

44+
// The latest message ID between self-user and other users.
45+
final Map<int, int> _dmRecencyData = {};
46+
47+
int getRecencyIndex(final int userId) => _dmRecencyData[userId] ?? -1;
48+
49+
void _addRecencyIndex({required int userId, required int maxId}) {
50+
final oldMaxId = _dmRecencyData[userId] ?? -1;
51+
if (maxId > oldMaxId) {
52+
_dmRecencyData[userId] = maxId;
53+
}
54+
}
55+
56+
bool isPartner(final int userId) => getRecencyIndex(userId) != -1;
57+
4358
/// Insert the key at the proper place in [sorted].
4459
///
4560
/// Optimized, taking O(1) time, for the case where that place is the start,

lib/model/store.dart

+5
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import 'autocomplete.dart';
2121
import 'database.dart';
2222
import 'message_list.dart';
2323
import 'recent_dm_conversations.dart';
24+
import 'recent_senders.dart';
2425
import 'stream.dart';
2526
import 'unreads.dart';
2627

@@ -287,6 +288,8 @@ class PerAccountStore extends ChangeNotifier with StreamStore {
287288

288289
final Map<int, User> users;
289290

291+
final RecentSenders recentSenders = RecentSenders();
292+
290293
////////////////////////////////
291294
// Streams, topics, and stuff about them.
292295

@@ -347,6 +350,7 @@ class PerAccountStore extends ChangeNotifier with StreamStore {
347350
void dispose() {
348351
unreads.dispose();
349352
recentDmConversationsView.dispose();
353+
recentSenders.clear();
350354
for (final view in _messageListViews.toList()) {
351355
view.dispose();
352356
}
@@ -436,6 +440,7 @@ class PerAccountStore extends ChangeNotifier with StreamStore {
436440
notifyListeners();
437441
} else if (event is MessageEvent) {
438442
assert(debugLog("server event: message ${jsonEncode(event.message.toJson())}"));
443+
recentSenders.processStreamMessage(event.message);
439444
recentDmConversationsView.handleMessageEvent(event);
440445
for (final view in _messageListViews) {
441446
view.maybeAddMessage(event.message);

0 commit comments

Comments
 (0)