autocomplete: Add "recent DM conversations" criterion

sm-sayedi · sm-sayedi · commit cf3737dfddc1 · 2024-05-29T01:15:18.000+04:30
In @-mention autocomplete, users are suggested based on: 1. Recent DM conversations. Fixes part of: #228
diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart
@@ -261,13 +261,53 @@ class MentionAutocompleteView extends ChangeNotifier {
 
   List<User>? _sortedUsers;
 
-  List<User> sortByRelevance({required List<User> users}) {
-    return users; // TODO(#228) sort for most relevant first
+  /// 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.
+  int compareByDms(User userA, User userB) {
+    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,
+    };
+  }
+
+  int compareByRelevance({
+    required User userA,
+    required User userB,
+  }) {
+    final dmPrecedence = compareByDms(userA, userB);
+    return dmPrecedence;
+  }
+
+  List<User> sortByRelevance({
+    required List<User> users,
+    required Narrow narrow,
+  }) {
+    switch (narrow) {
+      case StreamNarrow():
+      case TopicNarrow():
+      case DmNarrow():
+        users.sort((userA, userB) => compareByRelevance(
+          userA: userA,
+          userB: userB));
+      case CombinedFeedNarrow():
+        // do nothing in this case for now
+    }
+    return users;
   }
 
   void _sortUsers() {
     final users = store.users.values.toList();
-    _sortedUsers = sortByRelevance(users: users);
+    _sortedUsers = sortByRelevance(users: users, narrow: narrow);
   }
 
   Future<List<MentionAutocompleteResult>?> _computeResults(MentionAutocompleteQuery query) async {
diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart
@@ -5,6 +5,7 @@ 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';
@@ -277,12 +278,12 @@ void main() {
 
   group('MentionAutocompleteView recomputes results', () {
     Future<void> doCheck({
+      Narrow narrow = const CombinedFeedNarrow(),
       required List<User> users,
       required String rawQuery,
       required Future<void> Function(PerAccountStore store) act,
       required void Function(Iterable<MentionAutocompleteResult>) expect,
     }) async {
-      const narrow = CombinedFeedNarrow();
       final store = eg.store();
       await store.addUsers(users);
       final view = MentionAutocompleteView.init(store: store, narrow: narrow);
@@ -380,6 +381,28 @@ void main() {
         expect: expect,
       );
     });
+
+    test('MessageEvent', () async {
+      final users = generateUsers(count: 1500);
+
+      Future<void> act(PerAccountStore store) async {
+        await store.addMessage(eg.dmMessage(from: eg.selfUser, to: [users[100]]));
+      }
+
+      void expect(Iterable<MentionAutocompleteResult> results) {
+        check(results.elementAt(0))
+        .isA<UserMentionAutocompleteResult>()
+        .userId.equals(100);
+      }
+
+      await doCheck(
+        narrow: DmNarrow.withUser(100, selfUserId: eg.selfUser.userId),
+        users: users,
+        rawQuery: '',
+        act: act,
+        expect: expect,
+      );
+    });
   });
 
   group('MentionAutocompleteQuery.testUser', () {
@@ -426,4 +449,157 @@ void main() {
       doCheck('Four F', eg.user(fullName: 'Full Name Four Words'), false);
     });
   });
+
+  group('MentionAutocompleteView users results', () {
+    late PerAccountStore store;
+    late MentionAutocompleteView view;
+
+    Future<void> prepare({
+      required List<User> users,
+      required List<RecentDmConversation> dmConversations,
+      required Narrow narrow,
+    }) async {
+      store = eg.store(initialSnapshot: eg.initialSnapshot(
+        recentPrivateConversations: dmConversations));
+      await store.addUsers(users);
+      view = MentionAutocompleteView.init(store: store, narrow: narrow);
+    }
+
+    group('MentionAutocompleteView.compareByDms', () {
+      test('has DMs with userA and userB, latest with userA, prioritizes userA', () async {
+        await prepare(
+          users: [],
+          dmConversations: [
+            RecentDmConversation(userIds: [1],    maxMessageId: 200),
+            RecentDmConversation(userIds: [1, 2], maxMessageId: 100),
+          ],
+          narrow: const CombinedFeedNarrow(),
+        );
+
+        final userA = eg.user(userId: 1);
+        final userB = eg.user(userId: 2);
+        final compareAB = view.compareByDms(userA, userB);
+        check(compareAB).isNegative();
+      });
+
+      test('has DMs with userA and userB, latest with userB, prioritizes userB', () async {
+        await prepare(
+          users: [],
+          dmConversations: [
+            RecentDmConversation(userIds: [2],    maxMessageId: 200),
+            RecentDmConversation(userIds: [1, 2], maxMessageId: 100),
+          ],
+          narrow: const CombinedFeedNarrow(),
+        );
+
+        final userA = eg.user(userId: 1);
+        final userB = eg.user(userId: 2);
+        final compareAB = view.compareByDms(userA, userB);
+        check(compareAB).isGreaterThan(0);
+      });
+
+      test('has DMs with userA and userB, equally recent, prioritizes neither', () async {
+        await prepare(
+          users: [],
+          dmConversations: [
+            RecentDmConversation(userIds: [1, 2], maxMessageId: 100),
+          ],
+          narrow: const CombinedFeedNarrow(),
+        );
+
+        final userA = eg.user(userId: 1);
+        final userB = eg.user(userId: 2);
+        final compareAB = view.compareByDms(userA, userB);
+        check(compareAB).equals(0);
+      });
+
+      test('has DMs with userA but not userB, prioritizes userA', () async {
+        await prepare(
+          users: [],
+          dmConversations: [
+            RecentDmConversation(userIds: [1], maxMessageId: 100),
+          ],
+          narrow: const CombinedFeedNarrow(),
+        );
+
+        final userA = eg.user(userId: 1);
+        final userB = eg.user(userId: 2);
+        final compareAB = view.compareByDms(userA, userB);
+        check(compareAB).isNegative();
+      });
+
+      test('has DMs with userB but not userA, prioritizes userB', () async {
+        await prepare(
+          users: [],
+          dmConversations: [
+            RecentDmConversation(userIds: [2], maxMessageId: 100),
+          ],
+          narrow: const CombinedFeedNarrow(),
+        );
+
+        final userA = eg.user(userId: 1);
+        final userB = eg.user(userId: 2);
+        final compareAB = view.compareByDms(userA, userB);
+        check(compareAB).isGreaterThan(0);
+      });
+
+      test('doesn\'t have DMs with userA or userB, prioritizes neither', () async {
+        await prepare(
+          users: [],
+          dmConversations: [],
+          narrow: const CombinedFeedNarrow(),
+        );
+
+        final userA = eg.user(userId: 1);
+        final userB = eg.user(userId: 2);
+        final compareAB = view.compareByDms(userA, userB);
+        check(compareAB).equals(0);
+      });
+    });
+
+    test('autocomplete suggests relevant users in the following order: '
+      '1. Users most recent in the DM conversations', () async {
+      final users = [
+        eg.user(userId: 1),
+        eg.user(userId: 2),
+        eg.user(userId: 3),
+        eg.user(userId: 4),
+        eg.user(userId: 5),
+      ];
+
+      final dmConversations = [
+        RecentDmConversation(userIds: [4],    maxMessageId: 300),
+        RecentDmConversation(userIds: [1],    maxMessageId: 200),
+        RecentDmConversation(userIds: [1, 2], maxMessageId: 100),
+      ];
+
+      Future<void> checkResultsIn(Narrow narrow, {required List<int> expected}) async {
+        await prepare(users: users, dmConversations: dmConversations, 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)
+          .toList();
+        check(results).deepEquals(expected);
+      }
+
+      const streamNarrow = StreamNarrow(1);
+      await checkResultsIn(streamNarrow, expected: [4, 1, 2, 3, 5]);
+
+      const topicNarrow = TopicNarrow(1, 'topic');
+      await checkResultsIn(topicNarrow, expected: [4, 1, 2, 3, 5]);
+
+      final dmNarrow = DmNarrow(allRecipientIds: [eg.selfUser.userId], selfUserId: eg.selfUser.userId);
+      await checkResultsIn(dmNarrow, expected: [4, 1, 2, 3, 5]);
+
+      const allMessagesNarrow = CombinedFeedNarrow();
+      // Results are in the original order as we do not sort them for
+      // [AllMessagesNarrow] because we can not access autocomplete for now.
+      await checkResultsIn(allMessagesNarrow, expected: [1, 2, 3, 4, 5]);
+    });
+  });
 }
