msglist: Update narrow when propageMode is changeAll or changeLater.

We only implement navigation change for the TopicNarrow. For StreamNarrow,
propagateMode is practically unused.

See also:

  zulip/zulip-mobile#5251

Fixes: zulip#150.

Signed-off-by: Zixuan James Li <[email protected]>

Author: PIG208

lib/model/message.dart

@@ -152,6 +152,7 @@ class MessageStoreImpl with MessageStore {
     final newStreamId = event.newStreamId; // null if topic-only move
     final origTopic = event.origTopic;
     final newTopic = event.newTopic;
+    final propagateMode = event.propagateMode;
 
     if (origTopic == null) {
       // There was no move.
@@ -178,6 +179,11 @@ class MessageStoreImpl with MessageStore {
       assert(debugLog('Malformed UpdateMessageEvent: move but no origStreamId')); // TODO(log)
       return;
     }
+    if (propagateMode == null) {
+      // The `propagate_mode` field (aka propagateMode) is documented to be present on moves.
+      assert(debugLog('Malformed UpdateMessageEvent: move but no propagateMode')); // TODO(log)
+      return;
+    }
 
     final wasResolveOrUnresolve = (newStreamId == null && newTopic != null
       && MessageEditState.topicMoveWasResolveOrUnresolve(origTopic, newTopic));
@@ -215,6 +221,7 @@ class MessageStoreImpl with MessageStore {
         origTopic: origTopic,
         newTopic: newTopic ?? origTopic,
         messageIds: event.messageIds,
+        propagateMode: propagateMode,
       );
     }
   }

lib/model/message_list.dart

@@ -359,7 +359,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
   }
 
   final PerAccountStore store;
-  final Narrow narrow;
+  Narrow narrow;
 
   /// Whether [message] should actually appear in this message list,
   /// given that it does belong to the narrow.
@@ -527,8 +527,26 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     fetchInitial();
   }
 
-  void _messagesMovedFromNarrow(List<int> messageIds) {
+  void _handlePropagateMode(PropagateMode propagateMode, Narrow newNarrow) {
+    switch (propagateMode) {
+      case PropagateMode.changeAll:
+      case PropagateMode.changeLater:
+        narrow = newNarrow;
+        _reset();
+        fetchInitial();
+      case PropagateMode.changeOne:
+    }
+  }
+
+  void _messagesMovedFromNarrow(
+    List<int> messageIds,
+    PropagateMode propagateMode,
+    {TopicNarrow? newNarrow}
+  ) {
     if (_removeMessagesById(messageIds)) {
+      if (newNarrow != null) {
+        _handlePropagateMode(propagateMode, newNarrow);
+      }
       notifyListeners();
     }
   }
@@ -539,6 +557,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     required String origTopic,
     required String newTopic,
     required List<int> messageIds,
+    required PropagateMode propagateMode,
   }) {
     switch (narrow) {
       case DmNarrow():
@@ -558,7 +577,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
           case (false, false): return;
           case (true,  true ): _messagesMovedInternally(messageIds);
           case (false, true ): _messagesMovedIntoNarrow();
-          case (true,  false): _messagesMovedFromNarrow(messageIds);
+          case (true,  false): _messagesMovedFromNarrow(messageIds, propagateMode);
         }
 
       case TopicNarrow(:final streamId, :final topic):
@@ -568,7 +587,8 @@ class MessageListView with ChangeNotifier, _MessageSequence {
           case (false, false): return;
           case (true,  true ): return; // TODO(log) no-op move
           case (false, true ): _messagesMovedIntoNarrow();
-          case (true,  false): _messagesMovedFromNarrow(messageIds); // TODO handle propagateMode
+          case (true,  false): _messagesMovedFromNarrow(messageIds, propagateMode,
+            newNarrow: TopicNarrow(newStreamId, newTopic));
         }
     }
   }

lib/widgets/message_list.dart

@@ -81,6 +81,12 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
     super.initState();
   }
 
+  void _narrowChanged(Narrow newNarrow) {
+    setState(() {
+      narrow = newNarrow;
+    });
+  }
+
   @override
   Widget build(BuildContext context) {
     final store = PerAccountStoreWidget.of(context);
@@ -138,7 +144,7 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
               removeBottom: narrow is! CombinedFeedNarrow,
 
               child: Expanded(
-                child: MessageList(narrow: narrow))),
+                child: MessageList(narrow: narrow, onNarrowChanged: _narrowChanged))),
             ComposeBox(controllerKey: _composeBoxKey, narrow: narrow),
           ]))));
   }
