Skip to content

Commit 2cfaa17

Browse files
chimnayajithchimnayajith
committed
actions [nfc]: Introduce ZulipAction for high-level operations
Move the high level operations into the new ZulipAction abstract class, per discussion in: https://chat.zulip.org/#narrow/channel/516-mobile-dev-help/topic/.23F1317.20showErrorDialog/near/2080576 This makes it clearer at call sites that the methods combine API operations with UI feedback.
1 parent 4f438d3 commit 2cfaa17

File tree

4 files changed

+172
-167
lines changed

4 files changed

+172
-167
lines changed

‎lib/widgets/action_sheet.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -796,7 +796,7 @@ class MarkAsUnreadButton extends MessageActionSheetMenuItemButton {
796796

797797
@override void onPressed() async {
798798
final narrow = findMessageListPage().narrow;
799-
unawaited(markNarrowAsUnreadFromMessage(pageContext, message, narrow));
799+
unawaited(ZulipAction.markNarrowAsUnreadFromMessage(pageContext, message, narrow));
800800
}
801801
}
802802

‎lib/widgets/actions.dart‎

Lines changed: 161 additions & 156 deletions
Original file line numberDiff line numberDiff line change
@@ -18,174 +18,179 @@ import '../model/narrow.dart';
1818
import 'dialog.dart';
1919
import 'store.dart';
2020

