message list: Show dialog when ActionSheet action fails.

Currently, there is no error handling for any of the actions performed through messageActionSheet. Add error handling for all actions, and show alert dialog if any action fails. [Comments amended by Ray Kraesig.] Fixes #3788.
zulip · Feb 1, 2020 · fc660d6 · fc660d6
1 parent 0e28596
commit fc660d6
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 22 deletions.
diff --git a/src/message/messageActionSheet.js b/src/message/messageActionSheet.js
@@ -1,5 +1,5 @@
 /* @flow strict-local */
-import { Clipboard, Share } from 'react-native';
+import { Clipboard, Share, Alert } from 'react-native';
 import type { Auth, Dispatch, GetText, Message, Narrow, Outbox, Subscription } from '../types';
 import type { BackgroundData } from '../webview/MessageList';
 import {
@@ -33,6 +33,11 @@ type ButtonDescription = {
     _: GetText,
   }): void | Promise<void>,
   title: string,
+
+  /** The title of the alert-box that will be displayed if the callback throws. */
+  // Required even when the callback can't throw (e.g., "Cancel"), since we can't
+  // otherwise ensure that everything that _can_ throw has one.
+  errorMessage: string,
 };
 
 const isAnOutboxMessage = (message: Message | Outbox): boolean => message.isOutbox;
@@ -45,6 +50,7 @@ const reply = ({ message, dispatch, ownEmail }) => {
   dispatch(doNarrow(getNarrowFromMessage(message, ownEmail), message.id));
 };
 reply.title = 'Reply';
