Skip to content

Commit 8caf6dc

Browse files
chrisbobbegnprice
authored andcommitted
ReadReceiptsScreen: Add, with TODO to show data state more visibly
This makes it possible to view read receipts. Fixes: #5367
1 parent 468ac71 commit 8caf6dc

File tree

7 files changed

+253
-1
lines changed

7 files changed

+253
-1
lines changed

src/action-sheets/__tests__/action-sheet-test.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,4 +169,18 @@ describe('constructMessageActionButtons', () => {
169169
const flags = { ...eg.plusBackgroundData.flags, starred: { [message.id]: true } };
170170
expect(titles({ ...eg.plusBackgroundData, flags }, message)).toContain('Unstar message');
171171
});
172+
173+
describe('viewReadReceipts', () => {
174+
test('show if read receipts enabled for org', () => {
175+
expect(
176+
titles({ ...eg.plusBackgroundData, enableReadReceipts: true }, eg.streamMessage()),
177+
).toContain('View read receipts');
178+
});
179+
180+
test('hide if read receipts not enabled for org', () => {
181+
expect(
182+
titles({ ...eg.plusBackgroundData, enableReadReceipts: false }, eg.streamMessage()),
183+
).not.toContain('View read receipts');
184+
});
185+
});
172186
});

src/action-sheets/index.js

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import { pmUiRecipientsFromKeyRecipients } from '../utils/recipient';
3131
import type { PmKeyRecipients } from '../utils/recipient';
3232
import { isTopicMuted } from '../mute/muteModel';
3333
import * as api from '../api';
34-
import { showConfirmationDialog, showToast } from '../utils/info';
34+
import { showConfirmationDialog, showErrorAlert, showToast } from '../utils/info';
3535
import {
3636
doNarrow,
3737
deleteOutboxMessage,
@@ -42,6 +42,7 @@ import {
4242
import {
4343
navigateToMessageReactionScreen,
4444
navigateToPmConversationDetails,
45+
navigateToReadReceiptsScreen,
4546
} from '../nav/navActions';
4647
import { deleteMessagesForTopic } from '../topics/topicActions';
4748
import * as logging from '../utils/logging';
@@ -51,6 +52,7 @@ import { getStreamTopicUrl, getStreamUrl } from '../utils/internalLinks';
5152
import { reactionTypeFromEmojiType } from '../emoji/data';
5253
import { Role, type RoleT } from '../api/permissionsTypes';
5354
import { roleIsAtLeast } from '../permissionSelectors';
55+
import { kNotificationBotEmail } from '../api/constants';
5456

5557
// TODO really this belongs in a libdef.
5658
export type ShowActionSheetWithOptions = (
@@ -433,6 +435,34 @@ const showReactions = {
433435
},
434436
};
435437

438+
const viewReadReceipts = {
439+
title: 'View read receipts',
440+
errorMessage: 'Failed to show read receipts',
441+
action: ({ message, _ }) => {
442+
// Notification Bot messages about resolved/unresolved topics have
443+
// confusing read receipts. Because those messages are immediately
444+
// marked as read for all non-participants in the thread, it looks
445+
// like many people have immediately read the message. So, disable
446+
// showing read receipts for messages sent by Notification Bot. See
447+
// https://github.com/zulip/zulip/issues/22905 .
448+
if (message.sender_email === kNotificationBotEmail) {
449+
// We might instead have handled this in the read-receipts screen by
450+
// showing this message there. But it's awkward code-wise to make that
451+
// screen sometimes skip the API call, api.getReadReceipts. And it's
452+
// nice to skip that API call because it lets the server add a
453+
// server-side check for this, if it wants, without fear of breaking
454+
// mobile releases that haven't adapted.
455+
showErrorAlert(
456+
_('Read receipts'),
457+
_('Read receipts are not available for Notification Bot messages.'),
458+
);
459+
return;
460+
}
461+
462+
NavigationService.dispatch(navigateToReadReceiptsScreen(message.id));
463+
},
464+
};
465+
436466
const cancel: Button<{ ... }> = {
437467
title: 'Cancel',
438468
errorMessage: 'Failed to hide menu',
@@ -595,6 +625,9 @@ export const constructMessageActionButtons = (args: {|
595625
} else {
596626
buttons.push(starMessage);
597627
}
628+
if (backgroundData.enableReadReceipts) {
629+
buttons.push(viewReadReceipts);
630+
}
598631
buttons.push(cancel);
599632
return buttons;
600633
};

src/api/constants.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,17 @@
1010
// This is hardcoded in the server, and therefore untranslated; that's
1111
// zulip/zulip#3639.
1212
export const kNoTopicTopic: string = '(no topic)';
13+
14+
/**
15+
* The notification bot's email address on a typical production server.
16+
*
17+
* Helpful for identifying which cross-realm bot is the notification bot, or
18+
* knowing if a message was sent by the notification bot.
19+
*
20+
* Of course, this won't be much use if the server we're dealing with isn't
21+
* typical and uses a custom value for the email address.
22+
*/
23+
// Copied from
24+
// https://github.com/zulip/zulip/blob/f6f7e4c53/zproject/default_settings.py#L231 .
25+
// TODO: Make all servers give us a way to identify the notification bot.
26+
export const kNotificationBotEmail: string = '[email protected]';

src/message/ReadReceiptsScreen.js

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
/* @flow strict-local */
2+
3+
import React, { useCallback, useContext, useMemo } from 'react';
4+
import type { Node } from 'react';
5+
import { View, FlatList } from 'react-native';
6+
import { SafeAreaView } from 'react-native-safe-area-context';
7+
8+
import useFetchedDataWithRefresh from '../common/useFetchedDataWithRefresh';
9+
import ZulipTextIntl from '../common/ZulipTextIntl';
10+
import { useGlobalSelector, useSelector } from '../react-redux';
11+
import * as api from '../api';
12+
import type { AppNavigationProp } from '../nav/AppNavigator';
13+
import type { RouteProp } from '../react-navigation';
14+
import Screen from '../common/Screen';
15+
import { useConditionalEffect, useHasNotChangedForMs, useHasStayedTrueForMs } from '../reactUtils';
16+
import { getAuth, getZulipFeatureLevel } from '../account/accountsSelectors';
17+
import { showToast } from '../utils/info';
18+
import { TranslationContext } from '../boot/TranslationProvider';
19+
import UserItem from '../users/UserItem';
20+
import { tryGetUserForId } from '../users/userSelectors';
21+
import type { UserId } from '../api/idTypes';
22+
import { getGlobalSettings } from '../directSelectors';
23+
import type { UserOrBot } from '../api/modelTypes';
24+
import LoadingIndicator from '../common/LoadingIndicator';
25+
import WebLink from '../common/WebLink';
26+
import { createStyleSheet } from '../styles';
27+
import ZulipText from '../common/ZulipText';
28+
29+
type Props = $ReadOnly<{|
30+
navigation: AppNavigationProp<'read-receipts'>,
31+
route: RouteProp<'read-receipts', {| +messageId: number |}>,
32+
|}>;
33+
34+
export default function ReadReceiptsScreen(props: Props): Node {
35+
const { navigation } = props;
36+
const { messageId } = props.route.params;
37+
38+
const auth = useSelector(getAuth);
39+
const zulipFeatureLevel = useSelector(getZulipFeatureLevel);
40+
const language = useGlobalSelector(state => getGlobalSettings(state).language);
41+
const _ = useContext(TranslationContext);
42+
43+
const callApiMethod = useCallback(
44+
() => api.getReadReceipts(auth, { message_id: messageId }, zulipFeatureLevel),
45+
[auth, messageId, zulipFeatureLevel],
46+
);
47+
48+
const { latestResult, latestSuccessResult } = useFetchedDataWithRefresh(callApiMethod, 15_000);
49+
50+
// TODO: These vanishing toasts are easy to miss. Instead, show
51+
// latestResultIsError, isFirstLoadLate, and haveStaleData with
52+
// something more persistent. A Material Design Snackbar that stays
53+
// until the user dismisses it or the problem resolves?
54+
// https://callstack.github.io/react-native-paper/snackbar.html
55+
56+
const latestResultIsError = latestResult?.type === 'error';
57+
useConditionalEffect(
58+
useCallback(() => {
59+
showToast(_('Could not load data.'));
60+
}, [_]),
61+
latestResultIsError,
62+
);
63+
64+
const isFirstLoadLate = useHasStayedTrueForMs(latestSuccessResult === null, 10_000);
65+
useConditionalEffect(
66+
useCallback(() => showToast(_('Still working…')), [_]),
67+
isFirstLoadLate
68+
// If the latest result was an error, we would've shown a "Could not
69+
// load data" message, which sounds final. Reduce confusion by
70+
// suppressing a "Still working…" message, even as the Hook continues
71+
// to try to load the data. Confusion, and also false hope: if the
72+
// last fetch failed, we're not optimistic about this one.
73+
&& !latestResultIsError,
74+
);
75+
76+
const haveStaleData =
77+
useHasNotChangedForMs(latestSuccessResult, 40_000) && latestSuccessResult !== null;
78+
useConditionalEffect(
79+
useCallback(() => showToast(_('Updates may be delayed.')), [_]),
80+
haveStaleData,
81+
);
82+
83+
const onPressUser = useCallback(
84+
(user: UserOrBot) => {
85+
navigation.push('account-details', { userId: user.user_id });
86+
},
87+
[navigation],
88+
);
89+
90+
// The web app tries Intl.Collator too, with a fallback for browsers that
91+
// don't support it. See `strcmp` in static/js/util.js in the web app. Our
92+
// platforms should support it:
93+
// - MDN shows that our simple usage here is supported since iOS 10:
94+
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/Collator
95+
// And we desupported iOS 10 a long time ago.
96+
// - On Android, I don't get an error that suggests an API is missing.
97+
// And it looks like Hermes, which we hope to switch to soon, supports it:
98+
// https://github.com/facebook/hermes/issues/23#issuecomment-1156832485
99+
const userSorter = useCallback(
100+
(a, b) => Intl.Collator(language).compare(a.full_name, b.full_name),
101+
[language],
102+
);
103+
104+
const displayUserIds = useSelector(state => {
105+
const userIds: $ReadOnlyArray<UserId> = latestSuccessResult?.data ?? [];
106+
const result = [];
107+
108+
userIds.forEach(userId => {
109+
const user = tryGetUserForId(state, userId);
110+
if (!user) {
111+
// E.g., data out of sync because we're working outside the event
112+
// system. Shrug, drop this one.
113+
return;
114+
}
115+
result.push(user);
116+
});
117+
result.sort(userSorter);
118+
119+
return result.map(user => user.user_id);
120+
});
121+
122+
const renderItem = useCallback(
123+
({ item }) => <UserItem key={item} userId={item} onPress={onPressUser} />,
124+
[onPressUser],
125+
);
126+
127+
const localizableSummaryText = useMemo(
128+
() =>
129+
displayUserIds.length > 0
130+
? {
131+
// This is actually the same string as in the web app; see where
132+
// that's set in static/js/read_receipts.js
133+
text: `\
134+
{num_of_people, plural,
135+
one {This message has been <z-link>read</z-link> by {num_of_people} person:}
136+
other {This message has been <z-link>read</z-link> by {num_of_people} people:}\
137+
}`,
138+
values: {
139+
num_of_people: displayUserIds.length,
140+
'z-link': chunks => (
141+
<WebLink url={new URL('/help/read-receipts', auth.realm)}>
142+
{chunks.map(chunk => (
143+
<ZulipText>{chunk}</ZulipText>
144+
))}
145+
</WebLink>
146+
),
147+
},
148+
}
149+
: 'No one has read this message yet.',
150+
[auth.realm, displayUserIds.length],
151+
);
152+
153+
const styles = useMemo(
154+
() =>
155+
createStyleSheet({
156+
summaryTextWrapper: { padding: 16 },
157+
flex1: { flex: 1 },
158+
}),
159+
[],
160+
);
161+
162+
return (
163+
<Screen title="Read receipts" scrollEnabled={false}>
164+
{latestSuccessResult ? (
165+
<View style={styles.flex1}>
166+
<SafeAreaView mode="padding" edges={['right', 'left']} style={styles.summaryTextWrapper}>
167+
<ZulipTextIntl text={localizableSummaryText} />
168+
</SafeAreaView>
169+
<FlatList style={styles.flex1} data={displayUserIds} renderItem={renderItem} />
170+
</View>
171+
) : (
172+
<LoadingIndicator size={48} />
173+
)}
174+
</Screen>
175+
);
176+
}

src/nav/AppNavigator.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import UserStatusScreen from '../user-statuses/UserStatusScreen';
5353
import SharingScreen from '../sharing/SharingScreen';
5454
import SelectableOptionsScreen from '../common/SelectableOptionsScreen';
5555
import StreamListScreen from '../subscriptions/StreamListScreen';
56+
import ReadReceiptsScreen from '../message/ReadReceiptsScreen';
5657
import { useHaveServerDataGate } from '../withHaveServerDataGate';
5758

5859
export type AppNavigatorParamList = {|
@@ -89,6 +90,7 @@ export type AppNavigatorParamList = {|
8990
+sharing: RouteParamsOf<typeof SharingScreen>,
9091
+settings: RouteParamsOf<typeof SettingsScreen>,
9192
+'selectable-options': RouteParamsOf<typeof SelectableOptionsScreen>,
93+
+'read-receipts': RouteParamsOf<typeof ReadReceiptsScreen>,
9294
|};
9395

9496
/**
@@ -197,6 +199,7 @@ export default function AppNavigator(props: Props): Node {
197199
<Stack.Screen name="legal" component={useHaveServerDataGate(LegalScreen)} />
198200
<Stack.Screen name="user-status" component={useHaveServerDataGate(UserStatusScreen)} />
199201
<Stack.Screen name="settings" component={useHaveServerDataGate(SettingsScreen)} />
202+
<Stack.Screen name="read-receipts" component={useHaveServerDataGate(ReadReceiptsScreen)} />
200203

201204
{/* These screens do not expect server data in order to function
202205
normally. */}

src/nav/navActions.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,5 +68,8 @@ export const navigateToMessageReactionScreen = (
6868
reactionName?: string,
6969
): NavigationAction => StackActions.push('message-reactions', { messageId, reactionName });
7070

71+
export const navigateToReadReceiptsScreen = (messageId: number): NavigationAction =>
72+
StackActions.push('read-receipts', { messageId });
73+
7174
export const navigateToSharing = (sharedData: SharedData): NavigationAction =>
7275
StackActions.push('sharing', { sharedData });

static/translations/messages_en.json

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
11
{
2+
"{num_of_people, plural,\n one {This message has been <z-link>read</z-link> by {num_of_people} person:}\n other {This message has been <z-link>read</z-link> by {num_of_people} people:}}": "{num_of_people, plural,\n one {This message has been <z-link>read</z-link> by {num_of_people} person:}\n other {This message has been <z-link>read</z-link> by {num_of_people} people:}}",
3+
"View read receipts": "View read receipts",
4+
"Failed to show read receipts": "Failed to show read receipts",
5+
"Read receipts": "Read receipts",
6+
"Read receipts are not available for Notification Bot messages.": "Read receipts are not available for Notification Bot messages.",
7+
"Could not load data.": "Could not load data.",
8+
"Still working…": "Still working…",
9+
"Updates may be delayed.": "Updates may be delayed.",
10+
"No one has read this message yet.": "No one has read this message yet.",
211
"Confirm": "Confirm",
312
"Configure permissions": "Configure permissions",
413
"You": "You",

0 commit comments

Comments
 (0)