Skip to content

Commit fc660d6

Browse files
agrawal-drk-for-zulip
authored andcommitted
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 zulip#3788.
1 parent 0e28596 commit fc660d6

File tree

2 files changed

+62
-22
lines changed

2 files changed

+62
-22
lines changed

src/message/messageActionSheet.js

+48-22
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* @flow strict-local */
2-
import { Clipboard, Share } from 'react-native';
2+
import { Clipboard, Share, Alert } from 'react-native';
33
import type { Auth, Dispatch, GetText, Message, Narrow, Outbox, Subscription } from '../types';
44
import type { BackgroundData } from '../webview/MessageList';
55
import {
@@ -33,6 +33,11 @@ type ButtonDescription = {
3333
_: GetText,
3434
}): void | Promise<void>,
3535
title: string,
36+
37+
/** The title of the alert-box that will be displayed if the callback throws. */
38+
// Required even when the callback can't throw (e.g., "Cancel"), since we can't
39+
// otherwise ensure that everything that _can_ throw has one.
40+
errorMessage: string,
3641
};
3742

3843
const isAnOutboxMessage = (message: Message | Outbox): boolean => message.isOutbox;
@@ -45,6 +50,7 @@ const reply = ({ message, dispatch, ownEmail }) => {
4550
dispatch(doNarrow(getNarrowFromMessage(message, ownEmail), message.id));
4651
};
4752
reply.title = 'Reply';
53+
reply.errorMessage = 'Failed to reply';
4854

