Skip to content

Commit

Permalink
msglist: Set unreads.oldUnreadsMissing to false on batched mark-all-a…
Browse files Browse the repository at this point in the history
…s-read
  • Loading branch information
chrisbobbe committed Nov 15, 2023
1 parent e390080 commit 4e62e7a
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 7 deletions.
12 changes: 5 additions & 7 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 39 additions & 0 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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: {});
Expand All @@ -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 {
Expand Down

0 comments on commit 4e62e7a

Please sign in to comment.