Skip to content

Commit d0c3f8d

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: zulip#228
1 parent 3457070 commit d0c3f8d

File tree

2 files changed

+145
-6
lines changed

2 files changed

+145
-6
lines changed

lib/model/autocomplete.dart

+71-1
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,78 @@ class MentionAutocompleteView extends ChangeNotifier {
188188
required Narrow narrow,
189189
}) {
190190
assert(narrow is! CombinedFeedNarrow);
191+
192+
final int? streamId;
193+
final String? topic;
194+
switch (narrow) {
195+
case StreamNarrow():
196+
streamId = narrow.streamId;
197+
topic = null;
198+
case TopicNarrow():
199+
streamId = narrow.streamId;
200+
topic = narrow.topic;
201+
case DmNarrow():
202+
case CombinedFeedNarrow(): // This case will not reach as we have an assert for that.
203+
streamId = null;
204+
topic = null;
205+
}
191206
return store.users.values.toList()
192-
..sort((userA, userB) => compareByDms(userA, userB, store: store));
207+
..sort((userA, userB) => _compareByRelevance(userA, userB,
208+
streamId: streamId,
209+
topic: topic,
210+
store: store));
211+
}
212+
213+
static int _compareByRelevance(User userA, User userB, {
214+
required int? streamId,
215+
required String? topic,
216+
required PerAccountStore store,
217+
}) {
218+
if (streamId != null) {
219+
final result = compareByRecency(userA, userB,
220+
streamId: streamId,
221+
topic: topic,
222+
store: store);
223+
if (result != 0) {
224+
return result;
225+
}
226+
}
227+
return compareByDms(userA, userB, store: store);
228+
}
229+
230+
/// Determines which of the two users has more recent activity (messages sent
231+
/// recently) in the topic/stream.
232+
///
233+
/// First checks for the activity in [topic] if provided.
234+
///
235+
/// If no [topic] is provided, or there is no activity in the topic at all,
236+
/// then checks for the activity in the stream with [streamId].
237+
///
238+
/// Returns a negative number if [userA] has more recent activity than [userB],
239+
/// returns a positive number if [userB] has more recent activity than [userA],
240+
/// and returns `0` if both [userA] and [userB] have no activity at all.
241+
@visibleForTesting
242+
static int compareByRecency(User userA, User userB, {
243+
required int streamId,
244+
required String? topic,
245+
required PerAccountStore store,
246+
}) {
247+
final recentSenders = store.recentSenders;
248+
if (topic != null) {
249+
final aMessageId = recentSenders.latestMessageIdOfSenderInTopic(
250+
streamId: streamId, topic: topic, senderId: userA.userId);
251+
final bMessageId = recentSenders.latestMessageIdOfSenderInTopic(
252+
streamId: streamId, topic: topic, senderId: userB.userId);
253+
254+
final result = -compareNullable(aMessageId, bMessageId);
255+
if (result != 0) return result;
256+
}
257+
258+
final aMessageId = recentSenders.latestMessageIdOfSenderInStream(
259+
streamId: streamId, senderId: userA.userId);
260+
final bMessageId = recentSenders.latestMessageIdOfSenderInStream(
261+
streamId: streamId, senderId: userB.userId);
262+
return -compareNullable(aMessageId, bMessageId);
193263
}
194264

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

test/model/autocomplete_test.dart

