Skip to content

Commit 5cc7896

Browse files
committed
autocomplete: Add "recent activity in current topic/stream" criterion
In @-mention autocomplete, users are suggested based on: 1. Recent activity in the current topic/stream. 2. Recent DM conversations. Note: By "recent activity" we mean recent messages sent to a topic/stream. Fixes part of: #228
1 parent 7e90350 commit 5cc7896

File tree

2 files changed

+264
-29
lines changed

2 files changed

+264
-29
lines changed

lib/model/autocomplete.dart

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,74 @@ class MentionAutocompleteView extends ChangeNotifier {
187187
required PerAccountStore store,
188188
required Narrow narrow,
189189
}) {
190-
assert(narrow is! CombinedFeedNarrow);
190+
final (int?, String?) streamAndTopic;
191+
switch (narrow) {
192+
case StreamNarrow():
193+
streamAndTopic = (narrow.streamId, null);
194+
case TopicNarrow():
195+
streamAndTopic = (narrow.streamId, narrow.topic);
196+
case DmNarrow():
197+
streamAndTopic = (null, null);
198+
case CombinedFeedNarrow():
199+
assert(false, 'No compose box, thus no autocomplete is available in ${narrow.runtimeType}.');
200+
streamAndTopic = (null, null);
201+
}
202+
203+
final (streamId, topic) = streamAndTopic;
191204
return store.users.values.toList()
192-
..sort((userA, userB) => compareByDms(userA, userB, store: store));
205+
..sort((userA, userB) => _compareByRelevance(userA, userB,
206+
streamId: streamId,
207+
topic: topic,
208+
store: store));
209+
}
210+
211+
static int _compareByRelevance(User userA, User userB, {
212+
required int? streamId,
213+
required String? topic,
214+
required PerAccountStore store,
215+
}) {
216+
if (streamId != null) {
217+
final result = compareByRecency(userA, userB,
218+
streamId: streamId,
219+
topic: topic,
220+
store: store);
221+
if (result != 0) return result;
222+
}
223+
return compareByDms(userA, userB, store: store);
224+
}
225+
226+
/// Determines which of the two users has more recent activity (messages sent
227+
/// recently) in the topic/stream.
228+
///
229+
/// First checks for the activity in [topic] if provided.
230+
///
231+
/// If no [topic] is provided, or there is no activity in the topic at all,
232+
/// then checks for the activity in the stream with [streamId].
233+
///
234+
/// Returns a negative number if [userA] has more recent activity than [userB],
235+
/// returns a positive number if [userB] has more recent activity than [userA],
236+
/// and returns `0` if both [userA] and [userB] have no activity at all.
237+
@visibleForTesting
238+
static int compareByRecency(User userA, User userB, {
239+
required int streamId,
240+
required String? topic,
241+
required PerAccountStore store,
242+
}) {
243+
final recentSenders = store.recentSenders;
244+
if (topic != null) {
245+
final result = -compareRecentMessageIds(
246+
recentSenders.latestMessageIdOfSenderInTopic(
247+
streamId: streamId, topic: topic, senderId: userA.userId),
248+
recentSenders.latestMessageIdOfSenderInTopic(
249+
streamId: streamId, topic: topic, senderId: userB.userId));
250+
if (result != 0) return result;
251+
}
252+
253+
final aMessageId = recentSenders.latestMessageIdOfSenderInStream(
254+
streamId: streamId, senderId: userA.userId);
255+
final bMessageId = recentSenders.latestMessageIdOfSenderInStream(
256+
streamId: streamId, senderId: userB.userId);
257+
return -compareRecentMessageIds(aMessageId, bMessageId);
193258
}
194259

195260
/// Determines which of the two users is more recent in DM conversations.

test/model/autocomplete_test.dart

Lines changed: 197 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,14 @@ import 'package:test/scaffolding.dart';
88
import 'package:zulip/api/model/initial_snapshot.dart';
99
import 'package:zulip/api/model/model.dart';
1010
import 'package:zulip/model/autocomplete.dart';
11+
import 'package:zulip/model/message_list.dart';
1112
import 'package:zulip/model/narrow.dart';
1213
import 'package:zulip/model/store.dart';
1314
import 'package:zulip/widgets/compose_box.dart';
1415

16+
import '../api/fake_api.dart';
1517
import '../example_data.dart' as eg;
18+
import 'message_list_test.dart';
1619
import 'test_store.dart';
1720
import 'autocomplete_checks.dart';
1821

@@ -359,10 +362,12 @@ void main() {
359362
Future<void> prepare({
360363
List<User> users = const [],
361364
List<RecentDmConversation> dmConversations = const [],
365+
List<Message> messages = const [],
362366
}) async {
363367
store = eg.store(initialSnapshot: eg.initialSnapshot(
364368
recentPrivateConversations: dmConversations));
365369
await store.addUsers(users);
370+
await store.addMessages(messages);
366371
}
367372

368373
group('MentionAutocompleteView.compareRecentMessageIds', () {
@@ -382,6 +387,61 @@ void main() {
382387
});
383388
});
384389

