Skip to content

Commit 3cd2b83

Browse files
committed
actions [nfc]: Move markNarrowAsRead into new ZulipAction class for clearer UI-level operation semantics
1 parent e01cae1 commit 3cd2b83

File tree

4 files changed

+60
-50
lines changed

4 files changed

+60
-50
lines changed

lib/widgets/action_sheet.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ class MarkTopicAsReadButton extends ActionSheetMenuItemButton {
400400

401401
@override void onPressed() async {
402402
if (!pageContext.mounted) return;
403-
await markNarrowAsRead(pageContext, TopicNarrow(channelId, topic));
403+
await ZulipAction.markNarrowAsRead(pageContext, TopicNarrow(channelId, topic));
404404
}
405405
}
406406

lib/widgets/actions.dart

Lines changed: 50 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -53,49 +53,59 @@ Future<void> unregisterToken(GlobalStore globalStore, int accountId) async {
5353
}
5454
}
5555

56-
Future<void> markNarrowAsRead(BuildContext context, Narrow narrow) async {
57-
final store = PerAccountStoreWidget.of(context);
58-
final connection = store.connection;
59-
final zulipLocalizations = ZulipLocalizations.of(context);
60-
final useLegacy = connection.zulipFeatureLevel! < 155; // TODO(server-6)
61-
if (useLegacy) {
62-
try {
63-
await _legacyMarkNarrowAsRead(context, narrow);
64-
return;
65-
} catch (e) {
66-
if (!context.mounted) return;
67-
showErrorDialog(context: context,
68-
title: zulipLocalizations.errorMarkAsReadFailedTitle,
69-
message: e.toString()); // TODO(#741): extract user-facing message better
70-
return;
56+
/// High-level operations that interact with the Zulip API and handle UI feedback.
57+
///
58+
/// Methods in this class show progress and error feedback in the UI,
59+
/// and handle error cases internally rather than expecting the caller to handle them.
60+
abstract final class ZulipAction {
61+
/// Marks all messages in the given [narrow] as read, showing feedback in the UI.
62+
///
63+
/// Shows progress feedback for long-running operations, and error feedback
64+
/// if the operation fails. Updates unread counts in the UI on success.
65+
static Future<void> markNarrowAsRead(BuildContext context, Narrow narrow) async {
66+
final store = PerAccountStoreWidget.of(context);
67+
final connection = store.connection;
68+
final zulipLocalizations = ZulipLocalizations.of(context);
69+
final useLegacy = connection.zulipFeatureLevel! < 155; // TODO(server-6)
70+
if (useLegacy) {
71+
try {
72+
await _legacyMarkNarrowAsRead(context, narrow);
73+
return;
74+
} catch (e) {
75+
if (!context.mounted) return;
76+
showErrorDialog(context: context,
77+
title: zulipLocalizations.errorMarkAsReadFailedTitle,
78+
message: e.toString()); // TODO(#741): extract user-facing message better
79+
return;
80+
}
7181
}
72-
}
7382

74-
final didPass = await updateMessageFlagsStartingFromAnchor(
75-
context: context,
76-
// Include `is:unread` in the narrow. That has a database index, so
77-
// this can be an important optimization in narrows with a lot of history.
78-
// The server applies the same optimization within the (deprecated)
79-
// specialized endpoints for marking messages as read; see
80-
// `do_mark_stream_messages_as_read` in `zulip:zerver/actions/message_flags.py`.
81-
apiNarrow: narrow.apiEncode()..add(ApiNarrowIs(IsOperand.unread)),
82-
// Use [AnchorCode.oldest], because [AnchorCode.firstUnread]
83-
// will be the oldest non-muted unread message, which would
84-
// result in muted unreads older than the first unread not
85-
// being processed.
86-
anchor: AnchorCode.oldest,
87-
// [AnchorCode.oldest] is an anchor ID lower than any valid
88-
// message ID.
89-
includeAnchor: false,
90-
op: UpdateMessageFlagsOp.add,
91-
flag: MessageFlag.read,
92-
onCompletedMessage: zulipLocalizations.markAsReadComplete,
93-
progressMessage: zulipLocalizations.markAsReadInProgress,
94-
onFailedTitle: zulipLocalizations.errorMarkAsReadFailedTitle);
83+
final didPass = await updateMessageFlagsStartingFromAnchor(
84+
context: context,
85+
// Include `is:unread` in the narrow. That has a database index, so
86+
// this can be an important optimization in narrows with a lot of history.
87+
// The server applies the same optimization within the (deprecated)
88+
// specialized endpoints for marking messages as read; see
89+
// `do_mark_stream_messages_as_read` in `zulip:zerver/actions/message_flags.py`.
90+
apiNarrow: narrow.apiEncode()..add(ApiNarrowIs(IsOperand.unread)),
91+
// Use [AnchorCode.oldest], because [AnchorCode.firstUnread]
92+
// will be the oldest non-muted unread message, which would
93+
// result in muted unreads older than the first unread not
94+
// being processed.
95+
anchor: AnchorCode.oldest,
96+
// [AnchorCode.oldest] is an anchor ID lower than any valid
97+
// message ID.
98+
includeAnchor: false,
99+
op: UpdateMessageFlagsOp.add,
100+
flag: MessageFlag.read,
101+
onCompletedMessage: zulipLocalizations.markAsReadComplete,
102+
progressMessage: zulipLocalizations.markAsReadInProgress,
103+
onFailedTitle: zulipLocalizations.errorMarkAsReadFailedTitle);
95104

96-
if (!didPass || !context.mounted) return;
97-
if (narrow is CombinedFeedNarrow) {
98-
PerAccountStoreWidget.of(context).unreads.handleAllMessagesReadSuccess();
105+
if (!didPass || !context.mounted) return;
106+
if (narrow is CombinedFeedNarrow) {
107+
PerAccountStoreWidget.of(context).unreads.handleAllMessagesReadSuccess();
108+
}
99109
}
100110
}
101111

lib/widgets/message_list.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,7 @@ class _MarkAsReadWidgetState extends State<MarkAsReadWidget> {
788788
void _handlePress(BuildContext context) async {
789789
if (!context.mounted) return;
790790
setState(() => _loading = true);
791-
await markNarrowAsRead(context, widget.narrow);
791+
await ZulipAction.markNarrowAsRead(context, widget.narrow);
792792
setState(() => _loading = false);
793793
}
794794

test/widgets/actions_test.dart

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ void main() {
289289
processedCount: 11, updatedCount: 3,
290290
firstProcessedId: null, lastProcessedId: null,
291291
foundOldest: true, foundNewest: true).toJson());
292-
final future = markNarrowAsRead(context, narrow);
292+
final future = ZulipAction.markNarrowAsRead(context, narrow);
293293
await tester.pump(Duration.zero);
294294
await future;
295295
final apiNarrow = narrow.apiEncode()..add(ApiNarrowIs(IsOperand.unread));
@@ -314,7 +314,7 @@ void main() {
314314
processedCount: 11, updatedCount: 3,
315315
firstProcessedId: null, lastProcessedId: null,
316316
foundOldest: true, foundNewest: true).toJson());
317-
final future = markNarrowAsRead(context, narrow);
317+
final future = ZulipAction.markNarrowAsRead(context, narrow);
318318
await tester.pump(Duration.zero);
319319
await future;
320320
check(connection.lastRequest).isA<http.Request>()
@@ -340,7 +340,7 @@ void main() {
340340
processedCount: 11, updatedCount: 3,
341341
firstProcessedId: null, lastProcessedId: null,
342342
foundOldest: true, foundNewest: true).toJson());
343-
final future = markNarrowAsRead(context, narrow);
343+
final future = ZulipAction.markNarrowAsRead(context, narrow);
344344
await tester.pump(Duration.zero);
345345
await future;
346346
check(store.unreads.oldUnreadsMissing).isFalse();
@@ -354,7 +354,7 @@ void main() {
354354

355355
connection.zulipFeatureLevel = 154;
356356
connection.prepare(json: {});
357-
final future = markNarrowAsRead(context, narrow);
357+
final future = ZulipAction.markNarrowAsRead(context, narrow);
358358
await tester.pump(Duration.zero);
359359
await future;
360360
check(connection.lastRequest).isA<http.Request>()
@@ -373,7 +373,7 @@ void main() {
373373
await prepare(tester);
374374
connection.zulipFeatureLevel = 154;
375375
connection.prepare(json: {});
376-
final future = markNarrowAsRead(context, narrow);
376+
final future = ZulipAction.markNarrowAsRead(context, narrow);
377377
await tester.pump(Duration.zero);
378378
await future;
379379
check(connection.lastRequest).isA<http.Request>()
@@ -389,7 +389,7 @@ void main() {
389389
await prepare(tester);
390390
connection.zulipFeatureLevel = 154;
391391
connection.prepare(json: {});
392-
final future = markNarrowAsRead(context, narrow);
392+
final future = ZulipAction.markNarrowAsRead(context, narrow);
393393
await tester.pump(Duration.zero);
394394
await future;
395395
check(connection.lastRequest).isA<http.Request>()
@@ -412,7 +412,7 @@ void main() {
412412
connection.zulipFeatureLevel = 154;
413413
connection.prepare(json:
414414
UpdateMessageFlagsResult(messages: [message.id]).toJson());
415-
final future = markNarrowAsRead(context, narrow);
415+
final future = ZulipAction.markNarrowAsRead(context, narrow);
416416
await tester.pump(Duration.zero);
417417
await future;
418418
check(connection.lastRequest).isA<http.Request>()
@@ -433,7 +433,7 @@ void main() {
433433
connection.zulipFeatureLevel = 154;
434434
connection.prepare(json:
435435
UpdateMessageFlagsResult(messages: [message.id]).toJson());
436-
final future = markNarrowAsRead(context, narrow);
436+
final future = ZulipAction.markNarrowAsRead(context, narrow);
437437
await tester.pump(Duration.zero);
438438
await future;
439439
check(connection.lastRequest).isA<http.Request>()

0 commit comments

Comments
 (0)