+74-5
Original file line numberDiff line numberDiff line change
@@ -359,10 +359,12 @@ void main() {
359359
Future<void> prepare({
360360
List<User> users = const [],
361361
List<RecentDmConversation> dmConversations = const [],
362+
List<Message> messages = const [],
362363
}) async {
363364
store = eg.store(initialSnapshot: eg.initialSnapshot(
364365
recentPrivateConversations: dmConversations));
365366
await store.addUsers(users);
367+
await store.addMessages(messages);
366368
}
367369

368370
group('MentionAutocompleteView.compareNullable', () {
@@ -379,6 +381,62 @@ void main() {
379381

380382
test('both of [a] and [b] are null', () async {
381383
check(MentionAutocompleteView.compareNullable(null, null)).equals(0);
384+
check(MentionAutocompleteView.compareNullable(null, null)).equals(0);
385+
});
386+
});
387+
388+
group('MentionAutocompleteView.compareByRecency', () {
389+
final userA = eg.otherUser;
390+
final userB = eg.thirdUser;
391+
final stream = eg.stream();
392+
const topic1 = 'topic1';
393+
const topic2 = 'topic2';
394+
395+
Message message(User sender, String topic) {
396+
return eg.streamMessage(
397+
sender: sender,
398+
stream: stream,
399+
topic: topic,
400+
);
401+
}
402+
403+
int compareAB({required String? topic}) {
404+
return MentionAutocompleteView.compareByRecency(userA, userB,
405+
streamId: stream.streamId,
406+
topic: topic,
407+
store: store,
408+
);
409+
}
410+
411+
test('prioritizes the user with more recent activity in the topic', () async {
412+
await prepare(messages: [
413+
message(userA, topic1),
414+
message(userB, topic1),
415+
]);
416+
check(compareAB(topic: topic1)).isGreaterThan(0);
417+
});
418+
419+
test('prioritizes the user with more recent activity in the stream '
420+
'if there is no activity in the topic from both users', () async {
421+
await prepare(messages: [
422+
message(userA, topic1),
423+
message(userB, topic1),
424+
]);
425+
check(compareAB(topic: topic2)).isGreaterThan(0);
426+
});
427+
428+
test('prioritizes the user with more recent activity in the stream '
429+
'if there is no topic provided', () async {
430+
await prepare(messages: [
431+
message(userA, topic1),
432+
message(userB, topic2),
433+
]);
434+
check(compareAB(topic: null)).isGreaterThan(0);
435+
});
436+
437+
test('prioritizes none of the users if there is no activity in the stream from both users', () async {
438+
await prepare(messages: []);
439+
check(compareAB(topic: null)).equals(0);
382440
});
383441
});
384442

@@ -436,8 +494,11 @@ void main() {
436494
});
437495

438496
group('autocomplete suggests relevant users in the intended order', () {
439-
// The order should be:
440-
// 1. Users most recent in the DM conversations
497+
// 1. Users most recent in the current topic/stream.
498+
// 2. Users most recent in the DM conversations.
499+
500+
final stream = eg.stream();
501+
const topic = 'topic';
441502

442503
Future<void> checkResultsIn(Narrow narrow, {required List<int> expected}) async {
443504
final users = [
@@ -454,7 +515,15 @@ void main() {
454515
RecentDmConversation(userIds: [0, 1], maxMessageId: 100),
455516
];
456517

457-
await prepare(users: users, dmConversations: dmConversations);
518+
final messages = [
519+
eg.streamMessage(sender: users[0], stream: stream, topic: topic),
520+
eg.streamMessage(sender: users[4], stream: stream),
521+
];
522+
523+
await prepare(
524+
users: users,
525+
dmConversations: dmConversations,
526+
messages: messages);
458527
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
459528

460529
bool done = false;
@@ -468,11 +537,11 @@ void main() {
468537
}
469538

470539
test('StreamNarrow', () async {
471-
await checkResultsIn(const StreamNarrow(1), expected: [3, 0, 1, 2, 4]);
540+
await checkResultsIn(StreamNarrow(stream.streamId), expected: [4, 0, 3, 1, 2]);
472541
});
473542

474543
test('TopicNarrow', () async {
475-
await checkResultsIn(const TopicNarrow(1, 'topic'), expected: [3, 0, 1, 2, 4]);
544+
await checkResultsIn(TopicNarrow(stream.streamId, topic), expected: [0, 4, 3, 1, 2]);
476545
});
477546

478547
test('DmNarrow', () async {

0 commit comments

Comments
 (0)