local echo (6.75/7): Revised commit for local echo (sans retrieving failed outbox content)

lib/model/message.dart

@@ -394,6 +394,8 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore, _OutboxMes
       }
     }
 
+    // TODO predict outbox message moves using propagateMode
+
     for (final view in _messageListViews) {
       view.messagesMoved(messageMove: messageMove, messageIds: event.messageIds);
     }

lib/model/message_list.dart

@@ -66,6 +66,22 @@ class MessageListMessageItem extends MessageListMessageBaseItem {
   });
 }
 
+/// An [OutboxMessage] to show in the message list.
+class MessageListOutboxMessageItem extends MessageListMessageBaseItem {
+  @override
+  final OutboxMessage message;
+  @override
+  final ZulipContent content;
+
+  MessageListOutboxMessageItem(
+    this.message, {
+    required super.showSender,
+    required super.isLastInBlock,
+  }) : content = ZulipContent(nodes: [
+    ParagraphNode(links: null, nodes: [TextNode(message.contentMarkdown)]),
+  ]);
+}
+
 /// The status of outstanding or recent fetch requests from a [MessageListView].
 enum FetchingStatus {
   /// The model has not made any fetch requests (since its last reset, if any).
@@ -158,14 +174,24 @@ mixin _MessageSequence {
   /// It exists as an optimization, to memoize the work of parsing.
   final List<ZulipMessageContent> contents = [];
 
+  /// The [OutboxMessage]s sent by the self-user, retrieved from
+  /// [MessageStore.outboxMessages].
+  ///
+  /// See also [items].
+  ///
+  /// O(N) iterations through this list are acceptable
+  /// because it won't normally have more than a few items.
+  final List<OutboxMessage> outboxMessages = [];
+
   /// The messages and their siblings in the UI, in order.
   ///
   /// This has a [MessageListMessageItem] corresponding to each element
   /// of [messages], in order.  It may have additional items interspersed
-  /// before, between, or after the messages.
+  /// before, between, or after the messages. Then, similarly,
+  /// [MessageListOutboxMessageItem]s corresponding to [outboxMessages].
   ///
-  /// This information is completely derived from [messages] and
-  /// the flags [haveOldest], [haveNewest], and [busyFetchingMore].
+  /// This information is completely derived from [messages], [outboxMessages],
+  /// and the flags [haveOldest], [haveNewest], and [busyFetchingMore].
   /// It exists as an optimization, to memoize that computation.
   ///
   /// See also [middleItem], an index which divides this list
@@ -177,11 +203,14 @@ mixin _MessageSequence {
   /// The indices 0 to before [middleItem] are the top slice of [items],
   /// and the indices from [middleItem] to the end are the bottom slice.
   ///
-  /// The top and bottom slices of [items] correspond to
-  /// the top and bottom slices of [messages] respectively.
-  /// Either the bottom slices of both [items] and [messages] are empty,
-  /// or the first item in the bottom slice of [items] is a [MessageListMessageItem]
-  /// for the first message in the bottom slice of [messages].
+  /// The top slice of [items] corresponds to the top slice of [messages].
+  /// The bottom slice of [items] corresponds to the bottom slice of [messages]
+  /// plus any [outboxMessages].
+  ///
+  /// The bottom slice will either be empty
+  /// or start with a [MessageListMessageBaseItem].
+  /// It will not start with a [MessageListDateSeparatorItem]
+  /// or a [MessageListRecipientHeaderItem].
   int middleItem = 0;
 
   int _findMessageWithId(int messageId) {
@@ -197,9 +226,10 @@ mixin _MessageSequence {
     switch (item) {
       case MessageListRecipientHeaderItem(:var message):
       case MessageListDateSeparatorItem(:var message):
-        if (message.id == null)                  return 1;  // TODO(#1441): test
+        if (message.id == null)                  return 1;
         return message.id! <= messageId ? -1 : 1;
       case MessageListMessageItem(:var message): return message.id.compareTo(messageId);
+      case MessageListOutboxMessageItem():       return 1;
     }
   }
 
@@ -316,11 +346,48 @@ mixin _MessageSequence {
     _reprocessAll();
   }
 
+  /// Append [outboxMessage] to [outboxMessages] and update derived data
+  /// accordingly.
+  ///
+  /// The caller is responsible for ensuring this is an appropriate thing to do
+  /// given [narrow] and other concerns.
+  void _addOutboxMessage(OutboxMessage outboxMessage) {
+    assert(haveNewest);
+    assert(!outboxMessages.contains(outboxMessage));
+    outboxMessages.add(outboxMessage);
+    _processOutboxMessage(outboxMessages.length - 1);
+  }
+
+  /// Remove the [outboxMessage] from the view.
+  ///
+  /// Returns true if the outbox message was removed, false otherwise.
+  bool _removeOutboxMessage(OutboxMessage outboxMessage) {
+    if (!outboxMessages.remove(outboxMessage)) {
+      return false;
+    }
+    _reprocessOutboxMessages();
+    return true;
+  }
+
+  /// Remove all outbox messages that satisfy [test] from [outboxMessages].
+  ///
+  /// Returns true if any outbox messages were removed, false otherwise.
+  bool _removeOutboxMessagesWhere(bool Function(OutboxMessage) test) {
+    final count = outboxMessages.length;
+    outboxMessages.removeWhere(test);
+    if (outboxMessages.length == count) {
+      return false;
+    }
+    _reprocessOutboxMessages();
+    return true;
+  }
+
   /// Reset all [_MessageSequence] data, and cancel any active fetches.
   void _reset() {
     generation += 1;
     messages.clear();
     middleMessage = 0;
+    outboxMessages.clear();
     _haveOldest = false;
     _haveNewest = false;
     _status = FetchingStatus.unstarted;
@@ -379,7 +446,6 @@ mixin _MessageSequence {
     assert(item.showSender == !canShareSender);
     assert(item.isLastInBlock);
     if (shouldSetMiddleItem) {
-      assert(item is MessageListMessageItem);
       middleItem = items.length;
     }
     items.add(item);
@@ -390,6 +456,7 @@ mixin _MessageSequence {
   /// The previous messages in the list must already have been processed.
   /// This message must already have been parsed and reflected in [contents].
   void _processMessage(int index) {
+    assert(items.lastOrNull is! MessageListOutboxMessageItem);
     final prevMessage = index == 0 ? null : messages[index - 1];
     final message = messages[index];
     final content = contents[index];
@@ -401,13 +468,67 @@ mixin _MessageSequence {
         message, content, showSender: !canShareSender, isLastInBlock: true));
   }
 
-  /// Recompute [items] from scratch, based on [messages], [contents], and flags.
+  /// Append to [items] based on the index-th message in [outboxMessages].
+  ///
+  /// All [messages] and previous messages in [outboxMessages] must already have
+  /// been processed.
+  void _processOutboxMessage(int index) {
+    final prevMessage = index == 0 ? messages.lastOrNull
+                                   : outboxMessages[index - 1];
+    final message = outboxMessages[index];
+
+    _addItemsForMessage(message,
+      // The first outbox message item becomes the middle item
+      // when the bottom slice of [messages] is empty.
+      shouldSetMiddleItem: index == 0 && middleMessage == messages.length,
+      prevMessage: prevMessage,
+      buildItem: (bool canShareSender) => MessageListOutboxMessageItem(
+        message, showSender: !canShareSender, isLastInBlock: true));
+  }
+
+  /// Remove items associated with [outboxMessages] from [items].
+  ///
+  /// This is designed to be idempotent; repeated calls will not change the
+  /// content of [items].
+  ///
+  /// This is efficient due to the expected small size of [outboxMessages].
+  void _removeOutboxMessageItems() {
+    // This loop relies on the assumption that all items that follow
+    // the last [MessageListMessageItem] are derived from outbox messages.
+    while (items.isNotEmpty && items.last is! MessageListMessageItem) {
+      items.removeLast();
+    }
+
+    if (items.isNotEmpty) {
+      final lastItem = items.last as MessageListMessageItem;
+      lastItem.isLastInBlock = true;
+    }
+    if (middleMessage == messages.length) middleItem = items.length;
+  }
+
+  /// Recompute the portion of [items] derived from outbox messages,
+  /// based on [outboxMessages] and [messages].
+  ///
+  /// All [messages] should have been processed when this is called.
+  void _reprocessOutboxMessages() {
+    assert(haveNewest);
+    _removeOutboxMessageItems();
+    for (var i = 0; i < outboxMessages.length; i++) {
+      _processOutboxMessage(i);
+    }
+  }
+
+  /// Recompute [items] from scratch, based on [messages], [contents],
+  /// [outboxMessages] and flags.
   void _reprocessAll() {
     items.clear();
     for (var i = 0; i < messages.length; i++) {
       _processMessage(i);
     }
     if (middleMessage == messages.length) middleItem = items.length;
+    for (var i = 0; i < outboxMessages.length; i++) {
+      _processOutboxMessage(i);
+    }
   }
 }
 
@@ -602,6 +723,11 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     }
     _haveOldest = result.foundOldest;
     _haveNewest = result.foundNewest;
+
+    if (haveNewest) {
+      _syncOutboxMessagesFromStore();
+    }
+
     _setStatus(FetchingStatus.idle, was: FetchingStatus.fetchInitial);
   }
 
@@ -706,6 +832,10 @@ class MessageListView with ChangeNotifier, _MessageSequence {
           }
         }
         _haveNewest = result.foundNewest;
+
+        if (haveNewest) {
+          _syncOutboxMessagesFromStore();
+        }
       });
   }
 
