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

Author: AkashDhiman

src/message/__tests__/messageActionSheet-test.js

@@ -1,7 +1,6 @@
 // @flow strict-local
 import deepFreeze from 'deep-freeze';
 import { HOME_NARROW } from '../../utils/narrow';
-import { streamNameOfStreamMessage } from '../../utils/recipient';
 
 import * as eg from '../../__tests__/lib/exampleData';
 import { constructMessageActionButtons, constructTopicActionButtons } from '../messageActionSheet';
@@ -66,7 +65,7 @@ describe('constructTopicActionButtons', () => {
     ]);
     const buttons = constructTopicActionButtons({
       backgroundData: { ...baseBackgroundData, subscriptions, unreadStreams },
-      stream: 'test stream',
+      streamId: stream.stream_id,
       topic: 'test topic',
     });
     expect(buttonTitles(buttons)).toContain('Mark topic as read');
@@ -80,65 +79,75 @@ describe('constructTopicActionButtons', () => {
     ]);
     const buttons = constructTopicActionButtons({
       backgroundData: { ...baseBackgroundData, subscriptions, unreadStreams },
-      stream: 'test stream',
+      streamId: stream.stream_id,
       topic: 'read topic',
     });
     expect(buttonTitles(buttons)).not.toContain('Mark topic as read');
   });
 
   test('show Unmute topic option if topic is muted', () => {
     const mute = deepFreeze([['electron issues', 'issue #556']]);
+    const stream = eg.makeStream({ name: 'electron issues' });
+    const subscriptions = [{ ...eg.subscription, ...stream }];
     const buttons = constructTopicActionButtons({
-      backgroundData: { ...baseBackgroundData, mute },
-      stream: 'electron issues',
+      backgroundData: { ...baseBackgroundData, subscriptions, mute },
+      streamId: stream.stream_id,
       topic: 'issue #556',
     });
     expect(buttonTitles(buttons)).toContain('Unmute topic');
   });
 
   test('show mute topic option if topic is not muted', () => {
+    const stream = eg.makeStream();
+    const subscriptions = [{ ...eg.subscription, ...stream }];
     const buttons = constructTopicActionButtons({
-      backgroundData: { ...baseBackgroundData, mute: [] },
-      stream: streamNameOfStreamMessage(eg.streamMessage()),
+      backgroundData: { ...baseBackgroundData, subscriptions, mute: [] },
+      streamId: stream.stream_id,
       topic: eg.streamMessage().subject,
     });
     expect(buttonTitles(buttons)).toContain('Mute topic');
   });
 
   test('show Unmute stream option if stream is not in home view', () => {
-    const subscriptions = [{ ...eg.subscription, in_home_view: false }];
+    const stream = eg.makeStream();
+    const subscriptions = [{ ...eg.subscription, in_home_view: false, ...stream }];
     const buttons = constructTopicActionButtons({
       backgroundData: { ...baseBackgroundData, subscriptions },
-      stream: streamNameOfStreamMessage(eg.streamMessage()),
+      streamId: stream.stream_id,
       topic: eg.streamMessage().subject,
     });
     expect(buttonTitles(buttons)).toContain('Unmute stream');
   });
 
   test('show mute stream option if stream is in home view', () => {
-    const subscriptions = [{ ...eg.subscription, in_home_view: true }];
+    const stream = eg.makeStream();
+    const subscriptions = [{ ...eg.subscription, in_home_view: true, ...stream }];
     const buttons = constructTopicActionButtons({
       backgroundData: { ...baseBackgroundData, subscriptions },
-      stream: streamNameOfStreamMessage(eg.streamMessage()),
+      streamId: stream.stream_id,
       topic: eg.streamMessage().subject,
     });
     expect(buttonTitles(buttons)).toContain('Mute stream');
   });
 
   test('show delete topic option if current user is an admin', () => {
+    const stream = eg.makeStream();
+    const subscriptions = [{ ...eg.subscription, ...stream }];
     const ownUser = { ...eg.selfUser, is_admin: true };
     const buttons = constructTopicActionButtons({
-      backgroundData: { ...baseBackgroundData, ownUser },
-      stream: streamNameOfStreamMessage(eg.streamMessage()),
+      backgroundData: { ...baseBackgroundData, ownUser, subscriptions },
+      streamId: stream.stream_id,
       topic: eg.streamMessage().subject,
     });
     expect(buttonTitles(buttons)).toContain('Delete topic');
   });
 
   test('do not show delete topic option if current user is not an admin', () => {
+    const stream = eg.makeStream();
+    const subscriptions = [{ ...eg.subscription, ...stream }];
     const buttons = constructTopicActionButtons({
-      backgroundData: baseBackgroundData,
-      stream: streamNameOfStreamMessage(eg.streamMessage()),
+      backgroundData: { ...baseBackgroundData, subscriptions },
+      streamId: stream.stream_id,
       topic: eg.streamMessage().subject,
     });
     expect(buttonTitles(buttons)).not.toContain('Delete topic');

src/message/messageActionSheet.js

@@ -38,8 +38,8 @@ export type ShowActionSheetWithOptions = (
 
 type TopicArgs = {
   auth: Auth,
-  stream: string,
   topic: string,
+  streamId: number,
   subscriptions: Subscription[],
   dispatch: Dispatch,
   _: GetText,
@@ -119,34 +119,25 @@ const deleteMessage = async ({ auth, message, dispatch }) => {
 deleteMessage.title = 'Delete message';
 deleteMessage.errorMessage = 'Failed to delete message';
 
-const markTopicAsRead = async ({ auth, stream, topic, subscriptions }) => {
-  const sub = subscriptions.find(x => x.name === stream);
-  if (sub) {
-    await api.markTopicAsRead(auth, sub.stream_id, topic);
-  }
+const markTopicAsRead = async ({ auth, streamId, topic }) => {
+  await api.markTopicAsRead(auth, streamId, topic);
 };
 markTopicAsRead.title = 'Mark topic as read';
 markTopicAsRead.errorMessage = 'Failed to mark topic as read';
 
-const unmuteTopic = async ({ auth, stream, topic, subscriptions }) => {
-  const sub = subscriptions.find(x => x.name === stream);
-  if (sub) {
-    await api.unmuteTopic(auth, sub.stream_id, topic);
-  }
+const unmuteTopic = async ({ auth, streamId, topic }) => {
+  await api.unmuteTopic(auth, streamId, topic);
 };
 unmuteTopic.title = 'Unmute topic';
 unmuteTopic.errorMessage = 'Failed to unmute topic';
 
-const muteTopic = async ({ auth, stream, topic, subscriptions }) => {
-  const sub = subscriptions.find(x => x.name === stream);
-  if (sub) {
-    await api.muteTopic(auth, sub.stream_id, topic);
-  }
+const muteTopic = async ({ auth, streamId, topic }) => {
+  await api.muteTopic(auth, streamId, topic);
 };
 muteTopic.title = 'Mute topic';
 muteTopic.errorMessage = 'Failed to mute topic';
 
-const deleteTopic = async ({ auth, stream, topic, subscriptions, dispatch, _ }) => {
+const deleteTopic = async ({ auth, streamId, topic, dispatch, _ }) => {
   const alertTitle = _('Are you sure you want to delete the topic “{topic}”?', { topic });
   const AsyncAlert = async (): Promise<boolean> =>
     new Promise((resolve, reject) => {
@@ -173,38 +164,26 @@ const deleteTopic = async ({ auth, stream, topic, subscriptions, dispatch, _ })
       );
     });
   if (await AsyncAlert()) {
-    const sub = subscriptions.find(x => x.name === stream);
-    if (sub) {
-      await dispatch(deleteMessagesForTopic(sub.stream_id, topic));
-    }
+    await dispatch(deleteMessagesForTopic(streamId, topic));
   }
 };
 deleteTopic.title = 'Delete topic';
 deleteTopic.errorMessage = 'Failed to delete topic';
 
-const unmuteStream = async ({ auth, stream, subscriptions }) => {
-  const sub = subscriptions.find(x => x.name === stream);
-  if (sub) {
-    await api.toggleMuteStream(auth, sub.stream_id, false);
-  }
+const unmuteStream = async ({ auth, streamId }) => {
+  await api.toggleMuteStream(auth, streamId, false);
 };
 unmuteStream.title = 'Unmute stream';
 unmuteStream.errorMessage = 'Failed to unmute stream';
 
-const muteStream = async ({ auth, stream, subscriptions }) => {
-  const sub = subscriptions.find(x => x.name === stream);
-  if (sub) {
-    await api.toggleMuteStream(auth, sub.stream_id, true);
-  }
+const muteStream = async ({ auth, streamId }) => {
+  await api.toggleMuteStream(auth, streamId, true);
 };
 muteStream.title = 'Mute stream';
 muteStream.errorMessage = 'Failed to mute stream';
 
-const showStreamSettings = ({ stream, subscriptions }) => {
-  const sub = subscriptions.find(x => x.name === stream);
-  if (sub) {
-    NavigationService.dispatch(navigateToStream(sub.stream_id));
-  }
+const showStreamSettings = ({ streamId }) => {
+  NavigationService.dispatch(navigateToStream(streamId));
 };
 showStreamSettings.title = 'Stream settings';
 showStreamSettings.errorMessage = 'Failed to show stream settings';
@@ -247,8 +226,8 @@ cancel.errorMessage = 'Failed to hide menu';
 
 export const constructTopicActionButtons = ({
   backgroundData: { mute, subscriptions, ownUser, unreadStreams },
-  stream,
   topic,
+  streamId,
 }: {|
   backgroundData: $ReadOnly<{
     mute: MuteState,
@@ -257,29 +236,27 @@ export const constructTopicActionButtons = ({
     unreadStreams: UnreadStreamsState,
     ...
   }>,
-  stream: string,
+  streamId: number,
   topic: string,
 |}): Button<TopicArgs>[] => {
+  const sub = subscriptions.find(x => x.stream_id === streamId);
   const buttons = [];
   if (ownUser.is_admin) {
     buttons.push(deleteTopic);
   }
-  if (isTopicMuted(stream, topic, mute)) {
+  if (sub && isTopicMuted(sub.name, topic, mute)) {
     buttons.push(unmuteTopic);
   } else {
     buttons.push(muteTopic);
   }
-  const sub = subscriptions.find(x => x.name === stream);
-  if (sub) {
-    const unreadCount = unreadStreams.get(sub.stream_id)?.get(topic)?.size;
-    if (unreadCount !== undefined && unreadCount > 0) {
-      buttons.push(markTopicAsRead);
-    }
-    if (!sub.in_home_view) {
-      buttons.push(unmuteStream);
-    } else {
-      buttons.push(muteStream);
-    }
+  const unreadCount = unreadStreams.get(streamId)?.get(topic)?.size;
+  if (unreadCount !== undefined && unreadCount > 0) {
+    buttons.push(markTopicAsRead);
+  }
+  if (sub && !sub.in_home_view) {
+    buttons.push(unmuteStream);
+  } else {
+    buttons.push(muteStream);
   }
   buttons.push(showStreamSettings);
   buttons.push(cancel);
@@ -420,7 +397,7 @@ export const showTopicActionSheet = ({
   callbacks,
   backgroundData,
   topic,
-  stream,
+  streamId,
 }: {|
   showActionSheetWithOptions: ShowActionSheetWithOptions,
   callbacks: {|
@@ -436,12 +413,22 @@ export const showTopicActionSheet = ({
     unreadStreams: UnreadStreamsState,
     ...
   }>,
-  stream: string,
+  streamId: number,
   topic: string,
 |}): void => {
+  const sub = backgroundData.subscriptions.find(x => x.stream_id === streamId);
+  try {
+    if (!sub) {
+      throw Error('Stream id does not exist.');
+    }
+  } catch (err) {
+    logging.error(err);
+    return;
+  }
+  const stream = sub.name;
   const buttonList = constructTopicActionButtons({
     backgroundData,
-    stream,
+    streamId,
     topic,
   });
   showActionSheetWithOptions(
@@ -453,7 +440,7 @@ export const showTopicActionSheet = ({
     makeButtonCallback(buttonList, {
       ...backgroundData,
       ...callbacks,
-      stream,
+      streamId,
       topic,
     }),
   );

src/streams/TopicItem.js

@@ -11,6 +11,8 @@ import { TranslationContext } from '../boot/TranslationProvider';
 import { useDispatch, useSelector } from '../react-redux';
 import { getAuth, getMute, getFlags, getSubscriptions, getOwnUser } from '../selectors';
 import { getUnreadStreams } from '../unread/unreadModel';
+import { showToast } from '../utils/info';
+import * as logging from '../utils/logging';
 
 const componentStyles = createStyleSheet({
   selectedRow: {
@@ -52,15 +54,27 @@ export default function TopicItem(props: Props) {
     unreadStreams: getUnreadStreams(state),
   }));
 
+  const subscription = backgroundData.subscriptions.find(x => x.name === stream);
+
   return (
     <Touchable
       onPress={() => onPress(stream, name)}
       onLongPress={() => {
+        try {
+          if (!subscription) {
+            throw Error('Could not find subscription with given stream name.');
+          }
+        } catch (err) {
+          logging.error(err);
+          showToast(_('Something went wrong.'));
+          return;
+        }
+
         showTopicActionSheet({
           showActionSheetWithOptions,
           callbacks: { dispatch, _ },
           backgroundData,
-          stream,
+          streamId: subscription.stream_id,
           topic: name,
         });
       }}

src/title/TitleStream.js

@@ -80,7 +80,7 @@ const TitleStream = (props: Props) => {
                 showActionSheetWithOptions,
                 callbacks: { dispatch, _ },
                 backgroundData,
-                stream: stream.name,
+                streamId: stream.stream_id,
                 topic: topicOfNarrow(narrow),
               });
             }

src/webview/handleOutboundEvents.js

@@ -214,11 +214,24 @@ const handleLongPress = (
   const { dispatch, showActionSheetWithOptions, backgroundData, narrow, startEditMessage } = props;
   if (target === 'header') {
     if (message.type === 'stream') {
+      const stream = streamNameOfStreamMessage(message);
+      const subscription = backgroundData.subscriptions.find(x => x.name === stream);
+
+      try {
+        if (!subscription) {
+          throw Error('Could not find subscription with given stream name.');
+        }
+      } catch (err) {
+        logging.error(err);
+        showToast(_('Something went wrong.'));
+        return;
+      }
+
       showTopicActionSheet({
         showActionSheetWithOptions,
         callbacks: { dispatch, _ },
         backgroundData,
-        stream: streamNameOfStreamMessage(message),
+        streamId: subscription.stream_id,
         topic: message.subject,
       });
     } else if (message.type === 'private') {

static/translations/messages_en.json

@@ -232,5 +232,6 @@
   "Signed out": "Signed out",
   "Remove account?": "Remove account?",
   "This will make the mobile app on this device forget {email} on {realmUrl}.": "This will make the mobile app on this device forget {email} on {realmUrl}.",
-  "Remove account": "Remove account"
+  "Remove account": "Remove account",
+  "Something went wrong.": "Something went wrong."
 }