Skip to content

Commit 2fa9e68

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. Fixes part of: #228
1 parent 3e374f0 commit 2fa9e68

File tree

3 files changed

+182
-7
lines changed

3 files changed

+182
-7
lines changed

lib/model/autocomplete.dart

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

195279
/// Determines which of the two users is more recent in DM conversations.
@@ -215,6 +299,7 @@ class MentionAutocompleteView extends ChangeNotifier {
215299
/// non-null and returns a positive number if [b] is null and [a] is non-null.
216300
///
217301
/// If both are null, returns zero.
302+
@visibleForTesting
218303
static int compareNullable(int? a, int? b) => switch ((a, b)) {
219304
(int a, int b) => a.compareTo(b),
220305
(int(), _) => 1,

lib/model/message_list.dart

+5
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
383383
if (_messageVisible(message)) {
384384
_addMessage(message);
385385
}
386+
store.recentSenders.handleMessage(message);
386387
}
387388
_fetched = true;
388389
_haveOldest = result.foundOldest;
@@ -420,6 +421,10 @@ class MessageListView with ChangeNotifier, _MessageSequence {
420421
? result.messages // Avoid unnecessarily copying the list.
421422
: result.messages.where(_messageVisible);
422423

424+
for (final message in fetchedMessages) {
425+
store.recentSenders.handleMessage(message);
426+
}
427+
423428
_insertAllMessages(0, fetchedMessages);
424429
_haveOldest = result.foundOldest;
425430
} finally {

test/model/autocomplete_test.dart

+90-5
Original file line numberDiff line numberDiff line change
@@ -358,10 +358,12 @@ void main() {
358358
Future<void> prepare({
359359
List<User> users = const [],
360360
List<RecentDmConversation> dmConversations = const [],
361+
List<Message> messages = const [],
361362
}) async {
362363
store = eg.store(initialSnapshot: eg.initialSnapshot(
363364
recentPrivateConversations: dmConversations));
364365
await store.addUsers(users);
366+
await store.addMessages(messages);
365367
}
366368

367369
group('MentionAutocompleteView.compareNullable', () {
@@ -378,6 +380,71 @@ void main() {
378380

379381
test('both of [a] and [b] are null', () async {
380382
check(MentionAutocompleteView.compareNullable(null, null)).equals(0);
383+
check(MentionAutocompleteView.compareNullable(null, null)).equals(0);
384+
});
385+
});
386+
387+
group('MentionAutocompleteView.compareByRecency', () {
388+
final userA = eg.otherUser;
389+
final userB = eg.thirdUser;
390+
final stream = eg.stream();
391+
const topic1 = 'topic1';
392+
const topic2 = 'topic2';
393+
394+
Message message(User sender, String topic) {
395+
return eg.streamMessage(
396+
sender: sender,
397+
stream: stream,
398+
topic: topic,
399+
);
400+
}
401+
402+
/// Determines the priority between [userA] and [userB] based on their activity.
403+
///
404+
/// The activity is first looked for in [topic] then in [stream].
405+
///
406+
/// Returns a negative number if [userA] has more recent activity,
407+
/// returns a positive number if [userB] has more recent activity, and
408+
/// returns `0` if the activity is the same or there is no activity at all.
409+
int compareAB({required String? topic}) {
410+
return MentionAutocompleteView.compareByRecency(
411+
userA,
412+
userB,
413+
streamId: stream.streamId,
414+
topic: topic,
415+
store: store,
416+
);
417+
}
418+
419+
test('prioritizes the user with more recent activity in the topic', () async {
420+
await prepare(messages: [
421+
message(userA, topic1),
422+
message(userB, topic1),
423+
]);
424+
check(compareAB(topic: topic1)).isGreaterThan(0);
425+
});
426+
427+
test('prioritizes the user with more recent activity in the stream '
428+
'if there is no activity in the topic from both users', () async {
429+
await prepare(messages: [
430+
message(userA, topic1),
431+
message(userB, topic1),
432+
]);
433+
check(compareAB(topic: topic2)).isGreaterThan(0);
434+
});
435+
436+
test('prioritizes the user with more recent activity in the stream '
437+
'if there is no topic provided', () async {
438+
await prepare(messages: [
439+
message(userA, topic1),
440+
message(userB, topic2),
441+
]);
442+
check(compareAB(topic: null)).isGreaterThan(0);
443+
});
444+
445+
test('prioritizes none of the users if there is no activity in the stream from both users', () async {
446+
await prepare(messages: []);
447+
check(compareAB(topic: null)).equals(0);
381448
});
382449
});
383450

@@ -435,8 +502,11 @@ void main() {
435502
});
436503

437504
group('autocomplete suggests relevant users in the intended order', () {
438-
// The order should be:
439-
// 1. Users most recent in the DM conversations
505+
// 1. Users most recent in the current topic/stream
506+
// 2. Users most recent in the DM conversations
507+
508+
final stream = eg.stream();
509+
const topic = 'topic';
440510

441511
Future<void> checkResultsIn(Narrow narrow, {required List<int> expected}) async {
442512
final users = [
@@ -453,7 +523,22 @@ void main() {
453523
RecentDmConversation(userIds: [0, 1], maxMessageId: 100),
454524
];
455525

456-
await prepare(users: users, dmConversations: dmConversations);
526+
final messages = [
527+
eg.streamMessage(
528+
sender: users[0],
529+
stream: stream,
530+
topic: topic,
531+
),
532+
eg.streamMessage(
533+
sender: users[4],
534+
stream: stream,
535+
),
536+
];
537+
538+
await prepare(
539+
users: users,
540+
dmConversations: dmConversations,
541+
messages: messages);
457542
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
458543

459544
bool done = false;
@@ -467,11 +552,11 @@ void main() {
467552
}
468553

469554
test('StreamNarrow', () async {
470-
await checkResultsIn(const StreamNarrow(1), expected: [3, 0, 1, 2, 4]);
555+
await checkResultsIn(StreamNarrow(stream.streamId), expected: [4, 0, 3, 1, 2]);
471556
});
472557

473558
test('TopicNarrow', () async {
474-
await checkResultsIn(const TopicNarrow(1, 'topic'), expected: [3, 0, 1, 2, 4]);
559+
await checkResultsIn(TopicNarrow(stream.streamId, topic), expected: [0, 4, 3, 1, 2]);
475560
});
476561

477562
test('DmNarrow', () async {

0 commit comments

Comments
 (0)