Skip to content

Commit 6dac3d8

Browse files
committed
autocomplete: Add "recent activity in current stream" criterion
In @-mention autocomplete, users are suggested based on: 1. Recent activity in the current stream. 2. Recent DM conversations. Fixes part of: #228
1 parent 600220a commit 6dac3d8

File tree

3 files changed

+186
-15
lines changed

3 files changed

+186
-15
lines changed

lib/model/autocomplete.dart

+62-1
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,42 @@ class MentionAutocompleteView extends ChangeNotifier {
267267

268268
List<User>? _sortedUsers;
269269

270+
/// Determines which of the two users has more recent activity.
271+
///
272+
/// First checks for the activity in [topic] if provided.
273+
///
274+
/// If no [topic] is provided, or the activity in the topic is the same (which
275+
/// is extremely rare) or there is no activity in the topic at all, then
276+
/// checks for the activity in the stream with [streamId].
277+
///
278+
/// Returns a negative number if [userA] has more recent activity than [userB],
279+
/// returns a positive number if [userB] has more recent activity than [userA],
280+
/// and returns `0` if both [userA] and [userB] have the same recent activity
281+
/// (which is extremely rare) or has no activity at all.
282+
int compareByRecency(
283+
User userA,
284+
User userB, {
285+
required int streamId,
286+
required String? topic,
287+
}) {
288+
final recentSenders = store.recentSenders;
289+
if (topic != null) {
290+
final aMessageId = recentSenders.latestMessageIdOfSenderInTopic(
291+
streamId: streamId, topic: topic, senderId: userA.userId);
292+
final bMessageId = recentSenders.latestMessageIdOfSenderInTopic(
293+
streamId: streamId, topic: topic, senderId: userB.userId);
294+
295+
final result = bMessageId.compareTo(aMessageId);
296+
if (result != 0) return result;
297+
}
298+
299+
final aMessageId =
300+
recentSenders.latestMessageIdOfSenderInStream(streamId: streamId, senderId: userA.userId);
301+
final bMessageId =
302+
recentSenders.latestMessageIdOfSenderInStream(streamId: streamId, senderId: userB.userId);
303+
return bMessageId.compareTo(aMessageId);
304+
}
305+
270306
/// Determines which of the two users are more recent in DM conversations.
271307
///
272308
/// Returns a negative number if [userA] is more recent than [userB],
@@ -284,7 +320,20 @@ class MentionAutocompleteView extends ChangeNotifier {
284320
int compareByRelevance({
285321
required User userA,
286322
required User userB,
323+
required int? streamId,
324+
required String? topic,
287325
}) {
326+
if (streamId != null) {
327+
final conversationPrecedence = compareByRecency(
328+
userA,
329+
userB,
330+
streamId: streamId,
331+
topic: topic);
332+
if (conversationPrecedence != 0) {
333+
return conversationPrecedence;
334+
}
335+
}
336+
288337
final dmPrecedence = compareByDms(userA, userB);
289338
return dmPrecedence;
290339
}
@@ -295,11 +344,23 @@ class MentionAutocompleteView extends ChangeNotifier {
295344
}) {
296345
switch (narrow) {
297346
case StreamNarrow():
347+
users.sort((userA, userB) => compareByRelevance(
348+
userA: userA,
349+
userB: userB,
350+
streamId: narrow.streamId,
351+
topic: null));
298352
case TopicNarrow():
353+
users.sort((userA, userB) => compareByRelevance(
354+
userA: userA,
355+
userB: userB,
356+
streamId: narrow.streamId,
357+
topic: narrow.topic));
299358
case DmNarrow():
300359
users.sort((userA, userB) => compareByRelevance(
301360
userA: userA,
302-
userB: userB));
361+
userB: userB,
362+
streamId: null,
363+
topic: null));
303364
case AllMessagesNarrow():
304365
// do nothing in this case for now
305366
}

lib/model/message_list.dart

