action_sheet: Mark as unread in current narrow, vs. from when action …

…sheet opened

The narrow of a given MessageListPage can change over time,
in particular if viewing a topic that gets moved.

If it does, the mark-unread action should follow along.  Otherwise
we'd have a frustrating race where mark-unread just didn't work
if, for example, someone happened to resolve the topic while you
were in the middle of trying to mark as unread.

Fixes: zulip#1045

lib/widgets/action_sheet.dart

@@ -29,11 +29,12 @@ void showMessageActionSheet({required BuildContext context, required Message mes
 
   // The UI that's conditioned on this won't live-update during this appearance
   // of the action sheet (we avoid calling composeBoxControllerOf in a build
-  // method; see its doc). But currently it will be constant through the life of
-  // any message list, so that's fine.
+  // method; see its doc).
+  // So we rely on the fact that isComposeBoxOffered for any given message list
+  // will be constant through the page's life.
   final messageListPage = MessageListPage.ancestorOf(context);
   final isComposeBoxOffered = messageListPage.composeBoxController != null;
-  final narrow = messageListPage.narrow;
+
   final isMessageRead = message.flags.contains(MessageFlag.read);
   final markAsUnreadSupported = store.connection.zulipFeatureLevel! >= 155; // TODO(server-6)
   final showMarkAsUnreadButton = markAsUnreadSupported && isMessageRead;
@@ -52,7 +53,7 @@ void showMessageActionSheet({required BuildContext context, required Message mes
     if (isComposeBoxOffered)
       QuoteAndReplyButton(message: message, pageContext: context),
     if (showMarkAsUnreadButton)
-      MarkAsUnreadButton(message: message, pageContext: context, narrow: narrow),
+      MarkAsUnreadButton(message: message, pageContext: context),
     CopyMessageTextButton(message: message, pageContext: context),
     CopyMessageLinkButton(message: message, pageContext: context),
     ShareButton(message: message, pageContext: context),
@@ -383,11 +384,8 @@ class MarkAsUnreadButton extends MessageActionSheetMenuItemButton {
     super.key,
     required super.message,
     required super.pageContext,
-    required this.narrow,
   });
 
-  final Narrow narrow;
-
   @override IconData get icon => Icons.mark_chat_unread_outlined;
 
   @override
@@ -396,6 +394,7 @@ class MarkAsUnreadButton extends MessageActionSheetMenuItemButton {
   }
 
   @override void onPressed() async {
+    final narrow = findMessageListPage().narrow;
     unawaited(markNarrowAsUnreadFromMessage(pageContext, message, narrow));
   }
 }

test/widgets/action_sheet_test.dart

@@ -5,6 +5,7 @@ import 'package:flutter/material.dart';
 import 'package:flutter/services.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:http/http.dart' as http;
+import 'package:zulip/api/model/events.dart';
 import 'package:zulip/api/model/model.dart';
 import 'package:zulip/api/route/channels.dart';
 import 'package:zulip/api/route/messages.dart';
@@ -438,6 +439,49 @@ void main() {
             });
       });
 
+      testWidgets('on topic move, acts on new topic', (tester) async {
+        final stream = eg.stream();
+        const topic = 'old topic';
+        final message = eg.streamMessage(flags: [MessageFlag.read],
+          stream: stream, topic: topic);
+        await setupToMessageActionSheet(tester, message: message,
+          narrow: TopicNarrow.ofMessage(message));
+
+        // Get the action sheet fully deployed while the old narrow applies.
+        // (This way we maximize the range of potential bugs this test can catch,
+        // by giving the code maximum opportunity to latch onto the old topic.)
+        await tester.pumpAndSettle();
+
+        final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
+        final newStream = eg.stream();
+        const newTopic = 'other topic';
+        // This result isn't quite realistic for this request: it should get
+        // the updated channel/stream ID and topic, because we don't even
+        // start the request until after we get the move event.
+        // But constructing the right result is annoying at the moment, and
+        // it doesn't matter anyway: [MessageStoreImpl.reconcileMessages] will
+        // keep the version updated by the event.  If that somehow changes in
+        // some future refactor, it'll cause this test to fail.
+        connection.prepare(json: newestResult(
+          foundOldest: true, messages: [message]).toJson());
+        await store.handleEvent(eg.updateMessageEventMoveFrom(
+          newStreamId: newStream.streamId, newTopic: newTopic,
+          propagateMode: PropagateMode.changeAll,
+          origMessages: [message]));
+
+        connection.prepare(json: UpdateMessageFlagsForNarrowResult(
+          processedCount: 11, updatedCount: 3,
+          firstProcessedId: 1, lastProcessedId: 1980,
+          foundOldest: true, foundNewest: true).toJson());
+        await tester.tap(find.byIcon(Icons.mark_chat_unread_outlined, skipOffstage: false));
+        await tester.pumpAndSettle();
+        check(connection.lastRequest).isA<http.Request>()
+          ..method.equals('POST')
+          ..url.path.equals('/api/v1/messages/flags/narrow')
+          ..bodyFields['narrow'].equals(
+              jsonEncode(TopicNarrow(newStream.streamId, newTopic).apiEncode()));
+      });
+
       testWidgets('shows error when fails', (tester) async {
         final message = eg.streamMessage(flags: [MessageFlag.read]);
         await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message));