Skip to content

Commit ae3e22e

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 stream_id in its place. Stream ID is a better parameter since it always remains constant and many of the functions present in action sheet were already dependent on stream_id. Related: #3918
1 parent 76674e6 commit ae3e22e

File tree

5 files changed

+80
-56
lines changed

5 files changed

+80
-56
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: 48 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ export type ShowActionSheetWithOptions = (
3838

3939
type TopicArgs = {
4040
auth: Auth,
41-
stream: string,
4241
topic: string,
42+
streamId?: number,
4343
subscriptions: Subscription[],
4444
dispatch: Dispatch,
4545
_: GetText,
@@ -119,28 +119,33 @@ const deleteMessage = async ({ auth, message, dispatch }) => {
119119
deleteMessage.title = 'Delete message';
120120
deleteMessage.errorMessage = 'Failed to delete message';
121121

122-
const markTopicAsRead = async ({ auth, stream, topic, subscriptions }) => {
123-
const sub = subscriptions.find(x => x.name === stream);
124-
if (sub) {
125-
await api.markTopicAsRead(auth, sub.stream_id, topic);
122+
const markTopicAsRead = async ({ auth, streamId, topic, subscriptions }) => {
123+
if (streamId !== undefined) {
124+
await api.markTopicAsRead(auth, streamId, topic);
126125
}
127126
};
128127
markTopicAsRead.title = 'Mark topic as read';
129128
markTopicAsRead.errorMessage = 'Failed to mark topic as read';
130129

131-
const unmuteTopic = async ({ auth, stream, topic }) => {
132-
await api.unmuteTopic(auth, stream, topic);
130+
const unmuteTopic = async ({ auth, streamId, topic, subscriptions }) => {
131+
const sub = subscriptions.find(x => x.stream_id === streamId);
132+
if (sub) {
133+
await api.unmuteTopic(auth, sub.name, topic);
134+
}
133135
};
134136
unmuteTopic.title = 'Unmute topic';
135137
unmuteTopic.errorMessage = 'Failed to unmute topic';
136138

137-
const muteTopic = async ({ auth, stream, topic }) => {
138-
await api.muteTopic(auth, stream, topic);
139+
const muteTopic = async ({ auth, streamId, topic, subscriptions }) => {
140+
const sub = subscriptions.find(x => x.stream_id === streamId);
141+
if (sub) {
142+
await api.muteTopic(auth, sub.name, topic);
143+
}
139144
};
140145
muteTopic.title = 'Mute topic';
141146
muteTopic.errorMessage = 'Failed to mute topic';
142147

143-
const deleteTopic = async ({ auth, stream, topic, dispatch, _ }) => {
148+
const deleteTopic = async ({ auth, streamId, topic, subscriptions, dispatch, _ }) => {
144149
const alertTitle = _('Are you sure you want to delete the topic “{topic}”?', { topic });
145150
const AsyncAlert = async (): Promise<boolean> =>
146151
new Promise((resolve, reject) => {
@@ -167,34 +172,34 @@ const deleteTopic = async ({ auth, stream, topic, dispatch, _ }) => {
167172
);
168173
});
169174
if (await AsyncAlert()) {
170-
await dispatch(deleteMessagesForTopic(stream, topic));
175+
const sub = subscriptions.find(x => x.stream_id === streamId);
176+
if (sub) {
177+
await dispatch(deleteMessagesForTopic(sub.name, topic));
178+
}
171179
}
172180
};
173181
deleteTopic.title = 'Delete topic';
174182
deleteTopic.errorMessage = 'Failed to delete topic';
175183

176-
const unmuteStream = async ({ auth, stream, subscriptions }) => {
177-
const sub = subscriptions.find(x => x.name === stream);
178-
if (sub) {
179-
await api.toggleMuteStream(auth, sub.stream_id, false);
184+
const unmuteStream = async ({ auth, streamId, subscriptions }) => {
185+
if (streamId !== undefined) {
186+
await api.toggleMuteStream(auth, streamId, false);
180187
}
181188
};
182189
unmuteStream.title = 'Unmute stream';
183190
unmuteStream.errorMessage = 'Failed to unmute stream';
184191

185-
const muteStream = async ({ auth, stream, subscriptions }) => {
186-
const sub = subscriptions.find(x => x.name === stream);
187-
if (sub) {
188-
await api.toggleMuteStream(auth, sub.stream_id, true);
192+
const muteStream = async ({ auth, streamId, subscriptions }) => {
193+
if (streamId !== undefined) {
194+
await api.toggleMuteStream(auth, streamId, true);
189195
}
190196
};
191197
muteStream.title = 'Mute stream';
192198
muteStream.errorMessage = 'Failed to mute stream';
193199

194-
const showStreamSettings = ({ stream, subscriptions }) => {
195-
const sub = subscriptions.find(x => x.name === stream);
196-
if (sub) {
197-
NavigationService.dispatch(navigateToStream(sub.stream_id));
200+
const showStreamSettings = ({ streamId, subscriptions }) => {
201+
if (streamId !== undefined) {
202+
NavigationService.dispatch(navigateToStream(streamId));
198203
}
199204
};
200205
showStreamSettings.title = 'Stream settings';
@@ -238,8 +243,8 @@ cancel.errorMessage = 'Failed to hide menu';
238243

239244
export const constructTopicActionButtons = ({
240245
backgroundData: { mute, subscriptions, ownUser, unreadStreams },
241-
stream,
242246
topic,
247+
streamId,
243248
}: {|
244249
backgroundData: $ReadOnly<{
245250
mute: MuteState,
@@ -248,29 +253,29 @@ export const constructTopicActionButtons = ({
248253
unreadStreams: UnreadStreamsState,
249254
...
250255
}>,
251-
stream: string,
256+
streamId?: number,
252257
topic: string,
253258
|}): Button<TopicArgs>[] => {
259+
const sub = subscriptions.find(x => x.stream_id === streamId);
254260
const buttons = [];
255261
if (ownUser.is_admin) {
256262
buttons.push(deleteTopic);
257263
}
258-
if (isTopicMuted(stream, topic, mute)) {
264+
if (sub && isTopicMuted(sub.name, topic, mute)) {
259265
buttons.push(unmuteTopic);
260266
} else {
261267
buttons.push(muteTopic);
262268
}
263-
const sub = subscriptions.find(x => x.name === stream);
264-
if (sub) {
265-
const unreadCount = unreadStreams.get(sub.stream_id)?.get(topic)?.size;
269+
if (streamId !== undefined) {
270+
const unreadCount = unreadStreams.get(streamId)?.get(topic)?.size;
266271
if (unreadCount !== undefined && unreadCount > 0) {
267272
buttons.push(markTopicAsRead);
268273
}
269-
if (!sub.in_home_view) {
270-
buttons.push(unmuteStream);
271-
} else {
272-
buttons.push(muteStream);
273-
}
274+
}
275+
if (sub && !sub.in_home_view) {
276+
buttons.push(unmuteStream);
277+
} else {
278+
buttons.push(muteStream);
274279
}
275280
buttons.push(showStreamSettings);
276281
buttons.push(cancel);
@@ -411,7 +416,7 @@ export const showTopicActionSheet = ({
411416
callbacks,
412417
backgroundData,
413418
topic,
414-
stream,
419+
streamId,
415420
}: {|
416421
showActionSheetWithOptions: ShowActionSheetWithOptions,
417422
callbacks: {|
@@ -427,12 +432,17 @@ export const showTopicActionSheet = ({
427432
unreadStreams: UnreadStreamsState,
428433
...
429434
}>,
430-
stream: string,
435+
streamId?: number,
431436
topic: string,
432437
|}): void => {
438+
const sub = backgroundData.subscriptions.find(x => x.stream_id === streamId);
439+
if (sub === undefined) {
440+
return;
441+
}
442+
const stream = sub.name;
433443
const buttonList = constructTopicActionButtons({
434444
backgroundData,
435-
stream,
445+
streamId,
436446
topic,
437447
});
438448
showActionSheetWithOptions(
@@ -444,7 +454,7 @@ export const showTopicActionSheet = ({
444454
makeButtonCallback(buttonList, {
445455
...backgroundData,
446456
...callbacks,
447-
stream,
457+
streamId,
448458
topic,
449459
}),
450460
);

src/streams/TopicItem.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ 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)}
@@ -60,7 +62,7 @@ export default function TopicItem(props: Props) {
6062
showActionSheetWithOptions,
6163
callbacks: { dispatch, _ },
6264
backgroundData,
63-
stream,
65+
streamId: subscription?.stream_id,
6466
topic: name,
6567
});
6668
}}

src/title/TitleStream.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ const TitleStream = (props: Props) => {
9191
showActionSheetWithOptions,
9292
callbacks: { dispatch, _ },
9393
backgroundData,
94-
stream: stream.name,
94+
streamId: stream.stream_id,
9595
topic: topicOfNarrow(narrow),
9696
});
9797
}}

src/webview/handleOutboundEvents.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,14 @@ const handleLongPress = (
214214
const { dispatch, showActionSheetWithOptions, backgroundData, narrow, startEditMessage } = props;
215215
if (target === 'header') {
216216
if (message.type === 'stream') {
217+
const stream = streamNameOfStreamMessage(message);
218+
const subscription = backgroundData.subscriptions.find(x => x.name === stream);
219+
217220
showTopicActionSheet({
218221
showActionSheetWithOptions,
219222
callbacks: { dispatch, _ },
220223
backgroundData,
221-
stream: streamNameOfStreamMessage(message),
224+
streamId: subscription?.stream_id,
222225
topic: message.subject,
223226
});
224227
} else if (message.type === 'private') {

0 commit comments

Comments
 (0)