@@ -214,9 +220,10 @@ const _kShortMessageHeight = 80;
 const kFetchMessagesBufferPixels = (kMessageListFetchBatchSize / 2) * _kShortMessageHeight;
 
 class MessageList extends StatefulWidget {
-  const MessageList({super.key, required this.narrow});
+  const MessageList({super.key, required this.narrow, required this.onNarrowChanged});
 
   final Narrow narrow;
+  final void Function(Narrow newNarrow) onNarrowChanged;
 
   @override
   State<StatefulWidget> createState() => _MessageListState();
@@ -253,6 +260,11 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
   }
 
   void _modelChanged() {
+    if (model!.narrow != widget.narrow) {
+      // A message move event occurred, where propagate mode is
+      // [PropagateMode.changeAll] or [PropagateMode.changeLater].
+      widget.onNarrowChanged(model!.narrow);
+    }
     setState(() {
       // The actual state lives in the [MessageListView] model.
       // This method was called because that just changed.

test/example_data.dart

@@ -471,6 +471,7 @@ UpdateMessageEvent _updateMessageMoveEvent(
   String? newTopic,
   String? origContent,
   String? newContent,
+  PropagateMode propagateMode = PropagateMode.changeOne,
 }) {
   assert(messages.isNotEmpty);
   assert(newTopic != origTopic || newStreamId != null);
@@ -486,7 +487,7 @@ UpdateMessageEvent _updateMessageMoveEvent(
     editTimestamp: 1234567890, // TODO generate timestamp
     origStreamId: origStreamId,
     newStreamId: newStreamId,
-    propagateMode: null,
+    propagateMode: propagateMode,
     origTopic: origTopic,
     newTopic: newTopic,
     origContent: origContent,
@@ -503,6 +504,7 @@ UpdateMessageEvent updateMessageEventMoveFrom({
   String? newTopic,
   int? newStreamId,
   String? newContent,
+  PropagateMode propagateMode = PropagateMode.changeOne,
 }) {
   assert(origMessages.isNotEmpty);
   final origMessage = origMessages.first;
@@ -515,6 +517,7 @@ UpdateMessageEvent updateMessageEventMoveFrom({
     newTopic: newTopic,
     origContent: effectiveOrigContent,
     newContent: newContent,
+    propagateMode: propagateMode,
   );
 }
 

test/flutter_checks.dart

@@ -60,8 +60,14 @@ extension TextChecks on Subject<Text> {
   Subject<TextStyle?> get style => has((t) => t.style, 'style');
 }
 
+extension TextEditingControllerChecks on Subject<TextEditingController> {
+  Subject<String?> get text => has((t) => t.text, 'text');
+}
+
 extension TextFieldChecks on Subject<TextField> {
   Subject<TextCapitalization?> get textCapitalization => has((t) => t.textCapitalization, 'textCapitalization');
+  Subject<InputDecoration?> get decoration => has((t) => t.decoration, 'decoration');
+  Subject<TextEditingController?> get controller => has((t) => t.controller, 'controller');
 }
 
 extension TextStyleChecks on Subject<TextStyle> {
@@ -131,3 +137,7 @@ extension MaterialChecks on Subject<Material> {
   Subject<Color?> get color => has((x) => x.color, 'color');
   // TODO more
 }
+
+extension InputDecorationChecks on Subject<InputDecoration> {
+  Subject<String?> get hintText => has((x) => x.hintText, 'hintText');
+}

test/model/message_list_test.dart

@@ -521,6 +521,31 @@ void main() {
         checkHasMessages(initialMessages);
         checkNotifiedOnce();
       });
+
+      void testMessageMove(PropagateMode propagateMode) => awaitFakeAsync((async) async {
+        await prepareNarrow(narrow, initialMessages + movedMessages);
+        connection.prepare(delay: const Duration(seconds: 1), json: newestResult(
+          foundOldest: false,
+          messages: movedMessages,
+        ).toJson());
+        await store.handleEvent(eg.updateMessageEventMoveFrom(
+          origMessages: movedMessages,
+          newTopic: 'new',
+          newStreamId: otherStream.streamId,
+          propagateMode: propagateMode,
+        ));
+        checkNotifiedOnce();
+        async.elapse(const Duration(seconds: 1));
+        checkHasMessages(movedMessages);
+        check(model).narrow.equals(TopicNarrow(otherStream.streamId, 'new'));
+        checkNotifiedOnce();
+      });
+
+      test('follow to the new narrow when propagateMode = changeLater', () =>
+        testMessageMove(PropagateMode.changeLater));
+
+      test('follow to the new narrow when propagateMode = changeAll', () =>
+        testMessageMove(PropagateMode.changeAll));
     });
 
     group('in stream narrow', () {
@@ -570,6 +595,31 @@ void main() {
         checkHasMessages(initialMessages);
         checkNotifiedOnce();
       });
+
+      void testMessageMove(PropagateMode propagateMode) => awaitFakeAsync((async) async {
+        await prepareNarrow(narrow, initialMessages + movedMessages);
+        connection.prepare(delay: const Duration(seconds: 1), json: newestResult(
+          foundOldest: false,
+          messages: movedMessages,
+        ).toJson());
+        await store.handleEvent(eg.updateMessageEventMoveFrom(
+          origMessages: movedMessages,
+          newTopic: 'new',
+          newStreamId: otherStream.streamId,
+          propagateMode: propagateMode,
+        ));
+        checkNotifiedOnce();
+        async.elapse(const Duration(seconds: 1));
+        checkHasMessages(initialMessages);
+        check(model).narrow.equals(StreamNarrow(stream.streamId));
+        checkNotNotified();
+      });
+
+      test('do not follow when propagateMode = changeLater', () =>
+        testMessageMove(PropagateMode.changeLater));
+
+      test('do not follow when propagateMode = changeAll', () =>
+        testMessageMove(PropagateMode.changeAll));
     });
 
     group('in combined feed narrow', () {

test/widgets/message_list_test.dart

@@ -14,6 +14,7 @@ import 'package:zulip/api/route/messages.dart';
 import 'package:zulip/model/localizations.dart';
 import 'package:zulip/model/narrow.dart';
 import 'package:zulip/model/store.dart';
+import 'package:zulip/widgets/autocomplete.dart';
 import 'package:zulip/widgets/content.dart';
 import 'package:zulip/widgets/emoji_reaction.dart';
 import 'package:zulip/widgets/icons.dart';
@@ -1032,4 +1033,84 @@ void main() {
       });
     });
   });