@@ -770,9 +900,42 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     fetchInitial();
   }
 
+  bool _shouldAddOutboxMessage(OutboxMessage outboxMessage) {
+    assert(haveNewest);
+    return !outboxMessage.hidden
+      && narrow.containsMessage(outboxMessage)
+      && _messageVisible(outboxMessage);
+  }
+
+  /// Reads [MessageStore.outboxMessages] and copies to [outboxMessages]
+  /// the ones belonging to this view.
+  ///
+  /// This should only be called when [haveNewest] is true
+  /// because outbox messages are considered newer than regular messages.
+  ///
+  /// This does not call [notifyListeners].
+  void _syncOutboxMessagesFromStore() {
+    assert(haveNewest);
+    assert(outboxMessages.isEmpty);
+    for (final outboxMessage in store.outboxMessages.values) {
+      if (_shouldAddOutboxMessage(outboxMessage)) {
+        _addOutboxMessage(outboxMessage);
+      }
+    }
+  }
+
   /// Add [outboxMessage] if it belongs to the view.
   void addOutboxMessage(OutboxMessage outboxMessage) {
-    // TODO(#1441) implement this
+    // We don't have the newest messages;
+    // we shouldn't show any outbox messages until we do.
+    if (!haveNewest) return;
+
+    assert(outboxMessages.none(
+      (message) => message.localMessageId == outboxMessage.localMessageId));
+    if (_shouldAddOutboxMessage(outboxMessage)) {
+      _addOutboxMessage(outboxMessage);
+      notifyListeners();
+    }
   }
 
   /// Remove the [outboxMessage] from the view.
