Skip to content

Commit 18e7da6

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 4e2775b commit 18e7da6

File tree

2 files changed

+159
-8
lines changed

2 files changed

+159
-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

+131-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 is not '
278280
'reflected in 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'));
@@ -348,4 +350,128 @@ void main() {
348350
doCheck('Four F', eg.user(fullName: 'Full Name Four Words'), false);
349351
});
350352
});
353+
354+
group('MentionAutocompleteView sorting users results', () {
355+
late PerAccountStore store;
356+
357+
Future<void> prepare({
358+
List<User> users = const [],
359+
List<RecentDmConversation> dmConversations = const [],
360+
}) async {
361+
store = eg.store(initialSnapshot: eg.initialSnapshot(
362+
recentPrivateConversations: dmConversations));
363+
await store.addUsers(users);
364+
}
365+
366+
group('MentionAutocompleteView.compareByDms', () {
367+
const idA = 1;
368+
const idB = 2;
369+
370+
int compareAB() => MentionAutocompleteView.compareByDms(
371+
eg.user(userId: idA),
372+
eg.user(userId: idB),
373+
store: store,
374+
);
375+
376+
test('has DMs with userA and userB, latest with userA -> prioritizes userA', () async {
377+
await prepare(dmConversations: [
378+
RecentDmConversation(userIds: [idA], maxMessageId: 200),
379+
RecentDmConversation(userIds: [idA, idB], maxMessageId: 100),
380+
]);
381+
check(compareAB()).isLessThan(0);
382+
});
383+
384+
test('has DMs with userA and userB, latest with userB -> prioritizes userB', () async {
385+
await prepare(dmConversations: [
386+
RecentDmConversation(userIds: [idB], maxMessageId: 200),
387+
RecentDmConversation(userIds: [idA, idB], maxMessageId: 100),
388+
]);
389+
check(compareAB()).isGreaterThan(0);
390+
});
391+
392+
test('has DMs with userA and userB, equally recent -> prioritizes neither', () async {
393+
await prepare(dmConversations: [
394+
RecentDmConversation(userIds: [idA, idB], maxMessageId: 100),
395+
]);
396+
check(compareAB()).equals(0);
397+
});
398+
399+
test('has DMs with userA but not userB -> prioritizes userA', () async {
400+
await prepare(dmConversations: [
401+
RecentDmConversation(userIds: [idA], maxMessageId: 100),
402+
]);
403+
check(compareAB()).isLessThan(0);
404+
});
405+
406+
test('has DMs with userB but not userA -> prioritizes userB', () async {
407+
await prepare(dmConversations: [
408+
RecentDmConversation(userIds: [idB], maxMessageId: 100),
409+
]);
410+
check(compareAB()).isGreaterThan(0);
411+
});
412+
413+
test('has no DMs with userA or userB -> prioritizes neither', () async {
414+
await prepare(dmConversations: []);
415+
check(compareAB()).equals(0);
416+
});
417+
});
418+
419+
group('autocomplete suggests relevant users in the intended order', () {
420+
// The order should be:
421+
// 1. Users most recent in the DM conversations
422+
423+
Future<void> checkResultsIn(Narrow narrow, {required List<int> expected}) async {
424+
final users = [
425+
eg.user(userId: 0),
426+
eg.user(userId: 1),
427+
eg.user(userId: 2),
428+
eg.user(userId: 3),
429+
eg.user(userId: 4),
430+
];
431+
432+
final dmConversations = [
433+
RecentDmConversation(userIds: [3], maxMessageId: 300),
434+
RecentDmConversation(userIds: [0], maxMessageId: 200),
435+
RecentDmConversation(userIds: [0, 1], maxMessageId: 100),
436+
];
437+
438+
await prepare(users: users, dmConversations: dmConversations);
439+
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
440+
441+
bool done = false;
442+
view.addListener(() { done = true; });
443+
view.query = MentionAutocompleteQuery('');
444+
await Future(() {});
445+
check(done).isTrue();
446+
final results = view.results
447+
.map((e) => (e as UserMentionAutocompleteResult).userId)
448+
.toList();
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+
});
351477
}

0 commit comments

Comments
 (0)