From 4e62e7a0f7bf2e2bbbef58af7face82952c4ba3a Mon Sep 17 00:00:00 2001
From: Chris Bobbe <cbobbe@zulip.com>
Date: Sat, 11 Nov 2023 15:39:43 -0500
Subject: [PATCH] msglist: Set unreads.oldUnreadsMissing to false on batched
 mark-all-as-read

---
 lib/widgets/message_list.dart       | 12 ++++-----
 test/widgets/message_list_test.dart | 39 +++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart
index 004b0cc280..a61499d955 100644
--- a/lib/widgets/message_list.dart
+++ b/lib/widgets/message_list.dart
@@ -371,14 +371,12 @@ class MarkAsReadWidget extends StatelessWidget {
       await showErrorDialog(context: context,
         title: zulipLocalizations.errorMarkAsReadFailedTitle,
         message: e.toString());
+      return;
+    }
+    if (!context.mounted) return;
+    if (narrow is AllMessagesNarrow && !useLegacy) {
+      PerAccountStoreWidget.of(context).unreads.handleAllMessagesReadSuccess();
     }
-    // TODO: clear Unreads.oldUnreadsMissing when `narrow` is [AllMessagesNarrow]
-    //   In the rare case that the user had more than 50K total unreads
-    //   on the server, the client won't have known about all of them;
-    //   this was communicated to the client via `oldUnreadsMissing`.
-    //
-    //   However, since we successfully marked **everything** as read,
-    //   we know that we now have a correct data set of unreads.
   }
 
   @override
diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart
index 1de30a719c..e6788c7afe 100644
--- a/test/widgets/message_list_test.dart
+++ b/test/widgets/message_list_test.dart
@@ -24,6 +24,7 @@ import '../model/binding.dart';
 import '../model/message_list_test.dart';
 import '../model/test_store.dart';
 import '../flutter_checks.dart';
+import '../model/unreads_checks.dart';
 import '../stdlib_checks.dart';
 import '../test_images.dart';
 import 'content_checks.dart';
@@ -570,6 +571,23 @@ void main() {
             });
       });
 
+      testWidgets('markNarrowAsRead on mark-all-as-read when Unreads.oldUnreadsMissing: true', (tester) async {
+        const narrow = AllMessagesNarrow();
+        await setupMessageListPage(tester,
+          narrow: narrow, messages: [message], unreadMsgs: unreadMsgs);
+        check(isMarkAsReadButtonVisible(tester)).isTrue();
+        store.unreads.oldUnreadsMissing = true;
+
+        final connection = store.connection as FakeApiConnection;
+        connection.prepare(json: UpdateMessageFlagsForNarrowResult(
+          processedCount: 11, updatedCount: 3,
+          firstProcessedId: null, lastProcessedId: null,
+          foundOldest: true, foundNewest: true).toJson());
+        await tester.tap(find.byType(MarkAsReadWidget));
+        await tester.pumpAndSettle();
+        check(store.unreads.oldUnreadsMissing).isFalse();
+      });
+
       testWidgets('markNarrowAsRead on invalid response', (WidgetTester tester) async {
         final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
         final narrow = TopicNarrow.ofMessage(message);
@@ -608,6 +626,19 @@ void main() {
           narrow: narrow, messages: [message], unreadMsgs: unreadMsgs);
         check(isMarkAsReadButtonVisible(tester)).isTrue();
 
+        int unreadsNotifiedCount = 0;
+        final unreadsModel = store.unreads;
+        unreadsModel.addListener(() {
+          unreadsNotifiedCount++;
+        });
+        // Might as well test with oldUnreadsMissing: true.
+        //
+        // We didn't fill the model with 50k unreads, so this is questionably
+        // realistic… but the 50k cap isn't actually API-guaranteed, and this is
+        // plausibly realistic for a hypothetical server that decides based on
+        // message age rather than the 50k cap.
+        unreadsModel.oldUnreadsMissing = true;
+
         final connection = store.connection as FakeApiConnection;
         connection.zulipFeatureLevel = 154;
         connection.prepare(json: {});
@@ -618,6 +649,14 @@ void main() {
           ..bodyFields.deepEquals({});
 
         await tester.pumpAndSettle(); // process pending timers
+
+        // Didn't redundantly call [Unreads.handleAllMessagesReadSuccess], which
+        // is meant only for the modern protocol and would be redundant in the
+        // legacy protocol. (In the legacy protocol, we set oldUnreadsMissing to
+        // false when an event comes, not when the mark-as-read request
+        // succeeds.)
+        check(unreadsNotifiedCount).equals(0);
+        check(unreadsModel).oldUnreadsMissing.isTrue();
       });
 
       testWidgets('StreamNarrow on legacy server', (WidgetTester tester) async {