Skip to content

Commit b419e14

Browse files
committed
autocomplete: Sort user-mention autocomplete results
Fixes: #228
1 parent 8605c7e commit b419e14

File tree

4 files changed

+294
-3
lines changed

4 files changed

+294
-3
lines changed

lib/model/autocomplete.dart

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

249+
List<User>? _sortedUsers;
250+
251+
int compareByDms(User userA, User userB) {
252+
final countA = store.recentDmConversationsView.getRecipientCount(userA.userId);
253+
final countB = store.recentDmConversationsView.getRecipientCount(userB.userId);
254+
255+
if (countA > countB) {
256+
return -1;
257+
} else if (countB > countA) {
258+
return 1;
259+
}
260+
261+
if (!userA.isBot && userB.isBot) {
262+
return -1;
263+
} else if (userA.isBot && !userB.isBot) {
264+
return 1;
265+
}
266+
267+
if (userA.fullName.toLowerCase().compareTo(
268+
userB.fullName.toLowerCase()) < 0) {
269+
return -1;
270+
}
271+
return 1;
272+
}
273+
274+
int compareByRelevance({
275+
required User userA,
276+
required User userB,
277+
required int Function(User, User) tertiaryCompare,
278+
}) {
279+
// TODO: give preference to "all", "everyone" or "stream".
280+
281+
// TODO: give preference to subscribed users first
282+
283+
// give preference to direct message partners if both (are)/(are not) subscribers
284+
final aIsPartner = store.recentDmConversationsView.isPartner(userA.userId);
285+
final bIsPartner = store.recentDmConversationsView.isPartner(userB.userId);
286+
287+
if (aIsPartner && !bIsPartner) {
288+
return -1;
289+
} else if (bIsPartner && !aIsPartner) {
290+
return 1;
291+
}
292+
293+
return tertiaryCompare(userA, userB);
294+
}
295+
296+
List<User> sortByRelevance({
297+
required List<User> users,
298+
required int? currentStreamId,
299+
required String? currentTopic,
300+
}) {
301+
// Sorting for DMs narrow
302+
if (currentStreamId == null) {
303+
users.sort((userA, userB) => compareByRelevance(
304+
userA: userA,
305+
userB: userB,
306+
tertiaryCompare: compareByDms));
307+
} else { // Sorting for stream/topic narrow
308+
users.sort((userA, userB) => compareByRelevance(
309+
userA: userA,
310+
userB: userB,
311+
tertiaryCompare: (userA, userB) => store.recentSenders.compareByRecency(
312+
userA: userA,
313+
userB: userB,
314+
streamId: currentStreamId,
315+
topic: currentTopic)));
316+
}
317+
return users;
318+
}
319+
320+
void _sortUsers() {
321+
final users = store.users.values.toList();
322+
switch(narrow) {
323+
case StreamNarrow():
324+
_sortedUsers = sortByRelevance(
325+
users: users,
326+
currentStreamId: (narrow as StreamNarrow).streamId,
327+
currentTopic: null);
328+
case TopicNarrow():
329+
_sortedUsers = sortByRelevance(
330+
users: users,
331+
currentStreamId: (narrow as TopicNarrow).streamId,
332+
currentTopic: (narrow as TopicNarrow).topic);
333+
case DmNarrow():
334+
_sortedUsers = sortByRelevance(
335+
users: users,
336+
currentStreamId: null,
337+
currentTopic: null);
338+
case AllMessagesNarrow():
339+
_sortedUsers = []; // TODO: sort (maybe in future)
340+
}
341+
}
342+
247343
Future<List<MentionAutocompleteResult>?> _computeResults(MentionAutocompleteQuery query) async {
248344
final List<MentionAutocompleteResult> results = [];
249-
final Iterable<User> users = store.users.values;
250345

251-
final iterator = users.iterator;
346+
if (_sortedUsers == null) {
347+
_sortUsers();
348+
}
349+
350+
final iterator = _sortedUsers!.iterator;
252351
bool isDone = false;
253352
while (!isDone) {
254353
// CPU perf: End this task; enqueue a new one for resuming this work
@@ -270,7 +369,7 @@ class MentionAutocompleteView extends ChangeNotifier {
270369
}
271370
}
272371
}
273-
return results; // TODO(#228) sort for most relevant first
372+
return results;
274373
}
275374
}
276375

