mute: Request and consume new user_topic format in API

In this commit, we start requesting the new `user_topic` format for
the data describing the "muted topics" feature.

On new servers, that causes the new format to appear, and the old to
disappear.  Describe the API changes, and start consuming the new
format.

This should have no user-visible effect -- currently the new format
describes exactly the same information as the old.  Taking the
client plus (a properly behaving) server as one system, this change
is therefore NFC.

We don't call the change NFC, though, because it does change the
client's behavior as witnessed by the server.

(This change does switch from stream names to stream IDs in the
API... but only for data that we get from the server through the
event system, not for data we send or receive in any request outside
that system.  That means that (unless affected by some server bug)
it's always working with a stream name-to-ID mapping that exactly
matches what the server had at the time it sent that data.  So even
if a stream gets renamed concurrently with getting some data here,
there's no race and no bug.

OTOH when we *change* this at the server on the user's behalf, that
does suffer from such a race.  But that part of the API was fixed
long ago to accept stream IDs; we already have a TODO-server
comment, at `api.setTopicMute`, to start relying on that update.)

Fixes: #5380

Author: gnprice

src/__tests__/lib/exampleData.js

@@ -737,9 +737,6 @@ export const action = Object.freeze({
       // InitialDataMessage
       max_message_id: 100,
 
-      // InitialDataMutedTopics
-      muted_topics: [],
-
       // InitialDataMutedUsers
       muted_users: [],
 
@@ -950,6 +947,9 @@ export const action = Object.freeze({
 
       // InitialDataUserStatus
       user_status: {},
+
+      // InitialDataUserTopic
+      user_topics: [],
     },
   }): RegisterCompleteAction),
 

src/actionTypes.js

@@ -75,6 +75,7 @@ import type {
   RestartEvent,
   UpdateMessageEvent,
   RealmUserUpdateEvent,
+  UserTopicEvent,
 } from './api/eventTypes';
 import type { MutedTopicTuple, PresenceSnapshot } from './api/apiTypes';
 import type { MessageMove } from './api/misc';
@@ -336,8 +337,9 @@ type GenericEventAction = $ReadOnly<{|
     | RestartEvent
     | RealmUpdateEvent
     | RealmUpdateDictEvent
+    | RealmUserUpdateEvent
     | UserSettingsUpdateEvent
-    | RealmUserUpdateEvent,
+    | UserTopicEvent,
 |}>;
 
 type EventNewMessageAction = $ReadOnly<{|

src/api/eventTypes.js

@@ -25,6 +25,7 @@ import type {
   UserStatusUpdate,
   UserSettings,
   ClientPresence,
+  UserTopic,
 } from './modelTypes';
 import type { RealmDataForUpdate } from './realmDataTypes';
 
@@ -69,6 +70,7 @@ export const EventTypes = keyMirror({
   user_group: null,
   user_settings: null,
   user_status: null,
+  user_topic: null,
 });
 
 export type EventType = $Keys<typeof EventTypes>;
@@ -449,3 +451,11 @@ export type RealmUserUpdateEvent = {|
         // avatar_url_medium: string,
       |},
 |};
+
+export type UserTopicEvent = {|
+  ...EventCommon,
+  +type: typeof EventTypes.user_topic,
+
+  // The event comprises the whole new state of the user-topic relationship.
+  ...UserTopic,
+|};

src/api/initialDataTypes.js

