message: Mutate messages for flags events here, instead of in msglists

Like a recent commit did for message edits, and one before that did
for reactions, this fixes the part of 455 that's concerned with
update-message-flags events.

Like in the commit that dealt with message edits, this fixes the
symptom of a dropped update when there are zero message lists open.
In this case I'm *pretty* sure there wasn't a user-facing symptom
with the double-processing situation when the message is in two or
more open message lists. That's because I believe we don't give
different behavior for a flag that's present multiple times in the
`flags` list, vs. just once.

Author: chrisbobbe

lib/model/message.dart

@@ -133,8 +133,36 @@ class MessageStoreImpl with MessageStore {
   }
 
   void handleUpdateMessageFlagsEvent(UpdateMessageFlagsEvent event) {
-    for (final view in _messageListViews) {
-      view.handleUpdateMessageFlagsEvent(event); // TODO update mainly in [messages] instead
+    final isAdd = switch (event) {
+      UpdateMessageFlagsAddEvent()    => true,
+      UpdateMessageFlagsRemoveEvent() => false,
+    };
+
+    if (isAdd && (event as UpdateMessageFlagsAddEvent).all) {
+      for (final message in messages.values) {
+        message.flags.add(event.flag);
+      }
+
+      for (final view in _messageListViews) {
+        if (view.messages.isEmpty) continue;
+        view.notifyListeners();
+      }
+    } else {
+      bool knowAboutAny = false;
+      for (final messageId in event.messages) {
+        final message = messages[messageId];
+        if (message == null) continue; // a message we don't know about yet
+        knowAboutAny = true;
+
+        isAdd
+          ? message.flags.add(event.flag)
+          : message.flags.remove(event.flag);
+      }
+      if (knowAboutAny) {
+        for (final view in _messageListViews) {
+          view.notifyListenersIfAnyMessagePresent(event.messages);
+        }
+      }
     }
   }
 

lib/model/message_list.dart

@@ -444,6 +444,15 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     notifyListeners();
   }
 
+  /// Calls [super.notifyListeners].
+  ///
+  /// Overridden here without annotations that would prevent us from calling it
+  /// from [MessageStoreImpl].
+  @override
+  void notifyListeners() {
+    super.notifyListeners();
+  }
+
   /// Notify listeners if the given message is present in this view.
   void notifyListenersIfMessagePresent(int messageId) {
     final index = _findMessageWithId(messageId);
@@ -452,6 +461,15 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     }
   }
 
+  /// Notify listeners if any of the given messages is present in this view.
+  void notifyListenersIfAnyMessagePresent(Iterable<int> messageIds) {
+    final isAnyPresent = messageIds
+      .any((messageId) => _findMessageWithId(messageId) != -1);
+    if (isAnyPresent) {
+      notifyListeners();
+    }
+  }
+
   void messageContentChanged(int messageId) {
     final index = _findMessageWithId(messageId);
     if (index != -1) {
@@ -460,35 +478,6 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     }
   }
 
-  void handleUpdateMessageFlagsEvent(UpdateMessageFlagsEvent event) {
-    final isAdd = switch (event) {
-      UpdateMessageFlagsAddEvent()    => true,
-      UpdateMessageFlagsRemoveEvent() => false,
-    };
-
-    bool didUpdateAny = false;
-    if (isAdd && (event as UpdateMessageFlagsAddEvent).all) {
-      for (final message in messages) {
-        message.flags.add(event.flag);
-        didUpdateAny = true;
-      }
-    } else {
-      for (final messageId in event.messages) {
-        final index = _findMessageWithId(messageId);
-        if (index != -1) {
-          final message = messages[index];
-          isAdd ? message.flags.add(event.flag) : message.flags.remove(event.flag);
-          didUpdateAny = true;
-        }
-      }
-    }
-    if (!didUpdateAny) {
-      return;
-    }
-
-    notifyListeners();
-  }
-
   /// Called when the app is reassembled during debugging, e.g. for hot reload.
   ///
   /// This will redo from scratch any computations we can, such as parsing

test/model/message_list_test.dart

@@ -306,6 +306,33 @@ void main() {
     });
   });
 
+  group('notifyListenersIfAnyMessagePresent', () {
+    final messages = List.generate(100, (i) => eg.streamMessage(id: 100 + i));
+
+    test('all messages present', () async {
+      await prepare(narrow: const CombinedFeedNarrow());
+      await prepareMessages(foundOldest: false, messages: messages);
+      model.notifyListenersIfAnyMessagePresent([150, 151, 152]);
+      checkNotifiedOnce();
+    });
+
+    test('some messages present', () async {
+      await prepare(narrow: const CombinedFeedNarrow());
+      await prepareMessages(foundOldest: false,
+        messages: messages.where((m) => m.id != 151).toList());
+      model.notifyListenersIfAnyMessagePresent([150, 151, 152]);
+      checkNotifiedOnce();
+    });
+
+    test('no messages present', () async {
+      await prepare(narrow: const CombinedFeedNarrow());
+      await prepareMessages(foundOldest: false, messages:
+        messages.where((m) => ![150, 151, 152].contains(m.id)).toList());
+      model.notifyListenersIfAnyMessagePresent([150, 151, 152]);
+      checkNotNotified();
+    });
+  });
+
   group('messageContentChanged', () {
     test('message present', () async {
       await prepare(narrow: const CombinedFeedNarrow());
@@ -428,7 +455,18 @@ void main() {
       );
     });
 
-    // TODO(#455) message flags
+    test('UpdateMessageFlagsEvent is applied even when message not in any msglists', () async {
+      await checkApplied(
+        mkEvent: (message) => UpdateMessageFlagsAddEvent(
+          id: 1,
+          flag: MessageFlag.starred,
+          messages: [message.id],
+          all: false,
+        ),
+        doCheckMessageAfterFetch:
+          (messageSubject) => messageSubject.flags.contains(MessageFlag.starred),
+      );
+    });
   });
 
   test('reassemble', () async {