message: Mutate messages for message edits here, instead of in msglists

Like a recent commit did for reactions, this fixes the part of 455
that's concerned with message edits.

With message edits, unlike reactions, the double-processing issue
when there are two or more message lists wasn't user-facing, since
the mutation done by message lists was (naturally) idempotent. So
there isn't really an easy or helpful regression test to write for
that. But we can and do write one against the dropped-update symptom
when there are zero message lists open. For an explanation of that
symptom, see the recent commit where we dealt with reactions.

Co-authored-by: Chris Bobbe <[email protected]>
Fixes-partly: #455

Author: gnprice

lib/model/message.dart

@@ -93,8 +93,38 @@ class MessageStoreImpl with MessageStore {
   }
 
   void handleUpdateMessageEvent(UpdateMessageEvent event) {
+    _handleUpdateMessageEventContent(event);
+    // TODO(#150): Handle message moves.  The views' recipient headers
+    //   may need updating, and consequently showSender too.
+  }
+
+  void _handleUpdateMessageEventContent(UpdateMessageEvent event) {
+    final message = messages[event.messageId];
+    if (message == null) return;
+
+    // TODO(server-5): Cut this fallback; rely on renderingOnly from FL 114
+    final isRenderingOnly = event.renderingOnly ?? (event.userId == null);
+    if (event.editTimestamp != null && !isRenderingOnly) {
+      // A rendering-only update gets omitted from the message edit history,
+      // and [Message.lastEditTimestamp] is the last timestamp of that history.
+      // So on a rendering-only update, the timestamp doesn't get updated.
+      message.lastEditTimestamp = event.editTimestamp;
+    }
+
+    message.flags = event.flags;
+
+    if (event.renderedContent != null) {
+      assert(message.contentType == 'text/html',
+        "Message contentType was ${message.contentType}; expected text/html.");
+      message.content = event.renderedContent!;
+    }
+
+    if (event.isMeMessage != null) {
+      message.isMeMessage = event.isMeMessage!;
+    }
+
     for (final view in _messageListViews) {
-      view.handleUpdateMessageEvent(event); // TODO update mainly in [messages] instead
+      view.messageContentChanged(event.messageId);
     }
   }
 

lib/model/message_list.dart

@@ -452,46 +452,12 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     }
   }
 
-  static void _applyChangesToMessage(UpdateMessageEvent event, Message message) {
-    // TODO(server-5): Cut this fallback; rely on renderingOnly from FL 114
-    final isRenderingOnly = event.renderingOnly ?? (event.userId == null);
-    if (event.editTimestamp != null && !isRenderingOnly) {
-      // A rendering-only update gets omitted from the message edit history,
-      // and [Message.lastEditTimestamp] is the last timestamp of that history.
-      // So on a rendering-only update, the timestamp doesn't get updated.
-      message.lastEditTimestamp = event.editTimestamp;
-    }
-
-    message.flags = event.flags;
-
-    if (event.renderedContent != null) {
-      assert(message.contentType == 'text/html',
-        "Message contentType was ${message.contentType}; expected text/html.");
-      message.content = event.renderedContent!;
-    }
-
-    if (event.isMeMessage != null) {
-      message.isMeMessage = event.isMeMessage!;
-    }
-  }
-
-  /// Update the message the given event applies to, if present in this view.
-  ///
-  /// This method only handles the case where the message's contents
-  /// were changed, and ignores any changes to its stream or topic.
-  ///
-  /// TODO(#150): Handle message moves.
-  // NB that when handling message moves (#150), recipient headers
-  // may need updating, and consequently showSender too.
-  void handleUpdateMessageEvent(UpdateMessageEvent event) {
-    final idx = _findMessageWithId(event.messageId);
-    if (idx == -1)  {
-      return;
+  void messageContentChanged(int messageId) {
+    final index = _findMessageWithId(messageId);
+    if (index != -1) {
+      _reparseContent(index);
+      notifyListeners();
     }
-
-    _applyChangesToMessage(event, messages[idx]);
-    _reparseContent(idx);
-    notifyListeners();
   }
 
   void maybeUpdateMessageFlags(UpdateMessageFlagsEvent event) {

test/model/message_list_test.dart

@@ -297,6 +297,39 @@ void main() {
     });
   });
 
+  group('messageContentChanged', () {
+    test('message present', () async {
+      await prepare(narrow: const CombinedFeedNarrow());
+      await prepareMessages(foundOldest: false,
+        messages: List.generate(10, (i) => eg.streamMessage(id: 10 + i)));
+
+      final message = model.messages[5];
+      await store.handleEvent(eg.updateMessageEditEvent(message,
+        renderedContent: '${message.content}<p>edited</p'));
+      checkNotifiedOnce();
+    });
+
+    test('message absent', () async {
+      final stream = eg.stream();
+      final narrow = StreamNarrow(stream.streamId);
+      await prepare(narrow: narrow);
+
+      final messagesInNarrow = List<Message>.generate(10,
+        (i) => eg.streamMessage(id: 10 + i, stream: stream));
+      check(messagesInNarrow.every(narrow.containsMessage)).isTrue();
+
+      final messageNotInNarrow = eg.dmMessage(id: 100, from: eg.otherUser, to: [eg.selfUser]);
+      check(narrow.containsMessage(messageNotInNarrow)).isFalse();
+
+      await prepareMessages(foundOldest: false, messages: messagesInNarrow);
+      await store.addMessage(messageNotInNarrow);
+
+      await store.handleEvent(eg.updateMessageEditEvent(messageNotInNarrow,
+        renderedContent: '${messageNotInNarrow.content}<p>edited</p'));
+      checkNotNotified();
+    });
+  });
+
   group('maybeUpdateMessageFlags', () {
     UpdateMessageFlagsAddEvent mkAddEvent(
       MessageFlag flag,
@@ -476,7 +509,17 @@ void main() {
           doCheckMessageAfterFetch:
             (messageSubject) => messageSubject.reactions.isNotNull().total.equals(1),
         ),
-        // TODO(#455) message edits; message flags
+        (
+          eventName: 'UpdateMessageEvent (edit)',
+          mkEvent: (message) {
+            final newContent = '${message.content}<p>edited</p>';
+            return eg.updateMessageEditEvent(message,
+              renderedContent: newContent);
+          },
+          doCheckMessageAfterFetch:
+            (messageSubject) => messageSubject.content.endsWith('<p>edited</p>'),
+        ),
+        // TODO(#455) message flags
     ]) {
       test('$eventName is applied even when message not in any msglists', () async {
         final stream = eg.stream();