From e44f9e859ed5e6f84a7ff91eb15d9aa21759fb22 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 14 May 2024 11:02:29 +0430 Subject: [PATCH 1/7] autocomplete: Add "human vs. bot users" 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). --- lib/model/autocomplete.dart | 23 +++++++++-- test/model/autocomplete_test.dart | 67 +++++++++++++++++++++++++++---- 2 files changed, 80 insertions(+), 10 deletions(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 3054fd5348..35e86c8530 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -236,13 +236,16 @@ class MentionAutocompleteView extends ChangeNotifier { required PerAccountStore store, }) { if (streamId != null) { - final result = compareByRecency(userA, userB, + final recencyResult = compareByRecency(userA, userB, streamId: streamId, topic: topic, store: store); - if (result != 0) return result; + if (recencyResult != 0) return recencyResult; } - return compareByDms(userA, userB, store: store); + final dmsResult = compareByDms(userA, userB, store: store); + if (dmsResult != 0) return dmsResult; + + return compareByBotStatus(userA, userB); } /// Determines which of the two users has more recent activity (messages sent @@ -310,6 +313,20 @@ class MentionAutocompleteView extends ChangeNotifier { }; } + /// Compares the bot status of two users and returns an integer indicating their order. + /// + /// Returns `-1` if `userA` is human and `userB` is bot, returns `1` if `userA` + /// is bot and `userB` is human, and returns `0` if both users have the same + /// bot status. + @visibleForTesting + static int compareByBotStatus(User userA, User userB) { + return switch ((userA.isBot, userB.isBot)) { + (false, true) => -1, + (true, false) => 1, + _ => 0, + }; + } + @override void dispose() { store.autocompleteViewManager.unregisterMentionAutocomplete(this); diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index 54ed729a64..15b20bc765 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -490,6 +490,26 @@ void main() { }); }); + group('compareByBotStatus', () { + final humanUser = eg.user(isBot: false); + final botUser = eg.user(isBot: true); + + int compareAB(User a, User b) => MentionAutocompleteView.compareByBotStatus(a, b); + + test('userA is human, userB is bot -> favor userA', () { + check(compareAB(humanUser, botUser)).isLessThan(0); + }); + + test('userA is bot, userB is human -> favor userB', () { + check(compareAB(botUser, humanUser)).isGreaterThan(0); + }); + + test('both users have the same bot status -> favor none', () { + check(compareAB(humanUser, humanUser)).equals(0); + check(compareAB(botUser, botUser)).equals(0); + }); + }); + group('ranking across signals', () { void checkPrecedes(Narrow narrow, User userA, Iterable usersB) { final view = MentionAutocompleteView.init(store: store, narrow: narrow); @@ -509,8 +529,17 @@ void main() { } } - test('TopicNarrow: topic recency > stream recency > DM recency', () async { - final users = List.generate(5, (i) => eg.user()); + test('TopicNarrow: topic recency > stream recency > DM recency ' + '> human vs. bot user', () async { + final users = [ + eg.user(), + eg.user(), + eg.user(isBot: true), + eg.user(), + eg.user(), + eg.user(), + eg.user(isBot: true), + ]; final stream = eg.stream(); final narrow = TopicNarrow(stream.streamId, 'this'); await prepare(users: users, messages: [ @@ -525,10 +554,18 @@ void main() { checkPrecedes(narrow, users[1], users.skip(2)); checkPrecedes(narrow, users[2], users.skip(3)); checkRankEqual(narrow, [users[3], users[4]]); + checkPrecedes(narrow, users[5], users.skip(6)); }); - test('ChannelNarrow: stream recency > DM recency', () async { - final users = List.generate(4, (i) => eg.user()); + test('ChannelNarrow: stream recency > DM recency > human vs. bot user', () async { + final users = [ + eg.user(isBot: true), + eg.user(), + eg.user(), + eg.user(), + eg.user(), + eg.user(isBot: true), + ]; final stream = eg.stream(); final narrow = ChannelNarrow(stream.streamId); await prepare(users: users, messages: [ @@ -536,14 +573,24 @@ void main() { eg.streamMessage(sender: users[0], stream: stream), eg.dmMessage(from: users[2], to: [users[3], eg.selfUser]), eg.dmMessage(from: users[1], to: [eg.selfUser]), + eg.dmMessage(from: users[4], to: [users[5], eg.selfUser]), ]); checkPrecedes(narrow, users[0], users.skip(1)); checkPrecedes(narrow, users[1], users.skip(2)); checkRankEqual(narrow, [users[2], users[3]]); + checkPrecedes(narrow, users[4], users.skip(5)); }); - test('DmNarrow: DM recency > this-conversation recency or stream recency', () async { - final users = List.generate(4, (i) => eg.user()); + test('DmNarrow: DM recency > this-conversation recency or stream recency ' + 'or human vs. bot user', () async { + final users = [ + eg.user(isBot: true), + eg.user(), + eg.user(), + eg.user(), + eg.user(), + eg.user(isBot: true), + ]; await prepare(users: users, messages: [ eg.dmMessage(from: users[3], to: [eg.selfUser]), eg.dmMessage(from: users[1], to: [users[2], eg.selfUser]), @@ -562,6 +609,7 @@ void main() { checkRankEqual(narrow, [users[1], users[2]]); checkPrecedes(narrow, users[1], users.skip(3)); checkPrecedes(narrow, users[2], users.skip(3)); + checkPrecedes(narrow, users[4], users.skip(5)); } }); @@ -605,6 +653,8 @@ void main() { eg.user(userId: 3, fullName: 'User Three'), eg.user(userId: 4, fullName: 'User Four'), eg.user(userId: 5, fullName: 'User Five'), + eg.user(userId: 6, fullName: 'User Six', isBot: true), + eg.user(userId: 7, fullName: 'User Seven'), ]; await prepare(users: users, messages: [ @@ -620,14 +670,17 @@ void main() { // The order should be: // 1. Users most recent in the current topic/stream. // 2. Users most recent in the DM conversations. + // 3. Human vs. Bot users (human users come first). check(await getResults(topicNarrow, MentionAutocompleteQuery(''))) - .deepEquals([1, 5, 4, 2, 3]); + .deepEquals([1, 5, 4, 2, 3, 7, 6]); // Check the ranking applies also to results filtered by a query. check(await getResults(topicNarrow, MentionAutocompleteQuery('t'))) .deepEquals([2, 3]); check(await getResults(topicNarrow, MentionAutocompleteQuery('f'))) .deepEquals([5, 4]); + check(await getResults(topicNarrow, MentionAutocompleteQuery('s'))) + .deepEquals([7, 6]); }); }); } From b9c048bd1a79286d650daee317717a23ef56fcd7 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 14 May 2024 11:07:04 +0430 Subject: [PATCH 2/7] autocomplete [nfc]: Support caching normalized user names in `AutocompleteDataCache` --- lib/model/autocomplete.dart | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 35e86c8530..527f8fd2b4 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -456,13 +456,21 @@ class MentionAutocompleteQuery { } class AutocompleteDataCache { + final Map _normalizedNamesByUser = {}; + + /// The lowercase `fullName` of [user]. + String normalizedNameForUser(User user) { + return _normalizedNamesByUser[user.userId] ??= user.fullName.toLowerCase(); + } + final Map> _nameWordsByUser = {}; List nameWordsForUser(User user) { - return _nameWordsByUser[user.userId] ??= user.fullName.toLowerCase().split(' '); + return _nameWordsByUser[user.userId] ??= normalizedNameForUser(user).split(' '); } void invalidateUser(int userId) { + _normalizedNamesByUser.remove(userId); _nameWordsByUser.remove(userId); } } From cef17695615508b26ea2834c83360f3c050bbe06 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 14 May 2024 11:09:00 +0430 Subject: [PATCH 3/7] 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: #228 --- lib/model/autocomplete.dart | 20 ++++++++++- test/model/autocomplete_test.dart | 58 +++++++++++++++++++++++++++---- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 527f8fd2b4..eacb22eef4 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -245,7 +245,10 @@ class MentionAutocompleteView extends ChangeNotifier { final dmsResult = compareByDms(userA, userB, store: store); if (dmsResult != 0) return dmsResult; - return compareByBotStatus(userA, userB); + final botStatusResult = compareByBotStatus(userA, userB); + if (botStatusResult != 0) return botStatusResult; + + return compareByAlphabeticalOrder(userA, userB, store: store); } /// Determines which of the two users has more recent activity (messages sent @@ -327,6 +330,21 @@ class MentionAutocompleteView extends ChangeNotifier { }; } + /// Compares the two users by [User.fullName] case-insensitively. + /// + /// Returns a negative number if `userA`'s `fullName` comes first alphabetically, + /// returns a positive number if `userB`'s `fullName` comes first alphabetically, + /// and returns `0` if both users have identical `fullName`. + @visibleForTesting + static int compareByAlphabeticalOrder(User userA, User userB, + {required PerAccountStore store}) { + final userAName = store.autocompleteViewManager.autocompleteDataCache + .normalizedNameForUser(userA); + final userBName = store.autocompleteViewManager.autocompleteDataCache + .normalizedNameForUser(userB); + return userAName.compareTo(userBName); // TODO(i18n): add locale-aware sorting + } + @override void dispose() { store.autocompleteViewManager.unregisterMentionAutocomplete(this); diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index 15b20bc765..0b484d38b3 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -510,6 +510,35 @@ void main() { }); }); + group('compareByAlphabeticalOrder', () { + int compareAB(String aName, String bName) => MentionAutocompleteView.compareByAlphabeticalOrder( + eg.user(fullName: aName), eg.user(fullName: bName), store: store); + + test("userA's fullName comes first than userB's fullName -> favor userA", () async { + await prepare(); + check(compareAB('alice', 'brian')).isLessThan(0); + check(compareAB('alice', 'BRIAN')).isLessThan(0); + // TODO(i18n): add locale-aware sorting + // check(compareAB('čarolína', 'david')).isLessThan(0); + }); + + test("userB's fullName comes first than userA's fullName -> favor userB", () async { + await prepare(); + check(compareAB('brian', 'alice')).isGreaterThan(0); + check(compareAB('BRIAN', 'alice')).isGreaterThan(0); + // TODO(i18n): add locale-aware sorting + // check(compareAB('david', 'čarolína')).isGreaterThan(0); + }); + + test('both users have identical fullName -> favor none', () async { + await prepare(); + check(compareAB('alice', 'alice')).equals(0); + check(compareAB('BRIAN', 'brian')).equals(0); + // TODO(i18n): add locale-aware sorting + // check(compareAB('čarolína', 'carolina')).equals(0); + }); + }); + group('ranking across signals', () { void checkPrecedes(Narrow narrow, User userA, Iterable usersB) { final view = MentionAutocompleteView.init(store: store, narrow: narrow); @@ -530,15 +559,17 @@ void main() { } test('TopicNarrow: topic recency > stream recency > DM recency ' - '> human vs. bot user', () async { + '> human vs. bot user > alphabetical order', () async { final users = [ - eg.user(), + eg.user(fullName: 'Z'), eg.user(), eg.user(isBot: true), eg.user(), eg.user(), eg.user(), eg.user(isBot: true), + eg.user(fullName: 'ab'), + eg.user(fullName: 'bc'), ]; final stream = eg.stream(); final narrow = TopicNarrow(stream.streamId, 'this'); @@ -555,16 +586,20 @@ void main() { checkPrecedes(narrow, users[2], users.skip(3)); checkRankEqual(narrow, [users[3], users[4]]); checkPrecedes(narrow, users[5], users.skip(6)); + checkPrecedes(narrow, users[7], users.skip(8)); }); - test('ChannelNarrow: stream recency > DM recency > human vs. bot user', () async { + test('ChannelNarrow: stream recency > DM recency > human vs. bot user ' + '> alphabetical order', () async { final users = [ - eg.user(isBot: true), + eg.user(isBot: true, fullName: 'Z'), eg.user(), eg.user(), eg.user(), eg.user(), eg.user(isBot: true), + eg.user(fullName: 'ab', isBot: true), + eg.user(fullName: 'bc', isBot: true), ]; final stream = eg.stream(); final narrow = ChannelNarrow(stream.streamId); @@ -579,17 +614,20 @@ void main() { checkPrecedes(narrow, users[1], users.skip(2)); checkRankEqual(narrow, [users[2], users[3]]); checkPrecedes(narrow, users[4], users.skip(5)); + checkPrecedes(narrow, users[6], users.skip(7)); }); test('DmNarrow: DM recency > this-conversation recency or stream recency ' - 'or human vs. bot user', () async { + 'or human vs. bot user or alphabetical order', () async { final users = [ - eg.user(isBot: true), + eg.user(isBot: true, fullName: 'Z'), eg.user(), eg.user(), eg.user(), eg.user(), eg.user(isBot: true), + eg.user(fullName: 'ab'), + eg.user(fullName: 'bc'), ]; await prepare(users: users, messages: [ eg.dmMessage(from: users[3], to: [eg.selfUser]), @@ -610,6 +648,7 @@ void main() { checkPrecedes(narrow, users[1], users.skip(3)); checkPrecedes(narrow, users[2], users.skip(3)); checkPrecedes(narrow, users[4], users.skip(5)); + checkPrecedes(narrow, users[6], users.skip(7)); } }); @@ -655,6 +694,8 @@ void main() { eg.user(userId: 5, fullName: 'User Five'), eg.user(userId: 6, fullName: 'User Six', isBot: true), eg.user(userId: 7, fullName: 'User Seven'), + eg.user(userId: 8, fullName: 'User Xy', isBot: true), + eg.user(userId: 9, fullName: 'User Xz', isBot: true), ]; await prepare(users: users, messages: [ @@ -671,8 +712,9 @@ void main() { // 1. Users most recent in the current topic/stream. // 2. Users most recent in the DM conversations. // 3. Human vs. Bot users (human users come first). + // 4. Alphabetical order. check(await getResults(topicNarrow, MentionAutocompleteQuery(''))) - .deepEquals([1, 5, 4, 2, 3, 7, 6]); + .deepEquals([1, 5, 4, 2, 7, 3, 6, 8, 9]); // Check the ranking applies also to results filtered by a query. check(await getResults(topicNarrow, MentionAutocompleteQuery('t'))) @@ -681,6 +723,8 @@ void main() { .deepEquals([5, 4]); check(await getResults(topicNarrow, MentionAutocompleteQuery('s'))) .deepEquals([7, 6]); + check(await getResults(topicNarrow, MentionAutocompleteQuery('user x'))) + .deepEquals([8, 9]); }); }); } From b36ba885f20fd37c32c5510c0995bcf591a5e258 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 14 May 2024 11:10:58 +0430 Subject: [PATCH 4/7] autocomplete [nfc]: Add TODO comments for future autocomplete criteria --- lib/model/autocomplete.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index eacb22eef4..4a99a7d196 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -235,6 +235,10 @@ class MentionAutocompleteView extends ChangeNotifier { required String? topic, required PerAccountStore store, }) { + // TODO(#234): give preference to "all", "everyone" or "stream" + + // TODO(#618): give preference to subscribed users first + if (streamId != null) { final recencyResult = compareByRecency(userA, userB, streamId: streamId, From b48e54b4f41b3ca706fc9afe3cbd735181848521 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 2 Aug 2024 15:58:16 -0700 Subject: [PATCH 5/7] autocomplete [nfc]: Simplify and clarify docs on comparator methods The details of negative vs. positive vs. zero return values are part of the interface of comparators in general. The details of 1 vs. any other positive value, or -1 vs. any other negative value, are implementation details of the methods -- callers should only care about negative vs. positive vs. zero. --- lib/model/autocomplete.dart | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 4a99a7d196..f87e7650b4 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -320,11 +320,7 @@ class MentionAutocompleteView extends ChangeNotifier { }; } - /// Compares the bot status of two users and returns an integer indicating their order. - /// - /// Returns `-1` if `userA` is human and `userB` is bot, returns `1` if `userA` - /// is bot and `userB` is human, and returns `0` if both users have the same - /// bot status. + /// Comparator that puts non-bots before bots. @visibleForTesting static int compareByBotStatus(User userA, User userB) { return switch ((userA.isBot, userB.isBot)) { @@ -334,11 +330,7 @@ class MentionAutocompleteView extends ChangeNotifier { }; } - /// Compares the two users by [User.fullName] case-insensitively. - /// - /// Returns a negative number if `userA`'s `fullName` comes first alphabetically, - /// returns a positive number if `userB`'s `fullName` comes first alphabetically, - /// and returns `0` if both users have identical `fullName`. + /// Comparator that orders alphabetically by [User.fullName]. @visibleForTesting static int compareByAlphabeticalOrder(User userA, User userB, {required PerAccountStore store}) { From 093eabae8d023409591517840f7c933ebf903fd4 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 8 Aug 2024 17:11:11 -0700 Subject: [PATCH 6/7] autocomplete test: Fix up cross-signal tests; add comments to clarify The way these tests were meant to work wasn't particularly explicit; and the recent changes adding more signals didn't take that intended structure into account, so that it became hard to see if the tests checked what they're intended to check. Bring them back to that structure, and add comments explaining it. --- test/model/autocomplete_test.dart | 99 +++++++++++++++++-------------- 1 file changed, 55 insertions(+), 44 deletions(-) diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index 0b484d38b3..53fa84c36c 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -558,18 +558,20 @@ void main() { } } - test('TopicNarrow: topic recency > stream recency > DM recency ' - '> human vs. bot user > alphabetical order', () async { + test('TopicNarrow: topic recency > stream recency > DM recency > human/bot > name', () async { + // The user with the greatest topic recency ranks last on each of the + // other criteria, but comes out first in the end, showing that + // topic recency comes first. Then among the remaining users, the one + // with the greatest stream recency ranks last on each of the remaining + // criteria, but comes out second in the end; and so on. final users = [ - eg.user(fullName: 'Z'), - eg.user(), - eg.user(isBot: true), - eg.user(), - eg.user(), - eg.user(), - eg.user(isBot: true), - eg.user(fullName: 'ab'), - eg.user(fullName: 'bc'), + eg.user(fullName: 'Z', isBot: true), // wins by topic recency + eg.user(fullName: 'Y', isBot: true), // runner-up, by stream recency + eg.user(fullName: 'X', isBot: true), // runner-up, by DM recency + eg.user(fullName: 'W', isBot: false), // runner-up, by human-vs-bot + eg.user(fullName: 'A', isBot: true), // runner-up, by name + eg.user(fullName: 'B', isBot: true), // tied because no remaining criteria + eg.user(fullName: 'b', isBot: true), ]; final stream = eg.stream(); final narrow = TopicNarrow(stream.streamId, 'this'); @@ -578,64 +580,71 @@ void main() { eg.streamMessage(sender: users[0], stream: stream, topic: 'this'), eg.streamMessage(sender: users[2], stream: stream, topic: 'other'), eg.streamMessage(sender: users[1], stream: stream, topic: 'other'), - eg.dmMessage(from: users[3], to: [users[4], eg.selfUser]), + eg.dmMessage(from: users[3], to: [...users.skip(4), eg.selfUser]), eg.dmMessage(from: users[2], to: [eg.selfUser]), ]); checkPrecedes(narrow, users[0], users.skip(1)); checkPrecedes(narrow, users[1], users.skip(2)); checkPrecedes(narrow, users[2], users.skip(3)); - checkRankEqual(narrow, [users[3], users[4]]); - checkPrecedes(narrow, users[5], users.skip(6)); - checkPrecedes(narrow, users[7], users.skip(8)); + checkPrecedes(narrow, users[3], users.skip(4)); + checkPrecedes(narrow, users[4], users.skip(5)); + checkRankEqual(narrow, [users[5], users[6]]); }); - test('ChannelNarrow: stream recency > DM recency > human vs. bot user ' - '> alphabetical order', () async { + test('ChannelNarrow: stream recency > DM recency > human/bot > name', () async { + // Same principle as for TopicNarrow; see that test case above. final users = [ - eg.user(isBot: true, fullName: 'Z'), - eg.user(), - eg.user(), - eg.user(), - eg.user(), - eg.user(isBot: true), - eg.user(fullName: 'ab', isBot: true), - eg.user(fullName: 'bc', isBot: true), + eg.user(fullName: 'Z', isBot: true), // wins by stream recency + eg.user(fullName: 'Y', isBot: true), // runner-up, by DM recency + eg.user(fullName: 'X', isBot: false), // runner-up, by human-vs-bot + eg.user(fullName: 'A', isBot: true), // runner-up, by name + eg.user(fullName: 'B', isBot: true), // tied because no remaining criteria + eg.user(fullName: 'b', isBot: true), ]; final stream = eg.stream(); final narrow = ChannelNarrow(stream.streamId); await prepare(users: users, messages: [ eg.streamMessage(sender: users[1], stream: stream), eg.streamMessage(sender: users[0], stream: stream), - eg.dmMessage(from: users[2], to: [users[3], eg.selfUser]), + eg.dmMessage(from: users[2], to: [...users.skip(3), eg.selfUser]), eg.dmMessage(from: users[1], to: [eg.selfUser]), - eg.dmMessage(from: users[4], to: [users[5], eg.selfUser]), ]); checkPrecedes(narrow, users[0], users.skip(1)); checkPrecedes(narrow, users[1], users.skip(2)); - checkRankEqual(narrow, [users[2], users[3]]); - checkPrecedes(narrow, users[4], users.skip(5)); - checkPrecedes(narrow, users[6], users.skip(7)); + checkPrecedes(narrow, users[2], users.skip(3)); + checkPrecedes(narrow, users[3], users.skip(4)); + checkRankEqual(narrow, [users[4], users[5]]); }); - test('DmNarrow: DM recency > this-conversation recency or stream recency ' - 'or human vs. bot user or alphabetical order', () async { + test('DmNarrow: DM recency > human/bot > name, ignore this-conversation recency and stream recency', () async { + // Same principle as for TopicNarrow; see that test case above. final users = [ - eg.user(isBot: true, fullName: 'Z'), - eg.user(), - eg.user(), - eg.user(), - eg.user(), - eg.user(isBot: true), - eg.user(fullName: 'ab'), - eg.user(fullName: 'bc'), + // First user wins by DM recency. + eg.user(fullName: 'Z', isBot: true), + // Next two are runners-up by DM recency, and have a two-way tie + // despite different this-conversation recency (because that doesn't count). + eg.user(fullName: 'Y', isBot: true), + eg.user(fullName: 'y', isBot: true), + // Next user is the runner-up due to DM recency, and comes after the + // above users even when it has greater this-conversation recency + // (because that doesn't count). + eg.user(fullName: 'X', isBot: true), + // Remainder have no DM recency and so come later. + // Next user is the runner-up due to human-vs-bot. + eg.user(fullName: 'W', isBot: false), + // Next user is the runner-up due to name. + eg.user(fullName: 'A', isBot: true), + // Remaining users are tied, even though they differ in stream recency + // (because that doesn't count). + eg.user(fullName: 'B', isBot: true), + eg.user(fullName: 'b', isBot: true), ]; await prepare(users: users, messages: [ eg.dmMessage(from: users[3], to: [eg.selfUser]), eg.dmMessage(from: users[1], to: [users[2], eg.selfUser]), eg.dmMessage(from: users[0], to: [eg.selfUser]), - eg.streamMessage(sender: users[1]), - eg.streamMessage(sender: users[2]), - eg.streamMessage(sender: users[3]), + for (final user in users.skip(1)) + eg.streamMessage(sender: user), ]); for (final narrow in [ DmNarrow.withUser(users[3].userId, selfUserId: eg.selfUser.userId), @@ -647,8 +656,10 @@ void main() { checkRankEqual(narrow, [users[1], users[2]]); checkPrecedes(narrow, users[1], users.skip(3)); checkPrecedes(narrow, users[2], users.skip(3)); + checkPrecedes(narrow, users[3], users.skip(4)); checkPrecedes(narrow, users[4], users.skip(5)); - checkPrecedes(narrow, users[6], users.skip(7)); + checkPrecedes(narrow, users[5], users.skip(6)); + checkRankEqual(narrow, [users[6], users[7]]); } }); From e51d2f2d2c8c1803b858277454aa5e510ef251d2 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 8 Aug 2024 17:17:14 -0700 Subject: [PATCH 7/7] autocomplete test: Tighten up "final results" test The recent changes that added two new ranking criteria added four new users to this test case. Two is enough to make the test's point, because it's enough for adding users that differ on the new criteria while tying on the criteria that have higher priority in the ranking. --- test/model/autocomplete_test.dart | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index 53fa84c36c..f3c8a30c13 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -705,8 +705,6 @@ void main() { eg.user(userId: 5, fullName: 'User Five'), eg.user(userId: 6, fullName: 'User Six', isBot: true), eg.user(userId: 7, fullName: 'User Seven'), - eg.user(userId: 8, fullName: 'User Xy', isBot: true), - eg.user(userId: 9, fullName: 'User Xz', isBot: true), ]; await prepare(users: users, messages: [ @@ -723,19 +721,15 @@ void main() { // 1. Users most recent in the current topic/stream. // 2. Users most recent in the DM conversations. // 3. Human vs. Bot users (human users come first). - // 4. Alphabetical order. + // 4. Alphabetical order by name. check(await getResults(topicNarrow, MentionAutocompleteQuery(''))) - .deepEquals([1, 5, 4, 2, 7, 3, 6, 8, 9]); + .deepEquals([1, 5, 4, 2, 7, 3, 6]); // Check the ranking applies also to results filtered by a query. check(await getResults(topicNarrow, MentionAutocompleteQuery('t'))) .deepEquals([2, 3]); check(await getResults(topicNarrow, MentionAutocompleteQuery('f'))) .deepEquals([5, 4]); - check(await getResults(topicNarrow, MentionAutocompleteQuery('s'))) - .deepEquals([7, 6]); - check(await getResults(topicNarrow, MentionAutocompleteQuery('user x'))) - .deepEquals([8, 9]); }); }); }