Skip to content

Commit 9a1c213

Browse files
committed
autocomplete: Add "recent DM conversations" criterion
In @-mention autocomplete, users are suggested based on: 1. Recent DM conversations. Fixes part of: #228
1 parent a24dbde commit 9a1c213

File tree

2 files changed

+173
-3
lines changed

2 files changed

+173
-3
lines changed

lib/model/autocomplete.dart

+44-3
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ class MentionAutocompleteView extends ChangeNotifier {
175175
required Narrow narrow,
176176
}) {
177177
final users = store.users.values.toList();
178-
final sortedUsers = sortByRelevance(users: users);
178+
final sortedUsers = sortByRelevance(users: users, narrow: narrow, store: store);
179179
final view = MentionAutocompleteView._(
180180
store: store,
181181
narrow: narrow,
@@ -185,8 +185,49 @@ class MentionAutocompleteView extends ChangeNotifier {
185185
return view;
186186
}
187187

188-
static List<User> sortByRelevance({required List<User> users}) {
189-
return users; // TODO(#228) sort for most relevant first
188+
static List<User> sortByRelevance({
189+
required List<User> users,
190+
required Narrow narrow,
191+
required PerAccountStore store,
192+
}) {
193+
switch (narrow) {
194+
case StreamNarrow():
195+
case TopicNarrow():
196+
case DmNarrow():
197+
users.sort((userA, userB) => compareByRelevance(
198+
userA: userA, userB: userB, store: store));
199+
case CombinedFeedNarrow():
200+
// do nothing in this case for now
201+
}
202+
return users;
203+
}
204+
205+
static int compareByRelevance({
206+
required User userA,
207+
required User userB,
208+
required PerAccountStore store,
209+
}) {
210+
final dmPrecedence = compareByDms(userA, userB, store: store);
211+
return dmPrecedence;
212+
}
213+
214+
/// Determines which of the two users is more recent in DM conversations.
215+
///
216+
/// Returns a negative number if [userA] is more recent than [userB],
217+
/// returns a positive number if [userB] is more recent than [userA],
218+
/// and returns `0` if both [userA] and [userB] are equally recent
219+
/// or there is no DM exchanged with them whatsoever.
220+
static int compareByDms(User userA, User userB, {required PerAccountStore store}) {
221+
final recentDms = store.recentDmConversationsView;
222+
final aLatestMessageId = recentDms.latestMessagesByRecipient[userA.userId];
223+
final bLatestMessageId = recentDms.latestMessagesByRecipient[userB.userId];
224+
225+
return switch((aLatestMessageId, bLatestMessageId)) {
226+
(int a, int b) => -a.compareTo(b),
227+
(int(), _) => -1,
228+
(_, int()) => 1,
229+
_ => 0,
230+
};
190231
}
191232

192233
@override

test/model/autocomplete_test.dart

+129
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ import 'package:checks/checks.dart';
55
import 'package:fake_async/fake_async.dart';
66
import 'package:flutter/cupertino.dart';
77
import 'package:test/scaffolding.dart';
8+
import 'package:zulip/api/model/initial_snapshot.dart';
89
import 'package:zulip/api/model/model.dart';
910
import 'package:zulip/model/autocomplete.dart';
1011
import 'package:zulip/model/narrow.dart';
12+
import 'package:zulip/model/store.dart';
1113
import 'package:zulip/widgets/compose_box.dart';
1214

1315
import '../example_data.dart' as eg;
@@ -318,4 +320,131 @@ void main() {
318320
doCheck('Four F', eg.user(fullName: 'Full Name Four Words'), false);
319321
});
320322
});
323+
324+
group('MentionAutocompleteView sorting users results', () {
325+
late PerAccountStore store;
326+
327+
Future<void> prepare({
328+
List<User> users = const [],
329+
List<RecentDmConversation> dmConversations = const [],
330+
}) async {
331+
store = eg.store(initialSnapshot: eg.initialSnapshot(
332+
recentPrivateConversations: dmConversations));
333+
await store.addUsers(users);
334+
}
335+
336+
group('MentionAutocompleteView.compareByDms', () {
337+
test('has DMs with userA and userB, latest with userA, prioritizes userA', () async {
338+
await prepare(dmConversations: [
339+
RecentDmConversation(userIds: [1], maxMessageId: 200),
340+
RecentDmConversation(userIds: [1, 2], maxMessageId: 100),
341+
]);
342+
343+
final userA = eg.user(userId: 1);
344+
final userB = eg.user(userId: 2);
345+
final compareAB = MentionAutocompleteView.compareByDms(userA, userB, store: store);
346+
check(compareAB).isNegative();
347+
});
348+
349+
test('has DMs with userA and userB, latest with userB, prioritizes userB', () async {
350+
await prepare(dmConversations: [
351+
RecentDmConversation(userIds: [2], maxMessageId: 200),
352+
RecentDmConversation(userIds: [1, 2], maxMessageId: 100),
353+
]);
354+
355+
final userA = eg.user(userId: 1);
356+
final userB = eg.user(userId: 2);
357+
final compareAB = MentionAutocompleteView.compareByDms(userA, userB, store: store);
358+
check(compareAB).isGreaterThan(0);
359+
});
360+
361+
test('has DMs with userA and userB, equally recent, prioritizes neither', () async {
362+
await prepare(dmConversations: [
363+
RecentDmConversation(userIds: [1, 2], maxMessageId: 100),
364+
]);
365+
366+
final userA = eg.user(userId: 1);
367+
final userB = eg.user(userId: 2);
368+
final compareAB = MentionAutocompleteView.compareByDms(userA, userB, store: store);
369+
check(compareAB).equals(0);
370+
});
371+
372+
test('has DMs with userA but not userB, prioritizes userA', () async {
373+
await prepare(dmConversations: [
374+
RecentDmConversation(userIds: [1], maxMessageId: 100),
375+
]);
376+
377+
final userA = eg.user(userId: 1);
378+
final userB = eg.user(userId: 2);
379+
final compareAB = MentionAutocompleteView.compareByDms(userA, userB, store: store);
380+
check(compareAB).isNegative();
381+
});
382+
383+
test('has DMs with userB but not userA, prioritizes userB', () async {
384+
await prepare(dmConversations: [
385+
RecentDmConversation(userIds: [2], maxMessageId: 100),
386+
]);
387+
388+
final userA = eg.user(userId: 1);
389+
final userB = eg.user(userId: 2);
390+
final compareAB = MentionAutocompleteView.compareByDms(userA, userB, store: store);
391+
check(compareAB).isGreaterThan(0);
392+
});
393+
394+
test('doesn\'t have DMs with userA or userB, prioritizes neither', () async {
395+
await prepare(dmConversations: []);
396+
397+
final userA = eg.user(userId: 1);
398+
final userB = eg.user(userId: 2);
399+
final compareAB = MentionAutocompleteView.compareByDms(userA, userB, store: store);
400+
check(compareAB).equals(0);
401+
});
402+
});
403+
404+
test('autocomplete suggests relevant users in the following order: '
405+
'1. Users most recent in the DM conversations', () async {
406+
final users = [
407+
eg.user(userId: 0),
408+
eg.user(userId: 1),
409+
eg.user(userId: 2),
410+
eg.user(userId: 3),
411+
eg.user(userId: 4),
412+
];
413+
414+
final dmConversations = [
415+
RecentDmConversation(userIds: [3], maxMessageId: 300),
416+
RecentDmConversation(userIds: [0], maxMessageId: 200),
417+
RecentDmConversation(userIds: [0, 1], maxMessageId: 100),
418+
];
419+
420+
Future<void> checkResultsIn(Narrow narrow, {required List<int> expected}) async {
421+
await prepare(users: users, dmConversations: dmConversations);
422+
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
423+
424+
bool done = false;
425+
view.addListener(() { done = true; });
426+
view.query = MentionAutocompleteQuery('');
427+
await Future(() {});
428+
check(done).isTrue();
429+
final results = view.results
430+
.map((e) => (e as UserMentionAutocompleteResult).userId)
431+
.toList();
432+
check(results).deepEquals(expected);
433+
}
434+
435+
const streamNarrow = StreamNarrow(1);
436+
await checkResultsIn(streamNarrow, expected: [3, 0, 1, 2, 4]);
437+
438+
const topicNarrow = TopicNarrow(1, 'topic');
439+
await checkResultsIn(topicNarrow, expected: [3, 0, 1, 2, 4]);
440+
441+
final dmNarrow = DmNarrow(allRecipientIds: [eg.selfUser.userId], selfUserId: eg.selfUser.userId);
442+
await checkResultsIn(dmNarrow, expected: [3, 0, 1, 2, 4]);
443+
444+
const allMessagesNarrow = CombinedFeedNarrow();
445+
// Results are in the original order as we do not sort them for
446+
// [CombinedFeedNarrow] because we can not access autocomplete for now.
447+
await checkResultsIn(allMessagesNarrow, expected: [0, 1, 2, 3, 4]);
448+
});
449+
});
321450
}

0 commit comments

Comments
 (0)