lib/model/message_list.dart

+3
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
317317
@override
318318
void dispose() {
319319
store.unregisterMessageList(this);
320+
store.recentDmConversationsView.clearRecipientCount(narrow);
320321
super.dispose();
321322
}
322323

@@ -382,6 +383,8 @@ class MessageListView with ChangeNotifier, _MessageSequence {
382383
if (_messageVisible(message)) {
383384
_addMessage(message);
384385
}
386+
store.recentSenders.processStreamMessage(message);
387+
store.recentDmConversationsView.maybeIncrRecipientCount(message);
385388
}
386389
_fetched = true;
387390
_haveOldest = result.foundOldest;

lib/model/recent_dm_conversations.dart

+41
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ class RecentDmConversationsView extends ChangeNotifier {
4040

4141
final int selfUserId;
4242

43+
// Number of messages sent per user
44+
final Map<int, int> _recipientCountMap = {};
45+
4346
/// Insert the key at the proper place in [sorted].
4447
///
4548
/// Optimized, taking O(1) time, for the case where that place is the start,
@@ -123,4 +126,42 @@ class RecentDmConversationsView extends ChangeNotifier {
123126
// TODO update from messages loaded in message lists. When doing so,
124127
// review handleMessageEvent so it acknowledges the subtle races that can
125128
// happen when taking data from outside the event system.
129+
130+
bool isPartner(int userId) {
131+
return sorted.any((dmNarrow) => dmNarrow.otherRecipientIds.contains(userId));
132+
}
133+
134+
int getRecipientCount(int userId) => _recipientCountMap[userId] ?? 0;
135+
136+
void incrRecipientCount(int userId) {
137+
final oldCount = _recipientCountMap[userId] ?? 0;
138+
_recipientCountMap[userId] = oldCount + 1;
139+
}
140+
141+
void clearRecipientCount(Narrow narrow) {
142+
if (narrow is! DmNarrow) {
143+
return;
144+
}
145+
146+
for (final userId in narrow.otherRecipientIds) {
147+
_recipientCountMap[userId] = 0;
148+
}
149+
}
150+
151+
void maybeIncrRecipientCount(Message message) {
152+
if (message is! DmMessage) {
153+
return;
154+
}
155+
156+
if (message.senderId != selfUserId) {
157+
return;
158+
}
159+
160+
for (final recipient in message.displayRecipient) {
161+
if (recipient.id == selfUserId) {
162+
continue;
163+
}
164+
incrRecipientCount(recipient.id);
165+
}
166+
}
126167
}

lib/model/store.dart

+148
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'dart:async';
22
import 'dart:convert';
33
import 'dart:io';
4+
import 'dart:math' as math;
45

56
import 'package:drift/native.dart';
67
import 'package:flutter/foundation.dart';
@@ -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,7 +440,9 @@ 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);
445+
recentDmConversationsView.maybeIncrRecipientCount(event.message);
440446
for (final view in _messageListViews) {
441447
view.maybeAddMessage(event.message);
442448
}
@@ -767,3 +773,145 @@ class UpdateMachine {
767773
@override
768774
String toString() => '${objectRuntimeType(this, 'UpdateMachine')}#${shortHash(this)}';
769775
}
776+
777+
extension Max<T extends num> on Iterable<T> {
778+
/// Finds the maximum number in an [Iterable].
779+
///
780+
/// Returns null if the [Iterable] is empty.
781+
T? max() => isNotEmpty ? reduce(math.max) : null;
782+
}
783+
784+
/// A data structure to keep track of message ids.
785+
class IdTracker {
786+
final Set<int> _ids = {};
787+
788+
int _maxId = -1;
789+
790+
void add(int id) {
791+
_ids.add(id);
792+
793+
if (id > _maxId) {
794+
_maxId = id;
795+
}
796+
}
797+
798+
// TODO: remove
799+
800+
int maxId() {
801+
if (_maxId == -1) {
802+
_maxId = _ids.max() ?? -1;
803+
}
804+
return _maxId;
805+
}
806+
807+
@override
808+
String toString() {
809+
return _ids.toString();
810+
}
811+
}
812+
813+
/// A data structure to keep track of stream and topic messages.
814+
class RecentSenders {
815+
// streamSenders[streamId][senderId] = IdTracker
816+
final Map<int, Map<int, IdTracker>> _streamSenders = {};
817+
818+
// topicSenders[streamId][topic][senderId] = IdTracker
819+
final Map<int, Map<String, Map<int, IdTracker>>> _topicSenders = {};
820+
821+
// TODO: dmSenders (maybe)
822+
823+
void clear() {
824+
_streamSenders.clear();
825+
_topicSenders.clear();
826+
}
827+
828+
int maxIdForStreamSender({required int streamId, required int senderId}) {
829+
return _streamSenders[streamId]?[senderId]?.maxId() ?? -1;
830+
}
831+
832+
int maxIdForStreamTopicSender({
833+
required int streamId,
834+
required String topic,
835+
required int senderId,
836+
}) {
837+
topic = topic.toLowerCase();
838+
return _topicSenders[streamId]?[topic]?[senderId]?.maxId() ?? -1;
839+
}
840+
841+
void addStreamMessage({
842+
required int streamId,
843+
required int senderId,
844+
required messageId,
845+
}) {
846+
final senderMap = _streamSenders[streamId] ?? {};
847+
final idTracker = senderMap[senderId] ?? IdTracker();
848+
_streamSenders[streamId] = senderMap;
849+
senderMap[senderId] = idTracker;
850+
idTracker.add(messageId);
851+
}
852+
853+
void addTopicMessage({
854+
required int streamId,
855+
required String topic,
856+
required int senderId,
857+
required messageId,
858+
}) {
859+
topic = topic.toLowerCase();
860+
final topicMap = _topicSenders[streamId] ?? {};
861+
final senderMap = topicMap[topic] ?? {};
862+
final idTracker = senderMap[senderId] ?? IdTracker();
863+
_topicSenders[streamId] = topicMap;
864+
topicMap[topic] = senderMap;
865+
senderMap[senderId] = idTracker;
866+
idTracker.add(messageId);
867+
}
868+
869+
void processStreamMessage(Message message) {
870+
if (message is! StreamMessage) {
871+
return;
872+
}
873+
874+
final streamId = message.streamId;
875+
final topic = message.subject;
876+
final senderId = message.senderId;
877+
final messageId = message.id;
878+
879+
addStreamMessage(
880+
streamId: streamId, senderId: senderId, messageId: messageId);
881+
addTopicMessage(
882+
streamId: streamId,
883+
topic: topic,
884+
senderId: senderId,
885+
messageId: messageId);
886+
}
887+
888+
// TODO: removeTopicMessage
889+
890+
int compareByRecency({
891+
required User userA,
892+
required User userB,
893+
required int streamId,
894+
// TODO: may turn this into a non-nullable string after (#493)
895+
required String? topic,
896+
}) {
897+
int aMessageId, bMessageId;
898+
899+
if (topic != null) {
900+
aMessageId = maxIdForStreamTopicSender(
901+
streamId: streamId, topic: topic, senderId: userA.userId);
902+
bMessageId = maxIdForStreamTopicSender(
903+
streamId: streamId, topic: topic, senderId: userB.userId);
904+
905+
if (aMessageId != bMessageId) {
906+
return bMessageId - aMessageId;
907+
}
908+
}
909+
910+
aMessageId =
911+
maxIdForStreamSender(streamId: streamId, senderId: userA.userId);
912+
bMessageId =
913+
maxIdForStreamSender(streamId: streamId, senderId: userB.userId);
914+
915+
return bMessageId - aMessageId;
916+
}
917+
}

0 commit comments

Comments
 (0)