Skip to content

Commit e60629e

Browse files
committed
autocomplete: Add "alphabetical order" criterion
In @-mention autocomplete, users are suggested based on: 1. Recent activity in the current topic/stream. 2. Recent DM conversations. 3. Human vs. Bot users (human users come first). 4. Alphabetical order. Fixes: zulip#228
1 parent 2858e6b commit e60629e

File tree

2 files changed

+70
-8
lines changed

2 files changed

+70
-8
lines changed

lib/model/autocomplete.dart

+19-1
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,10 @@ class MentionAutocompleteView extends ChangeNotifier {
245245
final dmsResult = compareByDms(userA, userB, store: store);
246246
if (dmsResult != 0) return dmsResult;
247247

248-
return compareByBotStatus(userA, userB);
248+
final botStatusResult = compareByBotStatus(userA, userB);
249+
if (botStatusResult != 0) return botStatusResult;
250+
251+
return compareByAlphabeticalOrder(userA, userB, store: store);
249252
}
250253

251254
/// Determines which of the two users has more recent activity (messages sent
@@ -327,6 +330,21 @@ class MentionAutocompleteView extends ChangeNotifier {
327330
};
328331
}
329332

333+
/// Compares the two users by [User.fullName] case-insensitively.
334+
///
335+
/// Returns a negative number if `userA`'s `fullName` comes first alphabetically,
336+
/// returns a positive number if `userB`'s `fullName` comes first alphabetically,
337+
/// and returns `0` if both users have identical `fullName`.
338+
@visibleForTesting
339+
static int compareByAlphabeticalOrder(User userA, User userB,
340+
{required PerAccountStore store}) {
341+
final userAName = store.autocompleteViewManager.autocompleteDataCache
342+
.normalizedNameForUser(userA);
343+
final userBName = store.autocompleteViewManager.autocompleteDataCache
344+
.normalizedNameForUser(userB);
345+
return userAName.compareTo(userBName); // TODO(i18n): add locale-aware sorting
346+
}
347+
330348
@override
331349
void dispose() {
332350
store.autocompleteViewManager.unregisterMentionAutocomplete(this);

test/model/autocomplete_test.dart

+51-7
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,35 @@ void main() {
510510
});
511511
});
512512

