Skip to content

Commit 2e0b7c9

Browse files
committed
ActionSheet [nfc]: Modify code to use stream_id instead of stream name.
This commit removes stream name as a parameter to functions related to `TopicActionSheet` and instead introduces streamId in its place. Stream ID is a better parameter since it always remains constant. Related: #3918
1 parent c56372b commit 2e0b7c9

File tree

5 files changed

+74
-71
lines changed

5 files changed

+74
-71
lines changed

src/message/__tests__/messageActionSheet-test.js

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// @flow strict-local
22
import deepFreeze from 'deep-freeze';
33
import { HOME_NARROW } from '../../utils/narrow';
4-
import { streamNameOfStreamMessage } from '../../utils/recipient';
54

65
import * as eg from '../../__tests__/lib/exampleData';
76
import { constructMessageActionButtons, constructTopicActionButtons } from '../messageActionSheet';
@@ -66,7 +65,7 @@ describe('constructTopicActionButtons', () => {
6665
]);
6766
const buttons = constructTopicActionButtons({
6867
backgroundData: { ...baseBackgroundData, subscriptions, unreadStreams },
69-
stream: 'test stream',
68+
streamId: stream.stream_id,
7069
topic: 'test topic',
7170
});
7271
expect(buttonTitles(buttons)).toContain('Mark topic as read');
@@ -80,65 +79,75 @@ describe('constructTopicActionButtons', () => {
8079
]);
8180
const buttons = constructTopicActionButtons({
8281
backgroundData: { ...baseBackgroundData, subscriptions, unreadStreams },
83-
stream: 'test stream',
82+
streamId: stream.stream_id,
8483
topic: 'read topic',
8584
});
8685
expect(buttonTitles(buttons)).not.toContain('Mark topic as read');
8786
});
8887

8988
test('show Unmute topic option if topic is muted', () => {
9089
const mute = deepFreeze([['electron issues', 'issue #556']]);
90+
const stream = eg.makeStream({ name: 'electron issues' });
91+
const subscriptions = [{ ...eg.subscription, ...stream }];
9192
const buttons = constructTopicActionButtons({
92-
backgroundData: { ...baseBackgroundData, mute },
93-
stream: 'electron issues',
93+
backgroundData: { ...baseBackgroundData, subscriptions, mute },
94+
streamId: stream.stream_id,
9495
topic: 'issue #556',
9596
});
9697
expect(buttonTitles(buttons)).toContain('Unmute topic');
9798
});
9899

99100
test('show mute topic option if topic is not muted', () => {
101+
const stream = eg.makeStream();
102+
const subscriptions = [{ ...eg.subscription, ...stream }];
100103
const buttons = constructTopicActionButtons({
101-
backgroundData: { ...baseBackgroundData, mute: [] },
102-
stream: streamNameOfStreamMessage(eg.streamMessage()),
104+
backgroundData: { ...baseBackgroundData, subscriptions, mute: [] },
105+
streamId: stream.stream_id,
103106
topic: eg.streamMessage().subject,
104107
});
105108
expect(buttonTitles(buttons)).toContain('Mute topic');
106109
});
107110

108111
test('show Unmute stream option if stream is not in home view', () => {
109-
const subscriptions = [{ ...eg.subscription, in_home_view: false }];
112+
const stream = eg.makeStream();
113+
const subscriptions = [{ ...eg.subscription, in_home_view: false, ...stream }];
110114
const buttons = constructTopicActionButtons({
111115
backgroundData: { ...baseBackgroundData, subscriptions },
112-
stream: streamNameOfStreamMessage(eg.streamMessage()),
116+
streamId: stream.stream_id,
113117
topic: eg.streamMessage().subject,
114118
});
115119
expect(buttonTitles(buttons)).toContain('Unmute stream');
116120
});
117121