+4
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.handleMessage(message);
384385
}
385386
}
386387
_fetched = true;
@@ -417,6 +418,9 @@ class MessageListView with ChangeNotifier, _MessageSequence {
417418
? result.messages // Avoid unnecessarily copying the list.
418419
: result.messages.where(_messageVisible);
419420

421+
for (final message in fetchedMessages) {
422+
store.recentSenders.handleMessage(message);
423+
}
420424
store.autocompleteViewManager.handleOlderMessages();
421425

422426
_insertAllMessages(0, fetchedMessages);

test/model/autocomplete_test.dart

+120-14
Original file line numberDiff line numberDiff line change
@@ -356,9 +356,10 @@ void main() {
356356
late PerAccountStore store;
357357
late MentionAutocompleteView view;
358358

359-
prepare({
359+
void prepare({
360360
required List<User> users,
361361
required List<RecentDmConversation> dmConversations,
362+
required List<Message> messages,
362363
required Narrow narrow,
363364
}) {
364365
store = eg.store(
@@ -368,16 +369,100 @@ void main() {
368369
for (final user in users) {
369370
store.addUser(user);
370371
}
372+
for (final message in messages) {
373+
store.addMessage(message);
374+
}
371375
view = MentionAutocompleteView.init(store: store, narrow: narrow);
372376
}
373377

374-
test('compareByDms give priority to user with DM exchanged more recently', () {
378+
group('compareByRecency gives priority to the user with latter message in topic/stream', () {
379+
final userA = eg.otherUser;
380+
final userB = eg.thirdUser;
381+
final stream = eg.stream();
382+
const topic1 = 'topic1';
383+
const topic2 = 'topic2';
384+
385+
void addMessage(User sender, String topic) {
386+
store.addMessage(eg.streamMessage(
387+
sender: sender,
388+
stream: stream,
389+
topic: topic,
390+
));
391+
}
392+
393+
/// Determines the priority between [userA] and [userB] based on their activity.
394+
///
395+
/// The activity is first looked for in [topic] then in [stream].
396+
///
397+
/// Returns a negative number if [userA] has more recent activity,
398+
/// returns a positive number if [userB] has more recent activity, and
399+
/// returns `0` if the activity is the same or there is no activity at all.
400+
int compareAB({required String? topic}) {
401+
return view.compareByRecency(
402+
userA,
403+
userB,
404+
streamId: stream.streamId,
405+
topic: topic,
406+
);
407+
}
408+
409+
test('prioritizes the user with more recent activity in the topic', () {
410+
prepare(
411+
users: [],
412+
dmConversations: [],
413+
messages: [],
414+
narrow: const AllMessagesNarrow(),
415+
);
416+
addMessage(userA, topic1);
417+
addMessage(userB, topic1);
418+
check(compareAB(topic: topic1)).isGreaterThan(0);
419+
});
420+
421+
test('prioritizes the user with more recent activity in the stream '
422+
'if there is no activity in the topic from both users', () {
423+
prepare(
424+
users: [],
425+
dmConversations: [],
426+
messages: [],
427+
narrow: const AllMessagesNarrow(),
428+
);
429+
addMessage(userA, topic1);
430+
addMessage(userB, topic1);
431+
check(compareAB(topic: topic2)).isGreaterThan(0);
432+
});
433+
434+
test('prioritizes the user with more recent activity in the stream '
435+
'if there is no topic provided', () {
436+
prepare(
437+
users: [],
438+
dmConversations: [],
439+
messages: [],
440+
narrow: const AllMessagesNarrow(),
441+
);
442+
addMessage(userA, topic1);
443+
addMessage(userB, topic2);
444+
check(compareAB(topic: null)).isGreaterThan(0);
445+
});
446+
447+
test('prioritizes none of the users if there is no activity in the stream from both users', () {
448+
prepare(
449+
users: [],
450+
dmConversations: [],
451+
messages: [],
452+
narrow: const AllMessagesNarrow(),
453+
);
454+
check(compareAB(topic: null)).equals(0);
455+
});
456+
});
457+
458+
test('compareByDms gives priority to user with DM exchanged more recently', () {
375459
prepare(
376460
users: [],
377461
dmConversations: [
378462
RecentDmConversation(userIds: [1], maxMessageId: 200),
379463
RecentDmConversation(userIds: [1, 2], maxMessageId: 100),
380464
],
465+
messages: [],
381466
narrow: const AllMessagesNarrow(),
382467
);
383468

@@ -388,23 +473,44 @@ void main() {
388473
});
389474

390475
test('autocomplete suggests relevant users in the following order: '
391-
'1. Users most recent in the DM conversations', () async {
476+
'1. User most recent in the current topic/stream '
477+
'2. Users most recent in the DM conversations', () async {
392478
final users = [
479+
eg.user(userId: 0),
393480
eg.user(userId: 1),
394481
eg.user(userId: 2),
395482
eg.user(userId: 3),
396483
eg.user(userId: 4),
397-
eg.user(userId: 5),
398484
];
399485

400486
final dmConversations = [
401-
RecentDmConversation(userIds: [4], maxMessageId: 300),
402-
RecentDmConversation(userIds: [1], maxMessageId: 200),
403-
RecentDmConversation(userIds: [1, 2], maxMessageId: 100),
487+
RecentDmConversation(userIds: [3], maxMessageId: 300),
488+
RecentDmConversation(userIds: [0], maxMessageId: 200),
489+
RecentDmConversation(userIds: [0, 1], maxMessageId: 100),
490+
];
491+
492+
const streamId = 1;
493+
const topic = 'topic';
494+
495+
final messages = [
496+
eg.streamMessage(
497+
sender: users[0],
498+
stream: eg.stream(streamId: streamId),
499+
topic: topic,
500+
),
501+
eg.streamMessage(
502+
sender: users[4],
503+
stream: eg.stream(streamId: streamId),
504+
),
404505
];
405506

406507
Future<void> checkResultsIn(Narrow narrow, {required List<int> expected}) async {
407-
prepare(users: users, dmConversations: dmConversations, narrow: narrow);
508+
prepare(
509+
users: users,
510+
dmConversations: dmConversations,
511+
messages: messages,
512+
narrow: narrow,
513+
);
408514

409515
bool done = false;
410516
view.addListener(() { done = true; });
@@ -417,19 +523,19 @@ void main() {
417523
check(results).deepEquals(expected);
418524
}
419525

420-
const streamNarrow = StreamNarrow(1);
421-
await checkResultsIn(streamNarrow, expected: [4, 1, 2, 3, 5]);
526+
const streamNarrow = StreamNarrow(streamId);
527+
await checkResultsIn(streamNarrow, expected: [4, 0, 3, 1, 2]);
422528

423-
const topicNarrow = TopicNarrow(1, 'topic');
424-
await checkResultsIn(topicNarrow, expected: [4, 1, 2, 3, 5]);
529+
const topicNarrow = TopicNarrow(streamId, topic);
530+
await checkResultsIn(topicNarrow, expected: [0, 4, 3, 1, 2]);
425531

426532
final dmNarrow = DmNarrow(allRecipientIds: [eg.selfUser.userId], selfUserId: eg.selfUser.userId);
427-
await checkResultsIn(dmNarrow, expected: [4, 1, 2, 3, 5]);
533+
await checkResultsIn(dmNarrow, expected: [3, 0, 1, 2, 4]);
428534

429535
const allMessagesNarrow = AllMessagesNarrow();
430536
// Results are in the original order as we do not sort them for
431537
// [AllMessagesNarrow] because we can not access autocomplete for now.
432-
await checkResultsIn(allMessagesNarrow, expected: [1, 2, 3, 4, 5]);
538+
await checkResultsIn(allMessagesNarrow, expected: [0, 1, 2, 3, 4]);
433539
});
434540
});
435541
}

0 commit comments

Comments
 (0)