4955
const copyToClipboard = async ({ _, auth, message }) => {
5056
const rawMessage = isAnOutboxMessage(message) /* $FlowFixMe: then really type Outbox */
@@ -54,76 +60,89 @@ const copyToClipboard = async ({ _, auth, message }) => {
5460
showToast(_('Message copied'));
5561
};
5662
copyToClipboard.title = 'Copy to clipboard';
63+
copyToClipboard.errorMessage = 'Failed to copy message to clipboard';
5764

5865
const editMessage = async ({ message, dispatch }) => {
5966
dispatch(startEditMessage(message.id, message.subject));
6067
};
6168
editMessage.title = 'Edit message';
69+
editMessage.errorMessage = 'Failed to edit message';
6270

6371
const deleteMessage = async ({ auth, message, dispatch }) => {
6472
if (isAnOutboxMessage(message)) {
6573
dispatch(deleteOutboxMessage(message.timestamp));
6674
} else {
67-
api.deleteMessage(auth, message.id);
75+
await api.deleteMessage(auth, message.id);
6876
}
6977
};
7078
deleteMessage.title = 'Delete message';
79+
deleteMessage.errorMessage = 'Failed to delete message';
7180

72-
const unmuteTopic = ({ auth, message }) => {
73-
api.unmuteTopic(auth, message.display_recipient, message.subject);
81+
const unmuteTopic = async ({ auth, message }) => {
82+
await api.unmuteTopic(auth, message.display_recipient, message.subject);
7483
};
7584
unmuteTopic.title = 'Unmute topic';
85+
unmuteTopic.errorMessage = 'Failed to unmute topic';
7686

77-
const muteTopic = ({ auth, message }) => {
78-
api.muteTopic(auth, message.display_recipient, message.subject);
87+
const muteTopic = async ({ auth, message }) => {
88+
await api.muteTopic(auth, message.display_recipient, message.subject);
7989
};
8090
muteTopic.title = 'Mute topic';
91+
muteTopic.errorMessage = 'Failed to mute topic';
8192

82-
const unmuteStream = ({ auth, message, subscriptions }) => {
93+
const unmuteStream = async ({ auth, message, subscriptions }) => {
8394
const sub = subscriptions.find(x => x.name === message.display_recipient);
8495
if (sub) {
85-
api.toggleMuteStream(auth, sub.stream_id, false);
96+
await api.toggleMuteStream(auth, sub.stream_id, false);
8697
}
8798
};
8899
unmuteStream.title = 'Unmute stream';
100+
unmuteStream.errorMessage = 'Failed to unmute stream';
89101

90-
const muteStream = ({ auth, message, subscriptions }) => {
102+
const muteStream = async ({ auth, message, subscriptions }) => {
91103
const sub = subscriptions.find(x => x.name === message.display_recipient);
92104
if (sub) {
93-
api.toggleMuteStream(auth, sub.stream_id, true);
105+
await api.toggleMuteStream(auth, sub.stream_id, true);
94106
}
95107
};
96108
muteStream.title = 'Mute stream';
109+
muteStream.errorMessage = 'Failed to mute stream';
97110

98-
const starMessage = ({ auth, message }) => {
99-
api.toggleMessageStarred(auth, [message.id], true);
111+
const starMessage = async ({ auth, message }) => {
112+
await api.toggleMessageStarred(auth, [message.id], true);
100113
};
101114
starMessage.title = 'Star message';
115+
starMessage.errorMessage = 'Failed to star message';
102116

103-
const unstarMessage = ({ auth, message }) => {
104-
api.toggleMessageStarred(auth, [message.id], false);
117+
const unstarMessage = async ({ auth, message }) => {
118+
await api.toggleMessageStarred(auth, [message.id], false);
105119
};
106120
unstarMessage.title = 'Unstar message';
121+
unstarMessage.errorMessage = 'Failed to unstar message';
107122

108123
const shareMessage = ({ message }) => {
109124
Share.share({
110125
message: message.content.replace(/<(?:.|\n)*?>/gm, ''),
111126
});
112127
};
113128
shareMessage.title = 'Share';
129+
shareMessage.errorMessage = 'Failed to share message';
114130

115131
const addReaction = ({ message, dispatch }) => {
116132
dispatch(navigateToEmojiPicker(message.id));
117133
};
118134
addReaction.title = 'Add a reaction';
135+
addReaction.errorMessage = 'Failed to add reaction';
119136

120137
const showReactions = ({ message, dispatch }) => {
121138
dispatch(navigateToMessageReactionScreen(message.id));
122139
};
123140
showReactions.title = 'See who reacted';
141+
showReactions.errorMessage = 'Failed to show reactions';
124142

125143
const cancel = params => {};
126144
cancel.title = 'Cancel';
145+
cancel.errorMessage = 'Failed to hide menu';
127146

128147
const allButtonsRaw = {
129148
// For messages
@@ -239,14 +258,21 @@ export const showActionSheet = (
239258
? constructHeaderActionButtons(params)
240259
: constructMessageActionButtons(params);
241260
const callback = buttonIndex => {
242-
allButtons[optionCodes[buttonIndex]]({
243-
dispatch,
244-
subscriptions: params.backgroundData.subscriptions,
245-
auth: params.backgroundData.auth,
246-
ownEmail: params.backgroundData.ownEmail,
247-
_,
248-
...params,
249-
});
261+
(async () => {
262+
const pressedButton: ButtonDescription = allButtons[optionCodes[buttonIndex]];
263+
try {
264+
await pressedButton({
265+
dispatch,
266+
subscriptions: params.backgroundData.subscriptions,
267+
auth: params.backgroundData.auth,
268+
ownEmail: params.backgroundData.ownEmail,
269+
_,
270+
...params,
271+
});
272+
} catch (err) {
273+
Alert.alert(_(pressedButton.errorMessage), err.message);
274+
}
275+
})();
250276
};
251277
showActionSheetWithOptions(
252278
{

static/translations/messages_en.json

+14
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,20 @@
121121
"Message copied": "Message copied",
122122
"Edit message": "Edit message",
123123
"Network request failed": "Network request failed",
124+
"Failed to add reaction": "Failed to add reaction",
125+
"Failed to reply": "Failed to reply",
126+
"Failed to copy message to clipboard": "Failed to copy message to clipboard",
127+
"Failed to share message": "Failed to share message",
128+
"Failed to edit message": "Failed to edit message",
129+
"Failed to delete message": "Failed to delete message",
130+
"Failed to star message": "Failed to star message",
131+
"Failed to unstar message": "Failed to unstar message",
132+
"Failed to show reactions": "Failed to show reactions",
133+
"Failed to hide menu": "Failed to hide menu",
134+
"Failed to unmute topic": "Failed to unmute topic",
135+
"Failed to mute topic": "Failed to mute topic",
136+
"Failed to mute stream": "Failed to mute stream",
137+
"Failed to unmute stream": "Failed to unmute stream",
124138
"show": "show",
125139
"hide": "hide",
126140
"Debug": "Debug",

0 commit comments

Comments
 (0)