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.

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.

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

@@ -97,8 +97,12 @@ class MessageStoreImpl with MessageStore {
     assert(event.messageIds.contains(event.messageId), "See https://github.com/zulip/zulip-flutter/pull/753#discussion_r1649463633");
     _handleUpdateMessageEventTimestamp(event);
     _handleUpdateMessageEventContent(event);
-    _handleUpdateMessageEventMove(event);
+    final viewsWithMovedMessages = _handleUpdateMessageEventMove(event);
     for (final view in _messageListViews) {
+      if (viewsWithMovedMessages.contains(view)) {
+        view.notifyListeners();
+        continue;
+      }
       view.notifyListenersIfAnyMessagePresent(event.messageIds);
     }
   }
@@ -144,7 +148,7 @@ class MessageStoreImpl with MessageStore {
     }
   }
 
-  void _handleUpdateMessageEventMove(UpdateMessageEvent event) {
+  Set<MessageListView> _handleUpdateMessageEventMove(UpdateMessageEvent event) {
     // The interaction between the fields of these events are a bit tricky.
     // For reference, see: https://zulip.com/api/get-events#update_message
 
@@ -164,37 +168,60 @@ class MessageStoreImpl with MessageStore {
         }
         return true;
       }());
-      return;
+      return {};
     }
 
     if (newTopic == null) {
       // The `subject` field (aka newTopic) is documented to be present on moves.
       assert(debugLog('Malformed UpdateMessageEvent: move but no newTopic')); // TODO(log)
-      return;
+      return {};
     }
     if (origStreamId == null) {
       // The `stream_id` field (aka origStreamId) is documented to be present on moves.
       assert(debugLog('Malformed UpdateMessageEvent: move but no origStreamId')); // TODO(log)
-      return;
+      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;
+      }
+    }
+
+
+    final viewsWithMovedMessages = <MessageListView>{};
+    for (final view in _messageListViews) {
+      if (view.messagesMoved(
+        origStreamId: origStreamId,
+        newStreamId: newStreamId ?? origStreamId,
+        origTopic: origTopic,
+        newTopic: newTopic,
+        messageIds: event.messageIds,
+      )) {
+        viewsWithMovedMessages.add(view);
+      }
     }
+    return viewsWithMovedMessages;
   }
 
   void handleDeleteMessageEvent(DeleteMessageEvent event) {

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)) {
@@ -423,6 +439,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     _fetchingOlder = true;
     _updateEndMarkers();
     notifyListeners();
+    final generation = this.generation;
     try {
       final result = await getMessages(store.connection,
         narrow: narrow.apiEncode(),
@@ -431,6 +448,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
         numBefore: kMessageListFetchBatchSize,
         numAfter: 0,
       );
+      if (this.generation > generation) return;
 
       if (result.messages.isNotEmpty
           && result.messages.last.id == messages[0].id) {
@@ -448,8 +466,10 @@ class MessageListView with ChangeNotifier, _MessageSequence {
       _haveOldest = result.foundOldest;
     } finally {
       _fetchingOlder = false;
-      _updateEndMarkers();
-      notifyListeners();
+      if (this.generation == generation) {
+        _updateEndMarkers();
+        notifyListeners();
+      }
     }
   }
 
@@ -485,6 +505,70 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     }
   }
 
+  bool _messagesMovedInternally(List<int> messageIds) {
+    for (final messageId in messageIds) {
+      if (_findMessageWithId(messageId) != -1) {
+        _reprocessAll();
+        return true;
+      }
+    }
+    return false;
+  }
+
+  bool _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();
+    fetchInitial();
+    return true;
+  }
+
+  bool _messagesMovedFromNarrow(List<int> messageIds) {
+    return _removeMessagesById(messageIds);
+  }
+
+  bool 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 false;
+
+      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.
+        return _messagesMovedInternally(messageIds);
+
+      case StreamNarrow(:final streamId):
+        switch ((origStreamId == streamId, newStreamId == streamId)) {
+          case (false, false): return false;
+          case (true,  true ): return _messagesMovedInternally(messageIds);
+          case (false, true ): return _messagesMovedIntoNarrow();
+          case (true,  false): return _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 false;
+          case (true,  true ): return _messagesMovedInternally(messageIds);
+          case (false, true ): return _messagesMovedIntoNarrow();
+          case (true,  false): return _messagesMovedFromNarrow(messageIds); // TODO handle propagateMode
+        }
+    }
+  }
+
   // Repeal the `@protected` annotation that applies on the base implementation,
   // so we can call this method from [MessageStoreImpl].
   @override