From fd08ab860e3a4f134631dcba2d76f90c0a5bd19f Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 28 May 2024 23:11:49 +0430 Subject: [PATCH 1/5] recent-dms test [nfc]: Comment on a missing not-notified check --- test/model/recent_dm_conversations_test.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/model/recent_dm_conversations_test.dart b/test/model/recent_dm_conversations_test.dart index f4ae547ee4..68ea72280e 100644 --- a/test/model/recent_dm_conversations_test.dart +++ b/test/model/recent_dm_conversations_test.dart @@ -129,13 +129,17 @@ void main() { }); test('existing conversation, not newest in conversation', () { + // bool listenersNotified = false; final message = eg.dmMessage(id: 99, from: eg.selfUser, to: [eg.user(userId: 1), eg.user(userId: 2)]); final expected = setupView(); check(setupView() + // ..addListener(() { listenersNotified = true; }) ..handleMessageEvent(MessageEvent(id: 1, message: message)) ) ..map.deepEquals(expected.map) ..sorted.deepEquals(expected.sorted); + // (listeners are notified unnecessarily, but that's OK) + // check(listenersNotified).isFalse(); }); }); }); From 177b71930b7e1eb3e96d0d8eb72ae8fe24e51f27 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Sat, 18 May 2024 15:46:41 +0430 Subject: [PATCH 2/5] recent-dms: Add `latestMessagesByRecipient` data structure This data structure is used to keep track of the latest message of each recipient across all DM conversations. --- lib/model/recent_dm_conversations.dart | 38 +++++++++++- .../model/recent_dm_conversations_checks.dart | 2 + test/model/recent_dm_conversations_test.dart | 61 ++++++++++++++++--- 3 files changed, 91 insertions(+), 10 deletions(-) diff --git a/lib/model/recent_dm_conversations.dart b/lib/model/recent_dm_conversations.dart index 6c93b84db0..b38610be15 100644 --- a/lib/model/recent_dm_conversations.dart +++ b/lib/model/recent_dm_conversations.dart @@ -1,3 +1,5 @@ +import 'dart:math'; + import 'package:collection/collection.dart'; import 'package:flutter/foundation.dart'; @@ -19,9 +21,21 @@ class RecentDmConversationsView extends ChangeNotifier { DmNarrow.ofRecentDmConversation(conversation, selfUserId: selfUserId), conversation.maxMessageId, )).toList()..sort((a, b) => -a.value.compareTo(b.value)); + + final latestMessagesByRecipient = {}; + for (final entry in entries) { + final dmNarrow = entry.key; + final maxMessageId = entry.value; + for (final userId in dmNarrow.otherRecipientIds) { + // Only take the latest message of a user across all the conversations. + latestMessagesByRecipient.putIfAbsent(userId, () => maxMessageId); + } + } + return RecentDmConversationsView._( map: Map.fromEntries(entries), sorted: QueueList.from(entries.map((e) => e.key)), + latestMessagesByRecipient: latestMessagesByRecipient, selfUserId: selfUserId, ); } @@ -29,6 +43,7 @@ class RecentDmConversationsView extends ChangeNotifier { RecentDmConversationsView._({ required this.map, required this.sorted, + required this.latestMessagesByRecipient, required this.selfUserId, }); @@ -38,6 +53,15 @@ class RecentDmConversationsView extends ChangeNotifier { /// The [DmNarrow] keys of [map], sorted by latest message descending. final QueueList sorted; + /// Map from user ID to the latest message ID in any conversation with the user. + /// + /// Both 1:1 and group DM conversations are considered. + /// The self-user ID is excluded even if there is a self-DM conversation. + /// + /// (The identified message was not necessarily sent by the identified user; + /// it might have been sent by anyone in its conversation.) + final Map latestMessagesByRecipient; + final int selfUserId; /// Insert the key at the proper place in [sorted]. @@ -58,7 +82,7 @@ class RecentDmConversationsView extends ChangeNotifier { } } - /// Handle [MessageEvent], updating [map] and [sorted]. + /// Handle [MessageEvent], updating [map], [sorted], and [latestMessagesByRecipient]. /// /// Can take linear time in general. That sounds inefficient... /// but it's what the webapp does, so must not be catastrophic. 🤷 @@ -89,6 +113,8 @@ class RecentDmConversationsView extends ChangeNotifier { return; } final key = DmNarrow.ofMessage(message, selfUserId: selfUserId); + + // Update [map] and [sorted]. final prev = map[key]; if (prev == null) { // The conversation is new. Add to both `map` and `sorted`. @@ -117,6 +143,16 @@ class RecentDmConversationsView extends ChangeNotifier { _insertSorted(key, message.id); } } + + // Update [latestMessagesByRecipient]. + for (final recipient in key.otherRecipientIds) { + latestMessagesByRecipient.update( + recipient, + (latestMessageId) => max(message.id, latestMessageId), + ifAbsent: () => message.id, + ); + } + notifyListeners(); } diff --git a/test/model/recent_dm_conversations_checks.dart b/test/model/recent_dm_conversations_checks.dart index af92ca25ac..bbb78593f9 100644 --- a/test/model/recent_dm_conversations_checks.dart +++ b/test/model/recent_dm_conversations_checks.dart @@ -6,4 +6,6 @@ import 'package:zulip/model/recent_dm_conversations.dart'; extension RecentDmConversationsViewChecks on Subject { Subject> get map => has((v) => v.map, 'map'); Subject> get sorted => has((v) => v.sorted, 'sorted'); + Subject> get latestMessagesByRecipient => has( + (v) => v.latestMessagesByRecipient, 'latestMessagesByRecipient'); } diff --git a/test/model/recent_dm_conversations_test.dart b/test/model/recent_dm_conversations_test.dart index 68ea72280e..5ef12a130b 100644 --- a/test/model/recent_dm_conversations_test.dart +++ b/test/model/recent_dm_conversations_test.dart @@ -22,7 +22,8 @@ void main() { check(RecentDmConversationsView(selfUserId: eg.selfUser.userId, initial: [])) ..map.isEmpty() - ..sorted.isEmpty(); + ..sorted.isEmpty() + ..latestMessagesByRecipient.isEmpty(); check(RecentDmConversationsView(selfUserId: eg.selfUser.userId, initial: [ @@ -35,7 +36,8 @@ void main() { key([]): 200, key([1]): 100, }) - ..sorted.deepEquals([key([1, 2]), key([]), key([1])]); + ..sorted.deepEquals([key([1, 2]), key([]), key([1])]) + ..latestMessagesByRecipient.deepEquals({1: 300, 2: 300}); }); group('message event (new message)', () { @@ -55,7 +57,8 @@ void main() { key([1]): 200, key([1, 2]): 100, }) - ..sorted.deepEquals([key([1]), key([1, 2])]); + ..sorted.deepEquals([key([1]), key([1, 2])]) + ..latestMessagesByRecipient.deepEquals({1: 200, 2: 100}); }); test('stream message -> do nothing', () { @@ -65,7 +68,8 @@ void main() { ..addListener(() { listenersNotified = true; }) ..handleMessageEvent(MessageEvent(id: 1, message: eg.streamMessage())) ) ..map.deepEquals(expected.map) - ..sorted.deepEquals(expected.sorted); + ..sorted.deepEquals(expected.sorted) + ..latestMessagesByRecipient.deepEquals(expected.latestMessagesByRecipient); check(listenersNotified).isFalse(); }); @@ -80,7 +84,8 @@ void main() { key([1]): 200, key([1, 2]): 100, }) - ..sorted.deepEquals([key([2]), key([1]), key([1, 2])]); + ..sorted.deepEquals([key([2]), key([1]), key([1, 2])]) + ..latestMessagesByRecipient.deepEquals({2: 300, 1: 200}); check(listenersNotified).isTrue(); }); @@ -95,7 +100,8 @@ void main() { key([2]): 150, key([1, 2]): 100, }) - ..sorted.deepEquals([key([1]), key([2]), key([1, 2])]); + ..sorted.deepEquals([key([1]), key([2]), key([1, 2])]) + ..latestMessagesByRecipient.deepEquals({1: 200, 2: 150}); check(listenersNotified).isTrue(); }); @@ -110,7 +116,8 @@ void main() { key([1, 2]): 300, key([1]): 200, }) - ..sorted.deepEquals([key([1, 2]), key([1])]); + ..sorted.deepEquals([key([1, 2]), key([1])]) + ..latestMessagesByRecipient.deepEquals({1: 300, 2: 300}); check(listenersNotified).isTrue(); }); @@ -124,7 +131,8 @@ void main() { key([1]): 300, key([1, 2]): 100, }) - ..sorted.deepEquals([key([1]), key([1, 2])]); + ..sorted.deepEquals([key([1]), key([1, 2])]) + ..latestMessagesByRecipient.deepEquals({1: 300, 2: 100}); check(listenersNotified).isTrue(); }); @@ -137,10 +145,45 @@ void main() { // ..addListener(() { listenersNotified = true; }) ..handleMessageEvent(MessageEvent(id: 1, message: message)) ) ..map.deepEquals(expected.map) - ..sorted.deepEquals(expected.sorted); + ..sorted.deepEquals(expected.sorted) + ..latestMessagesByRecipient.deepEquals(expected.latestMessagesByRecipient); // (listeners are notified unnecessarily, but that's OK) // check(listenersNotified).isFalse(); }); + + test('new conversation with one existing and one new user, newest message', () { + bool listenersNotified = false; + final message = eg.dmMessage(id: 300, from: eg.selfUser, + to: [eg.user(userId: 1), eg.user(userId: 3)]); + check(setupView() + ..addListener(() { listenersNotified = true; }) + ..handleMessageEvent(MessageEvent(id: 1, message: message)) + ) ..map.deepEquals({ + key([1, 3]): 300, + key([1]): 200, + key([1, 2]): 100, + }) + ..sorted.deepEquals([key([1, 3]), key([1]), key([1, 2])]) + ..latestMessagesByRecipient.deepEquals({1: 300, 3: 300, 2: 100}); + check(listenersNotified).isTrue(); + }); + + test('new conversation with one existing and one new user, not newest message', () { + bool listenersNotified = false; + final message = eg.dmMessage(id: 150, from: eg.selfUser, + to: [eg.user(userId: 1), eg.user(userId: 3)]); + check(setupView() + ..addListener(() { listenersNotified = true; }) + ..handleMessageEvent(MessageEvent(id: 1, message: message)) + ) ..map.deepEquals({ + key([1]): 200, + key([1, 3]): 150, + key([1, 2]): 100, + }) + ..sorted.deepEquals([key([1]), key([1, 3]), key([1, 2])]) + ..latestMessagesByRecipient.deepEquals({1: 200, 3: 150, 2: 100}); + check(listenersNotified).isTrue(); + }); }); }); } From f39aecd4ebac4771d06bb1fd20180abab8aa77dd Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Fri, 17 May 2024 00:04:39 +0430 Subject: [PATCH 3/5] autocomplete: Remove the `refreshStaleUserResults` method This method was used to search for the new user results in autocomplete when the users in store were modified during an in-progress computation. Along with this, `_startSearch` would do the same thing by retrying, resulting in duplicate actions. By removing this, the unnecessary search for user results is prevented. Besides, this commit also removes some live-updating behavior of user-mention autocomplete results in order to provide a better UX. To read more about this decision, see: https://github.com/zulip/zulip-flutter/pull/693#pullrequestreview-2091696195 --- lib/model/autocomplete.dart | 21 --------------------- lib/model/store.dart | 1 - test/model/autocomplete_test.dart | 2 ++ 3 files changed, 2 insertions(+), 22 deletions(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 560b69a3e9..7391844d37 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -125,23 +125,11 @@ class AutocompleteViewManager { assert(removed); } - void handleRealmUserAddEvent(RealmUserAddEvent event) { - for (final view in _mentionAutocompleteViews) { - view.refreshStaleUserResults(); - } - } - void handleRealmUserRemoveEvent(RealmUserRemoveEvent event) { - for (final view in _mentionAutocompleteViews) { - view.refreshStaleUserResults(); - } autocompleteDataCache.invalidateUser(event.userId); } void handleRealmUserUpdateEvent(RealmUserUpdateEvent event) { - for (final view in _mentionAutocompleteViews) { - view.refreshStaleUserResults(); - } autocompleteDataCache.invalidateUser(event.userId); } @@ -208,15 +196,6 @@ class MentionAutocompleteView extends ChangeNotifier { } } - /// Recompute user results for the current query, if any. - /// - /// Called in particular when we get a [RealmUserEvent]. - void refreshStaleUserResults() { - if (_query != null) { - _startSearch(_query!); - } - } - /// Called when the app is reassembled during debugging, e.g. for hot reload. /// /// This will redo the search from scratch for the current query, if any. diff --git a/lib/model/store.dart b/lib/model/store.dart index baae805051..27c7bd2f42 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -418,7 +418,6 @@ class PerAccountStore extends ChangeNotifier with StreamStore, MessageStore { } else if (event is RealmUserAddEvent) { assert(debugLog("server event: realm_user/add")); users[event.person.userId] = event.person; - autocompleteViewManager.handleRealmUserAddEvent(event); notifyListeners(); } else if (event is RealmUserRemoveEvent) { assert(debugLog("server event: realm_user/remove")); diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index 576a6aa262..e5b06f5ad5 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -292,6 +292,8 @@ void main() { await Future(() {}); check(done).isFalse(); await Future(() {}); + check(done).isFalse(); + await Future(() {}); check(done).isTrue(); check(view.results).single .isA() From 0960fa9de66298894d0962abc1b6c3b68d6eb959 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Fri, 17 May 2024 00:06:14 +0430 Subject: [PATCH 4/5] autocomplete: Introduce a field for sorted users This field will be used to maintain a list of sorted users based on the most relevant autocomplete criteria in the upcoming commits. This commit also removes the retry logic for an in-progress query computation as there is no longer the cause that would necessitate such a retry. --- lib/model/autocomplete.dart | 38 +++++++++++++++---------------- test/model/autocomplete_test.dart | 30 ++++++++++++------------ 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 7391844d37..e6a4ab3606 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -164,17 +164,29 @@ class AutocompleteViewManager { /// * When the object will no longer be used, call [dispose] to free /// resources on the [PerAccountStore]. class MentionAutocompleteView extends ChangeNotifier { - MentionAutocompleteView._({required this.store, required this.narrow}); + MentionAutocompleteView._({ + required this.store, + required this.narrow, + required this.sortedUsers, + }); factory MentionAutocompleteView.init({ required PerAccountStore store, required Narrow narrow, }) { - final view = MentionAutocompleteView._(store: store, narrow: narrow); + final view = MentionAutocompleteView._( + store: store, + narrow: narrow, + sortedUsers: _usersByRelevance(store: store), + ); store.autocompleteViewManager.registerMentionAutocomplete(view); return view; } + static List _usersByRelevance({required PerAccountStore store}) { + return store.users.values.toList(); // TODO(#228): sort for most relevant first + } + @override void dispose() { store.autocompleteViewManager.unregisterMentionAutocomplete(this); @@ -186,6 +198,7 @@ class MentionAutocompleteView extends ChangeNotifier { final PerAccountStore store; final Narrow narrow; + final List sortedUsers; MentionAutocompleteQuery? get query => _query; MentionAutocompleteQuery? _query; @@ -209,18 +222,7 @@ class MentionAutocompleteView extends ChangeNotifier { List _results = []; Future _startSearch(MentionAutocompleteQuery query) async { - List? newResults; - - while (true) { - try { - newResults = await _computeResults(query); - break; - } on ConcurrentModificationError { - // Retry - // TODO backoff? - } - } - + final newResults = await _computeResults(query); if (newResults == null) { // Query was old; new search is in progress. Or, no listeners to notify. return; @@ -232,9 +234,7 @@ class MentionAutocompleteView extends ChangeNotifier { Future?> _computeResults(MentionAutocompleteQuery query) async { final List results = []; - final Iterable users = store.users.values; - - final iterator = users.iterator; + final iterator = sortedUsers.iterator; bool isDone = false; while (!isDone) { // CPU perf: End this task; enqueue a new one for resuming this work @@ -245,7 +245,7 @@ class MentionAutocompleteView extends ChangeNotifier { } for (int i = 0; i < 1000; i++) { - if (!iterator.moveNext()) { // Can throw ConcurrentModificationError + if (!iterator.moveNext()) { isDone = true; break; } @@ -256,7 +256,7 @@ class MentionAutocompleteView extends ChangeNotifier { } } } - return results; // TODO(#228) sort for most relevant first + return results; } } diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index e5b06f5ad5..eeab67eba5 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -274,37 +274,35 @@ void main() { } }); - test('MentionAutocompleteView mutating store.users while in progress causes retry', () async { + test('MentionAutocompleteView mutating store.users while in progress does not ' + 'prevent query from finishing', () async { const narrow = CombinedFeedNarrow(); final store = eg.store(); - for (int i = 0; i < 1500; i++) { + for (int i = 0; i < 2500; i++) { await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i')); } final view = MentionAutocompleteView.init(store: store, narrow: narrow); bool done = false; view.addListener(() { done = true; }); - view.query = MentionAutocompleteQuery('User 10000'); + view.query = MentionAutocompleteQuery('User 110'); await Future(() {}); check(done).isFalse(); - await store.addUser(eg.user(userId: 10000, email: 'user10000@example.com', fullName: 'User 10000')); - await Future(() {}); - check(done).isFalse(); + await store.addUser(eg.user(userId: 11000, email: 'user11000@example.com', fullName: 'User 11000')); await Future(() {}); check(done).isFalse(); - await Future(() {}); - check(done).isTrue(); - check(view.results).single - .isA() - .userId.equals(10000); - // new result sticks; no "zombie" result from `store.users` pre-mutation - for (int i = 0; i < 10; i++) { // for good measure + for (int i = 0; i < 3; i++) { await Future(() {}); - check(view.results).single - .isA() - .userId.equals(10000); } + check(done).isTrue(); + final results = view.results + .map((e) => (e as UserMentionAutocompleteResult).userId); + check(results) + ..contains(110) + ..contains(1100) + // Does not include the newly-added user as we finish the query with stale users. + ..not((results) => results.contains(11000)); }); group('MentionAutocompleteQuery.testUser', () { From 37ad773274e3ea73d33c54a9f5eddaff35d9938c Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Wed, 29 May 2024 00:27:00 +0430 Subject: [PATCH 5/5] autocomplete: Add "recent DM conversations" criterion In @-mention autocomplete, users are suggested based on: 1. Recent DM conversations. Fixes part of: #228 --- lib/model/autocomplete.dart | 31 ++++++- test/model/autocomplete_test.dart | 135 ++++++++++++++++++++++++++++-- 2 files changed, 158 insertions(+), 8 deletions(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index e6a4ab3606..5250dba6d4 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -177,14 +177,39 @@ class MentionAutocompleteView extends ChangeNotifier { final view = MentionAutocompleteView._( store: store, narrow: narrow, - sortedUsers: _usersByRelevance(store: store), + sortedUsers: _usersByRelevance(store: store, narrow: narrow), ); store.autocompleteViewManager.registerMentionAutocomplete(view); return view; } - static List _usersByRelevance({required PerAccountStore store}) { - return store.users.values.toList(); // TODO(#228): sort for most relevant first + static List _usersByRelevance({ + required PerAccountStore store, + required Narrow narrow, + }) { + assert(narrow is! CombinedFeedNarrow); + return store.users.values.toList() + ..sort((userA, userB) => compareByDms(userA, userB, store: store)); + } + + /// Determines which of the two users is more recent in DM conversations. + /// + /// Returns a negative number if [userA] is more recent than [userB], + /// returns a positive number if [userB] is more recent than [userA], + /// and returns `0` if both [userA] and [userB] are equally recent + /// or there is no DM exchanged with them whatsoever. + @visibleForTesting + static int compareByDms(User userA, User userB, {required PerAccountStore store}) { + final recentDms = store.recentDmConversationsView; + final aLatestMessageId = recentDms.latestMessagesByRecipient[userA.userId]; + final bLatestMessageId = recentDms.latestMessagesByRecipient[userB.userId]; + + return switch((aLatestMessageId, bLatestMessageId)) { + (int a, int b) => -a.compareTo(b), + (int(), _) => -1, + (_, int()) => 1, + _ => 0, + }; } @override diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index eeab67eba5..03fbaa9c28 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -5,9 +5,11 @@ import 'package:checks/checks.dart'; import 'package:fake_async/fake_async.dart'; import 'package:flutter/cupertino.dart'; import 'package:test/scaffolding.dart'; +import 'package:zulip/api/model/initial_snapshot.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/model/autocomplete.dart'; import 'package:zulip/model/narrow.dart'; +import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/compose_box.dart'; import '../example_data.dart' as eg; @@ -166,7 +168,7 @@ void main() { }); test('MentionAutocompleteView misc', () async { - const narrow = CombinedFeedNarrow(); + const narrow = StreamNarrow(1); final store = eg.store(); await store.addUsers([eg.selfUser, eg.otherUser, eg.thirdUser]); final view = MentionAutocompleteView.init(store: store, narrow: narrow); @@ -183,7 +185,7 @@ void main() { test('MentionAutocompleteView not starve timers', () { fakeAsync((binding) async { - const narrow = CombinedFeedNarrow(); + const narrow = StreamNarrow(1); final store = eg.store(); await store.addUsers([eg.selfUser, eg.otherUser, eg.thirdUser]); final view = MentionAutocompleteView.init(store: store, narrow: narrow); @@ -218,7 +220,7 @@ void main() { }); test('MentionAutocompleteView yield between batches of 1000', () async { - const narrow = CombinedFeedNarrow(); + const narrow = StreamNarrow(1); final store = eg.store(); for (int i = 0; i < 2500; i++) { await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i')); @@ -241,7 +243,7 @@ void main() { }); test('MentionAutocompleteView new query during computation replaces old', () async { - const narrow = CombinedFeedNarrow(); + const narrow = StreamNarrow(1); final store = eg.store(); for (int i = 0; i < 1500; i++) { await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i')); @@ -276,7 +278,7 @@ void main() { test('MentionAutocompleteView mutating store.users while in progress does not ' 'prevent query from finishing', () async { - const narrow = CombinedFeedNarrow(); + const narrow = StreamNarrow(1); final store = eg.store(); for (int i = 0; i < 2500; i++) { await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i')); @@ -349,4 +351,127 @@ void main() { doCheck('Four F', eg.user(fullName: 'Full Name Four Words'), false); }); }); + + group('MentionAutocompleteView sorting users results', () { + late PerAccountStore store; + + Future prepare({ + List users = const [], + List dmConversations = const [], + }) async { + store = eg.store(initialSnapshot: eg.initialSnapshot( + recentPrivateConversations: dmConversations)); + await store.addUsers(users); + } + + group('MentionAutocompleteView.compareByDms', () { + const idA = 1; + const idB = 2; + + int compareAB() => MentionAutocompleteView.compareByDms( + eg.user(userId: idA), + eg.user(userId: idB), + store: store, + ); + + test('has DMs with userA and userB, latest with userA -> prioritizes userA', () async { + await prepare(dmConversations: [ + RecentDmConversation(userIds: [idA], maxMessageId: 200), + RecentDmConversation(userIds: [idA, idB], maxMessageId: 100), + ]); + check(compareAB()).isLessThan(0); + }); + + test('has DMs with userA and userB, latest with userB -> prioritizes userB', () async { + await prepare(dmConversations: [ + RecentDmConversation(userIds: [idB], maxMessageId: 200), + RecentDmConversation(userIds: [idA, idB], maxMessageId: 100), + ]); + check(compareAB()).isGreaterThan(0); + }); + + test('has DMs with userA and userB, equally recent -> prioritizes neither', () async { + await prepare(dmConversations: [ + RecentDmConversation(userIds: [idA, idB], maxMessageId: 100), + ]); + check(compareAB()).equals(0); + }); + + test('has DMs with userA but not userB -> prioritizes userA', () async { + await prepare(dmConversations: [ + RecentDmConversation(userIds: [idA], maxMessageId: 100), + ]); + check(compareAB()).isLessThan(0); + }); + + test('has DMs with userB but not userA -> prioritizes userB', () async { + await prepare(dmConversations: [ + RecentDmConversation(userIds: [idB], maxMessageId: 100), + ]); + check(compareAB()).isGreaterThan(0); + }); + + test('has no DMs with userA or userB -> prioritizes neither', () async { + await prepare(dmConversations: []); + check(compareAB()).equals(0); + }); + }); + + group('autocomplete suggests relevant users in the intended order', () { + // The order should be: + // 1. Users most recent in the DM conversations + + Future checkResultsIn(Narrow narrow, {required List expected}) async { + final users = [ + eg.user(userId: 0), + eg.user(userId: 1), + eg.user(userId: 2), + eg.user(userId: 3), + eg.user(userId: 4), + ]; + + final dmConversations = [ + RecentDmConversation(userIds: [3], maxMessageId: 300), + RecentDmConversation(userIds: [0], maxMessageId: 200), + RecentDmConversation(userIds: [0, 1], maxMessageId: 100), + ]; + + await prepare(users: users, dmConversations: dmConversations); + final view = MentionAutocompleteView.init(store: store, narrow: narrow); + + bool done = false; + view.addListener(() { done = true; }); + view.query = MentionAutocompleteQuery(''); + await Future(() {}); + check(done).isTrue(); + final results = view.results + .map((e) => (e as UserMentionAutocompleteResult).userId); + check(results).deepEquals(expected); + } + + test('StreamNarrow', () async { + await checkResultsIn(const StreamNarrow(1), expected: [3, 0, 1, 2, 4]); + }); + + test('TopicNarrow', () async { + await checkResultsIn(const TopicNarrow(1, 'topic'), expected: [3, 0, 1, 2, 4]); + }); + + test('DmNarrow', () async { + await checkResultsIn( + DmNarrow.withUser(eg.selfUser.userId, selfUserId: eg.selfUser.userId), + expected: [3, 0, 1, 2, 4], + ); + }); + + test('CombinedFeedNarrow', () async { + // As we do not expect a compose box in [CombinedFeedNarrow], it should + // not proceed to show any results. + await check(checkResultsIn( + const CombinedFeedNarrow(), + expected: [0, 1, 2, 3, 4]) + ).throws(); + }); + }); + }); }