Skip to content

Commit 3010865

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 b45ec95 commit 3010865

File tree

2 files changed

+158
-8
lines changed

2 files changed

+158
-8
lines changed

lib/model/autocomplete.dart

+28-3
Original file line numberDiff line numberDiff line change
@@ -177,14 +177,39 @@ class MentionAutocompleteView extends ChangeNotifier {
177177
final view = MentionAutocompleteView._(
178178
store: store,
179179
narrow: narrow,
180-
sortedUsers: _usersByRelevance(store: store),
180+
sortedUsers: _usersByRelevance(store: store, narrow: narrow),
181181
);
182182
store.autocompleteViewManager.registerMentionAutocomplete(view);
183183
return view;
184184
}
185185

186-
static List<User> _usersByRelevance({required PerAccountStore store}) {
187-
return store.users.values.toList(); // TODO(#228): sort for most relevant first
186+
static List<User> _usersByRelevance({
187+
required PerAccountStore store,
188+
required Narrow narrow,
189+
}) {
190+
assert(narrow is! CombinedFeedNarrow);
191+
return store.users.values.toList()
192+
..sort((userA, userB) => compareByDms(userA, userB, store: store));
193+
}
194+
195+
/// Determines which of the two users is more recent in DM conversations.
196+
///
197+
/// Returns a negative number if [userA] is more recent than [userB],
198+
/// returns a positive number if [userB] is more recent than [userA],
199+
/// and returns `0` if both [userA] and [userB] are equally recent
200+
/// or there is no DM exchanged with them whatsoever.
201+
@visibleForTesting
202+
static int compareByDms(User userA, User userB, {required PerAccountStore store}) {
203+
final recentDms = store.recentDmConversationsView;
204+
final aLatestMessageId = recentDms.latestMessagesByRecipient[userA.userId];
205+
final bLatestMessageId = recentDms.latestMessagesByRecipient[userB.userId];
206+
207+
return switch((aLatestMessageId, bLatestMessageId)) {
208+
(int a, int b) => -a.compareTo(b),
209+
(int(), _) => -1,
210+
(_, int()) => 1,
211+
_ => 0,
212+
};
188213
}
189214

190215
@override

test/model/autocomplete_test.dart

+130-5
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;
@@ -166,7 +168,7 @@ void main() {
166168
});
167169

