mute: Maintain muted-topics state by stream ID, not name

src/__tests__/lib/exampleData.js

@@ -17,6 +17,7 @@ import type {
   UserId,
 } from '../../api/modelTypes';
 import { makeUserId } from '../../api/idTypes';
+import type { InitialData } from '../../api/apiTypes';
 import type {
   AccountSwitchAction,
   LoginSuccessAction,
@@ -34,6 +35,7 @@ import type {
 import type { Auth, Account, StreamOutbox } from '../../types';
 import { dubJointState } from '../../reduxTypes';
 import { UploadedAvatarURL } from '../../utils/avatar';
+import type { AvatarURL } from '../../utils/avatar';
 import { ZulipVersion } from '../../utils/zulipVersion';
 import {
   ACCOUNT_SWITCH,
@@ -48,6 +50,7 @@ import { authOfAccount } from '../../account/accountMisc';
 import { HOME_NARROW } from '../../utils/narrow';
 import type { BackgroundData } from '../../webview/MessageList';
 import { getSettings, getStreamsById, getSubscriptionsById } from '../../selectors';
+import { getGlobalSettings } from '../../directSelectors';
 
 /* ========================================================================
  * Utilities
@@ -125,29 +128,34 @@ export const diverseCharacters: string =
 type UserOrBotPropertiesArgs = {|
   name?: string,
   user_id?: number, // accept a plain number, for convenience in tests
+  email?: string,
+  avatar_url?: AvatarURL,
 |};
 
 const randUserId: () => UserId = (mk => () => makeUserId(mk()))(
   makeUniqueRandInt('user IDs', 10000),
 );
-const userOrBotProperties = ({ name: _name, user_id }: UserOrBotPropertiesArgs) => {
-  const name = _name ?? randString();
+const userOrBotProperties = (args: UserOrBotPropertiesArgs) => {
+  const name = args.name ?? randString();
+  const email = args.email ?? `${name}@example.org`;
   const capsName = name.substring(0, 1).toUpperCase() + name.substring(1);
   return deepFreeze({
     // Internally the UploadedAvatarURL mutates itself for memoization.
     // That conflicts with the deepFreeze we do for tests; so construct it
     // here with a full-blown URL object in the first place to prevent that.
-    avatar_url: new UploadedAvatarURL(new URL(`https://zulip.example.org/yo/avatar-${name}.png`)),
+    avatar_url:
+      args.avatar_url
+      ?? new UploadedAvatarURL(new URL(`https://zulip.example.org/yo/avatar-${name}.png`)),
 
     date_joined: `2014-04-${randInt(30)
       .toString()
       .padStart(2, '0')}`,
 
-    email: `${name}@example.org`,
+    email,
     full_name: `${capsName} User`,
     is_admin: false,
     timezone: 'UTC',
-    user_id: user_id != null ? makeUserId(user_id) : randUserId(),
+    user_id: args.user_id != null ? makeUserId(args.user_id) : randUserId(),
   });
 };
 
@@ -246,14 +254,13 @@ export const crossRealmBot: CrossRealmBot = makeCrossRealmBot({ name: 'bot' });
 
 const randStreamId: () => number = makeUniqueRandInt('stream IDs', 1000);
 export const makeStream = (
-  args: {| name?: string, description?: string |} = Object.freeze({}),
+  args: {| stream_id?: number, name?: string, description?: string |} = Object.freeze({}),
 ): Stream => {
   const name = args.name ?? randString();
-  const description = args.description ?? `On the ${randString()} of ${name}`;
   return deepFreeze({
-    stream_id: randStreamId(),
+    stream_id: args.stream_id ?? randStreamId(),
     name,
-    description,
+    description: args.description ?? `On the ${randString()} of ${name}`,
     invite_only: false,
     is_announcement_only: false,
     history_public_to_subscribers: true,
@@ -271,14 +278,19 @@ export const otherStream: Stream = makeStream({
 
 /** A subscription, by default to eg.stream. */
 export const makeSubscription = (
-  args: {| stream?: Stream, in_home_view?: boolean |} = Object.freeze({}),
+  args: {|
+    stream?: Stream,
+    in_home_view?: boolean,
+    pin_to_top?: boolean,
+    color?: string,
+  |} = Object.freeze({}),
 ): Subscription => {
-  const { stream: streamInner = stream, in_home_view } = args;
+  const { stream: streamInner = stream } = args;
   return deepFreeze({
     ...streamInner,
-    color: '#123456',
-    in_home_view: in_home_view ?? true,
-    pin_to_top: false,
+    color: args.color ?? '#123456',
+    in_home_view: args.in_home_view ?? true,
+    pin_to_top: args.pin_to_top ?? false,
     audible_notifications: false,
     desktop_notifications: false,
     email_address: '??? make this value representative before using in a test :)',
@@ -617,7 +629,15 @@ export const action = Object.freeze({
     apiKey: selfAccount.apiKey,
   }): LoginSuccessAction),
 
-  /** Beware! Data contained may not be representative. */
+  /**
+   * A minimal well-typed REGISTER_COMPLETE action.
+   *
+   * Beware!  The data here may not be representative.  Generally each test
+   * should specify the particular properties that it cares about.
+   *
+   * See also `eg.mkActionRegisterComplete`, for combining this data with
+   * test-specific other data.
+   */
   register_complete: (deepFreeze({
     type: REGISTER_COMPLETE,
     data: {
@@ -767,6 +787,7 @@ export const action = Object.freeze({
       user_status: {},
     },
   }): RegisterCompleteAction),
+
   message_fetch_start: (deepFreeze({
     type: MESSAGE_FETCH_START,
     narrow: HOME_NARROW,
@@ -805,6 +826,30 @@ export const action = Object.freeze({
  * each test might as well define the action values it needs directly.
  */
 
+/**
+ * A REGISTER_COMPLETE action.
+ *
+ * The result will be based on `eg.action.register_complete`, but with the
+ * additional data given in the argument.
+ */
+export const mkActionRegisterComplete = (extra: {|
+  ...$Rest<InitialData, { ... }>,
+  unread_msgs?: $Rest<$PropertyType<InitialData, 'unread_msgs'>, { ... }>,
+|}): PerAccountAction => {
+  const { unread_msgs, ...rest } = extra;
+  return deepFreeze({
+    ...action.register_complete,
+    data: {
+      ...action.register_complete.data,
+      ...rest,
+      unread_msgs: {
+        ...action.register_complete.data.unread_msgs,
+        ...unread_msgs,
+      },
+    },
+  });
+};
+
 /**
  * An EVENT_NEW_MESSAGE action.
  *
@@ -836,20 +881,20 @@ export const mkActionEventNewMessage = (
  */
 
 export const backgroundData: BackgroundData = deepFreeze({
-  alertWords: [],
-  allImageEmojiById: action.register_complete.data.realm_emoji,
+  alertWords: baseReduxState.alertWords,
+  allImageEmojiById: {}, // in reality would also have :zulip:
   auth: selfAuth,
   debug: baseReduxState.session.debug,
   doNotMarkMessagesAsRead: baseReduxState.settings.doNotMarkMessagesAsRead,
   flags: baseReduxState.flags,
   allUsersById: new Map([[selfUser.user_id, selfUser]]),
-  mute: [],
-  mutedUsers: Immutable.Map(),
+  mute: baseReduxState.mute,
+  mutedUsers: baseReduxState.mutedUsers,
   ownUser: selfUser,
   streams: getStreamsById(baseReduxState),
   subscriptions: getSubscriptionsById(baseReduxState),
   unread: baseReduxState.unread,
-  theme: 'default',
-  twentyFourHourTime: false,
+  theme: getGlobalSettings(baseReduxState).theme,
+  twentyFourHourTime: baseReduxState.realm.twentyFourHourTime,
   userSettingStreamNotification: getSettings(baseReduxState).streamNotification,
 });

src/account/__tests__/accountsReducer-test.js

@@ -25,10 +25,7 @@ describe('accountsReducer', () => {
       expect(
         accountsReducer(
           deepFreeze([account1, account2, account3]),
-          deepFreeze({
-            ...eg.action.register_complete,
-            data: { ...eg.action.register_complete.data, zulip_version: newZulipVersion.raw() },
-          }),
+          eg.mkActionRegisterComplete({ zulip_version: newZulipVersion.raw() }),
         ),
       ).toEqual([{ ...account1, zulipVersion: newZulipVersion }, account2, account3]);
     });
@@ -38,10 +35,7 @@ describe('accountsReducer', () => {
       expect(
         accountsReducer(
           deepFreeze([account1, account2, account3]),
-          deepFreeze({
-            ...eg.action.register_complete,
-            data: { ...eg.action.register_complete.data, user_id: newUserId },
-          }),
+          eg.mkActionRegisterComplete({ user_id: newUserId }),
         ),
       ).toEqual([{ ...account1, userId: newUserId }, account2, account3]);
     });
@@ -51,12 +45,8 @@ describe('accountsReducer', () => {
       expect(
         accountsReducer(
           deepFreeze([account1, account2, account3]),
-          deepFreeze({
-            ...eg.action.register_complete,
-            data: {
-              ...eg.action.register_complete.data,
-              zulip_feature_level: newZulipFeatureLevel,
-            },
+          eg.mkActionRegisterComplete({
+            zulip_feature_level: newZulipFeatureLevel,
           }),
         ),
       ).toEqual([{ ...account1, zulipFeatureLevel: newZulipFeatureLevel }, account2, account3]);

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

@@ -10,6 +10,7 @@ import {
 } from '../index';
 import { reducer } from '../../unread/unreadModel';
 import { initialState } from '../../unread/__tests__/unread-testlib';
+import { makeMuteState } from '../../mute/__tests__/mute-testlib';
 
 const buttonTitles = buttons => buttons.map(button => button.title);
 
@@ -82,7 +83,7 @@ describe('constructTopicActionButtons', () => {
   });
 
   test('show Unmute topic option if topic is muted', () => {
-    const mute = deepFreeze([[stream.name, topic]]);
+    const mute = makeMuteState([[stream, topic]]);
     const buttons = constructTopicActionButtons({
       backgroundData: { ...eg.backgroundData, streams, mute },
       streamId,
@@ -93,7 +94,7 @@ describe('constructTopicActionButtons', () => {
 
   test('show mute topic option if topic is not muted', () => {
     const buttons = constructTopicActionButtons({
-      backgroundData: { ...eg.backgroundData, streams, mute: [] },
+      backgroundData: { ...eg.backgroundData, streams, mute: makeMuteState([]) },
       streamId,
       topic,
     });

src/action-sheets/index.js

@@ -344,13 +344,12 @@ export const constructStreamActionButtons = ({
 };
 
 export const constructTopicActionButtons = ({
-  backgroundData: { mute, ownUser, streams, subscriptions, unread },
+  backgroundData: { mute, ownUser, subscriptions, unread },
   streamId,
   topic,
 }: {|
   backgroundData: $ReadOnly<{
     mute: MuteState,
-    streams: Map<number, Stream>,
     subscriptions: Map<number, Subscription>,
     unread: UnreadState,
     ownUser: User,
@@ -367,9 +366,7 @@ export const constructTopicActionButtons = ({
   if (unreadCount > 0) {
     buttons.push(markTopicAsRead);
   }
-  const stream = streams.get(streamId);
-  invariant(stream !== undefined, 'Stream with provided streamId not found.');
-  if (isTopicMuted(stream.name, topic, mute)) {
+  if (isTopicMuted(streamId, topic, mute)) {
     buttons.push(unmuteTopic);
   } else {
     buttons.push(muteTopic);

src/actionTypes.js

@@ -70,6 +70,7 @@ import type {
   SubmessageEvent,
   RestartEvent,
 } from './api/eventTypes';
+import type { MutedTopicTuple } from './api/apiTypes';
 
 import type {
   Orientation,
@@ -90,7 +91,6 @@ import type {
   RealmEmojiById,
   GlobalSettingsState,
   CaughtUpState,
-  MuteState,
   AlertWordsState,
   UserId,
   UserStatusEvent,
@@ -460,7 +460,7 @@ type EventUserUpdateAction = $ReadOnly<{|
 type EventMutedTopicsAction = $ReadOnly<{|
   ...ServerEvent,
   type: typeof EVENT_MUTED_TOPICS,
-  muted_topics: MuteState,
+  muted_topics: $ReadOnlyArray<MutedTopicTuple>,
 |}>;
 
 type EventMutedUsersAction = $ReadOnly<{|

src/api/initialDataTypes.js

@@ -2,6 +2,7 @@
 
 import type {
   CrossRealmBot,
+  MutedTopicTuple,
   MutedUser,
   RealmEmojiById,
   RealmFilter,
@@ -66,10 +67,8 @@ export type InitialDataMessage = $ReadOnly<{|
   max_message_id: number,
 |}>;
 
-export type MuteTuple = [string, string];
-
 export type InitialDataMutedTopics = $ReadOnly<{|
-  muted_topics: $ReadOnlyArray<MuteTuple>,
+  muted_topics: $ReadOnlyArray<MutedTopicTuple>,
 |}>;
 
 export type InitialDataMutedUsers = $ReadOnly<{|

src/api/modelTypes.js

@@ -329,6 +329,18 @@ export type Topic = $ReadOnly<{|
   max_id: number,
 |}>;
 
+/**
+ * A muted topic.
+ *
+ * Found in the initial data, and in `muted_topics` events.
+ *
+ * The elements are the stream name, then topic, then possibly timestamp.
+ */
+// Server issue for using stream IDs (#3918) for muted topics, not names:
+//   https://github.com/zulip/zulip/issues/21015
+// TODO(server-3.0): Simplify away the no-timestamp version, new in FL 1.
+export type MutedTopicTuple = [string, string] | [string, string, number];
+
 //
 //
 //