513+
group('compareByAlphabeticalOrder', () {
514+
int compareAB(String aName, String bName) => MentionAutocompleteView.compareByAlphabeticalOrder(
515+
eg.user(fullName: aName), eg.user(fullName: bName), store: store);
516+
517+
test("userA's fullName comes first than userB's fullName -> favor userA", () async {
518+
await prepare();
519+
check(compareAB('alice', 'brian')).isLessThan(0);
520+
check(compareAB('alice', 'BRIAN')).isLessThan(0);
521+
// TODO(i18n): add locale-aware sorting
522+
// check(compareAB('čarolína', 'david')).isLessThan(0);
523+
});
524+
525+
test("userB's fullName comes first than userA's fullName -> favor userB", () async {
526+
await prepare();
527+
check(compareAB('brian', 'alice')).isGreaterThan(0);
528+
check(compareAB('BRIAN', 'alice')).isGreaterThan(0);
529+
// TODO(i18n): add locale-aware sorting
530+
// check(compareAB('david', 'čarolína')).isGreaterThan(0);
531+
});
532+
533+
test('both users have identical fullName -> favor none', () async {
534+
await prepare();
535+
check(compareAB('alice', 'alice')).equals(0);
536+
check(compareAB('BRIAN', 'brian')).equals(0);
537+
// TODO(i18n): add locale-aware sorting
538+
// check(compareAB('čarolína', 'carolina')).equals(0);
539+
});
540+
});
541+
513542
group('ranking across signals', () {
514543
void checkPrecedes(Narrow narrow, User userA, Iterable<User> usersB) {
515544
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
@@ -530,15 +559,17 @@ void main() {
530559
}
531560

532561
test('TopicNarrow: topic recency > stream recency > DM recency '
533-
'> human vs. bot user', () async {
562+
'> human vs. bot user > alphabetical order', () async {
534563
final users = [
535-
eg.user(),
564+
eg.user(fullName: 'Z'),
536565
eg.user(),
537566
eg.user(isBot: true),
538567
eg.user(),
539568
eg.user(),
540569
eg.user(),
541570
eg.user(isBot: true),
571+
eg.user(fullName: 'ab'),
572+
eg.user(fullName: 'bc'),
542573
];
543574
final stream = eg.stream();
544575
final narrow = TopicNarrow(stream.streamId, 'this');
@@ -555,16 +586,20 @@ void main() {
555586
checkPrecedes(narrow, users[2], users.skip(3));
556587
checkRankEqual(narrow, [users[3], users[4]]);
557588
checkPrecedes(narrow, users[5], users.skip(6));
589+
checkPrecedes(narrow, users[7], users.skip(8));
558590
});
559591

560-
test('ChannelNarrow: stream recency > DM recency > human vs. bot user', () async {
592+
test('ChannelNarrow: stream recency > DM recency > human vs. bot user '
593+
'> alphabetical order', () async {
561594
final users = [
562-
eg.user(isBot: true),
595+
eg.user(isBot: true, fullName: 'Z'),
563596
eg.user(),
564597
eg.user(),
565598
eg.user(),
566599
eg.user(),
567600
eg.user(isBot: true),
601+
eg.user(fullName: 'ab', isBot: true),
602+
eg.user(fullName: 'bc', isBot: true),
568603
];
569604
final stream = eg.stream();
570605
final narrow = ChannelNarrow(stream.streamId);
@@ -579,17 +614,20 @@ void main() {
579614
checkPrecedes(narrow, users[1], users.skip(2));
580615
checkRankEqual(narrow, [users[2], users[3]]);
581616
checkPrecedes(narrow, users[4], users.skip(5));
617+
checkPrecedes(narrow, users[6], users.skip(7));
582618
});
583619

584620
test('DmNarrow: DM recency > this-conversation recency or stream recency '
585-
'or human vs. bot user', () async {
621+
'or human vs. bot user or alphabetical order', () async {
586622
final users = [
587-
eg.user(isBot: true),
623+
eg.user(isBot: true, fullName: 'Z'),
588624
eg.user(),
589625
eg.user(),
590626
eg.user(),
591627
eg.user(),
592628
eg.user(isBot: true),
629+
eg.user(fullName: 'ab'),
630+
eg.user(fullName: 'bc'),
593631
];
594632
await prepare(users: users, messages: [
595633
eg.dmMessage(from: users[3], to: [eg.selfUser]),
@@ -610,6 +648,7 @@ void main() {
610648
checkPrecedes(narrow, users[1], users.skip(3));
611649
checkPrecedes(narrow, users[2], users.skip(3));
612650
checkPrecedes(narrow, users[4], users.skip(5));
651+
checkPrecedes(narrow, users[6], users.skip(7));
613652
}
614653
});
615654

@@ -655,6 +694,8 @@ void main() {
655694
eg.user(userId: 5, fullName: 'User Five'),
656695
eg.user(userId: 6, fullName: 'User Six', isBot: true),
657696
eg.user(userId: 7, fullName: 'User Seven'),
697+
eg.user(userId: 8, fullName: 'User Xy', isBot: true),
698+
eg.user(userId: 9, fullName: 'User Xz', isBot: true),
658699
];
659700

660701
await prepare(users: users, messages: [
@@ -671,8 +712,9 @@ void main() {
671712
// 1. Users most recent in the current topic/stream.
672713
// 2. Users most recent in the DM conversations.
673714
// 3. Human vs. Bot users (human users come first).
715+
// 4. Alphabetical order.
674716
check(await getResults(topicNarrow, MentionAutocompleteQuery('')))
675-
.deepEquals([1, 5, 4, 2, 3, 7, 6]);
717+
.deepEquals([1, 5, 4, 2, 7, 3, 6, 8, 9]);
676718

677719
// Check the ranking applies also to results filtered by a query.
678720
check(await getResults(topicNarrow, MentionAutocompleteQuery('t')))
@@ -681,6 +723,8 @@ void main() {
681723
.deepEquals([5, 4]);
682724
check(await getResults(topicNarrow, MentionAutocompleteQuery('s')))
683725
.deepEquals([7, 6]);
726+
check(await getResults(topicNarrow, MentionAutocompleteQuery('user x')))
727+
.deepEquals([8, 9]);
684728
});
685729
});
686730
}

0 commit comments

Comments
 (0)