diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index fe344189de..0b17f50b27 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -8,11 +8,11 @@ import '../api/model/model.dart'; import '../api/route/channels.dart'; import '../generated/l10n/zulip_localizations.dart'; import '../widgets/compose_box.dart'; +import 'algorithms.dart'; import 'compose.dart'; import 'emoji.dart'; import 'narrow.dart'; import 'store.dart'; -import 'user.dart'; extension ComposeContentAutocomplete on ComposeContentController { AutocompleteIntent? autocompleteIntent() { @@ -249,6 +249,14 @@ class AutocompleteViewManager { autocompleteDataCache.invalidateUser(event.userId); } + void handleUserGroupRemoveEvent(UserGroupRemoveEvent event) { + autocompleteDataCache.invalidateUserGroup(event.groupId); + } + + void handleUserGroupUpdateEvent(UserGroupUpdateEvent event) { + autocompleteDataCache.invalidateUserGroup(event.groupId); + } + /// Called when the app is reassembled during debugging, e.g. for hot reload. /// /// Calls [AutocompleteView.reassemble] for all that are registered. @@ -423,6 +431,7 @@ class MentionAutocompleteView extends AutocompleteView sortedUsers; + final List sortedUserGroups; final ZulipLocalizations localizations; static List _usersByRelevance({ @@ -455,18 +466,23 @@ class MentionAutocompleteView extends AutocompleteView _userGroupsByRelevance({required PerAccountStore store}) { + return store.activeGroups + // TODO(#1776) Follow new "Who can mention this group" setting instead + .where((userGroup) => !userGroup.isSystemGroup) + .toList() + ..sort(_userGroupComparator(store: store)); + } + + static int Function(UserGroup, UserGroup) _userGroupComparator({ + required PerAccountStore store, + }) { + // See also [MentionAutocompleteQuery._rankUserGroupResult]; + // that ranking takes precedence over this. + + return (userGroupA, userGroupB) => + compareGroupsByAlphabeticalOrder(userGroupA, userGroupB, store: store); + } + + static int compareGroupsByAlphabeticalOrder(UserGroup userGroupA, UserGroup userGroupB, + {required PerAccountStore store}) { + final groupAName = store.autocompleteViewManager.autocompleteDataCache + .lowercaseNameForUserGroup(userGroupA); + final groupBName = store.autocompleteViewManager.autocompleteDataCache + .lowercaseNameForUserGroup(userGroupB); + return groupAName.compareTo(groupBName); // TODO(i18n): add locale-aware sorting + } + void computeWildcardMentionResults({ required List results, required bool isComposingChannelMessage, @@ -610,11 +656,10 @@ class MentionAutocompleteView extends AutocompleteView?> computeResults() async { - final results = []; + final unsorted = []; // Give priority to wildcard mentions. - computeWildcardMentionResults(results: results, + computeWildcardMentionResults(results: unsorted, isComposingChannelMessage: narrow is ChannelNarrow || narrow is TopicNarrow); if (await filterCandidates(filter: _testUser, - candidates: sortedUsers, results: results)) { + candidates: sortedUsers, results: unsorted)) { return null; } - return results; + + if (await filterCandidates(filter: _testUserGroup, + candidates: sortedUserGroups, results: unsorted)) { + return null; + } + + return bucketSort(unsorted, + (r) => r.rank, numBuckets: MentionAutocompleteQuery._numResultRanks); } MentionAutocompleteResult? _testUser(MentionAutocompleteQuery query, User user) { - if (query.testUser(user, store.autocompleteViewManager.autocompleteDataCache, store)) { - return UserMentionAutocompleteResult(userId: user.userId); - } - return null; + return query.testUser(user, store); + } + + MentionAutocompleteResult? _testUserGroup(MentionAutocompleteQuery query, UserGroup userGroup) { + return query.testUserGroup(userGroup, store); } @override @@ -695,11 +748,15 @@ abstract class AutocompleteQuery { late final List _lowercaseWords; - /// Whether all of this query's words have matches in [words] that appear in order. + /// Whether all of this query's words have matches in [words], + /// modulo case, that appear in order. /// /// A "match" means the word in [words] starts with the query word. + /// + /// [words] must all be lowercased. bool _testContainsQueryWords(List words) { - // TODO(#237) test with diacritics stripped, where appropriate + // TODO(#237) test with diacritics stripped, where appropriate, + // and update dartdoc's summary line and its restriction about [words]. int wordsIndex = 0; int queryWordsIndex = 0; while (true) { @@ -718,6 +775,24 @@ abstract class AutocompleteQuery { } } +/// The match quality of a [User.fullName] or [UserGroup.name] +/// to a mention autocomplete query. +/// +/// All matches are case-insensitive. +enum NameMatchQuality { + /// The query matches the whole name exactly. + exact, + + /// The name starts with the query. + totalPrefix, + + /// All of the query's words have matches in the words of the name + /// that appear in order. + /// + /// A "match" means the word in the name starts with the query word. + wordPrefixes, +} + /// Any autocomplete query in the compose box's content input. abstract class ComposeAutocompleteQuery extends AutocompleteQuery { ComposeAutocompleteQuery(super.raw); @@ -748,25 +823,122 @@ class MentionAutocompleteQuery extends ComposeAutocompleteQuery { store: store, localizations: localizations, narrow: narrow, query: this); } - bool testWildcardOption(WildcardMentionOption wildcardOption, { + WildcardMentionAutocompleteResult? testWildcardOption(WildcardMentionOption wildcardOption, { required ZulipLocalizations localizations}) { // TODO(#237): match insensitively to diacritics - return wildcardOption.canonicalString.contains(_lowercase) + final matches = wildcardOption.canonicalString.contains(_lowercase) || wildcardOption.localizedCanonicalString(localizations).contains(_lowercase); + if (!matches) return null; + return WildcardMentionAutocompleteResult( + wildcardOption: wildcardOption, rank: _rankWildcardResult); } - bool testUser(User user, AutocompleteDataCache cache, UserStore store) { - if (!user.isActive) return false; - if (store.isUserMuted(user.userId)) return false; + MentionAutocompleteResult? testUser(User user, PerAccountStore store) { + if (!user.isActive) return null; + if (store.isUserMuted(user.userId)) return null; + + final cache = store.autocompleteViewManager.autocompleteDataCache; + final nameMatchQuality = _matchName( + normalizedName: cache.normalizedNameForUser(user), + normalizedNameWords: cache.normalizedNameWordsForUser(user)); + bool? matchesEmail; + if (nameMatchQuality == null) { + matchesEmail = _matchEmail(user, cache); + if (!matchesEmail) return null; + } - // TODO(#236) test email too, not just name - return _testName(user, cache); + return UserMentionAutocompleteResult( + userId: user.userId, + rank: _rankUserResult(user, + nameMatchQuality: nameMatchQuality, matchesEmail: matchesEmail)); } - bool _testName(User user, AutocompleteDataCache cache) { - return _testContainsQueryWords(cache.nameWordsForUser(user)); + NameMatchQuality? _matchName({ + required String normalizedName, + required List normalizedNameWords, + }) { + if (normalizedName.startsWith(_lowercase)) { + if (normalizedName.length == _lowercase.length) { + return NameMatchQuality.exact; + } else { + return NameMatchQuality.totalPrefix; + } + } + + if (_testContainsQueryWords(normalizedNameWords)) { + return NameMatchQuality.wordPrefixes; + } + + return null; } + bool _matchEmail(User user, AutocompleteDataCache cache) { + final lowercaseEmail = cache.normalizedEmailForUser(user); + if (lowercaseEmail == null) return false; // Email not known + return lowercaseEmail.startsWith(_lowercase); + } + + MentionAutocompleteResult? testUserGroup(UserGroup userGroup, PerAccountStore store) { + final cache = store.autocompleteViewManager.autocompleteDataCache; + + final nameMatchQuality = _matchName( + normalizedName: cache.lowercaseNameForUserGroup(userGroup), + normalizedNameWords: cache.lowercaseNameWordsForUserGroup(userGroup)); + + if (nameMatchQuality == null) return null; + + return UserGroupMentionAutocompleteResult( + groupId: userGroup.id, + rank: _rankUserGroupResult(userGroup, nameMatchQuality: nameMatchQuality)); + } + + /// A measure of a wildcard result's quality in the context of the query, + /// from 0 (best) to one less than [_numResultRanks]. + /// + /// See also [_rankUserResult] and [_rankUserGroupResult]. + static const _rankWildcardResult = 0; + + /// A measure of a user result's quality in the context of the query, + /// from 0 (best) to one less than [_numResultRanks]. + /// + /// When [nameMatchQuality] is non-null (the name matches), + /// callers should skip computing [matchesEmail] and pass null for that. + /// + /// See also [_rankWildcardResult] and [_rankUserGroupResult]. + static int _rankUserResult(User user, { + required NameMatchQuality? nameMatchQuality, + required bool? matchesEmail, + }) { + if (nameMatchQuality != null) { + assert(matchesEmail == null); + return switch (nameMatchQuality) { + NameMatchQuality.exact => 1, + NameMatchQuality.totalPrefix => 2, + NameMatchQuality.wordPrefixes => 3, + }; + } + assert(matchesEmail == true); + return 7; + } + + /// A measure of a user-group result's quality in the context of the query, + /// from 0 (best) to one less than [_numResultRanks]. + /// + /// See also [_rankWildcardResult] and [_rankUserResult]. + static int _rankUserGroupResult(UserGroup userGroup, { + required NameMatchQuality nameMatchQuality, + }) { + return switch (nameMatchQuality) { + NameMatchQuality.exact => 4, + NameMatchQuality.totalPrefix => 5, + NameMatchQuality.wordPrefixes => 6, + }; + } + + /// The number of possible values returned by + /// [_rankWildcardResult], [_rankUserResult], and [_rankUserGroupResult].. + static const _numResultRanks = 8; + @override String toString() { return '${objectRuntimeType(this, 'MentionAutocompleteQuery')}(raw: $raw, silent: $silent})'; @@ -801,20 +973,48 @@ extension WildcardMentionOptionExtension on WildcardMentionOption { class AutocompleteDataCache { final Map _normalizedNamesByUser = {}; - /// The lowercase `fullName` of [user]. + /// The normalized `fullName` of [user]. String normalizedNameForUser(User user) { return _normalizedNamesByUser[user.userId] ??= user.fullName.toLowerCase(); } - final Map> _nameWordsByUser = {}; + final Map> _normalizedNameWordsByUser = {}; + + List normalizedNameWordsForUser(User user) { + return _normalizedNameWordsByUser[user.userId] + ??= normalizedNameForUser(user).split(' '); + } + + final Map _normalizedEmailsByUser = {}; + + /// The normalized `deliveryEmail` of [user], or null if that's null. + String? normalizedEmailForUser(User user) { + return _normalizedEmailsByUser[user.userId] ??= user.deliveryEmail?.toLowerCase(); + } + + final Map _lowercaseNamesByUserGroup = {}; + + /// The normalized `name` of [userGroup]. + String lowercaseNameForUserGroup(UserGroup userGroup) { + return _lowercaseNamesByUserGroup[userGroup.id] ??= userGroup.name.toLowerCase(); + } + + final Map> _lowercaseNameWordsByUserGroup = {}; - List nameWordsForUser(User user) { - return _nameWordsByUser[user.userId] ??= normalizedNameForUser(user).split(' '); + List lowercaseNameWordsForUserGroup(UserGroup userGroup) { + return _lowercaseNameWordsByUserGroup[userGroup.id] + ??= lowercaseNameForUserGroup(userGroup).split(' '); } void invalidateUser(int userId) { _normalizedNamesByUser.remove(userId); - _nameWordsByUser.remove(userId); + _normalizedNameWordsByUser.remove(userId); + _normalizedEmailsByUser.remove(userId); + } + + void invalidateUserGroup(int id) { + _lowercaseNamesByUserGroup.remove(id); + _lowercaseNameWordsByUserGroup.remove(id); } } @@ -853,23 +1053,72 @@ class EmojiAutocompleteResult extends ComposeAutocompleteResult { /// This is abstract because there are several kinds of result /// that can all be offered in the same @-mention autocomplete interaction: /// a user, a wildcard, or a user group. -sealed class MentionAutocompleteResult extends ComposeAutocompleteResult {} +sealed class MentionAutocompleteResult extends ComposeAutocompleteResult { + /// A measure of the result's quality in the context of the query. + /// + /// Used internally by [MentionAutocompleteView] for ranking the results. + // See also [MentionAutocompleteView._usersByRelevance]; + // results with equal [rank] will appear in the order they were put in + // by that method. + // + // Compare sort_recipients in Zulip web: + // https://github.com/zulip/zulip/blob/afdf20c67/web/src/typeahead_helper.ts#L472 + // + // Behavior we have that web doesn't and might like to follow: + // - A "word-prefixes" match quality on user and user-group names: + // see [NameMatchQuality.wordPrefixes], which we rank on. + // + // Behavior web has that seems undesired, which we don't plan to follow: + // - Ranking humans above bots, even when the bots have higher relevance + // and better match quality. If there's a bot participating in the + // current conversation and I start typing its name, why wouldn't we want + // that as a top result? Issue: https://github.com/zulip/zulip/issues/35467 + // - A "word-boundary" match quality on user and user-group names: + // special rank when the whole query appears contiguously + // right after a word-boundary character. + // Our [NameMatchQuality.wordPrefixes] seems smarter. + // - An "exact" match quality on emails: probably not worth its complexity. + // Emails are much more uniform in their endings than users' names are, + // so a prefix match should be adequate. (If I've typed "email@example.co", + // that'll probably be the only result. There might be an "email@example.com", + // and an "exact" match would downrank that, but still that's just two items + // to scan through.) + // - A "word-boundary" match quality on user emails: + // "words" is a wrong abstraction when matching on emails. + // - Ranking some case-sensitive matches differently from case-insensitive + // matches. Users will expect a lowercase query to be adequate. + int get rank; +} /// An autocomplete result for an @-mention of an individual user. class UserMentionAutocompleteResult extends MentionAutocompleteResult { - UserMentionAutocompleteResult({required this.userId}); + UserMentionAutocompleteResult({required this.userId, required this.rank}); final int userId; + + @override + final int rank; } /// An autocomplete result for an @-mention of all the users in a conversation. class WildcardMentionAutocompleteResult extends MentionAutocompleteResult { - WildcardMentionAutocompleteResult({required this.wildcardOption}); + WildcardMentionAutocompleteResult({required this.wildcardOption, required this.rank}); final WildcardMentionOption wildcardOption; + + @override + final int rank; } -// TODO(#233): // class UserGroupMentionAutocompleteResult extends MentionAutocompleteResult { +/// An autocomplete result for an @-mention of a user group. +class UserGroupMentionAutocompleteResult extends MentionAutocompleteResult { + UserGroupMentionAutocompleteResult({required this.groupId, required this.rank}); + + final int groupId; + + @override + final int rank; +} /// An autocomplete interaction for choosing a topic for a message. class TopicAutocompleteView extends AutocompleteView { diff --git a/lib/model/compose.dart b/lib/model/compose.dart index 09b02cb08c..3a7a75976f 100644 --- a/lib/model/compose.dart +++ b/lib/model/compose.dart @@ -181,6 +181,10 @@ String wildcardMention(WildcardMentionOption wildcardOption, { return '@**$name**'; } +/// An @-mention of a user group, like @*mobile*. +String userGroupMention(String userGroupName, {bool silent = false}) => + '@${silent ? '_' : ''}*$userGroupName*'; + /// https://spec.commonmark.org/0.30/#inline-link /// /// The "link text" is made by enclosing [visibleText] in square brackets. diff --git a/lib/widgets/autocomplete.dart b/lib/widgets/autocomplete.dart index 350eb86d56..5c853d2da0 100644 --- a/lib/widgets/autocomplete.dart +++ b/lib/widgets/autocomplete.dart @@ -202,12 +202,30 @@ class ComposeAutocomplete extends AutocompleteField { @@ -20,6 +21,14 @@ extension UserMentionAutocompleteResultChecks on Subject get userId => has((r) => r.userId, 'userId'); } +extension WildcardMentionAutocompleteResultChecks on Subject { + Subject get wildcardOption => has((x) => x.wildcardOption, 'wildcardOption'); +} + +extension UserGroupMentionAutocompleteResultChecks on Subject { + Subject get groupId => has((r) => r.groupId, 'groupId'); +} + extension TopicAutocompleteResultChecks on Subject { Subject get topic => has((r) => r.topic, 'topic'); } diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index b95bb58ccb..92e5a81d9a 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -320,21 +320,39 @@ void main() { for (int i = 1; i <= 2500; i++) { await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i')); } + for (int i = 1; i <= 2500; i++) { + await store.addUserGroup(eg.userGroup(id: i, name: 'User Group $i')); + } bool done = false; final view = MentionAutocompleteView.init(store: store, localizations: zulipLocalizations, narrow: narrow, query: MentionAutocompleteQuery('User 2222')); view.addListener(() { done = true; }); + // three batches for users + await Future(() {}); + check(done).isFalse(); + await Future(() {}); + check(done).isFalse(); + await Future(() {}); + check(done).isFalse(); + + // three batches for user groups await Future(() {}); check(done).isFalse(); await Future(() {}); check(done).isFalse(); await Future(() {}); check(done).isTrue(); - check(view.results).single - .isA() - .userId.equals(2222); + + check(view.results).deepEquals(>[ + (it) => it + .isA() + .userId.equals(2222), + (it) => it + .isA() + .groupId.equals(2222), + ]); }); test('MentionAutocompleteView new query during computation replaces old', () async { @@ -343,6 +361,9 @@ void main() { for (int i = 1; i <= 1500; i++) { await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i')); } + for (int i = 1; i <= 1500; i++) { + await store.addUserGroup(eg.userGroup(id: i, name: 'User Group $i')); + } bool done = false; final view = MentionAutocompleteView.init(store: store, localizations: zulipLocalizations, @@ -353,21 +374,33 @@ void main() { check(done).isFalse(); view.query = MentionAutocompleteQuery('User 234'); - // …new query goes through all batches + // …new query goes through all user batches + await Future(() {}); + check(done).isFalse(); + await Future(() {}); + check(done).isFalse(); + // …and all user-group batches await Future(() {}); check(done).isFalse(); await Future(() {}); check(done).isTrue(); // new result is set - check(view.results).single - .isA() - .userId.equals(234); + + void checkResult() { + check(view.results).deepEquals(>[ + (it) => it + .isA() + .userId.equals(234), + (it) => it + .isA() + .groupId.equals(234), + ]); + } + checkResult(); // new result sticks; it isn't clobbered with old query's result for (int i = 0; i < 10; i++) { // for good measure await Future(() {}); - check(view.results).single - .isA() - .userId.equals(234); + checkResult(); } }); @@ -403,76 +436,19 @@ void main() { ..not((results) => results.contains(11000)); }); - group('MentionAutocompleteQuery.testUser', () { - late PerAccountStore store; - - void doCheck(String rawQuery, User user, bool expected) { - final result = MentionAutocompleteQuery(rawQuery) - .testUser(user, AutocompleteDataCache(), store); - expected ? check(result).isTrue() : check(result).isFalse(); - } - - test('user is always excluded when not active regardless of other criteria', () { - store = eg.store(); - - doCheck('Full Name', eg.user(fullName: 'Full Name', isActive: false), false); - // When active then other criteria will be checked - doCheck('Full Name', eg.user(fullName: 'Full Name', isActive: true), true); - }); - - test('user is always excluded when muted, regardless of other criteria', () async { - store = eg.store(); - await store.setMutedUsers([1]); - doCheck('Full Name', eg.user(userId: 1, fullName: 'Full Name'), false); - // When not muted, then other criteria will be checked - doCheck('Full Name', eg.user(userId: 2, fullName: 'Full Name'), true); - }); - - test('user is included if fullname words match the query', () { - store = eg.store(); - - doCheck('', eg.user(fullName: 'Full Name'), true); - doCheck('', eg.user(fullName: ''), true); // Unlikely case, but should not crash - doCheck('Full Name', eg.user(fullName: 'Full Name'), true); - doCheck('full name', eg.user(fullName: 'Full Name'), true); - doCheck('Full Name', eg.user(fullName: 'full name'), true); - doCheck('Full', eg.user(fullName: 'Full Name'), true); - doCheck('Name', eg.user(fullName: 'Full Name'), true); - doCheck('Full Name', eg.user(fullName: 'Fully Named'), true); - doCheck('Full Four', eg.user(fullName: 'Full Name Four Words'), true); - doCheck('Name Words', eg.user(fullName: 'Full Name Four Words'), true); - doCheck('Full F', eg.user(fullName: 'Full Name Four Words'), true); - doCheck('F Four', eg.user(fullName: 'Full Name Four Words'), true); - doCheck('full full', eg.user(fullName: 'Full Full Name'), true); - doCheck('full full', eg.user(fullName: 'Full Name Full'), true); - - doCheck('F', eg.user(fullName: ''), false); // Unlikely case, but should not crash - doCheck('Fully Named', eg.user(fullName: 'Full Name'), false); - doCheck('Full Name', eg.user(fullName: 'Full'), false); - doCheck('Full Name', eg.user(fullName: 'Name'), false); - doCheck('ull ame', eg.user(fullName: 'Full Name'), false); - doCheck('ull Name', eg.user(fullName: 'Full Name'), false); - doCheck('Full ame', eg.user(fullName: 'Full Name'), false); - doCheck('Full Full', eg.user(fullName: 'Full Name'), false); - doCheck('Name Name', eg.user(fullName: 'Full Name'), false); - doCheck('Name Full', eg.user(fullName: 'Full Name'), false); - doCheck('Name Four Full Words', eg.user(fullName: 'Full Name Four Words'), false); - doCheck('F Full', eg.user(fullName: 'Full Name Four Words'), false); - doCheck('Four F', eg.user(fullName: 'Full Name Four Words'), false); - }); - }); - - group('MentionAutocompleteView sorting users results', () { + group('MentionAutocompleteView sorting results', () { late PerAccountStore store; Future prepare({ List users = const [], + List userGroups = const [], List dmConversations = const [], List messages = const [], }) async { store = eg.store(initialSnapshot: eg.initialSnapshot( recentPrivateConversations: dmConversations)); await store.addUsers(users); + await store.addUserGroups(userGroups); await store.addMessages(messages); } @@ -805,18 +781,28 @@ void main() { final view = MentionAutocompleteView.init(store: store, localizations: zulipLocalizations, narrow: narrow, query: query); view.addListener(() { done = true; }); - await Future(() {}); + await Future(() {}); // users + await Future(() {}); // groups check(done).isTrue(); final results = view.results; view.dispose(); return results; } - Iterable getUsersFromResults(Iterable results) - => results.map((e) => (e as UserMentionAutocompleteResult).userId); + Condition isUser(int userId) { + return (it) => it.isA() + .userId.equals(userId); + } + + Condition isUserGroup(int id) { + return (it) => it.isA() + .groupId.equals(id); + } - Iterable getWildcardOptionsFromResults(Iterable results) - => results.map((e) => (e as WildcardMentionAutocompleteResult).wildcardOption); + Condition isWildcard(WildcardMentionOption option) { + return (it) => it.isA() + .wildcardOption.equals(option); + } final stream = eg.stream(); const topic = 'topic'; @@ -832,36 +818,48 @@ void main() { eg.user(userId: 7, fullName: 'User Seven'), ]; - await prepare(users: users, messages: [ - eg.streamMessage(id: 50, sender: users[1-1], stream: stream, topic: topic), - eg.streamMessage(id: 60, sender: users[5-1], stream: stream, topic: 'other $topic'), - ], dmConversations: [ - RecentDmConversation(userIds: [4], maxMessageId: 300), - RecentDmConversation(userIds: [1], maxMessageId: 200), - RecentDmConversation(userIds: [1, 2], maxMessageId: 100), + final userGroups = [ + eg.userGroup(id: 1, name: 'User Group One'), + eg.userGroup(id: 2, name: 'User Group Two'), + eg.userGroup(id: 3, name: 'User Group Three'), + eg.userGroup(id: 4, name: 'User Group Four'), + ]; + + await prepare(users: users, userGroups: userGroups, messages: [ + eg.streamMessage(sender: users[1-1], stream: stream, topic: topic), + eg.streamMessage(sender: users[5-1], stream: stream, topic: 'other $topic'), + eg.dmMessage(from: users[1-1], to: [users[2-1], eg.selfUser]), + eg.dmMessage(from: users[1-1], to: [eg.selfUser]), + eg.dmMessage(from: users[4-1], to: [eg.selfUser]), ]); - // Check the ranking of the full list of mentions. + // Check the ranking of the full list of mentions, + // i.e. the results for an empty query. // The order should be: - // 1. Wildcards before individual users. + // 1. Wildcards before individual users; user groups (alphabetically) after. // 2. Users most recent in the current topic/stream. // 3. Users most recent in the DM conversations. // 4. Human vs. Bot users (human users come first). // 5. Users by name alphabetical order. - final results1 = await getResults(topicNarrow, MentionAutocompleteQuery('')); - check(getWildcardOptionsFromResults(results1.take(2))) - .deepEquals([WildcardMentionOption.all, WildcardMentionOption.topic]); - check(getUsersFromResults(results1.skip(2))) - .deepEquals([1, 5, 4, 2, 7, 3, 6]); + // 6. User groups by name alphabetical order. + check(await getResults(topicNarrow, MentionAutocompleteQuery(''))).deepEquals([ + isWildcard(WildcardMentionOption.all), + isWildcard(WildcardMentionOption.topic), + ...[1, 5, 4, 2, 7, 3, 6].map(isUser), + ...[4, 1, 3, 2].map(isUserGroup), + ]); // Check the ranking applies also to results filtered by a query. - final results2 = await getResults(topicNarrow, MentionAutocompleteQuery('t')); - check(getWildcardOptionsFromResults(results2.take(2))) - .deepEquals([WildcardMentionOption.stream, WildcardMentionOption.topic]); - check(getUsersFromResults(results2.skip(2))).deepEquals([2, 3]); - final results3 = await getResults(topicNarrow, MentionAutocompleteQuery('f')); - check(getWildcardOptionsFromResults(results3.take(0))).deepEquals([]); - check(getUsersFromResults(results3.skip(0))).deepEquals([5, 4]); + check(await getResults(topicNarrow, MentionAutocompleteQuery('t'))).deepEquals([ + isWildcard(WildcardMentionOption.stream), + isWildcard(WildcardMentionOption.topic), + isUser(2), isUser(3), // 2 before 3 by DM recency + isUserGroup(3), isUserGroup(2), // 3 before 2 by alphabet ("…Three" before "…Two") + ]); + check(await getResults(topicNarrow, MentionAutocompleteQuery('f'))).deepEquals([ + isUser(5), isUser(4), + isUserGroup(4), + ]); }); }); @@ -971,6 +969,187 @@ void main() { }); }); + group('MentionAutocompleteQuery.testUser', () { + late PerAccountStore store; + + void doCheck(String rawQuery, User user, bool expected) { + final result = MentionAutocompleteQuery(rawQuery).testUser(user, store); + expected + ? check(result).isA() + : check(result).isNull(); + } + + test('user is always excluded when not active regardless of other criteria', () { + store = eg.store(); + + doCheck('Full Name', eg.user(fullName: 'Full Name', isActive: false), false); + // When active then other criteria will be checked + doCheck('Full Name', eg.user(fullName: 'Full Name', isActive: true), true); + }); + + test('user is always excluded when muted, regardless of other criteria', () async { + store = eg.store(); + await store.setMutedUsers([1]); + doCheck('Full Name', eg.user(userId: 1, fullName: 'Full Name'), false); + // When not muted, then other criteria will be checked + doCheck('Full Name', eg.user(userId: 2, fullName: 'Full Name'), true); + }); + + test('user is included if fullname words match the query', () { + store = eg.store(); + + doCheck('', eg.user(fullName: 'Full Name'), true); + doCheck('', eg.user(fullName: ''), true); // Unlikely case, but should not crash + doCheck('Full Name', eg.user(fullName: 'Full Name'), true); + doCheck('full name', eg.user(fullName: 'Full Name'), true); + doCheck('Full Name', eg.user(fullName: 'full name'), true); + doCheck('Full', eg.user(fullName: 'Full Name'), true); + doCheck('Name', eg.user(fullName: 'Full Name'), true); + doCheck('Full Name', eg.user(fullName: 'Fully Named'), true); + doCheck('Full Four', eg.user(fullName: 'Full Name Four Words'), true); + doCheck('Name Words', eg.user(fullName: 'Full Name Four Words'), true); + doCheck('Full F', eg.user(fullName: 'Full Name Four Words'), true); + doCheck('F Four', eg.user(fullName: 'Full Name Four Words'), true); + doCheck('full full', eg.user(fullName: 'Full Full Name'), true); + doCheck('full full', eg.user(fullName: 'Full Name Full'), true); + + doCheck('F', eg.user(fullName: ''), false); // Unlikely case, but should not crash + doCheck('Fully Named', eg.user(fullName: 'Full Name'), false); + doCheck('Full Name', eg.user(fullName: 'Full'), false); + doCheck('Full Name', eg.user(fullName: 'Name'), false); + doCheck('ull ame', eg.user(fullName: 'Full Name'), false); + doCheck('ull Name', eg.user(fullName: 'Full Name'), false); + doCheck('Full ame', eg.user(fullName: 'Full Name'), false); + doCheck('Full Full', eg.user(fullName: 'Full Name'), false); + doCheck('Name Name', eg.user(fullName: 'Full Name'), false); + doCheck('Name Full', eg.user(fullName: 'Full Name'), false); + doCheck('Name Four Full Words', eg.user(fullName: 'Full Name Four Words'), false); + doCheck('F Full', eg.user(fullName: 'Full Name Four Words'), false); + doCheck('Four F', eg.user(fullName: 'Full Name Four Words'), false); + }); + }); + + group('MentionAutocompleteQuery ranking', () { + // This gets filled lazily, but never reset. + // We're counting on this group's tests never doing anything to mutate it. + PerAccountStore? store; + + int? rankOf(String queryStr, Object candidate) { + final query = MentionAutocompleteQuery(queryStr); + final result = switch (candidate) { + WildcardMentionOption() => query.testWildcardOption(candidate, + localizations: GlobalLocalizations.zulipLocalizations), + User() => query.testUser(candidate, (store ??= eg.store())), + UserGroup() => query.testUserGroup(candidate, (store ??= eg.store())), + _ => throw StateError('invalid candidate'), + }; + return result?.rank; + } + + void checkPrecedes(String query, Object a, Object b) { + check(rankOf(query, a)!).isLessThan(rankOf(query, b)!); + } + + void checkSameRank(String query, Object a, Object b) { + check(rankOf(query, a)!).equals(rankOf(query, b)!); + } + + test('wildcards, then users', () { + checkSameRank('', WildcardMentionOption.all, WildcardMentionOption.topic); + checkPrecedes('', WildcardMentionOption.topic, eg.user()); + checkSameRank('', eg.user(), eg.user()); + }); + + test('wildcard-vs-user more significant than match quality', () { + // Make the query an exact match for the user's name. + final user = eg.user(fullName: 'Ann'); + checkPrecedes(user.fullName, WildcardMentionOption.channel, user); + }); + + test('user name matched case-insensitively', () { + final user1 = eg.user(fullName: 'Chris Bobbe'); + final user2 = eg.user(fullName: 'chris bobbe'); + + checkSameRank('chris bobbe', user1, user2); // exact + checkSameRank('chris bo', user1, user2); // total-prefix + checkSameRank('chr bo', user1, user2); // word-prefixes + }); + + test('user name match: exact over total-prefix', () { + final user1 = eg.user(fullName: 'Chris'); + final user2 = eg.user(fullName: 'Chris Bobbe'); + + checkPrecedes('chris', user1, user2); + }); + + test('user name match: total-prefix over word-prefixes', () { + final user1 = eg.user(fullName: 'So Many Ideas'); + final user2 = eg.user(fullName: 'Some Merry User'); + + checkPrecedes('so m', user1, user2); + }); + + test('group name matched case-insensitively', () { + final userGroup1 = eg.userGroup(name: 'Mobile Team'); + final userGroup2 = eg.userGroup(name: 'mobile team'); + + checkSameRank('mobile team', userGroup1, userGroup2); // exact + checkSameRank('mobile te', userGroup1, userGroup2); // total-prefix + checkSameRank('mob te', userGroup1, userGroup2); // word-prefixes + }); + + test('group name match: exact over total-prefix', () { + final userGroup1 = eg.userGroup(name: 'Mobile'); + final userGroup2 = eg.userGroup(name: 'Mobile Team'); + + checkPrecedes('mobile', userGroup1, userGroup2); + }); + + test('group name match: total-prefix over word-prefixes', () { + final userGroup1 = eg.userGroup(name: 'So Many Ideas'); + final userGroup2 = eg.userGroup(name: 'Some Merry Group'); + + checkPrecedes('so m', userGroup1, userGroup2); + }); + + test('email matched case-insensitively', () { + // "z" name to prevent accidental name match with example data + final user1 = eg.user(fullName: 'z', deliveryEmail: 'email@example.com'); + final user2 = eg.user(fullName: 'z', deliveryEmail: 'EmAiL@ExAmPlE.com'); + + checkSameRank('email@example.com', user1, user2); + checkSameRank('email@e', user1, user2); + checkSameRank('email@', user1, user2); + checkSameRank('email', user1, user2); + checkSameRank('ema', user1, user2); + }); + + test('email match is by prefix only', () { + // "z" name to prevent accidental name match with example data + final user = eg.user(fullName: 'z', deliveryEmail: 'email@example.com'); + + check(rankOf('e', user)).isNotNull(); + check(rankOf('mail', user)).isNull(); + check(rankOf('example', user)).isNull(); + check(rankOf('example.com', user)).isNull(); + }); + + test('full list of ranks', () { + final user1 = eg.user(fullName: 'some user', deliveryEmail: 'email@example.com'); + final userGroup1 = eg.userGroup(name: 'some user group'); + check([ + rankOf('', WildcardMentionOption.all), // wildcard + rankOf('some user', user1), // user, exact name match + rankOf('some us', user1), // user, total-prefix name match + rankOf('so us', user1), // user, word-prefixes name match + rankOf('some user group', userGroup1), // user group, exact name match + rankOf('some us', userGroup1), // user group, total-prefix name match + rankOf('so us gr', userGroup1), // user group, word-prefixes name match + rankOf('email', user1), // user, no name match, email match + ]).deepEquals([0, 1, 2, 3, 4, 5, 6, 7]); + }); + }); + group('ComposeTopicAutocomplete.autocompleteIntent', () { void doTest(String markedText, TopicAutocompleteQuery? expectedQuery) { final parsed = parseMarkedText(markedText); diff --git a/test/model/compose_test.dart b/test/model/compose_test.dart index d7479f8ec5..23a2fe8971 100644 --- a/test/model/compose_test.dart +++ b/test/model/compose_test.dart @@ -316,6 +316,22 @@ hello check(wildcardMention(WildcardMentionOption.topic, store: store())) .equals('@**topic**'); }); + + group('user group', () { + final userGroup = eg.userGroup(name: 'Group Name'); + test('not silent', () async { + final store = eg.store(); + await store.addUserGroup(userGroup); + check(userGroupMention(userGroup.name, silent: false)) + .equals('@*Group Name*'); + }); + test('silent', () async { + final store = eg.store(); + await store.addUserGroup(userGroup); + check(userGroupMention(userGroup.name, silent: true)) + .equals('@_*Group Name*'); + }); + }); }); test('inlineLink', () { diff --git a/test/model/test_store.dart b/test/model/test_store.dart index 18e41bcfb5..2d9b42e8d8 100644 --- a/test/model/test_store.dart +++ b/test/model/test_store.dart @@ -267,6 +267,16 @@ extension PerAccountStoreTestExtension on PerAccountStore { } } + Future addUserGroup(UserGroup userGroup) async { + await handleEvent(UserGroupAddEvent(id: 1, group: userGroup)); + } + + Future addUserGroups(Iterable userGroups) async { + for (final userGroup in userGroups) { + await addUserGroup(userGroup); + } + } + Future setMutedUsers(List userIds) async { await handleEvent(eg.mutedUsersEvent(userIds)); }