+reply.errorMessage = 'Failed to reply';
 
 const copyToClipboard = async ({ _, auth, message }) => {
   const rawMessage = isAnOutboxMessage(message) /* $FlowFixMe: then really type Outbox */
@@ -54,76 +60,89 @@ const copyToClipboard = async ({ _, auth, message }) => {
   showToast(_('Message copied'));
 };
 copyToClipboard.title = 'Copy to clipboard';
+copyToClipboard.errorMessage = 'Failed to copy message to clipboard';
 
 const editMessage = async ({ message, dispatch }) => {
   dispatch(startEditMessage(message.id, message.subject));
 };
 editMessage.title = 'Edit message';
+editMessage.errorMessage = 'Failed to edit message';
 
 const deleteMessage = async ({ auth, message, dispatch }) => {
   if (isAnOutboxMessage(message)) {
     dispatch(deleteOutboxMessage(message.timestamp));
   } else {
-    api.deleteMessage(auth, message.id);
+    await api.deleteMessage(auth, message.id);
   }
 };
 deleteMessage.title = 'Delete message';
+deleteMessage.errorMessage = 'Failed to delete message';
 
-const unmuteTopic = ({ auth, message }) => {
-  api.unmuteTopic(auth, message.display_recipient, message.subject);
+const unmuteTopic = async ({ auth, message }) => {
+  await api.unmuteTopic(auth, message.display_recipient, message.subject);
 };
 unmuteTopic.title = 'Unmute topic';
+unmuteTopic.errorMessage = 'Failed to unmute topic';
 
-const muteTopic = ({ auth, message }) => {
-  api.muteTopic(auth, message.display_recipient, message.subject);
+const muteTopic = async ({ auth, message }) => {
+  await api.muteTopic(auth, message.display_recipient, message.subject);
 };
 muteTopic.title = 'Mute topic';
+muteTopic.errorMessage = 'Failed to mute topic';
 
-const unmuteStream = ({ auth, message, subscriptions }) => {
+const unmuteStream = async ({ auth, message, subscriptions }) => {
   const sub = subscriptions.find(x => x.name === message.display_recipient);
   if (sub) {
-    api.toggleMuteStream(auth, sub.stream_id, false);
+    await api.toggleMuteStream(auth, sub.stream_id, false);
   }
 };
 unmuteStream.title = 'Unmute stream';
+unmuteStream.errorMessage = 'Failed to unmute stream';
 
-const muteStream = ({ auth, message, subscriptions }) => {
+const muteStream = async ({ auth, message, subscriptions }) => {
   const sub = subscriptions.find(x => x.name === message.display_recipient);
   if (sub) {
-    api.toggleMuteStream(auth, sub.stream_id, true);
+    await api.toggleMuteStream(auth, sub.stream_id, true);
   }
 };
 muteStream.title = 'Mute stream';
+muteStream.errorMessage = 'Failed to mute stream';
 
-const starMessage = ({ auth, message }) => {
-  api.toggleMessageStarred(auth, [message.id], true);
+const starMessage = async ({ auth, message }) => {
+  await api.toggleMessageStarred(auth, [message.id], true);
 };
 starMessage.title = 'Star message';
+starMessage.errorMessage = 'Failed to star message';
 
-const unstarMessage = ({ auth, message }) => {
-  api.toggleMessageStarred(auth, [message.id], false);
+const unstarMessage = async ({ auth, message }) => {
+  await api.toggleMessageStarred(auth, [message.id], false);
 };
 unstarMessage.title = 'Unstar message';
+unstarMessage.errorMessage = 'Failed to unstar message';
 
 const shareMessage = ({ message }) => {
   Share.share({
     message: message.content.replace(/<(?:.|\n)*?>/gm, ''),
   });
 };
 shareMessage.title = 'Share';
+shareMessage.errorMessage = 'Failed to share message';
 
 const addReaction = ({ message, dispatch }) => {
   dispatch(navigateToEmojiPicker(message.id));
 };
 addReaction.title = 'Add a reaction';
+addReaction.errorMessage = 'Failed to add reaction';
 
 const showReactions = ({ message, dispatch }) => {
   dispatch(navigateToMessageReactionScreen(message.id));
 };
 showReactions.title = 'See who reacted';
+showReactions.errorMessage = 'Failed to show reactions';
 
 const cancel = params => {};
 cancel.title = 'Cancel';
+cancel.errorMessage = 'Failed to hide menu';
 
 const allButtonsRaw = {
   // For messages
@@ -239,14 +258,21 @@ export const showActionSheet = (
     ? constructHeaderActionButtons(params)
     : constructMessageActionButtons(params);
   const callback = buttonIndex => {
-    allButtons[optionCodes[buttonIndex]]({
-      dispatch,
-      subscriptions: params.backgroundData.subscriptions,
-      auth: params.backgroundData.auth,
-      ownEmail: params.backgroundData.ownEmail,
-      _,
-      ...params,
-    });
+    (async () => {
+      const pressedButton: ButtonDescription = allButtons[optionCodes[buttonIndex]];
+      try {
+        await pressedButton({
+          dispatch,
+          subscriptions: params.backgroundData.subscriptions,
+          auth: params.backgroundData.auth,
+          ownEmail: params.backgroundData.ownEmail,
+          _,
+          ...params,
+        });
+      } catch (err) {
+        Alert.alert(_(pressedButton.errorMessage), err.message);
+      }
+    })();
   };
   showActionSheetWithOptions(
     {

diff --git a/static/translations/messages_en.json b/static/translations/messages_en.json
@@ -121,6 +121,20 @@
   "Message copied": "Message copied",
   "Edit message": "Edit message",
   "Network request failed": "Network request failed",
+  "Failed to add reaction": "Failed to add reaction",
+  "Failed to reply": "Failed to reply",
+  "Failed to copy message to clipboard": "Failed to copy message to clipboard",
+  "Failed to share message": "Failed to share message",
+  "Failed to edit message": "Failed to edit message",
+  "Failed to delete message": "Failed to delete message",
+  "Failed to star message": "Failed to star message",
+  "Failed to unstar message": "Failed to unstar message",
+  "Failed to show reactions": "Failed to show reactions",
+  "Failed to hide menu": "Failed to hide menu",
+  "Failed to unmute topic": "Failed to unmute topic",
+  "Failed to mute topic": "Failed to mute topic",
+  "Failed to mute stream": "Failed to mute stream",
+  "Failed to unmute stream": "Failed to unmute stream",
   "show": "show",
   "hide": "hide",
   "Debug": "Debug",