From fc660d61543c34243b825a53f9c580230a583b3c Mon Sep 17 00:00:00 2001 From: Divyanshu Agrawal Date: Mon, 13 Jan 2020 18:07:53 +0530 Subject: [PATCH] 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. --- src/message/messageActionSheet.js | 70 +++++++++++++++++++--------- static/translations/messages_en.json | 14 ++++++ 2 files changed, 62 insertions(+), 22 deletions(-) diff --git a/src/message/messageActionSheet.js b/src/message/messageActionSheet.js index c7697d0db03..1fdcb767e6e 100644 --- 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, 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,56 +60,65 @@ 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({ @@ -111,19 +126,23 @@ const shareMessage = ({ message }) => { }); }; 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 index ae180c2f115..10587c4a16e 100644 --- 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",