compose: Fix error on quote-and-replying message from unknown sender

Related: #716

Author: chrisbobbe

lib/model/compose.dart

@@ -130,14 +130,33 @@ String wrapWithBacktickFence({required String content, String? infoString}) {
 /// To omit the user ID part ("|13313") whenever the name part is unambiguous,
 /// pass the full UserStore.  This means accepting a linear scan
 /// through all users; avoid it in performance-sensitive codepaths.
+///
+/// See also [userMentionFromMessage].
 String userMention(User user, {bool silent = false, UserStore? users}) {
   bool includeUserId = users == null
     || users.allUsers.where((u) => u.fullName == user.fullName)
          .take(2).length == 2;
-
-  return '@${silent ? '_' : ''}**${user.fullName}${includeUserId ? '|${user.userId}' : ''}**';
+  return _userMentionImpl(
+    silent: silent,
+    fullName: user.fullName,
+    userId: includeUserId ? user.userId : null);
 }
 
+/// An @-mention of an individual user, like @**Chris Bobbe|13313**,
+/// from sender data in a [Message].
+///
+/// The user ID part ("|13313") is always included.
+///
+/// See also [userMention].
+String userMentionFromMessage(Message message, {bool silent = false, required UserStore users}) =>
+  _userMentionImpl(
+    silent: silent,
+    fullName: users.senderDisplayName(message),
+    userId: message.senderId);
+
+String _userMentionImpl({required bool silent, required String fullName, int? userId}) =>
+  '@${silent ? '_' : ''}**$fullName${userId != null ? '|$userId' : ''}**';
+
 /// An @-mention of all the users in a conversation, like @**channel**.
 String wildcardMention(WildcardMentionOption wildcardOption, {
   required PerAccountStore store,
@@ -190,13 +209,11 @@ String quoteAndReplyPlaceholder(
   PerAccountStore store, {
   required Message message,
 }) {
-  final sender = store.getUser(message.senderId);
-  assert(sender != null); // TODO(#716): should use `store.senderDisplayName`
   final url = narrowLink(store,
     SendableNarrow.ofMessage(message, selfUserId: store.selfUserId),
     nearMessageId: message.id);
-  // See note in [quoteAndReply] about asking `mention` to omit the |<id> part.
-  return '${userMention(sender!, silent: true)} ${inlineLink('said', url)}: ' // TODO(#1285)
+  return '${userMentionFromMessage(message, silent: true, users: store)} '
+    '${inlineLink('said', url)}: ' // TODO(#1285)
     '*${zulipLocalizations.composeBoxLoadingMessage(message.id)}*\n';
 }
 
@@ -212,14 +229,14 @@ String quoteAndReply(PerAccountStore store, {
   required Message message,
   required String rawContent,
 }) {
-  final sender = store.getUser(message.senderId);
-  assert(sender != null); // TODO(#716): should use `store.senderDisplayName`
   final url = narrowLink(store,
     SendableNarrow.ofMessage(message, selfUserId: store.selfUserId),
     nearMessageId: message.id);
-    // Could ask `mention` to omit the |<id> part unless the mention is ambiguous…
-    // but that would mean a linear scan through all users, and the extra noise
-    // won't much matter with the already probably-long message link in there too.
-    return '${userMention(sender!, silent: true)} ${inlineLink('said', url)}:\n' // TODO(#1285)
-      '${wrapWithBacktickFence(content: rawContent, infoString: 'quote')}';
+  // Could ask userMentionFromMessage to omit the |<id> part unless the mention
+  // is ambiguous… but that would mean a linear scan through all users,
+  // and the extra noise won't much matter with the already probably-long
+  // message link in there too.
+  return '${userMentionFromMessage(message, silent: true, users: store)} '
+    '${inlineLink('said', url)}:\n' // TODO(#1285)
+    '${wrapWithBacktickFence(content: rawContent, infoString: 'quote')}';
 }

test/model/compose_test.dart

@@ -1,5 +1,6 @@
 import 'package:checks/checks.dart';
 import 'package:test/scaffolding.dart';
+import 'package:zulip/api/model/events.dart';
 import 'package:zulip/model/compose.dart';
 import 'package:zulip/model/localizations.dart';
 import 'package:zulip/model/store.dart';
@@ -225,26 +226,60 @@ hello
   group('mention', () {
     group('user', () {
       final user = eg.user(userId: 123, fullName: 'Full Name');
-      test('not silent', () {
+      final message = eg.streamMessage(sender: user);
+      test('not silent', () async {
+        final store = eg.store();
+        await store.addUser(user);
         check(userMention(user, silent: false)).equals('@**Full Name|123**');
+        check(userMentionFromMessage(message, silent: false, users: store))
+          .equals('@**Full Name|123**');
       });
-      test('silent', () {
+      test('silent', () async {
+        final store = eg.store();
+        await store.addUser(user);
         check(userMention(user, silent: true)).equals('@_**Full Name|123**');
+        check(userMentionFromMessage(message, silent: true, users: store))
+          .equals('@_**Full Name|123**');
       });
       test('`users` passed; has two users with same fullName', () async {
         final store = eg.store();
         await store.addUsers([user, eg.user(userId: 5), eg.user(userId: 234, fullName: user.fullName)]);
         check(userMention(user, silent: true, users: store)).equals('@_**Full Name|123**');
+        check(userMentionFromMessage(message, silent: true, users: store))
+          .equals('@_**Full Name|123**');
       });
       test('`users` passed; has two same-name users but one of them is deactivated', () async {
         final store = eg.store();
         await store.addUsers([user, eg.user(userId: 5), eg.user(userId: 234, fullName: user.fullName, isActive: false)]);
         check(userMention(user, silent: true, users: store)).equals('@_**Full Name|123**');
+        check(userMentionFromMessage(message, silent: true, users: store))
+          .equals('@_**Full Name|123**');
       });
       test('`users` passed; user has unique fullName', () async {
         final store = eg.store();
         await store.addUsers([user, eg.user(userId: 234, fullName: 'Another Name')]);
         check(userMention(user, silent: true, users: store)).equals('@_**Full Name**');
+        check(userMentionFromMessage(message, silent: true, users: store))
+          .equals('@_**Full Name|123**');
+      });
+
+      test('userMentionFromMessage, known user', () async {
+        final user = eg.user(userId: 123, fullName: 'Full Name');
+        final store = eg.store();
+        await store.addUser(user);
+        check(userMentionFromMessage(message, silent: false, users: store))
+          .equals('@**Full Name|123**');
+        await store.handleEvent(RealmUserUpdateEvent(id: 1,
+          userId: user.userId, fullName: 'New Name'));
+        check(userMentionFromMessage(message, silent: false, users: store))
+          .equals('@**New Name|123**');
+      });
+
+      test('userMentionFromMessage, unknown user', () async {
+        final store = eg.store();
+        check(store.getUser(user.userId)).isNull();
+        check(userMentionFromMessage(message, silent: false, users: store))
+          .equals('@**Full Name|123**');
       });
     });