+
+  group('Update Narrow on message move', () {
+    final channel = eg.stream(name: 'move test stream');
+    final message = eg.streamMessage(stream: channel, content: 'Message to move');
+    final narrow = TopicNarrow.ofMessage(message);
+
+    void prepareGetMessageResponse(List<Message> messages) {
+      connection.prepare(json: newestResult(
+        foundOldest: false, messages: messages).toJson());
+    }
+
+    void handleMessageMoveEvent(List<StreamMessage> messages, String newTopic) {
+      store.handleEvent(eg.updateMessageEventMoveFrom(
+        origMessages: messages,
+        newTopic: newTopic,
+        propagateMode: PropagateMode.changeAll));
+    }
+
+    testWidgets('compose box send message after move', (WidgetTester tester) async {
+      await setupMessageListPage(tester, narrow: narrow, messages: [message], streams: [channel]);
+
+      final channelContentInputFinder = find.descendant(
+        of: find.byType(ComposeAutocomplete),
+        matching: find.byType(TextField));
+
+      await tester.enterText(channelContentInputFinder, 'Some text');
+      prepareGetMessageResponse([message]);
+      handleMessageMoveEvent([message], 'new topic');
+      await tester.pump(const Duration(seconds: 1));
+      check(tester.widget<TextField>(channelContentInputFinder))
+        ..decoration.isNotNull().hintText.equals('Message #${channel.name} > new topic')
+        ..controller.isNotNull().text.equals('Some text');
+
+      connection.prepare(json: SendMessageResult(id: 1).toJson());
+      await tester.tap(find.byIcon(Icons.send));
+      await tester.pump();
+      check(connection.lastRequest).isA<http.Request>()
+        ..method.equals('POST')
+        ..url.path.equals('/api/v1/messages')
+        ..bodyFields.deepEquals({
+          'type': 'stream',
+          'to': '${message.streamId}',
+          'topic': 'new topic',
+          'content': 'Some text',
+          'read_by_sender': 'true'});
+      await tester.pumpAndSettle();
+    });
+
+    testWidgets('Move to narrow with existing messages', (WidgetTester tester) async {
+      await setupMessageListPage(tester, narrow: narrow, messages: [message], streams: [channel]);
+      check(find.textContaining('Existing message').evaluate()).length.equals(0);
+      check(find.textContaining('Message to move').evaluate()).length.equals(1);
+
+      final existingMessage = eg.streamMessage(
+        stream: eg.stream(), topic: 'new topic', content: 'Existing message');
+      prepareGetMessageResponse([existingMessage, message]);
+      handleMessageMoveEvent([message], 'new topic');
+      await tester.pump(const Duration(seconds: 1));
+
+      check(find.textContaining('Existing message').evaluate()).length.equals(1);
+      check(find.textContaining('Message to move').evaluate()).length.equals(1);
+    });
+
+    testWidgets('show new topic in TopicNarrow after move', (tester) async {
+      await setupMessageListPage(tester, narrow: narrow, messages: [message], streams: [channel]);
+
+      prepareGetMessageResponse([message]);
+      handleMessageMoveEvent([message], 'new topic');
+      await tester.pump(const Duration(seconds: 1));
+
+      check(find.descendant(
+        of: find.byType(RecipientHeader),
+        matching: find.text('new topic')).evaluate()
+      ).length.equals(1);
+      check(find.descendant(
+        of: find.byType(MessageListAppBarTitle),
+        matching: find.text('${channel.name} > new topic')).evaluate()
+      ).length.equals(1);
+    });
+  });
 }