From e172c58be8b1f15ea085ba28412de22a943c6e18 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 4 Dec 2024 16:09:27 -0800 Subject: [PATCH] action_sheet: Implement resolve/unresolve in topic action sheet Fixes: #744 --- assets/l10n/app_en.arb | 36 ++++ lib/api/model/model.dart | 6 + lib/generated/l10n/zulip_localizations.dart | 54 ++++++ .../l10n/zulip_localizations_ar.dart | 27 +++ .../l10n/zulip_localizations_en.dart | 27 +++ .../l10n/zulip_localizations_ja.dart | 27 +++ .../l10n/zulip_localizations_nb.dart | 27 +++ .../l10n/zulip_localizations_pl.dart | 27 +++ .../l10n/zulip_localizations_ru.dart | 27 +++ .../l10n/zulip_localizations_sk.dart | 27 +++ lib/widgets/action_sheet.dart | 110 +++++++++++ lib/widgets/inbox.dart | 7 +- lib/widgets/message_list.dart | 21 ++- test/widgets/action_sheet_test.dart | 173 +++++++++++++++++- 14 files changed, 585 insertions(+), 11 deletions(-) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index 58822303fd..a385fec160 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -88,6 +88,42 @@ "@actionSheetOptionUnfollowTopic": { "description": "Label for unfollowing a topic on action sheet." }, + "actionSheetOptionResolveTopic": "Mark as resolved", + "@actionSheetOptionResolveTopic": { + "description": "Label for the 'Mark as resolved' button on the topic action sheet." + }, + "actionSheetOptionUnresolveTopic": "Mark as unresolved", + "@actionSheetOptionUnresolveTopic": { + "description": "Label for the 'Mark as unresolved' button on the topic action sheet." + }, + "resolveTopicInterruptedTitle": "Problem marking topic as resolved", + "@resolveTopicInterruptedTitle": { + "description": "Error title when trying to 'Mark as resolved' and the app decides not to continue." + }, + "unresolveTopicInterruptedTitle": "Problem marking topic as unresolved", + "@unresolveTopicInterruptedTitle": { + "description": "Error title when trying to 'Mark as unresolved' and the app decides not to continue." + }, + "topicAlreadyResolvedMessage": "This topic is already marked as resolved.", + "@topicAlreadyResolvedMessage": { + "description": "Error message when trying to 'Mark as resolved' a topic that was already marked by someone else." + }, + "topicAlreadyUnresolvedMessage": "This topic is already marked as unresolved.", + "@topicAlreadyUnresolvedMessage": { + "description": "Error message when trying to 'Mark as unresolved' a topic that was already marked by someone else." + }, + "topicRenamedMessage": "This topic has been renamed.", + "@topicRenamedMessage": { + "description": "Error title when trying to 'Mark as resolved' or 'Mark as unresolved' a topic that was renamed by someone else." + }, + "errorResolveTopicFailedTitle": "Failed to mark topic as resolved", + "@errorResolveTopicFailedTitle": { + "description": "Error title when marking a topic as resolved failed." + }, + "errorUnresolveTopicFailedTitle": "Failed to mark topic as unresolved", + "@errorUnresolveTopicFailedTitle": { + "description": "Error title when marking a topic as unresolved failed." + }, "actionSheetOptionCopyMessageText": "Copy message text", "@actionSheetOptionCopyMessageText": { "description": "Label for copy message text button on action sheet." diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 03af104baf..68c240b36a 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -695,6 +695,12 @@ extension type const TopicName(String _value) { /// The key to use for "same topic as" comparisons. String canonicalize() => apiName.toLowerCase(); + /// Whether the topic starts with [resolvedTopicPrefixRegexp]. + bool get isResolved => resolvedTopicPrefixRegexp.hasMatch(_value); + + /// This [TopicName] plus the [resolvedTopicPrefix] prefix. + TopicName resolve() => TopicName(resolvedTopicPrefix + _value); + /// A [TopicName] with [resolvedTopicPrefixRegexp] stripped if present. TopicName unresolve() => TopicName(_value.replaceFirst(resolvedTopicPrefixRegexp, '')); diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index 6ff41633fd..b3c02abe36 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -237,6 +237,60 @@ abstract class ZulipLocalizations { /// **'Unfollow topic'** String get actionSheetOptionUnfollowTopic; + /// Label for the 'Mark as resolved' button on the topic action sheet. + /// + /// In en, this message translates to: + /// **'Mark as resolved'** + String get actionSheetOptionResolveTopic; + + /// Label for the 'Mark as unresolved' button on the topic action sheet. + /// + /// In en, this message translates to: + /// **'Mark as unresolved'** + String get actionSheetOptionUnresolveTopic; + + /// Error title when trying to 'Mark as resolved' and the app decides not to continue. + /// + /// In en, this message translates to: + /// **'Problem marking topic as resolved'** + String get resolveTopicInterruptedTitle; + + /// Error title when trying to 'Mark as unresolved' and the app decides not to continue. + /// + /// In en, this message translates to: + /// **'Problem marking topic as unresolved'** + String get unresolveTopicInterruptedTitle; + + /// Error message when trying to 'Mark as resolved' a topic that was already marked by someone else. + /// + /// In en, this message translates to: + /// **'This topic is already marked as resolved.'** + String get topicAlreadyResolvedMessage; + + /// Error message when trying to 'Mark as unresolved' a topic that was already marked by someone else. + /// + /// In en, this message translates to: + /// **'This topic is already marked as unresolved.'** + String get topicAlreadyUnresolvedMessage; + + /// Error title when trying to 'Mark as resolved' or 'Mark as unresolved' a topic that was renamed by someone else. + /// + /// In en, this message translates to: + /// **'This topic has been renamed.'** + String get topicRenamedMessage; + + /// Error title when marking a topic as resolved failed. + /// + /// In en, this message translates to: + /// **'Failed to mark topic as resolved'** + String get errorResolveTopicFailedTitle; + + /// Error title when marking a topic as unresolved failed. + /// + /// In en, this message translates to: + /// **'Failed to mark topic as unresolved'** + String get errorUnresolveTopicFailedTitle; + /// Label for copy message text button on action sheet. /// /// In en, this message translates to: diff --git a/lib/generated/l10n/zulip_localizations_ar.dart b/lib/generated/l10n/zulip_localizations_ar.dart index 542b85031b..69dff2f6bc 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -76,6 +76,33 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get resolveTopicInterruptedTitle => 'Problem marking topic as resolved'; + + @override + String get unresolveTopicInterruptedTitle => 'Problem marking topic as unresolved'; + + @override + String get topicAlreadyResolvedMessage => 'This topic is already marked as resolved.'; + + @override + String get topicAlreadyUnresolvedMessage => 'This topic is already marked as unresolved.'; + + @override + String get topicRenamedMessage => 'This topic has been renamed.'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Copy message text'; diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index b6bc9f72e7..e0f1c1f2f7 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -76,6 +76,33 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get resolveTopicInterruptedTitle => 'Problem marking topic as resolved'; + + @override + String get unresolveTopicInterruptedTitle => 'Problem marking topic as unresolved'; + + @override + String get topicAlreadyResolvedMessage => 'This topic is already marked as resolved.'; + + @override + String get topicAlreadyUnresolvedMessage => 'This topic is already marked as unresolved.'; + + @override + String get topicRenamedMessage => 'This topic has been renamed.'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Copy message text'; diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index 7adbc9ae8a..b2275b8e76 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -76,6 +76,33 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get resolveTopicInterruptedTitle => 'Problem marking topic as resolved'; + + @override + String get unresolveTopicInterruptedTitle => 'Problem marking topic as unresolved'; + + @override + String get topicAlreadyResolvedMessage => 'This topic is already marked as resolved.'; + + @override + String get topicAlreadyUnresolvedMessage => 'This topic is already marked as unresolved.'; + + @override + String get topicRenamedMessage => 'This topic has been renamed.'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Copy message text'; diff --git a/lib/generated/l10n/zulip_localizations_nb.dart b/lib/generated/l10n/zulip_localizations_nb.dart index 99c545f98e..caf5104cfe 100644 --- a/lib/generated/l10n/zulip_localizations_nb.dart +++ b/lib/generated/l10n/zulip_localizations_nb.dart @@ -76,6 +76,33 @@ class ZulipLocalizationsNb extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get resolveTopicInterruptedTitle => 'Problem marking topic as resolved'; + + @override + String get unresolveTopicInterruptedTitle => 'Problem marking topic as unresolved'; + + @override + String get topicAlreadyResolvedMessage => 'This topic is already marked as resolved.'; + + @override + String get topicAlreadyUnresolvedMessage => 'This topic is already marked as unresolved.'; + + @override + String get topicRenamedMessage => 'This topic has been renamed.'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Copy message text'; diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index e7a05a58aa..a0e8935534 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -76,6 +76,33 @@ class ZulipLocalizationsPl extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Nie śledź wątku'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get resolveTopicInterruptedTitle => 'Problem marking topic as resolved'; + + @override + String get unresolveTopicInterruptedTitle => 'Problem marking topic as unresolved'; + + @override + String get topicAlreadyResolvedMessage => 'This topic is already marked as resolved.'; + + @override + String get topicAlreadyUnresolvedMessage => 'This topic is already marked as unresolved.'; + + @override + String get topicRenamedMessage => 'This topic has been renamed.'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Skopiuj tekst wiadomości'; diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index 2082984588..dea6040409 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -76,6 +76,33 @@ class ZulipLocalizationsRu extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Не отслеживать тему'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get resolveTopicInterruptedTitle => 'Problem marking topic as resolved'; + + @override + String get unresolveTopicInterruptedTitle => 'Problem marking topic as unresolved'; + + @override + String get topicAlreadyResolvedMessage => 'This topic is already marked as resolved.'; + + @override + String get topicAlreadyUnresolvedMessage => 'This topic is already marked as unresolved.'; + + @override + String get topicRenamedMessage => 'This topic has been renamed.'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Скопировать текст сообщения'; diff --git a/lib/generated/l10n/zulip_localizations_sk.dart b/lib/generated/l10n/zulip_localizations_sk.dart index fabfa06eb4..058afbebc7 100644 --- a/lib/generated/l10n/zulip_localizations_sk.dart +++ b/lib/generated/l10n/zulip_localizations_sk.dart @@ -76,6 +76,33 @@ class ZulipLocalizationsSk extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Prestať sledovať tému'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get resolveTopicInterruptedTitle => 'Problem marking topic as resolved'; + + @override + String get unresolveTopicInterruptedTitle => 'Problem marking topic as unresolved'; + + @override + String get topicAlreadyResolvedMessage => 'This topic is already marked as resolved.'; + + @override + String get topicAlreadyUnresolvedMessage => 'This topic is already marked as unresolved.'; + + @override + String get topicRenamedMessage => 'This topic has been renamed.'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Skopírovať text správy'; diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index e697d20029..a838df57ec 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -168,9 +168,13 @@ class ActionSheetCancelButton extends StatelessWidget { /// not of the specific element that was long-pressed. /// The long-pressed element live-updates on server events, /// so it might unmount before the action-sheet buttons have finished using it. +/// +/// The API request for resolving/unresolving a topic needs a message ID. +/// If [someMessageIdInTopic] is null, the button for that will be absent. void showTopicActionSheet(BuildContext pageContext, { required int channelId, required TopicName topic, + required int? someMessageIdInTopic, }) { final store = PerAccountStoreWidget.of(pageContext); final subscription = store.subscriptions[channelId]; @@ -245,6 +249,12 @@ void showTopicActionSheet(BuildContext pageContext, { pageContext: pageContext); })); + if (someMessageIdInTopic != null) { + optionButtons.add(ResolveUnresolveButton(pageContext: pageContext, + topic: topic, + someMessageIdInTopic: someMessageIdInTopic)); + } + if (optionButtons.isEmpty) { // TODO(a11y): This case makes a no-op gesture handler; as a consequence, // we're presenting some UI (to people who use screen-reader software) as @@ -377,6 +387,106 @@ class UserTopicUpdateButton extends ActionSheetMenuItemButton { } } +class ResolveUnresolveButton extends ActionSheetMenuItemButton { + ResolveUnresolveButton({ + super.key, + required this.topic, + required this.someMessageIdInTopic, + required super.pageContext, + }) : _actionIsResolve = !topic.isResolved; + + final bool _actionIsResolve; + + /// The topic that the action sheet was opened for. + /// + /// There might not currently be any messages with this topic; + /// see dartdoc of [ActionSheetMenuItemButton]. + final TopicName topic; + + /// The message ID that was passed when opening the action sheet. + /// + /// The message with this ID might currently not exist, + /// or might exist with a different topic; + /// see dartdoc of [ActionSheetMenuItemButton]. + final int someMessageIdInTopic; + + @override + IconData get icon => _actionIsResolve ? ZulipIcons.check : ZulipIcons.check_remove; + + @override + String label(ZulipLocalizations zulipLocalizations) { + return _actionIsResolve + ? zulipLocalizations.actionSheetOptionResolveTopic + : zulipLocalizations.actionSheetOptionUnresolveTopic; + } + + @override void onPressed() async { + final zulipLocalizations = ZulipLocalizations.of(pageContext); + final store = PerAccountStoreWidget.of(pageContext); + final message = store.messages[someMessageIdInTopic] as StreamMessage?; + + if (message != null && !message.topic.isSameAs(topic)) { + // TODO also interrupt on a channel move, with a specific message? + + // The message's topic doesn't match what it was when we opened the + // action sheet. So either the resolve/unresolve action was already done + // or the topic was renamed; either way, tell the user and don't make a + // request. + final title = _actionIsResolve + ? zulipLocalizations.resolveTopicInterruptedTitle + : zulipLocalizations.unresolveTopicInterruptedTitle; + final errorMessage = message.topic.unresolve().isSameAs(topic.unresolve()) + ? _actionIsResolve + ? zulipLocalizations.topicAlreadyResolvedMessage + : zulipLocalizations.topicAlreadyUnresolvedMessage + : zulipLocalizations.topicRenamedMessage; + showErrorDialog(context: pageContext, title: title, message: errorMessage); + return; + } + + if (message == null) { + // Proceed. + // + // This happens when [someMessageIdInTopic] came from [Unreads] + // and we just haven't fetched the message's details yet + // because we haven't opened a message list containing the message. + // + // Less commonly, this happens if the message was deleted + // after the action sheet was opened; + // see dartdoc of [ActionSheetMenuItemButton]. + // + // Anyway, we have a message ID, which the server asks for; use it. + } + + try { + await updateMessage(store.connection, + messageId: someMessageIdInTopic, + topic: _actionIsResolve ? topic.resolve() : topic.unresolve(), + propagateMode: PropagateMode.changeAll, + sendNotificationToOldThread: false, + sendNotificationToNewThread: true, + ); + } catch (e) { + if (!pageContext.mounted) return; + + String? errorMessage; + + switch (e) { + case ZulipApiException(): + errorMessage = e.message; + // TODO(#741) specific messages for common errors, like network errors + // (support with reusable code) + default: + } + + final title = _actionIsResolve + ? zulipLocalizations.errorResolveTopicFailedTitle + : zulipLocalizations.errorUnresolveTopicFailedTitle; + showErrorDialog(context: pageContext, title: title, message: errorMessage); + } + } +} + /// Show a sheet of actions you can take on a message in the message list. /// /// Must have a [MessageListPage] ancestor. diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index 06ea3ea22f..f94fc9dd2d 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -519,7 +519,8 @@ class _TopicItem extends StatelessWidget { @override Widget build(BuildContext context) { - final _StreamSectionTopicData(:topic, :count, :hasMention) = data; + final _StreamSectionTopicData( + :topic, :count, :hasMention, :lastUnreadId) = data; final store = PerAccountStoreWidget.of(context); final subscription = store.subscriptions[streamId]!; @@ -538,7 +539,9 @@ class _TopicItem extends StatelessWidget { }, onLongPress: () => showTopicActionSheet( InboxPageBody.ancestorOf(context).context, - channelId: streamId, topic: topic), + channelId: streamId, + topic: topic, + someMessageIdInTopic: lastUnreadId), child: ConstrainedBox(constraints: const BoxConstraints(minHeight: 34), child: Row(crossAxisAlignment: CrossAxisAlignment.center, children: [ const SizedBox(width: 63), diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index fc55fcc286..f4f2949aa9 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -402,9 +402,20 @@ class MessageListAppBarTitle extends StatelessWidget { width: double.infinity, child: GestureDetector( behavior: HitTestBehavior.translucent, - onLongPress: () => showTopicActionSheet( - MessageListPage.ancestorOf(context).context, - channelId: streamId, topic: topic), + onLongPress: () { + final someMessage = MessageListPage.ancestorOf(context) + .model?.messages.firstOrNull; + // If someMessage is null, the topic action sheet won't have a + // resolve/unresolve button. That seems OK; in that case we're + // either still fetching messages (and the user can reopen the + // sheet after that finishes) or there aren't any messages to + // act on anyway. + assert(someMessage == null || narrow.containsMessage(someMessage)); + showTopicActionSheet(MessageListPage.ancestorOf(context).context, + channelId: streamId, + topic: topic, + someMessageIdInTopic: someMessage?.id); + }, child: Column( crossAxisAlignment: willCenterTitle ? CrossAxisAlignment.center : CrossAxisAlignment.start, @@ -1117,7 +1128,9 @@ class StreamMessageRecipientHeader extends StatelessWidget { narrow: TopicNarrow.ofMessage(message))), onLongPress: () => showTopicActionSheet( MessageListPage.ancestorOf(context).context, - channelId: message.streamId, topic: topic), + channelId: message.streamId, + topic: topic, + someMessageIdInTopic: message.id), child: ColoredBox( color: backgroundColor, child: Row( diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 7015665236..66d2c4dc6b 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -228,9 +228,10 @@ void main() { } checkButton('Follow topic'); + checkButton('Mark as resolved'); } - testWidgets('show from inbox', (tester) async { + testWidgets('show from inbox; message in Unreads but not in MessageStore', (tester) async { await prepare(unreadMsgs: eg.unreadMsgs(count: 1, channels: [eg.unreadChannelMsgs( streamId: someChannel.streamId, @@ -238,6 +239,17 @@ void main() { unreadMessageIds: [someMessage.id], )])); await showFromInbox(tester); + check(store.unreads.isUnread(someMessage.id)).isNotNull().isTrue(); + check(store.messages).not((it) => it.containsKey(someMessage.id)); + checkButtons(); + }); + + testWidgets('show from inbox; message in Unreads and in MessageStore', (tester) async { + await prepare(); + await store.addMessage(someMessage); + await showFromInbox(tester); + check(store.unreads.isUnread(someMessage.id)).isNotNull().isTrue(); + check(store.messages)[someMessage.id].isNotNull(); checkButtons(); }); @@ -247,6 +259,13 @@ void main() { checkButtons(); }); + testWidgets('show from app bar: resolve/unresolve not offered when msglist empty', (tester) async { + await prepare(); + await showFromAppBar(tester, messages: []); + check(findButtonForLabel('Mark as resolved')).findsNothing(); + check(findButtonForLabel('Mark as unresolved')).findsNothing(); + }); + testWidgets('show from recipient header', (tester) async { await prepare(); await showFromRecipientHeader(tester); @@ -290,10 +309,6 @@ void main() { } void checkButtons(List expectedButtonFinders) { - if (expectedButtonFinders.isEmpty) { - check(actionSheetFinder).findsNothing(); - return; - } check(actionSheetFinder).findsOne(); for (final buttonFinder in expectedButtonFinders) { @@ -451,6 +466,154 @@ void main() { } }); }); + + group('ResolveUnresolveButton', () { + void checkRequest(int messageId, String topic) { + check(connection.takeRequests()).single.isA() + ..method.equals('PATCH') + ..url.path.equals('/api/v1/messages/$messageId') + ..bodyFields.deepEquals({ + 'topic': topic, + 'propagate_mode': 'change_all', + 'send_notification_to_old_thread': 'false', + 'send_notification_to_new_thread': 'true', + }); + } + + testWidgets('resolve: happy path from inbox; message in Unreads but not MessageStore', (tester) async { + final message = eg.streamMessage(stream: someChannel, topic: 'zulip'); + await prepare( + topic: 'zulip', + unreadMsgs: eg.unreadMsgs(count: 1, + channels: [eg.unreadChannelMsgs( + streamId: someChannel.streamId, + topic: 'zulip', + unreadMessageIds: [message.id], + )])); + await showFromInbox(tester, topic: 'zulip'); + check(store.messages).not((it) => it.containsKey(message.id)); + connection.prepare(json: UpdateMessageResult().toJson()); + await tester.tap(findButtonForLabel('Mark as resolved')); + await tester.pumpAndSettle(); + + checkNoErrorDialog(tester); + checkRequest(message.id, '✔ zulip'); + }); + + testWidgets('resolve: happy path from inbox; message in Unreads and MessageStore', (tester) async { + final message = eg.streamMessage(stream: someChannel, topic: 'zulip'); + await prepare(topic: 'zulip'); + await store.addMessage(message); + await showFromInbox(tester, topic: 'zulip'); + check(store.unreads.isUnread(message.id)).isNotNull().isTrue(); + check(store.messages)[message.id].isNotNull(); + connection.prepare(json: UpdateMessageResult().toJson()); + await tester.tap(findButtonForLabel('Mark as resolved')); + await tester.pumpAndSettle(); + checkNoErrorDialog(tester); + checkRequest(message.id, '✔ zulip'); + }); + + testWidgets('unresolve: happy path', (tester) async { + final message = eg.streamMessage(stream: someChannel, topic: '✔ zulip'); + await prepare(topic: '✔ zulip'); + await showFromAppBar(tester, topic: '✔ zulip', messages: [message]); + connection.takeRequests(); + connection.prepare(json: UpdateMessageResult().toJson()); + await tester.tap(findButtonForLabel('Mark as unresolved')); + await tester.pumpAndSettle(); + + checkNoErrorDialog(tester); + checkRequest(message.id, 'zulip'); + }); + + testWidgets('resolve: request fails', (tester) async { + final message = eg.streamMessage(stream: someChannel, topic: 'zulip'); + await prepare(topic: 'zulip'); + await showFromRecipientHeader(tester, topic: 'zulip', message: message); + connection.takeRequests(); + connection.prepare(exception: http.ClientException('Oops')); + await tester.tap(findButtonForLabel('Mark as resolved')); + await tester.pumpAndSettle(); + checkRequest(message.id, '✔ zulip'); + + checkErrorDialog(tester, + expectedTitle: 'Failed to mark topic as resolved'); + }); + + testWidgets('unresolve: request fails', (tester) async { + final message = eg.streamMessage(stream: someChannel, topic: '✔ zulip'); + await prepare(topic: '✔ zulip'); + await showFromRecipientHeader(tester, topic: '✔ zulip', message: message); + connection.takeRequests(); + connection.prepare(exception: http.ClientException('Oops')); + await tester.tap(findButtonForLabel('Mark as unresolved')); + await tester.pumpAndSettle(); + checkRequest(message.id, 'zulip'); + + checkErrorDialog(tester, + expectedTitle: 'Failed to mark topic as unresolved'); + }); + + Future setupTopicChangeAfterSheetOpened( + WidgetTester tester, { + required String oldTopic, + required String newTopic, + required String expectedButtonLabel, + }) async { + final message = eg.streamMessage(stream: someChannel, topic: oldTopic); + await prepare(topic: oldTopic); + await showFromAppBar(tester, topic: oldTopic, messages: [message]); + connection.takeRequests(); + await store.handleEvent(eg.updateMessageEventMoveFrom( + origMessages: [message], newTopicStr: newTopic)); + connection.prepare(json: UpdateMessageResult().toJson()); + + await tester.tap(findButtonForLabel(expectedButtonLabel)); + await tester.pumpAndSettle(); + check(connection.takeRequests()).isEmpty(); + } + + testWidgets("'zulip' changed to '✔ zulip' while action sheet open, then tap resolve", (tester) async { + await setupTopicChangeAfterSheetOpened(tester, + oldTopic: 'zulip', + newTopic: '✔ zulip', + expectedButtonLabel: 'Mark as resolved'); + checkErrorDialog(tester, + expectedTitle: 'Problem marking topic as resolved', + expectedMessage: 'This topic is already marked as resolved.'); + }); + + testWidgets("'zulip' changed to 'asdfjkl;' while action sheet open, then tap resolve", (tester) async { + await setupTopicChangeAfterSheetOpened(tester, + oldTopic: 'zulip', + newTopic: 'asdfjkl;', + expectedButtonLabel: 'Mark as resolved'); + checkErrorDialog(tester, + expectedTitle: 'Problem marking topic as resolved', + expectedMessage: 'This topic has been renamed.'); + }); + + testWidgets("'✔ zulip' changed to 'zulip' while action sheet open, then tap unresolve", (tester) async { + await setupTopicChangeAfterSheetOpened(tester, + oldTopic: '✔ zulip', + newTopic: 'zulip', + expectedButtonLabel: 'Mark as unresolved'); + checkErrorDialog(tester, + expectedTitle: 'Problem marking topic as unresolved', + expectedMessage: 'This topic is already marked as unresolved.'); + }); + + testWidgets("'✔ zulip' changed to 'asdfjkl;' while action sheet open, then tap unresolve", (tester) async { + await setupTopicChangeAfterSheetOpened(tester, + oldTopic: '✔ zulip', + newTopic: 'asdfjkl;', + expectedButtonLabel: 'Mark as unresolved'); + checkErrorDialog(tester, + expectedTitle: 'Problem marking topic as unresolved', + expectedMessage: 'This topic has been renamed.'); + }); + }); }); group('message action sheet', () {