118122
test('show mute stream option if stream is in home view', () => {
119-
const subscriptions = [{ ...eg.subscription, in_home_view: true }];
123+
const stream = eg.makeStream();
124+
const subscriptions = [{ ...eg.subscription, in_home_view: true, ...stream }];
120125
const buttons = constructTopicActionButtons({
121126
backgroundData: { ...baseBackgroundData, subscriptions },
122-
stream: streamNameOfStreamMessage(eg.streamMessage()),
127+
streamId: stream.stream_id,
123128
topic: eg.streamMessage().subject,
124129
});
125130
expect(buttonTitles(buttons)).toContain('Mute stream');
126131
});
127132

128133
test('show delete topic option if current user is an admin', () => {
134+
const stream = eg.makeStream();
135+
const subscriptions = [{ ...eg.subscription, ...stream }];
129136
const ownUser = { ...eg.selfUser, is_admin: true };
130137
const buttons = constructTopicActionButtons({
131-
backgroundData: { ...baseBackgroundData, ownUser },
132-
stream: streamNameOfStreamMessage(eg.streamMessage()),
138+
backgroundData: { ...baseBackgroundData, ownUser, subscriptions },
139+
streamId: stream.stream_id,
133140
topic: eg.streamMessage().subject,
134141
});
135142
expect(buttonTitles(buttons)).toContain('Delete topic');
136143
});
137144