@@ -17,6 +17,7 @@ import type {
   UserId,
   UserSettings,
   UserStatusUpdate,
+  UserTopic,
 } from './modelTypes';
 import type {
   CreatePublicOrPrivateStreamPolicyT,
@@ -90,7 +91,9 @@ export type InitialDataMessage = $ReadOnly<{|
 |}>;
 
 export type InitialDataMutedTopics = $ReadOnly<{|
-  muted_topics: $ReadOnlyArray<MutedTopicTuple>,
+  /** Omitted by new servers that send `user_topics`; read that instead. */
+  // TODO(server-6.0): Remove; gone in FL 134 (given that we request `user_topic`).
+  muted_topics?: $ReadOnlyArray<MutedTopicTuple>,
 |}>;
 
 export type InitialDataMutedUsers = $ReadOnly<{|
@@ -674,6 +677,18 @@ export type InitialDataUserStatus = $ReadOnly<{|
   |}>,
 |}>;
 
+/** Initial data for the `user_topic` event type. */
+export type InitialDataUserTopic = {|
+  /**
+   * When absent (on older servers), read `muted_topics` instead.
+   *
+   * For user-topic pairs not found here, the value is implicitly a
+   * zero value: `visibility_policy` is `UserTopicVisibilityPolicy.None`.
+   */
+  // TODO(server-6.0): Introduced in FL 134.
+  +user_topics?: $ReadOnlyArray<UserTopic>,
+|};
+
 /**
  * The initial data snapshot sent on `/register`.
  *
@@ -718,6 +733,7 @@ export type InitialData = {|
   ...InitialDataUpdateMessageFlags,
   ...InitialDataUserSettings,
   ...InitialDataUserStatus,
+  ...InitialDataUserTopic,
 |};
 
 /**

src/api/registerForEvents.js

@@ -149,7 +149,7 @@ export default async (
       'alert_words',
       'custom_profile_fields',
       'message',
-      'muted_topics',
+      'muted_topics', // TODO(server-6.0): Stop requesting, in favor of user_topic
       'muted_users',
       'presence',
       'realm',
@@ -170,6 +170,7 @@ export default async (
       'update_message_flags',
       'user_settings',
       'user_status',
+      'user_topic',
       'zulip_version',
     ]),
 

src/events/eventToAction.js

@@ -168,6 +168,7 @@ export default (state: PerAccountState, event: $FlowFixMe): EventAction | null =
     case 'custom_profile_fields':
     case 'stream':
     case 'user_settings':
+    case 'user_topic':
       return {
         type: EVENT,
         event,

src/mute/__tests__/mute-testlib.js

@@ -2,20 +2,27 @@
 
 import { reducer } from '../muteModel';
 import type { MuteState } from '../muteModelTypes';
-import type { Stream } from '../../api/modelTypes';
+import { type Stream, type UserTopic, UserTopicVisibilityPolicy } from '../../api/modelTypes';
 import * as eg from '../../__tests__/lib/exampleData';
-import { EVENT_MUTED_TOPICS } from '../../actionConstants';
 
-export const makeMuteState = (data: $ReadOnlyArray<[Stream, string]>): MuteState => {
-  const streams = new Set(data.map(([stream, topic]) => stream));
+export const makeUserTopic = (
+  stream: Stream,
+  topic: string,
+  visibility_policy: UserTopicVisibilityPolicy,
+): UserTopic => ({
+  stream_id: stream.stream_id,
+  topic_name: topic,
+  visibility_policy,
+  last_updated: 12345, // arbitrary value; we ignore last_updated here
+});
 
-  return reducer(
+export const makeMuteState = (data: $ReadOnlyArray<[Stream, string]>): MuteState =>
+  reducer(
     undefined,
-    {
-      type: EVENT_MUTED_TOPICS,
-      id: -1,
-      muted_topics: data.map(([stream, topic]) => [stream.name, topic]),
-    },
-    eg.reduxState({ streams: [...streams] }),
+    eg.mkActionRegisterComplete({
+      user_topics: data.map(([stream, topic]) =>
+        makeUserTopic(stream, topic, UserTopicVisibilityPolicy.Muted),
+      ),
+    }),
+    eg.reduxState(),
   );
-};

src/mute/__tests__/muteModel-test.js

@@ -3,11 +3,12 @@ import deepFreeze from 'deep-freeze';
 
 import fullReducer from '../../boot/reducers';
 import { getMute, getTopicVisibilityPolicy, isTopicMuted, reducer } from '../muteModel';
-import { EVENT_MUTED_TOPICS } from '../../actionConstants';
+import { EVENT, EVENT_MUTED_TOPICS } from '../../actionConstants';
 import * as eg from '../../__tests__/lib/exampleData';
-import { makeMuteState } from './mute-testlib';
+import { makeMuteState, makeUserTopic } from './mute-testlib';
 import { tryGetActiveAccountState } from '../../selectors';
 import { UserTopicVisibilityPolicy } from '../../api/modelTypes';
+import { EventTypes } from '../../api/eventTypes';
 
 /* eslint jest/expect-expect: ["error", { "assertFunctionNames": ["expect", "check"] }] */
 
@@ -53,28 +54,55 @@ describe('getters', () => {
 
 describe('reducer', () => {
   describe('REGISTER_COMPLETE', () => {
-    test('when mute data is provided init state with it: local', () => {
-      const action = eg.mkActionRegisterComplete({ muted_topics: [[eg.stream.name, 'topic']] });
+    test('in modern user_topics format: unit test', () => {
+      const action = eg.mkActionRegisterComplete({
+        user_topics: [makeUserTopic(eg.stream, 'topic', UserTopicVisibilityPolicy.Muted)],
+      });
       expect(reducer(initialState, action, eg.plusReduxState)).toEqual(
         makeMuteState([[eg.stream, 'topic']]),
       );
     });
 
-    test('when mute data is provided init state with it: end-to-end', () => {
+    test('in modern user_topics format: end-to-end test', () => {
       const action = eg.mkActionRegisterComplete({
         streams: [eg.stream],
         subscriptions: [eg.subscription],
+        user_topics: [makeUserTopic(eg.stream, 'topic', UserTopicVisibilityPolicy.Muted)],
+      });
+      const newState = tryGetActiveAccountState(fullReducer(eg.plusReduxState, action));
+      expect(newState).toBeTruthy();
+      expect(newState && getMute(newState)).toEqual(makeMuteState([[eg.stream, 'topic']]));
+    });
+
+    test('in old muted_topics format: unit test', () => {
+      const action = eg.mkActionRegisterComplete({
         muted_topics: [[eg.stream.name, 'topic']],
+        user_topics: undefined,
+      });
+      expect(reducer(initialState, action, eg.plusReduxState)).toEqual(
+        makeMuteState([[eg.stream, 'topic']]),
+      );
+    });
+
+    test('in old muted_topics format: end-to-end test', () => {
+      const action = eg.mkActionRegisterComplete({
+        streams: [eg.stream],
+        subscriptions: [eg.subscription],
+        muted_topics: [[eg.stream.name, 'topic']],
+        user_topics: undefined,
       });
       const newState = tryGetActiveAccountState(fullReducer(eg.plusReduxState, action));
       expect(newState).toBeTruthy();
       expect(newState && getMute(newState)).toEqual(makeMuteState([[eg.stream, 'topic']]));
     });
 
     // TODO(#5102): Delete; see comment on implementation.
-    test('when no `mute` data is given reset state', () => {
+    test('in ancient no-muted-topics format', () => {
       const state = makeMuteState([[eg.stream, 'topic']]);
-      const action = eg.mkActionRegisterComplete({ muted_topics: [] });
+      const action = eg.mkActionRegisterComplete({
+        muted_topics: undefined,
+        user_topics: undefined,
+      });
       expect(reducer(state, action, eg.plusReduxState)).toEqual(initialState);
     });
   });
@@ -86,7 +114,91 @@ describe('reducer', () => {
     });
   });
 
-  describe('EVENT_MUTED_TOPICS', () => {
+  describe('EVENT > user_topic', () => {
+    function mkAction(userTopic) {
+      return { type: EVENT, event: { id: 0, type: EventTypes.user_topic, ...userTopic } };
+    }
+
+    function check(state, userTopic, expected) {
+      expect(reducer(state, mkAction(userTopic), eg.plusReduxState)).toEqual(expected);
+    }
+
+    test('add with new stream', () => {
+      check(
+        initialState,
+        makeUserTopic(eg.stream, 'topic', UserTopicVisibilityPolicy.Muted),
+        makeMuteState([[eg.stream, 'topic']]),
+      );
+    });
+
+    test('add in existing stream', () => {
+      check(
+        makeMuteState([[eg.stream, 'topic']]),
+        makeUserTopic(eg.stream, 'other topic', UserTopicVisibilityPolicy.Muted),
+        makeMuteState([
+          [eg.stream, 'topic'],
+          [eg.stream, 'other topic'],
+        ]),
+      );
+    });
+
+    test('remove, with others in stream', () => {
+      check(
+        makeMuteState([
+          [eg.stream, 'topic'],
+          [eg.stream, 'other topic'],
+        ]),
+        makeUserTopic(eg.stream, 'other topic', UserTopicVisibilityPolicy.None),
+        makeMuteState([[eg.stream, 'topic']]),
+      );
+    });
+
+    test('remove, as last in stream', () => {
+      check(
+        makeMuteState([[eg.stream, 'topic']]),
+        makeUserTopic(eg.stream, 'topic', UserTopicVisibilityPolicy.None),
+        initialState,
+      );
+    });
+
+    describe('redundantly after EVENT_MUTED_TOPICS', () => {
+      // The server may send both muted_topics events and user_topic events,
+      // because we don't set event_types in our /register request:
+      //   https://github.com/zulip/zulip/pull/21251#issuecomment-1133466148
+      // So we may get one of these after a muted_topics event has already
+      // set the new state.
+      //
+      // (Or we might get user_topic first and then muted_topics, but that
+      // doesn't require any testing of its own -- when handling the
+      // muted_topics event it doesn't matter what the previous state was.)
+
+      test('add', () => {
+        check(
+          makeMuteState([[eg.stream, 'topic']]),
+          makeUserTopic(eg.stream, 'topic', UserTopicVisibilityPolicy.Muted),
+          makeMuteState([[eg.stream, 'topic']]),
+        );
+      });
+
+      test('remove, leaving others in stream', () => {
+        check(
+          makeMuteState([[eg.stream, 'topic']]),
+          makeUserTopic(eg.stream, 'other topic', UserTopicVisibilityPolicy.None),
+          makeMuteState([[eg.stream, 'topic']]),
+        );
+      });
+
+      test('remove, as last in stream', () => {
+        check(
+          makeMuteState([]),
+          makeUserTopic(eg.stream, 'topic', UserTopicVisibilityPolicy.None),
+          makeMuteState([]),
+        );
+      });
+    });
+  });
+
+  describe('EVENT_MUTED_TOPICS (legacy)', () => {
     test('appends and test a new muted topic', () => {
       const action = deepFreeze({
         type: EVENT_MUTED_TOPICS,