21-
Future<void> markNarrowAsRead(BuildContext context, Narrow narrow) async {
22-
final store = PerAccountStoreWidget.of(context);
23-
final connection = store.connection;
24-
final zulipLocalizations = ZulipLocalizations.of(context);
25-
final useLegacy = connection.zulipFeatureLevel! < 155; // TODO(server-6)
26-
if (useLegacy) {
27-
try {
28-
await _legacyMarkNarrowAsRead(context, narrow);
29-
return;
30-
} catch (e) {
31-
if (!context.mounted) return;
32-
showErrorDialog(context: context,
33-
title: zulipLocalizations.errorMarkAsReadFailedTitle,
34-
message: e.toString()); // TODO(#741): extract user-facing message better
35-
return;
21+
/// High-level operations that combine API calls with UI feedback.
22+
///
23+
/// Methods in this class provide UI feedback while performing API operations.
24+
abstract final class ZulipAction {
25+
static Future<void> markNarrowAsRead(BuildContext context, Narrow narrow) async {
26+
final store = PerAccountStoreWidget.of(context);
27+
final connection = store.connection;
28+
final zulipLocalizations = ZulipLocalizations.of(context);
29+
final useLegacy = connection.zulipFeatureLevel! < 155; // TODO(server-6)
30+
if (useLegacy) {
31+
try {
32+
await _legacyMarkNarrowAsRead(context, narrow);
33+
return;
34+
} catch (e) {
35+
if (!context.mounted) return;
36+
showErrorDialog(context: context,
37+
title: zulipLocalizations.errorMarkAsReadFailedTitle,
38+
message: e.toString()); // TODO(#741): extract user-facing message better
39+
return;
40+
}
3641
}
37-
}
3842

39-
final didPass = await updateMessageFlagsStartingFromAnchor(
40-
context: context,
41-
// Include `is:unread` in the narrow. That has a database index, so
42-
// this can be an important optimization in narrows with a lot of history.
43-
// The server applies the same optimization within the (deprecated)
44-
// specialized endpoints for marking messages as read; see
45-
// `do_mark_stream_messages_as_read` in `zulip:zerver/actions/message_flags.py`.
46-
apiNarrow: narrow.apiEncode()..add(ApiNarrowIs(IsOperand.unread)),
47-
// Use [AnchorCode.oldest], because [AnchorCode.firstUnread]
48-
// will be the oldest non-muted unread message, which would
49-
// result in muted unreads older than the first unread not
50-
// being processed.
51-
anchor: AnchorCode.oldest,
52-
// [AnchorCode.oldest] is an anchor ID lower than any valid
53-
// message ID.
54-
includeAnchor: false,
55-
op: UpdateMessageFlagsOp.add,
56-
flag: MessageFlag.read,
57-
onCompletedMessage: zulipLocalizations.markAsReadComplete,
58-
progressMessage: zulipLocalizations.markAsReadInProgress,
59-
onFailedTitle: zulipLocalizations.errorMarkAsReadFailedTitle);
43+
final didPass = await updateMessageFlagsStartingFromAnchor(
44+
context: context,
45+
// Include `is:unread` in the narrow. That has a database index, so
46+
// this can be an important optimization in narrows with a lot of history.
47+
// The server applies the same optimization within the (deprecated)
48+
// specialized endpoints for marking messages as read; see
49+
// `do_mark_stream_messages_as_read` in `zulip:zerver/actions/message_flags.py`.
50+
apiNarrow: narrow.apiEncode()..add(ApiNarrowIs(IsOperand.unread)),
51+
// Use [AnchorCode.oldest], because [AnchorCode.firstUnread]
52+
// will be the oldest non-muted unread message, which would
53+
// result in muted unreads older than the first unread not
54+
// being processed.
55+
anchor: AnchorCode.oldest,
56+
// [AnchorCode.oldest] is an anchor ID lower than any valid
57+
// message ID.
58+
includeAnchor: false,
59+
op: UpdateMessageFlagsOp.add,
60+
flag: MessageFlag.read,
61+
onCompletedMessage: zulipLocalizations.markAsReadComplete,
62+
progressMessage: zulipLocalizations.markAsReadInProgress,
63+
onFailedTitle: zulipLocalizations.errorMarkAsReadFailedTitle);
6064

61-
if (!didPass || !context.mounted) return;
62-
if (narrow is CombinedFeedNarrow) {
63-
PerAccountStoreWidget.of(context).unreads.handleAllMessagesReadSuccess();
65+
if (!didPass || !context.mounted) return;
66+
if (narrow is CombinedFeedNarrow) {
67+
PerAccountStoreWidget.of(context).unreads.handleAllMessagesReadSuccess();
68+
}
6469
}
65-
}
6670

67-
Future<void> markNarrowAsUnreadFromMessage(
68-
BuildContext context,
69-
Message message,
70-
Narrow narrow,
71-
) async {
72-
final connection = PerAccountStoreWidget.of(context).connection;
73-
assert(connection.zulipFeatureLevel! >= 155); // TODO(server-6)
74-
final zulipLocalizations = ZulipLocalizations.of(context);
75-
await updateMessageFlagsStartingFromAnchor(
76-
context: context,
77-
apiNarrow: narrow.apiEncode(),
78-
anchor: NumericAnchor(message.id),
79-
includeAnchor: true,
80-
op: UpdateMessageFlagsOp.remove,
81-
flag: MessageFlag.read,
82-
onCompletedMessage: zulipLocalizations.markAsUnreadComplete,
83-
progressMessage: zulipLocalizations.markAsUnreadInProgress,
84-
onFailedTitle: zulipLocalizations.errorMarkAsUnreadFailedTitle);
85-
}
71+
/// Add or remove the given flag from the anchor to the end of the narrow,
72+
/// showing feedback to the user on progress or failure.
73+
///
74+
/// This has the semantics of [updateMessageFlagsForNarrow]
75+
/// (see https://zulip.com/api/update-message-flags-for-narrow)
76+
/// with `numBefore: 0` and infinite `numAfter`. It operates by calling that
77+
/// endpoint with a finite `numAfter` as a batch size, in a loop.
78+
///
79+
/// If the operation requires more than one batch, the user is shown progress
80+
/// feedback through [SnackBar], using [progressMessage] and [onCompletedMessage].
81+
/// If the operation fails, the user is shown an error dialog box with title
82+
/// [onFailedTitle].
83+
///
84+
/// Returns true just if the operation finished successfully.
85+
static Future<bool> updateMessageFlagsStartingFromAnchor({
86+
required BuildContext context,
87+
required List<ApiNarrowElement> apiNarrow,
88+
required Anchor anchor,
89+
required bool includeAnchor,
90+
required UpdateMessageFlagsOp op,
91+
required MessageFlag flag,
92+
required String Function(int) onCompletedMessage,
93+
required String progressMessage,
94+
required String onFailedTitle,
95+
}) async {
96+
try {
97+
final store = PerAccountStoreWidget.of(context);
98+
final connection = store.connection;
99+
final scaffoldMessenger = ScaffoldMessenger.of(context);
86100

87-
/// Add or remove the given flag from the anchor to the end of the narrow,
88-
/// showing feedback to the user on progress or failure.
89-
///
90-
/// This has the semantics of [updateMessageFlagsForNarrow]
91-
/// (see https://zulip.com/api/update-message-flags-for-narrow)
92-
/// with `numBefore: 0` and infinite `numAfter`. It operates by calling that
93-
/// endpoint with a finite `numAfter` as a batch size, in a loop.
94-
///
95-
/// If the operation requires more than one batch, the user is shown progress
96-
/// feedback through [SnackBar], using [progressMessage] and [onCompletedMessage].
97-
/// If the operation fails, the user is shown an error dialog box with title
98-
/// [onFailedTitle].
99-
///
100-
/// Returns true just if the operation finished successfully.
101-
Future<bool> updateMessageFlagsStartingFromAnchor({
102-
required BuildContext context,
103-
required List<ApiNarrowElement> apiNarrow,
104-
required Anchor anchor,
105-
required bool includeAnchor,
106-
required UpdateMessageFlagsOp op,
107-
required MessageFlag flag,
108-
required String Function(int) onCompletedMessage,
109-
required String progressMessage,
110-
required String onFailedTitle,
111-
}) async {
112-
try {
113-
final store = PerAccountStoreWidget.of(context);
114-
final connection = store.connection;
115-
final scaffoldMessenger = ScaffoldMessenger.of(context);
101+
// Compare web's `mark_all_as_read` in web/src/unread_ops.js
102+
// and zulip-mobile's `markAsUnreadFromMessage` in src/action-sheets/index.js .
103+
int responseCount = 0;
104+
int updatedCount = 0;
105+
while (true) {
106+
final result = await updateMessageFlagsForNarrow(connection,
107+
anchor: anchor,
108+
includeAnchor: includeAnchor,
109+
// There is an upper limit of 5000 messages per batch
110+
// (numBefore + numAfter <= 5000) enforced on the server.
111+
// See `update_message_flags_in_narrow` in zerver/views/message_flags.py .
112+
// zulip-mobile uses `numAfter` of 5000, but web uses 1000
113+
// for more responsive feedback. See zulip@f0d87fcf6.
114+
numBefore: 0,
115+
numAfter: 1000,
116+
narrow: apiNarrow,
117+
op: op,
118+
flag: flag);
119+
if (!context.mounted) {
120+
scaffoldMessenger.clearSnackBars();
121+
return false;
122+
}
123+
responseCount++;
124+
updatedCount += result.updatedCount;
116125

117-
// Compare web's `mark_all_as_read` in web/src/unread_ops.js
118-
// and zulip-mobile's `markAsUnreadFromMessage` in src/action-sheets/index.js .
119-
int responseCount = 0;
120-
int updatedCount = 0;
121-
while (true) {
122-
final result = await updateMessageFlagsForNarrow(connection,
123-
anchor: anchor,
124-
includeAnchor: includeAnchor,
125-
// There is an upper limit of 5000 messages per batch
126-
// (numBefore + numAfter <= 5000) enforced on the server.
127-
// See `update_message_flags_in_narrow` in zerver/views/message_flags.py .
128-
// zulip-mobile uses `numAfter` of 5000, but web uses 1000
129-
// for more responsive feedback. See zulip@f0d87fcf6.
130-
numBefore: 0,
131-
numAfter: 1000,
132-
narrow: apiNarrow,
133-
op: op,
134-
flag: flag);
135-
if (!context.mounted) {
136-
scaffoldMessenger.clearSnackBars();
137-
return false;
138-
}
139-
responseCount++;
140-
updatedCount += result.updatedCount;
126+
if (result.foundNewest) {
127+
if (responseCount > 1) {
128+
// We previously showed an in-progress [SnackBar], so say we're done.
129+
// There may be a backlog of [SnackBar]s accumulated in the queue
130+
// so be sure to clear them out here.
131+
scaffoldMessenger
132+
..clearSnackBars()
133+
..showSnackBar(SnackBar(behavior: SnackBarBehavior.floating,
134+
content: Text(onCompletedMessage(updatedCount))));
135+
}
136+
return true;
137+
}
141138

142-
if (result.foundNewest) {
143-
if (responseCount > 1) {
144-
// We previously showed an in-progress [SnackBar], so say we're done.
145-
// There may be a backlog of [SnackBar]s accumulated in the queue
146-
// so be sure to clear them out here.
147-
scaffoldMessenger
148-
..clearSnackBars()
149-
..showSnackBar(SnackBar(behavior: SnackBarBehavior.floating,
150-
content: Text(onCompletedMessage(updatedCount))));
139+
if (result.lastProcessedId == null) {
140+
final zulipLocalizations = ZulipLocalizations.of(context);
141+
// No messages were in the range of the request.
142+
// This should be impossible given that `foundNewest` was false
143+
// (and that our `numAfter` was positive.)
144+
showErrorDialog(context: context,
145+
title: onFailedTitle,
146+
message: zulipLocalizations.errorInvalidResponse);
147+
return false;
151148
}
152-
return true;
153-
}
149+
anchor = NumericAnchor(result.lastProcessedId!);
150+
includeAnchor = false;
154151

155-
if (result.lastProcessedId == null) {
156-
final zulipLocalizations = ZulipLocalizations.of(context);
157-
// No messages were in the range of the request.
158-
// This should be impossible given that `foundNewest` was false
159-
// (and that our `numAfter` was positive.)
160-
showErrorDialog(context: context,
161-
title: onFailedTitle,
162-
message: zulipLocalizations.errorInvalidResponse);
163-
return false;
152+
// The task is taking a while, so tell the user we're working on it.
153+
// TODO: Ideally we'd have a progress widget here that showed up based
154+
// on actual time elapsed -- so it could appear before the first
155+
// batch returns, if that takes a while -- and that then stuck
156+
// around continuously until the task ends. For now we use a
157+
// series of [SnackBar]s, which may feel a bit janky.
158+
// There is complexity in tracking the status of each [SnackBar],
159+
// due to having no way to determine which is currently active,
160+
// or if there is an active one at all. Resetting the [SnackBar] here
161+
// results in the same message popping in and out and the user experience
162+
// is better for now if we allow them to run their timer through
163+
// and clear the backlog later.
164+
scaffoldMessenger.showSnackBar(SnackBar(behavior: SnackBarBehavior.floating,
165+
content: Text(progressMessage)));
164166
}
165-
anchor = NumericAnchor(result.lastProcessedId!);
166-
includeAnchor = false;
167-
168-
// The task is taking a while, so tell the user we're working on it.
169-
// TODO: Ideally we'd have a progress widget here that showed up based
170-
// on actual time elapsed -- so it could appear before the first
171-
// batch returns, if that takes a while -- and that then stuck
172-
// around continuously until the task ends. For now we use a
173-
// series of [SnackBar]s, which may feel a bit janky.
174-
// There is complexity in tracking the status of each [SnackBar],
175-
// due to having no way to determine which is currently active,
176-
// or if there is an active one at all. Resetting the [SnackBar] here
177-
// results in the same message popping in and out and the user experience
178-
// is better for now if we allow them to run their timer through
179-
// and clear the backlog later.
180-
scaffoldMessenger.showSnackBar(SnackBar(behavior: SnackBarBehavior.floating,
181-
content: Text(progressMessage)));
167+
} catch (e) {
168+
if (!context.mounted) return false;
169+
showErrorDialog(context: context,
170+
title: onFailedTitle,
171+
message: e.toString()); // TODO(#741): extract user-facing message better
172+
return false;
182173
}
183-
} catch (e) {
184-
if (!context.mounted) return false;
185-
showErrorDialog(context: context,
186-
title: onFailedTitle,
187-
message: e.toString()); // TODO(#741): extract user-facing message better
188-
return false;
174+
}
175+
176+
static Future<void> markNarrowAsUnreadFromMessage(
177+
BuildContext context,
178+
Message message,
179+
Narrow narrow,
180+
) async {
181+
final connection = PerAccountStoreWidget.of(context).connection;
182+
assert(connection.zulipFeatureLevel! >= 155); // TODO(server-6)
183+
final zulipLocalizations = ZulipLocalizations.of(context);
184+
await updateMessageFlagsStartingFromAnchor(
185+
context: context,
186+
apiNarrow: narrow.apiEncode(),
187+
anchor: NumericAnchor(message.id),
188+
includeAnchor: true,
189+
op: UpdateMessageFlagsOp.remove,
190+
flag: MessageFlag.read,
191+
onCompletedMessage: zulipLocalizations.markAsUnreadComplete,
192+
progressMessage: zulipLocalizations.markAsUnreadInProgress,
193+
onFailedTitle: zulipLocalizations.errorMarkAsUnreadFailedTitle);
189194
}
190195
}
191196

‎lib/widgets/message_list.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -803,7 +803,7 @@ class _MarkAsReadWidgetState extends State<MarkAsReadWidget> {
803803
void _handlePress(BuildContext context) async {
804804
if (!context.mounted) return;
805805
setState(() => _loading = true);
806-
await markNarrowAsRead(context, widget.narrow);
806+
await ZulipAction.markNarrowAsRead(context, widget.narrow);
807807
setState(() => _loading = false);
808808
}
809809

0 commit comments

Comments
 (0)