wip message: Handle moved messages from UpdateMessageEvent

We already handle the case where only a message's content is edited.
This handles the case where messages are moved too, between topics
and/or channels.

TODO: tests.  There's a variety of kinds of test cases this needs,
  but here's one in particular that may not be apparent:

  If the `generation += 1` line is commented out, the message list
  has a race bug where a fetchOlder starts; we reset (because
  messages were moved into the narrow); and then the fetch returns
  and appends in the wrong spot.  Also a related race bug where it's
  fetchInitial, and it lacks the moved messages.  So there should be
  test cases exercising each of those situations, which would fail if
  that line gets commented out.

TODO for followup commits:

  * Update StreamMessage.displayRecipient.

    If we wanted, we could update it by looking up the new stream's
    name.  To enable MessageStoreImpl to do that, we'd pass it a
    StreamStore, the same way we do with Unreads.

    But in fact the only time displayRecipient is useful is when we
    don't have data on that stream.  So there's not much point in
    looking it up here.  Instead, make the field nullable, and just
    set it to null when the message is moved between streams.

  * Handle UpdateMessageEvent.propagateMode.  This probably means
    _MessageListPageState having its own notion of the current narrow,
    which is initialized with `widget.narrow` but can change.
    Then `_MessageListState._modelChanged` can check if the model's
    narrow has changed, and if so tell the page state to update.

    For comparison, see zulip-mobile's `src/events/doEventActionSideEffects.js`,
    or web.

Author: gnprice

lib/api/model/model.dart

