narrow: Store stream IDs instead of names!

.eslintrc.yaml

@@ -75,12 +75,13 @@ rules:
   # of Airbnb's settings where we don't want a full repeal.
   #
 
-  # Tricky code.
+  # Tricky-looking code.  These are all sometimes perfectly legitimate.
   no-bitwise: off
   no-confusing-arrow: off
   no-continue: off
   no-plusplus: off
   no-nested-ternary: off
+  no-control-regex: off
 
   # We prefer `let foo = undefined;` over `let foo;`, not vice versa.
   #

android/app/src/main/java/com/zulipmobile/notifications/FcmMessage.kt

@@ -55,7 +55,7 @@ sealed class Recipient {
     }
 
     /** A stream message. */
-    data class Stream(val stream: String, val topic: String) : Recipient()
+    data class Stream(val streamName: String, val topic: String) : Recipient()
 }
 
 /**
@@ -116,7 +116,7 @@ data class MessageFcmMessage(
         when (recipient) {
             is Recipient.Stream -> {
                 putString("recipient_type", "stream")
-                putString("stream", recipient.stream)
+                putString("stream_name", recipient.streamName)
                 putString("topic", recipient.topic)
             }
             is Recipient.GroupPm -> {

android/app/src/main/java/com/zulipmobile/notifications/NotificationUiManager.kt

@@ -259,7 +259,7 @@ private fun extractConversationKey(fcmMessage: MessageFcmMessage): String {
         // So long as this does use the stream name, we use `\u0000` as the delimiter because
         // it's the one character not allowed in Zulip stream names.
         // (See `check_stream_name` in zulip.git:zerver/lib/streams.py.)
-        is Recipient.Stream -> "stream:${fcmMessage.recipient.stream}\u0000${fcmMessage.recipient.topic}"
+        is Recipient.Stream -> "stream:${fcmMessage.recipient.streamName}\u0000${fcmMessage.recipient.topic}"
         is Recipient.GroupPm -> "groupPM:${fcmMessage.recipient.pmUsers.toString()}"
         is Recipient.Pm -> "private:${fcmMessage.sender.id}"
     }
@@ -321,7 +321,7 @@ private fun updateNotification(
     // group-PM threads (pending #5116) get titled with the latest sender, rather than
     // the first.
     messagingStyle.setConversationTitle(when (fcmMessage.recipient) {
-        is Recipient.Stream -> "#${fcmMessage.recipient.stream} > ${fcmMessage.recipient.topic}"
+        is Recipient.Stream -> "#${fcmMessage.recipient.streamName} > ${fcmMessage.recipient.topic}"
         // TODO(#5116): use proper title for GroupPM, for which we will need
         //   to have a way to get names of PM users here.
         is Recipient.GroupPm -> context.resources.getQuantityString(

android/app/src/test/java/com/zulipmobile/notifications/FcmMessageTest.kt

@@ -127,7 +127,7 @@ class MessageFcmMessageTest : FcmMessageTestBase() {
                 ),
                 zulipMessageId = 12345,
                 recipient = Recipient.Stream(
-                    stream = Example.stream["stream"]!!,
+                    streamName = Example.stream["stream"]!!,
                     topic = Example.stream["topic"]!!
                 ),
                 content = Example.stream["content"]!!,

src/__tests__/lib/exampleData.js

@@ -47,12 +47,7 @@ import rootReducer from '../../boot/reducers';
 import { authOfAccount } from '../../account/accountMisc';
 import { HOME_NARROW } from '../../utils/narrow';
 import type { BackgroundData } from '../../webview/MessageList';
-import {
-  getSettings,
-  getStreamsById,
-  getStreamsByName,
-  getSubscriptionsById,
-} from '../../selectors';
+import { getSettings, getStreamsById, getSubscriptionsById } from '../../selectors';
 
 /* ========================================================================
  * Utilities
@@ -269,6 +264,10 @@ export const stream: Stream = makeStream({
   name: 'a stream',
   description: 'An example stream.',
 });
+export const otherStream: Stream = makeStream({
+  name: 'another stream',
+  description: 'Another example stream.',
+});
 
 /** A subscription, by default to eg.stream. */
 export const makeSubscription = (
@@ -292,6 +291,9 @@ export const makeSubscription = (
 /** A subscription to eg.stream. */
 export const subscription: Subscription = makeSubscription();
 
+/** A subscription to eg.otherStream. */
+export const otherSubscription: Subscription = makeSubscription({ stream: otherStream });
+
 /* ========================================================================
  * Messages
  */
@@ -572,8 +574,8 @@ export const plusReduxState: GlobalState & PerAccountState = reduxState({
   realm: { ...baseReduxState.realm, user_id: selfUser.user_id, email: selfUser.email },
   // TODO add crossRealmBot
   users: [selfUser, otherUser, thirdUser],
-  streams: [stream],
-  subscriptions: [subscription],
+  streams: [stream, otherStream],
+  subscriptions: [subscription, otherSubscription],
 });
 
 /**
@@ -845,7 +847,6 @@ export const backgroundData: BackgroundData = deepFreeze({
   mutedUsers: Immutable.Map(),
   ownUser: selfUser,
   streams: getStreamsById(baseReduxState),
-  streamsByName: getStreamsByName(baseReduxState),
   subscriptions: getSubscriptionsById(baseReduxState),
   unread: baseReduxState.unread,
   theme: 'default',

src/autocomplete/StreamAutocomplete.js

@@ -6,7 +6,7 @@ import { FlatList } from 'react-native';
 
 import { useSelector } from '../react-redux';
 import { Popup } from '../common';
-import { getSubscribedStreams } from '../subscriptions/subscriptionSelectors';
+import { getSubscriptions } from '../directSelectors';
 import StreamItem from '../streams/StreamItem';
 
 type Props = $ReadOnly<{|
@@ -16,7 +16,7 @@ type Props = $ReadOnly<{|
 
 export default function StreamAutocomplete(props: Props): Node {
   const { filter, onAutocomplete } = props;
-  const subscriptions = useSelector(getSubscribedStreams);
+  const subscriptions = useSelector(getSubscriptions);
 
   const handleStreamItemAutocomplete = useCallback(
     (streamId: number, streamName: string): void => {

src/chat/ChatScreen.js

@@ -138,7 +138,7 @@ export default function ChatScreen(props: Props): Node {
         const content = editMessage.content !== message ? message : undefined;
         const subject = caseNarrowDefault(
           destinationNarrow,
-          { topic: (stream, topic) => (topic !== editMessage.topic ? topic : undefined) },
+          { topic: (streamId, topic) => (topic !== editMessage.topic ? topic : undefined) },
           () => undefined,
         );
 

src/chat/MarkAsReadButton.js

@@ -8,14 +8,14 @@ import { createStyleSheet } from '../styles';
 import { useSelector } from '../react-redux';
 import { ZulipButton } from '../common';
 import * as api from '../api';
-import { getAuth, getStreams, getOwnUserId } from '../selectors';
+import { getAuth, getOwnUserId } from '../selectors';
 import { getUnread, getUnreadIdsForPmNarrow } from '../unread/unreadModel';
 import {
   isHomeNarrow,
   isStreamNarrow,
   isTopicNarrow,
   isPmNarrow,
-  streamNameOfNarrow,
+  streamIdOfNarrow,
   topicOfNarrow,
 } from '../utils/narrow';
 
@@ -35,7 +35,6 @@ type Props = $ReadOnly<{|
 export default function MarkAsReadButton(props: Props): Node {
   const { narrow } = props;
   const auth = useSelector(getAuth);
-  const streams = useSelector(getStreams);
   const unread = useSelector(getUnread);
   const ownUserId = useSelector(getOwnUserId);
 
@@ -44,20 +43,12 @@ export default function MarkAsReadButton(props: Props): Node {
   }, [auth]);
 
   const markStreamAsRead = useCallback(() => {
-    const streamName = streamNameOfNarrow(narrow);
-    const stream = streams.find(s => s.name === streamName);
-    if (stream) {
-      api.markStreamAsRead(auth, stream.stream_id);
-    }
-  }, [auth, narrow, streams]);
+    api.markStreamAsRead(auth, streamIdOfNarrow(narrow));
+  }, [auth, narrow]);
 
   const markTopicAsRead = useCallback(() => {
-    const streamName = streamNameOfNarrow(narrow);
-    const stream = streams.find(s => s.name === streamName);
-    if (stream) {
-      api.markTopicAsRead(auth, stream.stream_id, topicOfNarrow(narrow));
-    }
-  }, [auth, narrow, streams]);
+    api.markTopicAsRead(auth, streamIdOfNarrow(narrow), topicOfNarrow(narrow));
+  }, [auth, narrow]);
 
   const markPmAsRead = useCallback(() => {
     // The message IDs come from our unread-messages data, which is

src/chat/__tests__/fetchingReducer-test.js

@@ -41,16 +41,17 @@ describe('fetchingReducer', () => {
         [HOME_NARROW_STR]: { older: false, newer: false },
       });
 
+      const narrow = streamNarrow(eg.stream.stream_id);
       const action = deepFreeze({
         type: MESSAGE_FETCH_START,
-        narrow: streamNarrow('some stream'),
+        narrow,
         numBefore: 10,
         numAfter: 0,
       });
 
       const expectedState = {
         [HOME_NARROW_STR]: { older: false, newer: false },
-        [keyFromNarrow(streamNarrow('some stream'))]: { older: true, newer: false },
+        [keyFromNarrow(narrow)]: { older: true, newer: false },
       };
 
       const newState = fetchingReducer(initialState, action);

src/chat/__tests__/narrowsReducer-test.js

@@ -27,9 +27,9 @@ import * as eg from '../../__tests__/lib/exampleData';
 describe('narrowsReducer', () => {
   const privateNarrowStr = keyFromNarrow(pm1to1NarrowFromUser(eg.otherUser));
   const groupNarrowStr = keyFromNarrow(pmNarrowFromUsersUnsafe([eg.otherUser, eg.thirdUser]));
-  const streamNarrowStr = keyFromNarrow(streamNarrow(eg.stream.name));
+  const streamNarrowStr = keyFromNarrow(streamNarrow(eg.stream.stream_id));
   const egTopic = eg.streamMessage().subject;
-  const topicNarrowStr = keyFromNarrow(topicNarrow(eg.stream.name, egTopic));
+  const topicNarrowStr = keyFromNarrow(topicNarrow(eg.stream.stream_id, egTopic));
 
   describe('EVENT_NEW_MESSAGE', () => {
     test('if not caught up in narrow, do not add message in home narrow', () => {

src/chat/__tests__/narrowsSelectors-test.js

@@ -146,7 +146,7 @@ describe('getShownMessagesForNarrow', () => {
   });
 
   describe('stream narrow', () => {
-    const narrow = streamNarrow(stream.name);
+    const narrow = streamNarrow(stream.stream_id);
     const makeState = extra => makeStateGeneral(message, narrow, extra);
     const shown = state => shownGeneral(state, narrow);
 
@@ -171,7 +171,7 @@ describe('getShownMessagesForNarrow', () => {
   });
 
   describe('topic narrow', () => {
-    const narrow = topicNarrow(stream.name, message.subject);
+    const narrow = topicNarrow(stream.stream_id, message.subject);
     const makeState = extra => makeStateGeneral(message, narrow, extra);
     const shown = state => shownGeneral(state, narrow);
 
@@ -306,26 +306,28 @@ describe('getStreamInNarrow', () => {
   });
 
   test('return subscription if stream in narrow is subscribed', () => {
-    const narrow = streamNarrow(stream1.name);
+    const narrow = streamNarrow(stream1.stream_id);
 
     expect(getStreamInNarrow(state, narrow)).toEqual(sub1);
   });
 
   test('return stream if stream in narrow is not subscribed', () => {
-    const narrow = streamNarrow(stream3.name);
+    const narrow = streamNarrow(stream3.stream_id);
 
     expect(getStreamInNarrow(state, narrow)).toEqual({ ...stream3, in_home_view: true });
   });
 
   test('return NULL_SUBSCRIPTION if stream in narrow is not valid', () => {
-    const narrow = streamNarrow(stream4.name);
+    const narrow = streamNarrow(stream4.stream_id);
 
     expect(getStreamInNarrow(state, narrow)).toEqual(NULL_SUBSCRIPTION);
   });
 
   test('return NULL_SUBSCRIPTION is narrow is not topic or stream', () => {
     expect(getStreamInNarrow(state, pm1to1NarrowFromUser(eg.otherUser))).toEqual(NULL_SUBSCRIPTION);
-    expect(getStreamInNarrow(state, topicNarrow(stream4.name, 'topic'))).toEqual(NULL_SUBSCRIPTION);
+    expect(getStreamInNarrow(state, topicNarrow(stream4.stream_id, 'topic'))).toEqual(
+      NULL_SUBSCRIPTION,
+    );
   });
 });
 
@@ -345,7 +347,7 @@ describe('isNarrowValid', () => {
     const state = eg.reduxState({
       streams: [stream],
     });
-    const narrow = streamNarrow(stream.name);
+    const narrow = streamNarrow(stream.stream_id);
 
     const result = isNarrowValid(state, narrow);
 
@@ -356,7 +358,7 @@ describe('isNarrowValid', () => {
     const state = eg.reduxState({
       streams: [],
     });
-    const narrow = streamNarrow('nonexisting');
+    const narrow = streamNarrow(eg.stream.stream_id);
 
     const result = isNarrowValid(state, narrow);
 
@@ -369,7 +371,7 @@ describe('isNarrowValid', () => {
     const state = eg.reduxState({
       streams: [stream],
     });
-    const narrow = topicNarrow(stream.name, 'topic does not matter');
+    const narrow = topicNarrow(stream.stream_id, 'topic does not matter');
 
     const result = isNarrowValid(state, narrow);
 

src/chat/narrowsSelectors.js

@@ -17,7 +17,6 @@ import {
   getSubscriptions,
   getMessages,
   getMute,
-  getStreams,
   getOutbox,
   getFlags,
 } from '../directSelectors';
@@ -28,13 +27,14 @@ import {
   isMessageInNarrow,
   caseNarrowDefault,
   keyFromNarrow,
-  streamNameOfNarrow,
   caseNarrow,
+  streamIdOfNarrow,
 } from '../utils/narrow';
 import { isTopicMuted } from '../mute/muteModel';
 import { streamNameOfStreamMessage } from '../utils/recipient';
 import { NULL_ARRAY, NULL_SUBSCRIPTION } from '../nullObjects';
 import * as logging from '../utils/logging';
+import { getStreamsById, getSubscriptionsById } from '../selectors';
 
 export const outboxMessagesForNarrow: Selector<$ReadOnlyArray<Outbox>, Narrow> = createSelector(
   (state, narrow) => narrow,
@@ -196,20 +196,20 @@ export const getLastMessageId = (state: PerAccountState, narrow: Narrow): number
 // TODO: clean up what this returns; possibly to just `Stream`
 export const getStreamInNarrow: Selector<Subscription | {| ...Stream, in_home_view: boolean |}, Narrow> = createSelector(
   (state, narrow) => narrow,
-  state => getSubscriptions(state),
-  state => getStreams(state),
+  state => getSubscriptionsById(state),
+  state => getStreamsById(state),
   (narrow, subscriptions, streams) => {
     if (!isStreamOrTopicNarrow(narrow)) {
       return NULL_SUBSCRIPTION;
     }
-    const streamName = streamNameOfNarrow(narrow);
+    const streamId = streamIdOfNarrow(narrow);
 
-    const subscription = subscriptions.find(x => x.name === streamName);
+    const subscription = subscriptions.get(streamId);
     if (subscription) {
       return subscription;
     }
 
-    const stream = streams.find(x => x.name === streamName);
+    const stream = streams.get(streamId);
     if (stream) {
       return {
         ...stream,
@@ -223,14 +223,14 @@ export const getStreamInNarrow: Selector<Subscription | {| ...Stream, in_home_vi
 
 export const isNarrowValid: Selector<boolean, Narrow> = createSelector(
   (state, narrow) => narrow,
-  state => getStreams(state),
+  state => getStreamsById(state),
   state => getAllUsersById(state),
   (narrow, streams, allUsersById) =>
     caseNarrowDefault(
       narrow,
       {
-        stream: streamName => streams.find(s => s.name === streamName) !== undefined,
-        topic: streamName => streams.find(s => s.name === streamName) !== undefined,
+        stream: streamId => streams.get(streamId) !== undefined,
+        topic: streamId => streams.get(streamId) !== undefined,
         pm: ids => ids.every(id => allUsersById.get(id) !== undefined),
       },
       () => true,