390+
group('MentionAutocompleteView.compareByRecency', () {
391+
final userA = eg.otherUser;
392+
final userB = eg.thirdUser;
393+
final stream = eg.stream();
394+
const topic1 = 'topic1';
395+
const topic2 = 'topic2';
396+
397+
Message message(User sender, String topic) {
398+
return eg.streamMessage(
399+
sender: sender,
400+
stream: stream,
401+
topic: topic,
402+
);
403+
}
404+
405+
int compareAB({required String? topic}) {
406+
return MentionAutocompleteView.compareByRecency(userA, userB,
407+
streamId: stream.streamId,
408+
topic: topic,
409+
store: store,
410+
);
411+
}
412+
413+
test('prioritizes the user with more recent activity in the topic', () async {
414+
await prepare(messages: [
415+
message(userA, topic1),
416+
message(userB, topic1),
417+
]);
418+
check(compareAB(topic: topic1)).isGreaterThan(0);
419+
});
420+
421+
test('no activity in topic -> prioritizes the user with more recent '
422+
'activity in the stream', () async {
423+
await prepare(messages: [
424+
message(userA, topic1),
425+
message(userB, topic1),
426+
]);
427+
check(compareAB(topic: topic2)).isGreaterThan(0);
428+
});
429+
430+
test('no topic provided -> prioritizes the user with more recent '
431+
'activity in the stream', () async {
432+
await prepare(messages: [
433+
message(userA, topic1),
434+
message(userB, topic2),
435+
]);
436+
check(compareAB(topic: null)).isGreaterThan(0);
437+
});
438+
439+
test('no activity in topic/stream -> prioritizes none', () async {
440+
await prepare(messages: []);
441+
check(compareAB(topic: null)).equals(0);
442+
});
443+
});
444+
385445
group('MentionAutocompleteView.compareByDms', () {
386446
const idA = 1;
387447
const idB = 2;
@@ -436,25 +496,37 @@ void main() {
436496
});
437497

438498
group('autocomplete suggests relevant users in the intended order', () {
439-
// The order should be:
440-
// 1. Users most recent in the DM conversations
499+
// 1. Users most recent in the current topic/stream.
500+
// 2. Users most recent in the DM conversations.
501+
502+
final stream = eg.stream();
503+
const topic = 'topic';
504+
final streamNarrow = StreamNarrow(stream.streamId);
505+
final topicNarrow = TopicNarrow(stream.streamId, topic);
506+
final dmNarrow = DmNarrow.withUser(eg.selfUser.userId, selfUserId: eg.selfUser.userId);
507+
508+
final users = List.generate(5, (i) => eg.user(userId: i));
509+
510+
final dmConversations = [
511+
RecentDmConversation(userIds: [3], maxMessageId: 300),
512+
RecentDmConversation(userIds: [0], maxMessageId: 200),
513+
RecentDmConversation(userIds: [0, 1], maxMessageId: 100),
514+
];
515+
516+
StreamMessage streamMessage({required int id, required int senderId, String? topic}) =>
517+
eg.streamMessage(id: id, sender: users[senderId], topic: topic, stream: stream);
518+
519+
final messages = [
520+
streamMessage(id: 50, senderId: 0, topic: topic),
521+
streamMessage(id: 60, senderId: 4),
522+
];
523+
524+
Future<void> prepareStore({bool includeMessageHistory = false}) async {
525+
await prepare(users: users, dmConversations: dmConversations,
526+
messages: includeMessageHistory ? messages : []);
527+
}
441528

442529
Future<void> checkResultsIn(Narrow narrow, {required List<int> expected}) async {
443-
final users = [
444-
eg.user(userId: 0),
445-
eg.user(userId: 1),
446-
eg.user(userId: 2),
447-
eg.user(userId: 3),
448-
eg.user(userId: 4),
449-
];
450-
451-
final dmConversations = [
452-
RecentDmConversation(userIds: [3], maxMessageId: 300),
453-
RecentDmConversation(userIds: [0], maxMessageId: 200),
454-
RecentDmConversation(userIds: [0, 1], maxMessageId: 100),
455-
];
456-
457-
await prepare(users: users, dmConversations: dmConversations);
458530
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
459531

460532
bool done = false;
@@ -467,22 +539,120 @@ void main() {
467539
check(results).deepEquals(expected);
468540
}
469541

470-
test('StreamNarrow', () async {
471-
await checkResultsIn(const StreamNarrow(1), expected: [3, 0, 1, 2, 4]);
472-
});
542+
group('StreamNarrow & TopicNarrow', () {
543+
late FakeApiConnection connection;
544+
late MessageListView messageList;
545+
546+
Future<void> fetchInitialMessagesIn(Narrow narrow) async {
547+
connection = store.connection as FakeApiConnection;
548+
connection.prepare(json: newestResult(
549+
foundOldest: false,
550+
messages: narrow is StreamNarrow
551+
? messages
552+
: messages.where((m) => m.topic == topic).toList(),
553+
).toJson());
554+
messageList = MessageListView.init(store: store, narrow: narrow);
555+
await messageList.fetchInitial();
556+
}
473557

474-
test('TopicNarrow', () async {
475-
await checkResultsIn(const TopicNarrow(1, 'topic'), expected: [3, 0, 1, 2, 4]);
558+
Future<void> checkInitialResultsIn(Narrow narrow,
559+
{required List<int> expected, bool includeStream = false}) async {
560+
assert(narrow is! StreamNarrow || !includeStream);
561+
await prepareStore(includeMessageHistory: includeStream);
562+
await fetchInitialMessagesIn(narrow);
563+
await checkResultsIn(narrow, expected: expected);
564+
}
565+
566+
test('StreamNarrow', () async {
567+
await checkInitialResultsIn(streamNarrow, expected: [4, 0, 3, 1, 2]);
568+
});
569+
570+
test('StreamNarrow, new message arrives', () async {
571+
await checkInitialResultsIn(streamNarrow, expected: [4, 0, 3, 1, 2]);
572+
573+
// Until now, latest message id in [stream] is 60.
574+
await store.addMessage(streamMessage(id: 70, senderId: 2));
575+
576+
await checkResultsIn(streamNarrow, expected: [2, 4, 0, 3, 1]);
577+
});
578+
579+
test('StreamNarrow, a batch of older messages arrives', () async {
580+
await checkInitialResultsIn(streamNarrow, expected: [4, 0, 3, 1, 2]);
581+
582+
// Until now, oldest message id in [stream] is 50.
583+
final oldMessages = [
584+
streamMessage(id: 30, senderId: 1),
585+
streamMessage(id: 40, senderId: 2),
586+
];
587+
connection.prepare(json: olderResult(
588+
anchor: 50, foundOldest: false,
589+
messages: oldMessages,
590+
).toJson());
591+
await messageList.fetchOlder();
592+
593+
await checkResultsIn(streamNarrow, expected: [4, 0, 2, 1, 3]);
594+
});
595+
596+
test('TopicNarrow, no other messages are in stream', () async {
597+
await checkInitialResultsIn(topicNarrow, expected: [0, 3, 1, 2, 4]);
598+
});
599+
600+
test('TopicNarrow, other messages are in stream', () async {
601+
await checkInitialResultsIn(topicNarrow, expected: [0, 4, 3, 1, 2],
602+
includeStream: true);
603+
});
604+
605+
test('TopicNarrow, new message arrives', () async {
606+
await checkInitialResultsIn(topicNarrow, expected: [0, 3, 1, 2, 4]);
607+
608+
// Until now, latest message id in [topic] is 50.
609+
await store.addMessage(streamMessage(id: 60, senderId: 2, topic: topic));
610+
611+
await checkResultsIn(topicNarrow, expected: [2, 0, 3, 1, 4]);
612+
});
613+
614+
test('TopicNarrow, a batch of older messages arrives', () async {
615+
await checkInitialResultsIn(topicNarrow, expected: [0, 3, 1, 2, 4]);
616+
617+
// Until now, oldest message id in [topic] is 50.
618+
final oldMessages = [
619+
streamMessage(id: 30, senderId: 2, topic: topic),
620+
streamMessage(id: 40, senderId: 4, topic: topic),
621+
];
622+
connection.prepare(json: olderResult(
623+
anchor: 50, foundOldest: false,
624+
messages: oldMessages,
625+
).toJson());
626+
627+
await messageList.fetchOlder();
628+
await checkResultsIn(topicNarrow, expected: [0, 4, 2, 3, 1]);
629+
});
476630
});
477631

478-
test('DmNarrow', () async {
479-
await checkResultsIn(
480-
DmNarrow.withUser(eg.selfUser.userId, selfUserId: eg.selfUser.userId),
481-
expected: [3, 0, 1, 2, 4],
482-
);
632+
group('DmNarrow', () {
633+
test('DmNarrow, with no topic/stream message history', () async {
634+
await prepareStore();
635+
await checkResultsIn(dmNarrow, expected: [3, 0, 1, 2, 4]);
636+
});
637+
638+
test('DmNarrow, with topic/stream message history', () async {
639+
await prepareStore(includeMessageHistory: true);
640+
await checkResultsIn(dmNarrow, expected: [3, 0, 1, 2, 4]);
641+
});
642+
643+
test('DmNarrow, new message arrives', () async {
644+
await prepareStore();
645+
await checkResultsIn(dmNarrow, expected: [3, 0, 1, 2, 4]);
646+
647+
// Until now, latest message id in recent DMs is 300.
648+
await store.addMessage(eg.dmMessage(id: 400, from: users[1], to: [eg.selfUser]));
649+
650+
await checkResultsIn(dmNarrow, expected: [1, 3, 0, 2, 4]);
651+
});
483652
});
484653

485654
test('CombinedFeedNarrow', () async {
655+
await prepareStore();
486656
// As we do not expect a compose box in [CombinedFeedNarrow], it should
487657
// not proceed to show any results.
488658
await check(checkResultsIn(

0 commit comments

Comments
 (0)