138145
test('do not show delete topic option if current user is not an admin', () => {
146+
const stream = eg.makeStream();
147+
const subscriptions = [{ ...eg.subscription, ...stream }];
139148
const buttons = constructTopicActionButtons({
140-
backgroundData: baseBackgroundData,
141-
stream: streamNameOfStreamMessage(eg.streamMessage()),
149+
backgroundData: { ...baseBackgroundData, subscriptions },
150+
streamId: stream.stream_id,
142151
topic: eg.streamMessage().subject,
143152
});
144153
expect(buttonTitles(buttons)).not.toContain('Delete topic');

src/message/messageActionSheet.js

Lines changed: 36 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ export type ShowActionSheetWithOptions = (
3939

4040
type TopicArgs = {
4141
auth: Auth,
42-
stream: string,
4342
topic: string,
43+
streamId: number,
4444
subscriptions: Subscription[],
4545
dispatch: Dispatch,
4646
_: GetText,
@@ -120,34 +120,25 @@ const deleteMessage = async ({ auth, message, dispatch }) => {
120120
deleteMessage.title = 'Delete message';
121121
deleteMessage.errorMessage = 'Failed to delete message';
122122

123-
const markTopicAsRead = async ({ auth, stream, topic, subscriptions }) => {
124-
const sub = subscriptions.find(x => x.name === stream);
125-
if (sub) {
126-
await api.markTopicAsRead(auth, sub.stream_id, topic);
127-
}
123+
const markTopicAsRead = async ({ auth, streamId, topic }) => {
124+
await api.markTopicAsRead(auth, streamId, topic);
128125
};
129126
markTopicAsRead.title = 'Mark topic as read';
130127
markTopicAsRead.errorMessage = 'Failed to mark topic as read';
131128

132-
const unmuteTopic = async ({ auth, stream, topic, subscriptions }) => {
133-
const sub = subscriptions.find(x => x.name === stream);
134-
if (sub) {
135-
await api.setTopicMute(auth, sub.stream_id, topic, false);
136-
}
129+
const unmuteTopic = async ({ auth, streamId, topic }) => {
130+
await api.setTopicMute(auth, streamId, topic, false);
137131
};
138132
unmuteTopic.title = 'Unmute topic';
139133
unmuteTopic.errorMessage = 'Failed to unmute topic';
140134

141-
const muteTopic = async ({ auth, stream, topic, subscriptions }) => {
142-
const sub = subscriptions.find(x => x.name === stream);
143-
if (sub) {
144-
await api.setTopicMute(auth, sub.stream_id, topic, true);
145-
}
135+
const muteTopic = async ({ auth, streamId, topic }) => {
136+
await api.setTopicMute(auth, streamId, topic, true);
146137
};
147138
muteTopic.title = 'Mute topic';
148139
muteTopic.errorMessage = 'Failed to mute topic';
149140

150-
const deleteTopic = async ({ auth, stream, topic, subscriptions, dispatch, _ }) => {
141+
const deleteTopic = async ({ auth, streamId, topic, dispatch, _ }) => {
151142
const alertTitle = _('Are you sure you want to delete the topic “{topic}”?', { topic });
152143
const AsyncAlert = async (): Promise<boolean> =>
153144
new Promise((resolve, reject) => {
@@ -174,37 +165,26 @@ const deleteTopic = async ({ auth, stream, topic, subscriptions, dispatch, _ })
174165
);
175166
});
176167
if (await AsyncAlert()) {
177-
const sub = subscriptions.find(x => x.name === stream);
178-
invariant(sub !== undefined, 'Stream with provided name not found.');
179-
await dispatch(deleteMessagesForTopic(sub.stream_id, topic));
168+
await dispatch(deleteMessagesForTopic(streamId, topic));
180169
}
181170
};
182171
deleteTopic.title = 'Delete topic';
183172
deleteTopic.errorMessage = 'Failed to delete topic';
184173

185-
const unmuteStream = async ({ auth, stream, subscriptions }) => {
186-
const sub = subscriptions.find(x => x.name === stream);
187-
if (sub) {
188-
await api.setSubscriptionProperty(auth, sub.stream_id, 'is_muted', false);
189-
}
174+
const unmuteStream = async ({ auth, streamId }) => {
175+
await api.setSubscriptionProperty(auth, streamId, 'is_muted', false);
190176
};
191177
unmuteStream.title = 'Unmute stream';
192178
unmuteStream.errorMessage = 'Failed to unmute stream';
193179

194-
const muteStream = async ({ auth, stream, subscriptions }) => {
195-
const sub = subscriptions.find(x => x.name === stream);
196-
if (sub) {
197-
await api.setSubscriptionProperty(auth, sub.stream_id, 'is_muted', true);
198-
}
180+
const muteStream = async ({ auth, streamId }) => {
181+
await api.setSubscriptionProperty(auth, streamId, 'is_muted', true);
199182
};
200183
muteStream.title = 'Mute stream';
201184
muteStream.errorMessage = 'Failed to mute stream';
202185

203-
const showStreamSettings = ({ stream, subscriptions }) => {
204-
const sub = subscriptions.find(x => x.name === stream);
205-
if (sub) {
206-
NavigationService.dispatch(navigateToStream(sub.stream_id));
207-
}
186+
const showStreamSettings = ({ streamId }) => {
187+
NavigationService.dispatch(navigateToStream(streamId));
208188
};
209189
showStreamSettings.title = 'Stream settings';
210190
showStreamSettings.errorMessage = 'Failed to show stream settings';
@@ -247,8 +227,8 @@ cancel.errorMessage = 'Failed to hide menu';
247227

248228
export const constructTopicActionButtons = ({
249229
backgroundData: { mute, subscriptions, ownUser, unreadStreams },
250-
stream,
251230
topic,
231+
streamId,
252232
}: {|
253233
backgroundData: $ReadOnly<{
254234
mute: MuteState,
@@ -257,29 +237,29 @@ export const constructTopicActionButtons = ({
257237
unreadStreams: UnreadStreamsState,
258238
...
259239
}>,
260-
stream: string,
240+
streamId: number,
261241
topic: string,
262242
|}): Button<TopicArgs>[] => {
243+
const sub = subscriptions.find(x => x.stream_id === streamId);
244+
invariant(sub, 'Subscription with provided stream id does not exist.');
245+
263246
const buttons = [];
264247
if (ownUser.is_admin) {
265248
buttons.push(deleteTopic);
266249
}
267-
if (isTopicMuted(stream, topic, mute)) {
250+
if (isTopicMuted(sub.name, topic, mute)) {
268251
buttons.push(unmuteTopic);
269252
} else {
270253
buttons.push(muteTopic);
271254
}
272-
const sub = subscriptions.find(x => x.name === stream);
273-
if (sub) {
274-
const unreadCount = unreadStreams.get(sub.stream_id)?.get(topic)?.size;
275-
if (unreadCount !== undefined && unreadCount > 0) {
276-
buttons.push(markTopicAsRead);
277-
}
278-
if (!sub.in_home_view) {
279-
buttons.push(unmuteStream);
280-
} else {
281-
buttons.push(muteStream);
282-
}
255+
const unreadCount = unreadStreams.get(streamId)?.get(topic)?.size;
256+
if (unreadCount !== undefined && unreadCount > 0) {
257+
buttons.push(markTopicAsRead);
258+
}
259+
if (!sub.in_home_view) {
260+
buttons.push(unmuteStream);
261+
} else {
262+
buttons.push(muteStream);
283263
}
284264
buttons.push(showStreamSettings);
285265
buttons.push(cancel);
@@ -420,7 +400,7 @@ export const showTopicActionSheet = ({
420400
callbacks,
421401
backgroundData,
422402
topic,
423-
stream,
403+
streamId,
424404
}: {|
425405
showActionSheetWithOptions: ShowActionSheetWithOptions,
426406
callbacks: {|
@@ -436,12 +416,15 @@ export const showTopicActionSheet = ({
436416
unreadStreams: UnreadStreamsState,
437417
...
438418
}>,
439-
stream: string,
419+
streamId: number,
440420
topic: string,
441421
|}): void => {
422+
const sub = backgroundData.subscriptions.find(x => x.stream_id === streamId);
423+
invariant(sub, 'Subscription with provided stream id does not exist.');
424+
const stream = sub.name;
442425
const buttonList = constructTopicActionButtons({
443426
backgroundData,
444-
stream,
427+
streamId,
445428
topic,
446429
});
447430
showActionSheetWithOptions(
@@ -453,7 +436,7 @@ export const showTopicActionSheet = ({
453436
makeButtonCallback(buttonList, {
454437
...backgroundData,
455438
...callbacks,
456-
stream,
439+
streamId,
457440
topic,
458441
}),
459442
);

src/streams/TopicItem.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,21 @@ export default function TopicItem(props: Props) {
5252
unreadStreams: getUnreadStreams(state),
5353
}));
5454

55+
const subscription = backgroundData.subscriptions.find(x => x.name === stream);
56+
5557
return (
5658
<Touchable
5759
onPress={() => onPress(stream, name)}
5860
onLongPress={() => {
61+
if (!subscription) {
62+
return;
63+
}
64+
5965
showTopicActionSheet({
6066
showActionSheetWithOptions,
6167
callbacks: { dispatch, _ },
6268
backgroundData,
63-
stream,
69+
streamId: subscription.stream_id,
6470
topic: name,
6571
});
6672
}}

src/title/TitleStream.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ const TitleStream = (props: Props) => {
8080
showActionSheetWithOptions,
8181
callbacks: { dispatch, _ },
8282
backgroundData,
83-
stream: stream.name,
83+
streamId: stream.stream_id,
8484
topic: topicOfNarrow(narrow),
8585
});
8686
}

src/webview/handleOutboundEvents.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/* @flow strict-local */
22
import { Clipboard, Alert } from 'react-native';
3+
import invariant from 'invariant';
34

45
import * as NavigationService from '../nav/NavigationService';
56
import * as api from '../api';
@@ -214,11 +215,15 @@ const handleLongPress = (
214215
const { dispatch, showActionSheetWithOptions, backgroundData, narrow, startEditMessage } = props;
215216
if (target === 'header') {
216217
if (message.type === 'stream') {
218+
const stream = streamNameOfStreamMessage(message);
219+
const subscription = backgroundData.subscriptions.find(x => x.name === stream);
220+
invariant(subscription, 'Subscription with provided stream id does not exist.');
221+
217222
showTopicActionSheet({
218223
showActionSheetWithOptions,
219224
callbacks: { dispatch, _ },
220225
backgroundData,
221-
stream: streamNameOfStreamMessage(message),
226+
streamId: subscription.stream_id,
222227
topic: message.subject,
223228
});
224229
} else if (message.type === 'private') {

0 commit comments

Comments
 (0)