Skip to content

Commit 4d5d823

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 15519d2 commit 4d5d823

File tree

2 files changed

+168
-8
lines changed

2 files changed

+168
-8
lines changed

lib/model/autocomplete.dart

+38-3
Original file line numberDiff line numberDiff line change
@@ -177,14 +177,49 @@ 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+
192+
final users = store.users.values.toList();
193+
return users..sort((userA, userB) => compareByRelevance(
194+
userA, userB, store: store));
195+
}
196+
197+
static int compareByRelevance(
198+
User userA,
199+
User userB, {
200+
required PerAccountStore store,
201+
}) {
202+
return compareByDms(userA, userB, store: store);
203+
}
204+
205+
/// Determines which of the two users is more recent in DM conversations.
206+
///
207+
/// Returns a negative number if [userA] is more recent than [userB],
208+
/// returns a positive number if [userB] is more recent than [userA],
209+
/// and returns `0` if both [userA] and [userB] are equally recent
210+
/// or there is no DM exchanged with them whatsoever.
211+
@visibleForTesting
212+
static int compareByDms(User userA, User userB, {required PerAccountStore store}) {
213+
final recentDms = store.recentDmConversationsView;
214+
final aLatestMessageId = recentDms.latestMessagesByRecipient[userA.userId];
215+
final bLatestMessageId = recentDms.latestMessagesByRecipient[userB.userId];
216+
217+
return switch((aLatestMessageId, bLatestMessageId)) {
218+
(int a, int b) => -a.compareTo(b),
219+
(int(), _) => -1,
220+
(_, int()) => 1,
221+
_ => 0,
222+
};
188223
}
189224

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

0 commit comments

Comments
 (0)