@@ -480,7 +480,7 @@ sealed class Message {
   final int senderId;
   final String senderRealmStr;
   @JsonKey(name: 'subject')
-  final String topic;
+  String topic;
   // final List<string> submessages; // TODO handle
   final int timestamp;
   String get type;
@@ -577,7 +577,7 @@ class StreamMessage extends Message {
   String get type => 'stream';
 
   final String displayRecipient;
-  final int streamId;
+  int streamId;
 
   StreamMessage({
     required super.client,

lib/model/message.dart

@@ -178,22 +178,40 @@ class MessageStoreImpl with MessageStore {
       return;
     }
 
-    if (newStreamId == null
-        && MessageEditState.topicMoveWasResolveOrUnresolve(origTopic, newTopic)) {
-      // The topic was only resolved/unresolved.
-      // No change to the messages' editState.
-      return;
-    }
+    final wasResolveOrUnresolve = (newStreamId == null
+      && MessageEditState.topicMoveWasResolveOrUnresolve(origTopic, newTopic));
 
-    // TODO(#150): Handle message moves.  The views' recipient headers
-    //   may need updating, and consequently showSender too.
-    //   Currently only editState gets updated.
     for (final messageId in event.messageIds) {
       final message = messages[messageId];
       if (message == null) continue;
-      // Do not override the edited marker if the message has also been moved.
-      if (message.editState == MessageEditState.edited) continue;
-      message.editState = MessageEditState.moved;
+
+      if (message is! StreamMessage) {
+        assert(debugLog('Bad UpdateMessageEvent: stream/topic move on a DM')); // TODO(log)
+        continue;
+      }
+
+      if (newStreamId != null) {
+        message.streamId = newStreamId;
+        // TODO update [StreamMessage.displayRecipient]; doesn't usually
+        //   matter, because we only consult it when the stream is unknown
+      }
+
+      message.topic = newTopic;
+
+      if (!wasResolveOrUnresolve
+          && message.editState == MessageEditState.none) {
+        message.editState = MessageEditState.moved;
+      }
+    }
+
+    for (final view in _messageListViews) {
+      view.messagesMoved(
+        origStreamId: origStreamId,
+        newStreamId: newStreamId ?? origStreamId,
+        origTopic: origTopic,
+        newTopic: newTopic,
+        messageIds: event.messageIds,
+      );
     }
   }
 

lib/model/message_list.dart

@@ -65,6 +65,9 @@ class MessageListHistoryStartItem extends MessageListItem {
 ///
 /// This comprises much of the guts of [MessageListView].
 mixin _MessageSequence {
+  /// A sequence number for invalidating stale fetches.
+  int generation = 0;
+
   /// The messages.
   ///
   /// See also [contents] and [items].
@@ -192,6 +195,17 @@ mixin _MessageSequence {
     _reprocessAll();
   }
 
+  /// Reset all [_MessageSequence] data, and cancel any active fetches.
+  void _reset() {
+    generation += 1;
+    messages.clear();
+    _fetched = false;
+    _haveOldest = false;
+    _fetchingOlder = false;
+    contents.clear();
+    items.clear();
+  }
+
   /// Redo all computations from scratch, based on [messages].
   void _recompute() {
     assert(contents.length == messages.length);
@@ -396,12 +410,14 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     assert(!fetched && !haveOldest && !fetchingOlder);
     assert(messages.isEmpty && contents.isEmpty);
     // TODO schedule all this in another isolate
+    final generation = this.generation;
     final result = await getMessages(store.connection,
       narrow: narrow.apiEncode(),
       anchor: AnchorCode.newest,
       numBefore: kMessageListFetchBatchSize,
       numAfter: 0,
     );
+    if (this.generation > generation) return;
     store.reconcileMessages(result.messages);
     for (final message in result.messages) {
       if (_messageVisible(message)) {
@@ -424,13 +440,15 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     _updateEndMarkers();
     notifyListeners();
     try {
+      final generation = this.generation;
       final result = await getMessages(store.connection,
         narrow: narrow.apiEncode(),
         anchor: NumericAnchor(messages[0].id),
         includeAnchor: false,
         numBefore: kMessageListFetchBatchSize,
         numAfter: 0,
       );
+      if (this.generation > generation) return;
 
       if (result.messages.isNotEmpty
           && result.messages.last.id == messages[0].id) {
@@ -485,6 +503,72 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     }
   }
 
+  void _messagesMovedInternally(List<int> messageIds) {
+    for (final messageId in messageIds) {
+      if (-1 != _findMessageWithId(messageId)) {
+        _reprocessAll();
+        notifyListeners();
+        return;
+      }
+    }
+  }
+
+  void _messagesMovedIntoNarrow() {
+    // If there are some messages we don't have in [MessageStore], and they
+    // occur later than the messages we have here, then we just have to
+    // re-fetch from scratch.  That's always valid, so just do that always.
+    // TODO in cases where we do have data to do better, do better.
+    _reset();
+    notifyListeners();
+    fetchInitial();
+  }
+
+  void _messagesMovedFromNarrow(List<int> messageIds) {
+    if (_removeMessagesById(messageIds)) {
+      notifyListeners();
+    }
+  }
+
+  void messagesMoved({
+    required int origStreamId,
+    required int newStreamId,
+    required String origTopic,
+    required String newTopic,
+    required List<int> messageIds,
+  }) {
+    switch (narrow) {
+      case DmNarrow():
+        // DMs can't be moved (nor created by moves),
+        // so the messages weren't in this narrow and still aren't.
+        return;
+
+      case CombinedFeedNarrow():
+        // The messages were and remain in this narrow.
+        // TODO(#421): … except they may have become muted or not.
+        //   We'll handle that at the same time as we handle muting itself changing.
+        // Recipient headers, and downstream of those, may change, though.
+        _messagesMovedInternally(messageIds);
+
+      case StreamNarrow(:final streamId):
+        switch ((origStreamId == streamId, newStreamId == streamId)) {
+          case (false, false): return;
+          case (true,  true ): _messagesMovedInternally(messageIds);
+          case (false, true ): _messagesMovedIntoNarrow();
+          case (true,  false): _messagesMovedFromNarrow(messageIds);
+        }
+
+      case TopicNarrow(:final streamId, :final topic):
+        final oldMatch = (origStreamId == streamId && origTopic == topic);
+        final newMatch = (newStreamId == streamId && newTopic == topic);
+        switch ((oldMatch, newMatch)) {
+          case (false, false): return;
+          case (true,  true ): _messagesMovedInternally(messageIds);
+          case (false, true ): _messagesMovedIntoNarrow();
+          case (true,  false): _messagesMovedFromNarrow(messageIds); // TODO handle propagateMode
+        }
+    }
+  }
+
   // Repeal the `@protected` annotation that applies on the base implementation,
   // so we can call this method from [MessageStoreImpl].
   @override