168170
test('MentionAutocompleteView misc', () async {
169-
const narrow = CombinedFeedNarrow();
171+
const narrow = StreamNarrow(1);
170172
final store = eg.store();
171173
await store.addUsers([eg.selfUser, eg.otherUser, eg.thirdUser]);
172174
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
@@ -183,7 +185,7 @@ void main() {
183185

184186
test('MentionAutocompleteView not starve timers', () {
185187
fakeAsync((binding) async {
186-
const narrow = CombinedFeedNarrow();
188+
const narrow = StreamNarrow(1);
187189
final store = eg.store();
188190
await store.addUsers([eg.selfUser, eg.otherUser, eg.thirdUser]);
189191
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
@@ -218,7 +220,7 @@ void main() {
218220
});
219221

220222
test('MentionAutocompleteView yield between batches of 1000', () async {
221-
const narrow = CombinedFeedNarrow();
223+
const narrow = StreamNarrow(1);
222224
final store = eg.store();
223225
for (int i = 0; i < 2500; i++) {
224226
await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i'));
@@ -241,7 +243,7 @@ void main() {
241243
});
242244

243245
test('MentionAutocompleteView new query during computation replaces old', () async {
244-
const narrow = CombinedFeedNarrow();
246+
const narrow = StreamNarrow(1);
245247
final store = eg.store();
246248
for (int i = 0; i < 1500; i++) {
247249
await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i'));
@@ -276,7 +278,7 @@ void main() {
276278

277279
test('MentionAutocompleteView mutating store.users while in progress does not '
278280
'interrupt the current query results', () async {
279-
const narrow = CombinedFeedNarrow();
281+
const narrow = StreamNarrow(1);
280282
final store = eg.store();
281283
for (int i = 0; i < 2500; i++) {
282284
await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i'));
@@ -349,4 +351,127 @@ void main() {
349351
doCheck('Four F', eg.user(fullName: 'Full Name Four Words'), false);
350352
});
351353
});
354+
355+
group('MentionAutocompleteView sorting users results', () {
356+
late PerAccountStore store;
357+
358+
Future<void> prepare({
359+
List<User> users = const [],
360+
List<RecentDmConversation> dmConversations = const [],
361+
}) async {
362+
store = eg.store(initialSnapshot: eg.initialSnapshot(
363+
recentPrivateConversations: dmConversations));
364+
await store.addUsers(users);
365+
}
366+
367+
group('MentionAutocompleteView.compareByDms', () {
368+
const idA = 1;
369+
const idB = 2;
370+
371+
int compareAB() => MentionAutocompleteView.compareByDms(
372+
eg.user(userId: idA),
373+
eg.user(userId: idB),
374+
store: store,
375+
);
376+
377+
test('has DMs with userA and userB, latest with userA -> prioritizes userA', () async {
378+
await prepare(dmConversations: [
379+
RecentDmConversation(userIds: [idA], maxMessageId: 200),
380+
RecentDmConversation(userIds: [idA, idB], maxMessageId: 100),
381+
]);
382+
check(compareAB()).isLessThan(0);
383+
});
384+
385+
test('has DMs with userA and userB, latest with userB -> prioritizes userB', () async {
386+
await prepare(dmConversations: [
387+
RecentDmConversation(userIds: [idB], maxMessageId: 200),
388+
RecentDmConversation(userIds: [idA, idB], maxMessageId: 100),
389+
]);
390+
check(compareAB()).isGreaterThan(0);
391+
});
392+
393+
test('has DMs with userA and userB, equally recent -> prioritizes neither', () async {
394+
await prepare(dmConversations: [
395+
RecentDmConversation(userIds: [idA, idB], maxMessageId: 100),
396+
]);
397+
check(compareAB()).equals(0);
398+
});
399+
400+
test('has DMs with userA but not userB -> prioritizes userA', () async {
401+
await prepare(dmConversations: [
402+
RecentDmConversation(userIds: [idA], maxMessageId: 100),
403+
]);
404+
check(compareAB()).isLessThan(0);
405+
});
406+
407+
test('has DMs with userB but not userA -> prioritizes userB', () async {
408+
await prepare(dmConversations: [
409+
RecentDmConversation(userIds: [idB], maxMessageId: 100),
410+
]);
411+
check(compareAB()).isGreaterThan(0);
412+
});
413+
414+
test('has no DMs with userA or userB -> prioritizes neither', () async {
415+
await prepare(dmConversations: []);
416+
check(compareAB()).equals(0);
417+
});
418+
});
419+
420+
group('autocomplete suggests relevant users in the intended order', () {
421+
// The order should be:
422+
// 1. Users most recent in the DM conversations
423+
424+
Future<void> checkResultsIn(Narrow narrow, {required List<int> expected}) async {
425+
final users = [
426+
eg.user(userId: 0),
427+
eg.user(userId: 1),
428+
eg.user(userId: 2),
429+
eg.user(userId: 3),
430+
eg.user(userId: 4),
431+
];
432+
433+
final dmConversations = [
434+
RecentDmConversation(userIds: [3], maxMessageId: 300),
435+
RecentDmConversation(userIds: [0], maxMessageId: 200),
436+
RecentDmConversation(userIds: [0, 1], maxMessageId: 100),
437+
];
438+
439+
await prepare(users: users, dmConversations: dmConversations);
440+
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
441+
442+
bool done = false;
443+
view.addListener(() { done = true; });
444+
view.query = MentionAutocompleteQuery('');
445+
await Future(() {});
446+
check(done).isTrue();
447+
final results = view.results
448+
.map((e) => (e as UserMentionAutocompleteResult).userId);
449+
check(results).deepEquals(expected);
450+
}
451+
452+
test('StreamNarrow', () async {
453+
await checkResultsIn(const StreamNarrow(1), expected: [3, 0, 1, 2, 4]);
454+
});
455+
456+
test('TopicNarrow', () async {
457+
await checkResultsIn(const TopicNarrow(1, 'topic'), expected: [3, 0, 1, 2, 4]);
458+
});
459+
460+
test('DmNarrow', () async {
461+
await checkResultsIn(
462+
DmNarrow.withUser(eg.selfUser.userId, selfUserId: eg.selfUser.userId),
463+
expected: [3, 0, 1, 2, 4],
464+
);
465+
});
466+
467+
test('CombinedFeedNarrow', () async {
468+
// As we do not expect a compose box in [CombinedFeedNarrow], it should
469+
// not proceed to show any results.
470+
await check(checkResultsIn(
471+
const CombinedFeedNarrow(),
472+
expected: [0, 1, 2, 3, 4])
473+
).throws();
474+
});
475+
});
476+
});
352477
}

0 commit comments

Comments
 (0)