@@ -781,7 +944,9 @@ class MessageListView with ChangeNotifier, _MessageSequence {
   ///
   /// This should only be called from [MessageStore.takeOutboxMessage].
   void removeOutboxMessage(OutboxMessage outboxMessage) {
-    // TODO(#1441) implement this
+    if (_removeOutboxMessage(outboxMessage)) {
+      notifyListeners();
+    }
   }
 
   void handleUserTopicEvent(UserTopicEvent event) {
@@ -790,10 +955,17 @@ class MessageListView with ChangeNotifier, _MessageSequence {
         return;
 
       case VisibilityEffect.muted:
-        if (_removeMessagesWhere((message) =>
-            (message is StreamMessage
-             && message.streamId == event.streamId
-             && message.topic == event.topicName))) {
+        bool removed = _removeMessagesWhere((message) =>
+          message is StreamMessage
+            && message.streamId == event.streamId
+            && message.topic == event.topicName);
+
+        removed |= _removeOutboxMessagesWhere((message) =>
+          message is StreamOutboxMessage
+            && message.conversation.streamId == event.streamId
+            && message.conversation.topic == event.topicName);
+
+        if (removed) {
           notifyListeners();
         }
 
@@ -819,6 +991,8 @@ class MessageListView with ChangeNotifier, _MessageSequence {
   void handleMessageEvent(MessageEvent event) {
     final message = event.message;
     if (!narrow.containsMessage(message) || !_messageVisible(message)) {
+      assert(event.localMessageId == null || outboxMessages.none((message) =>
+        message.localMessageId == int.parse(event.localMessageId!, radix: 10)));
       return;
     }
     if (!haveNewest) {
@@ -833,8 +1007,20 @@ class MessageListView with ChangeNotifier, _MessageSequence {
       //   didn't include this message.
       return;
     }
-    // TODO insert in middle instead, when appropriate
+
+    // Remove the outbox messages temporarily.
+    // We'll add them back after the new message.
+    _removeOutboxMessageItems();
+    // TODO insert in middle of [messages] instead, when appropriate
     _addMessage(message);
+    if (event.localMessageId != null) {
+      final localMessageId = int.parse(event.localMessageId!, radix: 10);
+      // [outboxMessages] is expected to be short, so removing the corresponding
+      // outbox message and reprocessing them all in linear time is efficient.
+      outboxMessages.removeWhere(
+        (message) => message.localMessageId == localMessageId);
+    }
+    _reprocessOutboxMessages();
     notifyListeners();
   }
 
@@ -955,7 +1141,11 @@ class MessageListView with ChangeNotifier, _MessageSequence {
 
   /// Notify listeners if the given outbox message is present in this view.
   void notifyListenersIfOutboxMessagePresent(int localMessageId) {
-    // TODO(#1441) implement this
+    final isAnyPresent =
+      outboxMessages.any((message) => message.localMessageId == localMessageId);
+    if (isAnyPresent) {
+      notifyListeners();
+    }
   }
 
   /// Called when the app is reassembled during debugging, e.g. for hot reload.