Skip to content

Commit 1805adc

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 a28cb9e commit 1805adc

File tree

2 files changed

+138
-6
lines changed

2 files changed

+138
-6
lines changed

lib/model/autocomplete.dart

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

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

test/model/autocomplete_test.dart

Lines changed: 74 additions & 5 deletions
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)